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

Use From field in a PING packet when creating a peer table entry #6225

Merged
merged 10 commits into from
Jan 19, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ https://hyperledger.jfrog.io/artifactory/besu-binaries/besu/23.10.3-hotfix/besu-
### Bug fixes
- Fix Docker image name clash between Besu and evmtool [#6194](https://github.com/hyperledger/besu/pull/6194)
- Fix `logIndex` in `eth_getTransactionReceipt` JSON RPC method [#6206](https://github.com/hyperledger/besu/pull/6206)
- Fix the way an advertised host configured with `--p2p-host` is treated when communicating with the originator of a PING packet [#6225](https://github.com/hyperledger/besu/pull/6225)

### Download Links
https://hyperledger.jfrog.io/artifactory/besu-binaries/besu/23.10.3/besu-23.10.3.zip / sha256 da7ef8a6ceb88d3e327cacddcdb32218d1750b464c14165a74068f6dc6e0871a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,8 +282,29 @@ protected void handleIncomingPacket(final Endpoint sourceEndpoint, final Packet
.flatMap(Endpoint::getTcpPort)
.orElse(udpPort);

// If the host is present in the P2P PING packet itself, use that as the endpoint. If the P2P
// PING packet specifies 127.0.0.1 (the default if a custom value is not specified with
// --p2p-host or via a suitable --nat-method) we ignore it in favour of the UDP source address.
// The likelihood is that the UDP source will be 127.0.0.1 anyway, but this reduces the chance
// of an unexpected change in behaviour as a result of
// https://github.com/hyperledger/besu/issues/6224 being fixed.
final String host =
packet
.getPacketData(PingPacketData.class)
.flatMap(PingPacketData::getFrom)
.map(Endpoint::getHost)
.filter(abc -> !abc.equals("127.0.0.1"))
.stream()
.peek(
h ->
LOG.trace(
"Using \"From\" endpoint {} specified in ping packet. Ignoring UDP source host {}",
h,
sourceEndpoint.getHost()))
.findFirst()
.orElseGet(sourceEndpoint::getHost);

// Notify the peer controller.
final String host = sourceEndpoint.getHost();
final DiscoveryPeer peer =
DiscoveryPeer.fromEnode(
EnodeURLImpl.builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,30 @@ public void neighborsPacketLimited() {
}
}

@Test
public void endpointHonoursCustomAdvertisedAddressInPingPacket() {

// Start a peer with the default advertised host
final MockPeerDiscoveryAgent agent1 = helper.startDiscoveryAgent();

// Start another peer with its advertised host set to a custom value
final MockPeerDiscoveryAgent agent2 = helper.startDiscoveryAgent("192.168.0.1");

// Send a PING so we can exchange messages
Packet packet = helper.createPingPacket(agent2, agent1);
helper.sendMessageBetweenAgents(agent2, agent1, packet);

// Agent 1's peers should have endpoints that match the custom advertised value...
agent1
.streamDiscoveredPeers()
.forEach(peer -> assertThat(peer.getEndpoint().getHost()).isEqualTo("192.168.0.1"));

// ...but agent 2's peers should have endpoints that match the default
agent2
.streamDiscoveredPeers()
.forEach(peer -> assertThat(peer.getEndpoint().getHost()).isEqualTo("127.0.0.1"));
}

@Test
public void shouldEvictPeerWhenPermissionsRevoked() {
final PeerPermissionsDenylist denylist = PeerPermissionsDenylist.create();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,14 @@ public MockPeerDiscoveryAgent startDiscoveryAgent(final DiscoveryPeer... bootstr
return startDiscoveryAgent(agentBuilder);
}

public MockPeerDiscoveryAgent startDiscoveryAgent(
final String advertisedHost, final DiscoveryPeer... bootstrapPeers) {
final AgentBuilder agentBuilder =
agentBuilder().bootstrapPeers(bootstrapPeers).advertisedHost(advertisedHost);

return startDiscoveryAgent(agentBuilder);
}

/**
* Start a single discovery agent with the provided bootstrap peers.
*
Expand Down
Loading