From b386c7c5aee42771c0ac13fa34cd9b651cd74e89 Mon Sep 17 00:00:00 2001 From: "Christopher L. Shannon (cshannon)" Date: Wed, 18 Oct 2017 10:07:23 -0400 Subject: [PATCH] ARTEMIS-1469 - Add support for trusting any client certificate Added a new trustAll flag which will support trusting any client keystore when doing testing against a broker. This setting should not be used in production and is strictly for testing. --- .../remoting/impl/netty/NettyConnector.java | 7 ++- .../impl/netty/TransportConstants.java | 4 ++ .../core/remoting/impl/ssl/SSLSupport.java | 23 +++++++-- docs/user-manual/en/configuring-transports.md | 8 ++++ .../ssl/CoreClientOverTwoWaySSLTest.java | 47 +++++++++++++++++++ .../remoting/impl/ssl/SSLSupportTest.java | 7 +++ 6 files changed, 92 insertions(+), 4 deletions(-) diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyConnector.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyConnector.java index 4452008e35..8faa22dbc6 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyConnector.java +++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyConnector.java @@ -212,6 +212,8 @@ public class NettyConnector extends AbstractConnector { private boolean verifyHost; + private boolean trustAll; + private String sniHost; private String kerb5Config; @@ -342,6 +344,8 @@ public class NettyConnector extends AbstractConnector { verifyHost = ConfigurationHelper.getBooleanProperty(TransportConstants.VERIFY_HOST_PROP_NAME, TransportConstants.DEFAULT_VERIFY_HOST, configuration); + trustAll = ConfigurationHelper.getBooleanProperty(TransportConstants.TRUST_ALL_PROP_NAME, TransportConstants.DEFAULT_TRUST_ALL, configuration); + sniHost = ConfigurationHelper.getStringProperty(TransportConstants.SNIHOST_PROP_NAME, TransportConstants.DEFAULT_SNIHOST_CONFIG, configuration); kerb5Config = ConfigurationHelper.getStringProperty(TransportConstants.SSL_KRB5_CONFIG_PROP_NAME, TransportConstants.DEFAULT_SSL_KRB5_CONFIG, configuration); @@ -357,6 +361,7 @@ public class NettyConnector extends AbstractConnector { enabledCipherSuites = TransportConstants.DEFAULT_ENABLED_CIPHER_SUITES; enabledProtocols = TransportConstants.DEFAULT_ENABLED_PROTOCOLS; verifyHost = TransportConstants.DEFAULT_VERIFY_HOST; + trustAll = TransportConstants.DEFAULT_TRUST_ALL; useDefaultSslContext = TransportConstants.DEFAULT_USE_DEFAULT_SSL_CONTEXT; } @@ -514,7 +519,7 @@ public class NettyConnector extends AbstractConnector { if (System.getProperty(ACTIVEMQ_TRUSTSTORE_PASSWORD_PROP_NAME) != null) { realTrustStorePassword = System.getProperty(ACTIVEMQ_TRUSTSTORE_PASSWORD_PROP_NAME); } - context = SSLSupport.createContext(realKeyStoreProvider, realKeyStorePath, realKeyStorePassword, realTrustStoreProvider, realTrustStorePath, realTrustStorePassword); + context = SSLSupport.createContext(realKeyStoreProvider, realKeyStorePath, realKeyStorePassword, realTrustStoreProvider, realTrustStorePath, realTrustStorePassword, trustAll); } } catch (Exception e) { close(); diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/TransportConstants.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/TransportConstants.java index a4f73dd7c7..9041348498 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/TransportConstants.java +++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/TransportConstants.java @@ -103,6 +103,8 @@ public class TransportConstants { public static final String VERIFY_HOST_PROP_NAME = "verifyHost"; + public static final String TRUST_ALL_PROP_NAME = "trustAll"; + public static final String SNIHOST_PROP_NAME = "sniHost"; public static final String BACKLOG_PROP_NAME = "backlog"; @@ -195,6 +197,8 @@ public class TransportConstants { public static final boolean DEFAULT_VERIFY_HOST = false; + public static final boolean DEFAULT_TRUST_ALL = false; + public static final boolean DEFAULT_USE_DEFAULT_SSL_CONTEXT = false; public static final boolean DEFAULT_TCP_NODELAY = true; diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/ssl/SSLSupport.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/ssl/SSLSupport.java index 8de398257f..b4d9dbfee3 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/ssl/SSLSupport.java +++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/ssl/SSLSupport.java @@ -33,6 +33,8 @@ import java.security.SecureRandom; import org.apache.activemq.artemis.utils.ClassloadingUtil; +import io.netty.handler.ssl.util.InsecureTrustManagerFactory; + /** * Please note, this class supports PKCS#11 keystores, but there are no specific tests in the ActiveMQ Artemis test-suite to * validate/verify this works because this requires a functioning PKCS#11 provider which is not available by default @@ -48,9 +50,20 @@ public class SSLSupport { final String trustStoreProvider, final String trustStorePath, final String trustStorePassword) throws Exception { + + return SSLSupport.createContext(keystoreProvider, keystorePath, keystorePassword, trustStoreProvider, trustStorePath, trustStorePassword, false); + } + + public static SSLContext createContext(final String keystoreProvider, + final String keystorePath, + final String keystorePassword, + final String trustStoreProvider, + final String trustStorePath, + final String trustStorePassword, + final boolean trustAll) throws Exception { SSLContext context = SSLContext.getInstance("TLS"); KeyManager[] keyManagers = SSLSupport.loadKeyManagers(keystoreProvider, keystorePath, keystorePassword); - TrustManager[] trustManagers = SSLSupport.loadTrustManager(trustStoreProvider, trustStorePath, trustStorePassword); + TrustManager[] trustManagers = SSLSupport.loadTrustManager(trustStoreProvider, trustStorePath, trustStorePassword, trustAll); context.init(keyManagers, trustManagers, new SecureRandom()); return context; } @@ -79,8 +92,12 @@ public class SSLSupport { private static TrustManager[] loadTrustManager(final String trustStoreProvider, final String trustStorePath, - final String trustStorePassword) throws Exception { - if (trustStorePath == null && (trustStoreProvider == null || !"PKCS11".equals(trustStoreProvider.toUpperCase()))) { + final String trustStorePassword, + final boolean trustAll) throws Exception { + if (trustAll) { + //This is useful for testing but not should be used outside of that purpose + return InsecureTrustManagerFactory.INSTANCE.getTrustManagers(); + } else if (trustStorePath == null && (trustStoreProvider == null || !"PKCS11".equals(trustStoreProvider.toUpperCase()))) { return null; } else { TrustManagerFactory trustMgrFactory; diff --git a/docs/user-manual/en/configuring-transports.md b/docs/user-manual/en/configuring-transports.md index 51ad14dd1c..69b39decad 100644 --- a/docs/user-manual/en/configuring-transports.md +++ b/docs/user-manual/en/configuring-transports.md @@ -406,6 +406,14 @@ following additional properties: and 2-way SSL. Valid values are `true` or `false`. Default is `false`. + +- `trustAll` + + When used on a `connector` the client will trust the provided server certificate + implicitly, regardless of any configured trust store. **Warning:** This setting is + primarily for testing purposes only and should not be used in production. + + Valid values are `true` or `false`. Default is `false`. - `useDefaultSslContext` diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/ssl/CoreClientOverTwoWaySSLTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/ssl/CoreClientOverTwoWaySSLTest.java index 11b3b0bff2..6e308790bb 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/ssl/CoreClientOverTwoWaySSLTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/ssl/CoreClientOverTwoWaySSLTest.java @@ -220,6 +220,53 @@ public class CoreClientOverTwoWaySSLTest extends ActiveMQTestBase { } } + @Test + public void testTwoWaySSLVerifyClientTrustAllTrue() throws Exception { + NettyAcceptor acceptor = (NettyAcceptor) server.getRemotingService().getAcceptor("nettySSL"); + acceptor.getConfiguration().put(TransportConstants.NEED_CLIENT_AUTH_PROP_NAME, true); + server.getRemotingService().stop(false); + server.getRemotingService().start(); + server.getRemotingService().startAcceptors(); + + //Set trust all so this should work even with no trust store set + tc.getParams().put(TransportConstants.SSL_ENABLED_PROP_NAME, true); + tc.getParams().put(TransportConstants.TRUST_ALL_PROP_NAME, true); + tc.getParams().put(TransportConstants.KEYSTORE_PROVIDER_PROP_NAME, storeType); + tc.getParams().put(TransportConstants.KEYSTORE_PATH_PROP_NAME, CLIENT_SIDE_KEYSTORE); + tc.getParams().put(TransportConstants.KEYSTORE_PASSWORD_PROP_NAME, PASSWORD); + + server.getRemotingService().addIncomingInterceptor(new MyInterceptor()); + + ServerLocator locator = addServerLocator(ActiveMQClient.createServerLocatorWithoutHA(tc)); + ClientSessionFactory sf = createSessionFactory(locator); + sf.close(); + } + + @Test + public void testTwoWaySSLVerifyClientTrustAllFalse() throws Exception { + NettyAcceptor acceptor = (NettyAcceptor) server.getRemotingService().getAcceptor("nettySSL"); + acceptor.getConfiguration().put(TransportConstants.NEED_CLIENT_AUTH_PROP_NAME, true); + server.getRemotingService().stop(false); + server.getRemotingService().start(); + server.getRemotingService().startAcceptors(); + + //Trust all defaults to false so this should fail with no trust store set + tc.getParams().put(TransportConstants.SSL_ENABLED_PROP_NAME, true); + tc.getParams().put(TransportConstants.KEYSTORE_PROVIDER_PROP_NAME, storeType); + tc.getParams().put(TransportConstants.KEYSTORE_PATH_PROP_NAME, CLIENT_SIDE_KEYSTORE); + tc.getParams().put(TransportConstants.KEYSTORE_PASSWORD_PROP_NAME, PASSWORD); + + server.getRemotingService().addIncomingInterceptor(new MyInterceptor()); + + ServerLocator locator = addServerLocator(ActiveMQClient.createServerLocatorWithoutHA(tc)); + try { + ClientSessionFactory sf = createSessionFactory(locator); + fail("Creating a session here should fail due to no trust store being set"); + } catch (Exception e) { + // ignore + } + } + @Test public void testTwoWaySSLWithoutClientKeyStore() throws Exception { tc.getParams().put(TransportConstants.SSL_ENABLED_PROP_NAME, true); diff --git a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/remoting/impl/ssl/SSLSupportTest.java b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/remoting/impl/ssl/SSLSupportTest.java index adcb235aeb..3cb6e6d433 100644 --- a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/remoting/impl/ssl/SSLSupportTest.java +++ b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/remoting/impl/ssl/SSLSupportTest.java @@ -161,4 +161,11 @@ public class SSLSupportTest extends ActiveMQTestBase { } catch (Exception e) { } } + + @Test + public void testContextWithTrustAll() throws Exception { + //This is using a bad password but should not fail because the trust store should be ignored with + //the trustAll flag set to true + SSLSupport.createContext(storeType, keyStorePath, keyStorePassword, storeType, trustStorePath, "bad passord", true); + } }