From e18a042af9b5a976ad8f41a82c21e5cfaf228ed8 Mon Sep 17 00:00:00 2001 From: Lucas Saldanha Date: Tue, 5 Feb 2019 14:18:54 +1300 Subject: [PATCH 1/2] [NC-2247] Fixing node whitelist isPermitted check --- .../internal/PeerDiscoveryController.java | 5 + .../NodeWhitelistController.java | 19 +++- .../NodeWhitelistControllerTest.java | 105 ++++++++++++++++++ 3 files changed, 128 insertions(+), 1 deletion(-) diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryController.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryController.java index 840ce490d1..f6debecb13 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryController.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryController.java @@ -211,6 +211,7 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { } if (!whitelistIfPresentIsNodePermitted(sender)) { + LOG.trace("Dropping packet from peer not in the whitelist ({})", sender.getEnodeURI()); return; } @@ -222,6 +223,7 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { switch (packet.getType()) { case PING: + LOG.trace("Received PING packet from {}", sender.getEnodeURI()); if (!peerBlacklisted && addToPeerTable(peer)) { final PingPacketData ping = packet.getPacketData(PingPacketData.class).get(); respondToPing(ping, packet.getHash(), peer); @@ -230,6 +232,7 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { break; case PONG: { + LOG.trace("Received PONG packet from {}", sender.getEnodeURI()); matchInteraction(packet) .ifPresent( interaction -> { @@ -246,6 +249,7 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { break; } case NEIGHBORS: + LOG.trace("Received NEIGHBORS packet from {}", sender.getEnodeURI()); matchInteraction(packet) .ifPresent( interaction -> { @@ -271,6 +275,7 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { break; case FIND_NEIGHBORS: + LOG.trace("Received FIND_NEIGHBORS packet from {}", sender.getEnodeURI()); if (!peerKnown || peerBlacklisted) { break; } diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/permissioning/NodeWhitelistController.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/permissioning/NodeWhitelistController.java index 68c10dde40..b7f4346483 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/permissioning/NodeWhitelistController.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/permissioning/NodeWhitelistController.java @@ -99,7 +99,24 @@ private NodesWhitelistResult validInput(final List peers) { } public boolean isPermitted(final Peer node) { - return (nodesWhitelist.contains(node)); + return nodesWhitelist + .stream() + .anyMatch( + p -> { + boolean idsMatch = node.getId().equals(p.getId()); + boolean hostsMatch = node.getEndpoint().getHost().equals(p.getEndpoint().getHost()); + boolean udpPortsMatch = + node.getEndpoint().getUdpPort() == p.getEndpoint().getUdpPort(); + boolean tcpPortsMatchIfPresent = true; + if (node.getEndpoint().getTcpPort().isPresent() + && p.getEndpoint().getTcpPort().isPresent()) { + tcpPortsMatchIfPresent = + node.getEndpoint().getTcpPort().getAsInt() + == p.getEndpoint().getTcpPort().getAsInt(); + } + + return idsMatch && hostsMatch && udpPortsMatch && tcpPortsMatchIfPresent; + }); } public List getNodesWhitelist() { diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/permissioning/NodeWhitelistControllerTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/permissioning/NodeWhitelistControllerTest.java index fe9311842c..f29abfe132 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/permissioning/NodeWhitelistControllerTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/permissioning/NodeWhitelistControllerTest.java @@ -16,8 +16,10 @@ import static tech.pegasys.pantheon.ethereum.p2p.permissioning.NodeWhitelistController.NodesWhitelistResult; import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer; +import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfiguration; import tech.pegasys.pantheon.ethereum.permissioning.WhitelistOperationResult; +import tech.pegasys.pantheon.util.bytes.BytesValue; import java.util.ArrayList; import java.util.Arrays; @@ -125,4 +127,107 @@ public void whenRemoveNodesInputHasNullListOfNodesShouldReturnErrorEmptyEntry() assertThat(actualResult).isEqualToComparingOnlyGivenFields(expected, "result"); } + + @Test + public void whenNodesIdAreDifferentItShouldNotBePermitted() { + Peer peer1 = + new DefaultPeer( + BytesValue.fromHexString( + "0xaaaa80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"), + "127.0.0.1", + 30303); + Peer peer2 = + new DefaultPeer( + BytesValue.fromHexString( + "0xbbba80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"), + "127.0.0.1", + 30303); + + controller.addNode(peer1); + + assertThat(controller.isPermitted(peer2)).isFalse(); + } + + @Test + public void whenNodesHostsAreDifferentItShouldNotBePermitted() { + Peer peer1 = + new DefaultPeer( + BytesValue.fromHexString( + "0xaaaa80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"), + "127.0.0.1", + 30303); + Peer peer2 = + new DefaultPeer( + BytesValue.fromHexString( + "0xaaaa80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"), + "127.0.0.2", + 30303); + + controller.addNode(peer1); + + assertThat(controller.isPermitted(peer2)).isFalse(); + } + + @Test + public void whenNodesUpdPortsAreDifferentItShouldNotBePermitted() { + Peer peer1 = + new DefaultPeer( + BytesValue.fromHexString( + "0xaaaa80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"), + "127.0.0.1", + 30301); + Peer peer2 = + new DefaultPeer( + BytesValue.fromHexString( + "0xaaaa80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"), + "127.0.0.1", + 30302); + + controller.addNode(peer1); + + assertThat(controller.isPermitted(peer2)).isFalse(); + } + + @Test + public void whenCheckingIfNodeIsPermittedTpcPortShouldNotBeConsideredIfAbsent() { + Peer peerWithTcpPortSet = + new DefaultPeer( + BytesValue.fromHexString( + "0x6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"), + "127.0.0.1", + 30303, + 10001); + Peer peerWithoutTpcPortSet = + new DefaultPeer( + BytesValue.fromHexString( + "0x6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"), + "127.0.0.1", + 30303); + + controller.addNode(peerWithoutTpcPortSet); + + assertThat(controller.isPermitted(peerWithTcpPortSet)).isTrue(); + } + + @Test + public void whenCheckingIfNodeIsPermittedTpcPortShouldBeConsideredIfPresent() { + Peer peerWithTcpPortSet = + new DefaultPeer( + BytesValue.fromHexString( + "0x6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"), + "127.0.0.1", + 30303, + 10001); + Peer peerWithDifferentTpcPortSet = + new DefaultPeer( + BytesValue.fromHexString( + "0x6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"), + "127.0.0.1", + 30303, + 10002); + + controller.addNode(peerWithDifferentTpcPortSet); + + assertThat(controller.isPermitted(peerWithTcpPortSet)).isFalse(); + } } From cc5bb406fdc76ea97805698f01bb510d8eac963b Mon Sep 17 00:00:00 2001 From: Lucas Saldanha Date: Tue, 5 Feb 2019 14:52:38 +1300 Subject: [PATCH 2/2] Typos --- .../NodeWhitelistControllerTest.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/permissioning/NodeWhitelistControllerTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/permissioning/NodeWhitelistControllerTest.java index f29abfe132..839ba67583 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/permissioning/NodeWhitelistControllerTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/permissioning/NodeWhitelistControllerTest.java @@ -129,7 +129,7 @@ public void whenRemoveNodesInputHasNullListOfNodesShouldReturnErrorEmptyEntry() } @Test - public void whenNodesIdAreDifferentItShouldNotBePermitted() { + public void whenNodeIdsAreDifferentItShouldNotBePermitted() { Peer peer1 = new DefaultPeer( BytesValue.fromHexString( @@ -169,7 +169,7 @@ public void whenNodesHostsAreDifferentItShouldNotBePermitted() { } @Test - public void whenNodesUpdPortsAreDifferentItShouldNotBePermitted() { + public void whenNodesUdpPortsAreDifferentItShouldNotBePermitted() { Peer peer1 = new DefaultPeer( BytesValue.fromHexString( @@ -189,7 +189,7 @@ public void whenNodesUpdPortsAreDifferentItShouldNotBePermitted() { } @Test - public void whenCheckingIfNodeIsPermittedTpcPortShouldNotBeConsideredIfAbsent() { + public void whenCheckingIfNodeIsPermittedTcpPortShouldNotBeConsideredIfAbsent() { Peer peerWithTcpPortSet = new DefaultPeer( BytesValue.fromHexString( @@ -197,20 +197,20 @@ public void whenCheckingIfNodeIsPermittedTpcPortShouldNotBeConsideredIfAbsent() "127.0.0.1", 30303, 10001); - Peer peerWithoutTpcPortSet = + Peer peerWithoutTcpPortSet = new DefaultPeer( BytesValue.fromHexString( "0x6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"), "127.0.0.1", 30303); - controller.addNode(peerWithoutTpcPortSet); + controller.addNode(peerWithoutTcpPortSet); assertThat(controller.isPermitted(peerWithTcpPortSet)).isTrue(); } @Test - public void whenCheckingIfNodeIsPermittedTpcPortShouldBeConsideredIfPresent() { + public void whenCheckingIfNodeIsPermittedTcpPortShouldBeConsideredIfPresent() { Peer peerWithTcpPortSet = new DefaultPeer( BytesValue.fromHexString( @@ -218,7 +218,7 @@ public void whenCheckingIfNodeIsPermittedTpcPortShouldBeConsideredIfPresent() { "127.0.0.1", 30303, 10001); - Peer peerWithDifferentTpcPortSet = + Peer peerWithDifferentTcpPortSet = new DefaultPeer( BytesValue.fromHexString( "0x6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"), @@ -226,7 +226,7 @@ public void whenCheckingIfNodeIsPermittedTpcPortShouldBeConsideredIfPresent() { 30303, 10002); - controller.addNode(peerWithDifferentTpcPortSet); + controller.addNode(peerWithDifferentTcpPortSet); assertThat(controller.isPermitted(peerWithTcpPortSet)).isFalse(); }