NIFI-8096 Deprecated ClientAuth references in SSLContextService

- Added SSLContextService.createContext() and refactored referencing components - Removed references to ClientAuth from SslContextFactory methods This closes apache#4737. Signed-off-by: Mark Payne <[email protected]>
nexr · Jan 6, 2021 · 817f621 · 817f621
1 parent 783633c
commit 817f621
Show file tree

Hide file tree

Showing 53 changed files with 147 additions and 305 deletions.
diff --git a/...ootstrap/src/test/java/org/apache/nifi/bootstrap/http/TestHttpNotificationServiceSSL.java b/...ootstrap/src/test/java/org/apache/nifi/bootstrap/http/TestHttpNotificationServiceSSL.java
@@ -31,7 +31,6 @@
 import javax.xml.parsers.ParserConfigurationException;
 import okhttp3.mockwebserver.MockWebServer;
 import org.apache.nifi.bootstrap.NotificationServiceManager;
-import org.apache.nifi.security.util.ClientAuth;
 import org.apache.nifi.security.util.SslContextFactory;
 import org.apache.nifi.security.util.StandardTlsConfiguration;
 import org.apache.nifi.security.util.TlsConfiguration;
@@ -138,7 +137,7 @@ public void startServer() throws IOException, TlsException {
 
         TlsConfiguration tlsConfiguration = new StandardTlsConfiguration("./src/test/resources/keystore.jks", "passwordpassword", null, "JKS",
                 "./src/test/resources/truststore.jks", "passwordpassword", "JKS", TlsConfiguration.getHighestCurrentSupportedTlsProtocolVersion());
-        final SSLContext sslContext = SslContextFactory.createSslContext(tlsConfiguration, ClientAuth.REQUIRED);
+        final SSLContext sslContext = SslContextFactory.createSslContext(tlsConfiguration);
         mockWebServer.useHttps(sslContext.getSocketFactory(), false);
 
         String configFileOutput = CONFIGURATION_FILE_TEXT.replace("${test.server}", String.valueOf(mockWebServer.url("/")));

diff --git a/...ns/nifi-security-utils/src/main/java/org/apache/nifi/security/util/SslContextFactory.java b/...ns/nifi-security-utils/src/main/java/org/apache/nifi/security/util/SslContextFactory.java
@@ -43,27 +43,13 @@ public final class SslContextFactory {
     // TODO: Move to nifi-security-utils-core
 
     /**
-     * Returns a configured {@link SSLContext} from the provided TLS configuration. Hardcodes the
-     * client auth setting to {@link ClientAuth#REQUIRED} because this method is usually used when
-     * creating a context for a client, which ignores the setting anyway.
+     * Create and initialize a {@link SSLContext} from the provided TLS configuration.
      *
      * @param tlsConfiguration the TLS configuration container object
-     * @return the configured SSLContext
+     * @return {@link SSLContext} initialized from TLS Configuration or null when TLS Configuration is empty
      * @throws TlsException if there is a problem configuring the SSLContext
      */
-    public static SSLContext createSslContext(TlsConfiguration tlsConfiguration) throws TlsException {
-        return createSslContext(tlsConfiguration, ClientAuth.REQUIRED);
-    }
-
-    /**
-     * Returns a configured {@link SSLContext} from the provided TLS configuration.
-     *
-     * @param tlsConfiguration the TLS configuration container object
-     * @param clientAuth       the {@link ClientAuth} setting
-     * @return the configured SSLContext
-     * @throws TlsException if there is a problem configuring the SSLContext
-     */
-    public static SSLContext createSslContext(TlsConfiguration tlsConfiguration, ClientAuth clientAuth) throws TlsException {
+    public static SSLContext createSslContext(final TlsConfiguration tlsConfiguration) throws TlsException {
         if (TlsConfiguration.isEmpty(tlsConfiguration)) {
             logger.debug("Cannot create SSLContext from empty TLS configuration; returning null");
             return null;
@@ -79,31 +65,25 @@ public static SSLContext createSslContext(TlsConfiguration tlsConfiguration, Cli
         }
         final TrustManager[] trustManagers = getTrustManagers(tlsConfiguration);
 
-        return createSslContext(tlsConfiguration, trustManagers, clientAuth);
+        return createSslContext(tlsConfiguration, trustManagers);
     }
 
     /**
-     * Returns a configured {@link SSLContext} from the provided TLS configuration and Trust Managers
+     * Create and initialize a {@link SSLContext} from the provided TLS configuration and Trust Managers.
      *
      * @param tlsConfiguration the TLS configuration container object
      * @param trustManagers    Trust Managers can be null to use platform default Trust Managers
-     * @param clientAuth       the {@link ClientAuth} setting
-     * @return the configured SSLContext
+     * @return {@link SSLContext} initialized from TLS Configuration or null when TLS Configuration is empty
      * @throws TlsException if there is a problem configuring the SSLContext
      */
-    public static SSLContext createSslContext(final TlsConfiguration tlsConfiguration, final TrustManager[] trustManagers, ClientAuth clientAuth) throws TlsException {
+    public static SSLContext createSslContext(final TlsConfiguration tlsConfiguration, final TrustManager[] trustManagers) throws TlsException {
         if (TlsConfiguration.isEmpty(tlsConfiguration)) {
             logger.debug("Cannot create SSLContext from empty TLS configuration; returning null");
             return null;
         }
 
-        if (clientAuth == null) {
-            clientAuth = ClientAuth.REQUIRED;
-            logger.debug("ClientAuth was null so defaulting to {}", clientAuth);
-        }
-
         final KeyManager[] keyManagers = getKeyManagers(tlsConfiguration);
-        return initializeSSLContext(tlsConfiguration, clientAuth, keyManagers, trustManagers);
+        return initializeSSLContext(tlsConfiguration, keyManagers, trustManagers);
     }
 
     /**
@@ -131,15 +111,13 @@ public static X509TrustManager getX509TrustManager(TlsConfiguration tlsConfigura
 
     /**
      * Convenience method to return the {@link SSLSocketFactory} from the created {@link SSLContext}
-     * because that is what most callers of {@link #createSslContext(TlsConfiguration, ClientAuth)}
-     * actually need and don't know what to provide for the {@link ClientAuth} parameter.
      *
      * @param tlsConfiguration the TLS configuration container object
      * @return the configured SSLSocketFactory (can be {@code null})
      * @throws TlsException if there is a problem creating the SSLContext or SSLSocketFactory
      */
-    public static SSLSocketFactory createSSLSocketFactory(TlsConfiguration tlsConfiguration) throws TlsException {
-        SSLContext sslContext = createSslContext(tlsConfiguration, ClientAuth.REQUIRED);
+    public static SSLSocketFactory createSSLSocketFactory(final TlsConfiguration tlsConfiguration) throws TlsException {
+        SSLContext sslContext = createSslContext(tlsConfiguration);
         if (sslContext == null) {
             // Only display an error in the log if the provided config wasn't empty
             if (!TlsConfiguration.isEmpty(tlsConfiguration)) {
@@ -209,25 +187,12 @@ public static TrustManager[] getTrustManagers(TlsConfiguration tlsConfiguration)
         return trustManagers;
     }
 
-    private static SSLContext initializeSSLContext(TlsConfiguration tlsConfiguration, ClientAuth clientAuth, KeyManager[] keyManagers, TrustManager[] trustManagers) throws TlsException {
-        final SSLContext sslContext;
+    private static SSLContext initializeSSLContext(final TlsConfiguration tlsConfiguration, final KeyManager[] keyManagers, final TrustManager[] trustManagers) throws TlsException {
         try {
-            sslContext = SSLContext.getInstance(tlsConfiguration.getProtocol());
+            final SSLContext sslContext = SSLContext.getInstance(tlsConfiguration.getProtocol());
             sslContext.init(keyManagers, trustManagers, new SecureRandom());
-            switch (clientAuth) {
-                case REQUIRED:
-                    sslContext.getDefaultSSLParameters().setNeedClientAuth(true);
-                    break;
-                case WANT:
-                    sslContext.getDefaultSSLParameters().setWantClientAuth(true);
-                    break;
-                case NONE:
-                default:
-                    sslContext.getDefaultSSLParameters().setWantClientAuth(false);
-            }
-
             return sslContext;
-        } catch (NoSuchAlgorithmException | KeyManagementException e) {
+        } catch (final NoSuchAlgorithmException | KeyManagementException e) {
             logger.error("Encountered an error creating SSLContext from TLS configuration ({}): {}", tlsConfiguration.toString(), e.getLocalizedMessage());
             throw new TlsException("Error creating SSL context", e);
         }

diff --git a/...security-utils/src/test/groovy/org/apache/nifi/security/util/SslContextFactoryTest.groovy b/...security-utils/src/test/groovy/org/apache/nifi/security/util/SslContextFactoryTest.groovy
@@ -99,18 +99,12 @@ class SslContextFactoryTest extends GroovyTestCase {
         logger.info("Creating SSL Context from TLS Configuration: ${tlsConfiguration}")
 
         // Act
-        SSLContext sslContext = SslContextFactory.createSslContext(tlsConfiguration, ClientAuth.NONE)
+        SSLContext sslContext = SslContextFactory.createSslContext(tlsConfiguration)
         logger.info("Created SSL Context: ${KeyStoreUtils.sslContextToString(sslContext)}")
 
         // Assert
         assert sslContext.protocol == tlsConfiguration.protocol
 
-        def defaultSSLParameters = sslContext.defaultSSLParameters
-        logger.info("Default SSL Parameters: ${KeyStoreUtils.sslParametersToString(defaultSSLParameters)}" as String)
-        assertProtocolVersions(defaultSSLParameters.protocols, TlsPlatform.supportedProtocols)
-        assert !defaultSSLParameters.needClientAuth
-        assert !defaultSSLParameters.wantClientAuth
-
         // Check a socket created from this context
         assertSocketProtocols(sslContext)
     }
@@ -129,18 +123,12 @@ class SslContextFactoryTest extends GroovyTestCase {
         logger.info("Creating SSL Context from TLS Configuration: ${configWithoutKeyPassword}")
 
         // Act
-        SSLContext sslContext = SslContextFactory.createSslContext(configWithoutKeyPassword, ClientAuth.NONE)
+        SSLContext sslContext = SslContextFactory.createSslContext(configWithoutKeyPassword)
         logger.info("Created SSL Context: ${KeyStoreUtils.sslContextToString(sslContext)}")
 
         // Assert
         assert sslContext.protocol == configWithoutKeyPassword.protocol
 
-        def defaultSSLParameters = sslContext.defaultSSLParameters
-        logger.info("Default SSL Parameters: ${KeyStoreUtils.sslParametersToString(defaultSSLParameters)}" as String)
-        assertProtocolVersions(defaultSSLParameters.protocols, TlsPlatform.supportedProtocols)
-        assert !defaultSSLParameters.needClientAuth
-        assert !defaultSSLParameters.wantClientAuth
-
         // Check a socket created from this context
         assertSocketProtocols(sslContext)
     }
@@ -175,12 +163,12 @@ class SslContextFactoryTest extends GroovyTestCase {
 
         // Act
         def noKeystorePathMsg = shouldFail(TlsException) {
-            SSLContext sslContext = SslContextFactory.createSslContext(configNoKeystorePath, ClientAuth.NONE)
+            SSLContext sslContext = SslContextFactory.createSslContext(configNoKeystorePath)
             logger.info("Created SSL Context missing keystore path: ${KeyStoreUtils.sslContextToString(sslContext)}")
         }
 
         def noTruststorePathMsg = shouldFail(TlsException) {
-            SSLContext sslContext = SslContextFactory.createSslContext(configNoTruststorePath, ClientAuth.NONE)
+            SSLContext sslContext = SslContextFactory.createSslContext(configNoTruststorePath)
             logger.info("Created SSL Context missing truststore path: ${KeyStoreUtils.sslContextToString(sslContext)}")
         }
 
@@ -206,18 +194,12 @@ class SslContextFactoryTest extends GroovyTestCase {
         logger.info("Creating SSL Context from TLS Configuration: ${configNoTruststorePassword}")
 
         // Act
-        SSLContext sslContext = SslContextFactory.createSslContext(configNoTruststorePassword, ClientAuth.NONE)
+        SSLContext sslContext = SslContextFactory.createSslContext(configNoTruststorePassword)
         logger.info("Created SSL Context: ${KeyStoreUtils.sslContextToString(sslContext)}")
 
         // Assert
         assert sslContext.protocol == configNoTruststorePassword.protocol
 
-        def defaultSSLParameters = sslContext.defaultSSLParameters
-        logger.info("Default SSL Parameters: ${KeyStoreUtils.sslParametersToString(defaultSSLParameters)}" as String)
-        assertProtocolVersions(defaultSSLParameters.protocols, TlsPlatform.supportedProtocols)
-        assert !defaultSSLParameters.needClientAuth
-        assert !defaultSSLParameters.wantClientAuth
-
         // Check a socket created from this context
         assertSocketProtocols(sslContext)
     }
@@ -239,7 +221,7 @@ class SslContextFactoryTest extends GroovyTestCase {
 
         // Act
         def msg = shouldFail(TlsException) {
-            SSLContext sslContext = SslContextFactory.createSslContext(keystoreOnlyConfig, ClientAuth.NONE)
+            SSLContext sslContext = SslContextFactory.createSslContext(keystoreOnlyConfig)
             logger.info("Created SSL Context: ${KeyStoreUtils.sslContextToString(sslContext)}")
         }
         logger.expected(msg)
@@ -259,7 +241,7 @@ class SslContextFactoryTest extends GroovyTestCase {
         logger.info("Creating SSL Context from TLS Configuration: ${emptyConfig}")
 
         // Act
-        SSLContext sslContext = SslContextFactory.createSslContext(emptyConfig, ClientAuth.NONE)
+        SSLContext sslContext = SslContextFactory.createSslContext(emptyConfig)
 
         // Assert
         assert !sslContext

diff --git a/.../nifi-socket-utils/src/main/java/org/apache/nifi/io/socket/ServerSocketConfiguration.java b/.../nifi-socket-utils/src/main/java/org/apache/nifi/io/socket/ServerSocketConfiguration.java
@@ -17,7 +17,6 @@
 package org.apache.nifi.io.socket;
 
 import javax.net.ssl.SSLContext;
-import org.apache.nifi.security.util.ClientAuth;
 import org.apache.nifi.security.util.SslContextFactory;
 import org.apache.nifi.security.util.TlsConfiguration;
 import org.apache.nifi.security.util.TlsException;
@@ -34,8 +33,7 @@ public ServerSocketConfiguration() {
     }
 
     public SSLContext createSSLContext() throws TlsException {
-        // ClientAuth was hardcoded to REQUIRED in removed SSLContextFactory and overridden in SocketUtils when the socket is created
-        return SslContextFactory.createSslContext(tlsConfiguration, ClientAuth.REQUIRED);
+        return SslContextFactory.createSslContext(tlsConfiguration);
     }
 
     public void setTlsConfiguration(final TlsConfiguration tlsConfiguration) {

diff --git a/...ommons/nifi-socket-utils/src/main/java/org/apache/nifi/io/socket/SocketConfiguration.java b/...ommons/nifi-socket-utils/src/main/java/org/apache/nifi/io/socket/SocketConfiguration.java
@@ -17,7 +17,6 @@
 package org.apache.nifi.io.socket;
 
 import javax.net.ssl.SSLContext;
-import org.apache.nifi.security.util.ClientAuth;
 import org.apache.nifi.security.util.SslContextFactory;
 import org.apache.nifi.security.util.TlsConfiguration;
 import org.apache.nifi.security.util.TlsException;
@@ -35,8 +34,7 @@ public final class SocketConfiguration {
     private TlsConfiguration tlsConfiguration;
 
     public SSLContext createSSLContext() throws TlsException {
-        // This is only used for client sockets, so the client auth setting is ignored
-        return SslContextFactory.createSslContext(tlsConfiguration, ClientAuth.NONE);
+        return SslContextFactory.createSslContext(tlsConfiguration);
     }
 
     public void setTlsConfiguration(final TlsConfiguration tlsConfiguration) {

diff --git a/...-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/AbstractAMQPProcessor.java b/...-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/AbstractAMQPProcessor.java
@@ -298,7 +298,7 @@ protected Connection createConnection(ProcessContext context, ExecutorService ex
         final Boolean useCertAuthentication = context.getProperty(USE_CERT_AUTHENTICATION).asBoolean();
 
         if (sslService != null) {
-            final SSLContext sslContext = sslService.createSSLContext(ClientAuth.NONE);
+            final SSLContext sslContext = sslService.createContext();
             cf.useSslProtocol(sslContext);
 
             if (useCertAuthentication) {

diff --git a/...bstract-processors/src/main/java/org/apache/nifi/processors/aws/AbstractAWSProcessor.java b/...bstract-processors/src/main/java/org/apache/nifi/processors/aws/AbstractAWSProcessor.java
@@ -58,7 +58,6 @@
 import org.apache.nifi.processors.aws.credentials.provider.factory.CredentialPropertyDescriptors;
 import org.apache.nifi.proxy.ProxyConfiguration;
 import org.apache.nifi.proxy.ProxySpec;
-import org.apache.nifi.security.util.ClientAuth;
 import org.apache.nifi.ssl.SSLContextService;
 
 /**
@@ -227,7 +226,7 @@ protected ClientConfiguration createConfiguration(final ProcessContext context)
         if(this.getSupportedPropertyDescriptors().contains(SSL_CONTEXT_SERVICE)) {
             final SSLContextService sslContextService = context.getProperty(SSL_CONTEXT_SERVICE).asControllerService(SSLContextService.class);
             if (sslContextService != null) {
-                final SSLContext sslContext = sslContextService.createSSLContext(ClientAuth.NONE);
+                final SSLContext sslContext = sslContextService.createContext();
                 // NIFI-3788: Changed hostnameVerifier from null to DHV (BrowserCompatibleHostnameVerifier is deprecated)
                 SdkTLSSocketFactory sdkTLSSocketFactory = new SdkTLSSocketFactory(sslContext, new DefaultHostnameVerifier());
                 config.getApacheHttpClientConfig().setSslSocketFactory(sdkTLSSocketFactory);

diff --git a/...dle/nifi-beats-processors/src/main/java/org/apache/nifi/processors/beats/ListenBeats.java b/...dle/nifi-beats-processors/src/main/java/org/apache/nifi/processors/beats/ListenBeats.java
@@ -157,7 +157,7 @@ protected ChannelDispatcher createDispatcher(final ProcessContext context, final
         final SSLContextService sslContextService = context.getProperty(SSL_CONTEXT_SERVICE).asControllerService(SSLContextService.class);
         if (sslContextService != null) {
             final String clientAuthValue = context.getProperty(CLIENT_AUTH).getValue();
-            sslContext = sslContextService.createSSLContext(ClientAuth.valueOf(clientAuthValue));
+            sslContext = sslContextService.createContext();
             clientAuth = ClientAuth.valueOf(clientAuthValue);
 
         }

diff --git a/...essors/src/main/java/org/apache/nifi/processors/cassandra/AbstractCassandraProcessor.java b/...essors/src/main/java/org/apache/nifi/processors/cassandra/AbstractCassandraProcessor.java
@@ -252,24 +252,10 @@ void connectToCassandra(ProcessContext context) {
 
             // Set up the client for secure (SSL/TLS communications) if configured to do so
             final SSLContextService sslService = context.getProperty(PROP_SSL_CONTEXT_SERVICE).asControllerService(SSLContextService.class);
-            final String rawClientAuth = context.getProperty(CLIENT_AUTH).getValue();
             final SSLContext sslContext;
 
             if (sslService != null) {
-                final ClientAuth clientAuth;
-
-                if (StringUtils.isBlank(rawClientAuth)) {
-                    clientAuth = ClientAuth.REQUIRED;
-                } else {
-                    try {
-                        clientAuth = ClientAuth.valueOf(rawClientAuth);
-                    } catch (final IllegalArgumentException iae) {
-                        throw new IllegalStateException(String.format("Unrecognized client auth '%s'. Possible values are [%s]",
-                                rawClientAuth, StringUtils.join(ClientAuth.values(), ", ")));
-                    }
-                }
-
-                sslContext = sslService.createSSLContext(clientAuth);
+                sslContext = sslService.createContext();
             } else {
                 sslContext = null;
             }

diff --git a/...rs/src/test/java/org/apache/nifi/processors/cassandra/AbstractCassandraProcessorTest.java b/...rs/src/test/java/org/apache/nifi/processors/cassandra/AbstractCassandraProcessorTest.java
@@ -232,23 +232,6 @@ public void testConnectToCassandraWithSSL() throws Exception {
         assertNotNull(processor.getCluster());
     }
 
-    @Test(expected = IllegalStateException.class)
-    public void testConnectToCassandraWithSSLBadClientAuth() throws Exception {
-        SSLContextService sslService = mock(SSLContextService.class);
-        when(sslService.getIdentifier()).thenReturn("ssl-context");
-        testRunner.addControllerService("ssl-context", sslService);
-        testRunner.enableControllerService(sslService);
-        testRunner.setProperty(AbstractCassandraProcessor.PROP_SSL_CONTEXT_SERVICE, "ssl-context");
-        testRunner.setProperty(AbstractCassandraProcessor.CONSISTENCY_LEVEL, "ONE");
-        testRunner.assertValid(sslService);
-        processor.connectToCassandra(testRunner.getProcessContext());
-        assertNotNull(processor.getCluster());
-        processor.setCluster(null);
-        // Try with a ClientAuth value
-        testRunner.setProperty(AbstractCassandraProcessor.CLIENT_AUTH, "BAD");
-        processor.connectToCassandra(testRunner.getProcessContext());
-    }
-
     @Test
     public void testConnectToCassandraUsernamePassword() throws Exception {
         testRunner.setProperty(AbstractCassandraProcessor.USERNAME, "user");