eclipse-vertx / vert.x

Vert.x is a tool-kit for building reactive applications on the JVM
http://vertx.io
Other
14.25k stars 2.06k forks source link

Missing peer host and port info in SSLEngine for server SslHandler #5290

Open ben1222 opened 2 weeks ago

ben1222 commented 2 weeks ago

Version

4.4.9

Context

We have a customized key manager that extends X509ExtendedKeyManager that want to override the public String chooseEngineServerAlias(String keyType, Principal[] issuers, SSLEngine engine) method to choose the server alias partly depending on the peer host address. However, the engine.getPeerHost() always returns null.

After read related code, I find that netty SslContext.newHandler do support passing in an advisory peer information of peer host and port. However, in vert.x, when creating SslHandler for server in SslChannelProvider, the peer host and port info is not passed to SslContext.newHandler, result in null for engine.getPeerHost() in X509ExtendedKeyManager.chooseEngineServerAlias. (The SslChannelProvider do provide peer host and port info when creating client SslHandler)

I tried to pass the peer host and port info from HttpServerWorker to SslChannelProvider.createServerHandler and find the peer host and port are available in the SSLEngine in X509ExtendedKeyManager.chooseEngineServerAlias:

diff --git a/src/main/java/io/vertx/core/http/impl/HttpServerWorker.java b/src/main/java/io/vertx/core/http/impl/HttpServerWorker.java
index cf37c4e8b..82402c858 100644
--- a/src/main/java/io/vertx/core/http/impl/HttpServerWorker.java
+++ b/src/main/java/io/vertx/core/http/impl/HttpServerWorker.java
@@ -35,6 +35,8 @@ import io.vertx.core.impl.VertxInternal;
import io.vertx.core.net.impl.*;
import io.vertx.core.spi.metrics.HttpServerMetrics;

