From 7d87837e08fbcfa59833e3042c7db31402423207 Mon Sep 17 00:00:00 2001 From: "Hiram R. Chirino" Date: Tue, 27 May 2008 16:36:10 +0000 Subject: [PATCH] Added test case provided by Gary for http://issues.apache.org/activemq/browse/AMQ-1749 And changed the way SSL is setup so that it passes that test case. Keep the SslContext in a ThreadLocal that is passed around to avoid setting it on a static field wich would get mixed up between brokers. git-svn-id: https://svn.apache.org/repos/asf/activemq/trunk@660591 13f79535-47bb-0310-9956-ffa450edef68 --- .../activemq/broker/SslBrokerService.java | 13 +- .../apache/activemq/broker/SslContext.java | 59 ++++++++ .../network/DiscoveryNetworkConnector.java | 34 +++-- .../activemq/transport/TransportFactory.java | 14 +- .../transport/tcp/SslTransportFactory.java | 88 +++++------ .../transport/tcp/TcpTransportFactory.java | 4 +- .../tcp/SslContextNBrokerServiceTest.java | 137 ++++++++++++++++++ .../src/test/resources/dummy.keystore | Bin 0 -> 1224 bytes .../activemq/transport/tcp/n-brokers-ssl.xml | 51 +++++++ 9 files changed, 329 insertions(+), 71 deletions(-) create mode 100644 activemq-core/src/test/java/org/apache/activemq/transport/tcp/SslContextNBrokerServiceTest.java create mode 100644 activemq-core/src/test/resources/dummy.keystore create mode 100644 activemq-core/src/test/resources/org/apache/activemq/transport/tcp/n-brokers-ssl.xml diff --git a/activemq-core/src/main/java/org/apache/activemq/broker/SslBrokerService.java b/activemq-core/src/main/java/org/apache/activemq/broker/SslBrokerService.java index c09995553d..27f6ea88d0 100644 --- a/activemq-core/src/main/java/org/apache/activemq/broker/SslBrokerService.java +++ b/activemq-core/src/main/java/org/apache/activemq/broker/SslBrokerService.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.net.URI; import java.security.KeyManagementException; import java.security.SecureRandom; +import java.util.Arrays; import javax.net.ssl.KeyManager; import javax.net.ssl.TrustManager; @@ -90,9 +91,15 @@ public class SslBrokerService extends BrokerService { // If given an SSL URI, use an SSL TransportFactory and configure // it to use the given key and trust managers. SslTransportFactory transportFactory = new SslTransportFactory(); - transportFactory.setKeyAndTrustManagers(km, tm, random); - - return transportFactory.doBind(brokerURI); + + SslContext ctx = new SslContext(km, tm, random); + SslContext.setCurrentSslContext(ctx); + try { + return transportFactory.doBind(brokerURI); + } finally { + SslContext.setCurrentSslContext(null); + } + } else { // Else, business as usual. return TransportFactory.bind(this, brokerURI); diff --git a/activemq-core/src/main/java/org/apache/activemq/broker/SslContext.java b/activemq-core/src/main/java/org/apache/activemq/broker/SslContext.java index 9674b8bbb8..c3843ae989 100644 --- a/activemq-core/src/main/java/org/apache/activemq/broker/SslContext.java +++ b/activemq-core/src/main/java/org/apache/activemq/broker/SslContext.java @@ -16,11 +16,16 @@ */ package org.apache.activemq.broker; +import java.security.KeyManagementException; +import java.security.NoSuchAlgorithmException; +import java.security.NoSuchProviderException; import java.security.SecureRandom; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import javax.net.ssl.KeyManager; +import javax.net.ssl.SSLContext; import javax.net.ssl.TrustManager; /** @@ -28,9 +33,34 @@ import javax.net.ssl.TrustManager; */ public class SslContext { + protected String protocol = "TLS"; + protected String provider = null; protected List keyManagers = new ArrayList(); protected List trustManagers = new ArrayList(); protected SecureRandom secureRandom; + private SSLContext sslContext; + + private static final ThreadLocal current = new ThreadLocal(); + + public SslContext() { + } + + public SslContext(KeyManager[] km, TrustManager[] tm, SecureRandom random) { + if( km!=null ) { + setKeyManagers(Arrays.asList(km)); + } + if( tm!=null ) { + setTrustManagers(Arrays.asList(tm)); + } + setSecureRandom(random); + } + + static public void setCurrentSslContext(SslContext bs) { + current.set(bs); + } + static public SslContext getCurrentSslContext() { + return current.get(); + } public KeyManager[] getKeyManagersAsArray() { KeyManager rc[] = new KeyManager[keyManagers.size()]; @@ -73,4 +103,33 @@ public class SslContext { this.secureRandom = secureRandom; } + public String getProtocol() { + return protocol; + } + public void setProtocol(String protocol) { + this.protocol = protocol; + } + public String getProvider() { + return provider; + } + public void setProvider(String provider) { + this.provider = provider; + } + + public SSLContext getSSLContext() throws NoSuchProviderException, NoSuchAlgorithmException, KeyManagementException { + if( sslContext == null ) { + if( provider == null ) { + sslContext = SSLContext.getInstance(protocol); + } else { + sslContext = SSLContext.getInstance(protocol, provider); + } + sslContext.init(getKeyManagersAsArray(), getTrustManagersAsArray(), getSecureRandom()); + } + return sslContext; + } + public void setSSLContext(SSLContext sslContext) { + this.sslContext = sslContext; + } + + } diff --git a/activemq-core/src/main/java/org/apache/activemq/network/DiscoveryNetworkConnector.java b/activemq-core/src/main/java/org/apache/activemq/network/DiscoveryNetworkConnector.java index 0ef8f60af7..3f15ac9955 100644 --- a/activemq-core/src/main/java/org/apache/activemq/network/DiscoveryNetworkConnector.java +++ b/activemq-core/src/main/java/org/apache/activemq/network/DiscoveryNetworkConnector.java @@ -22,6 +22,7 @@ import java.net.URISyntaxException; import java.util.Iterator; import java.util.concurrent.ConcurrentHashMap; +import org.apache.activemq.broker.SslContext; import org.apache.activemq.command.DiscoveryEvent; import org.apache.activemq.transport.Transport; import org.apache.activemq.transport.TransportFactory; @@ -82,22 +83,29 @@ public class DiscoveryNetworkConnector extends NetworkConnector implements Disco } URI connectUri = uri; LOG.info("Establishing network connection between from " + localURIName + " to " + connectUri); + Transport remoteTransport; - try { - remoteTransport = TransportFactory.connect(connectUri); - } catch (Exception e) { - LOG.warn("Could not connect to remote URI: " + localURIName + ": " + e.getMessage()); - LOG.debug("Connection failure exception: " + e, e); - return; - } Transport localTransport; try { - localTransport = createLocalTransport(); - } catch (Exception e) { - ServiceSupport.dispose(remoteTransport); - LOG.warn("Could not connect to local URI: " + localURIName + ": " + e.getMessage()); - LOG.debug("Connection failure exception: " + e, e); - return; + // Allows the transport to access the broker's ssl configuration. + SslContext.setCurrentSslContext(getBrokerService().getSslContext()); + try { + remoteTransport = TransportFactory.connect(connectUri); + } catch (Exception e) { + LOG.warn("Could not connect to remote URI: " + localURIName + ": " + e.getMessage()); + LOG.debug("Connection failure exception: " + e, e); + return; + } + try { + localTransport = createLocalTransport(); + } catch (Exception e) { + ServiceSupport.dispose(remoteTransport); + LOG.warn("Could not connect to local URI: " + localURIName + ": " + e.getMessage()); + LOG.debug("Connection failure exception: " + e, e); + return; + } + } finally { + SslContext.setCurrentSslContext(null); } NetworkBridge bridge = createBridge(localTransport, remoteTransport, event); bridges.put(uri, bridge); diff --git a/activemq-core/src/main/java/org/apache/activemq/transport/TransportFactory.java b/activemq-core/src/main/java/org/apache/activemq/transport/TransportFactory.java index 91202493e7..c3d8a26557 100755 --- a/activemq-core/src/main/java/org/apache/activemq/transport/TransportFactory.java +++ b/activemq-core/src/main/java/org/apache/activemq/transport/TransportFactory.java @@ -26,8 +26,10 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executor; +import javax.net.ssl.SSLContext; + import org.apache.activemq.broker.BrokerService; -import org.apache.activemq.broker.BrokerServiceAware; +import org.apache.activemq.broker.SslContext; import org.apache.activemq.util.FactoryFinder; import org.apache.activemq.util.IOExceptionSupport; import org.apache.activemq.util.IntrospectionSupport; @@ -117,10 +119,14 @@ public abstract class TransportFactory { public static TransportServer bind(BrokerService brokerService, URI location) throws IOException { TransportFactory tf = findTransportFactory(location); - if (brokerService != null && tf instanceof BrokerServiceAware) { - ((BrokerServiceAware)tf).setBrokerService(brokerService); + try { + if( brokerService!=null ) { + SslContext.setCurrentSslContext(brokerService.getSslContext()); + } + return tf.doBind(location); + } finally { + SslContext.setCurrentSslContext(null); } - return tf.doBind(location); } public Transport doConnect(URI location) throws Exception { diff --git a/activemq-core/src/main/java/org/apache/activemq/transport/tcp/SslTransportFactory.java b/activemq-core/src/main/java/org/apache/activemq/transport/tcp/SslTransportFactory.java index 0cee264e46..32e1122564 100644 --- a/activemq-core/src/main/java/org/apache/activemq/transport/tcp/SslTransportFactory.java +++ b/activemq-core/src/main/java/org/apache/activemq/transport/tcp/SslTransportFactory.java @@ -22,6 +22,7 @@ import java.net.URISyntaxException; import java.net.UnknownHostException; import java.security.KeyManagementException; import java.security.NoSuchAlgorithmException; +import java.security.NoSuchProviderException; import java.security.SecureRandom; import java.util.HashMap; import java.util.Map; @@ -40,6 +41,7 @@ import org.apache.activemq.broker.SslContext; import org.apache.activemq.openwire.OpenWireFormat; import org.apache.activemq.transport.InactivityMonitor; import org.apache.activemq.transport.Transport; +import org.apache.activemq.transport.TransportFactory; import org.apache.activemq.transport.TransportLoggerFactory; import org.apache.activemq.transport.TransportServer; import org.apache.activemq.transport.WireFormatNegotiator; @@ -60,20 +62,10 @@ import org.apache.commons.logging.LogFactory; * @author David Martin Clavo david(dot)martin(dot)clavo(at)gmail.com (logging improvement modifications) * @version $Revision$ */ -public class SslTransportFactory extends TcpTransportFactory implements BrokerServiceAware { +public class SslTransportFactory extends TcpTransportFactory { // The log this uses., private static final Log LOG = LogFactory.getLog(SslTransportFactory.class); - - // The context used to creat ssl sockets. - private SSLContext sslContext; - - - /** - * Constructor. Nothing special. - */ - public SslTransportFactory() { - } - + /** * Overriding to use SslTransportServer and allow for proper reflection. */ @@ -147,47 +139,25 @@ public class SslTransportFactory extends TcpTransportFactory implements BrokerSe return new SslTransport(wf, (SSLSocketFactory)socketFactory, location, localLocation, false); } - /** - * Sets the key and trust managers used in constructed socket factories. - * Passes given arguments to SSLContext.init(...). - * - * @param km The sources of authentication keys or null. - * @param tm The sources of peer authentication trust decisions or null. - * @param random The source of randomness for this generator or null. - */ - public void setKeyAndTrustManagers(KeyManager[] km, TrustManager[] tm, SecureRandom random) throws KeyManagementException { - // Killing old context and making a new one just to be safe. - try { - sslContext = SSLContext.getInstance("TLS"); - } catch (NoSuchAlgorithmException e) { - // This should not happen unless this class is improperly modified. - throw new RuntimeException("Unknown SSL algorithm encountered.", e); - } - sslContext.init(km, tm, random); - } - - public void setBrokerService(BrokerService brokerService) { - SslContext c = brokerService.getSslContext(); - if( sslContext == null && c!=null ) { - try { - setKeyAndTrustManagers(c.getKeyManagersAsArray(), c.getTrustManagersAsArray(), c.getSecureRandom()); - } catch (KeyManagementException e) { - throw new RuntimeException(e); - } - } - } + /** * Creates a new SSL ServerSocketFactory. The given factory will use * user-provided key and trust managers (if the user provided them). * * @return Newly created (Ssl)ServerSocketFactory. + * @throws IOException */ - protected ServerSocketFactory createServerSocketFactory() { - if (sslContext == null) { - return SSLServerSocketFactory.getDefault(); + protected ServerSocketFactory createServerSocketFactory() throws IOException { + if( SslContext.getCurrentSslContext()!=null ) { + SslContext ctx = SslContext.getCurrentSslContext(); + try { + return ctx.getSSLContext().getServerSocketFactory(); + } catch (Exception e) { + throw IOExceptionSupport.create(e); + } } else { - return sslContext.getServerSocketFactory(); + return SSLServerSocketFactory.getDefault(); } } @@ -196,13 +166,33 @@ public class SslTransportFactory extends TcpTransportFactory implements BrokerSe * key and trust managers (if the user provided them). * * @return Newly created (Ssl)SocketFactory. + * @throws IOException */ - protected SocketFactory createSocketFactory() { - if (sslContext == null) { - return SSLSocketFactory.getDefault(); + protected SocketFactory createSocketFactory() throws IOException { + + if( SslContext.getCurrentSslContext()!=null ) { + SslContext ctx = SslContext.getCurrentSslContext(); + try { + return ctx.getSSLContext().getSocketFactory(); + } catch (Exception e) { + throw IOExceptionSupport.create(e); + } } else { - return sslContext.getSocketFactory(); + return SSLSocketFactory.getDefault(); } + + } + + /** + * + * @param km + * @param tm + * @param object + * @deprecated "Do not use anymore... using static initializers like this method only allows the JVM to use 1 SSL configuration per broker." + */ + public void setKeyAndTrustManagers(KeyManager[] km, TrustManager[] tm, SecureRandom random) { + SslContext ctx = new SslContext(km, tm, random); + SslContext.setCurrentSslContext(ctx); } } diff --git a/activemq-core/src/main/java/org/apache/activemq/transport/tcp/TcpTransportFactory.java b/activemq-core/src/main/java/org/apache/activemq/transport/tcp/TcpTransportFactory.java index 12d5af4f25..0f82b73ba9 100755 --- a/activemq-core/src/main/java/org/apache/activemq/transport/tcp/TcpTransportFactory.java +++ b/activemq-core/src/main/java/org/apache/activemq/transport/tcp/TcpTransportFactory.java @@ -149,11 +149,11 @@ public class TcpTransportFactory extends TransportFactory { return new TcpTransport(wf, socketFactory, location, localLocation); } - protected ServerSocketFactory createServerSocketFactory() { + protected ServerSocketFactory createServerSocketFactory() throws IOException { return ServerSocketFactory.getDefault(); } - protected SocketFactory createSocketFactory() { + protected SocketFactory createSocketFactory() throws IOException { return SocketFactory.getDefault(); } } diff --git a/activemq-core/src/test/java/org/apache/activemq/transport/tcp/SslContextNBrokerServiceTest.java b/activemq-core/src/test/java/org/apache/activemq/transport/tcp/SslContextNBrokerServiceTest.java new file mode 100644 index 0000000000..79486ab834 --- /dev/null +++ b/activemq-core/src/test/java/org/apache/activemq/transport/tcp/SslContextNBrokerServiceTest.java @@ -0,0 +1,137 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.activemq.transport.tcp; + +import java.net.URI; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; +import java.util.Iterator; +import java.util.Map; + +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLSocket; +import javax.net.ssl.SSLSocketFactory; +import javax.net.ssl.TrustManager; +import javax.net.ssl.X509TrustManager; + +import junit.framework.TestCase; + +import org.apache.activemq.AMQDeadlockTest3; +import org.apache.activemq.broker.BrokerService; +import org.apache.activemq.broker.TransportConnector; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.springframework.context.support.ClassPathXmlApplicationContext; + + +public class SslContextNBrokerServiceTest extends TestCase { + private static final transient Log LOG = LogFactory.getLog(SslContextNBrokerServiceTest.class); + + private ClassPathXmlApplicationContext context; + Map beansOfType; + + public void testConfigurationIsolation() throws Exception { + + assertTrue("dummy bean has dummy cert", verifyCredentials("dummy")); + assertTrue("good bean has amq cert", verifyCredentials("activemq.org")); + } + + private boolean verifyCredentials(String name) throws Exception { + boolean result = false; + BrokerService broker = getBroker(name); + assertNotNull(name, broker); + broker.start(); + try { + result = verifySslCredentials(broker); + } finally { + broker.stop(); + } + return result; + } + + private boolean verifySslCredentials(BrokerService broker) throws Exception { + TransportConnector connector = broker.getTransportConnectors().get(0); + URI brokerUri = connector.getConnectUri(); + + SSLContext context = SSLContext.getInstance("TLS"); + CertChainCatcher catcher = new CertChainCatcher(); + context.init(null, new TrustManager[] {catcher}, null); + + SSLSocketFactory factory = context.getSocketFactory(); + SSLSocket socket = (SSLSocket)factory.createSocket(brokerUri.getHost(), brokerUri.getPort()); + socket.setSoTimeout(5000); + socket.startHandshake(); + socket.close(); + + boolean matches = false; + if (catcher.serverCerts != null) { + for (int i = 0; i < catcher.serverCerts.length; i++) { + X509Certificate cert = catcher.serverCerts[i]; + LOG.info(" " + (i + 1) + " Issuer " + cert.getIssuerDN()); + } + if (catcher.serverCerts.length > 0) { + String issuer = catcher.serverCerts[0].getIssuerDN().toString(); + if (issuer.indexOf(broker.getBrokerName()) != -1) { + matches = true; + } + } + } + return matches; + } + + + private BrokerService getBroker(String name) { + BrokerService result = null; + Iterator iterator = beansOfType.values().iterator(); + while(iterator.hasNext()) { + BrokerService candidate = (BrokerService)iterator.next(); + if (candidate.getBrokerName().equals(name)) { + result = candidate; + break; + } + } + return result; + } + + + protected void setUp() throws Exception { + //System.setProperty("javax.net.debug", "ssl"); + Thread.currentThread().setContextClassLoader(SslContextNBrokerServiceTest.class.getClassLoader()); + context = new ClassPathXmlApplicationContext("org/apache/activemq/transport/tcp/n-brokers-ssl.xml"); + beansOfType = context.getBeansOfType(BrokerService.class); + + } + + @Override + protected void tearDown() throws Exception { + context.destroy(); + } + + + class CertChainCatcher implements X509TrustManager { + X509Certificate[] serverCerts; + + public void checkClientTrusted(X509Certificate[] arg0, String arg1) throws CertificateException { + } + public void checkServerTrusted(X509Certificate[] arg0, String arg1) throws CertificateException { + serverCerts = arg0; + } + public X509Certificate[] getAcceptedIssuers() { + return null; + } + } +} diff --git a/activemq-core/src/test/resources/dummy.keystore b/activemq-core/src/test/resources/dummy.keystore new file mode 100644 index 0000000000000000000000000000000000000000..9f705e5c28e1b80137d8bbe203bf3ce234fc1712 GIT binary patch literal 1224 zcmezO_TO6u1_mY|W&~r_+{*0KN+3^4>$>$(Al+}!#Mo`X$Ht}2#>m2`#U#kc$jZRd z#8{q|^~z{t<#Da_sk{DcPgb(#+dKDWgudtU1EG=z{1(;4?vvB`&dodc^X0@1&rFXz zzxUF$wYBbPk;Mtq%Tmj>MwUbvUY?}Ae1W2n^|i*y&MO4?q+c95d;Q^$LcRa|#fmL+ z_WO1gP0zX7tWe!izr@|Lu`8z}tF!Oe8@@%W7Ku&z!K&mCDO-}48NJrGQm#d`gFknY ziuI%OrPmAM_k8L6WN7qd)^7uux%Y1;cJKSA&HeT*U#PDlOI&+x%aykbtN%>BF1fc$ z!qwI9l}rKOW#gkywI;6C_!TvM%0;!;_YXJRl3TD(fATWBkj)kutD{z}aBDOBQ^{B0 zx>9V(nFn_tU3jB5@y#kv^DoKgoV8c%XIJ#^ms)o|=3!~KM)sa(cdxFQ-25Y|@%R$g zsq9J>XNuiF#;dvPJMsHM*u7~hIoz#Uzcy*5s(hXBT#%G4|4dldm4J-^yjm!;AOiYaoqQG1WBU31s*48&M?f@oD9hiq1fgE|J#zuy} z^}&s@r9zrP_r2!my?OHdo8a~!oA)jZhvn=WwgswXy2d&v1^$k-?Tz@g|LE6U2H#H4 z4$ZmVXx$)dlK&|!VSDz>fTc_3&ws&EEueJBfA`Ak|F$2K{$U;d{Md?RqWmnso4;K? zDtDO4G(Z%Pl04DZ`@CDQ*{A-h0h z!xm4mLgg1e8e+oR&vP2}oLKhIMy#qkq^4Kn!Qypt+MAb^PRv_e&;3i$H0k5-q$)-g z>oZF}?pqN!p>sv^{fALc=d(@fPK#@(4pmKI^s)YT-cyUYv6TVjj;`32#NJ=Crv=L1 ze>qvSW>Kwa`_b^KWAd9AW1RBsuc~eBGHzaX=k}aO|K~|${mgj$b)ELhhKkP2!Un@|9744(?OFS-Ch4PcHMzLe}!$_)*X}0j!`Oob^G96naOHw0AFtUPXGV_ literal 0 HcmV?d00001 diff --git a/activemq-core/src/test/resources/org/apache/activemq/transport/tcp/n-brokers-ssl.xml b/activemq-core/src/test/resources/org/apache/activemq/transport/tcp/n-brokers-ssl.xml new file mode 100644 index 0000000000..4bd5fc7cdb --- /dev/null +++ b/activemq-core/src/test/resources/org/apache/activemq/transport/tcp/n-brokers-ssl.xml @@ -0,0 +1,51 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + +