QPID-3429: ensure that SSL is enabled correctly in MinaNetworkHandler. Refactor SSLContextFactory to be a factory, and present a useful interface for both client and server side use. Added keystore for the Java broker, renamed existing client trust/key stores for clarity. Fix SSL port configuration. Added new SSL tests, and ensure these are *always* run in the Java 0-10 profiles.

Committing work by myself and Keith Wall.

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk/qpid@1159250 13f79535-47bb-0310-9956-ffa450edef68

9 files changed, 118 insertions, 197 deletions

diff --git a/java/common/src/main/java/org/apache/qpid/ssl/SSLContextFactory.java b/java/common/src/main/java/org/apache/qpid/ssl/SSLContextFactory.java
index 702746b3da..01f13408b0 100644
--- a/java/common/src/main/java/org/apache/qpid/ssl/SSLContextFactory.java
+++ b/java/common/src/main/java/org/apache/qpid/ssl/SSLContextFactory.java
@@ -20,18 +20,17 @@
  */
 package org.apache.qpid.ssl;
 
-import java.io.File;
-import java.io.FileInputStream;
 import java.io.IOException;
-import java.io.InputStream;
 import java.security.GeneralSecurityException;
 import java.security.KeyStore;
 
 import javax.net.ssl.KeyManager;
 import javax.net.ssl.KeyManagerFactory;
 import javax.net.ssl.SSLContext;
+import javax.net.ssl.TrustManager;
 import javax.net.ssl.TrustManagerFactory;
 
+import org.apache.qpid.transport.network.security.ssl.QpidClientX509KeyManager;
 import org.apache.qpid.transport.network.security.ssl.SSLUtil;
 
 /**
@@ -39,157 +38,92 @@ import org.apache.qpid.transport.network.security.ssl.SSLUtil;
  * before this will work.
  * 
  */
