[activemq] branch master updated: AMQ-7406 - ActiveMQSslConnectionFactory truststore settings don't work for the HTTPS connector

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[activemq] branch master updated: AMQ-7406 - ActiveMQSslConnectionFactory truststore settings don't work for the HTTPS connector

jbonofre-2
This is an automated email from the ASF dual-hosted git repository.

jbonofre pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/activemq.git


The following commit(s) were added to refs/heads/master by this push:
     new 2f9e2b1  AMQ-7406 - ActiveMQSslConnectionFactory truststore settings don't work for the HTTPS connector
     new ef59309  Merge pull request #454 from coheigea/AMQ-7406
2f9e2b1 is described below

commit 2f9e2b1e839126edf15785417ea1f6789b7c0b2f
Author: Colm O hEigeartaigh <[hidden email]>
AuthorDate: Fri Feb 14 14:37:59 2020 +0000

    AMQ-7406 - ActiveMQSslConnectionFactory truststore settings don't work for the HTTPS connector
---
 .../transport/https/HttpsClientTransport.java      |  22 ++++-
 .../transport/https/HttpsTransportFactory.java     |  24 ++++-
 ...ntSettingsHostnameVerificationDisabledTest.java |  99 +++++++++++++++++++
 ...ttpsClientSettingsHostnameVerificationTest.java | 106 +++++++++++++++++++++
 .../transport/https/HttpsClientSettingsTest.java   |  97 +++++++++++++++++++
 .../src/test/resources/server-somehost.keystore    | Bin 0 -> 2007 bytes
 6 files changed, 343 insertions(+), 5 deletions(-)

diff --git a/activemq-http/src/main/java/org/apache/activemq/transport/https/HttpsClientTransport.java b/activemq-http/src/main/java/org/apache/activemq/transport/https/HttpsClientTransport.java
index a837818..7ad45c0 100644
--- a/activemq-http/src/main/java/org/apache/activemq/transport/https/HttpsClientTransport.java
+++ b/activemq-http/src/main/java/org/apache/activemq/transport/https/HttpsClientTransport.java
@@ -20,6 +20,8 @@ package org.apache.activemq.transport.https;
 import java.io.IOException;
 import java.net.URI;
 
+import javax.net.ssl.HostnameVerifier;
+
 import org.apache.activemq.broker.SslContext;
 import org.apache.activemq.transport.http.HttpClientTransport;
 import org.apache.activemq.transport.util.TextWireFormat;
@@ -29,13 +31,22 @@ import org.apache.http.config.RegistryBuilder;
 import org.apache.http.conn.HttpClientConnectionManager;
 import org.apache.http.conn.socket.ConnectionSocketFactory;
 import org.apache.http.conn.ssl.DefaultHostnameVerifier;
