From d36e5864dbd4e7044a601be3f4d9b62b3a9cc110 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 7 Mar 2016 15:20:12 +0100 Subject: [PATCH] Issue #377 (HttpClient - No supported cipher suites leads to stuck requests) Fixed by rethrowing the exception thrown by onOpen() so that the SelectorManager can act appropriately. --- .../client/HostnameVerificationTest.java | 3 +- .../jetty/client/HttpClientTLSTest.java | 183 ++++++++++++++++++ .../org/eclipse/jetty/io/SelectorManager.java | 23 +-- .../eclipse/jetty/io/ssl/SslConnection.java | 5 +- 4 files changed, 200 insertions(+), 14 deletions(-) create mode 100644 jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HostnameVerificationTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HostnameVerificationTest.java index 2b70e3b1dec..0fee488928c 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HostnameVerificationTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HostnameVerificationTest.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.nio.channels.ClosedChannelException; import java.security.cert.CertificateException; import java.util.concurrent.ExecutionException; + import javax.net.ssl.SSLHandshakeException; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; @@ -124,7 +125,7 @@ public class HostnameVerificationTest if (cause instanceof SSLHandshakeException) Assert.assertThat(cause.getCause().getCause(), Matchers.instanceOf(CertificateException.class)); else - Assert.assertThat(cause.getCause(), Matchers.instanceOf(ClosedChannelException.class)); + Assert.assertThat(cause, Matchers.instanceOf(ClosedChannelException.class)); } } 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 new file mode 100644 index 00000000000..43559aa2184 --- /dev/null +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java @@ -0,0 +1,183 @@ +// +// ======================================================================== +// Copyright (c) 1995-2016 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.client; + +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; + +import org.eclipse.jetty.http.HttpScheme; +import org.eclipse.jetty.server.Handler; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.util.ssl.SslContextFactory; +import org.eclipse.jetty.util.thread.QueuedThreadPool; +import org.junit.After; +import org.junit.Assert; +import org.junit.Test; + +public class HttpClientTLSTest +{ + private Server server; + private ServerConnector connector; + private HttpClient client; + + private void startServer(SslContextFactory sslContextFactory, Handler handler) throws Exception + { + QueuedThreadPool serverThreads = new QueuedThreadPool(); + serverThreads.setName("server"); + server = new Server(serverThreads); + + connector = new ServerConnector(server, sslContextFactory); + server.addConnector(connector); + + server.setHandler(handler); + server.start(); + } + + private void startClient(SslContextFactory sslContextFactory) throws Exception + { + QueuedThreadPool clientThreads = new QueuedThreadPool(); + clientThreads.setName("client"); + client = new HttpClient(sslContextFactory); + client.setExecutor(clientThreads); + client.start(); + } + + private SslContextFactory createSslContextFactory() + { + SslContextFactory sslContextFactory = new SslContextFactory(); + sslContextFactory.setEndpointIdentificationAlgorithm(""); + sslContextFactory.setKeyStorePath("src/test/resources/keystore.jks"); + sslContextFactory.setKeyStorePassword("storepwd"); + sslContextFactory.setTrustStorePath("src/test/resources/truststore.jks"); + sslContextFactory.setTrustStorePassword("storepwd"); + return sslContextFactory; + } + + @After + public void dispose() throws Exception + { + if (client != null) + client.stop(); + if (server != null) + server.stop(); + } + + @Test + public void testNoCommonTLSProtocol() throws Exception + { + SslContextFactory serverTLSFactory = createSslContextFactory(); + serverTLSFactory.setIncludeProtocols("TLSv1.2"); + startServer(serverTLSFactory, new EmptyServerHandler()); + + SslContextFactory clientTLSFactory = createSslContextFactory(); + clientTLSFactory.setIncludeProtocols("TLSv1.1"); + startClient(clientTLSFactory); + + try + { + client.newRequest("localhost", connector.getLocalPort()) + .scheme(HttpScheme.HTTPS.asString()) + .timeout(5, TimeUnit.SECONDS) + .send(); + Assert.fail(); + } + catch (ExecutionException x) + { + // Expected. + } + } + + @Test + public void testNoCommonTLSCiphers() throws Exception + { + SslContextFactory serverTLSFactory = createSslContextFactory(); + serverTLSFactory.setIncludeCipherSuites("TLS_RSA_WITH_AES_128_CBC_SHA"); + startServer(serverTLSFactory, new EmptyServerHandler()); + + SslContextFactory clientTLSFactory = createSslContextFactory(); + clientTLSFactory.setExcludeCipherSuites(".*_SHA$"); + startClient(clientTLSFactory); + + try + { + client.newRequest("localhost", connector.getLocalPort()) + .scheme(HttpScheme.HTTPS.asString()) + .timeout(5, TimeUnit.SECONDS) + .send(); + Assert.fail(); + } + catch (ExecutionException x) + { + // Expected. + } + } + + @Test + public void testMismatchBetweenTLSProtocolAndTLSCiphersOnServer() throws Exception + { + SslContextFactory serverTLSFactory = createSslContextFactory(); + // TLS 1.1 protocol, but only TLS 1.2 ciphers. + serverTLSFactory.setIncludeProtocols("TLSv1.1"); + serverTLSFactory.setIncludeCipherSuites("TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"); + startServer(serverTLSFactory, new EmptyServerHandler()); + + SslContextFactory clientTLSFactory = createSslContextFactory(); + startClient(clientTLSFactory); + + try + { + client.newRequest("localhost", connector.getLocalPort()) + .scheme(HttpScheme.HTTPS.asString()) + .timeout(5, TimeUnit.SECONDS) + .send(); + Assert.fail(); + } + catch (ExecutionException x) + { + // Expected. + } + } + + @Test + public void testMismatchBetweenTLSProtocolAndTLSCiphersOnClient() throws Exception + { + SslContextFactory serverTLSFactory = createSslContextFactory(); + startServer(serverTLSFactory, new EmptyServerHandler()); + + SslContextFactory clientTLSFactory = createSslContextFactory(); + // TLS 1.1 protocol, but only TLS 1.2 ciphers. + clientTLSFactory.setIncludeProtocols("TLSv1.1"); + clientTLSFactory.setIncludeCipherSuites("TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"); + startClient(clientTLSFactory); + + try + { + client.newRequest("localhost", connector.getLocalPort()) + .scheme(HttpScheme.HTTPS.asString()) + .timeout(5, TimeUnit.SECONDS) + .send(); + Assert.fail(); + } + catch (ExecutionException x) + { + // Expected. + } + } +} diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java index 47f188a3af3..26a18c3d3b3 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java @@ -62,14 +62,14 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa public static final int DEFAULT_CONNECT_TIMEOUT = 15000; protected static final Logger LOG = Log.getLogger(SelectorManager.class); private final static boolean __submitKeyUpdates = Boolean.valueOf(System.getProperty(SUBMIT_KEY_UPDATES, "true")); - + private final Executor executor; private final Scheduler scheduler; private final ManagedSelector[] _selectors; private long _connectTimeout = DEFAULT_CONNECT_TIMEOUT; private long _selectorIndex; private int _priorityDelta; - + protected SelectorManager(Executor executor, Scheduler scheduler) { this(executor, scheduler, (Runtime.getRuntime().availableProcessors() + 1) / 2); @@ -149,7 +149,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa } } } - + /** * Executes the given task in a different thread. * @@ -217,13 +217,13 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa final ManagedSelector selector = chooseSelector(); selector.submit(selector.new Accept(channel, attachment)); } - + /** *

