Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to add SNI header in TLS ClientHello message #4894

Closed
wilsonfv opened this issue Jan 9, 2023 · 3 comments · Fixed by #5439
Closed

Ability to add SNI header in TLS ClientHello message #4894

wilsonfv opened this issue Jan 9, 2023 · 3 comments · Fixed by #5439
Labels
good first issue Good for newcomers

Comments

@wilsonfv
Copy link

wilsonfv commented Jan 9, 2023

Description

When join a network / connect to another node, as an INITIATOR during first plain handshake message, I want to add SNI header in TLS ClientHello message so that Target Server can return correct certificate by reading TLS SNI header.

Modern Server may host multiple virtual hostnames. In order to return correct TLS certificate to client, client may add SNI header during TLS clientHello. This is common industry solution to resolve one server hosting multiple virtual hostnames problem.

Existing P2P RLPx agent does not add SNI header when constructing TLS ClientHello.

private CompletableFuture<PeerConnection> initiateOutboundConnection(final Peer peer) {
LOG.trace("Initiating connection to peer: {}", peer.getEnodeURL());
if (peer instanceof DiscoveryPeer) {
((DiscoveryPeer) peer).setLastAttemptedConnection(System.currentTimeMillis());
}
return connectionInitializer
.connect(peer)
.whenComplete(
(conn, err) -> {
if (err != null) {
LOG.debug("Failed to connect to peer {}: {}", peer.getId(), err);
} else {
LOG.debug("Outbound connection established to peer: {}", peer.getId());
}
});
}

private void addHandlersToChannelPipeline(final Channel ch, final SslContext sslContext) {
ch.pipeline().addLast(sslContext.newHandler(ch.alloc()));
ch.pipeline().addLast(new SnappyFrameDecoder());
ch.pipeline().addLast(new SnappyFrameEncoder());
ch.pipeline()
.addLast(
new LengthFieldBasedFrameDecoder(
LENGTH_MAX_MESSAGE_FRAME, 0, LENGTH_FRAME_SIZE, 0, LENGTH_FRAME_SIZE));
ch.pipeline().addLast(new LengthFieldPrepender(LENGTH_FRAME_SIZE));
}

In order to add SNI header in TLS ClientHello, NettyTLSConnectionInitializer should call another implementation of sslContext.newHandler

    public final SslHandler newHandler(ByteBufAllocator alloc, String peerHost, int peerPort) {
        return newHandler(alloc, peerHost, peerPort, startTls);
    }

We could add a new command line option in Besu to enable this feature. Such as proposed command line option p2p-tls-clienthello-sni, once this is enabled, SNI header will be added in TLS ClientHello. Default behavior is disabled.

Acceptance Criteria

If besu command line option p2p-tls-clienthello-sni is enabled, RLPx agent will add SNI header in TLS ClientHello message.

@non-fungible-nelson non-fungible-nelson added the good first issue Good for newcomers label Jan 26, 2023
@non-fungible-nelson
Copy link
Contributor

non-fungible-nelson commented Jan 26, 2023

Hi there - thank you for requesting this! This is a great issue for a new contributor to pick up. It is likely the Maintainers cannot pick this up at this time.

@atoulme
Copy link
Contributor

atoulme commented Jan 28, 2023

I'm sorry, I'm unsure why I'm tagged on this issue. I have no insight into this work.

megglos pushed a commit to megglos/besu that referenced this issue May 5, 2023
* add --Xp2p-tls-clienthello-sni option to enable the SNI header

Issue hyperledger#4894

Signed-off-by: Sebastian Bathke <sebastian.bathke@gmail.com>
megglos pushed a commit to megglos/besu that referenced this issue May 5, 2023
* add --Xp2p-tls-clienthello-sni option to enable the SNI header

Issue hyperledger#4894

Signed-off-by: Sebastian Bathke <sebastian.bathke@gmail.com>
megglos pushed a commit to megglos/besu that referenced this issue May 5, 2023
* add --Xp2p-tls-clienthello-sni option to enable the SNI header

Issue hyperledger#4894

Signed-off-by: Sebastian Bathke <sebastian.bathke@gmail.com>
megglos pushed a commit to megglos/besu that referenced this issue May 5, 2023
…#4894)

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

Issue hyperledger#4894

Signed-off-by: Sebastian Bathke <sebastian.bathke@gmail.com>
megglos pushed a commit to megglos/besu that referenced this issue May 6, 2023
…#4894)

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

Issue hyperledger#4894

Signed-off-by: Sebastian Bathke <sebastian.bathke@gmail.com>
macfarla added a commit that referenced this issue May 10, 2023
* add --Xp2p-tls-clienthello-sni option to enable the SNI header

Issue #4894