+import java.net.InetSocketAddress;
+import java.net.SocketAddress;
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.function.BiConsumer;
@@ -131,7 +133,12 @@ public class HttpServerWorker implements BiConsumer<Channel, SslChannelProvider>
   private void configurePipeline(Channel ch, SslChannelProvider sslChannelProvider) {
     ChannelPipeline pipeline = ch.pipeline();
     if (options.isSsl()) {
-      pipeline.addLast("ssl", sslChannelProvider.createServerHandler());
+      SocketAddress remoteAddress = ch.remoteAddress();
+      if (remoteAddress instanceof InetSocketAddress) {
+        pipeline.addLast("ssl", sslChannelProvider.createServerHandler(((InetSocketAddress) remoteAddress).getHostString(), ((InetSocketAddress) remoteAddress).getPort()));
+      } else {
+        pipeline.addLast("ssl", sslChannelProvider.createServerHandler());
+      }
       ChannelPromise p = ch.newPromise();
       pipeline.addLast("handshaker", new SslHandshakeCompletionHandler(p));
       p.addListener(future -> {
diff --git a/src/main/java/io/vertx/core/net/impl/SslChannelProvider.java b/src/main/java/io/vertx/core/net/impl/SslChannelProvider.java
index 290bf8c23..cb5aba5d1 100644
--- a/src/main/java/io/vertx/core/net/impl/SslChannelProvider.java
+++ b/src/main/java/io/vertx/core/net/impl/SslChannelProvider.java
@@ -144,17 +144,21 @@ public class SslChannelProvider {
   }

   public ChannelHandler createServerHandler() {
+    return createServerHandler(null, -1);
+  }
+
+  public ChannelHandler createServerHandler(String peerHost, int peerPort) {
     if (sni) {
       return createSniHandler();
     } else {
-      return createServerSslHandler(useAlpn);
+      return createServerSslHandler(useAlpn, peerHost, peerPort);
     }
   }

-  private SslHandler createServerSslHandler(boolean useAlpn) {
+  private SslHandler createServerSslHandler(boolean useAlpn, String peerHost, int peerPort) {
     SslContext sslContext = sslServerContext(useAlpn);
     Executor delegatedTaskExec = useWorkerPool ? workerPool : ImmediateExecutor.INSTANCE;
-    SslHandler sslHandler = sslContext.newHandler(ByteBufAllocator.DEFAULT, delegatedTaskExec);
+    SslHandler sslHandler = sslContext.newHandler(ByteBufAllocator.DEFAULT, peerHost, peerPort, delegatedTaskExec);
     sslHandler.setHandshakeTimeout(sslHandshakeTimeout, sslHandshakeTimeoutUnit);
     return sslHandler;
   }

There are a few other places calling SslChannelProvider.createServerHandler so although this change works in my use case, a more complete fix may be needed.

vietj commented 2 weeks ago

@ben1222 can you provide a reproducer for this using the vertx tests so we are covered, that would help

ben1222 commented 2 weeks ago

@vietj I tried to create a unit test under Http1xTLSTest:

public class Http1xTLSTest extends HttpTLSTest {
  private static final Logger LOG = LogManager.getLogger(Http1xTLSTest.class);

  @Test
  public void testTLSServerSSLEnginePeerHost() throws Exception {
    testTLS(Cert.NONE, Trust.SERVER_JKS, () -> {
      try {
        return KeyCertOptions.wrap(new MyKeyManager((X509KeyManager) Cert.SERVER_JKS.get().getKeyManagerFactory(vertx).getKeyManagers()[0]));
      } catch (Exception e) {
        throw new RuntimeException(e);
      }
    }, Trust.NONE).pass();
  }

  private static class MyKeyManager extends X509ExtendedKeyManager {
    private final X509KeyManager wrapped;

    MyKeyManager(X509KeyManager wrapped) {
      this.wrapped = wrapped;
    }

    @Override
    public String chooseEngineClientAlias(String[] keyType, Principal[] issuers, SSLEngine engine) {
      throw new UnsupportedOperationException();
    }

    @Override
    public String chooseEngineServerAlias(String keyType, Principal[] issuers, SSLEngine engine) {
      LOG.info("In chooseEngineServerAlias, keyType: {}, issuers: {}, peer host: {}, peer port: {}",
        keyType, issuers, engine.getPeerHost(), engine.getPeerPort());
      if (engine.getPeerHost() == null || engine.getPeerPort() == -1) {
        throw new RuntimeException("Missing peer host/port");
      }
      return wrapped.chooseServerAlias(keyType, issuers, null);
    }

    @Override
    public String chooseClientAlias(String[] keyType, Principal[] issuers, Socket socket) {
      throw new UnsupportedOperationException();
    }

    @Override
    public String chooseServerAlias(String keyType, Principal[] issuers, Socket socket) {
      throw new UnsupportedOperationException();
    }

    @Override
    public String[] getClientAliases(String s, Principal[] principals) {
      throw new UnsupportedOperationException();
    }

    @Override
    public String[] getServerAliases(String s, Principal[] principals) {
      return wrapped.getServerAliases(s, principals);
    }

    @Override
    public X509Certificate[] getCertificateChain(String s) {
      LOG.info("In getCertificateChain, s: {}", s);
      return wrapped.getCertificateChain(s);
    }

    @Override
    public PrivateKey getPrivateKey(String s) {
      LOG.info("In getPrivateKey, s: {}", s);
      return wrapped.getPrivateKey(s);
    }
  }

//...
}

Currently it will fail with:

Starting test: Http1xTLSTest#testTLSServerSSLEnginePeerHost 
2024-08-30 00:02:25,606 [vert.x-eventloop-thread-2] INFO io.vertx.core.http.Http1xTLSTest Http1xTLSTest$MyKeyManager.chooseEngineServerAlias:296 null - In chooseEngineServerAlias, keyType: EC, issuers: null, peer host: null, peer port: -1
java.lang.RuntimeException: Missing peer host/port
    at io.vertx.core.http.Http1xTLSTest$MyKeyManager.chooseEngineServerAlias(Http1xTLSTest.java:299)
    at java.base/sun.security.ssl.X509Authentication$X509PossessionGenerator.createServerPossession(X509Authentication.java:293)
    at java.base/sun.security.ssl.X509Authentication$X509PossessionGenerator.createPossession(X509Authentication.java:214)
    at java.base/sun.security.ssl.X509Authentication.createPossession(X509Authentication.java:90)
    at java.base/sun.security.ssl.CertificateMessage$T13CertificateProducer.choosePossession(CertificateMessage.java:1081)
    at java.base/sun.security.ssl.CertificateMessage$T13CertificateProducer.onProduceCertificate(CertificateMessage.java:970)
    at java.base/sun.security.ssl.CertificateMessage$T13CertificateProducer.produce(CertificateMessage.java:961)
    at java.base/sun.security.ssl.SSLHandshake.produce(SSLHandshake.java:436)
...
java.lang.AssertionError: Should not fail Failed to create SSL connection
    at org.junit.Assert.fail(Assert.java:89)
    at org.junit.Assert.assertTrue(Assert.java:42)
    at org.junit.Assert.assertFalse(Assert.java:65)
    at io.vertx.test.core.AsyncTestBase.assertFalse(AsyncTestBase.java:259)
    at io.vertx.core.http.HttpTLSTest.access$300(HttpTLSTest.java:68)
    at io.vertx.core.http.HttpTLSTest$TLSTest.lambda$run$10(HttpTLSTest.java:1312)
    at io.vertx.core.impl.future.FutureImpl$2.onFailure(FutureImpl.java:117)
...

With the changes in HttpServerWorker and SslChannelProvider, it succeeds:

Starting test: Http1xTLSTest#testTLSServerSSLEnginePeerHost 
2024-08-30 00:10:35,108 [vert.x-eventloop-thread-2] INFO io.vertx.core.http.Http1xTLSTest Http1xTLSTest$MyKeyManager.chooseEngineServerAlias:294 null - In chooseEngineServerAlias, keyType: EC, issuers: null, peer host: 127.0.0.1, peer port: 48470
2024-08-30 00:10:35,111 [vert.x-eventloop-thread-2] INFO io.vertx.core.http.Http1xTLSTest Http1xTLSTest$MyKeyManager.chooseEngineServerAlias:294 null - In chooseEngineServerAlias, keyType: EC, issuers: null, peer host: 127.0.0.1, peer port: 48470
2024-08-30 00:10:35,112 [vert.x-eventloop-thread-2] INFO io.vertx.core.http.Http1xTLSTest Http1xTLSTest$MyKeyManager.chooseEngineServerAlias:294 null - In chooseEngineServerAlias, keyType: EC, issuers: null, peer host: 127.0.0.1, peer port: 48470
2024-08-30 00:10:35,112 [vert.x-eventloop-thread-2] INFO io.vertx.core.http.Http1xTLSTest Http1xTLSTest$MyKeyManager.chooseEngineServerAlias:294 null - In chooseEngineServerAlias, keyType: RSA, issuers: null, peer host: 127.0.0.1, peer port: 48470
2024-08-30 00:10:35,112 [vert.x-eventloop-thread-2] INFO io.vertx.core.http.Http1xTLSTest Http1xTLSTest$MyKeyManager.getPrivateKey:330 null - In getPrivateKey, s: test-store
2024-08-30 00:10:35,113 [vert.x-eventloop-thread-2] INFO io.vertx.core.http.Http1xTLSTest Http1xTLSTest$MyKeyManager.getCertificateChain:324 null - In getCertificateChain, s: test-store
vietj commented 2 weeks ago

do you mind contributing a pull request to the 4.x branch and master branch ?

ben1222 commented 1 week ago

@vietj I can have a try. Do I need to go through some process before sending the pull request? I see the contributing guideline mentioned about signing ECA? Since there are a few other places (the NetServerImpl, NetSocketImpl) calling SslChannelProvider.createServerHandler, I'll try to also update them to pass the peer host info to SslChannelProvider.createServerHandler, is that ok?

vietj commented 1 week ago

you should sign the Eclipse Agreement indeed

everything should be updated and tested in master and 4.x branches