diff --git a/nifi-bootstrap/pom.xml b/nifi-bootstrap/pom.xml index c92a5fa100..d1bfb0998d 100644 --- a/nifi-bootstrap/pom.xml +++ b/nifi-bootstrap/pom.xml @@ -73,5 +73,10 @@ language governing permissions and limitations under the License. --> jna-platform 4.4.0 + + ch.qos.logback + logback-classic + test + diff --git a/nifi-bootstrap/src/test/java/org/apache/nifi/bootstrap/http/TestHttpNotificationServiceSSL.java b/nifi-bootstrap/src/test/java/org/apache/nifi/bootstrap/http/TestHttpNotificationServiceSSL.java index 5ab8cab496..2f54f1cec4 100644 --- a/nifi-bootstrap/src/test/java/org/apache/nifi/bootstrap/http/TestHttpNotificationServiceSSL.java +++ b/nifi-bootstrap/src/test/java/org/apache/nifi/bootstrap/http/TestHttpNotificationServiceSSL.java @@ -16,13 +16,21 @@ */ package org.apache.nifi.bootstrap.http; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; import okhttp3.mockwebserver.MockWebServer; +import org.apache.nifi.bootstrap.NotificationServiceManager; import org.apache.nifi.security.util.SslContextFactory; -import org.junit.AfterClass; -import org.junit.BeforeClass; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; import org.mockito.internal.util.io.IOUtil; +import org.slf4j.LoggerFactory; +import org.xml.sax.SAXException; +import ch.qos.logback.classic.Logger; import javax.net.ssl.SSLContext; +import javax.xml.parsers.ParserConfigurationException; import java.io.File; import java.io.IOException; import java.nio.file.Files; @@ -32,11 +40,47 @@ import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; import java.security.UnrecoverableKeyException; import java.security.cert.CertificateException; +import java.util.List; -public class TestHttpNotificationServiceSSL extends TestHttpNotificationServiceCommon{ +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +public class TestHttpNotificationServiceSSL extends TestHttpNotificationServiceCommon { static final String CONFIGURATION_FILE_TEXT = "\n"+ + "\n"+ + " \n"+ + " http-notification\n"+ + " org.apache.nifi.bootstrap.notification.http.HttpNotificationService\n"+ + " ${test.server}\n"+ + " ./src/test/resources/truststore.jks\n"+ + " JKS\n"+ + " passwordpassword\n"+ + " ./src/test/resources/keystore.jks\n"+ + " JKS\n"+ + " passwordpassword\n"+ + " passwordpassword\n"+ + " ${literal('testing')}\n"+ + " \n"+ + ""; + + static final String CONFIGURATION_FILE_TEXT_NO_KEYSTORE_PASSWORD = "\n"+ + "\n"+ + " \n"+ + " http-notification\n"+ + " org.apache.nifi.bootstrap.notification.http.HttpNotificationService\n"+ + " ${test.server}\n"+ + " ./src/test/resources/truststore.jks\n"+ + " JKS\n"+ + " passwordpassword\n"+ + " ./src/test/resources/keystore.jks\n"+ + " JKS\n"+ + " passwordpassword\n"+ + " ${literal('testing')}\n"+ + " \n"+ + ""; + + static final String CONFIGURATION_FILE_TEXT_NO_KEY_PASSWORD = "\n"+ "\n"+ " \n"+ " http-notification\n"+ @@ -52,9 +96,42 @@ public class TestHttpNotificationServiceSSL extends TestHttpNotificationService " \n"+ ""; + static final String CONFIGURATION_FILE_TEXT_BLANK_KEY_PASSWORD = "\n"+ + "\n"+ + " \n"+ + " http-notification\n"+ + " org.apache.nifi.bootstrap.notification.http.HttpNotificationService\n"+ + " ${test.server}\n"+ + " ./src/test/resources/truststore.jks\n"+ + " JKS\n"+ + " passwordpassword\n"+ + " ./src/test/resources/keystore.jks\n"+ + " JKS\n"+ + " passwordpassword\n"+ + " \n"+ + " ${literal('testing')}\n"+ + " \n"+ + ""; - @BeforeClass - public static void startServer() throws IOException, UnrecoverableKeyException, CertificateException, NoSuchAlgorithmException, KeyStoreException, KeyManagementException { + static final String CONFIGURATION_FILE_TEXT_BLANK_KEYSTORE_PASSWORD = "\n"+ + "\n"+ + " \n"+ + " http-notification\n"+ + " org.apache.nifi.bootstrap.notification.http.HttpNotificationService\n"+ + " ${test.server}\n"+ + " ./src/test/resources/truststore.jks\n"+ + " JKS\n"+ + " passwordpassword\n"+ + " ./src/test/resources/keystore.jks\n"+ + " JKS\n"+ + " \n"+ + " passwordpassword\n"+ + " ${literal('testing')}\n"+ + " \n"+ + ""; + + @Before + public void startServer() throws IOException, UnrecoverableKeyException, CertificateException, NoSuchAlgorithmException, KeyStoreException, KeyManagementException { tempConfigFilePath = "./target/TestHttpNotificationService-config.xml"; Files.deleteIfExists(Paths.get(tempConfigFilePath)); @@ -78,10 +155,109 @@ public class TestHttpNotificationServiceSSL extends TestHttpNotificationService IOUtil.writeText(configFileOutput, new File(tempConfigFilePath)); } - @AfterClass - public static void shutdownServer() throws IOException { + @After + public void shutdownServer() throws IOException { Files.deleteIfExists(Paths.get(tempConfigFilePath)); mockWebServer.shutdown(); } + @Test + public void testStartNotificationSucceedsNoKeystorePasswd() throws ParserConfigurationException, SAXException, IOException { + Logger notificationServiceLogger = (Logger) LoggerFactory.getLogger(NotificationServiceManager.class); + ListAppender listAppender = new ListAppender<>(); + listAppender.start(); + notificationServiceLogger.addAppender(listAppender); + + String configFileOutput = CONFIGURATION_FILE_TEXT_NO_KEYSTORE_PASSWORD.replace("${test.server}", String.valueOf(mockWebServer.url("/"))); + IOUtil.writeText(configFileOutput, new File(tempConfigFilePath)); + + NotificationServiceManager notificationServiceManager = new NotificationServiceManager(); + notificationServiceManager.setMaxNotificationAttempts(1); + notificationServiceManager.loadNotificationServices(new File(tempConfigFilePath)); + + List logsList = listAppender.list; + boolean notificationServiceFailed = false; + for(ILoggingEvent logMessage : logsList) { + if(logMessage.getFormattedMessage().contains("is not valid for the following reasons")) { + notificationServiceFailed = true; + } + } + + assertFalse(notificationServiceFailed); + } + + @Test + public void testStartNotificationSucceedsNoKeyPasswd() throws ParserConfigurationException, SAXException, IOException { + Logger notificationServiceLogger = (Logger) LoggerFactory.getLogger(NotificationServiceManager.class); + ListAppender listAppender = new ListAppender<>(); + listAppender.start(); + notificationServiceLogger.addAppender(listAppender); + + String configFileOutput = CONFIGURATION_FILE_TEXT_NO_KEY_PASSWORD.replace("${test.server}", String.valueOf(mockWebServer.url("/"))); + IOUtil.writeText(configFileOutput, new File(tempConfigFilePath)); + + NotificationServiceManager notificationServiceManager = new NotificationServiceManager(); + notificationServiceManager.setMaxNotificationAttempts(1); + notificationServiceManager.loadNotificationServices(new File(tempConfigFilePath)); + + List logsList = listAppender.list; + boolean notificationServiceFailed = false; + for(ILoggingEvent logMessage : logsList) { + if(logMessage.getFormattedMessage().contains("is not valid for the following reasons")) { + notificationServiceFailed = true; + } + } + + assertFalse(notificationServiceFailed); + } + + @Test + public void testStartNotificationFailsBlankKeystorePasswdCorrectKeypasswd() throws ParserConfigurationException, SAXException, IOException { + Logger notificationServiceLogger = (Logger) LoggerFactory.getLogger(NotificationServiceManager.class); + ListAppender listAppender = new ListAppender<>(); + listAppender.start(); + notificationServiceLogger.addAppender(listAppender); + + String configFileOutput = CONFIGURATION_FILE_TEXT_BLANK_KEYSTORE_PASSWORD.replace("${test.server}", String.valueOf(mockWebServer.url("/"))); + IOUtil.writeText(configFileOutput, new File(tempConfigFilePath)); + + NotificationServiceManager notificationServiceManager = new NotificationServiceManager(); + notificationServiceManager.setMaxNotificationAttempts(1); + notificationServiceManager.loadNotificationServices(new File(tempConfigFilePath)); + + List logsList = listAppender.list; + boolean notificationServiceFailed = false; + for(ILoggingEvent logMessage : logsList) { + if(logMessage.getFormattedMessage().contains("'Keystore Password' validated against '' is invalid because Keystore Password cannot be empty")) { + notificationServiceFailed = true; + } + } + + assertTrue(notificationServiceFailed); + } + + @Test + public void testStartNotificationFailsCorrectKeystorePasswdBlankKeypasswd() throws ParserConfigurationException, SAXException, IOException { + Logger notificationServiceLogger = (Logger) LoggerFactory.getLogger(NotificationServiceManager.class); + ListAppender listAppender = new ListAppender<>(); + listAppender.start(); + notificationServiceLogger.addAppender(listAppender); + + String configFileOutput = CONFIGURATION_FILE_TEXT_BLANK_KEY_PASSWORD.replace("${test.server}", String.valueOf(mockWebServer.url("/"))); + IOUtil.writeText(configFileOutput, new File(tempConfigFilePath)); + + NotificationServiceManager notificationServiceManager = new NotificationServiceManager(); + notificationServiceManager.setMaxNotificationAttempts(1); + notificationServiceManager.loadNotificationServices(new File(tempConfigFilePath)); + + List logsList = listAppender.list; + boolean notificationServiceFailed = false; + for(ILoggingEvent logMessage : logsList) { + if(logMessage.getFormattedMessage().contains("'Key Password' validated against '' is invalid because Key Password cannot be empty")) { + notificationServiceFailed = true; + } + } + + assertTrue(notificationServiceFailed); + } } diff --git a/nifi-docs/src/main/asciidoc/administration-guide.adoc b/nifi-docs/src/main/asciidoc/administration-guide.adoc index 5c90725396..c97aa896a5 100644 --- a/nifi-docs/src/main/asciidoc/administration-guide.adoc +++ b/nifi-docs/src/main/asciidoc/administration-guide.adoc @@ -2252,8 +2252,9 @@ It has the following properties available: |`Truststore Type`||The Type of the Truststore. Either `JKS` or `PKCS12` |`Truststore Password`||The password for the Truststore |`Keystore Filename`||The fully-qualified filename of the Keystore -|`Keystore Type`||The password for the Keystore -|`Keystore Password`||The password for the key. If this is not specified, but the Keystore Filename, Password, and Type are specified, then the Keystore Password will be assumed to be the same as the Key Password. +|`Keystore Type`||The Type of the Keystore. Either `JKS` or `PKCS12` +|`Keystore Password`||The password for the Keystore +|`Key Password`||The password for the key. If this is not specified, but the Keystore Filename, Password, and Type are specified, then the Key Password will be assumed to be the same as the Keystore Password. |`SSL Protocol`||The algorithm to use for this SSL context. This can either be `SSL` or `TLS`. |==== diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/coordination/http/replication/okhttp/OkHttpReplicationClient.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/coordination/http/replication/okhttp/OkHttpReplicationClient.java index 2352026d1d..ff63836681 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/coordination/http/replication/okhttp/OkHttpReplicationClient.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/coordination/http/replication/okhttp/OkHttpReplicationClient.java @@ -334,11 +334,11 @@ public class OkHttpReplicationClient implements HttpReplicationClient { try { Tuple sslContextTuple = createTrustSslContextWithTrustManagers( properties.getProperty(NiFiProperties.SECURITY_KEYSTORE), - properties.getProperty(NiFiProperties.SECURITY_KEYSTORE_PASSWD) != null ? properties.getProperty(NiFiProperties.SECURITY_KEYSTORE_PASSWD).toCharArray() : null, - properties.getProperty(NiFiProperties.SECURITY_KEY_PASSWD) != null ? properties.getProperty(NiFiProperties.SECURITY_KEY_PASSWD).toCharArray() : null, + StringUtils.isNotBlank(properties.getProperty(NiFiProperties.SECURITY_KEYSTORE_PASSWD)) ? properties.getProperty(NiFiProperties.SECURITY_KEYSTORE_PASSWD).toCharArray() : null, + StringUtils.isNotBlank(properties.getProperty(NiFiProperties.SECURITY_KEY_PASSWD)) ? properties.getProperty(NiFiProperties.SECURITY_KEY_PASSWD).toCharArray() : null, properties.getProperty(NiFiProperties.SECURITY_KEYSTORE_TYPE), properties.getProperty(NiFiProperties.SECURITY_TRUSTSTORE), - properties.getProperty(NiFiProperties.SECURITY_TRUSTSTORE_PASSWD) != null ? properties.getProperty(NiFiProperties.SECURITY_TRUSTSTORE_PASSWD).toCharArray() : null, + StringUtils.isNotBlank(properties.getProperty(NiFiProperties.SECURITY_TRUSTSTORE_PASSWD)) ? properties.getProperty(NiFiProperties.SECURITY_TRUSTSTORE_PASSWD).toCharArray() : null, properties.getProperty(NiFiProperties.SECURITY_TRUSTSTORE_TYPE), WANT, sslContext.getProtocol()); @@ -346,7 +346,10 @@ public class OkHttpReplicationClient implements HttpReplicationClient { .filter(trustManager -> trustManager instanceof X509TrustManager) .map(trustManager -> (X509TrustManager) trustManager).collect(Collectors.toList()); return new Tuple<>(sslContextTuple.getKey().getSocketFactory(), x509TrustManagers.get(0)); - } catch (CertificateException | UnrecoverableKeyException | NoSuchAlgorithmException | KeyStoreException | KeyManagementException | IOException e) { + } catch(UnrecoverableKeyException e) { + logger.error("Key password may be incorrect or not set. Check your keystore passwords." + e.getMessage()); + return null; + } catch (CertificateException | NoSuchAlgorithmException | KeyStoreException | KeyManagementException | IOException e) { return null; } } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/test/groovy/org/apache/nifi/cluster/coordination/http/replication/okhttp/OkHttpReplicationClientTest.groovy b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/test/groovy/org/apache/nifi/cluster/coordination/http/replication/okhttp/OkHttpReplicationClientTest.groovy index cad27f182b..712f80a9eb 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/test/groovy/org/apache/nifi/cluster/coordination/http/replication/okhttp/OkHttpReplicationClientTest.groovy +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/test/groovy/org/apache/nifi/cluster/coordination/http/replication/okhttp/OkHttpReplicationClientTest.groovy @@ -26,6 +26,8 @@ import org.junit.runner.RunWith import org.junit.runners.JUnit4 import org.slf4j.Logger import org.slf4j.LoggerFactory +import sun.security.ssl.DummyX509KeyManager +import sun.security.ssl.SunX509KeyManagerImpl @RunWith(JUnit4.class) class OkHttpReplicationClientTest extends GroovyTestCase { @@ -135,4 +137,77 @@ class OkHttpReplicationClientTest extends GroovyTestCase { assert headers."Content-Length" == "123" } } + + @Test + void testShouldUseKeystorePasswdIfKeypasswdIsBlank() { + // Arrange + Map flowfileEncryptionProps = [ + (NiFiProperties.SECURITY_TRUSTSTORE): "./src/test/resources/conf/truststore.jks", + (NiFiProperties.SECURITY_TRUSTSTORE_TYPE): "JKS", + (NiFiProperties.SECURITY_TRUSTSTORE_PASSWD): "passwordpassword", + (NiFiProperties.SECURITY_KEYSTORE): "./src/test/resources/conf/keystore.jks", + (NiFiProperties.SECURITY_KEYSTORE_TYPE): "JKS", + (NiFiProperties.SECURITY_KEYSTORE_PASSWD): "passwordpassword", + (NiFiProperties.SECURITY_KEY_PASSWD): "", + (NiFiProperties.WEB_HTTPS_HOST): "localhost", + (NiFiProperties.WEB_HTTPS_PORT): "51552", + ] + NiFiProperties mockNiFiProperties = new StandardNiFiProperties(new Properties(flowfileEncryptionProps)) + + // Act + OkHttpReplicationClient client = new OkHttpReplicationClient(mockNiFiProperties) + + // Assert + assertNotNull(client.okHttpClient.sslSocketFactory) + assertEquals(SunX509KeyManagerImpl.class, client.okHttpClient.sslSocketFactory.context.getX509KeyManager().getClass()) + assertNotNull(client.okHttpClient.sslSocketFactory.context.getX509KeyManager().credentialsMap["nifi-key"]) + } + + @Test + void testShouldFailIfKeyPasswdIsSetButKeystorePasswdIsBlank() { + // Arrange + Map flowfileEncryptionProps = [ + (NiFiProperties.SECURITY_TRUSTSTORE): "./src/test/resources/conf/truststore.jks", + (NiFiProperties.SECURITY_TRUSTSTORE_TYPE): "JKS", + (NiFiProperties.SECURITY_TRUSTSTORE_PASSWD): "passwordpassword", + (NiFiProperties.SECURITY_KEYSTORE): "./src/test/resources/conf/keystore.jks", + (NiFiProperties.SECURITY_KEYSTORE_TYPE): "JKS", + (NiFiProperties.SECURITY_KEYSTORE_PASSWD): "", + (NiFiProperties.SECURITY_KEY_PASSWD): "passwordpassword", + (NiFiProperties.WEB_HTTPS_HOST): "localhost", + (NiFiProperties.WEB_HTTPS_PORT): "51552", + ] + NiFiProperties mockNiFiProperties = new StandardNiFiProperties(new Properties(flowfileEncryptionProps)) + + // Act + OkHttpReplicationClient client = new OkHttpReplicationClient(mockNiFiProperties) + + // Assert + // The replication client will fail to initialize if the keystore password is missing, and will use + // a default empty DummyX509KeyManager instead. This is considered a failure to start the service. + assertSame(DummyX509KeyManager.class, client.okHttpClient.sslSocketFactory.context.getX509KeyManager().getClass()) + } + + @Test + void testShouldFailIfKeyPasswdIsBlankAndKeystorePasswd() { + // Arrange + Map flowfileEncryptionProps = [ + (NiFiProperties.SECURITY_TRUSTSTORE): "./src/test/resources/conf/truststore.jks", + (NiFiProperties.SECURITY_TRUSTSTORE_TYPE): "JKS", + (NiFiProperties.SECURITY_TRUSTSTORE_PASSWD): "passwordpassword", + (NiFiProperties.SECURITY_KEYSTORE): "./src/test/resources/conf/keystore.jks", + (NiFiProperties.SECURITY_KEYSTORE_TYPE): "JKS", + (NiFiProperties.SECURITY_KEYSTORE_PASSWD): "", + (NiFiProperties.SECURITY_KEY_PASSWD): "", + (NiFiProperties.WEB_HTTPS_HOST): "localhost", + (NiFiProperties.WEB_HTTPS_PORT): "51552", + ] + NiFiProperties mockNiFiProperties = new StandardNiFiProperties(new Properties(flowfileEncryptionProps)) + + // Act + OkHttpReplicationClient client = new OkHttpReplicationClient(mockNiFiProperties) + + // Assert + assertEquals(DummyX509KeyManager.class, client.okHttpClient.sslSocketFactory.context.getX509KeyManager().getClass()) + } } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/test/resources/conf/keystore.jks b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/test/resources/conf/keystore.jks new file mode 100644 index 0000000000..246fe888ef Binary files /dev/null and b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/test/resources/conf/keystore.jks differ diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/test/resources/conf/truststore.jks b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/test/resources/conf/truststore.jks new file mode 100644 index 0000000000..87f4be1cb7 Binary files /dev/null and b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/test/resources/conf/truststore.jks differ