+import org.apache.http.conn.ssl.NoopHostnameVerifier;
 import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
 import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
 
 public class HttpsClientTransport extends HttpClientTransport {
 
+    private final javax.net.ssl.SSLSocketFactory sslSocketFactory;
+    private boolean verifyHostName = true;
+
     public HttpsClientTransport(TextWireFormat wireFormat, URI remoteUrl) {
         super(wireFormat, remoteUrl);
+        try {
+            sslSocketFactory = createSocketFactory();
+        } catch (IOException e) {
+            throw new IllegalStateException("Error trying to configure TLS", e);
+        }
     }
 
     @Override
@@ -47,7 +58,8 @@ public class HttpsClientTransport extends HttpClientTransport {
 
         RegistryBuilder<ConnectionSocketFactory> registryBuilder = RegistryBuilder.<ConnectionSocketFactory>create();
         try {
-            SSLConnectionSocketFactory sslConnectionFactory = new SSLConnectionSocketFactory(createSocketFactory(), new DefaultHostnameVerifier());
+            HostnameVerifier hostnameVerifier = verifyHostName ? new DefaultHostnameVerifier() : new NoopHostnameVerifier();
+            SSLConnectionSocketFactory sslConnectionFactory = new SSLConnectionSocketFactory(sslSocketFactory, hostnameVerifier);
             registryBuilder.register("https", sslConnectionFactory);
             return registryBuilder.build();
         } catch (Exception e) {
@@ -81,4 +93,12 @@ public class HttpsClientTransport extends HttpClientTransport {
         return "https.";
     }
 
+    public Boolean getVerifyHostName() {
+        return verifyHostName;
+    }
+
+    public void setVerifyHostName(Boolean verifyHostName) {
+        this.verifyHostName = verifyHostName;
+    }
+
 }
diff --git a/activemq-http/src/main/java/org/apache/activemq/transport/https/HttpsTransportFactory.java b/activemq-http/src/main/java/org/apache/activemq/transport/https/HttpsTransportFactory.java
index bf382aa..d8f5ea2 100644
--- a/activemq-http/src/main/java/org/apache/activemq/transport/https/HttpsTransportFactory.java
+++ b/activemq-http/src/main/java/org/apache/activemq/transport/https/HttpsTransportFactory.java
@@ -57,16 +57,32 @@ public class HttpsTransportFactory extends HttpTransportFactory {
     }
 
     @Override
-    protected Transport createTransport(URI location, WireFormat wf) throws MalformedURLException {
+    protected Transport createTransport(URI location, WireFormat wf) throws IOException {
         // need to remove options from uri
-        URI uri;
         try {
-            uri = URISupport.removeQuery(location);
+            URI uri = URISupport.removeQuery(location);
+
+            Map<String, String> options = new HashMap<>(URISupport.parseParameters(location));
+            Map<String, Object> transportOptions = IntrospectionSupport.extractProperties(options, "transport.");
+            boolean verifyHostName = true;
+            if (transportOptions.containsKey("verifyHostName")) {
+                verifyHostName = Boolean.parseBoolean(transportOptions.get("verifyHostName").toString());
+            }
+
+            HttpsClientTransport clientTransport = new HttpsClientTransport(asTextWireFormat(wf), uri);
+            clientTransport.setVerifyHostName(verifyHostName);
+            return clientTransport;
         } catch (URISyntaxException e) {
             MalformedURLException cause = new MalformedURLException("Error removing query on " + location);
             cause.initCause(e);
             throw cause;
         }
-        return new HttpsClientTransport(asTextWireFormat(wf), uri);
+    }
+
+    // TODO Not sure if there is a better way of removing transport.verifyHostName here?
+    @Override
+    public Transport compositeConfigure(Transport transport, WireFormat format, Map options) {
+        options.remove("transport.verifyHostName");
+        return super.compositeConfigure(transport, format, options);
     }
 }
diff --git a/activemq-http/src/test/java/org/apache/activemq/transport/https/HttpsClientSettingsHostnameVerificationDisabledTest.java b/activemq-http/src/test/java/org/apache/activemq/transport/https/HttpsClientSettingsHostnameVerificationDisabledTest.java
new file mode 100644
index 0000000..0a56f64
--- /dev/null
+++ b/activemq-http/src/test/java/org/apache/activemq/transport/https/HttpsClientSettingsHostnameVerificationDisabledTest.java
@@ -0,0 +1,99 @@
+/**
+ * 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.activemq.transport.https;
+
+import org.apache.activemq.ActiveMQConnectionFactory;
+import org.apache.activemq.ActiveMQSslConnectionFactory;
+import org.apache.activemq.JmsTopicSendReceiveTest;
+import org.apache.activemq.broker.BrokerService;
+import org.apache.activemq.spring.SpringSslContext;
+
+/**
+ * Here we are using a TLS cert which does not have "localhost" as the CN. However we configure the client not to enable
+ * hostname verification, and so the test passes
+ */
+public class HttpsClientSettingsHostnameVerificationDisabledTest extends JmsTopicSendReceiveTest {
+
+    /**
+     *
+     */
+    private static final String URI_LOCATION = "https://localhost:8161";
+    public static final String KEYSTORE_TYPE = "jks";
+    public static final String PASSWORD = "password";
+    public static final String TRUST_KEYSTORE = "src/test/resources/server-somehost.keystore";
+    public static final String SERVER_KEYSTORE = "src/test/resources/server-somehost.keystore";
+
+    private BrokerService broker;
+
+    /*
+     * (non-Javadoc)
+     *
+     * @see org.apache.activemq.JmsSendReceiveTestSupport#setUp()
+     */
+    @Override
+    protected void setUp() throws Exception {
+        // Create the broker service from the configuration and wait until it
+        // has been started...
+        broker = new BrokerService();
+        SpringSslContext sslContext = new SpringSslContext();
+        sslContext.setKeyStorePassword(PASSWORD);
+        sslContext.setKeyStore(SERVER_KEYSTORE);
+        sslContext.setTrustStore(TRUST_KEYSTORE);
+        sslContext.setTrustStorePassword(PASSWORD);
+        sslContext.afterPropertiesSet(); // This is required so that the SSLContext instance is generated with the passed information.
+        broker.setSslContext(sslContext);
+        broker.addConnector(URI_LOCATION);
+        broker.setPersistent(false);
+        broker.setUseJmx(false);
+        broker.start();
+        broker.waitUntilStarted();
+        super.setUp();
+    }
+
+    /*
+     * (non-Javadoc)
+     *
+     * @see org.apache.activemq.AutoFailTestSupport#tearDown()
+     */
+    @Override
+    protected void tearDown() throws Exception {
+        super.tearDown();
+        if (broker != null) {
+            broker.stop();
+        }
+    }
+
+    /*
+     * (non-Javadoc)
+     *
+     * @see org.apache.activemq.TestSupport#createConnectionFactory()
+     */
+    @Override
+    protected ActiveMQConnectionFactory createConnectionFactory()
+        throws Exception {
+        ActiveMQSslConnectionFactory factory = new ActiveMQSslConnectionFactory(URI_LOCATION + "?transport.verifyHostName=false");
+
+        // Configure TLS for the client
+        factory.setTrustStore(TRUST_KEYSTORE);
+        factory.setTrustStorePassword(PASSWORD);
+
+        return factory;
+    }
+    
+
+
+}
diff --git a/activemq-http/src/test/java/org/apache/activemq/transport/https/HttpsClientSettingsHostnameVerificationTest.java b/activemq-http/src/test/java/org/apache/activemq/transport/https/HttpsClientSettingsHostnameVerificationTest.java
new file mode 100644
index 0000000..482e186
--- /dev/null
+++ b/activemq-http/src/test/java/org/apache/activemq/transport/https/HttpsClientSettingsHostnameVerificationTest.java
@@ -0,0 +1,106 @@
+/**
+ * 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.activemq.transport.https;
+
+import org.apache.activemq.ActiveMQConnectionFactory;
+import org.apache.activemq.ActiveMQSslConnectionFactory;
+import org.apache.activemq.JmsTopicSendReceiveTest;
+import org.apache.activemq.broker.BrokerService;
+import org.apache.activemq.spring.SpringSslContext;
+
+/**
+ * Here we are using a TLS cert which does not have "localhost" as the CN. We expect hostname verification to fail by default.
+ */
+public class HttpsClientSettingsHostnameVerificationTest extends JmsTopicSendReceiveTest {
+
+    /**
+     *
+     */
+    private static final String URI_LOCATION = "https://localhost:8161";
+    public static final String KEYSTORE_TYPE = "jks";
+    public static final String PASSWORD = "password";
+    public static final String TRUST_KEYSTORE = "src/test/resources/server-somehost.keystore";
+    public static final String SERVER_KEYSTORE = "src/test/resources/server-somehost.keystore";
+
+    private BrokerService broker;
+
+    /*
+     * (non-Javadoc)
+     *
+     * @see org.apache.activemq.JmsSendReceiveTestSupport#setUp()
+     */
+    @Override
+    protected void setUp() throws Exception {
+        // Create the broker service from the configuration and wait until it
+        // has been started...
+        broker = new BrokerService();
+        SpringSslContext sslContext = new SpringSslContext();
+        sslContext.setKeyStorePassword(PASSWORD);
+        sslContext.setKeyStore(SERVER_KEYSTORE);
+        sslContext.setTrustStore(TRUST_KEYSTORE);
+        sslContext.setTrustStorePassword(PASSWORD);
+        sslContext.afterPropertiesSet(); // This is required so that the SSLContext instance is generated with the passed information.
+        broker.setSslContext(sslContext);
+        broker.addConnector(URI_LOCATION);
+        broker.setPersistent(false);
+        broker.setUseJmx(false);
+        broker.start();
+        broker.waitUntilStarted();
+    }
+
+    /*
+     * (non-Javadoc)
+     *
+     * @see org.apache.activemq.AutoFailTestSupport#tearDown()
+     */
+    @Override
+    protected void tearDown() throws Exception {
+        if (broker != null) {
+            broker.stop();
+        }
+    }
+
+    /*
+     * (non-Javadoc)
+     *
+     * @see org.apache.activemq.TestSupport#createConnectionFactory()
+     */
+    @Override
+    protected ActiveMQConnectionFactory createConnectionFactory()
+        throws Exception {
+        ActiveMQSslConnectionFactory factory = new ActiveMQSslConnectionFactory(URI_LOCATION);
+
+        // Configure TLS for the client
+        factory.setTrustStore(TRUST_KEYSTORE);
+        factory.setTrustStorePassword(PASSWORD);
+
+        return factory;
+    }
+    
+    @Override
+    public void testSendReceive() throws Exception {
+        connectionFactory = createConnectionFactory();
+        try {
+            connection = createConnection();
+            fail("Failure expected on hostname verification");
+        } catch (Exception ex) {
+            // expected
+        }
+    }
+
+
+}
diff --git a/activemq-http/src/test/java/org/apache/activemq/transport/https/HttpsClientSettingsTest.java b/activemq-http/src/test/java/org/apache/activemq/transport/https/HttpsClientSettingsTest.java
new file mode 100644
index 0000000..e5eee45
--- /dev/null
+++ b/activemq-http/src/test/java/org/apache/activemq/transport/https/HttpsClientSettingsTest.java
@@ -0,0 +1,97 @@
+/**
+ * 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.activemq.transport.https;
+
+import org.apache.activemq.ActiveMQConnectionFactory;
+import org.apache.activemq.ActiveMQSslConnectionFactory;
+import org.apache.activemq.JmsTopicSendReceiveTest;
+import org.apache.activemq.broker.BrokerService;
+import org.apache.activemq.spring.SpringSslContext;
+
+/**
+ * Test the HTTPS Connector, where we configure the client truststore directly on the connection factory, as opposed to
+ * using system properties.
+ */
+public class HttpsClientSettingsTest extends JmsTopicSendReceiveTest {
+
+    /**
+     *
+     */
+    private static final String URI_LOCATION = "https://localhost:8161";
+    public static final String KEYSTORE_TYPE = "jks";
+    public static final String PASSWORD = "password";
+    public static final String TRUST_KEYSTORE = "src/test/resources/client.keystore";
+    public static final String SERVER_KEYSTORE = "src/test/resources/server.keystore";
+
+    private BrokerService broker;
+
+    /*
+     * (non-Javadoc)
+     *
+     * @see org.apache.activemq.JmsSendReceiveTestSupport#setUp()
+     */
+    @Override
+    protected void setUp() throws Exception {
+        // Create the broker service from the configuration and wait until it
+        // has been started...
+        broker = new BrokerService();
+        SpringSslContext sslContext = new SpringSslContext();
+        sslContext.setKeyStorePassword(PASSWORD);
+        sslContext.setKeyStore(SERVER_KEYSTORE);
+        sslContext.setTrustStore(TRUST_KEYSTORE);
+        sslContext.setTrustStorePassword(PASSWORD);
+        sslContext.afterPropertiesSet(); // This is required so that the SSLContext instance is generated with the passed information.
+        broker.setSslContext(sslContext);
+        broker.addConnector(URI_LOCATION);
+        broker.setPersistent(false);
+        broker.setUseJmx(false);
+        broker.start();
+        broker.waitUntilStarted();
+        super.setUp();
+    }
+
+    /*
+     * (non-Javadoc)
+     *
+     * @see org.apache.activemq.AutoFailTestSupport#tearDown()
+     */
+    @Override
+    protected void tearDown() throws Exception {
+        super.tearDown();
+        if (broker != null) {
+            broker.stop();
+        }
+    }
+
+    /*
+     * (non-Javadoc)
+     *
+     * @see org.apache.activemq.TestSupport#createConnectionFactory()
+     */
+    @Override
+    protected ActiveMQConnectionFactory createConnectionFactory()
+        throws Exception {
+        ActiveMQSslConnectionFactory factory = new ActiveMQSslConnectionFactory(URI_LOCATION);
+
+        // Configure TLS for the client
+        factory.setTrustStore(TRUST_KEYSTORE);
+        factory.setTrustStorePassword(PASSWORD);
+
+        return factory;
+    }
+
+}
diff --git a/activemq-http/src/test/resources/server-somehost.keystore b/activemq-http/src/test/resources/server-somehost.keystore
new file mode 100644
index 0000000..f9e8d0f
Binary files /dev/null and b/activemq-http/src/test/resources/server-somehost.keystore differ