This is an automated email from the ASF dual-hosted git repository. joewitt pushed a commit to branch support/nifi-1.11.x in repository https://gitbox.apache.org/repos/asf/nifi.git
commit 554a637b4055429649a70de785da3246530bde24 Author: Andy LoPresto <[email protected]> AuthorDate: Fri Mar 13 20:47:21 2020 -0700 NIFI-7223 [WIP] Resolved compilation issues in unit test on OpenJDK 11 by removing Sun security class references. Added OkHttpReplicationClient#isTLSConfigured() method. Added unit test. NIFI-7223 Fixed remaining unit tests for TLS regression. Renamed tests for clarity. --- .../okhttp/OkHttpReplicationClient.java | 18 ++- .../okhttp/OkHttpReplicationClientTest.groovy | 125 ++++++++++++++------- 2 files changed, 99 insertions(+), 44 deletions(-) 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 ff63836..a123d81 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 @@ -17,6 +17,9 @@ package org.apache.nifi.cluster.coordination.http.replication.okhttp; +import static org.apache.nifi.security.util.SslContextFactory.ClientAuth.WANT; +import static org.apache.nifi.security.util.SslContextFactory.createTrustSslContextWithTrustManagers; + import com.fasterxml.jackson.annotation.JsonInclude.Include; import com.fasterxml.jackson.annotation.JsonInclude.Value; import com.fasterxml.jackson.databind.ObjectMapper; @@ -71,9 +74,8 @@ import org.apache.nifi.util.Tuple; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.util.StreamUtils; + // Using static imports because of the name conflict: -import static org.apache.nifi.security.util.SslContextFactory.ClientAuth.WANT; -import static org.apache.nifi.security.util.SslContextFactory.createTrustSslContextWithTrustManagers; public class OkHttpReplicationClient implements HttpReplicationClient { private static final Logger logger = LoggerFactory.getLogger(OkHttpReplicationClient.class); @@ -84,6 +86,7 @@ public class OkHttpReplicationClient implements HttpReplicationClient { private final ObjectMapper jsonCodec = new ObjectMapper(); private final OkHttpClient okHttpClient; + private boolean tlsConfigured = false; public OkHttpReplicationClient(final NiFiProperties properties) { jsonCodec.setDefaultPropertyInclusion(Value.construct(Include.NON_NULL, Include.ALWAYS)); @@ -150,6 +153,16 @@ public class OkHttpReplicationClient implements HttpReplicationClient { return response; } + /** + * Returns {@code true} if the client has TLS enabled and configured. Even clients created without explicit + * keystore and truststore values have a default cipher suite list available, but no keys to use. + * + * @return true if this client can present keys + */ + public boolean isTLSConfigured() { + return tlsConfigured; + } + private MultivaluedMap<String, String> getHeaders(final okhttp3.Response callResponse) { final Headers headers = callResponse.headers(); final MultivaluedMap<String, String> headerMap = new MultivaluedHashMap<>(); @@ -319,6 +332,7 @@ public class OkHttpReplicationClient implements HttpReplicationClient { final Tuple<SSLSocketFactory, X509TrustManager> tuple = createSslSocketFactory(properties); if (tuple != null) { okHttpClientBuilder.sslSocketFactory(tuple.getKey(), tuple.getValue()); + tlsConfigured = true; } return okHttpClientBuilder.build(); 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 712f80a..2cfb2cb 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,8 +26,6 @@ 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 { @@ -139,75 +137,118 @@ class OkHttpReplicationClientTest extends GroovyTestCase { } @Test - void testShouldUseKeystorePasswdIfKeypasswdIsBlank() { + void testShouldUseKeystorePasswordIfKeyPasswordIsBlank() { // Arrange - Map flowfileEncryptionProps = [ - (NiFiProperties.SECURITY_TRUSTSTORE): "./src/test/resources/conf/truststore.jks", - (NiFiProperties.SECURITY_TRUSTSTORE_TYPE): "JKS", + Map propsMap = [ + (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.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)) + NiFiProperties mockNiFiProperties = new StandardNiFiProperties(new Properties(propsMap)) // Act OkHttpReplicationClient client = new OkHttpReplicationClient(mockNiFiProperties) + logger.info("Created secure HTTPS client with TLS configured: ${client.isTLSConfigured()}") // Assert - assertNotNull(client.okHttpClient.sslSocketFactory) - assertEquals(SunX509KeyManagerImpl.class, client.okHttpClient.sslSocketFactory.context.getX509KeyManager().getClass()) - assertNotNull(client.okHttpClient.sslSocketFactory.context.getX509KeyManager().credentialsMap["nifi-key"]) + assert client.isTLSConfigured() + assert client.okHttpClient.sslSocketFactory + assert client.okHttpClient.sslSocketFactory.context.getX509KeyManager().credentialsMap["nifi-key"] } @Test - void testShouldFailIfKeyPasswdIsSetButKeystorePasswdIsBlank() { + void testShouldFailIfKeyPasswordIsSetButKeystorePasswordIsBlank() { // Arrange - Map flowfileEncryptionProps = [ - (NiFiProperties.SECURITY_TRUSTSTORE): "./src/test/resources/conf/truststore.jks", - (NiFiProperties.SECURITY_TRUSTSTORE_TYPE): "JKS", + Map propsMap = [ + (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.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)) + NiFiProperties mockNiFiProperties = new StandardNiFiProperties(new Properties(propsMap)) // Act OkHttpReplicationClient client = new OkHttpReplicationClient(mockNiFiProperties) + logger.info("Created (invalid) secure HTTPS client with TLS configured: ${client.isTLSConfigured()}") // 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()) + assert !client.isTLSConfigured() } @Test - void testShouldFailIfKeyPasswdIsBlankAndKeystorePasswd() { + void testShouldFailIfKeyPasswordIsBlankAndKeystorePassword() { // Arrange - Map flowfileEncryptionProps = [ - (NiFiProperties.SECURITY_TRUSTSTORE): "./src/test/resources/conf/truststore.jks", - (NiFiProperties.SECURITY_TRUSTSTORE_TYPE): "JKS", + Map propsMap = [ + (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.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)) + NiFiProperties mockNiFiProperties = new StandardNiFiProperties(new Properties(propsMap)) // Act OkHttpReplicationClient client = new OkHttpReplicationClient(mockNiFiProperties) + logger.info("Created (invalid) secure HTTPS client with TLS configured: ${client.isTLSConfigured()}") // Assert - assertEquals(DummyX509KeyManager.class, client.okHttpClient.sslSocketFactory.context.getX509KeyManager().getClass()) + assert !client.isTLSConfigured() + } + + @Test + void testShouldDetermineIfTLSConfigured() { + // Arrange + Map propsMap = [(NiFiProperties.WEB_HTTPS_HOST): "localhost", + (NiFiProperties.WEB_HTTPS_PORT): "51552",] + + Map tlsPropsMap = [ + (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) : "", + ] + propsMap + + + NiFiProperties mockNiFiProperties = new StandardNiFiProperties(new Properties(propsMap)) + NiFiProperties mockTLSNiFiProperties = new StandardNiFiProperties(new Properties(tlsPropsMap)) + + // Remove the keystore password to create an invalid configuration + Map invalidTlsPropsMap = tlsPropsMap + invalidTlsPropsMap.remove(NiFiProperties.SECURITY_KEYSTORE_PASSWD) + NiFiProperties mockInvalidTLSNiFiProperties = new StandardNiFiProperties(new Properties(invalidTlsPropsMap)) + + // Act + OkHttpReplicationClient client = new OkHttpReplicationClient(mockNiFiProperties) + logger.info("Created plaintext HTTP client with TLS configured: ${client.isTLSConfigured()}") + + OkHttpReplicationClient invalidTlsClient = new OkHttpReplicationClient(mockInvalidTLSNiFiProperties) + logger.info("Created (invalid) secure HTTPS client with TLS configured: ${invalidTlsClient.isTLSConfigured()}") + + OkHttpReplicationClient tlsClient = new OkHttpReplicationClient(mockTLSNiFiProperties) + logger.info("Created secure HTTPS client with TLS configured: ${tlsClient.isTLSConfigured()}") + + + // Assert + assert !client.isTLSConfigured() + assert !invalidTlsClient.isTLSConfigured() + assert tlsClient.isTLSConfigured() } }
