diff --git a/src/main/java/com/ning/http/client/providers/netty/NettyConnectListener.java b/src/main/java/com/ning/http/client/providers/netty/NettyConnectListener.java index de6ed82eb4..ca05843964 100644 --- a/src/main/java/com/ning/http/client/providers/netty/NettyConnectListener.java +++ b/src/main/java/com/ning/http/client/providers/netty/NettyConnectListener.java @@ -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; @@ -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 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 future; private final HttpRequest nettyRequest; - private final AtomicBoolean handshakeDone = new AtomicBoolean(false); private NettyConnectListener(AsyncHttpClientConfig config, NettyResponseFuture future) { @@ -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()); diff --git a/src/test/java/com/ning/http/client/async/BasicHttpsTest.java b/src/test/java/com/ning/http/client/async/BasicHttpsTest.java index 2d2c180fd2..24ef8052a5 100644 --- a/src/test/java/com/ning/http/client/async/BasicHttpsTest.java +++ b/src/test/java/com/ning/http/client/async/BasicHttpsTest.java @@ -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; @@ -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; @@ -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 @@ -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"; @@ -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(); @@ -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"); @@ -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); + + } } diff --git a/src/test/java/com/ning/http/client/async/grizzly/GrizzlyBasicHttpsTest.java b/src/test/java/com/ning/http/client/async/grizzly/GrizzlyBasicHttpsTest.java index 2ebeedc182..19d2397199 100644 --- a/src/test/java/com/ning/http/client/async/grizzly/GrizzlyBasicHttpsTest.java +++ b/src/test/java/com/ning/http/client/async/grizzly/GrizzlyBasicHttpsTest.java @@ -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. - } }