From a861d5faefb4e5e9f02c535f1f42f2b2b316386c Mon Sep 17 00:00:00 2001 From: alexlehm Date: Sat, 3 Sep 2016 14:25:47 +0200 Subject: [PATCH] fixes #1602: NetClient with SOCKS Proxy checks wrong hostname on upgradeToSsl() pass the host to from NetClient connect to NetSocket and use actual address of host when using upgradeToSsl to check ssl cert add unit test for ssl connect and ssl upgrade with SOCKS proxy to NetTest remove a few NetServer close/reopen in a few tests where it was not needed Signed-off-by: alexlehm --- .../io/vertx/core/net/impl/NetClientImpl.java | 8 +- .../io/vertx/core/net/impl/NetSocketImpl.java | 14 ++- src/test/java/io/vertx/test/core/NetTest.java | 112 ++++++++++++------ 3 files changed, 94 insertions(+), 40 deletions(-) diff --git a/src/main/java/io/vertx/core/net/impl/NetClientImpl.java b/src/main/java/io/vertx/core/net/impl/NetClientImpl.java index 993253cf263..c01a945a768 100644 --- a/src/main/java/io/vertx/core/net/impl/NetClientImpl.java +++ b/src/main/java/io/vertx/core/net/impl/NetClientImpl.java @@ -200,13 +200,13 @@ private void connect(int port, String host, Handler> conn io.netty.util.concurrent.Future fut = sslHandler.handshakeFuture(); fut.addListener(future2 -> { if (future2.isSuccess()) { - connected(context, ch, connectHandler); + connected(context, ch, connectHandler, host, port); } else { failed(context, ch, future2.cause(), connectHandler); } }); } else { - connected(context, ch, connectHandler); + connected(context, ch, connectHandler, host, port); } } else { @@ -227,10 +227,12 @@ private void connect(int port, String host, Handler> conn channelProvider.connect(vertx, bootstrap, options.getProxyOptions(), host, port, channelInitializer, channelHandler); } - private void connected(ContextImpl context, Channel ch, Handler> connectHandler) { + private void connected(ContextImpl context, Channel ch, Handler> connectHandler, String host, int port) { // Need to set context before constructor is called as writehandler registration needs this ContextImpl.setContext(context); NetSocketImpl sock = new NetSocketImpl(vertx, ch, context, sslHelper, true, metrics, null); + // remember host and port in case upgradeToSsl needs it + sock.setHostPort(host, port); VertxNetHandler handler = ch.pipeline().get(VertxNetHandler.class); handler.conn = sock; socketMap.put(ch, sock); diff --git a/src/main/java/io/vertx/core/net/impl/NetSocketImpl.java b/src/main/java/io/vertx/core/net/impl/NetSocketImpl.java index 5373f8b6b05..3f6ab3e1eb3 100644 --- a/src/main/java/io/vertx/core/net/impl/NetSocketImpl.java +++ b/src/main/java/io/vertx/core/net/impl/NetSocketImpl.java @@ -64,6 +64,8 @@ public class NetSocketImpl extends ConnectionBase implements NetSocket { private final SSLHelper helper; private final boolean client; private Object metric; + private String host; + private int port; private Handler dataHandler; private Handler endHandler; private Handler drainHandler; @@ -81,6 +83,11 @@ public NetSocketImpl(VertxInternal vertx, Channel channel, ContextImpl context, registration = vertx.eventBus().localConsumer(writeHandlerID).handler(writeHandler); } + protected void setHostPort(String host, int port) { + this.host = host; + this.port = port; + } + protected synchronized void setMetric(Object metric) { this.metric = metric; } @@ -246,8 +253,11 @@ public synchronized void close() { public synchronized NetSocket upgradeToSsl(final Handler handler) { SslHandler sslHandler = channel.pipeline().get(SslHandler.class); if (sslHandler == null) { - - sslHandler = helper.createSslHandler(vertx, this.remoteName(), this.remoteAddress().port()); + if (host != null) { + sslHandler = helper.createSslHandler(vertx, host, port); + } else { + sslHandler = helper.createSslHandler(vertx, this.remoteName(), this.remoteAddress().port()); + } channel.pipeline().addFirst("ssl", sslHandler); } sslHandler.handshakeFuture().addListener(future -> context.executeFromIO(() -> { diff --git a/src/test/java/io/vertx/test/core/NetTest.java b/src/test/java/io/vertx/test/core/NetTest.java index 399b6bc8640..3cd58f55327 100644 --- a/src/test/java/io/vertx/test/core/NetTest.java +++ b/src/test/java/io/vertx/test/core/NetTest.java @@ -2299,11 +2299,6 @@ private TestLoggerFactory testLogging() throws Exception { */ @Test public void testWithSocks5Proxy() { - server.close(); - NetServerOptions options = new NetServerOptions().setHost("localhost").setPort(1234); - - NetServer server = vertx.createNetServer(options); - NetClientOptions clientOptions = new NetClientOptions() .setProxyOptions(new ProxyOptions().setType(ProxyType.SOCKS5).setPort(11080)); NetClient client = vertx.createNetClient(clientOptions); @@ -2333,11 +2328,6 @@ public void testWithSocks5Proxy() { */ @Test public void testWithSocks5ProxyAuth() { - server.close(); - NetServerOptions options = new NetServerOptions().setHost("localhost").setPort(1234); - - NetServer server = vertx.createNetServer(options); - NetClientOptions clientOptions = new NetClientOptions() .setProxyOptions(new ProxyOptions().setType(ProxyType.SOCKS5).setPort(11080) .setUsername("username").setPassword("username")); @@ -2350,12 +2340,7 @@ public void testWithSocks5ProxyAuth() { server.listen(ar -> { assertTrue(ar.succeeded()); client.connect(1234, "localhost", ar2 -> { - if (ar2.failed()) { - log.warn("failed", ar2.cause()); - } assertTrue(ar2.succeeded()); - // make sure we have gone through the proxy - assertEquals("localhost:1234", proxy.getLastUri()); testComplete(); }); }); @@ -2364,17 +2349,89 @@ public void testWithSocks5ProxyAuth() { } /** - * test http connect proxy for accessing a arbitrary server port - * note that this may not work with a "real" proxy since there are usually access rules defined - * that limit the target host and ports (e.g. connecting to localhost may not be allowed) + * test socks5 proxy when accessing ssl server port with correct cert. */ @Test - public void testWithHttpConnectProxy() { + public void testConnectSSLWithSocks5Proxy() { server.close(); - NetServerOptions options = new NetServerOptions().setHost("localhost").setPort(1234); + NetServerOptions options = new NetServerOptions() + .setPort(1234) + .setHost("localhost") + .setSsl(true) + .setKeyCertOptions(TLSCert.JKS_ROOT_CA.getServerKeyCertOptions()); + NetServer server = vertx.createNetServer(options); + NetClientOptions clientOptions = new NetClientOptions() + .setHostnameVerificationAlgorithm("HTTPS") + .setSsl(true) + .setProxyOptions(new ProxyOptions().setType(ProxyType.SOCKS5).setHost("127.0.0.1").setPort(11080)) + .setTrustOptions(TLSCert.JKS_ROOT_CA.getClientTrustOptions()); + NetClient client = vertx.createNetClient(clientOptions); + server.connectHandler(sock -> { + + }); + proxy = new SocksProxy(null); + proxy.start(vertx, v -> { + server.listen(ar -> { + assertTrue(ar.succeeded()); + client.connect(1234, "localhost", ar2 -> { + assertTrue(ar2.succeeded()); + testComplete(); + }); + }); + }); + await(); + } + + /** + * test socks5 proxy for accessing ssl server port with upgradeToSsl. + * https://github.com/eclipse/vert.x/issues/1602 + */ + @Test + public void testUpgradeSSLWithSocks5Proxy() { + server.close(); + NetServerOptions options = new NetServerOptions() + .setPort(1234) + .setHost("localhost") + .setSsl(true) + .setKeyCertOptions(TLSCert.JKS_ROOT_CA.getServerKeyCertOptions()); NetServer server = vertx.createNetServer(options); + NetClientOptions clientOptions = new NetClientOptions() + .setHostnameVerificationAlgorithm("HTTPS") + .setProxyOptions(new ProxyOptions().setType(ProxyType.SOCKS5).setHost("127.0.0.1").setPort(11080)) + .setTrustOptions(TLSCert.JKS_ROOT_CA.getClientTrustOptions()); + NetClient client = vertx.createNetClient(clientOptions); + server.connectHandler(sock -> { + + }); + proxy = new SocksProxy(null); + proxy.start(vertx, v -> { + server.listen(ar -> { + assertTrue(ar.succeeded()); + client.connect(1234, "localhost", ar2 -> { + assertTrue(ar2.succeeded()); + NetSocket ns = ar2.result(); + ns.exceptionHandler(th -> { + fail(th); + }); + ns.upgradeToSsl(v2 -> { + // failure would occur before upgradeToSsl completes + testComplete(); + }); + }); + }); + }); + await(); + } + + /** + * test http connect proxy for accessing a arbitrary server port + * note that this may not work with a "real" proxy since there are usually access rules defined + * that limit the target host and ports (e.g. connecting to localhost or to port 25 may not be allowed) + */ + @Test + public void testWithHttpConnectProxy() { NetClientOptions clientOptions = new NetClientOptions() .setProxyOptions(new ProxyOptions().setType(ProxyType.HTTP).setPort(13128)); NetClient client = vertx.createNetClient(clientOptions); @@ -2404,11 +2461,6 @@ public void testWithHttpConnectProxy() { */ @Test public void testWithSocks4aProxy() { - server.close(); - NetServerOptions options = new NetServerOptions().setHost("localhost").setPort(1234); - - NetServer server = vertx.createNetServer(options); - NetClientOptions clientOptions = new NetClientOptions() .setProxyOptions(new ProxyOptions().setType(ProxyType.SOCKS4).setPort(11080)); NetClient client = vertx.createNetClient(clientOptions); @@ -2438,11 +2490,6 @@ public void testWithSocks4aProxy() { */ @Test public void testWithSocks4aProxyAuth() { - server.close(); - NetServerOptions options = new NetServerOptions().setHost("localhost").setPort(1234); - - NetServer server = vertx.createNetServer(options); - NetClientOptions clientOptions = new NetClientOptions() .setProxyOptions(new ProxyOptions().setType(ProxyType.SOCKS4).setPort(11080) .setUsername("username")); @@ -2473,11 +2520,6 @@ public void testWithSocks4aProxyAuth() { */ @Test public void testWithSocks4LocalResolver() { - server.close(); - NetServerOptions options = new NetServerOptions().setHost("localhost").setPort(1234); - - NetServer server = vertx.createNetServer(options); - NetClientOptions clientOptions = new NetClientOptions() .setProxyOptions(new ProxyOptions().setType(ProxyType.SOCKS4).setPort(11080)); NetClient client = vertx.createNetClient(clientOptions);