From 2d394b383cf80681f53e24003cbaed9b1c6c1de6 Mon Sep 17 00:00:00 2001 From: gtully Date: Wed, 13 Mar 2019 10:21:19 +0000 Subject: [PATCH] AMQ-7167 - ensure remote IP is visible in acceptor error messages from the transport connector - test and fix --- .../activemq/broker/TransportConnector.java | 7 ++-- .../transport/nio/NIOSSLBasicTest.java | 32 ++++++++++++++++++- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/TransportConnector.java b/activemq-broker/src/main/java/org/apache/activemq/broker/TransportConnector.java index ba2a3a906f..42abf5c41a 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/TransportConnector.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/TransportConnector.java @@ -208,6 +208,7 @@ public class TransportConnector implements Connector, BrokerServiceAware { getServer().setAcceptListener(new TransportAcceptListener() { @Override public void onAccept(final Transport transport) { + final String remoteHost = transport.getRemoteAddress(); try { brokerService.getTaskRunnerFactory().execute(new Runnable() { @Override @@ -220,14 +221,12 @@ public class TransportConnector implements Connector, BrokerServiceAware { throw new BrokerStoppedException("Broker " + brokerService + " is being stopped"); } } catch (Exception e) { - String remoteHost = transport.getRemoteAddress(); ServiceSupport.dispose(transport); onAcceptError(e, remoteHost); } } }); } catch (Exception e) { - String remoteHost = transport.getRemoteAddress(); ServiceSupport.dispose(transport); onAcceptError(e, remoteHost); } @@ -240,9 +239,9 @@ public class TransportConnector implements Connector, BrokerServiceAware { private void onAcceptError(Exception error, String remoteHost) { if (brokerService != null && brokerService.isStopping()) { - LOG.info("Could not accept connection during shutdown {} : {}", (remoteHost == null ? "" : "from " + remoteHost), error); + LOG.info("Could not accept connection during shutdown {} : {}", (remoteHost == null ? "" : "from " + remoteHost), error.getLocalizedMessage()); } else { - LOG.error("Could not accept connection {} : {}", (remoteHost == null ? "" : "from " + remoteHost), error); + LOG.error("Could not accept connection {} : {}", (remoteHost == null ? "" : "from " + remoteHost), error.getLocalizedMessage()); LOG.debug("Reason: " + error, error); } } 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 6444d2c8bd..01c5e57067 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 @@ -29,12 +29,21 @@ import javax.net.ssl.SSLHandshakeException; import org.apache.activemq.ActiveMQConnectionFactory; import org.apache.activemq.broker.BrokerService; import org.apache.activemq.broker.TransportConnector; +import org.apache.activemq.util.DefaultTestAppender; +import org.apache.log4j.Level; +import org.apache.log4j.spi.LoggingEvent; import org.junit.After; import org.junit.Before; import org.junit.Test; import junit.framework.TestCase; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + +import static junit.framework.TestCase.assertTrue; + public class NIOSSLBasicTest { public static final String KEYSTORE_TYPE = "jks"; @@ -87,7 +96,7 @@ public class NIOSSLBasicTest { @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"); + BrokerService broker = createBroker("nio+ssl", getTransportType() + "://localhost:0?transport.needClientAuth=true&transport.verifyHostName=false&transport.enabledCipherSuites=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256"); basicSendReceive("ssl://localhost:" + broker.getConnectorByName("nio+ssl").getConnectUri().getPort() + "?socket.verifyHostName=false"); stopBroker(broker); } @@ -102,14 +111,35 @@ public class NIOSSLBasicTest { //Client is missing verifyHostName=false so it should fail as cert doesn't have right host name @Test(expected = Exception.class) public void verifyHostNameErrorClient() throws Exception { + + final CountDownLatch gotLogMessage = new CountDownLatch(1); + final AtomicBoolean gotRemoteAddressInLog = new AtomicBoolean(); + final DefaultTestAppender appender = new DefaultTestAppender() { + @Override + public void doAppend(LoggingEvent event) { + if (event.getLevel().equals(Level.ERROR) && event.getRenderedMessage().contains("Could not accept connection")) { + gotLogMessage.countDown(); + if (event.getRenderedMessage().contains("tcp")) { + // got remote address + gotRemoteAddressInLog.set(true); + } + } + } + }; + org.apache.log4j.Logger rootLogger = org.apache.log4j.Logger.getRootLogger(); + rootLogger.addAppender(appender); + BrokerService broker = null; try { broker = createBroker("nio+ssl", getTransportType() + "://localhost:61616?transport.needClientAuth=true"); basicSendReceive("ssl://localhost:" + broker.getConnectorByName("nio+ssl").getConnectUri().getPort()); } finally { + gotLogMessage.await(5, TimeUnit.SECONDS); if (broker != null) { stopBroker(broker); } + rootLogger.removeAppender(appender); + assertTrue("Got remote address in log", gotRemoteAddressInLog.get()); } }