Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Commit

Permalink
[NC-2247] Fixing node whitelist isPermitted check (#766)
Browse files Browse the repository at this point in the history 
* [NC-2247] Fixing node whitelist isPermitted check

* Typos
  • Loading branch information
@lucassaldanha
lucassaldanha authored Feb 5, 2019
1 parent 2040799 commit 9ca278c
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 1 deletion.
5 changes: 5 additions & 0 deletions ...n/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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);
Expand All @@ -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 -> {
Expand All @@ -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 -> {
Expand All @@ -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;
}
Expand Down
19 changes: 18 additions & 1 deletion ...c/main/java/tech/pegasys/pantheon/ethereum/p2p/permissioning/NodeWhitelistController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,24 @@ private NodesWhitelistResult validInput(final List<DefaultPeer> 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<Peer> getNodesWhitelist() {
Expand Down
105 changes: 105 additions & 0 deletions ...st/java/tech/pegasys/pantheon/ethereum/p2p/permissioning/NodeWhitelistControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -125,4 +127,107 @@ public void whenRemoveNodesInputHasNullListOfNodesShouldReturnErrorEmptyEntry()

assertThat(actualResult).isEqualToComparingOnlyGivenFields(expected, "result");
}

@Test
public void whenNodeIdsAreDifferentItShouldNotBePermitted() {
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 whenNodesUdpPortsAreDifferentItShouldNotBePermitted() {
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 whenCheckingIfNodeIsPermittedTcpPortShouldNotBeConsideredIfAbsent() {
Peer peerWithTcpPortSet =
new DefaultPeer(
BytesValue.fromHexString(
"0x6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"),
"127.0.0.1",
30303,
10001);
Peer peerWithoutTcpPortSet =
new DefaultPeer(
BytesValue.fromHexString(
"0x6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"),
"127.0.0.1",
30303);

controller.addNode(peerWithoutTcpPortSet);

assertThat(controller.isPermitted(peerWithTcpPortSet)).isTrue();
}

@Test
public void whenCheckingIfNodeIsPermittedTcpPortShouldBeConsideredIfPresent() {
Peer peerWithTcpPortSet =
new DefaultPeer(
BytesValue.fromHexString(
"0x6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"),
"127.0.0.1",
30303,
10001);
Peer peerWithDifferentTcpPortSet =
new DefaultPeer(
BytesValue.fromHexString(
"0x6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"),
"127.0.0.1",
30303,
10002);

controller.addNode(peerWithDifferentTcpPortSet);

assertThat(controller.isPermitted(peerWithTcpPortSet)).isFalse();
}
}

0 comments on commit 9ca278c

Please sign in to comment.