Skip to content

Commit

Permalink
Add option to send SNI header in TLS ClientHello message (hyperledger…
Browse files Browse the repository at this point in the history 
…#4894)

* add --Xp2p-tls-clienthello-sni option to enable the SNI header

Issue hyperledger#4894

Signed-off-by: Sebastian Bathke <sebastian.bathke@gmail.com>
  • Loading branch information
Sebastian Bathke committed May 5, 2023
1 parent 2571353 commit 941a489
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 29 deletions.
8 changes: 8 additions & 0 deletions besu/src/main/java/org/hyperledger/besu/cli/options/stable/P2PTLSConfigOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -132,6 +139,7 @@ public Optional<TLSConfiguration> p2pTLSConfiguration(final CommandLine commandL
: new FileBasedPasswordProvider(p2pTLSTrustStorePasswordFile))
.withTrustStorePasswordPath(p2pTLSTrustStorePasswordFile)
.withCrlPath(p2pCrlFile)
.withClientHelloSniEnabled(p2pTlsClientHelloSniHeaderEnabled)
.build());
}

Expand Down
1 change: 1 addition & 0 deletions besu/src/test/resources/everything_config.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,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
4 changes: 2 additions & 2 deletions .../org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
};
Expand Down Expand Up @@ -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)
Expand Down
34 changes: 27 additions & 7 deletions ...g/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyTLSConnectionInitializer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -58,7 +62,8 @@ public NettyTLSConnectionInitializer(
localNode,
eventDispatcher,
metricsSystem,
defaultTlsContextFactorySupplier(p2pTLSConfiguration));
defaultTlsContextFactorySupplier(p2pTLSConfiguration),
p2pTLSConfiguration.getClientHelloSniHeaderEnabled());
}

@VisibleForTesting
Expand All @@ -68,22 +73,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());
}
}

Expand All @@ -92,12 +112,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()
Expand Down
18 changes: 16 additions & 2 deletions .../main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/TLSConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand All @@ -54,6 +56,7 @@ private TLSConfiguration(
this.trustStorePasswordPath = trustStorePasswordPath;
this.crlPath = crlPath;
this.allowedProtocols = allowedProtocols;
this.clientHelloSniHeaderEnabled = clientHelloSniHeaderEnabled;
}

public String getKeyStoreType() {
Expand Down Expand Up @@ -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;
Expand All @@ -107,6 +114,7 @@ public static final class Builder {
private Path trustStorePasswordPath;
private Path crlPath;
private String[] allowedProtocols;
private Boolean clientHelloSniHeaderEnabled;

private Builder() {}

Expand Down Expand Up @@ -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");
Expand All @@ -178,7 +191,8 @@ public TLSConfiguration build() {
trustStorePasswordSupplier,
trustStorePasswordPath,
crlPath,
allowedProtocols);
allowedProtocols,
clientHelloSniHeaderEnabled);
}
}
}
3 changes: 2 additions & 1 deletion ...reum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/network/P2PPlainNetworkTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,8 @@ public Optional<TLSConfiguration> p2pTLSEnabled(final String clientDirName) {
.withTrustStoreType(KEYSTORE_TYPE_PKCS12)
.withTrustStorePath(toPath(String.format(truststorePath, clientDirName)))
.withTrustStorePasswordSupplier(() -> "test123")
.withCrlPath(toPath(crl));
.withCrlPath(toPath(crl))
.withClientHelloSniEnabled(false);

return Optional.of(builder.build());
}
Expand Down
72 changes: 55 additions & 17 deletions ...perledger/besu/ethereum/p2p/rlpx/connections/netty/NettyTLSConnectionInitializerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,22 @@
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 org.hyperledger.besu.plugin.data.EnodeURL;

import java.util.ArrayList;
import java.util.List;
import java.util.Optional;

import com.google.common.collect.ImmutableList;
import io.netty.channel.ChannelPipeline;
Expand All @@ -48,40 +53,56 @@
@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();
Expand All @@ -94,14 +115,31 @@ 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();
Expand Down

0 comments on commit 941a489

Please sign in to comment.