Merge pull request #454 from coheigea/AMQ-7406

AMQ-7406 - ActiveMQSslConnectionFactory truststore settings don't wor…

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;
+    }
+
 }

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);
     }
 }

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;
+    }
+    
+
+
+}

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
+        }
+    }
+
+
+}

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;
+    }
+
+}