-public class SSLContextFactory {
-	
-	/**
-	 * Path to the Java keystore file
-	 */
-	private String _keyStorePath;
-	
-	/**
-	 * Password for the keystore
-	 */
-	private String _keyStorePassword;
-	
-	/**
-	 * Cert type to use in keystore
-	 */
-	private String _keyStoreCertType;
-	
-	/**
-     * Path to the Java truststore file
-     */
-    private String _trustStorePath;
-    
-    /**
-     * Password for the truststore
-     */
-    private String _trustStorePassword;
-    
-    /**
-     * Cert type to use in truststore
-     */
-    private String _trustStoreCertType;
-    
-	private KeyManager customKeyManager;
-    
-    public SSLContextFactory(String trustStorePath, String trustStorePassword,
-            String trustStoreCertType) 
+public class SSLContextFactory
+{
+    public static final String JAVA_KEY_STORE_CODE = "JKS";
+    public static final String TRANSPORT_LAYER_SECURITY_CODE = "TLS";
+    public static final String KEY_STORE_CERTIFICATE_TYPE = "SunX509";
+
+    private SSLContextFactory()
     {
-        this(trustStorePath,trustStorePassword,trustStoreCertType,
-                          trustStorePath,trustStorePassword,trustStoreCertType);
+        //no instances
     }
 
-    /**
-	 * Create a factory instance
-	 * @param keystorePath path to the Java keystore file
-	 * @param keystorePassword password for the Java keystore
-	 * @param certType certificate type
-	 */
-	public SSLContextFactory(String trustStorePath, String trustStorePassword, String trustStoreCertType,
-            String keyStorePath, String keyStorePassword, String keyStoreCertType) 
-	{
+    public static SSLContext buildServerContext(final String keyStorePath,
+            final String keyStorePassword, final String keyStoreCertType)
+            throws GeneralSecurityException, IOException
+    {
+        return buildContext(null, null, null, keyStorePath, keyStorePassword,
+                keyStoreCertType, null);
+    }
 
-	    _trustStorePath = trustStorePath;
-        _trustStorePassword = trustStorePassword;
-                
-        if (_trustStorePassword != null && _trustStorePassword.equals("none"))
-        {
-            _trustStorePassword = null;
-        }
-        _trustStoreCertType = trustStoreCertType;
-        
-	    _keyStorePath = keyStorePath;
-		_keyStorePassword = keyStorePassword;
-				
-		if (_keyStorePassword != null && _keyStorePassword.equals("none"))
-		{
-			_keyStorePassword = null;
-		}
-		_keyStoreCertType = keyStoreCertType;
-		
-		if (_trustStorePath == null) {
-			throw new IllegalArgumentException("A TrustStore path or KeyStore path must be specified");
-		}
-		if (_trustStoreCertType == null) {
-			throw new IllegalArgumentException("Cert type must be specified");
-		}
-	}
-	
-	public SSLContextFactory(String trustStorePath, String trustStorePassword, String trustStoreCertType,
-	                         KeyManager customKeyManager) 
+    public static SSLContext buildClientContext(final String trustStorePath,
+            final String trustStorePassword, final String trustStoreCertType,
+            final String keyStorePath, final String keyStorePassword,
+            final String keyStoreCertType, final String certAlias)
+            throws GeneralSecurityException, IOException
+    {
+        return buildContext(trustStorePath, trustStorePassword,
+                trustStoreCertType, keyStorePath, keyStorePassword,
+                keyStoreCertType, certAlias);
+    }
+    
+    private static SSLContext buildContext(final String trustStorePath,
+            final String trustStorePassword, final String trustStoreCertType,
+            final String keyStorePath, final String keyStorePassword,
+            final String keyStoreCertType, final String certAlias)
+            throws GeneralSecurityException, IOException
     {
+        // Initialize the SSLContext to work with our key managers.
+        final SSLContext sslContext = SSLContext
+                .getInstance(TRANSPORT_LAYER_SECURITY_CODE);
 
-        _trustStorePath = trustStorePath;
-        _trustStorePassword = trustStorePassword;
-                
-        if (_trustStorePassword != null && _trustStorePassword.equals("none"))
+        final TrustManager[] trustManagers;
+        final KeyManager[] keyManagers;
+
+        if (trustStorePath != null)
         {
-            _trustStorePassword = null;
-        }
-        _trustStoreCertType = trustStoreCertType;
-        
-        if (_trustStorePath == null) {
-            throw new IllegalArgumentException("A TrustStore path or KeyStore path must be specified");
+            final KeyStore ts = SSLUtil.getInitializedKeyStore(trustStorePath,
+                    trustStorePassword);
+            final TrustManagerFactory tmf = TrustManagerFactory
+                    .getInstance(trustStoreCertType);
+            tmf.init(ts);
+
+            trustManagers = tmf.getTrustManagers();
         }
-        if (_trustStoreCertType == null) {
-            throw new IllegalArgumentException("Cert type must be specified");
+        else
+        {
+            trustManagers = null;
         }
-        
-        this.customKeyManager = customKeyManager;
-    }
-	
-	
-	/**
-	 * Builds a SSLContext appropriate for use with a server
-	 * @return SSLContext
-	 * @throws GeneralSecurityException
-	 * @throws IOException
-	 */
 
-	public SSLContext buildServerContext() throws GeneralSecurityException, IOException
-	{
-        KeyStore ts = SSLUtil.getInitializedKeyStore(_trustStorePath,_trustStorePassword);
-        TrustManagerFactory tmf = TrustManagerFactory.getInstance(_trustStoreCertType);
-        tmf.init(ts);
-        
-        // Initialize the SSLContext to work with our key managers.
-        SSLContext sslContext = SSLContext.getInstance("TLS");
-        
-        if (customKeyManager != null)
+        if (keyStorePath != null)
         {
-            sslContext.init(new KeyManager[]{customKeyManager},
-                            tmf.getTrustManagers(), null);
-            
+            if (certAlias != null)
+            {
+                keyManagers = new KeyManager[] { new QpidClientX509KeyManager(
+                        certAlias, keyStorePath, keyStorePassword,
+                        keyStoreCertType) };
+            }
+            else
+            {
+                final KeyStore ks = SSLUtil.getInitializedKeyStore(
+                        keyStorePath, keyStorePassword);
+
+                char[] keyStoreCharPassword = keyStorePassword == null ? null : keyStorePassword.toCharArray();
+                // Set up key manager factory to use our key store
+                final KeyManagerFactory kmf = KeyManagerFactory
+                        .getInstance(keyStoreCertType);
+                kmf.init(ks, keyStoreCharPassword);
+                keyManagers = kmf.getKeyManagers();
+            }
         }
         else
         {
-            // Create keystore
-            KeyStore ks = SSLUtil.getInitializedKeyStore(_keyStorePath,_keyStorePassword);
-            // Set up key manager factory to use our key store
-            KeyManagerFactory kmf = KeyManagerFactory.getInstance(_keyStoreCertType);
-            kmf.init(ks, _keyStorePassword.toCharArray());
-
-            sslContext.init(kmf.getKeyManagers(), tmf.getTrustManagers(), null);    
+            keyManagers = null;
         }
-        
-        return sslContext;		
-	}
-	
-	/**
-	 * Creates a SSLContext factory appropriate for use with a client
-	 * @return SSLContext
-	 * @throws GeneralSecurityException
-	 * @throws IOException
-	 */
-	public SSLContext buildClientContext() throws GeneralSecurityException, IOException
-	{
-		KeyStore ks = SSLUtil.getInitializedKeyStore(_trustStorePath,_trustStorePassword);
-        TrustManagerFactory tmf = TrustManagerFactory.getInstance(_trustStoreCertType);
-        tmf.init(ks);
-        SSLContext context = SSLContext.getInstance("TLS");
-        context.init(null, tmf.getTrustManagers(), null);
-        return context;		
-	}
-	
+
+        sslContext.init(keyManagers, trustManagers, null);
+
+        return sslContext;
+    }
 }
diff --git a/java/common/src/main/java/org/apache/qpid/transport/network/IncomingNetworkTransport.java b/java/common/src/main/java/org/apache/qpid/transport/network/IncomingNetworkTransport.java
index 7099916c33..b371df639e 100644
--- a/java/common/src/main/java/org/apache/qpid/transport/network/IncomingNetworkTransport.java
+++ b/java/common/src/main/java/org/apache/qpid/transport/network/IncomingNetworkTransport.java
@@ -20,11 +20,12 @@
  */
 package org.apache.qpid.transport.network;
 
+import javax.net.ssl.SSLContext;
+
 import org.apache.qpid.protocol.ProtocolEngineFactory;
-import org.apache.qpid.ssl.SSLContextFactory;
 import org.apache.qpid.transport.NetworkTransportConfiguration;
 
 public interface IncomingNetworkTransport extends NetworkTransport
 {
-    public void accept(NetworkTransportConfiguration config, ProtocolEngineFactory factory, SSLContextFactory sslFactory);
+    public void accept(NetworkTransportConfiguration config, ProtocolEngineFactory factory, SSLContext sslContext);
 }
\ No newline at end of file
diff --git a/java/common/src/main/java/org/apache/qpid/transport/network/OutgoingNetworkTransport.java b/java/common/src/main/java/org/apache/qpid/transport/network/OutgoingNetworkTransport.java
index ff86ba481f..0bae46e8eb 100644
--- a/java/common/src/main/java/org/apache/qpid/transport/network/OutgoingNetworkTransport.java
+++ b/java/common/src/main/java/org/apache/qpid/transport/network/OutgoingNetworkTransport.java
@@ -22,11 +22,12 @@ package org.apache.qpid.transport.network;
 
 import java.nio.ByteBuffer;
 
-import org.apache.qpid.ssl.SSLContextFactory;
+import javax.net.ssl.SSLContext;
+
 import org.apache.qpid.transport.ConnectionSettings;
 import org.apache.qpid.transport.Receiver;
 
 public interface OutgoingNetworkTransport extends NetworkTransport
 {
-    public NetworkConnection connect(ConnectionSettings settings, Receiver<ByteBuffer> delegate, SSLContextFactory sslFactory);
+    public NetworkConnection connect(ConnectionSettings settings, Receiver<ByteBuffer> delegate, SSLContext sslContext);
 }
\ No newline at end of file
diff --git a/java/common/src/main/java/org/apache/qpid/transport/network/io/IoNetworkTransport.java b/java/common/src/main/java/org/apache/qpid/transport/network/io/IoNetworkTransport.java
index d611ab1cf3..30e2856c59 100644
--- a/java/common/src/main/java/org/apache/qpid/transport/network/io/IoNetworkTransport.java
+++ b/java/common/src/main/java/org/apache/qpid/transport/network/io/IoNetworkTransport.java
@@ -27,7 +27,8 @@ import java.net.Socket;
 import java.net.SocketException;
 import java.nio.ByteBuffer;
 
-import org.apache.qpid.ssl.SSLContextFactory;
+import javax.net.ssl.SSLContext;
+
 import org.apache.qpid.transport.ConnectionSettings;
 import org.apache.qpid.transport.Receiver;
 import org.apache.qpid.transport.TransportException;
@@ -51,7 +52,7 @@ public class IoNetworkTransport implements OutgoingNetworkTransport
     private IoNetworkConnection _connection;
     private long _timeout = 60000;
     
-    public NetworkConnection connect(ConnectionSettings settings, Receiver<ByteBuffer> delegate, SSLContextFactory sslFactory)
+    public NetworkConnection connect(ConnectionSettings settings, Receiver<ByteBuffer> delegate, SSLContext sslContext)
     {
         int sendBufferSize = settings.getWriteBufferSize();
         int receiveBufferSize = settings.getReadBufferSize();
diff --git a/java/common/src/main/java/org/apache/qpid/transport/network/mina/MinaNetworkHandler.java b/java/common/src/main/java/org/apache/qpid/transport/network/mina/MinaNetworkHandler.java
index c00187480c..ce461a6342 100644
--- a/java/common/src/main/java/org/apache/qpid/transport/network/mina/MinaNetworkHandler.java
+++ b/java/common/src/main/java/org/apache/qpid/transport/network/mina/MinaNetworkHandler.java
@@ -21,6 +21,8 @@
 
 package org.apache.qpid.transport.network.mina;
 
+import javax.net.ssl.SSLContext;
+
 import org.apache.mina.common.ByteBuffer;
 import org.apache.mina.common.IdleStatus;
 import org.apache.mina.common.IoHandlerAdapter;
@@ -30,7 +32,6 @@ import org.apache.mina.filter.SSLFilter;
 import org.apache.mina.util.SessionUtil;
 import org.apache.qpid.protocol.ProtocolEngine;
 import org.apache.qpid.protocol.ProtocolEngineFactory;
-import org.apache.qpid.ssl.SSLContextFactory;
 import org.apache.qpid.transport.network.NetworkConnection;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -40,7 +41,7 @@ public class MinaNetworkHandler extends IoHandlerAdapter
     private static final Logger LOGGER = LoggerFactory.getLogger(MinaNetworkHandler.class);
 
     private ProtocolEngineFactory _factory;
-    private SSLContextFactory _sslFactory = null;
+    private SSLContext _sslContext = null;
 
     static
     {
@@ -52,15 +53,15 @@ public class MinaNetworkHandler extends IoHandlerAdapter
         ByteBuffer.setAllocator(new SimpleByteBufferAllocator());
     }
 
-    public MinaNetworkHandler(SSLContextFactory sslFactory, ProtocolEngineFactory factory)
+    public MinaNetworkHandler(SSLContext sslContext, ProtocolEngineFactory factory)
     {
-        _sslFactory = sslFactory;
+        _sslContext = sslContext;
         _factory = factory;
     }
 
-    public MinaNetworkHandler(SSLContextFactory sslFactory)
+    public MinaNetworkHandler(SSLContext sslContext)
     {
-        this(sslFactory, null);
+        this(sslContext, null);
     }
 
     public void messageReceived(IoSession session, Object message)
@@ -100,10 +101,10 @@ public class MinaNetworkHandler extends IoHandlerAdapter
 
         SessionUtil.initialize(ioSession);
 
-        if (_sslFactory != null)
+        if (_sslContext != null)
         {
-            ioSession.getFilterChain().addBefore("protocolFilter", "sslFilter",
-                    new SSLFilter(_sslFactory.buildServerContext()));
+            ioSession.getFilterChain().addFirst("sslFilter",
+                            new SSLFilter(_sslContext));
         }
 
         if (_factory != null)
diff --git a/java/common/src/main/java/org/apache/qpid/transport/network/mina/MinaNetworkTransport.java b/java/common/src/main/java/org/apache/qpid/transport/network/mina/MinaNetworkTransport.java
index d0367b82f4..85b42da2b2 100644
--- a/java/common/src/main/java/org/apache/qpid/transport/network/mina/MinaNetworkTransport.java
+++ b/java/common/src/main/java/org/apache/qpid/transport/network/mina/MinaNetworkTransport.java
@@ -26,6 +26,8 @@ import java.io.IOException;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 
+import javax.net.ssl.SSLContext;
+
 import org.apache.mina.common.ConnectFuture;
 import org.apache.mina.common.ExecutorThreadModel;
 import org.apache.mina.common.IoConnector;
@@ -62,7 +64,7 @@ public class MinaNetworkTransport implements OutgoingNetworkTransport, IncomingN
     private InetSocketAddress _address;
 
     public NetworkConnection connect(ConnectionSettings settings,
-            Receiver<java.nio.ByteBuffer> delegate, SSLContextFactory sslFactory)
+            Receiver<java.nio.ByteBuffer> delegate, SSLContext sslContext)
     {
         int transport = getTransport(settings.getProtocol());
         
@@ -77,7 +79,7 @@ public class MinaNetworkTransport implements OutgoingNetworkTransport, IncomingN
                         return new SocketConnector(1, new QpidThreadExecutor()); // non-blocking connector
                     }
                 });
