From 4769de8a2b5129b4745d1b9af465a1195ca895ac Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 16 Oct 2019 13:02:24 +0200 Subject: [PATCH 1/2] Issue #4209 - Unused TLS connection is not closed in Java 11. Code cleanup. Signed-off-by: Simone Bordet --- .../jetty/client/HttpClientTLSTest.java | 46 +++++++-------- .../io/ssl/SslClientConnectionFactory.java | 36 ++++++++++-- .../eclipse/jetty/io/ssl/SslConnection.java | 57 +++++++++++++++---- 3 files changed, 99 insertions(+), 40 deletions(-) diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java index d5c17170c12..dbfbdd93669 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java @@ -67,8 +67,6 @@ public class HttpClientTLSTest private ServerConnector connector; private HttpClient client; - private SSLSocket sslSocket; - private void startServer(SslContextFactory sslContextFactory, Handler handler) throws Exception { ExecutorThreadPool serverThreads = new ExecutorThreadPool(); @@ -420,16 +418,16 @@ public class HttpClientTLSTest String host = "localhost"; int port = connector.getLocalPort(); - Socket socket = new Socket(host, port); - sslSocket = (SSLSocket)clientTLSFactory.getSslContext().getSocketFactory().createSocket(socket, host, port, true); + Socket socket1 = new Socket(host, port); + SSLSocket sslSocket1 = (SSLSocket)clientTLSFactory.getSslContext().getSocketFactory().createSocket(socket1, host, port, true); CountDownLatch handshakeLatch1 = new CountDownLatch(1); AtomicReference session1 = new AtomicReference<>(); - sslSocket.addHandshakeCompletedListener(event -> + sslSocket1.addHandshakeCompletedListener(event -> { session1.set(event.getSession().getId()); handshakeLatch1.countDown(); }); - sslSocket.startHandshake(); + sslSocket1.startHandshake(); assertTrue(handshakeLatch1.await(5, TimeUnit.SECONDS)); // In TLS 1.3 the server sends a NewSessionTicket post-handshake message @@ -437,29 +435,29 @@ public class HttpClientTLSTest assertThrows(SocketTimeoutException.class, () -> { - sslSocket.setSoTimeout(1000); - sslSocket.getInputStream().read(); + sslSocket1.setSoTimeout(1000); + sslSocket1.getInputStream().read(); }); // The client closes abruptly. - socket.close(); + socket1.close(); // Try again and compare the session ids. - socket = new Socket(host, port); - sslSocket = (SSLSocket)clientTLSFactory.getSslContext().getSocketFactory().createSocket(socket, host, port, true); + Socket socket2 = new Socket(host, port); + SSLSocket sslSocket2 = (SSLSocket)clientTLSFactory.getSslContext().getSocketFactory().createSocket(socket2, host, port, true); CountDownLatch handshakeLatch2 = new CountDownLatch(1); AtomicReference session2 = new AtomicReference<>(); - sslSocket.addHandshakeCompletedListener(event -> + sslSocket2.addHandshakeCompletedListener(event -> { session2.set(event.getSession().getId()); handshakeLatch2.countDown(); }); - sslSocket.startHandshake(); + sslSocket2.startHandshake(); assertTrue(handshakeLatch2.await(5, TimeUnit.SECONDS)); assertArrayEquals(session1.get(), session2.get()); - sslSocket.close(); + sslSocket2.close(); } @Test @@ -477,7 +475,7 @@ public class HttpClientTLSTest protected ClientConnectionFactory newSslClientConnectionFactory(SslContextFactory sslContextFactory, ClientConnectionFactory connectionFactory) { SslClientConnectionFactory ssl = (SslClientConnectionFactory)super.newSslClientConnectionFactory(sslContextFactory, connectionFactory); - ssl.setAllowMissingCloseMessage(false); + ssl.setRequireCloseMessage(true); return ssl; } }; @@ -505,19 +503,19 @@ public class HttpClientTLSTest break; } - // If the response is Content-Length delimited, allowing the - // missing TLS Close Message is fine because the application - // will see a EOFException anyway. - // If the response is connection delimited, allowing the - // missing TLS Close Message is bad because the application - // will see a successful response with truncated content. + // If the response is Content-Length delimited, the lack of + // the TLS Close Message is fine because the application + // will see a EOFException anyway: the Content-Length and + // the actual content bytes count won't match. + // If the response is connection delimited, the lack of the + // TLS Close Message is bad because the application will + // see a successful response, but with truncated content. - // Verify that by not allowing the missing - // TLS Close Message we get a response failure. + // Verify that by requiring the TLS Close Message we get + // a response failure. byte[] half = new byte[8]; String response = "HTTP/1.1 200 OK\r\n" + -// "Content-Length: " + (half.length * 2) + "\r\n" + "Connection: close\r\n" + "\r\n"; OutputStream output = sslSocket.getOutputStream(); diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslClientConnectionFactory.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslClientConnectionFactory.java index 236d4397f78..cfe960bd56f 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslClientConnectionFactory.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslClientConnectionFactory.java @@ -47,7 +47,7 @@ public class SslClientConnectionFactory implements ClientConnectionFactory private final ClientConnectionFactory connectionFactory; private boolean _directBuffersForEncryption = true; private boolean _directBuffersForDecryption = true; - private boolean allowMissingCloseMessage = true; + private boolean _requireCloseMessage; public SslClientConnectionFactory(SslContextFactory sslContextFactory, ByteBufferPool byteBufferPool, Executor executor, ClientConnectionFactory connectionFactory) { @@ -77,14 +77,42 @@ public class SslClientConnectionFactory implements ClientConnectionFactory return _directBuffersForEncryption; } + /** + * @return whether is not required that peers send the TLS {@code close_notify} message + * @deprecated use {@link #isRequireCloseMessage()} instead + */ + @Deprecated public boolean isAllowMissingCloseMessage() { - return allowMissingCloseMessage; + return !isRequireCloseMessage(); } + /** + * @param allowMissingCloseMessage whether is not required that peers send the TLS {@code close_notify} message + * @deprecated use {@link #setRequireCloseMessage(boolean)} instead + */ + @Deprecated public void setAllowMissingCloseMessage(boolean allowMissingCloseMessage) { - this.allowMissingCloseMessage = allowMissingCloseMessage; + setRequireCloseMessage(!allowMissingCloseMessage); + } + + /** + * @return whether peers must send the TLS {@code close_notify} message + * @see SslConnection#isRequireCloseMessage() + */ + public boolean isRequireCloseMessage() + { + return _requireCloseMessage; + } + + /** + * @param requireCloseMessage whether peers must send the TLS {@code close_notify} message + * @see SslConnection#setRequireCloseMessage(boolean) + */ + public void setRequireCloseMessage(boolean requireCloseMessage) + { + _requireCloseMessage = requireCloseMessage; } @Override @@ -120,7 +148,7 @@ public class SslClientConnectionFactory implements ClientConnectionFactory SslConnection sslConnection = (SslConnection)connection; sslConnection.setRenegotiationAllowed(sslContextFactory.isRenegotiationAllowed()); sslConnection.setRenegotiationLimit(sslContextFactory.getRenegotiationLimit()); - sslConnection.setAllowMissingCloseMessage(isAllowMissingCloseMessage()); + sslConnection.setRequireCloseMessage(isRequireCloseMessage()); ContainerLifeCycle connector = (ContainerLifeCycle)context.get(ClientConnectionFactory.CONNECTOR_CONTEXT_KEY); connector.getBeans(SslHandshakeListener.class).forEach(sslConnection::addHandshakeListener); } diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java index 6e61f5ad451..5dd6da588b1 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java @@ -113,7 +113,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr private boolean _renegotiationAllowed; private int _renegotiationLimit = -1; private boolean _closedOutbound; - private boolean _allowMissingCloseMessage = true; + private boolean _requireCloseMessage; private FlushState _flushState = FlushState.IDLE; private FillState _fillState = FillState.IDLE; private AtomicReference _handshake = new AtomicReference<>(Handshake.INITIAL); @@ -231,7 +231,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr } /** - * @return The number of renegotions allowed for this connection. When the limit + * @return The number of renegotiations allowed for this connection. When the limit * is 0 renegotiation will be denied. If the limit is less than 0 then no limit is applied. */ public int getRenegotiationLimit() @@ -240,7 +240,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr } /** - * @param renegotiationLimit The number of renegotions allowed for this connection. + * @param renegotiationLimit The number of renegotiations allowed for this connection. * When the limit is 0 renegotiation will be denied. If the limit is less than 0 then no limit is applied. * Default -1. */ @@ -249,14 +249,46 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr _renegotiationLimit = renegotiationLimit; } + /** + * @return whether is not required that peers send the TLS {@code close_notify} message + * @deprecated use inverted {@link #isRequireCloseMessage()} instead + */ + @Deprecated public boolean isAllowMissingCloseMessage() { - return _allowMissingCloseMessage; + return !isRequireCloseMessage(); } + /** + * @param allowMissingCloseMessage whether is not required that peers send the TLS {@code close_notify} message + * @deprecated use inverted {@link #setRequireCloseMessage(boolean)} instead + */ + @Deprecated public void setAllowMissingCloseMessage(boolean allowMissingCloseMessage) { - this._allowMissingCloseMessage = allowMissingCloseMessage; + setRequireCloseMessage(!allowMissingCloseMessage); + } + + /** + * @return whether peers must send the TLS {@code close_notify} message + */ + public boolean isRequireCloseMessage() + { + return _requireCloseMessage; + } + + /** + *

