QPID-5960: Turn on SSL host name verification by default

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

6 files changed, 107 insertions, 12 deletions

diff --git a/qpid/doc/book/src/jms-client-0-8/JMS-Client-Connection-URL.xml b/qpid/doc/book/src/jms-client-0-8/JMS-Client-Connection-URL.xml
index 09ad985368..9480d38c74 100644
--- a/qpid/doc/book/src/jms-client-0-8/JMS-Client-Connection-URL.xml
+++ b/qpid/doc/book/src/jms-client-0-8/JMS-Client-Connection-URL.xml
@@ -287,8 +287,9 @@
 				<row>
 					<entry> ssl_verify_hostname </entry>
 					<entry> Boolean </entry>
-					<entry> When using SSL you can enable hostname verification by using
-							<literal>ssl_verify_hostname='true'</literal> in the broker URL.
+					<entry> This option is used for turning on/off hostname verification when using SSL.
+                        It is set to 'true' by default. You can disable verification by setting it to 'false':
+							<literal>ssl_verify_hostname='false'</literal>.
 					</entry>
 				</row>
 				<row id="JMS-Client-0-8-Connection-URL-BrokerOptions-Retries">
diff --git a/qpid/doc/book/src/jms-client-0-8/JMS-Client-System-Properties.xml b/qpid/doc/book/src/jms-client-0-8/JMS-Client-System-Properties.xml
index bb8385c914..a1ffafec76 100644
--- a/qpid/doc/book/src/jms-client-0-8/JMS-Client-System-Properties.xml
+++ b/qpid/doc/book/src/jms-client-0-8/JMS-Client-System-Properties.xml
@@ -78,6 +78,15 @@
 						exception. <para>Setting this property to 'true' will disable that check and
 							allow you to set a client ID of your choice later on.</para></entry>
 				</row>
+                <row>
+                    <entry>qpid.connection_ssl_verify_hostname</entry>
+                    <entry>boolean</entry>
+                    <entry>true</entry>
+                    <entry>This property is used to turn on/off broker host name verification on SSL negotiation
+                        if SSL transport is used. It is set to 'true' by default.
+                        <para>Setting this property to 'false' will disable that check and
+                            allow you to ignore host name errors.</para></entry>
+                </row>
 			</tbody>
 		</tgroup>
 	</table>
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/AMQBrokerDetails.java b/qpid/java/client/src/main/java/org/apache/qpid/client/AMQBrokerDetails.java
index a659e44363..bde20d0550 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/AMQBrokerDetails.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/AMQBrokerDetails.java
@@ -20,6 +20,7 @@
  */
 package org.apache.qpid.client;
 
+import org.apache.qpid.configuration.ClientProperties;
 import org.apache.qpid.jms.BrokerDetails;
 import org.apache.qpid.transport.ConnectionSettings;
 import org.apache.qpid.url.URLHelper;
@@ -470,7 +471,10 @@ public class AMQBrokerDetails implements BrokerDetails, Serializable
         }
         // ----------------------------
 
