Skip to content

Commit

Permalink
Move the hostname verification to after the SSL handshake has complet…
Browse files Browse the repository at this point in the history 
…ed (Netty Only), back port #525
  • Loading branch information
Stephane Landelle committed Jul 10, 2014
1 parent dc138e2 commit 899fd7a
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 75 deletions.
56 changes: 34 additions & 22 deletions src/main/java/com/ning/http/client/providers/netty/NettyConnectListener.java
View file
Expand Up @@ -21,7 +21,7 @@
import com.ning.http.client.ProxyServer;
import com.ning.http.client.Request;
import com.ning.http.client.uri.UriComponents;
import com.ning.http.util.AllowAllHostnameVerifier;
import com.ning.http.util.Base64;
import com.ning.http.util.ProxyUtils;

import org.jboss.netty.buffer.ChannelBuffer;
Expand All @@ -34,22 +34,21 @@
import org.slf4j.LoggerFactory;

import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLSession;

import java.io.IOException;
import java.net.ConnectException;
import java.net.InetSocketAddress;
import java.nio.channels.ClosedChannelException;
import java.util.concurrent.atomic.AtomicBoolean;

/**
* Non Blocking connect.
*/
final class NettyConnectListener<T> implements ChannelFutureListener {
private final static Logger logger = LoggerFactory.getLogger(NettyConnectListener.class);
private static final Logger LOGGER = LoggerFactory.getLogger(NettyConnectListener.class);
private final AsyncHttpClientConfig config;
private final NettyResponseFuture<T> future;
private final HttpRequest nettyRequest;
private final AtomicBoolean handshakeDone = new AtomicBoolean(false);

private NettyConnectListener(AsyncHttpClientConfig config,
NettyResponseFuture<T> future) {
Expand All @@ -66,38 +65,51 @@ public final void operationComplete(ChannelFuture f) throws Exception {
if (f.isSuccess()) {
Channel channel = f.getChannel();
channel.getPipeline().getContext(NettyAsyncHttpProvider.class).setAttachment(future);
SslHandler sslHandler = (SslHandler) channel.getPipeline().get(NettyAsyncHttpProvider.SSL_HANDLER);
if (!handshakeDone.getAndSet(true) && (sslHandler != null)) {
((SslHandler) channel.getPipeline().get(NettyAsyncHttpProvider.SSL_HANDLER)).handshake().addListener(this);
return;
}

HostnameVerifier v = config.getHostnameVerifier();
if (sslHandler != null && !(v instanceof AllowAllHostnameVerifier)) {
// TODO: channel.getRemoteAddress()).getHostName() is very expensive. Should cache the result.
if (!v.verify(InetSocketAddress.class.cast(channel.getRemoteAddress()).getHostName(),
sslHandler.getEngine().getSession())) {
throw new ConnectException("HostnameVerifier exception.");
}
final SslHandler sslHandler = (SslHandler) channel.getPipeline().get(NettyAsyncHttpProvider.SSL_HANDLER);

final HostnameVerifier hostnameVerifier = config.getHostnameVerifier();
if (hostnameVerifier != null && sslHandler != null) {
final String host = future.getURI().getHost();
sslHandler.handshake().addListener(new ChannelFutureListener() {
@Override
public void operationComplete(ChannelFuture handshakeFuture) throws Exception {
if (handshakeFuture.isSuccess()) {
Channel channel = (Channel) handshakeFuture.getChannel();
SSLEngine engine = sslHandler.getEngine();
SSLSession session = engine.getSession();

LOGGER.debug("onFutureSuccess: session = {}, id = {}, isValid = {}, host = {}", session.toString(),
Base64.encode(session.getId()), session.isValid(), host);
if (!hostnameVerifier.verify(host, session)) {
ConnectException exception = new ConnectException("HostnameVerifier exception");
future.abort(exception);
throw exception;
} else {
future.provider().writeRequest(channel, config, future);
}
}
}
});
} else {
future.provider().writeRequest(f.getChannel(), config, future);
}

future.provider().writeRequest(f.getChannel(), config, future);
} else {
Throwable cause = f.getCause();

boolean canRetry = future.canRetry();
logger.debug("Trying to recover a dead cached channel {} with a retry value of {} ", f.getChannel(), canRetry);
LOGGER.debug("Trying to recover a dead cached channel {} with a retry value of {} ", f.getChannel(), canRetry);
if (canRetry && cause != null && (NettyAsyncHttpProvider.abortOnDisconnectException(cause)
|| cause instanceof ClosedChannelException
|| future.getState() != NettyResponseFuture.STATE.NEW)) {

logger.debug("Retrying {} ", nettyRequest);
LOGGER.debug("Retrying {} ", nettyRequest);
if (future.provider().remotelyClosed(f.getChannel(), future)) {
return;
}
}

logger.debug("Failed to recover from exception: {} with channel {}", cause, f.getChannel());
LOGGER.debug("Failed to recover from exception: {} with channel {}", cause, f.getChannel());

boolean printCause = f.getCause() != null && cause.getMessage() != null;
ConnectException e = new ConnectException(printCause ? cause.getMessage() + " to " + future.getURI().toString() : future.getURI().toString());
Expand Down
136 changes: 88 additions & 48 deletions src/test/java/com/ning/http/client/async/BasicHttpsTest.java
View file
Expand Up @@ -18,6 +18,7 @@
import com.ning.http.client.AsyncHttpClient;
import com.ning.http.client.AsyncHttpClientConfig.Builder;
import com.ning.http.client.Response;

import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.handler.AbstractHandler;
Expand All @@ -34,16 +35,19 @@
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLHandshakeException;
import javax.net.ssl.TrustManager;
import javax.net.ssl.TrustManagerFactory;
import javax.net.ssl.X509TrustManager;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.net.ConnectException;
import java.net.ServerSocket;
import java.net.URL;
import java.security.GeneralSecurityException;
import java.security.KeyStore;
import java.security.SecureRandom;
import java.security.cert.CertificateException;
Expand Down Expand Up @@ -207,7 +211,7 @@ public void setUpGlobal() throws Exception {

@Test(groups = { "standalone", "default_provider" })
public void zeroCopyPostTest() throws Throwable {
final AsyncHttpClient client = getAsyncHttpClient(new Builder().setSSLContext(createSSLContext()).build());
final AsyncHttpClient client = getAsyncHttpClient(new Builder().setSSLContext(createSSLContext(new AtomicBoolean(true))).build());
try {
ClassLoader cl = getClass().getClassLoader();
// override system properties
Expand All @@ -226,7 +230,7 @@ public void zeroCopyPostTest() throws Throwable {

@Test(groups = { "standalone", "default_provider" })
public void multipleSSLRequestsTest() throws Throwable {
final AsyncHttpClient c = getAsyncHttpClient(new Builder().setSSLContext(createSSLContext()).build());
final AsyncHttpClient c = getAsyncHttpClient(new Builder().setSSLContext(createSSLContext(new AtomicBoolean(true))).build());
try {
String body = "hello there";

Expand All @@ -246,7 +250,7 @@ public void multipleSSLRequestsTest() throws Throwable {

@Test(groups = { "standalone", "default_provider" })
public void multipleSSLWithoutCacheTest() throws Throwable {
final AsyncHttpClient c = getAsyncHttpClient(new Builder().setSSLContext(createSSLContext()).setAllowSslConnectionPool(false).build());
final AsyncHttpClient c = getAsyncHttpClient(new Builder().setSSLContext(createSSLContext(new AtomicBoolean(true))).setAllowSslConnectionPool(false).build());
try {
String body = "hello there";
c.preparePost(getTargetUrl()).setBody(body).setHeader("Content-Type", "text/html").execute();
Expand All @@ -262,55 +266,72 @@ public void multipleSSLWithoutCacheTest() throws Throwable {
}

@Test(groups = { "standalone", "default_provider" })
public void reconnectsAfterFailedCertificationPath() throws Throwable {
final AsyncHttpClient c = getAsyncHttpClient(new Builder().setSSLContext(createSSLContext()).build());
public void reconnectsAfterFailedCertificationPath() throws Exception {

AtomicBoolean trust = new AtomicBoolean(false);
AsyncHttpClient c = getAsyncHttpClient(new Builder().setSSLContext(createSSLContext(trust)).build());
try {
final String body = "hello there";
String body = "hello there";

TRUST_SERVER_CERT.set(false);
// first request fails because server certificate is rejected
Throwable cause = null;
try {
// first request fails because server certificate is rejected
try {
c.preparePost(getTargetUrl()).setBody(body).setHeader("Content-Type", "text/html").execute().get(TIMEOUT, TimeUnit.SECONDS);
} catch (final ExecutionException e) {
Throwable cause = e.getCause();
if (cause instanceof ConnectException) {
assertNotNull(cause.getCause());
assertTrue(cause.getCause() instanceof SSLHandshakeException);
} else {
assertTrue(cause instanceof SSLHandshakeException);
}
c.preparePost(getTargetUrl()).setBody(body).setHeader("Content-Type", "text/html").execute().get(TIMEOUT, TimeUnit.SECONDS);
} catch (final ExecutionException e) {
cause = e.getCause();
if (cause instanceof ConnectException) {
//assertNotNull(cause.getCause());
assertTrue(cause.getCause() instanceof SSLHandshakeException, "Expected an SSLHandshakeException, got a " + cause.getCause());
} else {
assertTrue(cause instanceof IOException, "Expected an IOException, got a " + cause);
}
} catch (Exception e) {
System.err.println("WTF"+ e.getMessage());
}
assertNotNull(cause);

TRUST_SERVER_CERT.set(true);

// second request should succeed
final Response response = c.preparePost(getTargetUrl()).setBody(body).setHeader("Content-Type", "text/html").execute().get(TIMEOUT, TimeUnit.SECONDS);
// second request should succeed
trust.set(true);
Response response = c.preparePost(getTargetUrl()).setBody(body).setHeader("Content-Type", "text/html").execute().get(TIMEOUT, TimeUnit.SECONDS);

assertEquals(response.getResponseBody(), body);
} finally {
TRUST_SERVER_CERT.set(true);
}
assertEquals(response.getResponseBody(), body);
} finally {
c.close();
}
}

private static SSLContext createSSLContext() {
private static KeyManager[] createKeyManagers() throws GeneralSecurityException, IOException {
InputStream keyStoreStream = Thread.currentThread().getContextClassLoader().getResourceAsStream("ssltest-cacerts.jks");
char[] keyStorePassword = "changeit".toCharArray();
KeyStore ks = KeyStore.getInstance("JKS");
ks.load(keyStoreStream, keyStorePassword);
assert(ks.size() > 0);

// Set up key manager factory to use our key store
char[] certificatePassword = "changeit".toCharArray();
KeyManagerFactory kmf = KeyManagerFactory.getInstance("SunX509");
kmf.init(ks, certificatePassword);

// Initialize the SSLContext to work with our key managers.
return kmf.getKeyManagers();
}

private static TrustManager[] createTrustManagers() throws GeneralSecurityException, IOException {
InputStream keyStoreStream = Thread.currentThread().getContextClassLoader().getResourceAsStream("ssltest-keystore.jks");
char[] keyStorePassword = "changeit".toCharArray();
KeyStore ks = KeyStore.getInstance("JKS");
ks.load(keyStoreStream, keyStorePassword);
assert(ks.size() > 0);

TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
tmf.init(ks);
return tmf.getTrustManagers();
}

public static SSLContext createSSLContext(AtomicBoolean trust) {
try {
InputStream keyStoreStream = BasicHttpsTest.class.getResourceAsStream("ssltest-cacerts.jks");
char[] keyStorePassword = "changeit".toCharArray();
KeyStore ks = KeyStore.getInstance("JKS");
ks.load(keyStoreStream, keyStorePassword);

// Set up key manager factory to use our key store
char[] certificatePassword = "changeit".toCharArray();
KeyManagerFactory kmf = KeyManagerFactory.getInstance("SunX509");
kmf.init(ks, certificatePassword);

// Initialize the SSLContext to work with our key managers.
KeyManager[] keyManagers = kmf.getKeyManagers();
TrustManager[] trustManagers = new TrustManager[] { DUMMY_TRUST_MANAGER };
KeyManager[] keyManagers = createKeyManagers();
TrustManager[] trustManagers = new TrustManager[] { dummyTrustManager(trust, (X509TrustManager) createTrustManagers()[0]) };
SecureRandom secureRandom = new SecureRandom();

SSLContext sslContext = SSLContext.getInstance("TLS");
Expand All @@ -322,20 +343,39 @@ private static SSLContext createSSLContext() {
}
}

private static final AtomicBoolean TRUST_SERVER_CERT = new AtomicBoolean(true);
private static final TrustManager DUMMY_TRUST_MANAGER = new X509TrustManager() {
public X509Certificate[] getAcceptedIssuers() {
return new X509Certificate[0];
public static class DummyTrustManager implements X509TrustManager {

private final X509TrustManager tm;
private final AtomicBoolean trust;

public DummyTrustManager(final AtomicBoolean trust, final X509TrustManager tm) {
this.trust = trust;
this.tm = tm;
}

public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException {
@Override
public void checkClientTrusted(X509Certificate[] chain, String authType)
throws CertificateException {
tm.checkClientTrusted(chain, authType);
}

public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException {
if (!TRUST_SERVER_CERT.get()) {
@Override
public void checkServerTrusted(X509Certificate[] chain, String authType)
throws CertificateException {
if (!trust.get()) {
throw new CertificateException("Server certificate not trusted.");
}
tm.checkServerTrusted(chain, authType);
}
};

@Override
public X509Certificate[] getAcceptedIssuers() {
return tm.getAcceptedIssuers();
}
}

private static TrustManager dummyTrustManager(final AtomicBoolean trust, final X509TrustManager tm) {
return new DummyTrustManager(trust, tm);

}
}
5 changes: 0 additions & 5 deletions src/test/java/com/ning/http/client/async/grizzly/GrizzlyBasicHttpsTest.java
View file
Expand Up @@ -24,9 +24,4 @@ public class GrizzlyBasicHttpsTest extends BasicHttpsTest {
public AsyncHttpClient getAsyncHttpClient(AsyncHttpClientConfig config) {
return ProviderUtil.grizzlyProvider(config);
}

@Override
public void zeroCopyPostTest() throws Throwable {
super.zeroCopyPostTest(); // To change body of overridden methods use File | Settings | File Templates.
}
}

0 comments on commit 899fd7a

Please sign in to comment.