Sets whether it is required that a peer send the TLS {@code close_notify} message + * to indicate the will to close the connection, otherwise it may be interpreted as a + * truncation attack.

+ *

This option is only useful on clients, since typically servers cannot accept + * connection-delimited content that may be truncated.

+ * + * @param requireCloseMessage whether peers must send the TLS {@code close_notify} message + */ + public void setRequireCloseMessage(boolean requireCloseMessage) + { + _requireCloseMessage = requireCloseMessage; } private void acquireEncryptedInput() @@ -1096,15 +1128,15 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr @Override public void doShutdownOutput() { - final EndPoint endp = getEndPoint(); + EndPoint endPoint = getEndPoint(); try { boolean close; boolean flush = false; synchronized (_decryptedEndPoint) { - boolean ishut = endp.isInputShutdown(); - boolean oshut = endp.isOutputShutdown(); + boolean ishut = endPoint.isInputShutdown(); + boolean oshut = endPoint.isOutputShutdown(); if (LOG.isDebugEnabled()) LOG.debug("shutdownOutput: {} oshut={}, ishut={}", SslConnection.this, oshut, ishut); @@ -1128,19 +1160,19 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr // let's just flush the encrypted output in the background. ByteBuffer write = _encryptedOutput; if (BufferUtil.hasContent(write)) - endp.write(Callback.from(Callback.NOOP::succeeded, t -> endp.close()), write); + endPoint.write(Callback.from(Callback.NOOP::succeeded, t -> endPoint.close()), write); } } if (close) - endp.close(); + endPoint.close(); else ensureFillInterested(); } catch (Throwable x) { LOG.ignore(x); - endp.close(); + endPoint.close(); } } @@ -1152,7 +1184,8 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr } catch (Throwable x) { - LOG.ignore(x); + if (LOG.isDebugEnabled()) + LOG.debug(x); } } From 1e360244a59d3ddb135fab4557b021606feb1c52 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 16 Oct 2019 13:10:40 +0200 Subject: [PATCH 2/2] Fixes #4209 - Unused TLS connection is not closed in Java 11. Added workarounds for the Java 11 behavior. In fill(), call closeInbound() if we filled -1 and the handshake did not start yet. This avoids to send a ClientHello to the peer even if we are closing. In flush(), if the handshake status is NEED_UNWRAP but we are closing, force a wrap(). Added test cases. Signed-off-by: Simone Bordet --- .../jetty/client/HttpClientTLSTest.java | 205 ++++++++++++++++++ .../eclipse/jetty/io/ssl/SslConnection.java | 69 +++++- 2 files changed, 262 insertions(+), 12 deletions(-) diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java index dbfbdd93669..145f92346e2 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java @@ -19,17 +19,22 @@ package org.eclipse.jetty.client; import java.io.BufferedReader; +import java.io.IOException; import java.io.InputStreamReader; import java.io.OutputStream; import java.net.ServerSocket; import java.net.Socket; import java.net.SocketTimeoutException; +import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; +import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLException; import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLSocket; @@ -38,12 +43,20 @@ import org.eclipse.jetty.client.api.ContentResponse; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpScheme; import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.ClientConnectionFactory; +import org.eclipse.jetty.io.EndPoint; import org.eclipse.jetty.io.ssl.SslClientConnectionFactory; +import org.eclipse.jetty.io.ssl.SslConnection; import org.eclipse.jetty.io.ssl.SslHandshakeListener; +import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.Handler; +import org.eclipse.jetty.server.HttpConfiguration; +import org.eclipse.jetty.server.HttpConnectionFactory; +import org.eclipse.jetty.server.SecureRequestCustomizer; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.server.SslConnectionFactory; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.ExecutorThreadPool; @@ -555,4 +568,196 @@ public class HttpClientTLSTest assertTrue(latch.await(5, TimeUnit.SECONDS)); } + + @Test + public void testNeverUsedConnectionThenServerIdleTimeout() throws Exception + { + long idleTimeout = 2000; + + SslContextFactory serverTLSFactory = createServerSslContextFactory(); + QueuedThreadPool serverThreads = new QueuedThreadPool(); + serverThreads.setName("server"); + server = new Server(serverThreads); + HttpConfiguration httpConfig = new HttpConfiguration(); + httpConfig.addCustomizer(new SecureRequestCustomizer()); + HttpConnectionFactory http = new HttpConnectionFactory(httpConfig); + AtomicLong serverBytes = new AtomicLong(); + SslConnectionFactory ssl = new SslConnectionFactory(serverTLSFactory, http.getProtocol()) + { + @Override + protected SslConnection newSslConnection(Connector connector, EndPoint endPoint, SSLEngine engine) + { + return new SslConnection(connector.getByteBufferPool(), connector.getExecutor(), endPoint, engine, isDirectBuffersForEncryption(), isDirectBuffersForDecryption()) + { + @Override + protected int networkFill(ByteBuffer input) throws IOException + { + int n = super.networkFill(input); + if (n > 0) + serverBytes.addAndGet(n); + return n; + } + }; + } + }; + connector = new ServerConnector(server, 1, 1, ssl, http); + connector.setIdleTimeout(idleTimeout); + server.addConnector(connector); + server.setHandler(new EmptyServerHandler()); + server.start(); + + SslContextFactory clientTLSFactory = createClientSslContextFactory(); + QueuedThreadPool clientThreads = new QueuedThreadPool(); + clientThreads.setName("client"); + AtomicLong clientBytes = new AtomicLong(); + client = new HttpClient(clientTLSFactory) + { + @Override + protected ClientConnectionFactory newSslClientConnectionFactory(SslContextFactory sslContextFactory, ClientConnectionFactory connectionFactory) + { + if (sslContextFactory == null) + sslContextFactory = getSslContextFactory(); + return new SslClientConnectionFactory(sslContextFactory, getByteBufferPool(), getExecutor(), connectionFactory) + { + @Override + protected SslConnection newSslConnection(ByteBufferPool byteBufferPool, Executor executor, EndPoint endPoint, SSLEngine engine) + { + return new SslConnection(byteBufferPool, executor, endPoint, engine, isDirectBuffersForEncryption(), isDirectBuffersForDecryption()) + { + @Override + protected int networkFill(ByteBuffer input) throws IOException + { + int n = super.networkFill(input); + if (n > 0) + clientBytes.addAndGet(n); + return n; + } + }; + } + }; + } + }; + client.setExecutor(clientThreads); + client.start(); + + // Create a connection but don't use it. + String scheme = HttpScheme.HTTPS.asString(); + String host = "localhost"; + int port = connector.getLocalPort(); + HttpDestination destination = (HttpDestination)client.getDestination(scheme, host, port); + DuplexConnectionPool connectionPool = (DuplexConnectionPool)destination.getConnectionPool(); + // Trigger the creation of a new connection, but don't use it. + connectionPool.tryCreate(-1); + // Verify that the connection has been created. + while (true) + { + Thread.sleep(50); + if (connectionPool.getConnectionCount() == 1) + break; + } + + // Wait for the server to idle timeout the connection. + Thread.sleep(idleTimeout + idleTimeout / 2); + + // The connection should be gone from the connection pool. + assertEquals(0, connectionPool.getConnectionCount(), connectionPool.dump()); + assertEquals(0, serverBytes.get()); + assertEquals(0, clientBytes.get()); + } + + @Test + public void testNeverUsedConnectionThenClientIdleTimeout() throws Exception + { + SslContextFactory serverTLSFactory = createServerSslContextFactory(); + QueuedThreadPool serverThreads = new QueuedThreadPool(); + serverThreads.setName("server"); + server = new Server(serverThreads); + HttpConfiguration httpConfig = new HttpConfiguration(); + httpConfig.addCustomizer(new SecureRequestCustomizer()); + HttpConnectionFactory http = new HttpConnectionFactory(httpConfig); + AtomicLong serverBytes = new AtomicLong(); + SslConnectionFactory ssl = new SslConnectionFactory(serverTLSFactory, http.getProtocol()) + { + @Override + protected SslConnection newSslConnection(Connector connector, EndPoint endPoint, SSLEngine engine) + { + return new SslConnection(connector.getByteBufferPool(), connector.getExecutor(), endPoint, engine, isDirectBuffersForEncryption(), isDirectBuffersForDecryption()) + { + @Override + protected int networkFill(ByteBuffer input) throws IOException + { + int n = super.networkFill(input); + if (n > 0) + serverBytes.addAndGet(n); + return n; + } + }; + } + }; + connector = new ServerConnector(server, 1, 1, ssl, http); + server.addConnector(connector); + server.setHandler(new EmptyServerHandler()); + server.start(); + + long idleTimeout = 2000; + + SslContextFactory clientTLSFactory = createClientSslContextFactory(); + QueuedThreadPool clientThreads = new QueuedThreadPool(); + clientThreads.setName("client"); + AtomicLong clientBytes = new AtomicLong(); + client = new HttpClient(clientTLSFactory) + { + @Override + protected ClientConnectionFactory newSslClientConnectionFactory(SslContextFactory sslContextFactory, ClientConnectionFactory connectionFactory) + { + if (sslContextFactory == null) + sslContextFactory = getSslContextFactory(); + return new SslClientConnectionFactory(sslContextFactory, getByteBufferPool(), getExecutor(), connectionFactory) + { + @Override + protected SslConnection newSslConnection(ByteBufferPool byteBufferPool, Executor executor, EndPoint endPoint, SSLEngine engine) + { + return new SslConnection(byteBufferPool, executor, endPoint, engine, isDirectBuffersForEncryption(), isDirectBuffersForDecryption()) + { + @Override + protected int networkFill(ByteBuffer input) throws IOException + { + int n = super.networkFill(input); + if (n > 0) + clientBytes.addAndGet(n); + return n; + } + }; + } + }; + } + }; + client.setIdleTimeout(idleTimeout); + client.setExecutor(clientThreads); + client.start(); + + // Create a connection but don't use it. + String scheme = HttpScheme.HTTPS.asString(); + String host = "localhost"; + int port = connector.getLocalPort(); + HttpDestination destination = (HttpDestination)client.getDestination(scheme, host, port); + DuplexConnectionPool connectionPool = (DuplexConnectionPool)destination.getConnectionPool(); + // Trigger the creation of a new connection, but don't use it. + connectionPool.tryCreate(-1); + // Verify that the connection has been created. + while (true) + { + Thread.sleep(50); + if (connectionPool.getConnectionCount() == 1) + break; + } + + // Wait for the client to idle timeout the connection. + Thread.sleep(idleTimeout + idleTimeout / 2); + + // The connection should be gone from the connection pool. + assertEquals(0, connectionPool.getConnectionCount(), connectionPool.dump()); + assertEquals(0, serverBytes.get()); + assertEquals(0, clientBytes.get()); + } } diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java index 5dd6da588b1..8d76d742dfa 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java @@ -80,9 +80,10 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr private static final Logger LOG = Log.getLogger(SslConnection.class); private static final String TLS_1_3 = "TLSv1.3"; - private enum Handshake + private enum HandshakeState { INITIAL, + HANDSHAKE, SUCCEEDED, FAILED } @@ -116,7 +117,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr private boolean _requireCloseMessage; private FlushState _flushState = FlushState.IDLE; private FillState _fillState = FillState.IDLE; - private AtomicReference _handshake = new AtomicReference<>(Handshake.INITIAL); + private AtomicReference _handshake = new AtomicReference<>(HandshakeState.INITIAL); private boolean _underflown; private abstract class RunnableTask implements Runnable, Invocable @@ -291,6 +292,22 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr _requireCloseMessage = requireCloseMessage; } + private boolean isHandshakeInitial() + { + return _handshake.get() == HandshakeState.INITIAL; + } + + private boolean isHandshakeSucceeded() + { + return _handshake.get() == HandshakeState.SUCCEEDED; + } + + private boolean isHandshakeComplete() + { + HandshakeState state = _handshake.get(); + return state == HandshakeState.SUCCEEDED || state == HandshakeState.FAILED; + } + private void acquireEncryptedInput() { if (_encryptedInput == null) @@ -393,6 +410,16 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr } } + protected int networkFill(ByteBuffer input) throws IOException + { + return getEndPoint().fill(input); + } + + protected boolean networkFlush(ByteBuffer output) throws IOException + { + return getEndPoint().flush(output); + } + public class DecryptedEndPoint extends AbstractEndPoint { private final Callback _incompleteWriteCallback = new IncompleteWriteCallback(); @@ -590,14 +617,23 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr } // Let's try reading some encrypted data... even if we have some already. - int netFilled = getEndPoint().fill(_encryptedInput); - + int netFilled = networkFill(_encryptedInput); if (LOG.isDebugEnabled()) LOG.debug("net filled={}", netFilled); - if (netFilled > 0 && _handshake.get() == Handshake.INITIAL && isOutboundDone()) + // Workaround for Java 11 behavior. + if (netFilled < 0 && isHandshakeInitial() && BufferUtil.isEmpty(_encryptedInput)) + closeInbound(); + + if (netFilled > 0 && !isHandshakeComplete() && isOutboundDone()) throw new SSLHandshakeException("Closed during handshake"); + if (_handshake.compareAndSet(HandshakeState.INITIAL, HandshakeState.HANDSHAKE)) + { + if (LOG.isDebugEnabled()) + LOG.debug("fill starting handshake {}", SslConnection.this); + } + // Let's unwrap even if we have no net data because in that // case we want to fall through to the handshake handling int pos = BufferUtil.flipToFill(appIn); @@ -803,7 +839,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr private void handshakeSucceeded() throws SSLException { - if (_handshake.compareAndSet(Handshake.INITIAL, Handshake.SUCCEEDED)) + if (_handshake.compareAndSet(HandshakeState.HANDSHAKE, HandshakeState.SUCCEEDED)) { if (LOG.isDebugEnabled()) LOG.debug("handshake succeeded {} {} {}/{}", SslConnection.this, @@ -811,7 +847,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr _sslEngine.getSession().getProtocol(), _sslEngine.getSession().getCipherSuite()); notifyHandshakeSucceeded(_sslEngine); } - else if (_handshake.get() == Handshake.SUCCEEDED) + else if (isHandshakeSucceeded()) { if (_renegotiationLimit > 0) _renegotiationLimit--; @@ -820,7 +856,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr private void handshakeFailed(Throwable failure) { - if (_handshake.compareAndSet(Handshake.INITIAL, Handshake.FAILED)) + if (_handshake.compareAndSet(HandshakeState.HANDSHAKE, HandshakeState.FAILED)) { if (LOG.isDebugEnabled()) LOG.debug("handshake failed {} {}", SslConnection.this, failure); @@ -852,7 +888,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr } catch (SSLException x) { - if (handshakeStatus == HandshakeStatus.NOT_HANDSHAKING && !isAllowMissingCloseMessage()) + if (handshakeStatus == HandshakeStatus.NOT_HANDSHAKING && isRequireCloseMessage()) throw x; LOG.ignore(x); return x; @@ -882,7 +918,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr } // finish of any previous flushes - if (BufferUtil.hasContent(_encryptedOutput) && !getEndPoint().flush(_encryptedOutput)) + if (BufferUtil.hasContent(_encryptedOutput) && !networkFlush(_encryptedOutput)) return false; boolean isEmpty = BufferUtil.isEmpty(appOuts); @@ -910,6 +946,9 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr continue; case NEED_UNWRAP: + // Workaround for Java 11 behavior. + if (isHandshakeInitial() && isOutboundDone()) + break; if (_fillState == FillState.IDLE) { int filled = fill(BufferUtil.EMPTY_BUFFER); @@ -927,6 +966,12 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr if (_encryptedOutput == null) _encryptedOutput = _bufferPool.acquire(_sslEngine.getSession().getPacketBufferSize(), _encryptedDirectBuffers); + if (_handshake.compareAndSet(HandshakeState.INITIAL, HandshakeState.HANDSHAKE)) + { + if (LOG.isDebugEnabled()) + LOG.debug("flush starting handshake {}", SslConnection.this); + } + // We call sslEngine.wrap to try to take bytes from appOut buffers and encrypt them into the _netOut buffer BufferUtil.compact(_encryptedOutput); int pos = BufferUtil.flipToFill(_encryptedOutput); @@ -952,7 +997,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr // if we have net bytes, let's try to flush them boolean flushed = true; if (BufferUtil.hasContent(_encryptedOutput)) - flushed = getEndPoint().flush(_encryptedOutput); + flushed = networkFlush(_encryptedOutput); if (LOG.isDebugEnabled()) LOG.debug("net flushed={}, ac={}", flushed, isEmpty); @@ -1291,7 +1336,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr private boolean isRenegotiating() { - if (_handshake.get() == Handshake.INITIAL) + if (!isHandshakeComplete()) return false; if (isTLS13()) return false;