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@1159250 13f79535-47bb-0310-9956-ffa450edef68

6 files changed, 230 insertions, 158 deletions

diff --git a/qpid/java/systests/etc/config-systests-settings.xml b/qpid/java/systests/etc/config-systests-settings.xml
index 751ff133cb..5ed208bfe7 100644
--- a/qpid/java/systests/etc/config-systests-settings.xml
+++ b/qpid/java/systests/etc/config-systests-settings.xml
@@ -20,11 +20,20 @@
  -
  -->
 <broker>
+    <connector>
+        <ssl>
+            <port>15671</port>
+            <enabled>false</enabled>
+            <sslOnly>false</sslOnly>
+            <keystorePath>${QPID_HOME}/../test-profiles/test_resources/ssl/java_broker_keystore.jks</keystorePath>
+            <keystorePassword>password</keystorePassword>
+        </ssl>
+    </connector>
     <management>
         <enabled>false</enabled>
         <ssl>
             <enabled>false</enabled>
-            <keyStorePath>${QPID_HOME}/../test-profiles/test_resources/ssl/keystore.jks</keyStorePath>
+            <keyStorePath>${QPID_HOME}/../test-profiles/test_resources/ssl/java_broker_keystore.jks</keyStorePath>
             <keyStorePassword>password</keyStorePassword>
         </ssl>
     </management>
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/client/ssl/SSLTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/client/ssl/SSLTest.java
index 8cdf12eaa4..471ebb16fc 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/client/ssl/SSLTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/client/ssl/SSLTest.java
@@ -31,62 +31,72 @@ import org.apache.qpid.test.utils.QpidBrokerTestCase;
 import org.apache.qpid.transport.Connection;
 
 public class SSLTest extends QpidBrokerTestCase
