QPID-4468: restore connection level ssl option to provide compatibility with older client configuration, add ability to override brokerlist ssl option

merge from trunk r1413364


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

7 files changed, 169 insertions, 5 deletions

diff --git a/qpid/doc/book/src/programming/Programming-In-Apache-Qpid.xml b/qpid/doc/book/src/programming/Programming-In-Apache-Qpid.xml
index fd32f42f2e..e2f6d8756c 100644
--- a/qpid/doc/book/src/programming/Programming-In-Apache-Qpid.xml
+++ b/qpid/doc/book/src/programming/Programming-In-Apache-Qpid.xml
@@ -3087,6 +3087,22 @@ spout - -content "$(cat rdu.xml | sed -e 's/70/45/')" xml/weather
 		  </para>
 	        </entry>
 	      </row>
+	      <row>
+	        <entry>
+		        ssl
+	        </entry>
+	        <entry>
+		        boolean
+	        </entry>
+	        <entry>
+	            <para>
+	                If <literal>ssl='true'</literal>, use SSL for all broker connections. Overrides any per-broker settings in the brokerlist (see below) entries. If not specified, the brokerlist entry for each given broker is used to determine whether SSL is used.
+	            </para>
+	            <para>
+	                Introduced in version 0.22.
+	            </para>
+	        </entry>
+	      </row>
 	    </tbody>
 	  </tgroup>
         </table>
@@ -3237,6 +3253,7 @@ spout - -content "$(cat rdu.xml | sed -e 's/70/45/')" xml/weather
 		  trust_store_password
 	        </entry>
 	        <entry>
+		        --
 	        </entry>
 	        <entry>
 		  Trust store password
@@ -3247,6 +3264,7 @@ spout - -content "$(cat rdu.xml | sed -e 's/70/45/')" xml/weather
 		  key_store
 	        </entry>
 	        <entry>
+		        --
 	        </entry>
 	        <entry>
 		  path to key store
@@ -3271,7 +3289,9 @@ spout - -content "$(cat rdu.xml | sed -e 's/70/45/')" xml/weather
 		  Boolean
 	        </entry>
 	        <entry>
-		  If <literal>ssl='true'</literal>, the JMS client will encrypt the connection using SSL.
+	            <para>If <literal>ssl='true'</literal>, the JMS client will encrypt the connection to this broker using SSL.</para>
+
+	            <para>This can also be set/overridden for all brokers using the <link linkend="section-jms-connection-url">Connection URL</link> options.</para>
 	        </entry>
 	      </row>
 	      <row>
@@ -3292,7 +3312,7 @@ spout - -content "$(cat rdu.xml | sed -e 's/70/45/')" xml/weather
 		  ssl_cert_alias
 	        </entry>
 	        <entry>
-
+		        --
 	        </entry>
 	        <entry>
 		  If multiple certificates are present in the keystore, the alias will be used to extract the correct certificate.
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_0_10.java b/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_0_10.java
index 51e7e4153d..5dd6e55e64 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_0_10.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_0_10.java
@@ -33,6 +33,7 @@ import org.apache.qpid.configuration.ClientProperties;
 import org.apache.qpid.framing.ProtocolVersion;
 import org.apache.qpid.jms.BrokerDetails;
 import org.apache.qpid.jms.ChannelLimitReachedException;
+import org.apache.qpid.jms.ConnectionURL;
 import org.apache.qpid.jms.Session;
 import org.apache.qpid.properties.ConnectionStartProperties;
 import org.apache.qpid.protocol.AMQConstant;
@@ -214,7 +215,8 @@ public class AMQConnectionDelegate_0_10 implements AMQConnectionDelegate, Connec
                         + "********");
             }
 
-            ConnectionSettings conSettings = retriveConnectionSettings(brokerDetail);
+            ConnectionSettings conSettings = retrieveConnectionSettings(brokerDetail);
+
             _qpidConnection.setConnectionDelegate(new ClientConnectionDelegate(conSettings, _conn.getConnectionURL()));
             _qpidConnection.connect(conSettings);
 
@@ -420,7 +422,7 @@ public class AMQConnectionDelegate_0_10 implements AMQConnectionDelegate, Connec
         return featureSupported;
     }
 
-    private ConnectionSettings retriveConnectionSettings(BrokerDetails brokerDetail)
+    private ConnectionSettings retrieveConnectionSettings(BrokerDetails brokerDetail)
     {
         ConnectionSettings conSettings = brokerDetail.buildConnectionSettings();
 
@@ -442,6 +444,24 @@ public class AMQConnectionDelegate_0_10 implements AMQConnectionDelegate, Connec
 
         conSettings.setHeartbeatInterval(getHeartbeatInterval(brokerDetail));
 
+        //Check connection-level ssl override setting
+        String connectionSslOption = _conn.getConnectionURL().getOption(ConnectionURL.OPTIONS_SSL);
+        if(connectionSslOption != null)
+        {
+            boolean connUseSsl = Boolean.parseBoolean(connectionSslOption);
+            boolean brokerlistUseSsl = conSettings.isUseSSL();
+
+            if( connUseSsl != brokerlistUseSsl)
+            {
+                conSettings.setUseSSL(connUseSsl);
+
+                if (_logger.isDebugEnabled())
+                {
+                    _logger.debug("Applied connection ssl option override, setting UseSsl to: " + connUseSsl );
+                }
+            }
+        }
+
         return conSettings;
     }
 
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java b/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java
index 91896d913d..740a81b939 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java
@@ -40,6 +40,7 @@ import org.apache.qpid.framing.TxSelectBody;
 import org.apache.qpid.framing.TxSelectOkBody;
 import org.apache.qpid.jms.BrokerDetails;
 import org.apache.qpid.jms.ChannelLimitReachedException;
