From 87456620af31532eb5af81c0207e7533ae67fb39 Mon Sep 17 00:00:00 2001 From: Alex Rudyy Date: Thu, 7 Aug 2014 00:20:24 +0000 Subject: 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 --- .../jms-client-0-8/JMS-Client-Connection-URL.xml | 5 +- .../JMS-Client-System-Properties.xml | 9 ++++ .../org/apache/qpid/client/AMQBrokerDetails.java | 6 ++- .../client/BrokerDetails/BrokerDetailsTest.java | 40 ++++++++++++++-- .../qpid/configuration/ClientProperties.java | 6 ++- .../java/org/apache/qpid/client/ssl/SSLTest.java | 53 ++++++++++++++++++++-- 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 @@ ssl_verify_hostname Boolean - When using SSL you can enable hostname verification by using - ssl_verify_hostname='true' in the broker URL. + 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': + ssl_verify_hostname='false'. 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. Setting this property to 'true' will disable that check and allow you to set a client ID of your choice later on. + + qpid.connection_ssl_verify_hostname + boolean + true + 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. + Setting this property to 'false' will disable that check and + allow you to ignore host name errors. + 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'" + "'"; -- cgit v1.2.1