-{      
-    
+{
+    private static final String KEYSTORE = TEST_RESOURCES_DIR + "/ssl/java_client_keystore.jks";
+    private static final String KEYSTORE_PASSWORD = "password";
+    private static final String TRUSTSTORE = TEST_RESOURCES_DIR + "/ssl/java_client_truststore.jks";
+    private static final String TRUSTSTORE_PASSWORD = "password";
+    private static final String CERT_ALIAS_APP1 = "app1";
+    private static final String CERT_ALIAS_APP2 = "app2";
+
     @Override
     protected void setUp() throws Exception
     {
-        System.setProperty("javax.net.debug", "ssl");
+        if(isJavaBroker())
+        {
+            setTestClientSystemProperty("profile.use_ssl", "true");
+            setConfigurationProperty("connector.ssl.enabled", "true");
+            setConfigurationProperty("connector.ssl.sslOnly", "true");
+        }
+
+        // set the ssl system properties
+        setSystemProperty("javax.net.ssl.keyStore", KEYSTORE);
+        setSystemProperty("javax.net.ssl.keyStorePassword", KEYSTORE_PASSWORD);
+        setSystemProperty("javax.net.ssl.trustStore", TRUSTSTORE);
+        setSystemProperty("javax.net.ssl.trustStorePassword", TRUSTSTORE_PASSWORD);
+        setSystemProperty("javax.net.debug", "ssl");
         super.setUp();
     }
 
-    @Override
-    protected void tearDown() throws Exception
-    {
-        System.setProperty("javax.net.debug", "");
-        super.tearDown();
-    }
-        
-    public void testCreateSSLContextFromConnectionURLParams()
+    public void testCreateSSLConnectionUsingConnectionURLParams() throws Exception
     {
         if (Boolean.getBoolean("profile.use_ssl"))
-        {   
+        {
+            // Clear the ssl system properties
+            setSystemProperty("javax.net.ssl.keyStore", null);
+            setSystemProperty("javax.net.ssl.keyStorePassword", null);
+            setSystemProperty("javax.net.ssl.trustStore", null);
+            setSystemProperty("javax.net.ssl.trustStorePassword", null);
+            
             String url = "amqp://guest:guest@test/?brokerlist='tcp://localhost:%s" +
             "?ssl='true'&ssl_verify_hostname='true'" + 
             "&key_store='%s'&key_store_password='%s'" +
             "&trust_store='%s'&trust_store_password='%s'" +
             "'";
             
-            String keyStore = System.getProperty("javax.net.ssl.keyStore");
-            String keyStorePass = System.getProperty("javax.net.ssl.keyStorePassword");
-            String trustStore = System.getProperty("javax.net.ssl.trustStore");
-            String trustStorePass = System.getProperty("javax.net.ssl.trustStorePassword");
+            url = String.format(url,QpidBrokerTestCase.DEFAULT_SSL_PORT,
+                    KEYSTORE,KEYSTORE_PASSWORD,TRUSTSTORE,TRUSTSTORE_PASSWORD);
             
-            url = String.format(url,System.getProperty("test.port.ssl"),
-                    keyStore,keyStorePass,trustStore,trustStorePass);
+            AMQConnection con = new AMQConnection(url);
+            assertNotNull("connection should be successful", con);
+            Session ssn = con.createSession(false,Session.AUTO_ACKNOWLEDGE); 
+            assertNotNull("create session should be successful", ssn);
+        }        
+    }
+
+    public void testCreateSSLConnectionUsingSystemProperties() throws Exception
+    {
+        if (Boolean.getBoolean("profile.use_ssl"))
+        {
+
+            String url = "amqp://guest:guest@test/?brokerlist='tcp://localhost:%s?ssl='true''";
+
+            url = String.format(url,QpidBrokerTestCase.DEFAULT_SSL_PORT);
             
-            // temporarily set the trust/key store jvm args to something else
-            // to ensure we only read from the connection URL param.
-            System.setProperty("javax.net.ssl.trustStore","fessgsdgd");
-            System.setProperty("javax.net.ssl.trustStorePassword","fessgsdgd");
-            System.setProperty("javax.net.ssl.keyStore","fessgsdgd");
-            System.setProperty("javax.net.ssl.keyStorePassword","fessgsdgd");
-            try
-            {
-                AMQConnection con = new AMQConnection(url);
-                Session ssn = con.createSession(false,Session.AUTO_ACKNOWLEDGE); 
-            }
-            catch (Exception e)
-            {
-                fail("SSL Connection should be successful");
-            }
-            finally
-            {
-                System.setProperty("javax.net.ssl.trustStore",trustStore);
-                System.setProperty("javax.net.ssl.trustStorePassword",trustStorePass);
-                System.setProperty("javax.net.ssl.keyStore",keyStore);
-                System.setProperty("javax.net.ssl.keyStorePassword",keyStorePass);
-            }
+            AMQConnection con = new AMQConnection(url);
+            assertNotNull("connection should be successful", con);
+            Session ssn = con.createSession(false,Session.AUTO_ACKNOWLEDGE); 
+            assertNotNull("create session should be successful", ssn);
         }        
     }
 
@@ -95,8 +105,8 @@ public class SSLTest extends QpidBrokerTestCase
         if (Boolean.getBoolean("profile.use_ssl"))
         {
             String url = "amqp://guest:guest@test/?brokerlist='tcp://localhost:" + 
-            System.getProperty("test.port.ssl") + 
-            "?ssl='true'&ssl_cert_alias='app1''";
+            QpidBrokerTestCase.DEFAULT_SSL_PORT + 
+            "?ssl='true'&ssl_cert_alias='" + CERT_ALIAS_APP1 + "''";
             
             AMQTestConnection_0_10 con = new AMQTestConnection_0_10(url);      
             Connection transportCon = con.getConnection();
@@ -105,8 +115,8 @@ public class SSLTest extends QpidBrokerTestCase
             con.close();
             
             url = "amqp://guest:guest@test/?brokerlist='tcp://localhost:" + 
-            System.getProperty("test.port.ssl") + 
-            "?ssl='true'&ssl_cert_alias='app2''";
+            QpidBrokerTestCase.DEFAULT_SSL_PORT + 
+            "?ssl='true'&ssl_cert_alias='" + CERT_ALIAS_APP2 + "''";
             
             con = new AMQTestConnection_0_10(url);      
             transportCon = con.getConnection();
@@ -116,12 +126,12 @@ public class SSLTest extends QpidBrokerTestCase
         }        
     }
     
-    public void testVerifyHostName()
+    public void testVerifyHostNameWithIncorrectHostname()
     {
         if (Boolean.getBoolean("profile.use_ssl"))
         {
             String url = "amqp://guest:guest@test/?brokerlist='tcp://127.0.0.1:" + 
-            System.getProperty("test.port.ssl") + 
+            QpidBrokerTestCase.DEFAULT_SSL_PORT + 
             "?ssl='true'&ssl_verify_hostname='true''";
             
             try
@@ -140,42 +150,53 @@ public class SSLTest extends QpidBrokerTestCase
         }        
     }
     
-    public void testVerifyLocalHost()
+    public void testVerifyLocalHost() throws Exception
     {
         if (Boolean.getBoolean("profile.use_ssl"))
         {
             String url = "amqp://guest:guest@test/?brokerlist='tcp://localhost:" + 
-            System.getProperty("test.port.ssl") + 
+            QpidBrokerTestCase.DEFAULT_SSL_PORT + 
             "?ssl='true'&ssl_verify_hostname='true''";
-            
-            try
-            {
-                AMQConnection con = new AMQConnection(url);
-            }
-            catch (Exception e)
-            {
-                fail("Hostname verification should succeed");
-            }            
-        }        
+
+            AMQConnection con = new AMQConnection(url);
+            assertNotNull("connection should have been created", con); 
+        }
     }
     
-    public void testVerifyLocalHostLocalDomain()
+    public void testVerifyLocalHostLocalDomain() throws Exception
     {
         if (Boolean.getBoolean("profile.use_ssl"))
         {
             String url = "amqp://guest:guest@test/?brokerlist='tcp://localhost.localdomain:" + 
-            System.getProperty("test.port.ssl") + 
+            QpidBrokerTestCase.DEFAULT_SSL_PORT + 
             "?ssl='true'&ssl_verify_hostname='true''";
+
+            AMQConnection con = new AMQConnection(url);
+            assertNotNull("connection should have been created", con);
+        }        
+    }
+
+    public void testCreateSSLConnectionUsingConnectionURLParamsTrustStoreOnly() throws Exception
+    {
+        if (Boolean.getBoolean("profile.use_ssl"))
+        {
+            // Clear the ssl system properties
+            setSystemProperty("javax.net.ssl.keyStore", null);
+            setSystemProperty("javax.net.ssl.keyStorePassword", null);
+            setSystemProperty("javax.net.ssl.trustStore", null);
+            setSystemProperty("javax.net.ssl.trustStorePassword", null);
             
-            try
-            {
-                AMQConnection con = new AMQConnection(url);
-            }
-            catch (Exception e)
-            {
-                fail("Hostname verification should succeed");
-            }
-            
+            String url = "amqp://guest:guest@test/?brokerlist='tcp://localhost:%s" +
+            "?ssl='true'&ssl_verify_hostname='true'" + 
+            "&trust_store='%s'&trust_store_password='%s'" +
+            "'";
+
+            url = String.format(url,QpidBrokerTestCase.DEFAULT_SSL_PORT, TRUSTSTORE,TRUSTSTORE_PASSWORD);
+
+            AMQConnection con = new AMQConnection(url);
+            assertNotNull("connection should be successful", con);
+            Session ssn = con.createSession(false,Session.AUTO_ACKNOWLEDGE); 
+            assertNotNull("create session should be successful", ssn);
         }        
     }
 }
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/server/logging/BrokerLoggingTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/server/logging/BrokerLoggingTest.java
index 9155b84365..e901903eb4 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/server/logging/BrokerLoggingTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/server/logging/BrokerLoggingTest.java
@@ -519,7 +519,7 @@ public class BrokerLoggingTest extends AbstractTestLogging
             setConfigurationProperty("connector.ssl.keyStorePath", getConfigurationStringProperty("management.ssl.keyStorePath"));
             setConfigurationProperty("connector.ssl.keyStorePassword", getConfigurationStringProperty("management.ssl.keyStorePassword"));
 