+import org.apache.qpid.jms.ConnectionURL;
 import org.apache.qpid.ssl.SSLContextFactory;
 import org.apache.qpid.transport.ConnectionSettings;
 import org.apache.qpid.transport.network.NetworkConnection;
@@ -100,6 +101,24 @@ public class AMQConnectionDelegate_8_0 implements AMQConnectionDelegate
         ConnectionSettings settings = brokerDetail.buildConnectionSettings();
         settings.setProtocol(brokerDetail.getTransport());
 
+        //Check connection-level ssl override setting
+        String connectionSslOption = _conn.getConnectionURL().getOption(ConnectionURL.OPTIONS_SSL);
+        if(connectionSslOption != null)
+        {
+            boolean connUseSsl = Boolean.parseBoolean(connectionSslOption);
+            boolean brokerlistUseSsl = settings.isUseSSL();
+
+            if( connUseSsl != brokerlistUseSsl)
+            {
+                settings.setUseSSL(connUseSsl);
+
+                if (_logger.isDebugEnabled())
+                {
+                    _logger.debug("Applied connection ssl option override, setting UseSsl to: " + connUseSsl );
+                }
+            }
+        }
+
         SecurityLayer securityLayer = SecurityLayerFactory.newInstance(settings);
 
         OutgoingNetworkTransport transport = Transport.getOutgoingTransportInstance(getProtocolVersion());
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/jms/ConnectionURL.java b/qpid/java/client/src/main/java/org/apache/qpid/jms/ConnectionURL.java
index af79787f94..237925f24b 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/jms/ConnectionURL.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/jms/ConnectionURL.java
@@ -44,6 +44,13 @@ public interface ConnectionURL
     public static final String OPTIONS_FAILOVER_CYCLE = "cyclecount";
 
     /**
+     * This option is used to apply a connection level override of
+     * the {@value BrokerDetails#OPTIONS_SSL} option values in the
+     * {@value ConnectionURL#OPTIONS_BROKERLIST};
+     */
+    public static final String OPTIONS_SSL = "ssl";
+
+    /**
      * This option is only applicable for 0-8/0-9/0-9-1 protocols connection
      * <p>
      * It tells the client to delegate the requeue/DLQ decision to the
diff --git a/qpid/java/client/src/test/java/org/apache/qpid/test/unit/client/BrokerDetails/BrokerDetailsTest.java b/qpid/java/client/src/test/java/org/apache/qpid/test/unit/client/BrokerDetails/BrokerDetailsTest.java
index 71aec68d30..1e9e5b00a5 100644
--- a/qpid/java/client/src/test/java/org/apache/qpid/test/unit/client/BrokerDetails/BrokerDetailsTest.java
+++ b/qpid/java/client/src/test/java/org/apache/qpid/test/unit/client/BrokerDetails/BrokerDetailsTest.java
@@ -143,4 +143,25 @@ public class BrokerDetailsTest extends TestCase
 
         assertEquals("Unexpected toString", expectedToString, actualToString);
     }
+
+    public void testDefaultSsl() throws URLSyntaxException
+    {
+        String brokerURL = "tcp://localhost:5672";
+        AMQBrokerDetails broker = new AMQBrokerDetails(brokerURL);
+
+        assertNull("default value should be null", broker.getProperty(BrokerDetails.OPTIONS_SSL));
+    }
+
+    public void testOverridingSsl() throws URLSyntaxException
+    {
+        String brokerURL = "tcp://localhost:5672?ssl='true'";
+        AMQBrokerDetails broker = new AMQBrokerDetails(brokerURL);
+
+        assertTrue("value should be true", Boolean.valueOf(broker.getProperty(BrokerDetails.OPTIONS_SSL)));
+
+        brokerURL = "tcp://localhost:5672?ssl='false''&maxprefetch='1'";
+        broker = new AMQBrokerDetails(brokerURL);
+
+        assertFalse("value should be false", Boolean.valueOf(broker.getProperty(BrokerDetails.OPTIONS_SSL)));
+    }
 }
diff --git a/qpid/java/client/src/test/java/org/apache/qpid/test/unit/client/connectionurl/ConnectionURLTest.java b/qpid/java/client/src/test/java/org/apache/qpid/test/unit/client/connectionurl/ConnectionURLTest.java
index 69f9954892..ac6cad879b 100644
--- a/qpid/java/client/src/test/java/org/apache/qpid/test/unit/client/connectionurl/ConnectionURLTest.java
+++ b/qpid/java/client/src/test/java/org/apache/qpid/test/unit/client/connectionurl/ConnectionURLTest.java
@@ -30,7 +30,6 @@ import org.apache.qpid.url.URLSyntaxException;
 
 public class ConnectionURLTest extends TestCase
 {
-
     public void testFailoverURL() throws URLSyntaxException
     {
         String url = "amqp://ritchiem:bob@/test?brokerlist='tcp://localhost:5672;tcp://fancyserver:3000/',failover='roundrobin?cyclecount='100''";
@@ -563,5 +562,34 @@ public class ConnectionURLTest extends TestCase
         assertNull("Reject behaviour option was not as expected",
                 connectionurl.getOption(ConnectionURL.OPTIONS_REJECT_BEHAVIOUR));
     }
+
+    /**
+     * Verify that when the ssl option is not specified, asking for the option returns null,
+     * such that this can later be used to verify it wasnt specified.
+     */
+    public void testDefaultSsl() throws URLSyntaxException
+    {
+        String url = "amqp://guest:guest@/test?brokerlist='tcp://localhost:5672'&foo='bar'";
+        ConnectionURL connectionURL = new AMQConnectionURL(url);
+
+        assertNull("default ssl value should be null", connectionURL.getOption(ConnectionURL.OPTIONS_SSL));
+    }
+
+    /**
+     * Verify that when the ssl option is specified, asking for the option returns the value,
+     * such that this can later be used to verify what value it was specified as.
+     */
+    public void testOverridingSsl() throws URLSyntaxException
+    {
+        String url = "amqp://guest:guest@/test?brokerlist='tcp://localhost:5672'&ssl='true'";
+        ConnectionURL connectionURL = new AMQConnectionURL(url);
+
+        assertTrue("value should be true", Boolean.valueOf(connectionURL.getOption(ConnectionURL.OPTIONS_SSL)));
+
+        url = "amqp://guest:guest@/test?brokerlist='tcp://localhost:5672'&ssl='false'";
+        connectionURL = new AMQConnectionURL(url);
+
+        assertFalse("value should be false", Boolean.valueOf(connectionURL.getOption(ConnectionURL.OPTIONS_SSL)));
+    }
 }
 
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 4f341e3c60..4fafcb3504 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
@@ -28,6 +28,7 @@ import static org.apache.qpid.test.utils.TestSSLConstants.TRUSTSTORE_PASSWORD;
 import org.apache.commons.configuration.ConfigurationException;
 import org.apache.qpid.client.AMQConnectionURL;
 import org.apache.qpid.client.AMQTestConnection_0_10;