-        conSettings.setVerifyHostname(getBooleanProperty(BrokerDetails.OPTIONS_SSL_VERIFY_HOSTNAME));
+        boolean defaultSSLVerifyHostName = Boolean.parseBoolean(
+                System.getProperty(ClientProperties.CONNECTION_OPTION_SSL_VERIFY_HOST_NAME,
+                    String.valueOf(ClientProperties.DEFAULT_CONNECTION_OPTION_SSL_VERIFY_HOST_NAME)));
+        conSettings.setVerifyHostname(getBooleanProperty(BrokerDetails.OPTIONS_SSL_VERIFY_HOSTNAME, defaultSSLVerifyHostName ));
 
         if (getProperty(BrokerDetails.OPTIONS_TCP_NO_DELAY) != null)
         {
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 ad9d3d3516..2733d7bf6d 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
@@ -20,14 +20,14 @@
  */
 package org.apache.qpid.test.unit.client.BrokerDetails;
 
-import junit.framework.TestCase;
-
 import org.apache.qpid.client.AMQBrokerDetails;
+import org.apache.qpid.configuration.ClientProperties;
 import org.apache.qpid.jms.BrokerDetails;
+import org.apache.qpid.test.utils.QpidTestCase;
 import org.apache.qpid.transport.ConnectionSettings;
 import org.apache.qpid.url.URLSyntaxException;
 
-public class BrokerDetailsTest extends TestCase
+public class BrokerDetailsTest extends QpidTestCase
 {
     public void testDefaultTCP_NODELAY() throws URLSyntaxException
     {
@@ -190,4 +190,38 @@ public class BrokerDetailsTest extends TestCase
 
         assertEquals(Integer.valueOf(60), broker.buildConnectionSettings().getHeartbeatInterval08());
     }
+
+    public void testSslVerifyHostNameIsTurnedOnByDefault() throws Exception
+    {
+        String brokerURL = "tcp://localhost:5672?ssl='true'";
+        AMQBrokerDetails broker = new AMQBrokerDetails(brokerURL);
+        ConnectionSettings connectionSettings = broker.buildConnectionSettings();
+        assertTrue(String.format("Unexpected '%s' option value", BrokerDetails.OPTIONS_SSL_VERIFY_HOSTNAME),
+                connectionSettings.isVerifyHostname());
+        assertNull(String.format("Unexpected '%s' property value", BrokerDetails.OPTIONS_SSL_VERIFY_HOSTNAME),
+                broker.getProperty(BrokerDetails.OPTIONS_SSL_VERIFY_HOSTNAME));
+    }
+
+    public void testSslVerifyHostNameIsTurnedOff() throws Exception
+    {
+        String brokerURL = "tcp://localhost:5672?ssl='true'&ssl_verify_hostname='false'";
+        AMQBrokerDetails broker = new AMQBrokerDetails(brokerURL);
+        ConnectionSettings connectionSettings = broker.buildConnectionSettings();
+        assertFalse(String.format("Unexpected '%s' option value", BrokerDetails.OPTIONS_SSL_VERIFY_HOSTNAME),
+                connectionSettings.isVerifyHostname());
+        assertEquals(String.format("Unexpected '%s' property value", BrokerDetails.OPTIONS_SSL_VERIFY_HOSTNAME),
+                "false", broker.getProperty(BrokerDetails.OPTIONS_SSL_VERIFY_HOSTNAME));
+    }
+
+    public void testSslVerifyHostNameTurnedOffViaSystemProperty() throws Exception
+    {
+        setTestSystemProperty(ClientProperties.CONNECTION_OPTION_SSL_VERIFY_HOST_NAME, "false");
+        String brokerURL = "tcp://localhost:5672?ssl='true'";
+        AMQBrokerDetails broker = new AMQBrokerDetails(brokerURL);
+        ConnectionSettings connectionSettings = broker.buildConnectionSettings();
+        assertFalse(String.format("Unexpected '%s' option value", BrokerDetails.OPTIONS_SSL_VERIFY_HOSTNAME),
+                connectionSettings.isVerifyHostname());
+        assertNull(String.format("Unexpected '%s' property value", BrokerDetails.OPTIONS_SSL_VERIFY_HOSTNAME),
+                broker.getProperty(BrokerDetails.OPTIONS_SSL_VERIFY_HOSTNAME));
+    }
 }
diff --git a/qpid/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java b/qpid/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java
index 0e7d061ba7..f10961c092 100644
--- a/qpid/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java
+++ b/qpid/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java
@@ -248,7 +248,11 @@ public class ClientProperties
      */
     public static final String SET_EXPIRATION_AS_TTL = "qpid.set_expiration_as_ttl";
 
-
+    /**
+     * System property to set a default value for a connection option 'ssl_verify_hostname'
+     */
+    public static final String CONNECTION_OPTION_SSL_VERIFY_HOST_NAME = "qpid.connection_ssl_verify_hostname";
+    public static final boolean DEFAULT_CONNECTION_OPTION_SSL_VERIFY_HOST_NAME = true;
 
     private ClientProperties()
     {
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 8225fce3a3..eb61e5a084 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
@@ -75,7 +75,7 @@ public class SSLTest extends QpidBrokerTestCase
             super.setUp();
 
             String url = "amqp://guest:guest@test/?brokerlist='tcp://localhost:%s" +
-            "?ssl='true'&ssl_verify_hostname='true'" +
+            "?ssl='true'" +
             "&key_store='%s'&key_store_password='%s'" +
             "&trust_store='%s'&trust_store_password='%s'" +
             "'";
@@ -90,6 +90,49 @@ public class SSLTest extends QpidBrokerTestCase
         }
     }
 