-            Integer sslPort = Integer.parseInt(getConfigurationStringProperty("connector.sslport"));
+            Integer sslPort = Integer.parseInt(getConfigurationStringProperty("connector.ssl.port"));
 
             startBroker();
 
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/client/AMQConnectionTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/client/AMQConnectionTest.java
index 481b144caf..95edcd353b 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/client/AMQConnectionTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/client/AMQConnectionTest.java
@@ -50,9 +50,9 @@ import org.slf4j.LoggerFactory;
 
 public class AMQConnectionTest extends QpidBrokerTestCase
 {
-    private static AMQConnection _connection;
-    private static AMQTopic _topic;
-    private static AMQQueue _queue;
+    protected static AMQConnection _connection;
+    protected static AMQTopic _topic;
+    protected static AMQQueue _queue;
     private static QueueSession _queueSession;
     private static TopicSession _topicSession;
     protected static final Logger _logger = LoggerFactory.getLogger(AMQConnectionTest.class);
@@ -60,15 +60,14 @@ public class AMQConnectionTest extends QpidBrokerTestCase
     protected void setUp() throws Exception
     {
         super.setUp();
-        _connection = (AMQConnection) getConnection("guest", "guest");
+        createConnection();
         _topic = new AMQTopic(_connection.getDefaultTopicExchangeName(), new AMQShortString("mytopic"));
         _queue = new AMQQueue(_connection.getDefaultQueueExchangeName(), new AMQShortString("myqueue"));
     }
-
-    protected void tearDown() throws Exception
+    
+    protected void createConnection() throws Exception
     {
-        _connection.close();
-        super.tearDown();
+        _connection = (AMQConnection) getConnection("guest", "guest");
     }
 
     /**
@@ -207,61 +206,50 @@ public class AMQConnectionTest extends QpidBrokerTestCase
 
     public void testPrefetchSystemProperty() throws Exception
     {
-        String oldPrefetch = System.getProperty(ClientProperties.MAX_PREFETCH_PROP_NAME);
-        try
-        {
-            _connection.close();
-            System.setProperty(ClientProperties.MAX_PREFETCH_PROP_NAME, new Integer(2).toString());
-            _connection = (AMQConnection) getConnection();
-            _connection.start();
-            // Create two consumers on different sessions
-            Session consSessA = _connection.createSession(true, Session.AUTO_ACKNOWLEDGE);
-            MessageConsumer consumerA = consSessA.createConsumer(_queue);
-
-            Session producerSession = _connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
-            MessageProducer producer = producerSession.createProducer(_queue);
+        _connection.close();
+        setTestClientSystemProperty(ClientProperties.MAX_PREFETCH_PROP_NAME, new Integer(2).toString());
+        
+        createConnection();
+        _connection.start();
+        // Create two consumers on different sessions
+        Session consSessA = _connection.createSession(true, Session.AUTO_ACKNOWLEDGE);
+        MessageConsumer consumerA = consSessA.createConsumer(_queue);
 
-            // Send 3 messages
-            for (int i = 0; i < 3; i++)
-            {
-                producer.send(producerSession.createTextMessage("test"));
-            }
-            
-            MessageConsumer consumerB = null;
-            // 0-8, 0-9, 0-9-1 prefetch is per session, not consumer.
-            if (!isBroker010())
-            {
-                Session consSessB = _connection.createSession(true, Session.AUTO_ACKNOWLEDGE);
-                consumerB = consSessB.createConsumer(_queue);
-            }
-            else
-            {
-                consumerB = consSessA.createConsumer(_queue);
-            }
+        Session producerSession = _connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+        MessageProducer producer = producerSession.createProducer(_queue);
 
-            Message msg;
-            // Check that consumer A has 2 messages
-            for (int i = 0; i < 2; i++)
-            {
-                msg = consumerA.receive(1500);
-                assertNotNull("Consumer A should receive 2 messages",msg);                
-            }
-            
-            msg = consumerA.receive(1500);
-            assertNull("Consumer A should not have received a 3rd message",msg);
-            
-            // Check that consumer B has the last message
-            msg = consumerB.receive(1500);
-            assertNotNull("Consumer B should have received the message",msg);
+        // Send 3 messages
+        for (int i = 0; i < 3; i++)
+        {
+            producer.send(producerSession.createTextMessage("test"));
         }
-        finally
+        
+        MessageConsumer consumerB = null;
+        // 0-8, 0-9, 0-9-1 prefetch is per session, not consumer.
+        if (!isBroker010())
         {
-            if (oldPrefetch == null)
-            {
-                oldPrefetch = ClientProperties.MAX_PREFETCH_DEFAULT;
-            }
-            System.setProperty(ClientProperties.MAX_PREFETCH_PROP_NAME, oldPrefetch);
+            Session consSessB = _connection.createSession(true, Session.AUTO_ACKNOWLEDGE);
+            consumerB = consSessB.createConsumer(_queue);
+        }
+        else
+        {
+            consumerB = consSessA.createConsumer(_queue);
+        }
+
+        Message msg;
+        // Check that consumer A has 2 messages
+        for (int i = 0; i < 2; i++)
+        {
+            msg = consumerA.receive(1500);
+            assertNotNull("Consumer A should receive 2 messages",msg);                
         }
+        
+        msg = consumerA.receive(1500);
+        assertNull("Consumer A should not have received a 3rd message",msg);
+        
+        // Check that consumer B has the last message
+        msg = consumerB.receive(1500);
+        assertNotNull("Consumer B should have received the message",msg);
     }
     
     public void testGetChannelID() throws Exception
@@ -311,7 +299,7 @@ public class AMQConnectionTest extends QpidBrokerTestCase
            _connection.close();
            stopBroker(port);
            
-           System.setProperty("qpid.heartbeat", "1");
+           setSystemProperty("qpid.heartbeat", "1");
            
            // in case this broker gets stuck, atleast the rest of the tests will not fail.
            port = port + 200;
@@ -381,9 +369,7 @@ public class AMQConnectionTest extends QpidBrokerTestCase
            throw e;
        }
        finally
-       {
-           System.setProperty("qpid.heartbeat", "");
-           
+       {           
            if (process != null)
            {
                process.destroy();
@@ -395,9 +381,4 @@ public class AMQConnectionTest extends QpidBrokerTestCase
            cleanBroker();
        }
     }
-    
-    public static junit.framework.Test suite()
-    {
-        return new junit.framework.TestSuite(AMQConnectionTest.class);
-    }
 }
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/client/AMQSSLConnectionTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/client/AMQSSLConnectionTest.java
new file mode 100644
index 0000000000..53a433c543
--- /dev/null
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/client/AMQSSLConnectionTest.java
@@ -0,0 +1,57 @@
+/*
+ *
+ * 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.qpid.test.unit.client;
+
+import org.apache.qpid.client.AMQConnection;
+import org.apache.qpid.client.AMQConnectionURL;
+
+public class AMQSSLConnectionTest extends AMQConnectionTest
+{
+    private static final String KEYSTORE = TEST_RESOURCES_DIR + "/ssl/java_client_keystore.jks";
+    private static final String KEYSTORE_PASSWORD = "password";
+    private static final String TRUSTSTORE = TEST_RESOURCES_DIR + "/ssl/java_client_truststore.jks";
+    private static final String TRUSTSTORE_PASSWORD = "password";
+    
+    @Override
+    protected void setUp() throws Exception 
+    {
+        setTestClientSystemProperty("profile.use_ssl", "true");
+        setConfigurationProperty("connector.ssl.enabled", "true");
+        setConfigurationProperty("connector.ssl.sslOnly", "true");
+        super.setUp();
+    }
+
+    protected void createConnection() throws Exception
+    {
+        
+        final String sslPrototypeUrl = "amqp://guest:guest@test/?brokerlist='tcp://localhost:%s" +
+        "?ssl='true'&ssl_verify_hostname='false'" + 
+        "&key_store='%s'&key_store_password='%s'" +
+        "&trust_store='%s'&trust_store_password='%s'" +
+        "'";
+
+        final String url = String.format(sslPrototypeUrl,System.getProperty("test.port.ssl"),
+                KEYSTORE,KEYSTORE_PASSWORD,TRUSTSTORE,TRUSTSTORE_PASSWORD);
+        
+        _connection = (AMQConnection) getConnection(new AMQConnectionURL(url));
+    }
+}
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java
index c8ccdf91bb..1d9afe3a68 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java
@@ -57,6 +57,7 @@ import org.apache.log4j.Logger;
 import org.apache.qpid.AMQException;
 import org.apache.qpid.client.AMQConnectionFactory;
 import org.apache.qpid.client.AMQQueue;
+import org.apache.qpid.client.SSLConfiguration;
 import org.apache.qpid.exchange.ExchangeDefaults;
 import org.apache.qpid.jms.BrokerDetails;
 import org.apache.qpid.jms.ConnectionURL;
@@ -83,7 +84,7 @@ public class QpidBrokerTestCase extends QpidTestCase
         INTERNAL /** Test case starts an embedded broker within this JVM */, 
         SPAWNED /** Test case spawns a new broker as a separate process */
     }