+import org.apache.qpid.jms.ConnectionURL;
 import org.apache.qpid.test.utils.QpidBrokerTestCase;
 
 import javax.jms.Connection;
@@ -78,6 +79,54 @@ public class SSLTest extends QpidBrokerTestCase
         }
     }
 
+    /**
+     * Create an SSL connection using the SSL system properties for the trust and key store, but using
+     * the {@link ConnectionURL} ssl='true' option to indicate use of SSL at a Connection level,
+     * without specifying anything at the {@link ConnectionURL#OPTIONS_BROKERLIST} level.
+     */
+    public void testSslConnectionOption() throws Exception
+    {
+        if (shouldPerformTest())
+        {
+            //Start the broker (NEEDing client certificate authentication)
+            configureJavaBrokerIfNecessary(true, true, true, false);
+            super.setUp();
+
+            //Create URL enabling SSL at the connection rather than brokerlist level
+            String url = "amqp://guest:guest@test/?ssl='true'&brokerlist='tcp://localhost:%s'";
+            url = String.format(url,QpidBrokerTestCase.DEFAULT_SSL_PORT);
+
+            Connection con = getConnection(new AMQConnectionURL(url));
+            assertNotNull("connection should be successful", con);
+            Session ssn = con.createSession(false,Session.AUTO_ACKNOWLEDGE);
+            assertNotNull("create session should be successful", ssn);
+        }
+    }
+
+    /**
+     * Create an SSL connection using the SSL system properties for the trust and key store, but using
+     * the {@link ConnectionURL} ssl='true' option to indicate use of SSL at a Connection level,
+     * overriding the false setting at the {@link ConnectionURL#OPTIONS_BROKERLIST} level.
+     */
+    public void testSslConnectionOptionOverridesBrokerlistOption() throws Exception
+    {
+        if (shouldPerformTest())
+        {
+            //Start the broker (NEEDing client certificate authentication)
+            configureJavaBrokerIfNecessary(true, true, true, false);
+            super.setUp();
+
+            //Create URL enabling SSL at the connection, overriding the false at the brokerlist level
+            String url = "amqp://guest:guest@test/?ssl='true'&brokerlist='tcp://localhost:%s?ssl='false''";
+            url = String.format(url,QpidBrokerTestCase.DEFAULT_SSL_PORT);
+
+            Connection con = getConnection(new AMQConnectionURL(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 (shouldPerformTest())