Skip to content

Commit 8e4d787

Browse files
wsargentwsargent
committed
Move the hostname verification to after the SSL handshake has completed.
1 parent bbdc1b3 commit 8e4d787

File tree

5 files changed

+130
-69
lines changed

5 files changed

+130
-69
lines changed

‎api/src/test/java/org/asynchttpclient/async/BasicHttpsTest.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import javax.net.ssl.SSLHandshakeException;
3131
import javax.servlet.http.HttpServletResponse;
3232

33+
import java.io.IOException;
3334
import java.net.ConnectException;
3435
import java.util.concurrent.ExecutionException;
3536
import java.util.concurrent.TimeUnit;
@@ -100,17 +101,19 @@ public void reconnectsAfterFailedCertificationPath() throws Exception {
100101
String body = "hello there";
101102

102103
// first request fails because server certificate is rejected
104+
Throwable cause = null;
103105
try {
104106
c.preparePost(getTargetUrl()).setBody(body).setHeader("Content-Type", "text/html").execute().get(TIMEOUT, TimeUnit.SECONDS);
105107
} catch (final ExecutionException e) {
106-
Throwable cause = e.getCause();
108+
cause = e.getCause();
107109
if (cause instanceof ConnectException) {
108-
assertNotNull(cause.getCause());
110+
//assertNotNull(cause.getCause());
109111
assertTrue(cause.getCause() instanceof SSLHandshakeException, "Expected an SSLHandshakeException, got a " + cause.getCause());
110112
} else {
111-
assertTrue(cause instanceof SSLHandshakeException, "Expected an SSLHandshakeException, got a " + cause);
113+
assertTrue(cause instanceof IOException, "Expected an IOException, got a " + cause);
112114
}
113115
}
116+
assertNotNull(cause);
114117

115118
trusted.set(true);
116119

‎api/src/test/java/org/asynchttpclient/async/util/TestUtils.java

Lines changed: 63 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,7 @@
2222
import org.eclipse.jetty.util.security.Constraint;
2323
import org.eclipse.jetty.util.ssl.SslContextFactory;
2424

25-
import javax.net.ssl.KeyManager;
26-
import javax.net.ssl.KeyManagerFactory;
27-
import javax.net.ssl.SSLContext;
28-
import javax.net.ssl.TrustManager;
29-
import javax.net.ssl.X509TrustManager;
25+
import javax.net.ssl.*;
3026

3127
import java.io.File;
3228
import java.io.FileNotFoundException;
@@ -38,8 +34,7 @@
3834
import java.net.URISyntaxException;
3935
import java.net.URL;
4036
import java.nio.charset.Charset;
41-
import java.security.KeyStore;
42-
import java.security.SecureRandom;
37+
import java.security.*;
4338
import java.security.cert.CertificateException;
4439
import java.security.cert.X509Certificate;
4540
import java.util.ArrayList;
@@ -149,7 +144,6 @@ public static void addHttpsConnector(Server server, int port) throws URISyntaxEx
149144

150145
ServerConnector connector = new ServerConnector(server, new SslConnectionFactory(sslContextFactory, "http/1.1"), new HttpConnectionFactory(httpsConfig));
151146
connector.setPort(port);
152-
server.addConnector(connector);
153147

154148
server.addConnector(connector);
155149
}
@@ -191,21 +185,38 @@ private static void addAuthHandler(Server server, String auth, LoginAuthenticato
191185
server.setHandler(security);
192186
}
193187