-    protected final String QpidHome = System.getProperty("QPID_HOME");
+    protected final static String QpidHome = System.getProperty("QPID_HOME");
     protected File _configFile = new File(System.getProperty("broker.config"));
 
     protected static final Logger _logger = Logger.getLogger(QpidBrokerTestCase.class);
@@ -139,7 +140,7 @@ public class QpidBrokerTestCase extends QpidTestCase
     public static final int DEFAULT_PORT = Integer.getInteger("test.port", ServerConfiguration.DEFAULT_PORT);
     public static final int FAILING_PORT = Integer.parseInt(System.getProperty("test.port.alt"));
     public static final int DEFAULT_MANAGEMENT_PORT = Integer.getInteger("test.mport", ServerConfiguration.DEFAULT_JMXPORT);
-    public static final int DEFAULT_SSL_PORT = Integer.getInteger("test.sslport", ServerConfiguration.DEFAULT_SSL_PORT);
+    public static final int DEFAULT_SSL_PORT = Integer.getInteger("test.port.ssl", ServerConfiguration.DEFAULT_SSL_PORT);
 
     protected String _brokerLanguage = System.getProperty(BROKER_LANGUAGE, JAVA);
     protected BrokerType _brokerType = BrokerType.valueOf(System.getProperty(BROKER_TYPE, "").toUpperCase());
