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 <simone.bordet@gmail.com>
This commit is contained in:
Simone Bordet 2019-10-15 16:44:30 +02:00
parent 320e848c57
commit 3ce87f717c
2 changed files with 70 additions and 7 deletions

View File

@ -24,13 +24,18 @@ import java.io.OutputStream;
import java.net.ServerSocket; import java.net.ServerSocket;
import java.net.Socket; import java.net.Socket;
import java.net.SocketTimeoutException; import java.net.SocketTimeoutException;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets; import java.nio.charset.StandardCharsets;
import java.util.Arrays; import java.util.Arrays;
import java.util.concurrent.CountDownLatch; import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executor;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference; 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.SSLException;
import javax.net.ssl.SSLHandshakeException;
import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLPeerUnverifiedException;
import javax.net.ssl.SSLSocket; 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.HttpHeader;
import org.eclipse.jetty.http.HttpScheme; import org.eclipse.jetty.http.HttpScheme;
import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.io.ClientConnectionFactory; 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.SslClientConnectionFactory;
import org.eclipse.jetty.io.ssl.SslConnection;
import org.eclipse.jetty.io.ssl.SslHandshakeListener; import org.eclipse.jetty.io.ssl.SslHandshakeListener;
import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.Server; 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.ssl.SslContextFactory;
import org.eclipse.jetty.util.thread.ExecutorThreadPool; import org.eclipse.jetty.util.thread.ExecutorThreadPool;
import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.eclipse.jetty.util.thread.QueuedThreadPool;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.EnabledOnJre; import org.junit.jupiter.api.condition.EnabledOnJre;
@ -557,4 +566,48 @@ public class HttpClientTLSTest
assertTrue(latch.await(5, TimeUnit.SECONDS)); 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));
}
} }

View File

@ -329,6 +329,16 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr
_decryptedEndPoint.onFillableFail(cause == null ? new IOException() : cause); _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 @Override
public String toConnectionString() public String toConnectionString()
{ {
@ -573,7 +583,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr
try try
{ {
_underflown = false; _underflown = false;
unwrapResult = _sslEngine.unwrap(_encryptedInput, appIn); unwrapResult = unwrap(_sslEngine, _encryptedInput, appIn);
} }
finally finally
{ {
@ -648,8 +658,8 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr
} }
catch (Throwable x) catch (Throwable x)
{ {
Throwable failure = handleException(x, "fill"); Throwable f = handleException(x, "fill");
handshakeFailed(failure); Throwable failure = handshakeFailed(f);
if (_flushState == FlushState.WAIT_FOR_FILL) if (_flushState == FlushState.WAIT_FOR_FILL)
{ {
_flushState = FlushState.IDLE; _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)) 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); failure = new SSLHandshakeException(failure.getMessage()).initCause(failure);
notifyHandshakeFailed(_sslEngine, failure); notifyHandshakeFailed(_sslEngine, failure);
} }
return failure;
} }
private void terminateInput() private void terminateInput()
@ -901,7 +912,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr
SSLEngineResult wrapResult; SSLEngineResult wrapResult;
try try
{ {
wrapResult = _sslEngine.wrap(appOuts, _encryptedOutput); wrapResult = wrap(_sslEngine, appOuts, _encryptedOutput);
} }
finally finally
{ {
@ -980,8 +991,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr
catch (Throwable x) catch (Throwable x)
{ {
Throwable failure = handleException(x, "flush"); Throwable failure = handleException(x, "flush");
handshakeFailed(failure); throw handshakeFailed(failure);
throw failure;
} }
finally finally
{ {