From ebf977971031febe40ca9a4cca26bf64fb9025d2 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Mon, 14 Jan 2019 12:43:25 +1000 Subject: [PATCH] NC-2013 devp2p whitelist (#430) * check peer against whitelist for incoming DevP2P messages --- .../ethereum/p2p/netty/NettyP2PNetwork.java | 17 ++++- .../NodeWhitelistController.java | 13 ++-- .../ethereum/p2p/NettyP2PNetworkTest.java | 74 +++++++++++++++++++ 3 files changed, 96 insertions(+), 8 deletions(-) diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java index 13acb3fd8a..95f5f36c35 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java @@ -24,6 +24,7 @@ import tech.pegasys.pantheon.ethereum.p2p.discovery.DiscoveryPeer; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryAgent; import tech.pegasys.pantheon.ethereum.p2p.discovery.VertxPeerDiscoveryAgent; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerRequirement; +import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer; import tech.pegasys.pantheon.ethereum.p2p.peers.Endpoint; import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; @@ -273,8 +274,11 @@ public final class NettyP2PNetwork implements P2PNetwork { connection.disconnect(DisconnectReason.TOO_MANY_PEERS); return; } - // Reject incoming connections that are blacklisted - if (peerBlacklist.contains(connection)) { + + boolean isPeerBlacklisted = peerBlacklist.contains(connection); + boolean isPeerNotWhitelisted = !isPeerWhitelisted(connection, ch); + + if (isPeerBlacklisted || isPeerNotWhitelisted) { connection.disconnect(DisconnectReason.UNKNOWN); return; } @@ -288,6 +292,15 @@ public final class NettyP2PNetwork implements P2PNetwork { }; } + private boolean isPeerWhitelisted(final PeerConnection connection, final SocketChannel ch) { + return nodeWhitelistController.contains( + new DefaultPeer( + connection.getPeer().getNodeId(), + ch.remoteAddress().getAddress().getHostAddress(), + connection.getPeer().getPort(), + connection.getPeer().getPort())); + } + private int connectionCount() { return pendingConnections.get() + connections.size(); } 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 d255b55f06..11bc96476c 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 @@ -25,18 +25,15 @@ import com.google.common.annotations.VisibleForTesting; public class NodeWhitelistController { - private final List nodesWhitelist; + private final List nodesWhitelist = new ArrayList<>(); private boolean nodeWhitelistSet = false; public NodeWhitelistController(final PermissioningConfiguration configuration) { - nodesWhitelist = new ArrayList<>(); - if (configuration != null && configuration.getNodeWhitelist() != null) { + if (configuration.isNodeWhitelistSet() && configuration.getNodeWhitelist() != null) { for (URI uri : configuration.getNodeWhitelist()) { nodesWhitelist.add(DefaultPeer.fromURI(uri)); } - if (configuration.isNodeWhitelistSet()) { - nodeWhitelistSet = true; - } + nodeWhitelistSet = true; } } @@ -109,6 +106,10 @@ public class NodeWhitelistController { } } + public boolean contains(final Peer node) { + return (!nodeWhitelistSet || (nodesWhitelist.contains(node))); + } + public enum NodesWhitelistResultType { SUCCESS, ADD_ERROR_DUPLICATED_ENTRY, diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java index 623c03cc5c..21f91a6e67 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java @@ -395,6 +395,80 @@ public final class NettyP2PNetworkTest { } } + @Test + public void rejectIncomingConnectionFromNonWhitelistedPeer() throws Exception { + final DiscoveryConfiguration noDiscovery = DiscoveryConfiguration.create().setActive(false); + final SECP256K1.KeyPair localKp = SECP256K1.KeyPair.generate(); + final SECP256K1.KeyPair remoteKp = SECP256K1.KeyPair.generate(); + final BytesValue localId = localKp.getPublicKey().getEncodedBytes(); + final BytesValue remoteId = remoteKp.getPublicKey().getEncodedBytes(); + final PeerBlacklist localBlacklist = new PeerBlacklist(); + final PeerBlacklist remoteBlacklist = new PeerBlacklist(); + final PermissioningConfiguration config = PermissioningConfiguration.createDefault(); + NodeWhitelistController localWhitelist = new NodeWhitelistController(config); + // turn on whitelisting by adding a different node NOT remote node + localWhitelist.addNode(mockPeer()); + + final SubProtocol subprotocol = subProtocol(); + final Capability cap = Capability.create(subprotocol.getName(), 63); + try (final P2PNetwork localNetwork = + new NettyP2PNetwork( + vertx, + localKp, + NetworkingConfiguration.create() + .setDiscovery(noDiscovery) + .setSupportedProtocols(subprotocol) + .setRlpx(RlpxConfiguration.create().setBindPort(0)), + singletonList(cap), + () -> false, + localBlacklist, + new NoOpMetricsSystem(), + localWhitelist); + final P2PNetwork remoteNetwork = + new NettyP2PNetwork( + vertx, + remoteKp, + NetworkingConfiguration.create() + .setSupportedProtocols(subprotocol) + .setRlpx(RlpxConfiguration.create().setBindPort(0)) + .setDiscovery(noDiscovery), + singletonList(cap), + () -> false, + remoteBlacklist, + new NoOpMetricsSystem(), + new NodeWhitelistController(PermissioningConfiguration.createDefault()))) { + final int localListenPort = localNetwork.getLocalPeerInfo().getPort(); + final Peer localPeer = + new DefaultPeer( + localId, + new Endpoint( + InetAddress.getLoopbackAddress().getHostAddress(), + localListenPort, + OptionalInt.of(localListenPort))); + + localNetwork.run(); + remoteNetwork.run(); + + // Setup disconnect listener + final CompletableFuture peerFuture = new CompletableFuture<>(); + final CompletableFuture reasonFuture = new CompletableFuture<>(); + remoteNetwork.subscribeDisconnect( + (peerConnection, reason, initiatedByPeer) -> { + peerFuture.complete(peerConnection); + reasonFuture.complete(reason); + }); + + // Remote connect to local + final CompletableFuture connectFuture = remoteNetwork.connect(localPeer); + + // Check connection is made, and then a disconnect is registered at remote + assertThat(connectFuture.get(5L, TimeUnit.SECONDS).getPeer().getNodeId()).isEqualTo(localId); + assertThat(peerFuture.get(5L, TimeUnit.SECONDS).getPeer().getNodeId()).isEqualTo(localId); + assertThat(reasonFuture.get(5L, TimeUnit.SECONDS)) + .isEqualByComparingTo(DisconnectReason.UNKNOWN); + } + } + private SubProtocol subProtocol() { return new SubProtocol() { @Override