From 4e5d9016b3762ebf8b5918616295f8836867dce1 Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Tue, 13 Dec 2016 14:55:53 -0500 Subject: [PATCH] INT-4183: Add SSL Handshake Timeout for TCP JIRA: https://jira.spring.io/browse/INT-4183 Previously, this was hard-coded to 30 seconds. * Fix typos according PR comments --- .../ip/config/IpAdapterParserUtils.java | 4 ++ .../TcpConnectionFactoryFactoryBean.java | 14 +++++++ .../ip/config/TcpConnectionFactoryParser.java | 5 ++- .../connection/AbstractConnectionFactory.java | 21 ++++++++++ .../TcpNioClientConnectionFactory.java | 3 ++ .../tcp/connection/TcpNioSSLConnection.java | 16 +++++++- .../TcpNioServerConnectionFactory.java | 3 ++ .../ip/config/spring-integration-ip-5.0.xsd | 8 ++++ .../ip/config/ParserUnitTests-context.xml | 12 ++++++ .../ip/config/ParserUnitTests.java | 5 +++ .../ip/tcp/connection/SocketSupportTests.java | 41 +++++++++++++++++++ src/reference/asciidoc/ip.adoc | 3 ++ 12 files changed, 133 insertions(+), 2 deletions(-) diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/config/IpAdapterParserUtils.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/config/IpAdapterParserUtils.java index fe912c1dce..630b0f5186 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/config/IpAdapterParserUtils.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/config/IpAdapterParserUtils.java @@ -124,6 +124,10 @@ public abstract class IpAdapterParserUtils { public static final String MAPPER = "mapper"; + public static final String READ_DELAY = "read-delay"; + + public static final String SSL_HANDSHAKE_TIMEOUT = "ssl-handshake-timeout"; + private IpAdapterParserUtils() { } diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/config/TcpConnectionFactoryFactoryBean.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/config/TcpConnectionFactoryFactoryBean.java index f8bec67fda..1dc3aac70f 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/config/TcpConnectionFactoryFactoryBean.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/config/TcpConnectionFactoryFactoryBean.java @@ -112,6 +112,8 @@ public class TcpConnectionFactoryFactoryBean extends AbstractFactoryBean getDelayedReads() { return this.delayedReads; } diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioClientConnectionFactory.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioClientConnectionFactory.java index cd387c1951..c373c3d05d 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioClientConnectionFactory.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioClientConnectionFactory.java @@ -88,6 +88,9 @@ public class TcpNioClientConnectionFactory extends socketChannel, false, this.isLookupHost(), this.getApplicationEventPublisher(), this.getComponentName()); connection.setUsingDirectBuffers(this.usingDirectBuffers); connection.setTaskExecutor(this.getTaskExecutor()); + if (getSslHandshakeTimeout() != null && connection instanceof TcpNioSSLConnection) { + ((TcpNioSSLConnection) connection).setHandshakeTimeout(getSslHandshakeTimeout()); + } TcpConnectionSupport wrappedConnection = wrapConnection(connection); initializeConnection(wrappedConnection, socketChannel.socket()); socketChannel.configureBlocking(false); diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioSSLConnection.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioSSLConnection.java index 8b75c59d00..ff63fde733 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioSSLConnection.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioSSLConnection.java @@ -53,6 +53,8 @@ import org.springframework.util.Assert; */ public class TcpNioSSLConnection extends TcpNioConnection { + private static final int DEFAULT_HANDSHAKE_TIMEOUT = 30; + private final SSLEngine sslEngine; private volatile ByteBuffer decoded; @@ -67,6 +69,8 @@ public class TcpNioSSLConnection extends TcpNioConnection { private volatile boolean writerActive; + private volatile int handshakeTimeout = DEFAULT_HANDSHAKE_TIMEOUT; + private boolean needMoreNetworkData; public TcpNioSSLConnection(SocketChannel socketChannel, boolean server, boolean lookupHost, @@ -76,6 +80,15 @@ public class TcpNioSSLConnection extends TcpNioConnection { this.sslEngine = sslEngine; } + /** + * Set the timeout while waiting for handshake data (in seconds). Default 30. + * @param handshakeTimeout the timeout. + * @since 4.3.6 + */ + public void setHandshakeTimeout(int handshakeTimeout) { + this.handshakeTimeout = handshakeTimeout; + } + @Override public SSLSession getSslSession() { return this.sslEngine.getSession(); @@ -368,7 +381,8 @@ public class TcpNioSSLConnection extends TcpNioConnection { if (logger.isTraceEnabled()) { logger.trace("Writer waiting for handshake"); } - if (!TcpNioSSLConnection.this.semaphore.tryAcquire(30, TimeUnit.SECONDS)) { + if (!TcpNioSSLConnection.this.semaphore.tryAcquire(TcpNioSSLConnection.this.handshakeTimeout, + TimeUnit.SECONDS)) { throw new MessagingException("SSL Handshaking taking too long"); } if (logger.isTraceEnabled()) { diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioServerConnectionFactory.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioServerConnectionFactory.java index 665dd113c6..1b631989f4 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioServerConnectionFactory.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioServerConnectionFactory.java @@ -220,6 +220,9 @@ public class TcpNioServerConnectionFactory extends AbstractServerConnectionFacto } connection.setTaskExecutor(getTaskExecutor()); connection.setLastRead(now); + if (getSslHandshakeTimeout() != null && connection instanceof TcpNioSSLConnection) { + ((TcpNioSSLConnection) connection).setHandshakeTimeout(getSslHandshakeTimeout()); + } this.channelMap.put(channel, connection); channel.register(selector, SelectionKey.OP_READ, connection); connection.publishConnectionOpenEvent(); diff --git a/spring-integration-ip/src/main/resources/org/springframework/integration/ip/config/spring-integration-ip-5.0.xsd b/spring-integration-ip/src/main/resources/org/springframework/integration/ip/config/spring-integration-ip-5.0.xsd index 80228410b9..17fd09032a 100644 --- a/spring-integration-ip/src/main/resources/org/springframework/integration/ip/config/spring-integration-ip-5.0.xsd +++ b/spring-integration-ip/src/main/resources/org/springframework/integration/ip/config/spring-integration-ip-5.0.xsd @@ -693,6 +693,14 @@ + + + + The timeout (in seconds) to use while performing handshakes on SSL sockets; + only applies when 'using-nio' is 'true'. Default: 30. + + + diff --git a/spring-integration-ip/src/test/java/org/springframework/integration/ip/config/ParserUnitTests-context.xml b/spring-integration-ip/src/test/java/org/springframework/integration/ip/config/ParserUnitTests-context.xml index ccc7f8d8ea..ed4c0c075e 100644 --- a/spring-integration-ip/src/test/java/org/springframework/integration/ip/config/ParserUnitTests-context.xml +++ b/spring-integration-ip/src/test/java/org/springframework/integration/ip/config/ParserUnitTests-context.xml @@ -71,6 +71,7 @@ apply-sequence="true" using-nio="true" ssl-context-support="sslContextSupport" + ssl-handshake-timeout="43" /> @@ -89,6 +90,17 @@ socket-support="socketSupport" socket-factory-support="socketFactorySupport" /> + + diff --git a/spring-integration-ip/src/test/java/org/springframework/integration/ip/config/ParserUnitTests.java b/spring-integration-ip/src/test/java/org/springframework/integration/ip/config/ParserUnitTests.java index 24fa91092a..456603f34d 100644 --- a/spring-integration-ip/src/test/java/org/springframework/integration/ip/config/ParserUnitTests.java +++ b/spring-integration-ip/src/test/java/org/springframework/integration/ip/config/ParserUnitTests.java @@ -256,6 +256,9 @@ public class ParserUnitTests { @Autowired TcpNetServerConnectionFactory secureServer; + @Autowired + TcpNioServerConnectionFactory secureServerNio; + @Autowired TcpSocketFactorySupport socketFactorySupport; @@ -343,6 +346,7 @@ public class ParserUnitTests { Object connectionSupport = TestUtils.getPropertyValue(cfS1Nio, "tcpNioConnectionSupport"); assertTrue(connectionSupport instanceof DefaultTcpNioSSLConnectionSupport); assertNotNull(TestUtils.getPropertyValue(connectionSupport, "sslContext")); + assertEquals(43, TestUtils.getPropertyValue(this.cfS1Nio, "sslHandshakeTimeout")); } @Test @@ -656,6 +660,7 @@ public class ParserUnitTests { DirectFieldAccessor dfa = new DirectFieldAccessor(secureServer); assertSame(socketFactorySupport, dfa.getPropertyValue("tcpSocketFactorySupport")); assertSame(socketSupport, dfa.getPropertyValue("tcpSocketSupport")); + assertEquals(34, TestUtils.getPropertyValue(this.secureServerNio, "sslHandshakeTimeout")); } public static class FooAdvice extends AbstractRequestHandlerAdvice { diff --git a/spring-integration-ip/src/test/java/org/springframework/integration/ip/tcp/connection/SocketSupportTests.java b/spring-integration-ip/src/test/java/org/springframework/integration/ip/tcp/connection/SocketSupportTests.java index c7d3e9056a..409693d9d0 100644 --- a/spring-integration-ip/src/test/java/org/springframework/integration/ip/tcp/connection/SocketSupportTests.java +++ b/spring-integration-ip/src/test/java/org/springframework/integration/ip/tcp/connection/SocketSupportTests.java @@ -34,6 +34,7 @@ import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import javax.net.ServerSocketFactory; import javax.net.SocketFactory; @@ -295,6 +296,9 @@ Certificate fingerprints: assertTrue(latch.await(10, TimeUnit.SECONDS)); assertEquals("Hello, world!", new String((byte[]) messages.get(0).getPayload())); assertNotNull(messages.get(0).getHeaders().get("cipher")); + + client.stop(); + server.stop(); } @Test @@ -330,12 +334,16 @@ Certificate fingerprints: connection.send(new GenericMessage("Hello, world!")); assertTrue(latch.await(10, TimeUnit.SECONDS)); assertEquals("Hello, world!", new String((byte[]) messages.get(0).getPayload())); + + client.stop(); + server.stop(); } @Test public void testNioClientAndServerSSL() throws Exception { System.setProperty("javax.net.debug", "all"); // SSL activity in the console TcpNioServerConnectionFactory server = new TcpNioServerConnectionFactory(0); + server.setSslHandshakeTimeout(43); DefaultTcpSSLContextSupport sslContextSupport = new DefaultTcpSSLContextSupport("test.ks", "test.truststore.ks", "secret", "secret"); sslContextSupport.setProtocol("SSL"); @@ -351,19 +359,36 @@ Certificate fingerprints: return false; }); server.setMapper(new SSLMapper()); + final AtomicReference serverConnectionId = new AtomicReference<>(); + server.setApplicationEventPublisher(e -> { + if (e instanceof TcpConnectionOpenEvent) { + serverConnectionId.set(((TcpConnectionEvent) e).getConnectionId()); + } + }); server.start(); TestingUtilities.waitListening(server, null); TcpNioClientConnectionFactory client = new TcpNioClientConnectionFactory("localhost", server.getPort()); + client.setSslHandshakeTimeout(34); client.setTcpNioConnectionSupport(tcpNioConnectionSupport); client.registerListener(message -> false); + client.setApplicationEventPublisher(e -> { }); client.start(); TcpConnection connection = client.getConnection(); + assertEquals(34, TestUtils.getPropertyValue(connection, "handshakeTimeout")); connection.send(new GenericMessage("Hello, world!")); assertTrue(latch.await(10, TimeUnit.SECONDS)); assertEquals("Hello, world!", new String((byte[]) messages.get(0).getPayload())); assertNotNull(messages.get(0).getHeaders().get("cipher")); + + Map connections = TestUtils.getPropertyValue(server, "connections", Map.class); + Object serverConnection = connections.get(serverConnectionId.get()); + assertNotNull(serverConnection); + assertEquals(43, TestUtils.getPropertyValue(serverConnection, "handshakeTimeout")); + + client.stop(); + server.stop(); } @Test @@ -394,6 +419,12 @@ Certificate fingerprints: ByteArrayCrLfSerializer deserializer = new ByteArrayCrLfSerializer(); deserializer.setMaxMessageSize(120000); server.setDeserializer(deserializer); + final AtomicReference serverConnectionId = new AtomicReference<>(); + server.setApplicationEventPublisher(e -> { + if (e instanceof TcpConnectionOpenEvent) { + serverConnectionId.set(((TcpConnectionEvent) e).getConnectionId()); + } + }); server.start(); TestingUtilities.waitListening(server, null); @@ -410,9 +441,11 @@ Certificate fingerprints: return false; }); client.setDeserializer(deserializer); + client.setApplicationEventPublisher(e -> { }); client.start(); TcpConnection connection = client.getConnection(); + assertEquals(30, TestUtils.getPropertyValue(connection, "handshakeTimeout")); byte[] bytes = new byte[100000]; connection.send(new GenericMessage("Hello, world!" + new String(bytes))); assertTrue(latch.await(60, TimeUnit.SECONDS)); @@ -422,6 +455,14 @@ Certificate fingerprints: payload = (byte[]) messages.get(1).getPayload(); assertEquals(13 + bytes.length, payload.length); assertEquals("Hello, world!", new String(payload).substring(0, 13)); + + Map connections = TestUtils.getPropertyValue(server, "connections", Map.class); + Object serverConnection = connections.get(serverConnectionId.get()); + assertNotNull(serverConnection); + assertEquals(30, TestUtils.getPropertyValue(serverConnection, "handshakeTimeout")); + + client.stop(); + server.stop(); } private static class Replier implements TcpSender { diff --git a/src/reference/asciidoc/ip.adoc b/src/reference/asciidoc/ip.adoc index 296a658ab3..a9e913a163 100644 --- a/src/reference/asciidoc/ip.adoc +++ b/src/reference/asciidoc/ip.adoc @@ -893,6 +893,9 @@ The `DefaulTcpSSLContextSupport` class also has an optional 'protocol' property, The keystore file names (first two constructor arguments) use the Spring `Resource` abstraction; by default the files will be located on the classpath, but this can be overridden by using the `file:` prefix, to find the files on the filesystem instead. +Starting with _version 4.3.6_, when using NIO, you can specify an `ssl-handshake-timeout` (seconds) on the connection factory. +This timeout (default 30) is used during SSL handshake when waiting for data; if the timeout is exceeded, the process is aborted and the socket closed. + ==== Advanced Techniques In many cases, the configuration described above is all that is needed to enable secure communication over TCP/IP.