Repository: eclipse-vertx/vert.x

Problem statement:
NetClient with SOCKS Proxy checks wrong hostname on upgradeToSsl()
When using a NetClient with SOCKS Proxy, the upgradeToSsl() checks the ssl cert against the name of the proxy, not the name of the target server.

I created a unit test that shows the issue by setting the proxy hostname to 127.0.0.1 and connecting to a server at localhost, this way it fails with "No subject alternative names matching IP address 127.0.0.1 found".

A unlikely usecase, however I got the error when trying to send a mail via smtp with starttls over TOR, which uses SOCKS5 to connect.



Hints:

Patch:
diff --git a/src/main/java/io/vertx/core/net/impl/NetClientImpl.java b/src/main/java/io/vertx/core/net/impl/NetClientImpl.java
--- 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<AsyncResult<NetSocket>> conn
           io.netty.util.concurrent.Future<Channel> 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<AsyncResult<NetSocket>> conn
     channelProvider.connect(vertx, bootstrap, options.getProxyOptions(), host, port, channelInitializer, channelHandler);
   }
 
-  private void connected(ContextImpl context, Channel ch, Handler<AsyncResult<NetSocket>> connectHandler) {
+  private void connected(ContextImpl context, Channel ch, Handler<AsyncResult<NetSocket>> 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
--- 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<Buffer> dataHandler;
   private Handler<Void> endHandler;
   private Handler<Void> drainHandler;
@@ -81,6 +83,11 @@ public NetSocketImpl(VertxInternal vertx, Channel channel, ContextImpl context,
     registration = vertx.eventBus().<Buffer>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<Void> 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(() -> {


Test patch:
diff --git a/src/test/java/io/vertx/test/core/NetTest.java b/src/test/java/io/vertx/test/core/NetTest.java
--- 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);