188+
private static KeyManager[] createKeyManagers() throws GeneralSecurityException, IOException {
189+
InputStream keyStoreStream = Thread.currentThread().getContextClassLoader().getResourceAsStream("ssltest-cacerts.jks");
190+
char[] keyStorePassword = "changeit".toCharArray();
191+
KeyStore ks = KeyStore.getInstance("JKS");
192+
ks.load(keyStoreStream, keyStorePassword);
193+
assert(ks.size() > 0);
194+
195+
// Set up key manager factory to use our key store
196+
char[] certificatePassword = "changeit".toCharArray();
197+
KeyManagerFactory kmf = KeyManagerFactory.getInstance("SunX509");
198+
kmf.init(ks, certificatePassword);
199+
200+
// Initialize the SSLContext to work with our key managers.
201+
return kmf.getKeyManagers();
202+
}
203+
204+
private static TrustManager[] createTrustManagers() throws GeneralSecurityException, IOException {
205+
InputStream keyStoreStream = Thread.currentThread().getContextClassLoader().getResourceAsStream("ssltest-keystore.jks");
206+
char[] keyStorePassword = "changeit".toCharArray();
207+
KeyStore ks = KeyStore.getInstance("JKS");
208+
ks.load(keyStoreStream, keyStorePassword);
209+
assert(ks.size() > 0);
210+
211+
TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
212+
tmf.init(ks);
213+
return tmf.getTrustManagers();
214+
}
215+
194216
public static SSLContext createSSLContext(AtomicBoolean trust) {
195217
try {
196-
InputStream keyStoreStream = HostnameVerifierTest.class.getResourceAsStream("ssltest-cacerts.jks");
197-
char[] keyStorePassword = "changeit".toCharArray();
198-
KeyStore ks = KeyStore.getInstance("JKS");
199-
ks.load(keyStoreStream, keyStorePassword);
200-
201-
// Set up key manager factory to use our key store
202-
char[] certificatePassword = "changeit".toCharArray();
203-
KeyManagerFactory kmf = KeyManagerFactory.getInstance("SunX509");
204-
kmf.init(ks, certificatePassword);
205-
206-
// Initialize the SSLContext to work with our key managers.
207-
KeyManager[] keyManagers = kmf.getKeyManagers();
208-
TrustManager[] trustManagers = new TrustManager[] { dummyTrustManager(trust) };
218+
KeyManager[] keyManagers = createKeyManagers();
219+
TrustManager[] trustManagers = new TrustManager[] { dummyTrustManager(trust, (X509TrustManager) createTrustManagers()[0]) };
209220
SecureRandom secureRandom = new SecureRandom();
210221

211222
SSLContext sslContext = SSLContext.getInstance("TLS");
@@ -217,21 +228,40 @@ public static SSLContext createSSLContext(AtomicBoolean trust) {
217228
}
218229
}
219230

220-
private static final TrustManager dummyTrustManager(final AtomicBoolean trust) {
221-
return new X509TrustManager() {
222-
public X509Certificate[] getAcceptedIssuers() {
223-
return new X509Certificate[0];
224-
}
231+
public static class DummyTrustManager implements X509TrustManager {
225232

226-
public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException {
227-
}
233+
private final X509TrustManager tm;
234+
private final AtomicBoolean trust;
228235

229-
public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException {
230-
if (!trust.get()) {
231-
throw new CertificateException("Server certificate not trusted.");
232-
}
236+
public DummyTrustManager(final AtomicBoolean trust, final X509TrustManager tm) {
237+
this.trust = trust;
238+
this.tm = tm;
239+
}
240+
241+
@Override
242+
public void checkClientTrusted(X509Certificate[] chain, String authType)
243+
throws CertificateException {
244+
tm.checkClientTrusted(chain, authType);
245+
}
246+
247+
@Override
248+
public void checkServerTrusted(X509Certificate[] chain, String authType)
249+
throws CertificateException {
250+
if (!trust.get()) {
251+
throw new CertificateException("Server certificate not trusted.");
233252
}
234-
};
253+
tm.checkServerTrusted(chain, authType);
254+
}
255+
256+
@Override
257+
public X509Certificate[] getAcceptedIssuers() {
258+
return tm.getAcceptedIssuers();
259+
}
260+
}
261+
262+
private static TrustManager dummyTrustManager(final AtomicBoolean trust, final X509TrustManager tm) {
263+
return new DummyTrustManager(trust, tm);
264+
235265
}
236266

237267
public static File getClasspathFile(String file) throws FileNotFoundException {

‎providers/grizzly/src/main/java/org/asynchttpclient/providers/grizzly/ConnectionManager.java

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,22 @@
2020
import org.asynchttpclient.ConnectionPoolKeyStrategy;
2121
import org.asynchttpclient.ProxyServer;
2222
import org.asynchttpclient.Request;
23+
import org.asynchttpclient.util.Base64;
2324
import org.glassfish.grizzly.CompletionHandler;
2425
import org.glassfish.grizzly.Connection;
25-
import org.glassfish.grizzly.EmptyCompletionHandler;
2626
import org.glassfish.grizzly.Grizzly;
2727
import org.glassfish.grizzly.GrizzlyFuture;
2828
import org.glassfish.grizzly.attributes.Attribute;
2929
import org.glassfish.grizzly.connectionpool.EndpointKey;
3030
import org.glassfish.grizzly.filterchain.FilterChainBuilder;
3131
import org.glassfish.grizzly.impl.FutureImpl;
32+
import org.glassfish.grizzly.ssl.SSLBaseFilter;
33+
import org.glassfish.grizzly.ssl.SSLFilter;
3234
import org.glassfish.grizzly.ssl.SSLUtils;
3335
import org.glassfish.grizzly.utils.Futures;
3436
import org.glassfish.grizzly.utils.IdleTimeoutFilter;
37+
import org.slf4j.Logger;
38+
import org.slf4j.LoggerFactory;
3539

3640
import javax.net.ssl.HostnameVerifier;
3741
import javax.net.ssl.SSLSession;
@@ -51,6 +55,8 @@
5155

5256
public class ConnectionManager {
5357

58+
private final static Logger LOGGER = LoggerFactory.getLogger(ConnectionManager.class);
59+
5460
private static final Attribute<Boolean> DO_NOT_CACHE = Grizzly.DEFAULT_ATTRIBUTE_BUILDER.createAttribute(ConnectionManager.class
5561
.getName());
5662
private final ConnectionPool connectionPool;
@@ -60,13 +66,15 @@ public class ConnectionManager {
6066
private final FilterChainBuilder secureBuilder;
6167
private final FilterChainBuilder nonSecureBuilder;
6268
private final boolean asyncConnect;
69+
private final SSLFilter sslFilter;
6370

6471
// ------------------------------------------------------------ Constructors
6572

6673
ConnectionManager(final GrizzlyAsyncHttpProvider provider,//
6774
final ConnectionPool connectionPool,//
6875
final FilterChainBuilder secureBuilder,//
69-
final FilterChainBuilder nonSecureBuilder) {
76+
final FilterChainBuilder nonSecureBuilder,//
77+
final SSLFilter sslFilter) {
7078

7179
this.provider = provider;
7280
final AsyncHttpClientConfig config = provider.getClientConfig();
@@ -87,6 +95,7 @@ public class ConnectionManager {
8795
AsyncHttpProviderConfig<?, ?> providerConfig = config.getAsyncHttpProviderConfig();
8896
asyncConnect = providerConfig instanceof GrizzlyAsyncHttpProviderConfig ? GrizzlyAsyncHttpProviderConfig.class.cast(providerConfig)
8997
.isAsyncConnectMode() : false;
98+
this.sslFilter = sslFilter;
9099
}
91100

92101
// ---------------------------------------------------------- Public Methods
@@ -95,7 +104,7 @@ public void doTrackedConnection(final Request request,//
95104
final GrizzlyResponseFuture requestFuture,//
96105
final CompletionHandler<Connection> connectHandler) throws IOException {
97106
final EndpointKey<SocketAddress> key = getEndPointKey(request, requestFuture.getProxyServer());
98-
CompletionHandler<Connection> handler = wrapHandler(request, getVerifier(), connectHandler);
107+
CompletionHandler<Connection> handler = wrapHandler(request, getVerifier(), connectHandler, sslFilter);
99108
if (asyncConnect) {
100109
connectionPool.take(key, handler);
101110
} else {
@@ -136,37 +145,32 @@ public Connection obtainConnection(final Request request, final GrizzlyResponseF
136145
// --------------------------------------------------Package Private Methods
137146

138147
static CompletionHandler<Connection> wrapHandler(final Request request, final HostnameVerifier verifier,
139-
final CompletionHandler<Connection> delegate) {
148+
final CompletionHandler<Connection> delegate, final SSLFilter sslFilter) {
140149
final URI uri = request.getURI();
141150
if (Utils.isSecure(uri) && verifier != null) {
142-
return new EmptyCompletionHandler<Connection>() {
151+
SSLBaseFilter.HandshakeListener handshakeListener = new SSLBaseFilter.HandshakeListener() {
143152
@Override
144-
public void completed(Connection result) {
145-
final String host = uri.getHost();
146-
final SSLSession session = SSLUtils.getSSLEngine(result).getSession();
147-
if (!verifier.verify(host, session)) {
148-
failed(new ConnectException("Host name verification failed for host " + host));
149-
} else {
150-
delegate.completed(result);
151-
}
152-
153+
public void onStart(Connection connection) {
154+
// do nothing
155+
LOGGER.debug("SSL Handshake onStart: ");
153156
}
154157

155158
@Override
156-
public void cancelled() {
157-
delegate.cancelled();
158-
}
159+
public void onComplete(Connection connection) {
160+
sslFilter.removeHandshakeListener(this);
159161

160-
@Override
161-
public void failed(Throwable throwable) {
162-
delegate.failed(throwable);
163-
}
162+
final String host = uri.getHost();
163+
final SSLSession session = SSLUtils.getSSLEngine(connection).getSession();
164+
LOGGER.debug("SSL Handshake onComplete: session = {}, id = {}, isValid = {}, host = {}", session.toString(), Base64.encode(session.getId()), session.isValid(), host);
164165

165-
@Override
166-
public void updated(Connection result) {
167-
delegate.updated(result);
166+
if (!verifier.verify(host, session)) {
167+
connection.close(); // XXX what's the correct way to kill a connection?
168+
IOException e = new ConnectException("Host name verification failed for host " + host);
169+
delegate.failed(e);
170+
}
168171
}
169172
};
173+
sslFilter.addHandshakeListener(handshakeListener);
170174
}
171175
return delegate;
172176
}

‎providers/grizzly/src/main/java/org/asynchttpclient/providers/grizzly/GrizzlyAsyncHttpProvider.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ public void onTimeout(Connection connection) {
334334
} else {
335335
pool = null;
336336
}
337-
connectionManager = new ConnectionManager(this, pool, secure, nonSecure);
337+
connectionManager = new ConnectionManager(this, pool, secure, nonSecure, filter);
338338

339339
}
340340

‎providers/netty/src/main/java/org/asynchttpclient/providers/netty/request/NettyConnectListener.java

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,13 @@
1616
*/
1717
package org.asynchttpclient.providers.netty.request;
1818

19+
import io.netty.util.concurrent.Future;
20+
import io.netty.util.concurrent.GenericFutureListener;
1921
import org.asynchttpclient.AsyncHttpClientConfig;
2022
import org.asynchttpclient.providers.netty.channel.Channels;
2123
import org.asynchttpclient.providers.netty.future.NettyResponseFuture;
2224
import org.asynchttpclient.providers.netty.future.StackTraceInspector;
25+
import org.asynchttpclient.util.Base64;
2326
import org.slf4j.Logger;
2427
import org.slf4j.LoggerFactory;
2528

@@ -28,6 +31,10 @@
2831
import io.netty.channel.ChannelFutureListener;
2932
import io.netty.handler.ssl.SslHandler;
3033

34+
import javax.net.ssl.HostnameVerifier;
35+
import javax.net.ssl.SSLEngine;
36+
import javax.net.ssl.SSLEngineResult;
37+
import javax.net.ssl.SSLSession;
3138
import java.net.ConnectException;
3239
import java.nio.channels.ClosedChannelException;
3340

@@ -54,15 +61,32 @@ public NettyResponseFuture<T> future() {
5461

5562
public void onFutureSuccess(final Channel channel) throws ConnectException {
5663
Channels.setDefaultAttribute(channel, future);
57-
SslHandler sslHandler = Channels.getSslHandler(channel);
58-
59-
if (sslHandler != null && !config.getHostnameVerifier().verify(future.getURI().getHost(), sslHandler.engine().getSession())) {
60-
ConnectException exception = new ConnectException("HostnameVerifier exception");
61-
future.abort(exception);
62-
throw exception;
64+
final HostnameVerifier hostnameVerifier = config.getHostnameVerifier();
65+
final SslHandler sslHandler = Channels.getSslHandler(channel);
66+
if (hostnameVerifier != null && sslHandler != null) {
67+
final String host = future.getURI().getHost();
68+
sslHandler.handshakeFuture().addListener(new GenericFutureListener<Future<? super Channel>>() {
69+
@Override
70+
public void operationComplete(Future<? super Channel> handshakeFuture) throws Exception {
71+
if (handshakeFuture.isSuccess()) {
72+
Channel channel = (Channel) handshakeFuture.getNow();
73+
SSLEngine engine = sslHandler.engine();
74+
SSLSession session = engine.getSession();
75+
76+
LOGGER.debug("onFutureSuccess: session = {}, id = {}, isValid = {}, host = {}", session.toString(), Base64.encode(session.getId()), session.isValid(), host);
77+
if (!hostnameVerifier.verify(host, session)) {
78+
ConnectException exception = new ConnectException("HostnameVerifier exception");
79+
future.abort(exception);
80+
throw exception;
81+
} else {
82+
requestSender.writeRequest(future, channel);
83+
}
84+
}
85+
}
86+
});
87+
} else {
88+
requestSender.writeRequest(future, channel);
6389
}
64-
65-
requestSender.writeRequest(future, channel);
6690
}
6791

6892
public void onFutureFailure(Channel channel, Throwable cause) {

0 commit comments

Comments
 (0)