Registers a server channel for accept operations. * When a {@link SocketChannel} is accepted from the given {@link ServerSocketChannel} * then the {@link #accepted(SocketChannel)} method is called, which must be * overridden by a derivation of this class to handle the accepted channel - * + * * @param server the server channel to register */ public void acceptor(ServerSocketChannel server) @@ -231,7 +231,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa final ManagedSelector selector = chooseSelector(); selector.submit(selector.new Acceptor(server)); } - + /** * Callback method when a channel is accepted from the {@link ServerSocketChannel} * passed to {@link #acceptor(ServerSocketChannel)}. @@ -315,6 +315,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa LOG.warn("Exception while notifying connection " + connection, x); else LOG.debug("Exception while notifying connection " + connection, x); + throw x; } } @@ -439,8 +440,8 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa /** * Submit a task to update a selector key. If the System property {@link SelectorManager#SUBMIT_KEY_UPDATES} - * is set true (default is false), the task is passed to {@link #submit(Runnable)}. Otherwise it is run immediately and the selector - * woken up if need be. + * is set true (default is false), the task is passed to {@link #submit(Runnable)}. Otherwise it is run immediately and the selector + * woken up if need be. * @param update the update to a key */ public void updateKey(Runnable update) @@ -460,7 +461,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa wakeup(); } } - + /** *

Submits a change to be executed in the selector thread.

*

Changes may be submitted from any thread, and the selector thread woken up @@ -587,7 +588,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa _state.set(State.CHANGES); continue; default: - throw new IllegalStateException(); + throw new IllegalStateException(); } } // Must check first for SELECT and *then* for WAKEUP @@ -696,7 +697,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa connect.failed(x); } } - + private void processAccept(SelectionKey key) { ServerSocketChannel server = (ServerSocketChannel)key.channel(); 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 dc20bf7bbac..c68cc06c48f 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 @@ -23,6 +23,7 @@ import java.nio.ByteBuffer; import java.nio.channels.ClosedChannelException; import java.util.Arrays; import java.util.concurrent.Executor; + import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLEngineResult; import javax.net.ssl.SSLEngineResult.HandshakeStatus; @@ -663,7 +664,7 @@ public class SslConnection extends AbstractConnection } catch (Exception e) { - getEndPoint().close(); + close(); throw e; } finally @@ -742,7 +743,7 @@ public class SslConnection extends AbstractConnection { BufferUtil.flipToFlush(_encryptedOutput, pos); } - + if (DEBUG) LOG.debug("{} wrap {}", SslConnection.this, wrapResult); if (wrapResult.bytesConsumed()>0)