Skip to content

Commit

Permalink
NIFI-8096 Deprecated ClientAuth references in SSLContextService
Browse files Browse the repository at this point in the history
- Added SSLContextService.createContext() and refactored referencing components
- Removed references to ClientAuth from SslContextFactory methods

This closes apache#4737.

Signed-off-by: Mark Payne <markap14@hotmail.com>
  • Loading branch information
exceptionfactory authored and driesva committed Mar 19, 2021
1 parent 7dd0d8c commit 1af9e5d
Show file tree
Hide file tree
Showing 53 changed files with 147 additions and 305 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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("/")));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)}")
}

Expand All @@ -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)
}
Expand All @@ -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)
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down

0 comments on commit 1af9e5d

Please sign in to comment.