@@ -258,6 +259,10 @@ public class QpidBrokerTestCase extends QpidTestCase
                 _logger.error("exception stopping broker", e);
             }
 
+            // reset properties used in the test
+            revertSystemProperties();
+            revertLoggingLevels();
+
             if(_brokerCleanBetweenTests)
             {
             	try
@@ -440,10 +445,11 @@ public class QpidBrokerTestCase extends QpidTestCase
 
     protected String getBrokerCommand(int port) throws MalformedURLException
     {
-        final String protocolExcludesList = _brokerProtocolExcludes.replace("@PORT", "" + port);
+        final int sslPort = port-1;
+        final String protocolExcludesList = getProtocolExcludesList(port, sslPort);
         return _brokerCommand
                 .replace("@PORT", "" + port)
-                .replace("@SSL_PORT", "" + (port - 1))
+                .replace("@SSL_PORT", "" + sslPort)
                 .replace("@MPORT", "" + getManagementPort(port))
                 .replace("@CONFIG_FILE", _configFile.toString())
                 .replace("@EXCLUDES", protocolExcludesList);
@@ -476,7 +482,7 @@ public class QpidBrokerTestCase extends QpidTestCase
             options.setConfigFile(_configFile.getAbsolutePath());
             options.addPort(port);
 
-            addExcludedPorts(port, options);
+            addExcludedPorts(port, DEFAULT_SSL_PORT, options);
 
             options.setJmxPort(getManagementPort(port));
 
@@ -597,9 +603,9 @@ public class QpidBrokerTestCase extends QpidTestCase
         }
     }
 
