From 3ce87f717c0dc8a52764cb2c8bd517e1cba2f738 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 15 Oct 2019 16:44:30 +0200 Subject: [PATCH] Fixes #4201 - Throw SSLHandshakeException in case of TLS handshake failures. Now rethrowing other exceptions as SSLHandshakeException if they happen during the TLS handshake. Signed-off-by: Simone Bordet --- .../jetty/client/HttpClientTLSTest.java | 53 +++++++++++++++++++ .../eclipse/jetty/io/ssl/SslConnection.java | 24 ++++++--- 2 files changed, 70 insertions(+), 7 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..2279517f456 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 @@ -24,13 +24,18 @@ 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.AtomicReference; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLEngineResult; import javax.net.ssl.SSLException; +import javax.net.ssl.SSLHandshakeException; import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLSocket; @@ -38,8 +43,11 @@ 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.Handler; import org.eclipse.jetty.server.Server; @@ -48,6 +56,7 @@ import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.ExecutorThreadPool; import org.eclipse.jetty.util.thread.QueuedThreadPool; +import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.EnabledOnJre; @@ -557,4 +566,48 @@ public class HttpClientTLSTest assertTrue(latch.await(5, TimeUnit.SECONDS)); } + + @Test + public void testSSLEngineClosedDuringHandshake() throws Exception + { + SslContextFactory serverTLSFactory = createServerSslContextFactory(); + startServer(serverTLSFactory, new EmptyServerHandler()); + + SslContextFactory clientTLSFactory = createClientSslContextFactory(); + QueuedThreadPool clientThreads = new QueuedThreadPool(); + clientThreads.setName("client"); + 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 SSLEngineResult wrap(SSLEngine sslEngine, ByteBuffer[] input, ByteBuffer output) throws SSLException + { + sslEngine.closeOutbound(); + return super.wrap(sslEngine, input, output); + } + }; + } + }; + } + }; + client.setExecutor(clientThreads); + client.start(); + + ExecutionException failure = assertThrows(ExecutionException.class, () -> client.newRequest("localhost", connector.getLocalPort()) + .scheme(HttpScheme.HTTPS.asString()) + .send()); + Throwable cause = failure.getCause(); + assertThat(cause, Matchers.instanceOf(SSLHandshakeException.class)); + } } 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..3cac939413b 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 @@ -329,6 +329,16 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr _decryptedEndPoint.onFillableFail(cause == null ? new IOException() : cause); } + protected SSLEngineResult wrap(SSLEngine sslEngine, ByteBuffer[] input, ByteBuffer output) throws SSLException + { + return sslEngine.wrap(input, output); + } + + protected SSLEngineResult unwrap(SSLEngine sslEngine, ByteBuffer input, ByteBuffer output) throws SSLException + { + return sslEngine.unwrap(input, output); + } + @Override public String toConnectionString() { @@ -573,7 +583,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr try { _underflown = false; - unwrapResult = _sslEngine.unwrap(_encryptedInput, appIn); + unwrapResult = unwrap(_sslEngine, _encryptedInput, appIn); } finally { @@ -648,8 +658,8 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr } catch (Throwable x) { - Throwable failure = handleException(x, "fill"); - handshakeFailed(failure); + Throwable f = handleException(x, "fill"); + Throwable failure = handshakeFailed(f); if (_flushState == FlushState.WAIT_FOR_FILL) { _flushState = FlushState.IDLE; @@ -786,7 +796,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr } } - private void handshakeFailed(Throwable failure) + private Throwable handshakeFailed(Throwable failure) { if (_handshake.compareAndSet(Handshake.INITIAL, Handshake.FAILED)) { @@ -796,6 +806,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr failure = new SSLHandshakeException(failure.getMessage()).initCause(failure); notifyHandshakeFailed(_sslEngine, failure); } + return failure; } private void terminateInput() @@ -901,7 +912,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr SSLEngineResult wrapResult; try { - wrapResult = _sslEngine.wrap(appOuts, _encryptedOutput); + wrapResult = wrap(_sslEngine, appOuts, _encryptedOutput); } finally { @@ -980,8 +991,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr catch (Throwable x) { Throwable failure = handleException(x, "flush"); - handshakeFailed(failure); - throw failure; + throw handshakeFailed(failure); } finally {