-                _connection = stc.connect(delegate, settings, sslFactory);
+                _connection = stc.connect(delegate, settings, sslContext);
                 break;
             case UNKNOWN:
             default:
@@ -115,7 +117,7 @@ public class MinaNetworkTransport implements OutgoingNetworkTransport, IncomingN
     }
 
     public void accept(final NetworkTransportConfiguration config, final ProtocolEngineFactory factory,
-            final SSLContextFactory sslFactory)
+            final SSLContext sslContext)
     {
         int processors = config.getConnectorProcessors();
         
@@ -146,7 +148,7 @@ public class MinaNetworkTransport implements OutgoingNetworkTransport, IncomingN
 
         try
         {
-            _acceptor.bind(_address, new MinaNetworkHandler(sslFactory, factory));
+            _acceptor.bind(_address, new MinaNetworkHandler(sslContext, factory));
         }
         catch (IOException e)
         {
@@ -168,7 +170,7 @@ public class MinaNetworkTransport implements OutgoingNetworkTransport, IncomingN
             _ioConnectorFactory = socketConnectorFactory;
         }
         
-        public NetworkConnection connect(Receiver<java.nio.ByteBuffer> receiver, ConnectionSettings settings, SSLContextFactory sslFactory)
+        public NetworkConnection connect(Receiver<java.nio.ByteBuffer> receiver, ConnectionSettings settings, SSLContext sslContext)
         {
             final IoConnector ioConnector = _ioConnectorFactory.newConnector();
             final SocketAddress address;
@@ -203,7 +205,7 @@ public class MinaNetworkTransport implements OutgoingNetworkTransport, IncomingN
                 ((SocketConnector) ioConnector).setWorkerTimeout(0);
             }
 
-            ConnectFuture future = ioConnector.connect(address, new MinaNetworkHandler(sslFactory), ioConnector.getDefaultConfig());
+            ConnectFuture future = ioConnector.connect(address, new MinaNetworkHandler(sslContext), ioConnector.getDefaultConfig());
             future.join();
             if (!future.isConnected())
             {
diff --git a/java/common/src/main/java/org/apache/qpid/transport/network/security/SecurityLayer.java b/java/common/src/main/java/org/apache/qpid/transport/network/security/SecurityLayer.java
index 3479aaa42a..e80f8904a3 100644
--- a/java/common/src/main/java/org/apache/qpid/transport/network/security/SecurityLayer.java
+++ b/java/common/src/main/java/org/apache/qpid/transport/network/security/SecurityLayer.java
@@ -25,6 +25,7 @@ import java.nio.ByteBuffer;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLEngine;
 
+import org.apache.qpid.ssl.SSLContextFactory;
 import org.apache.qpid.transport.Connection;
 import org.apache.qpid.transport.ConnectionListener;
 import org.apache.qpid.transport.ConnectionSettings;
@@ -114,7 +115,14 @@ public class SecurityLayer
             SSLContext sslCtx;
             try
             {
-                sslCtx = SSLUtil.createSSLContext(settings);
+                sslCtx = SSLContextFactory
+                        .buildClientContext(settings.getTrustStorePath(),
+                                settings.getTrustStorePassword(),
+                                settings.getTrustStoreCertType(),
+                                settings.getKeyStorePath(),
+                                settings.getKeyStorePassword(),
+                                settings.getKeyStoreCertType(),
+                                settings.getCertAlias());
             }
             catch (Exception e)
             {
diff --git a/java/common/src/main/java/org/apache/qpid/transport/network/security/ssl/QpidClientX509KeyManager.java b/java/common/src/main/java/org/apache/qpid/transport/network/security/ssl/QpidClientX509KeyManager.java
index 0dd86d4560..4391e8adfc 100644
--- a/java/common/src/main/java/org/apache/qpid/transport/network/security/ssl/QpidClientX509KeyManager.java
+++ b/java/common/src/main/java/org/apache/qpid/transport/network/security/ssl/QpidClientX509KeyManager.java
@@ -20,7 +20,9 @@
  */
 package org.apache.qpid.transport.network.security.ssl;
 
+import java.io.IOException;
 import java.net.Socket;
+import java.security.GeneralSecurityException;
 import java.security.KeyStore;
 import java.security.Principal;
 import java.security.PrivateKey;
@@ -40,7 +42,7 @@ public class QpidClientX509KeyManager extends X509ExtendedKeyManager
     String alias;
     
     public QpidClientX509KeyManager(String alias, String keyStorePath,
-                           String keyStorePassword,String keyStoreCertType) throws Exception
+                           String keyStorePassword,String keyStoreCertType) throws GeneralSecurityException, IOException
     {
         this.alias = alias;    
         KeyStore ks = SSLUtil.getInitializedKeyStore(keyStorePath,keyStorePassword);
diff --git a/java/common/src/main/java/org/apache/qpid/transport/network/security/ssl/SSLUtil.java b/java/common/src/main/java/org/apache/qpid/transport/network/security/ssl/SSLUtil.java
index fd73915b65..6bb038a581 100644
--- a/java/common/src/main/java/org/apache/qpid/transport/network/security/ssl/SSLUtil.java
+++ b/java/common/src/main/java/org/apache/qpid/transport/network/security/ssl/SSLUtil.java
@@ -125,38 +125,6 @@ public class SSLUtil
         return id.toString();
     }
     
-    public static SSLContext createSSLContext(ConnectionSettings settings) throws Exception
-    {
-        SSLContextFactory sslContextFactory;
-        
-        if (settings.getCertAlias() == null)
-        {
-            sslContextFactory = 
-                new SSLContextFactory(settings.getTrustStorePath(),
-                                      settings.getTrustStorePassword(),
-                                      settings.getTrustStoreCertType(),
-                                      settings.getKeyStorePath(),
-                                      settings.getKeyStorePassword(),
-                                      settings.getKeyStoreCertType());
-
-        } else
-        {
-            sslContextFactory = 
-                new SSLContextFactory(settings.getTrustStorePath(),
-                                      settings.getTrustStorePassword(),
-                                      settings.getTrustStoreCertType(),
-                    new QpidClientX509KeyManager(settings.getCertAlias(),
-                                                     settings.getKeyStorePath(),
-                                                     settings.getKeyStorePassword(),
-                                                     settings.getKeyStoreCertType()));
-            
-            log.debug("Using custom key manager");
-        }
-
-        return sslContextFactory.buildServerContext();
-        
-    }
-    
     public static KeyStore getInitializedKeyStore(String storePath, String storePassword) throws GeneralSecurityException, IOException
     {
         KeyStore ks = KeyStore.getInstance("JKS");
@@ -176,7 +144,10 @@ public class SSLUtil
             {
                 throw new IOException("Unable to load keystore resource: " + storePath);
             }
-            ks.load(in, storePassword.toCharArray());
+
+            char[] storeCharPassword = storePassword == null ? null : storePassword.toCharArray();
+
+            ks.load(in, storeCharPassword);
         }
         finally
         {