+    public void testHostVerificationIsOnByDefault() throws Exception
+    {
+        if (shouldPerformTest())
+        {
+            clearSslStoreSystemProperties();
+
+            //Start the broker (NEEDing client certificate authentication)
+            configureJavaBrokerIfNecessary(true, true, true, false, false);
+            super.setUp();
+
+            String url = "amqp://guest:guest@test/?brokerlist='tcp://127.0.0.1:%s" +
+                    "?ssl='true'" +
+                    "&key_store='%s'&key_store_password='%s'" +
+                    "&trust_store='%s'&trust_store_password='%s'" +
+                    "'";
+
+            url = String.format(url,QpidBrokerTestCase.DEFAULT_SSL_PORT,
+                    KEYSTORE,KEYSTORE_PASSWORD,TRUSTSTORE,TRUSTSTORE_PASSWORD);
+
+            try
+            {
+                getConnection(new AMQConnectionURL(url));
+            }
+            catch(JMSException e)
+            {
+                assertTrue("Unexpected exception message", e.getMessage().contains("SSL hostname verification failed"));
+            }
+
+            url = "amqp://guest:guest@test/?brokerlist='tcp://127.0.0.1:%s" +
+                    "?ssl='true'&ssl_verify_hostname='false'" +
+                    "&key_store='%s'&key_store_password='%s'" +
+                    "&trust_store='%s'&trust_store_password='%s'" +
+                    "'";
+            url = String.format(url,QpidBrokerTestCase.DEFAULT_SSL_PORT,
+                    KEYSTORE,KEYSTORE_PASSWORD,TRUSTSTORE,TRUSTSTORE_PASSWORD);
+
+            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,
@@ -197,7 +240,7 @@ public class SSLTest extends QpidBrokerTestCase
 
             String url = "amqp://guest:guest@test/?brokerlist='tcp://127.0.0.1:" +
             QpidBrokerTestCase.DEFAULT_SSL_PORT +
-            "?ssl='true'&ssl_verify_hostname='true''";
+            "?ssl='true''";
 
             try
             {
@@ -230,7 +273,7 @@ public class SSLTest extends QpidBrokerTestCase
 
             String url = "amqp://guest:guest@test/?brokerlist='tcp://localhost:" +
             QpidBrokerTestCase.DEFAULT_SSL_PORT +
-            "?ssl='true'&ssl_verify_hostname='true''";
+            "?ssl='true''";
 
             Connection con = getConnection(new AMQConnectionURL(url));
             assertNotNull("connection should have been created", con);
@@ -247,7 +290,7 @@ public class SSLTest extends QpidBrokerTestCase
 
             String url = "amqp://guest:guest@test/?brokerlist='tcp://localhost.localdomain:" +
             QpidBrokerTestCase.DEFAULT_SSL_PORT +
-            "?ssl='true'&ssl_verify_hostname='true''";
+            "?ssl='true''";
 
             Connection con = getConnection(new AMQConnectionURL(url));
             assertNotNull("connection should have been created", con);
@@ -266,7 +309,7 @@ public class SSLTest extends QpidBrokerTestCase
 
 
             String url = "amqp://guest:guest@test/?brokerlist='tcp://localhost:%s" +
-            "?ssl='true'&ssl_verify_hostname='true'" +
+            "?ssl='true'" +
             "&trust_store='%s'&trust_store_password='%s'" +
             "'";