Add option to send SNI header in TLS ClientHello message

* add --Xp2p-tls-clienthello-sni option to enable the SNI header Issue hyperledger#4894 Signed-off-by: Sebastian Bathke <sebastian.bathke@gmail.com>
megglos · May 5, 2023 · c30a2e3 · c30a2e3
1 parent 4f5dcaa
commit c30a2e3
Show file tree

Hide file tree

Showing 6 changed files with 110 additions and 28 deletions.
diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/P2PTLSConfigOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/P2PTLSConfigOptions.java
@@ -96,6 +96,13 @@ public class P2PTLSConfigOptions {
       description = "Certificate revocation list for the P2P service.")
   private final Path p2pCrlFile = null;
 
+  @Option(
+      names = {"--Xp2p-tls-clienthello-sni"},
+      hidden = true,
+      description =
+          "Whether to send a SNI header in the TLS ClientHello message (default: ${DEFAULT-VALUE})")
+  private final Boolean p2pTlsClientHelloSniHeaderEnabled = false;
+
   /**
    * Generate P2p tls configuration.
    *
@@ -132,6 +139,7 @@ public Optional<TLSConfiguration> p2pTLSConfiguration(final CommandLine commandL
                     : new FileBasedPasswordProvider(p2pTLSTrustStorePasswordFile))
             .withTrustStorePasswordPath(p2pTLSTrustStorePasswordFile)
             .withCrlPath(p2pCrlFile)
+            .withClientHelloSniEnabled(p2pTlsClientHelloSniHeaderEnabled)
             .build());
   }
 

diff --git a/besu/src/test/resources/everything_config.toml b/besu/src/test/resources/everything_config.toml
@@ -213,6 +213,7 @@ Xp2p-tls-truststore-type="none"
 Xp2p-tls-truststore-file="none.file"
 Xp2p-tls-keystore-password-file="none"
 Xp2p-tls-crl-file="none.file"
+Xp2p-tls-clienthello-sni=false
 
 #contracts
 Xevm-jumpdest-cache-weight-kb=32000
diff --git a/.../org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java b/.../org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java
@@ -207,7 +207,7 @@ protected void initChannel(final SocketChannel ch) throws Exception {
                 timeoutHandler(
                     connectionFuture,
                     "Timed out waiting to establish connection with peer: " + peer.getId()));
-        addAdditionalOutboundHandlers(ch);
+        addAdditionalOutboundHandlers(ch, peer);
         ch.pipeline().addLast(outboundHandler(peer, connectionFuture));
       }
     };
@@ -271,7 +271,7 @@ private TimeoutHandler<Channel> timeoutHandler(
         () -> connectionFuture.completeExceptionally(new TimeoutException(s)));
   }
 
-  void addAdditionalOutboundHandlers(final Channel ch)
+  void addAdditionalOutboundHandlers(final Channel ch, final Peer peer)
       throws GeneralSecurityException, IOException {}
 
   void addAdditionalInboundHandlers(final Channel ch)

diff --git a/...g/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyTLSConnectionInitializer.java b/...g/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyTLSConnectionInitializer.java
@@ -20,12 +20,14 @@
 import org.hyperledger.besu.cryptoservices.NodeKey;
 import org.hyperledger.besu.ethereum.p2p.config.RlpxConfiguration;
 import org.hyperledger.besu.ethereum.p2p.peers.LocalNode;
+import org.hyperledger.besu.ethereum.p2p.peers.Peer;
 import org.hyperledger.besu.ethereum.p2p.plain.PlainFramer;
 import org.hyperledger.besu.ethereum.p2p.plain.PlainHandshaker;
 import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnectionEventDispatcher;
 import org.hyperledger.besu.ethereum.p2p.rlpx.framing.Framer;
 import org.hyperledger.besu.ethereum.p2p.rlpx.handshake.HandshakeSecrets;
 import org.hyperledger.besu.ethereum.p2p.rlpx.handshake.Handshaker;
+import org.hyperledger.besu.plugin.data.EnodeURL;
 import org.hyperledger.besu.plugin.services.MetricsSystem;
 
 import java.security.GeneralSecurityException;
@@ -40,10 +42,12 @@
 import io.netty.handler.codec.compression.SnappyFrameDecoder;
 import io.netty.handler.codec.compression.SnappyFrameEncoder;
 import io.netty.handler.ssl.SslContext;
+import io.netty.handler.ssl.SslHandler;
 
 public class NettyTLSConnectionInitializer extends NettyConnectionInitializer {
 
   private final Optional<Supplier<TLSContextFactory>> tlsContextFactorySupplier;
+  private final Boolean clientHelloSniHeaderEnabled;
 
   public NettyTLSConnectionInitializer(
       final NodeKey nodeKey,
@@ -58,7 +62,9 @@ public NettyTLSConnectionInitializer(
         localNode,
         eventDispatcher,
         metricsSystem,
-        defaultTlsContextFactorySupplier(p2pTLSConfiguration));
+        defaultTlsContextFactorySupplier(p2pTLSConfiguration),
+        p2pTLSConfiguration.getClientHelloSniHeaderEnabled()
+    );
   }
 
   @VisibleForTesting
@@ -68,22 +74,37 @@ public NettyTLSConnectionInitializer(
       final LocalNode localNode,
       final PeerConnectionEventDispatcher eventDispatcher,
       final MetricsSystem metricsSystem,
-      final Supplier<TLSContextFactory> tlsContextFactorySupplier) {
+      final Supplier<TLSContextFactory> tlsContextFactorySupplier,
+      final Boolean clientHelloSniHeaderEnabled) {
     super(nodeKey, config, localNode, eventDispatcher, metricsSystem);
     if (tlsContextFactorySupplier != null) {
       this.tlsContextFactorySupplier =
           Optional.of(Suppliers.memoize(tlsContextFactorySupplier::get));
     } else {
       this.tlsContextFactorySupplier = Optional.empty();
     }
+    this.clientHelloSniHeaderEnabled = clientHelloSniHeaderEnabled;
   }
 
   @Override
-  void addAdditionalOutboundHandlers(final Channel ch) throws GeneralSecurityException {
+  void addAdditionalOutboundHandlers(final Channel ch, final Peer peer)
+      throws GeneralSecurityException {
     if (tlsContextFactorySupplier.isPresent()) {
       final SslContext clientSslContext =
           tlsContextFactorySupplier.get().get().createNettyClientSslContext();
-      addHandlersToChannelPipeline(ch, clientSslContext);
+      final EnodeURL enode = peer.getEnodeURL();
+      final SslHandler sslHandler = createClientSslHandler(ch, clientSslContext, enode);
+      addHandlersToChannelPipeline(ch, sslHandler);
+    }
+  }
+
+  private SslHandler createClientSslHandler(final Channel ch, final SslContext sslContext,
+      final EnodeURL enode) {
+    if (this.clientHelloSniHeaderEnabled) {
+      return sslContext.newHandler(
+          ch.alloc(), enode.getHost(), enode.getListeningPort().orElseThrow());
+    } else {
+      return sslContext.newHandler(ch.alloc());
     }
   }
 
@@ -92,12 +113,12 @@ void addAdditionalInboundHandlers(final Channel ch) throws GeneralSecurityExcept
     if (tlsContextFactorySupplier.isPresent()) {
       final SslContext serverSslContext =
           tlsContextFactorySupplier.get().get().createNettyServerSslContext();
-      addHandlersToChannelPipeline(ch, serverSslContext);
+      addHandlersToChannelPipeline(ch, serverSslContext.newHandler(ch.alloc()));
     }
   }
 
-  private void addHandlersToChannelPipeline(final Channel ch, final SslContext sslContext) {
-    ch.pipeline().addLast(sslContext.newHandler(ch.alloc()));
+  private void addHandlersToChannelPipeline(final Channel ch, final SslHandler sslHandler) {
+    ch.pipeline().addLast(sslHandler);
     ch.pipeline().addLast(new SnappyFrameDecoder());
     ch.pipeline().addLast(new SnappyFrameEncoder());
     ch.pipeline()

diff --git a/.../main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/TLSConfiguration.java b/.../main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/TLSConfiguration.java
@@ -32,6 +32,7 @@ public class TLSConfiguration {
   private final Path trustStorePasswordPath;
   private final Path crlPath;
   private final String[] allowedProtocols;
+  private final Boolean clientHelloSniHeaderEnabled;
 
   private TLSConfiguration(
       final String keyStoreType,
@@ -43,7 +44,8 @@ private TLSConfiguration(
       final Supplier<String> trustStorePasswordSupplier,
       final Path trustStorePasswordPath,
       final Path crlPath,
-      final String[] allowedProtocols) {
+      final String[] allowedProtocols,
+      final Boolean clientHelloSniHeaderEnabled) {
     this.keyStoreType = keyStoreType;
     this.keyStorePath = keyStorePath;
     this.keyStorePasswordSupplier = keyStorePasswordSupplier;
@@ -54,6 +56,7 @@ private TLSConfiguration(
     this.trustStorePasswordPath = trustStorePasswordPath;
     this.crlPath = crlPath;
     this.allowedProtocols = allowedProtocols;
+    this.clientHelloSniHeaderEnabled = clientHelloSniHeaderEnabled;
   }
 
   public String getKeyStoreType() {
@@ -96,6 +99,10 @@ public String[] getAllowedProtocols() {
     return allowedProtocols;
   }
 
+  public Boolean getClientHelloSniHeaderEnabled() {
+    return clientHelloSniHeaderEnabled;
+  }
+
   public static final class Builder {
     private String keyStoreType;
     private Path keyStorePath;
@@ -107,6 +114,7 @@ public static final class Builder {
     private Path trustStorePasswordPath;
     private Path crlPath;
     private String[] allowedProtocols;
+    private Boolean clientHelloSniHeaderEnabled;
 
     private Builder() {}
 
@@ -165,6 +173,11 @@ public Builder withAllowedProtocols(final String[] allowedProtocols) {
       return this;
     }
 
+    public Builder withClientHelloSniEnabled(final Boolean clientHelloSniHeaderEnabled) {
+      this.clientHelloSniHeaderEnabled = clientHelloSniHeaderEnabled;
+      return this;
+    }
+
     public TLSConfiguration build() {
       requireNonNull(keyStoreType, "Key Store Type must not be null");
       requireNonNull(keyStorePasswordSupplier, "Key Store password supplier must not be null");
@@ -178,7 +191,8 @@ public TLSConfiguration build() {
           trustStorePasswordSupplier,
           trustStorePasswordPath,
           crlPath,
-          allowedProtocols);
+          allowedProtocols, clientHelloSniHeaderEnabled
+      );
     }
   }
 }
diff --git a/...perledger/besu/ethereum/p2p/rlpx/connections/netty/NettyTLSConnectionInitializerTest.java b/...perledger/besu/ethereum/p2p/rlpx/connections/netty/NettyTLSConnectionInitializerTest.java
@@ -18,17 +18,21 @@
 import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
 import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import org.hyperledger.besu.cryptoservices.NodeKey;
 import org.hyperledger.besu.ethereum.p2p.config.RlpxConfiguration;
 import org.hyperledger.besu.ethereum.p2p.peers.LocalNode;
+import org.hyperledger.besu.ethereum.p2p.peers.Peer;
 import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnectionEventDispatcher;
 import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
 
 import com.google.common.collect.ImmutableList;
 import io.netty.channel.ChannelPipeline;
@@ -39,6 +43,7 @@
 import io.netty.handler.codec.compression.SnappyFrameEncoder;
 import io.netty.handler.ssl.SslContext;
 import io.netty.handler.ssl.SslHandler;
+import org.hyperledger.besu.plugin.data.EnodeURL;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -48,40 +53,57 @@
 @RunWith(MockitoJUnitRunner.class)
 public class NettyTLSConnectionInitializerTest {
 
+  private static final String PEER_HOST = "hyperledger.org";
+  private static final int PEER_PORT = 30303;
   @Mock private NodeKey nodeKey;
   @Mock private RlpxConfiguration rlpxConfiguration;
   @Mock private LocalNode localNode;
   @Mock private PeerConnectionEventDispatcher eventDispatcher;
   @Mock private TLSContextFactory tlsContextFactory;
-  @Mock private SslContext sslContext;
-  @Mock private SslHandler sslHandler;
+  @Mock private SslContext clientSslContext;
+  @Mock private SslContext serverSslContext;
+  @Mock private SslHandler clientSslHandler;
+  @Mock private SslHandler serverSslHandler;
+  @Mock private Peer peer;
+  @Mock private EnodeURL enodeURL;
 
   private NettyTLSConnectionInitializer nettyTLSConnectionInitializer;
 
   @Before
   public void before() throws Exception {
-    nettyTLSConnectionInitializer =
-        new NettyTLSConnectionInitializer(
-            nodeKey,
-            rlpxConfiguration,
-            localNode,
-            eventDispatcher,
-            new NoOpMetricsSystem(),
-            () -> tlsContextFactory);
-
-    when(tlsContextFactory.createNettyServerSslContext()).thenReturn(sslContext);
-    when(tlsContextFactory.createNettyClientSslContext()).thenReturn(sslContext);
-    when(sslContext.newHandler(any())).thenReturn(sslHandler);
+    nettyTLSConnectionInitializer = createNettyTLSConnectionInitializer(false);
+
+    when(tlsContextFactory.createNettyServerSslContext()).thenReturn(serverSslContext);
+    when(serverSslContext.newHandler(any())).thenReturn(serverSslHandler);
+
+    when(tlsContextFactory.createNettyClientSslContext()).thenReturn(clientSslContext);
+    when(clientSslContext.newHandler(any())).thenReturn(clientSslHandler);
+    when(peer.getEnodeURL()).thenReturn(enodeURL);
+    when(enodeURL.getHost()).thenReturn(PEER_HOST);
+    when(enodeURL.getListeningPort()).thenReturn(Optional.of(PEER_PORT));
+  }
+
+  private NettyTLSConnectionInitializer createNettyTLSConnectionInitializer(
+      final boolean clientHelloSniHeaderEnabled) {
+    return new NettyTLSConnectionInitializer(
+        nodeKey,
+        rlpxConfiguration,
+        localNode,
+        eventDispatcher,
+        new NoOpMetricsSystem(),
+        () -> tlsContextFactory,
+        clientHelloSniHeaderEnabled
+    );
   }
 
   @Test
   public void addAdditionalOutboundHandlersIncludesAllExpectedHandlersToChannelPipeline()
       throws Exception {
     final EmbeddedChannel embeddedChannel = new EmbeddedChannel();
-    nettyTLSConnectionInitializer.addAdditionalOutboundHandlers(embeddedChannel);
+    nettyTLSConnectionInitializer.addAdditionalOutboundHandlers(embeddedChannel, peer);
 
     // TLS
-    assertThat(embeddedChannel.pipeline().get(SslHandler.class)).isNotNull();
+    assertThat(embeddedChannel.pipeline().get(SslHandler.class)).isEqualTo(clientSslHandler);
 
     // Snappy compression
     assertThat(embeddedChannel.pipeline().get(SnappyFrameDecoder.class)).isNotNull();
@@ -94,14 +116,30 @@ public void addAdditionalOutboundHandlersIncludesAllExpectedHandlersToChannelPip
     assertHandlersOrderInPipeline(embeddedChannel.pipeline());
   }
 
+  @Test
+  public void addAdditionalOutboundHandlersUsesSslHandlerWithSniHeaderEnabledIfConfigured()
+      throws Exception {
+    nettyTLSConnectionInitializer = createNettyTLSConnectionInitializer(true);
+    when(clientSslContext.newHandler(any(), eq(PEER_HOST), eq(PEER_PORT))).thenReturn(clientSslHandler);
+
+    final EmbeddedChannel embeddedChannel = new EmbeddedChannel();
+    nettyTLSConnectionInitializer.addAdditionalOutboundHandlers(embeddedChannel, peer);
+
+    // Handler with SNI params was called
+    verify(clientSslContext).newHandler(any(), eq(PEER_HOST), eq(PEER_PORT));
+
+    // Other handlers are still present as expected
+    assertHandlersOrderInPipeline(embeddedChannel.pipeline());
+  }
+
   @Test
   public void addAdditionalInboundHandlersIncludesAllExpectedHandlersToChannelPipeline()
       throws Exception {
     final EmbeddedChannel embeddedChannel = new EmbeddedChannel();
     nettyTLSConnectionInitializer.addAdditionalInboundHandlers(embeddedChannel);
 
     // TLS
-    assertThat(embeddedChannel.pipeline().get(SslHandler.class)).isNotNull();
+    assertThat(embeddedChannel.pipeline().get(SslHandler.class)).isEqualTo(serverSslHandler);
 
     // Snappy compression
     assertThat(embeddedChannel.pipeline().get(SnappyFrameDecoder.class)).isNotNull();