Signed-off-by: Sebastian Bathke <sebastian.bathke@gmail.com>
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Co-authored-by: Sebastian Bathke <sebastian.bathke@gmail.com>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
fab-10 pushed a commit to fab-10/besu that referenced this issue May 10, 2023
* Update dependencies - commons-net (hyperledger#5444)

* update antlr and commons-net dependencies

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>

* revert antlr uprev

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>

* comment

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>

---------

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>

* Add option to send SNI header in TLS ClientHello message (hyperledger#4894) (hyperledger#5439)

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

Issue hyperledger#4894

Signed-off-by: Sebastian Bathke <sebastian.bathke@gmail.com>
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Co-authored-by: Sebastian Bathke <sebastian.bathke@gmail.com>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>

* ZkTrieLogFactoryImpl, rebased off of main

Signed-off-by: garyschulte <garyschulte@gmail.com>

* code before account

Signed-off-by: garyschulte <garyschulte@gmail.com>

* trielog shipping observer

Signed-off-by: garyschulte <garyschulte@gmail.com>

* use hex encoding in trielog shipping, add a TESTING default ZkTrieLogObserver to AbstractTrieLogManager

Signed-off-by: garyschulte <garyschulte@gmail.com>

* fix unintended recursion

Signed-off-by: garyschulte <garyschulte@gmail.com>

* filter out self destruct storage changes in ZkTrieLogImpl

Signed-off-by: garyschulte <garyschulte@gmail.com>

* store both slotHash and slotKey, defer to shomei to handle it as it sees fit

Signed-off-by: garyschulte <garyschulte@gmail.com>

* reorder trielog storage to differentiate between ZERO slot key and null slot key

Signed-off-by: garyschulte <garyschulte@gmail.com>

* do not filter unchanged accounts for zktrielogfactory

Signed-off-by: garyschulte <garyschulte@gmail.com>

* use blockHeader during trielog construction, add blockNumber to trielog, add blockHeader to trielogaddedevent

Signed-off-by: garyschulte <garyschulte@gmail.com>

* add blockNumber to rpc call to shomei

Signed-off-by: garyschulte <garyschulte@gmail.com>

* add isSyncing to the trieLogParameter and ZkTrieLogObserver

Signed-off-by: garyschulte <garyschulte@gmail.com>

* initial plumbing for trielog shipping plugin

Signed-off-by: garyschulte <garyschulte@gmail.com>

* halfway through generics hell

Signed-off-by: garyschulte <garyschulte@gmail.com>

* TrieLogs in plugin data

Signed-off-by: garyschulte <garyschulte@gmail.com>

* end of friday, green build with plugin refactoring, still need to add TrieLog Plugin Service

Signed-off-by: garyschulte <garyschulte@gmail.com>

* remove errant reference to ZkTrieLogFactory

Signed-off-by: garyschulte <garyschulte@gmail.com>

* adding dagger-wired plugincontext and TrieLogService

Signed-off-by: garyschulte <garyschulte@gmail.com>

* javadoc and fixes

Signed-off-by: garyschulte <garyschulte@gmail.com>

* add plugin observer subscription

Signed-off-by: garyschulte <garyschulte@gmail.com>

* plugin-api hash

Signed-off-by: garyschulte <garyschulte@gmail.com>

* fix besuComponent ref, move addService up into BesuContext

Signed-off-by: garyschulte <garyschulte@gmail.com>

* add TrieLogRangePair composition of blocknumber and TrieLog, use that for getTrieLogByRange return

Signed-off-by: garyschulte <garyschulte@gmail.com>

* javadoc

Signed-off-by: garyschulte <garyschulte@gmail.com>

* move TrieLog from datatypes to plugin-api

Signed-off-by: garyschulte <garyschulte@gmail.com>

* add blockHash to TrieLogRangeTuple

Signed-off-by: garyschulte <garyschulte@gmail.com>

---------

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Sebastian Bathke <sebastian.bathke@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Co-authored-by: Sebastian Bathke <sebastian.bathke@camunda.com>
Co-authored-by: Sebastian Bathke <sebastian.bathke@gmail.com>
elenduuche pushed a commit to elenduuche/besu that referenced this issue Aug 16, 2023
…#4894) (hyperledger#5439)

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

Issue hyperledger#4894

Signed-off-by: Sebastian Bathke <sebastian.bathke@gmail.com>
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Co-authored-by: Sebastian Bathke <sebastian.bathke@gmail.com>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
eum602 pushed a commit to lacchain/besu that referenced this issue Nov 3, 2023
…#4894) (hyperledger#5439)

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

Issue hyperledger#4894

Signed-off-by: Sebastian Bathke <sebastian.bathke@gmail.com>
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Co-authored-by: Sebastian Bathke <sebastian.bathke@gmail.com>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
@atoulme @wilsonfv @non-fungible-nelson and others