From 02971a40e281713a8397d3a1809c164b594abfbb Mon Sep 17 00:00:00 2001 From: "Christopher L. Shannon (cshannon)" Date: Fri, 31 Aug 2018 13:04:14 -0400 Subject: [PATCH] AMQ-7047 - Switch default for hostname verification to be false for server For the client it makes sense to have it true by default but for the server it makes sense to have it false by default (cherry picked from commit 1e31df9800fc2db258f2458628bd9863c11b2846) --- .../transport/amqp/AmqpTestSupport.java | 4 ++-- .../amqp/auto/JMSClientAutoSslAuthTest.java | 2 +- .../transport/nio/NIOSSLTransport.java | 2 +- .../activemq/transport/tcp/SslTransport.java | 1 + .../transport/tcp/TcpTransportServer.java | 4 +++- .../mqtt/auto/MQTTAutoSslAuthTest.java | 2 +- .../transport/stomp/StompSslAuthTest.java | 2 +- .../org/apache/activemq/bugs/AMQ6599Test.java | 2 +- .../network/NetworkReconnectSslNioTest.java | 2 +- .../transport/auto/AutoSslAuthTest.java | 4 ++-- .../auto/AutoTransportConnectionsTest.java | 3 --- .../transport/nio/NIOSSLBasicTest.java | 11 +++++----- .../transport/nio/NIOSSLLoadTest.java | 2 +- .../transport/nio/NIOSSLWindowSizeTest.java | 20 +++++++++---------- .../bugs/amq4126/JaasStompSSLBroker.xml | 8 ++++---- .../JaasDualAuthenticationNetworkBridge.xml | 2 +- ...sDualAuthenticationNetworkBridgeNioSsl.xml | 2 +- 17 files changed, 36 insertions(+), 37 deletions(-) diff --git a/activemq-amqp/src/test/java/org/apache/activemq/transport/amqp/AmqpTestSupport.java b/activemq-amqp/src/test/java/org/apache/activemq/transport/amqp/AmqpTestSupport.java index 8fb26f2fce..69d1998629 100644 --- a/activemq-amqp/src/test/java/org/apache/activemq/transport/amqp/AmqpTestSupport.java +++ b/activemq-amqp/src/test/java/org/apache/activemq/transport/amqp/AmqpTestSupport.java @@ -185,7 +185,7 @@ public class AmqpTestSupport { } if (isUseSslConnector()) { connector = brokerService.addConnector( - "amqp+ssl://0.0.0.0:" + amqpSslPort + "?transport.verifyHostName=false&transport.tcpNoDelay=true&transport.transformer=" + getAmqpTransformer() + getAdditionalConfig()); + "amqp+ssl://0.0.0.0:" + amqpSslPort + "?transport.tcpNoDelay=true&transport.transformer=" + getAmqpTransformer() + getAdditionalConfig()); amqpSslPort = connector.getConnectUri().getPort(); amqpSslURI = connector.getPublishableConnectURI(); LOG.debug("Using amqp+ssl port " + amqpSslPort); @@ -199,7 +199,7 @@ public class AmqpTestSupport { } if (isUseNioPlusSslConnector()) { connector = brokerService.addConnector( - "amqp+nio+ssl://0.0.0.0:" + amqpNioPlusSslPort + "?transport.verifyHostName=false&transport.tcpNoDelay=true&transport.transformer=" + getAmqpTransformer() + getAdditionalConfig()); + "amqp+nio+ssl://0.0.0.0:" + amqpNioPlusSslPort + "?transport.tcpNoDelay=true&transport.transformer=" + getAmqpTransformer() + getAdditionalConfig()); amqpNioPlusSslPort = connector.getConnectUri().getPort(); amqpNioPlusSslURI = connector.getPublishableConnectURI(); LOG.debug("Using amqp+nio+ssl port " + amqpNioPlusSslPort); diff --git a/activemq-amqp/src/test/java/org/apache/activemq/transport/amqp/auto/JMSClientAutoSslAuthTest.java b/activemq-amqp/src/test/java/org/apache/activemq/transport/amqp/auto/JMSClientAutoSslAuthTest.java index d611ee69e5..40c1eb363c 100644 --- a/activemq-amqp/src/test/java/org/apache/activemq/transport/amqp/auto/JMSClientAutoSslAuthTest.java +++ b/activemq-amqp/src/test/java/org/apache/activemq/transport/amqp/auto/JMSClientAutoSslAuthTest.java @@ -79,7 +79,7 @@ public class JMSClientAutoSslAuthTest extends JMSClientTestSupport { @Override protected String getAdditionalConfig() { - return "?transport.needClientAuth=true&transport.verifyHostName=false"; + return "?transport.needClientAuth=true"; } diff --git a/activemq-client/src/main/java/org/apache/activemq/transport/nio/NIOSSLTransport.java b/activemq-client/src/main/java/org/apache/activemq/transport/nio/NIOSSLTransport.java index 74aa3425dc..9f5e65d9a2 100644 --- a/activemq-client/src/main/java/org/apache/activemq/transport/nio/NIOSSLTransport.java +++ b/activemq-client/src/main/java/org/apache/activemq/transport/nio/NIOSSLTransport.java @@ -57,7 +57,7 @@ public class NIOSSLTransport extends NIOTransport { protected boolean wantClientAuth; protected String[] enabledCipherSuites; protected String[] enabledProtocols; - protected boolean verifyHostName = true; + protected boolean verifyHostName = false; protected SSLContext sslContext; protected SSLEngine sslEngine; diff --git a/activemq-client/src/main/java/org/apache/activemq/transport/tcp/SslTransport.java b/activemq-client/src/main/java/org/apache/activemq/transport/tcp/SslTransport.java index 91ba42caa8..f512cce461 100644 --- a/activemq-client/src/main/java/org/apache/activemq/transport/tcp/SslTransport.java +++ b/activemq-client/src/main/java/org/apache/activemq/transport/tcp/SslTransport.java @@ -96,6 +96,7 @@ public class SslTransport extends TcpTransport { verifyHostName = Boolean.parseBoolean(socketOptions.get("verifyHostName").toString()); socketOptions.remove("verifyHostName"); } else { + //If null and not set then this is a client so default to true verifyHostName = true; } } diff --git a/activemq-client/src/main/java/org/apache/activemq/transport/tcp/TcpTransportServer.java b/activemq-client/src/main/java/org/apache/activemq/transport/tcp/TcpTransportServer.java index 61aec1d22f..6d642c02d4 100644 --- a/activemq-client/src/main/java/org/apache/activemq/transport/tcp/TcpTransportServer.java +++ b/activemq-client/src/main/java/org/apache/activemq/transport/tcp/TcpTransportServer.java @@ -80,7 +80,7 @@ public class TcpTransportServer extends TransportServerThreadSupport implements protected int minmumWireFormatVersion; protected boolean useQueueForAccept = true; protected boolean allowLinkStealing; - protected boolean verifyHostName = true; + protected boolean verifyHostName = false; /** * trace=true -> the Transport stack where this TcpTransport object will be, will have a TransportLogger layer @@ -176,6 +176,8 @@ public class TcpTransportServer extends TransportServerThreadSupport implements if (socket instanceof SSLServerSocket) { if (transportOptions.containsKey("verifyHostName")) { verifyHostName = Boolean.parseBoolean(transportOptions.get("verifyHostName").toString()); + } else { + transportOptions.put("verifyHostName", verifyHostName); } if (verifyHostName) { diff --git a/activemq-mqtt/src/test/java/org/apache/activemq/transport/mqtt/auto/MQTTAutoSslAuthTest.java b/activemq-mqtt/src/test/java/org/apache/activemq/transport/mqtt/auto/MQTTAutoSslAuthTest.java index 3fb67a4ef7..4fae9c44c6 100644 --- a/activemq-mqtt/src/test/java/org/apache/activemq/transport/mqtt/auto/MQTTAutoSslAuthTest.java +++ b/activemq-mqtt/src/test/java/org/apache/activemq/transport/mqtt/auto/MQTTAutoSslAuthTest.java @@ -55,7 +55,7 @@ public class MQTTAutoSslAuthTest extends MQTTTestSupport { */ public MQTTAutoSslAuthTest(String protocol) { this.protocol = protocol; - protocolConfig = "transport.needClientAuth=true&transport.verifyHostName=false&"; + protocolConfig = "transport.needClientAuth=true"; } @Override diff --git a/activemq-stomp/src/test/java/org/apache/activemq/transport/stomp/StompSslAuthTest.java b/activemq-stomp/src/test/java/org/apache/activemq/transport/stomp/StompSslAuthTest.java index d295dfb8ae..03c24c436b 100644 --- a/activemq-stomp/src/test/java/org/apache/activemq/transport/stomp/StompSslAuthTest.java +++ b/activemq-stomp/src/test/java/org/apache/activemq/transport/stomp/StompSslAuthTest.java @@ -54,7 +54,7 @@ public class StompSslAuthTest extends StompTest { @Override public void addOpenWireConnector() throws Exception { - TransportConnector connector = brokerService.addConnector("ssl://0.0.0.0:0?transport.needClientAuth=true&transport.verifyHostName=false"); + TransportConnector connector = brokerService.addConnector("ssl://0.0.0.0:0?transport.needClientAuth=true"); cf = new ActiveMQConnectionFactory(connector.getPublishableConnectString() + "?socket.verifyHostName=false"); } diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/bugs/AMQ6599Test.java b/activemq-unit-tests/src/test/java/org/apache/activemq/bugs/AMQ6599Test.java index 3de3ee935e..72c9b88ddc 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/bugs/AMQ6599Test.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/bugs/AMQ6599Test.java @@ -71,7 +71,7 @@ public class AMQ6599Test { brokerService.setPersistent(false); TransportConnector connector = brokerService.addConnector(protocol + - "://localhost:0?transport.soTimeout=3500&transport.verifyHostName=false"); + "://localhost:0?transport.soTimeout=3500"); connector.setName("connector"); uri = connector.getPublishableConnectString(); diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/network/NetworkReconnectSslNioTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/network/NetworkReconnectSslNioTest.java index b97fdcfc6d..95309a30fb 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/network/NetworkReconnectSslNioTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/network/NetworkReconnectSslNioTest.java @@ -47,7 +47,7 @@ public class NetworkReconnectSslNioTest { remote.setSslContext(sslContext); remote.setUseJmx(false); remote.setPersistent(false); - final TransportConnector transportConnector = remote.addConnector("nio+ssl://0.0.0.0:0?transport.verifyHostName=false"); + final TransportConnector transportConnector = remote.addConnector("nio+ssl://0.0.0.0:0"); remote.start(); BrokerService local = new BrokerService(); diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/transport/auto/AutoSslAuthTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/transport/auto/AutoSslAuthTest.java index f24620d228..be6043b363 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/transport/auto/AutoSslAuthTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/transport/auto/AutoSslAuthTest.java @@ -75,7 +75,7 @@ public class AutoSslAuthTest { BrokerService brokerService = new BrokerService(); brokerService.setPersistent(false); - TransportConnector connector = brokerService.addConnector(protocol + "://localhost:0?transport.needClientAuth=true&transport.verifyHostName=false"); + TransportConnector connector = brokerService.addConnector(protocol + "://localhost:0?transport.needClientAuth=true"); connector.setName("auto"); uri = connector.getPublishableConnectString(); @@ -126,7 +126,7 @@ public class AutoSslAuthTest { @Test(timeout = 60000) public void testConnect() throws Exception { ActiveMQConnectionFactory factory = new ActiveMQConnectionFactory(); - factory.setBrokerURL(uri + "?socket.verifyHostName=false"); + factory.setBrokerURL(uri); //Create 5 connections to make sure all are properly set for (int i = 0; i < 5; i++) { diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/transport/auto/AutoTransportConnectionsTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/transport/auto/AutoTransportConnectionsTest.java index 1de13accc0..46f82d487c 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/transport/auto/AutoTransportConnectionsTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/transport/auto/AutoTransportConnectionsTest.java @@ -103,9 +103,6 @@ public class AutoTransportConnectionsTest { } public void configureConnectorAndStart(String bindAddress) throws Exception { - if (bindAddress.contains("ssl")) { - bindAddress += bindAddress.contains("?") ? "&transport.verifyHostName=false" : "?transport.verifyHostName=false"; - } connector = service.addConnector(bindAddress); connectionUri = connector.getPublishableConnectString(); if (connectionUri.contains("ssl")) { diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/transport/nio/NIOSSLBasicTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/transport/nio/NIOSSLBasicTest.java index d9ea3aeba7..6444d2c8bd 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/transport/nio/NIOSSLBasicTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/transport/nio/NIOSSLBasicTest.java @@ -80,28 +80,28 @@ public class NIOSSLBasicTest { @Test public void basicConnector() throws Exception { - BrokerService broker = createBroker("nio+ssl", getTransportType() + "://localhost:0?transport.needClientAuth=true&transport.verifyHostName=false"); + BrokerService broker = createBroker("nio+ssl", getTransportType() + "://localhost:0?transport.needClientAuth=true"); basicSendReceive("ssl://localhost:" + broker.getConnectorByName("nio+ssl").getConnectUri().getPort() + "?socket.verifyHostName=false"); stopBroker(broker); } @Test public void enabledCipherSuites() throws Exception { - BrokerService broker = createBroker("nio+ssl", getTransportType() + "://localhost:0?transport.needClientAuth=true&transport.verifyHostName=false&transport.enabledCipherSuites=TLS_RSA_WITH_AES_256_CBC_SHA256&transport.verifyHostName=false"); + BrokerService broker = createBroker("nio+ssl", getTransportType() + "://localhost:0?transport.needClientAuth=true&transport.verifyHostName=false&transport.enabledCipherSuites=TLS_RSA_WITH_AES_256_CBC_SHA256"); basicSendReceive("ssl://localhost:" + broker.getConnectorByName("nio+ssl").getConnectUri().getPort() + "?socket.verifyHostName=false"); stopBroker(broker); } @Test public void enabledProtocols() throws Exception { - BrokerService broker = createBroker("nio+ssl", getTransportType() + "://localhost:61616?transport.needClientAuth=true&transport.enabledProtocols=TLSv1,TLSv1.1,TLSv1.2&transport.verifyHostName=false"); + BrokerService broker = createBroker("nio+ssl", getTransportType() + "://localhost:61616?transport.needClientAuth=true&transport.enabledProtocols=TLSv1,TLSv1.1,TLSv1.2"); basicSendReceive("ssl://localhost:" + broker.getConnectorByName("nio+ssl").getConnectUri().getPort() + "?socket.verifyHostName=false"); stopBroker(broker); } - //Client/server is missing verifyHostName=false so it should fail as cert doesn't have right host name + //Client is missing verifyHostName=false so it should fail as cert doesn't have right host name @Test(expected = Exception.class) - public void verifyHostNameError() throws Exception { + public void verifyHostNameErrorClient() throws Exception { BrokerService broker = null; try { broker = createBroker("nio+ssl", getTransportType() + "://localhost:61616?transport.needClientAuth=true"); @@ -113,7 +113,6 @@ public class NIOSSLBasicTest { } } - public void basicSendReceive(String uri) throws Exception { ActiveMQConnectionFactory factory = new ActiveMQConnectionFactory(uri); Connection connection = factory.createConnection(); diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/transport/nio/NIOSSLLoadTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/transport/nio/NIOSSLLoadTest.java index 4a92d66c51..0e50f4449e 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/transport/nio/NIOSSLLoadTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/transport/nio/NIOSSLLoadTest.java @@ -74,7 +74,7 @@ public class NIOSSLLoadTest { broker = new BrokerService(); broker.setPersistent(false); broker.setUseJmx(false); - connector = broker.addConnector("nio+ssl://localhost:0?transport.needClientAuth=true&transport.verifyHostName=false&transport.enabledCipherSuites=TLS_RSA_WITH_AES_256_CBC_SHA256"); + connector = broker.addConnector("nio+ssl://localhost:0?transport.needClientAuth=true&transport.enabledCipherSuites=TLS_RSA_WITH_AES_256_CBC_SHA256"); broker.start(); broker.waitUntilStarted(); diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/transport/nio/NIOSSLWindowSizeTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/transport/nio/NIOSSLWindowSizeTest.java index e92b4fe04d..17cdc415f2 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/transport/nio/NIOSSLWindowSizeTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/transport/nio/NIOSSLWindowSizeTest.java @@ -30,11 +30,11 @@ import javax.jms.Session; @SuppressWarnings("javadoc") public class NIOSSLWindowSizeTest extends TestCase { - + BrokerService broker; Connection connection; Session session; - + public static final String KEYSTORE_TYPE = "jks"; public static final String PASSWORD = "password"; public static final String SERVER_KEYSTORE = "src/test/resources/server.keystore"; @@ -46,7 +46,7 @@ public class NIOSSLWindowSizeTest extends TestCase { public static final int MESSAGE_SIZE = 65536; byte[] messageData; - + @Override protected void setUp() throws Exception { System.setProperty("javax.net.ssl.trustStore", TRUST_KEYSTORE); @@ -59,19 +59,19 @@ public class NIOSSLWindowSizeTest extends TestCase { broker = new BrokerService(); broker.setPersistent(false); broker.setUseJmx(false); - TransportConnector connector = broker.addConnector("nio+ssl://localhost:0?transport.needClientAuth=true&transport.verifyHostName=false"); + TransportConnector connector = broker.addConnector("nio+ssl://localhost:0?transport.needClientAuth=true"); broker.start(); broker.waitUntilStarted(); - + messageData = new byte[MESSAGE_SIZE]; for (int i = 0; i < MESSAGE_SIZE; i++) { messageData[i] = (byte) (i & 0xff); } - + ActiveMQConnectionFactory factory = new ActiveMQConnectionFactory("nio+ssl://localhost:" + connector.getConnectUri().getPort()); connection = factory.createConnection(); - session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); + session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); connection.start(); } @@ -100,14 +100,14 @@ public class NIOSSLWindowSizeTest extends TestCase { prod.send(msg); } finally { prod.close(); - } + } MessageConsumer cons = null; - try + try { cons = session.createConsumer(dest); assertNotNull(cons.receive(30000L)); } finally { cons.close(); - } + } } } diff --git a/activemq-unit-tests/src/test/resources/org/apache/activemq/bugs/amq4126/JaasStompSSLBroker.xml b/activemq-unit-tests/src/test/resources/org/apache/activemq/bugs/amq4126/JaasStompSSLBroker.xml index 3778173912..70af5fa1d8 100644 --- a/activemq-unit-tests/src/test/resources/org/apache/activemq/bugs/amq4126/JaasStompSSLBroker.xml +++ b/activemq-unit-tests/src/test/resources/org/apache/activemq/bugs/amq4126/JaasStompSSLBroker.xml @@ -36,10 +36,10 @@ - - - - + + + + diff --git a/activemq-unit-tests/src/test/resources/org/apache/activemq/security/JaasDualAuthenticationNetworkBridge.xml b/activemq-unit-tests/src/test/resources/org/apache/activemq/security/JaasDualAuthenticationNetworkBridge.xml index e2eddb9a81..faae4dbf0c 100644 --- a/activemq-unit-tests/src/test/resources/org/apache/activemq/security/JaasDualAuthenticationNetworkBridge.xml +++ b/activemq-unit-tests/src/test/resources/org/apache/activemq/security/JaasDualAuthenticationNetworkBridge.xml @@ -171,7 +171,7 @@ - + diff --git a/activemq-unit-tests/src/test/resources/org/apache/activemq/security/JaasDualAuthenticationNetworkBridgeNioSsl.xml b/activemq-unit-tests/src/test/resources/org/apache/activemq/security/JaasDualAuthenticationNetworkBridgeNioSsl.xml index eb3d2fd6f9..9e5e7d1f91 100644 --- a/activemq-unit-tests/src/test/resources/org/apache/activemq/security/JaasDualAuthenticationNetworkBridgeNioSsl.xml +++ b/activemq-unit-tests/src/test/resources/org/apache/activemq/security/JaasDualAuthenticationNetworkBridgeNioSsl.xml @@ -171,7 +171,7 @@ - +