-    private void addExcludedPorts(int port, BrokerOptions options)
+    private void addExcludedPorts(int port, int sslPort, BrokerOptions options)
     {
-        final String protocolExcludesList = _brokerProtocolExcludes.replace("@PORT", "" + port);
+        final String protocolExcludesList = getProtocolExcludesList(port, sslPort);
         
         if (protocolExcludesList.equals(""))
         {
@@ -621,6 +627,13 @@ public class QpidBrokerTestCase extends QpidTestCase
         }
     }
 
+    protected String getProtocolExcludesList(int port, int sslPort)
+    {
+        final String protocolExcludesList =
+            _brokerProtocolExcludes.replace("@PORT", "" + port).replace("@SSL_PORT", "" + sslPort);
+        return protocolExcludesList;
+    }
+
     private boolean existingInternalBroker()
     {
         for(BrokerHolder holder : _brokers.values())
@@ -1049,7 +1062,7 @@ public class QpidBrokerTestCase extends QpidTestCase
     {
         return (AMQConnectionFactory) getInitialContext().lookup(factoryName);
     }
-
+    
     public Connection getConnection() throws JMSException, NamingException
     {
         return getConnection("guest", "guest");
@@ -1117,19 +1130,10 @@ public class QpidBrokerTestCase extends QpidTestCase
 
     protected void tearDown() throws java.lang.Exception
     {
-        try
-        {
-            // close all the connections used by this test.
-            for (Connection c : _connections)
-            {
-                c.close();
-            }
-        }
-        finally
+        // close all the connections used by this test.
+        for (Connection c : _connections)
         {
-            // Ensure any problems with close does not interfer with property resets
-            revertSystemProperties();
-            revertLoggingLevels();
+            c.close();
         }
     }