From c227d1b42559ec91f3934e1109076ba5ffc662e9 Mon Sep 17 00:00:00 2001 From: mbaxter Date: Fri, 17 May 2019 14:09:49 -0400 Subject: [PATCH] [PAN-2614] Add simple PeerPermissions interface (#1446) --- .../dsl/node/ThreadPantheonNodeRunner.java | 2 - .../ethereum/eth/transactions/TestNode.java | 2 - .../JsonRpcHttpServiceRpcApisTest.java | 2 - .../ethereum/p2p/discovery/DiscoveryPeer.java | 9 + .../p2p/discovery/PeerDiscoveryAgent.java | 87 ++----- .../discovery/VertxPeerDiscoveryAgent.java | 6 +- .../internal/OutboundMessageHandler.java | 2 +- .../internal/PeerDiscoveryController.java | 229 ++++++++++++++---- .../discovery/internal/PeerRequirement.java | 2 + .../internal/RecursivePeerRefreshState.java | 26 +- .../p2p/network/DefaultP2PNetwork.java | 92 +++---- .../p2p/network/PeerReputationManager.java | 52 ++++ .../ethereum/p2p/peers/PeerBlacklist.java | 117 --------- .../p2p/permissions/PeerPermissions.java | 111 +++++++++ .../permissions/PeerPermissionsBlacklist.java | 81 +++++++ .../PermissionsUpdateCallback.java | 33 +++ .../p2p/discovery/PeerDiscoveryAgentTest.java | 150 +----------- .../discovery/PeerDiscoveryTestHelper.java | 20 +- .../PeerDiscoveryTimestampsTest.java | 27 +-- .../internal/MockPeerDiscoveryAgent.java | 6 +- .../internal/PeerDiscoveryControllerTest.java | 97 +++----- .../PeerDiscoveryTableRefreshTest.java | 28 +-- .../RecursivePeerRefreshStateTest.java | 74 ++---- .../p2p/network/DefaultP2PNetworkTest.java | 76 +++++- .../NetworkingServiceLifecycleTest.java | 2 - .../ethereum/p2p/network/P2PNetworkTest.java | 10 +- .../network/PeerReputationManagerTest.java | 107 ++++++++ .../ethereum/p2p/peers/PeerBlacklistTest.java | 219 ----------------- .../PeerPermissionsBlacklistTest.java | 218 +++++++++++++++++ .../p2p/permissions/PeerPermissionsTest.java | 131 ++++++++++ .../tech/pegasys/pantheon/RunnerBuilder.java | 15 +- .../pegasys/pantheon/cli/PantheonCommand.java | 15 +- .../tech/pegasys/pantheon/RunnerTest.java | 1 - .../pantheon/cli/CommandTestAbstract.java | 2 + .../pantheon/cli/PantheonCommandTest.java | 54 +++-- .../pegasys/pantheon/util/LimitedSet.java | 48 ++++ .../pegasys/pantheon/util/enode/EnodeURL.java | 11 + .../pegasys/pantheon/util/LimitedSetTest.java | 60 +++++ .../pantheon/util/enode/EnodeURLTest.java | 18 ++ 39 files changed, 1369 insertions(+), 873 deletions(-) create mode 100644 ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/PeerReputationManager.java delete mode 100644 ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/PeerBlacklist.java create mode 100644 ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/permissions/PeerPermissions.java create mode 100644 ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/permissions/PeerPermissionsBlacklist.java create mode 100644 ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/permissions/PermissionsUpdateCallback.java create mode 100644 ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/PeerReputationManagerTest.java delete mode 100644 ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/peers/PeerBlacklistTest.java create mode 100644 ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/permissions/PeerPermissionsBlacklistTest.java create mode 100644 ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/permissions/PeerPermissionsTest.java create mode 100644 util/src/main/java/tech/pegasys/pantheon/util/LimitedSet.java create mode 100644 util/src/test/java/tech/pegasys/pantheon/util/LimitedSetTest.java diff --git a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ThreadPantheonNodeRunner.java b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ThreadPantheonNodeRunner.java index 436deaad2b..237a0ae405 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ThreadPantheonNodeRunner.java +++ b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ThreadPantheonNodeRunner.java @@ -32,7 +32,6 @@ import java.io.IOException; import java.nio.file.Path; import java.time.Clock; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -108,7 +107,6 @@ public void startNode(final PantheonNode node) { .jsonRpcConfiguration(node.jsonRpcConfiguration()) .webSocketConfiguration(node.webSocketConfiguration()) .dataDir(node.homeDirectory()) - .bannedNodeIds(Collections.emptySet()) .metricsSystem(noOpMetricsSystem) .metricsConfiguration(node.metricsConfiguration()) .p2pEnabled(node.isP2pEnabled()) diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TestNode.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TestNode.java index 9809f8bb53..ec8948b5cb 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TestNode.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TestNode.java @@ -44,7 +44,6 @@ import tech.pegasys.pantheon.ethereum.p2p.network.DefaultP2PNetwork; import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer; import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; -import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; import tech.pegasys.pantheon.ethereum.p2p.wire.messages.DisconnectMessage.DisconnectReason; import tech.pegasys.pantheon.ethereum.worldstate.WorldStateArchive; import tech.pegasys.pantheon.metrics.MetricsSystem; @@ -126,7 +125,6 @@ public TestNode( .vertx(vertx) .keyPair(this.kp) .config(networkingConfiguration) - .peerBlacklist(new PeerBlacklist()) .metricsSystem(new NoOpMetricsSystem()) .supportedCapabilities(capabilities) .build()) diff --git a/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/JsonRpcHttpServiceRpcApisTest.java b/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/JsonRpcHttpServiceRpcApisTest.java index f3868ca990..b017c70799 100644 --- a/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/JsonRpcHttpServiceRpcApisTest.java +++ b/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/JsonRpcHttpServiceRpcApisTest.java @@ -37,7 +37,6 @@ import tech.pegasys.pantheon.ethereum.p2p.config.NetworkingConfiguration; import tech.pegasys.pantheon.ethereum.p2p.config.RlpxConfiguration; import tech.pegasys.pantheon.ethereum.p2p.network.DefaultP2PNetwork; -import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; import tech.pegasys.pantheon.ethereum.p2p.wire.Capability; import tech.pegasys.pantheon.ethereum.permissioning.AccountLocalConfigPermissioningController; import tech.pegasys.pantheon.ethereum.permissioning.NodeLocalConfigPermissioningController; @@ -243,7 +242,6 @@ private P2PNetwork createP2pNetwork() { .keyPair(SECP256K1.KeyPair.generate()) .vertx(vertx) .config(config) - .peerBlacklist(new PeerBlacklist()) .metricsSystem(new NoOpMetricsSystem()) .build(); diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/DiscoveryPeer.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/DiscoveryPeer.java index 148644d472..8185449b64 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/DiscoveryPeer.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/DiscoveryPeer.java @@ -31,6 +31,7 @@ public class DiscoveryPeer extends DefaultPeer { private long firstDiscovered = 0; private long lastContacted = 0; private long lastSeen = 0; + private long lastAttemptedConnection = 0; private DiscoveryPeer(final EnodeURL enode, final Endpoint endpoint) { super(enode); @@ -88,6 +89,14 @@ public void setLastContacted(final long lastContacted) { this.lastContacted = lastContacted; } + public long getLastAttemptedConnection() { + return lastAttemptedConnection; + } + + public void setLastAttemptedConnection(final long lastAttemptedConnection) { + this.lastAttemptedConnection = lastAttemptedConnection; + } + public long getLastSeen() { return lastSeen; } diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgent.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgent.java index 72e4bfd725..c44a5bd256 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgent.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgent.java @@ -14,25 +14,19 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; -import static java.util.concurrent.TimeUnit.MILLISECONDS; import static tech.pegasys.pantheon.util.bytes.BytesValue.wrapBuffer; import tech.pegasys.pantheon.crypto.SECP256K1; -import tech.pegasys.pantheon.ethereum.p2p.api.DisconnectCallback; -import tech.pegasys.pantheon.ethereum.p2p.api.PeerConnection; import tech.pegasys.pantheon.ethereum.p2p.config.DiscoveryConfiguration; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryEvent.PeerBondedEvent; -import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryEvent.PeerDroppedEvent; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.Packet; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerDiscoveryController; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerDiscoveryController.AsyncExecutor; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerRequirement; -import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerTable; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PingPacketData; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.TimerUtil; -import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeerId; -import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; -import tech.pegasys.pantheon.ethereum.p2p.wire.messages.DisconnectMessage; +import tech.pegasys.pantheon.ethereum.p2p.peers.PeerId; +import tech.pegasys.pantheon.ethereum.p2p.permissions.PeerPermissions; import tech.pegasys.pantheon.ethereum.permissioning.node.NodePermissioningController; import tech.pegasys.pantheon.metrics.MetricsSystem; import tech.pegasys.pantheon.util.NetworkUtility; @@ -47,7 +41,6 @@ import java.util.OptionalInt; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.TimeUnit; import java.util.function.Consumer; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -61,17 +54,16 @@ * The peer discovery agent is the network component that sends and receives peer discovery messages * via UDP. */ -public abstract class PeerDiscoveryAgent implements DisconnectCallback { +public abstract class PeerDiscoveryAgent { protected static final Logger LOG = LogManager.getLogger(); // The devp2p specification says only accept packets up to 1280, but some // clients ignore that, so we add in a little extra padding. private static final int MAX_PACKET_SIZE_BYTES = 1600; - private static final long PEER_REFRESH_INTERVAL_MS = MILLISECONDS.convert(30, TimeUnit.MINUTES); protected final List bootstrapPeers; private final List peerRequirements = new CopyOnWriteArrayList<>(); - private final PeerBlacklist peerBlacklist; + private final PeerPermissions peerPermissions; private final Optional nodePermissioningController; private final MetricsSystem metricsSystem; /* The peer controller, which takes care of the state machine of peers. */ @@ -80,7 +72,6 @@ public abstract class PeerDiscoveryAgent implements DisconnectCallback { /* The keypair used to sign messages. */ protected final SECP256K1.KeyPair keyPair; private final BytesValue id; - private final PeerTable peerTable; protected final DiscoveryConfiguration config; /* This is the {@link tech.pegasys.pantheon.ethereum.p2p.Peer} object holding who we are. */ @@ -89,12 +80,11 @@ public abstract class PeerDiscoveryAgent implements DisconnectCallback { /* Is discovery enabled? */ private boolean isActive = false; private final Subscribers> peerBondedObservers = new Subscribers<>(); - private final Subscribers> peerDroppedObservers = new Subscribers<>(); public PeerDiscoveryAgent( final SECP256K1.KeyPair keyPair, final DiscoveryConfiguration config, - final PeerBlacklist peerBlacklist, + final PeerPermissions peerPermissions, final Optional nodePermissioningController, final MetricsSystem metricsSystem) { this.metricsSystem = metricsSystem; @@ -103,14 +93,13 @@ public PeerDiscoveryAgent( validateConfiguration(config); - this.peerBlacklist = peerBlacklist; + this.peerPermissions = peerPermissions; this.nodePermissioningController = nodePermissioningController; this.bootstrapPeers = config.getBootnodes().stream().map(DiscoveryPeer::fromEnode).collect(Collectors.toList()); this.config = config; this.keyPair = keyPair; - this.peerTable = new PeerTable(keyPair.getPublicKey().getEncodedBytes(), 16); id = keyPair.getPublicKey().getEncodedBytes(); } @@ -164,21 +153,19 @@ private void startController() { } private PeerDiscoveryController createController() { - return new PeerDiscoveryController( - keyPair, - advertisedPeer, - peerTable, - bootstrapPeers, - this::handleOutgoingPacket, - createTimer(), - createWorkerExecutor(), - PEER_REFRESH_INTERVAL_MS, - PeerRequirement.combine(peerRequirements), - peerBlacklist, - nodePermissioningController, - peerBondedObservers, - peerDroppedObservers, - metricsSystem); + return PeerDiscoveryController.builder() + .keypair(keyPair) + .localPeer(advertisedPeer) + .bootstrapNodes(bootstrapPeers) + .outboundMessageHandler(this::handleOutgoingPacket) + .timerUtil(createTimer()) + .workerExecutor(createWorkerExecutor()) + .peerRequirement(PeerRequirement.combine(peerRequirements)) + .peerPermissions(peerPermissions) + .nodePermissioningController(nodePermissioningController) + .peerBondedObservers(peerBondedObservers) + .metricsSystem(metricsSystem) + .build(); } protected boolean validatePacketSize(final int packetSize) { @@ -240,6 +227,10 @@ public Stream streamDiscoveredPeers() { return controller.map(PeerDiscoveryController::streamDiscoveredPeers).orElse(Stream.empty()); } + public void dropPeer(final PeerId peer) { + controller.ifPresent(c -> c.dropPeer(peer)); + } + public Optional getAdvertisedPeer() { return Optional.ofNullable(advertisedPeer); } @@ -272,29 +263,6 @@ public boolean removePeerBondedObserver(final long observerId) { return peerBondedObservers.unsubscribe(observerId); } - /** - * Adds an observer that will get called when a peer is dropped from the peer table. - * - *

No guarantees are made about the order in which observers are invoked. - * - * @param observer The observer to call. - * @return A unique ID identifying this observer, to that it can be removed later. - */ - public long observePeerDroppedEvents(final Consumer observer) { - checkNotNull(observer); - return peerDroppedObservers.subscribe(observer); - } - - /** - * Removes an previously added peer dropped observer. - * - * @param observerId The unique ID identifying the observer to remove. - * @return Whether the observer was located and removed. - */ - public boolean removePeerDroppedObserver(final long observerId) { - return peerDroppedObservers.unsubscribe(observerId); - } - /** * Returns the count of observers that are registered on this controller. * @@ -320,15 +288,6 @@ private static void validateConfiguration(final DiscoveryConfiguration config) { checkArgument(config.getBucketSize() > 0, "bucket size cannot be negative nor zero"); } - @Override - public void onDisconnect( - final PeerConnection connection, - final DisconnectMessage.DisconnectReason reason, - final boolean initiatedByPeer) { - final BytesValue nodeId = connection.getPeerInfo().getNodeId(); - peerTable.tryEvict(new DefaultPeerId(nodeId)); - } - /** * Returns the current state of the PeerDiscoveryAgent. * diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/VertxPeerDiscoveryAgent.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/VertxPeerDiscoveryAgent.java index 6b307ae0a9..9de51be986 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/VertxPeerDiscoveryAgent.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/VertxPeerDiscoveryAgent.java @@ -21,7 +21,7 @@ import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerDiscoveryController.AsyncExecutor; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.TimerUtil; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.VertxTimerUtil; -import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; +import tech.pegasys.pantheon.ethereum.p2p.permissions.PeerPermissions; import tech.pegasys.pantheon.ethereum.permissioning.node.NodePermissioningController; import tech.pegasys.pantheon.metrics.MetricCategory; import tech.pegasys.pantheon.metrics.MetricsSystem; @@ -58,10 +58,10 @@ public VertxPeerDiscoveryAgent( final Vertx vertx, final KeyPair keyPair, final DiscoveryConfiguration config, - final PeerBlacklist peerBlacklist, + final PeerPermissions peerPermissions, final Optional nodePermissioningController, final MetricsSystem metricsSystem) { - super(keyPair, config, peerBlacklist, nodePermissioningController, metricsSystem); + super(keyPair, config, peerPermissions, nodePermissioningController, metricsSystem); checkArgument(vertx != null, "vertx instance cannot be null"); this.vertx = vertx; diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/OutboundMessageHandler.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/OutboundMessageHandler.java index c04085c646..0c43da994f 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/OutboundMessageHandler.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/OutboundMessageHandler.java @@ -16,7 +16,7 @@ @FunctionalInterface public interface OutboundMessageHandler { - public static OutboundMessageHandler NOOP = (peer, packet) -> {}; + OutboundMessageHandler NOOP = (peer, packet) -> {}; void send(final DiscoveryPeer toPeer, final Packet packet); } 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 81b4ffb7b5..fc896853ea 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 @@ -12,6 +12,9 @@ */ package tech.pegasys.pantheon.ethereum.p2p.discovery.internal; +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.SECONDS; import static tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerTable.AddResult.AddOutcome; @@ -21,12 +24,10 @@ import tech.pegasys.pantheon.ethereum.p2p.discovery.DiscoveryPeer; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryEvent; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryEvent.PeerBondedEvent; -import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryEvent.PeerDroppedEvent; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryStatus; -import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerTable.EvictResult; -import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerTable.EvictResult.EvictOutcome; import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; -import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; +import tech.pegasys.pantheon.ethereum.p2p.peers.PeerId; +import tech.pegasys.pantheon.ethereum.p2p.permissions.PeerPermissions; import tech.pegasys.pantheon.ethereum.permissioning.node.NodePermissioningController; import tech.pegasys.pantheon.metrics.Counter; import tech.pegasys.pantheon.metrics.LabelledMetric; @@ -35,6 +36,7 @@ import tech.pegasys.pantheon.util.Subscribers; import tech.pegasys.pantheon.util.bytes.BytesValue; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.List; @@ -43,6 +45,7 @@ import java.util.OptionalLong; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; import java.util.function.Predicate; @@ -99,7 +102,6 @@ * condition, the peer will be physically dropped (eliminated) from the table. */ public class PeerDiscoveryController { - private static final Logger LOG = LogManager.getLogger(); private static final long REFRESH_CHECK_INTERVAL_MILLIS = MILLISECONDS.convert(30, SECONDS); private static final int PEER_REFRESH_ROUND_TIMEOUT_IN_SECONDS = 5; @@ -118,7 +120,7 @@ public class PeerDiscoveryController { // The peer representation of this node private final DiscoveryPeer localPeer; private final OutboundMessageHandler outboundMessageHandler; - private final PeerBlacklist peerBlacklist; + private final PeerPermissions peerPermissions; private final Optional nodePermissioningController; private final DiscoveryProtocolLogger discoveryProtocolLogger; private final LabelledMetric interactionCounter; @@ -137,11 +139,10 @@ public class PeerDiscoveryController { // Observers for "peer bonded" discovery events. private final Subscribers> peerBondedObservers; - private final Subscribers> peerDroppedObservers; private RecursivePeerRefreshState recursivePeerRefreshState; - public PeerDiscoveryController( + private PeerDiscoveryController( final KeyPair keypair, final DiscoveryPeer localPeer, final PeerTable peerTable, @@ -151,10 +152,9 @@ public PeerDiscoveryController( final AsyncExecutor workerExecutor, final long tableRefreshIntervalMs, final PeerRequirement peerRequirement, - final PeerBlacklist peerBlacklist, + final PeerPermissions peerPermissions, final Optional nodePermissioningController, final Subscribers> peerBondedObservers, - final Subscribers> peerDroppedObservers, final MetricsSystem metricsSystem) { this.timerUtil = timerUtil; this.keypair = keypair; @@ -164,11 +164,10 @@ public PeerDiscoveryController( this.workerExecutor = workerExecutor; this.tableRefreshIntervalMs = tableRefreshIntervalMs; this.peerRequirement = peerRequirement; - this.peerBlacklist = peerBlacklist; + this.peerPermissions = peerPermissions; this.nodePermissioningController = nodePermissioningController; this.outboundMessageHandler = outboundMessageHandler; this.peerBondedObservers = peerBondedObservers; - this.peerDroppedObservers = peerDroppedObservers; this.discoveryProtocolLogger = new DiscoveryProtocolLogger(metricsSystem); metricsSystem.createIntegerGauge( @@ -192,6 +191,10 @@ public PeerDiscoveryController( "type"); } + public static Builder builder() { + return new Builder(); + } + public void start() { if (!started.compareAndSet(false, true)) { throw new IllegalStateException("The peer table had already been started"); @@ -199,22 +202,23 @@ public void start() { final List initialDiscoveryPeers = bootstrapNodes.stream() - .filter(p -> isPeerPermitted(localPeer, p)) + .filter(this::isPeerPermittedToReceiveMessages) .collect(Collectors.toList()); initialDiscoveryPeers.stream().forEach(peerTable::tryAdd); recursivePeerRefreshState = new RecursivePeerRefreshState( - peerBlacklist, - nodePermissioningController, this::bond, this::findNodes, timerUtil, localPeer, peerTable, + this::isPeerPermittedToReceiveMessages, PEER_REFRESH_ROUND_TIMEOUT_IN_SECONDS, 100); + peerPermissions.subscribeUpdate(this::handlePermissionsUpdate); + if (nodePermissioningController.isPresent()) { // if smart contract permissioning is enabled, bond with bootnodes @@ -252,10 +256,33 @@ public CompletableFuture stop() { return CompletableFuture.completedFuture(null); } - private boolean isPeerPermitted(final Peer sourcePeer, final Peer destinationPeer) { - return nodePermissioningController - .map(c -> c.isPermitted(sourcePeer.getEnodeURL(), destinationPeer.getEnodeURL())) - .orElse(true); + private boolean isPeerPermittedToReceiveMessages(final Peer remotePeer) { + return peerPermissions.isPermitted(remotePeer) + && nodePermissioningController + .map(c -> c.isPermitted(localPeer.getEnodeURL(), remotePeer.getEnodeURL())) + .orElse(true); + } + + private boolean isPeerPermittedToSendMessages(final Peer remotePeer) { + return peerPermissions.isPermitted(remotePeer) + && nodePermissioningController + .map(c -> c.isPermitted(remotePeer.getEnodeURL(), localPeer.getEnodeURL())) + .orElse(true); + } + + private void handlePermissionsUpdate( + final boolean addRestrictions, final Optional> affectedPeers) { + if (!addRestrictions) { + // Nothing to do if permissions were relaxed + return; + } + + // If we have an explicit list of peers, drop each peer from our discovery table + affectedPeers.ifPresent(peers -> peers.forEach(this::dropPeer)); + } + + public void dropPeer(final PeerId peer) { + peerTable.tryEvict(peer); } /** @@ -277,8 +304,8 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { return; } - if (!isPeerPermitted(sender, localPeer)) { - LOG.trace("Dropping packet from peer not in the whitelist ({})", sender.getEnodeURLString()); + if (!isPeerPermittedToSendMessages(sender)) { + LOG.trace("Dropping packet from disallowed peer ({})", sender.getEnodeURLString()); return; } @@ -286,11 +313,10 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { final Optional maybeKnownPeer = peerTable.get(sender); final DiscoveryPeer peer = maybeKnownPeer.orElse(sender); final boolean peerKnown = maybeKnownPeer.isPresent(); - final boolean peerBlacklisted = peerBlacklist.contains(peer); switch (packet.getType()) { case PING: - if (!peerBlacklisted && addToPeerTable(peer)) { + if (addToPeerTable(peer)) { final PingPacketData ping = packet.getPacketData(PingPacketData.class).get(); respondToPing(ping, packet.getHash(), peer); } @@ -299,9 +325,6 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { matchInteraction(packet) .ifPresent( interaction -> { - if (peerBlacklisted) { - return; - } addToPeerTable(peer); recursivePeerRefreshState.onBondingComplete(peer); }); @@ -314,7 +337,7 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { peer, getPeersFromNeighborsPacket(packet))); break; case FIND_NEIGHBORS: - if (!peerKnown || peerBlacklisted) { + if (!peerKnown) { break; } final FindNeighborsPacketData fn = @@ -367,27 +390,11 @@ private boolean addToPeerTable(final DiscoveryPeer peer) { return true; } - @VisibleForTesting - boolean dropFromPeerTable(final DiscoveryPeer peer) { - final EvictResult evictResult = peerTable.tryEvict(peer); - if (evictResult.getOutcome() == EvictOutcome.EVICTED) { - notifyPeerDropped(peer, System.currentTimeMillis()); - return true; - } else { - return false; - } - } - private void notifyPeerBonded(final DiscoveryPeer peer, final long now) { final PeerBondedEvent event = new PeerBondedEvent(peer, now); dispatchEvent(peerBondedObservers, event); } - private void notifyPeerDropped(final DiscoveryPeer peer, final long now) { - final PeerDroppedEvent event = new PeerDroppedEvent(peer, now); - dispatchEvent(peerDroppedObservers, event); - } - private Optional matchInteraction(final Packet packet) { final PeerInteractionState interaction = inflightInteractions.get(packet.getNodeId()); if (interaction == null || !interaction.test(packet)) { @@ -641,4 +648,140 @@ void cancelTimers() { public interface AsyncExecutor { CompletableFuture execute(Supplier action); } + + public static class Builder { + // Options with default values + private OutboundMessageHandler outboundMessageHandler = OutboundMessageHandler.NOOP; + private PeerRequirement peerRequirement = PeerRequirement.NOOP; + private PeerPermissions peerPermissions = PeerPermissions.noop(); + private long tableRefreshIntervalMs = MILLISECONDS.convert(30, TimeUnit.MINUTES); + private List bootstrapNodes = new ArrayList<>(); + private Optional nodePermissioningController = Optional.empty(); + private PeerTable peerTable; + private Subscribers> peerBondedObservers = new Subscribers<>(); + + // Required dependencies + private KeyPair keypair; + private DiscoveryPeer localPeer; + private TimerUtil timerUtil; + private AsyncExecutor workerExecutor; + private MetricsSystem metricsSystem; + + private Builder() {} + + public PeerDiscoveryController build() { + validate(); + + if (peerTable == null) { + peerTable = new PeerTable(this.keypair.getPublicKey().getEncodedBytes(), 16); + } + + return new PeerDiscoveryController( + keypair, + localPeer, + peerTable, + bootstrapNodes, + outboundMessageHandler, + timerUtil, + workerExecutor, + tableRefreshIntervalMs, + peerRequirement, + peerPermissions, + nodePermissioningController, + peerBondedObservers, + metricsSystem); + } + + private void validate() { + validateRequiredDependency(keypair, "KeyPair"); + validateRequiredDependency(localPeer, "LocalPeer"); + validateRequiredDependency(timerUtil, "TimerUtil"); + validateRequiredDependency(workerExecutor, "AsyncExecutor"); + validateRequiredDependency(metricsSystem, "MetricsSystem"); + validateRequiredDependency(peerBondedObservers, "PeerBondedObservers"); + } + + private void validateRequiredDependency(final Object object, final String name) { + checkState(object != null, name + " must be configured."); + } + + public Builder keypair(final KeyPair keypair) { + checkNotNull(keypair); + this.keypair = keypair; + return this; + } + + public Builder localPeer(final DiscoveryPeer localPeer) { + checkNotNull(localPeer); + this.localPeer = localPeer; + return this; + } + + public Builder peerTable(final PeerTable peerTable) { + checkNotNull(peerTable); + this.peerTable = peerTable; + return this; + } + + public Builder bootstrapNodes(final Collection bootstrapNodes) { + this.bootstrapNodes.addAll(bootstrapNodes); + return this; + } + + public Builder outboundMessageHandler(final OutboundMessageHandler outboundMessageHandler) { + checkNotNull(outboundMessageHandler); + this.outboundMessageHandler = outboundMessageHandler; + return this; + } + + public Builder timerUtil(final TimerUtil timerUtil) { + checkNotNull(timerUtil); + this.timerUtil = timerUtil; + return this; + } + + public Builder workerExecutor(final AsyncExecutor workerExecutor) { + checkNotNull(workerExecutor); + this.workerExecutor = workerExecutor; + return this; + } + + public Builder tableRefreshIntervalMs(final long tableRefreshIntervalMs) { + checkArgument(tableRefreshIntervalMs >= 0); + this.tableRefreshIntervalMs = tableRefreshIntervalMs; + return this; + } + + public Builder peerRequirement(final PeerRequirement peerRequirement) { + checkNotNull(peerRequirement); + this.peerRequirement = peerRequirement; + return this; + } + + public Builder peerPermissions(final PeerPermissions peerPermissions) { + checkNotNull(peerPermissions); + this.peerPermissions = peerPermissions; + return this; + } + + public Builder nodePermissioningController( + final Optional nodePermissioningController) { + checkNotNull(nodePermissioningController); + this.nodePermissioningController = nodePermissioningController; + return this; + } + + public Builder peerBondedObservers( + final Subscribers> peerBondedObservers) { + checkNotNull(peerBondedObservers); + this.peerBondedObservers = peerBondedObservers; + return this; + } + + public Builder metricsSystem(final MetricsSystem metricsSystem) { + checkNotNull(metricsSystem); + this.metricsSystem = metricsSystem; + return this; + } + } } diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerRequirement.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerRequirement.java index 8f21fd42cf..7398324a4b 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerRequirement.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerRequirement.java @@ -17,6 +17,8 @@ @FunctionalInterface public interface PeerRequirement { + PeerRequirement NOOP = () -> true; + boolean hasSufficientPeers(); static PeerRequirement combine(final Collection peerRequirements) { diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/RecursivePeerRefreshState.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/RecursivePeerRefreshState.java index 6efd328297..15c02c882a 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/RecursivePeerRefreshState.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/RecursivePeerRefreshState.java @@ -16,8 +16,7 @@ import tech.pegasys.pantheon.ethereum.p2p.discovery.DiscoveryPeer; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryStatus; -import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; -import tech.pegasys.pantheon.ethereum.permissioning.node.NodePermissioningController; +import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; import tech.pegasys.pantheon.util.bytes.BytesValue; import java.util.List; @@ -38,8 +37,7 @@ public class RecursivePeerRefreshState { private static final Logger LOG = LogManager.getLogger(); private static final int MAX_CONCURRENT_REQUESTS = 3; private BytesValue target; - private final PeerBlacklist peerBlacklist; - private final Optional nodePermissioningController; + private final OutboundDiscoveryMessagingPermissions peerPermissions; private final PeerTable peerTable; private final DiscoveryPeer localPeer; @@ -58,22 +56,20 @@ public class RecursivePeerRefreshState { List initialPeers; RecursivePeerRefreshState( - final PeerBlacklist peerBlacklist, - final Optional nodePermissioningController, final BondingAgent bondingAgent, final FindNeighbourDispatcher neighborFinder, final TimerUtil timerUtil, final DiscoveryPeer localPeer, final PeerTable peerTable, + final OutboundDiscoveryMessagingPermissions peerPermissions, final int timeoutPeriodInSeconds, final int maxRounds) { - this.peerBlacklist = peerBlacklist; - this.nodePermissioningController = nodePermissioningController; this.bondingAgent = bondingAgent; this.findNeighbourDispatcher = neighborFinder; this.timerUtil = timerUtil; this.localPeer = localPeer; this.peerTable = peerTable; + this.peerPermissions = peerPermissions; this.timeoutPeriodInSeconds = timeoutPeriodInSeconds; this.maxRounds = maxRounds; } @@ -186,18 +182,11 @@ private void neighboursCancelOutstandingRequests() { private boolean satisfiesMapAdditionCriteria(final DiscoveryPeer discoPeer) { return !oneTrueMap.containsKey(discoPeer.getId()) - && !peerBlacklist.contains(discoPeer) - && isPeerPermitted(discoPeer) + && peerPermissions.isPermitted(discoPeer) && (initialPeers.contains(discoPeer) || !peerTable.get(discoPeer).isPresent()) && !discoPeer.getId().equals(localPeer.getId()); } - private Boolean isPeerPermitted(final DiscoveryPeer discoPeer) { - return nodePermissioningController - .map(controller -> controller.isPermitted(localPeer.getEnodeURL(), discoPeer.getEnodeURL())) - .orElse(true); - } - void onNeighboursReceived(final DiscoveryPeer peer, final List peers) { final MetadataPeer metadataPeer = oneTrueMap.get(peer.getId()); if (metadataPeer == null) { @@ -390,4 +379,9 @@ public void cancelTimeout() { timeoutCancelled.set(true); } } + + @FunctionalInterface + public interface OutboundDiscoveryMessagingPermissions { + boolean isPermitted(Peer remotePeer); + } } diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/DefaultP2PNetwork.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/DefaultP2PNetwork.java index d3a2e819a2..7206b49e7e 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/DefaultP2PNetwork.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/DefaultP2PNetwork.java @@ -18,7 +18,6 @@ import tech.pegasys.pantheon.crypto.SECP256K1; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; -import tech.pegasys.pantheon.ethereum.chain.BlockAddedEvent; import tech.pegasys.pantheon.ethereum.chain.Blockchain; import tech.pegasys.pantheon.ethereum.p2p.api.DisconnectCallback; import tech.pegasys.pantheon.ethereum.p2p.api.Message; @@ -28,7 +27,6 @@ import tech.pegasys.pantheon.ethereum.p2p.discovery.DiscoveryPeer; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryAgent; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryEvent.PeerBondedEvent; -import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryEvent.PeerDroppedEvent; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryStatus; import tech.pegasys.pantheon.ethereum.p2p.discovery.VertxPeerDiscoveryAgent; import tech.pegasys.pantheon.ethereum.p2p.network.netty.Callbacks; @@ -37,7 +35,8 @@ import tech.pegasys.pantheon.ethereum.p2p.network.netty.PeerConnectionRegistry; import tech.pegasys.pantheon.ethereum.p2p.network.netty.TimeoutHandler; import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; -import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; +import tech.pegasys.pantheon.ethereum.p2p.permissions.PeerPermissions; +import tech.pegasys.pantheon.ethereum.p2p.permissions.PeerPermissionsBlacklist; import tech.pegasys.pantheon.ethereum.p2p.wire.Capability; import tech.pegasys.pantheon.ethereum.p2p.wire.PeerInfo; import tech.pegasys.pantheon.ethereum.p2p.wire.SubProtocol; @@ -54,7 +53,7 @@ import java.net.InetSocketAddress; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; +import java.util.Comparator; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -154,7 +153,7 @@ public class DefaultP2PNetwork implements P2PNetwork { private volatile Optional localEnode = Optional.empty(); private volatile Optional ourPeerInfo = Optional.empty(); - private final PeerBlacklist peerBlacklist; + private final PeerPermissions peerPermissions; private final Optional nodePermissioningController; private final Optional blockchain; @@ -173,7 +172,6 @@ public class DefaultP2PNetwork implements P2PNetwork { private final LabelledMetric outboundMessagesCounter; private OptionalLong blockAddedObserverId = OptionalLong.empty(); private OptionalLong peerBondedObserverId = OptionalLong.empty(); - private OptionalLong peerDroppedObserverId = OptionalLong.empty(); private final AtomicBoolean started = new AtomicBoolean(false); private final AtomicBoolean stopped = new AtomicBoolean(false); @@ -189,7 +187,7 @@ public class DefaultP2PNetwork implements P2PNetwork { * @param keyPair This node's keypair. * @param config The network configuration to use. * @param supportedCapabilities The wire protocol capabilities to advertise to connected peers. - * @param peerBlacklist The peers with which this node will not connect + * @param peerPermissions An object that determines whether peers are allowed to connect * @param metricsSystem The metrics system to capture metrics with. * @param nodePermissioningController Controls node permissioning. * @param blockchain The blockchain to subscribe to BlockAddedEvents. @@ -199,7 +197,7 @@ public class DefaultP2PNetwork implements P2PNetwork { final SECP256K1.KeyPair keyPair, final NetworkingConfiguration config, final List supportedCapabilities, - final PeerBlacklist peerBlacklist, + final PeerPermissions peerPermissions, final MetricsSystem metricsSystem, final Optional nodePermissioningController, final Blockchain blockchain) { @@ -208,7 +206,6 @@ public class DefaultP2PNetwork implements P2PNetwork { this.keyPair = keyPair; this.config = config; this.supportedCapabilities = supportedCapabilities; - this.peerBlacklist = peerBlacklist; this.nodePermissioningController = nodePermissioningController; this.blockchain = Optional.ofNullable(blockchain); this.peerMaintainConnectionList = new HashSet<>(); @@ -218,12 +215,16 @@ public class DefaultP2PNetwork implements P2PNetwork { this.subProtocols = config.getSupportedProtocols(); this.maxPeers = config.getRlpx().getMaxPeers(); + // Set up permissions + final PeerPermissionsBlacklist misbehavingPeers = PeerPermissionsBlacklist.create(500); + PeerReputationManager reputationManager = new PeerReputationManager(misbehavingPeers); + this.peerPermissions = PeerPermissions.combine(peerPermissions, misbehavingPeers); + peerDiscoveryAgent.addPeerRequirement(() -> connections.size() >= maxPeers); this.nodePermissioningController.ifPresent( c -> c.subscribeToUpdates(this::checkCurrentConnections)); - subscribeDisconnect(peerDiscoveryAgent); - subscribeDisconnect(peerBlacklist); + subscribeDisconnect(reputationManager); subscribeDisconnect(connections); outboundMessagesCounter = @@ -348,6 +349,7 @@ protected void initChannel(final SocketChannel ch) { if (!isPeerAllowed(connection)) { connection.disconnect(DisconnectReason.UNKNOWN); + peerDiscoveryAgent.dropPeer(connection.getPeer()); return; } @@ -386,6 +388,8 @@ public boolean removeMaintainedConnectionPeer(final Peer peer) { final Optional peerConnection = connections.getConnectionForPeer(peer.getId()); peerConnection.ifPresent(pc -> pc.disconnect(DisconnectReason.REQUESTED)); + peerDiscoveryAgent.dropPeer(peer); + return removed; } @@ -406,9 +410,11 @@ void attemptPeerConnections() { final List peers = streamDiscoveredPeers() .filter(peer -> peer.getStatus() == PeerDiscoveryStatus.BONDED) + .filter(this::isPeerAllowed) .filter(peer -> !isConnected(peer) && !isConnecting(peer)) + .sorted(Comparator.comparing(DiscoveryPeer::getLastAttemptedConnection)) .collect(Collectors.toList()); - Collections.shuffle(peers); + if (peers.size() == 0) { return; } @@ -459,6 +465,9 @@ public CompletableFuture connect(final Peer peer) { return connectionFuture; } + if (peer instanceof DiscoveryPeer) { + ((DiscoveryPeer) peer).setLastAttemptedConnection(System.currentTimeMillis()); + } new Bootstrap() .group(workers) .channel(NioSocketChannel.class) @@ -544,15 +553,14 @@ public void start() { peerDiscoveryAgent.start(listeningPort).join(); peerBondedObserverId = OptionalLong.of(peerDiscoveryAgent.observePeerBondedEvents(handlePeerBondedEvent())); - peerDroppedObserverId = - OptionalLong.of(peerDiscoveryAgent.observePeerDroppedEvents(handlePeerDroppedEvents())); if (nodePermissioningController.isPresent()) { if (blockchain.isPresent()) { synchronized (this) { if (!blockAddedObserverId.isPresent()) { blockAddedObserverId = - OptionalLong.of(blockchain.get().observeBlockAdded(this::handleBlockAddedEvent)); + OptionalLong.of( + blockchain.get().observeBlockAdded((evt, chain) -> checkCurrentConnections())); } } } else { @@ -579,28 +587,6 @@ Consumer handlePeerBondedEvent() { }; } - private Consumer handlePeerDroppedEvents() { - return event -> { - final Peer peer = event.getPeer(); - getPeers().stream() - .filter(p -> p.getPeerInfo().getNodeId().equals(peer.getId())) - .findFirst() - .ifPresent(p -> p.disconnect(DisconnectReason.REQUESTED)); - }; - } - - private synchronized void handleBlockAddedEvent( - final BlockAddedEvent event, final Blockchain blockchain) { - connections - .getPeerConnections() - .forEach( - peerConnection -> { - if (!isPeerAllowed(peerConnection)) { - peerConnection.disconnect(DisconnectReason.REQUESTED); - } - }); - } - private synchronized void checkCurrentConnections() { connections .getPeerConnections() @@ -608,19 +594,16 @@ private synchronized void checkCurrentConnections() { peerConnection -> { if (!isPeerAllowed(peerConnection)) { peerConnection.disconnect(DisconnectReason.REQUESTED); + peerDiscoveryAgent.dropPeer(peerConnection.getPeer()); } }); } private boolean isPeerAllowed(final PeerConnection conn) { - return isPeerAllowed(conn.getRemoteEnode()); + return isPeerAllowed(conn.getPeer()); } private boolean isPeerAllowed(final Peer peer) { - return isPeerAllowed(peer.getEnodeURL()); - } - - private boolean isPeerAllowed(final EnodeURL enode) { final Optional maybeEnode = getLocalEnode(); if (!maybeEnode.isPresent()) { // If the network isn't ready yet, deny connections @@ -628,15 +611,17 @@ private boolean isPeerAllowed(final EnodeURL enode) { } final EnodeURL localEnode = maybeEnode.get(); - if (peerBlacklist.contains(enode.getNodeId())) { + if (peer.getId().equals(nodeId)) { + // Peer matches our node id return false; } - if (enode.getNodeId().equals(nodeId)) { - // Peer matches our node id + if (!peerPermissions.isPermitted(peer)) { return false; } - return nodePermissioningController.map(c -> c.isPermitted(localEnode, enode)).orElse(true); + return nodePermissioningController + .map(c -> c.isPermitted(localEnode, peer.getEnodeURL())) + .orElse(true); } @VisibleForTesting @@ -665,8 +650,6 @@ public void stop() { peerDiscoveryAgent.stop().join(); peerBondedObserverId.ifPresent(peerDiscoveryAgent::removePeerBondedObserver); peerBondedObserverId = OptionalLong.empty(); - peerDroppedObserverId.ifPresent(peerDiscoveryAgent::removePeerDroppedObserver); - peerDroppedObserverId = OptionalLong.empty(); blockchain.ifPresent(b -> blockAddedObserverId.ifPresent(b::removeObserver)); blockAddedObserverId = OptionalLong.empty(); peerDiscoveryAgent.stop().join(); @@ -747,7 +730,7 @@ public static class Builder { private KeyPair keyPair; private NetworkingConfiguration config = NetworkingConfiguration.create(); private List supportedCapabilities; - private PeerBlacklist peerBlacklist; + private PeerPermissions peerPermissions = PeerPermissions.noop(); private MetricsSystem metricsSystem; private Optional nodePermissioningController = Optional.empty(); private Blockchain blockchain = null; @@ -766,7 +749,7 @@ private P2PNetwork doBuild() { keyPair, config, supportedCapabilities, - peerBlacklist, + peerPermissions, metricsSystem, nodePermissioningController, blockchain); @@ -778,7 +761,6 @@ private void validate() { checkState( supportedCapabilities != null && supportedCapabilities.size() > 0, "Supported capabilities must be set and non-empty."); - checkState(peerBlacklist != null, "PeerBlacklist must be set."); checkState(metricsSystem != null, "MetricsSystem must be set."); checkState( !nodePermissioningController.isPresent() || blockchain != null, @@ -792,7 +774,7 @@ private PeerDiscoveryAgent createDiscoveryAgent() { vertx, keyPair, config.getDiscovery(), - peerBlacklist, + peerPermissions, nodePermissioningController, metricsSystem); } @@ -826,9 +808,9 @@ public Builder supportedCapabilities(final Capability... supportedCapabilities) return this; } - public Builder peerBlacklist(final PeerBlacklist peerBlacklist) { - checkNotNull(peerBlacklist); - this.peerBlacklist = peerBlacklist; + public Builder peerPermissions(final PeerPermissions peerPermissions) { + checkNotNull(peerPermissions); + this.peerPermissions = peerPermissions; return this; } diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/PeerReputationManager.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/PeerReputationManager.java new file mode 100644 index 0000000000..07faeb49fe --- /dev/null +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/PeerReputationManager.java @@ -0,0 +1,52 @@ +/* + * Copyright 2018 ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package tech.pegasys.pantheon.ethereum.p2p.network; + +import tech.pegasys.pantheon.ethereum.p2p.api.DisconnectCallback; +import tech.pegasys.pantheon.ethereum.p2p.api.PeerConnection; +import tech.pegasys.pantheon.ethereum.p2p.permissions.PeerPermissionsBlacklist; +import tech.pegasys.pantheon.ethereum.p2p.wire.messages.DisconnectMessage.DisconnectReason; + +import java.util.Set; + +import com.google.common.collect.ImmutableSet; + +public class PeerReputationManager implements DisconnectCallback { + private static final Set locallyTriggeredDisconnectReasons = + ImmutableSet.of( + DisconnectReason.BREACH_OF_PROTOCOL, DisconnectReason.INCOMPATIBLE_P2P_PROTOCOL_VERSION); + + private static final Set remotelyTriggeredDisconnectReasons = + ImmutableSet.of(DisconnectReason.INCOMPATIBLE_P2P_PROTOCOL_VERSION); + + private final PeerPermissionsBlacklist blacklist; + + public PeerReputationManager(final PeerPermissionsBlacklist blacklist) { + this.blacklist = blacklist; + } + + @Override + public void onDisconnect( + final PeerConnection connection, + final DisconnectReason reason, + final boolean initiatedByPeer) { + if (shouldBlock(reason, initiatedByPeer)) { + blacklist.add(connection.getPeer()); + } + } + + private boolean shouldBlock(final DisconnectReason reason, final boolean initiatedByPeer) { + return (!initiatedByPeer && locallyTriggeredDisconnectReasons.contains(reason)) + || (initiatedByPeer && remotelyTriggeredDisconnectReasons.contains(reason)); + } +} diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/PeerBlacklist.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/PeerBlacklist.java deleted file mode 100644 index 9a84be454f..0000000000 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/PeerBlacklist.java +++ /dev/null @@ -1,117 +0,0 @@ -/* - * Copyright 2018 ConsenSys AG. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ -package tech.pegasys.pantheon.ethereum.p2p.peers; - -import tech.pegasys.pantheon.ethereum.p2p.api.DisconnectCallback; -import tech.pegasys.pantheon.ethereum.p2p.api.PeerConnection; -import tech.pegasys.pantheon.ethereum.p2p.wire.messages.DisconnectMessage; -import tech.pegasys.pantheon.ethereum.p2p.wire.messages.DisconnectMessage.DisconnectReason; -import tech.pegasys.pantheon.util.bytes.BytesValue; - -import java.util.Collections; -import java.util.LinkedHashMap; -import java.util.Map; -import java.util.Set; - -import com.google.common.collect.ImmutableSet; - -/** - * A list of nodes that the running client will not communicate with. This can be because of network - * issues, protocol issues, or by being explicitly set on the command line. - * - *

Peers are stored and identified strictly by their nodeId, the convenience methods taking - * {@link Peer}s and {@link PeerConnection}s redirect to the methods that take {@link BytesValue} - * object that represent the node ID of the banned nodes. - * - *

The storage list is not infinite. A default cap of 500 is applied and nodes are removed on a - * first added first removed basis. Adding a new copy of the same node will not affect the priority - * for removal. The exception to this is a list of banned nodes passed in by reference to the - * constructor. This list neither adds nor removes from that list passed in. - */ -public class PeerBlacklist implements DisconnectCallback { - private static final int DEFAULT_BLACKLIST_CAP = 500; - - private static final Set locallyTriggeredBlacklistReasons = - ImmutableSet.of( - DisconnectReason.BREACH_OF_PROTOCOL, DisconnectReason.INCOMPATIBLE_P2P_PROTOCOL_VERSION); - - private static final Set remotelyTriggeredBlacklistReasons = - ImmutableSet.of(DisconnectReason.INCOMPATIBLE_P2P_PROTOCOL_VERSION); - - private final int blacklistCap; - private final Set blacklistedNodeIds = - Collections.synchronizedSet( - Collections.newSetFromMap( - new LinkedHashMap(20, 0.75f, true) { - @Override - protected boolean removeEldestEntry(final Map.Entry eldest) { - return size() > blacklistCap; - } - })); - - /** These nodes are always banned for the life of this list. They are not subject to rollover. */ - private final Set bannedNodeIds; - - public PeerBlacklist(final int blacklistCap, final Set bannedNodeIds) { - this.blacklistCap = blacklistCap; - this.bannedNodeIds = bannedNodeIds; - } - - public PeerBlacklist(final int blacklistCap) { - this(blacklistCap, Collections.emptySet()); - } - - public PeerBlacklist(final Set bannedNodeIds) { - this(DEFAULT_BLACKLIST_CAP, bannedNodeIds); - } - - public PeerBlacklist() { - this(DEFAULT_BLACKLIST_CAP, Collections.emptySet()); - } - - public boolean contains(final BytesValue nodeId) { - return blacklistedNodeIds.contains(nodeId) || bannedNodeIds.contains(nodeId); - } - - public boolean contains(final PeerConnection peer) { - return contains(peer.getPeerInfo().getNodeId()); - } - - public boolean contains(final Peer peer) { - return contains(peer.getId()); - } - - public void add(final Peer peer) { - add(peer.getId()); - } - - public void add(final BytesValue peerId) { - blacklistedNodeIds.add(peerId); - } - - @Override - public void onDisconnect( - final PeerConnection connection, - final DisconnectReason reason, - final boolean initiatedByPeer) { - if (shouldBlacklistForDisconnect(reason, initiatedByPeer)) { - add(connection.getPeerInfo().getNodeId()); - } - } - - private boolean shouldBlacklistForDisconnect( - final DisconnectMessage.DisconnectReason reason, final boolean initiatedByPeer) { - return (!initiatedByPeer && locallyTriggeredBlacklistReasons.contains(reason)) - || (initiatedByPeer && remotelyTriggeredBlacklistReasons.contains(reason)); - } -} diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/permissions/PeerPermissions.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/permissions/PeerPermissions.java new file mode 100644 index 0000000000..f728df2862 --- /dev/null +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/permissions/PeerPermissions.java @@ -0,0 +1,111 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package tech.pegasys.pantheon.ethereum.p2p.permissions; + +import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; +import tech.pegasys.pantheon.util.Subscribers; + +import java.util.Arrays; +import java.util.List; +import java.util.Optional; +import java.util.stream.Stream; + +import com.google.common.collect.ImmutableList; + +public abstract class PeerPermissions { + private final Subscribers updateSubscribers = new Subscribers<>(); + + public static final PeerPermissions NOOP = new NoopPeerPermissions(); + + public static PeerPermissions noop() { + return NOOP; + } + + public static PeerPermissions combine(final PeerPermissions... permissions) { + return combine(Arrays.asList(permissions)); + } + + public static PeerPermissions combine(final List permissions) { + return CombinedPeerPermissions.create(permissions); + } + + /** + * @param peer The {@link Peer} object representing the remote node + * @return True if we are allowed to communicate with this peer. + */ + public abstract boolean isPermitted(final Peer peer); + + public void subscribeUpdate(final PermissionsUpdateCallback callback) { + updateSubscribers.subscribe(callback); + } + + protected void dispatchUpdate( + final boolean permissionsRestricted, final Optional> affectedPeers) { + updateSubscribers.forEach(s -> s.onUpdate(permissionsRestricted, affectedPeers)); + } + + private static class NoopPeerPermissions extends PeerPermissions { + @Override + public boolean isPermitted(final Peer peer) { + return true; + } + } + + private static class CombinedPeerPermissions extends PeerPermissions { + private final ImmutableList permissions; + + private CombinedPeerPermissions(final ImmutableList permissions) { + this.permissions = permissions; + } + + public static PeerPermissions create(final List permissions) { + final ImmutableList filteredPermissions = + permissions.stream() + .flatMap( + p -> { + if (p instanceof CombinedPeerPermissions) { + return ((CombinedPeerPermissions) p).permissions.stream(); + } else { + return Stream.of(p); + } + }) + .filter(p -> !(p instanceof NoopPeerPermissions)) + .collect(ImmutableList.toImmutableList()); + + if (filteredPermissions.size() == 0) { + return PeerPermissions.NOOP; + } else if (filteredPermissions.size() == 1) { + return filteredPermissions.get(0); + } else { + return new CombinedPeerPermissions(filteredPermissions); + } + } + + @Override + public void subscribeUpdate(final PermissionsUpdateCallback callback) { + for (final PeerPermissions permission : permissions) { + permission.subscribeUpdate(callback); + } + } + + @Override + public boolean isPermitted(final Peer peer) { + for (PeerPermissions permission : permissions) { + if (!permission.isPermitted(peer)) { + return false; + } + } + return true; + } + } +} diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/permissions/PeerPermissionsBlacklist.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/permissions/PeerPermissionsBlacklist.java new file mode 100644 index 0000000000..94e55f58a1 --- /dev/null +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/permissions/PeerPermissionsBlacklist.java @@ -0,0 +1,81 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package tech.pegasys.pantheon.ethereum.p2p.permissions; + +import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; +import tech.pegasys.pantheon.util.LimitedSet; +import tech.pegasys.pantheon.util.LimitedSet.Mode; +import tech.pegasys.pantheon.util.bytes.BytesValue; + +import java.util.Collections; +import java.util.Optional; +import java.util.OptionalInt; +import java.util.Set; + +import io.vertx.core.impl.ConcurrentHashSet; + +public class PeerPermissionsBlacklist extends PeerPermissions { + private static int DEFAULT_INITIAL_CAPACITY = 20; + + private final Set blacklist; + + private PeerPermissionsBlacklist(final int initialCapacity, final OptionalInt maxSize) { + if (maxSize.isPresent()) { + blacklist = + LimitedSet.create(initialCapacity, maxSize.getAsInt(), Mode.DROP_LEAST_RECENTLY_ACCESSED); + } else { + blacklist = new ConcurrentHashSet<>(initialCapacity); + } + } + + private PeerPermissionsBlacklist(final OptionalInt maxSize) { + this(DEFAULT_INITIAL_CAPACITY, maxSize); + } + + public static PeerPermissionsBlacklist create() { + return new PeerPermissionsBlacklist(OptionalInt.empty()); + } + + public static PeerPermissionsBlacklist create(final int maxSize) { + return new PeerPermissionsBlacklist(OptionalInt.of(maxSize)); + } + + @Override + public boolean isPermitted(final Peer peer) { + return !blacklist.contains(peer.getId()); + } + + public void add(final Peer peer) { + if (blacklist.add(peer.getId())) { + dispatchUpdate(true, Optional.of(Collections.singletonList(peer))); + } + } + + public void remove(final Peer peer) { + if (blacklist.remove(peer.getId())) { + dispatchUpdate(false, Optional.of(Collections.singletonList(peer))); + } + } + + public void add(final BytesValue peerId) { + if (blacklist.add(peerId)) { + dispatchUpdate(true, Optional.empty()); + } + } + + public void remove(final BytesValue peerId) { + if (blacklist.remove(peerId)) { + dispatchUpdate(false, Optional.empty()); + } + } +} diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/permissions/PermissionsUpdateCallback.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/permissions/PermissionsUpdateCallback.java new file mode 100644 index 0000000000..9cd1cb6bed --- /dev/null +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/permissions/PermissionsUpdateCallback.java @@ -0,0 +1,33 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package tech.pegasys.pantheon.ethereum.p2p.permissions; + +import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; + +import java.util.List; +import java.util.Optional; + +public interface PermissionsUpdateCallback { + + /** + * @param permissionsRestricted True if permissions were narrowed in any way, meaning that + * previously permitted peers may no longer be permitted. False indicates that permissions + * were made less restrictive, meaning peers that were previously restricted may now be + * permitted. + * @param affectedPeers If non-empty, contains the entire set of peers affected by this + * permissions update. If permissions were restricted, this is the list of peers that are no + * longer permitted. If permissions were broadened, this is the list of peers that are now + * permitted. + */ + void onUpdate(final boolean permissionsRestricted, final Optional> affectedPeers); +} diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java index 55d26a23c8..0d93294970 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java @@ -15,10 +15,7 @@ import static java.util.stream.Collectors.toList; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; -import tech.pegasys.pantheon.ethereum.p2p.api.PeerConnection; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryTestHelper.AgentBuilder; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.FindNeighborsPacketData; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.MockPeerDiscoveryAgent; @@ -27,10 +24,7 @@ import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.Packet; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PacketType; import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; -import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; -import tech.pegasys.pantheon.ethereum.p2p.wire.PeerInfo; -import tech.pegasys.pantheon.ethereum.p2p.wire.messages.DisconnectMessage.DisconnectReason; -import tech.pegasys.pantheon.util.bytes.BytesValue; +import tech.pegasys.pantheon.ethereum.p2p.permissions.PeerPermissionsBlacklist; import tech.pegasys.pantheon.util.enode.EnodeURL; import java.util.Collections; @@ -141,49 +135,24 @@ public void neighborsPacketLimited() { } @Test - public void shouldEvictPeerOnDisconnect() { + public void shouldEvictPeerWhenPermissionsRevoked() { + final PeerPermissionsBlacklist blacklist = PeerPermissionsBlacklist.create(); final MockPeerDiscoveryAgent peerDiscoveryAgent1 = helper.startDiscoveryAgent(); peerDiscoveryAgent1.start(BROADCAST_TCP_PORT).join(); final DiscoveryPeer peer = peerDiscoveryAgent1.getAdvertisedPeer().get(); - final MockPeerDiscoveryAgent peerDiscoveryAgent2 = helper.startDiscoveryAgent(peer); + final MockPeerDiscoveryAgent peerDiscoveryAgent2 = + helper.startDiscoveryAgent( + helper.agentBuilder().peerPermissions(blacklist).bootstrapPeers(peer)); peerDiscoveryAgent2.start(BROADCAST_TCP_PORT).join(); assertThat(peerDiscoveryAgent2.streamDiscoveredPeers().collect(toList()).size()).isEqualTo(1); - final PeerConnection peerConnection = createAnonymousPeerConnection(peer.getId()); - peerDiscoveryAgent2.onDisconnect(peerConnection, DisconnectReason.REQUESTED, true); + blacklist.add(peer); assertThat(peerDiscoveryAgent2.streamDiscoveredPeers().collect(toList()).size()).isEqualTo(0); } - @Test - public void doesNotBlacklistPeerForNormalDisconnect() { - // Start an agent with no bootstrap peers. - final PeerBlacklist blacklist = new PeerBlacklist(); - final MockPeerDiscoveryAgent agent = - helper.startDiscoveryAgent(Collections.emptyList(), blacklist); - // Setup peer - final MockPeerDiscoveryAgent otherNode = helper.startDiscoveryAgent(); - final PeerConnection wirePeer = createAnonymousPeerConnection(otherNode.getId()); - - // Bond to peer - bondViaIncomingPing(agent, otherNode); - assertThat(agent.streamDiscoveredPeers()).hasSize(1); - - // Disconnect with innocuous reason - blacklist.onDisconnect(wirePeer, DisconnectReason.TOO_MANY_PEERS, false); - agent.onDisconnect(wirePeer, DisconnectReason.TOO_MANY_PEERS, false); - // Confirm peer was removed - assertThat(agent.streamDiscoveredPeers()).hasSize(0); - - // Bond again - bondViaIncomingPing(agent, otherNode); - - // Check peer was allowed to connect - assertThat(agent.streamDiscoveredPeers()).hasSize(1); - } - protected void bondViaIncomingPing( final MockPeerDiscoveryAgent agent, final MockPeerDiscoveryAgent otherNode) { final Packet pingPacket = helper.createPingPacket(otherNode, agent); @@ -191,107 +160,17 @@ protected void bondViaIncomingPing( } @Test - public void blacklistPeerForBadBehavior() { + public void dontBondWithNonPermittedPeer() { // Start an agent with no bootstrap peers. - final PeerBlacklist blacklist = new PeerBlacklist(); + final PeerPermissionsBlacklist blacklist = PeerPermissionsBlacklist.create(); final MockPeerDiscoveryAgent agent = helper.startDiscoveryAgent(Collections.emptyList(), blacklist); // Setup peer final MockPeerDiscoveryAgent otherNode = helper.startDiscoveryAgent(); - final PeerConnection wirePeer = createAnonymousPeerConnection(otherNode.getId()); - - // Bond to peer - bondViaIncomingPing(agent, otherNode); - assertThat(agent.streamDiscoveredPeers()).hasSize(1); - // Disconnect with problematic reason - blacklist.onDisconnect(wirePeer, DisconnectReason.BREACH_OF_PROTOCOL, false); - agent.onDisconnect(wirePeer, DisconnectReason.BREACH_OF_PROTOCOL, false); - // Confirm peer was removed - assertThat(agent.streamDiscoveredPeers()).hasSize(0); + blacklist.add(otherNode.getId()); - // Bond again - bondViaIncomingPing(agent, otherNode); - - // Check peer was not allowed to connect - assertThat(agent.streamDiscoveredPeers()).hasSize(0); - } - - @Test - public void doesNotBlacklistPeerForOurBadBehavior() throws Exception { - // Start an agent with no bootstrap peers. - final PeerBlacklist blacklist = new PeerBlacklist(); - final MockPeerDiscoveryAgent agent = - helper.startDiscoveryAgent(Collections.emptyList(), blacklist); - // Setup peer - final MockPeerDiscoveryAgent otherNode = helper.startDiscoveryAgent(); - final PeerConnection wirePeer = createAnonymousPeerConnection(otherNode.getId()); - - // Bond to peer - bondViaIncomingPing(agent, otherNode); - assertThat(agent.streamDiscoveredPeers()).hasSize(1); - - // Disconnect with problematic reason - blacklist.onDisconnect(wirePeer, DisconnectReason.BREACH_OF_PROTOCOL, true); - agent.onDisconnect(wirePeer, DisconnectReason.BREACH_OF_PROTOCOL, true); - // Confirm peer was removed - assertThat(agent.streamDiscoveredPeers()).hasSize(0); - - // Bond again - bondViaIncomingPing(agent, otherNode); - - // Check peer was allowed to connect - assertThat(agent.streamDiscoveredPeers()).hasSize(1); - } - - @Test - public void blacklistIncompatiblePeer() throws Exception { - // Start an agent with no bootstrap peers. - final PeerBlacklist blacklist = new PeerBlacklist(); - final MockPeerDiscoveryAgent agent = - helper.startDiscoveryAgent(Collections.emptyList(), blacklist); - // Setup peer - final MockPeerDiscoveryAgent otherNode = helper.startDiscoveryAgent(); - final PeerConnection wirePeer = createAnonymousPeerConnection(otherNode.getId()); - - // Bond to peer - bondViaIncomingPing(agent, otherNode); - assertThat(agent.streamDiscoveredPeers()).hasSize(1); - - // Disconnect - blacklist.onDisconnect(wirePeer, DisconnectReason.INCOMPATIBLE_P2P_PROTOCOL_VERSION, false); - agent.onDisconnect(wirePeer, DisconnectReason.INCOMPATIBLE_P2P_PROTOCOL_VERSION, false); - // Confirm peer was removed - assertThat(agent.streamDiscoveredPeers()).hasSize(0); - - // Bond again - bondViaIncomingPing(agent, otherNode); - - // Check peer was not allowed to connect - assertThat(agent.streamDiscoveredPeers()).hasSize(0); - } - - @Test - public void blacklistIncompatiblePeerWhoIssuesDisconnect() throws Exception { - // Start an agent with no bootstrap peers. - final PeerBlacklist blacklist = new PeerBlacklist(); - final MockPeerDiscoveryAgent agent = - helper.startDiscoveryAgent(Collections.emptyList(), blacklist); - // Setup peer - final MockPeerDiscoveryAgent otherNode = helper.startDiscoveryAgent(); - final PeerConnection wirePeer = createAnonymousPeerConnection(otherNode.getId()); - - // Bond to peer - bondViaIncomingPing(agent, otherNode); - assertThat(agent.streamDiscoveredPeers()).hasSize(1); - - // Disconnect - blacklist.onDisconnect(wirePeer, DisconnectReason.INCOMPATIBLE_P2P_PROTOCOL_VERSION, true); - agent.onDisconnect(wirePeer, DisconnectReason.INCOMPATIBLE_P2P_PROTOCOL_VERSION, true); - // Confirm peer was removed - assertThat(agent.streamDiscoveredPeers()).hasSize(0); - - // Bond again + // Bond bondViaIncomingPing(agent, otherNode); // Check peer was not allowed to connect @@ -313,11 +192,4 @@ public void shouldNotBeActiveWhenConfigIsFalse() { assertThat(agent.isActive()).isFalse(); } - - private PeerConnection createAnonymousPeerConnection(final BytesValue id) { - final PeerConnection conn = mock(PeerConnection.class); - final PeerInfo peerInfo = new PeerInfo(0, null, null, 0, id); - when(conn.getPeerInfo()).thenReturn(peerInfo); - return conn; - } } diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java index ad9264061c..3ed39eb33a 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java @@ -22,7 +22,7 @@ import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PacketType; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PingPacketData; import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; -import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; +import tech.pegasys.pantheon.ethereum.p2p.permissions.PeerPermissions; import tech.pegasys.pantheon.ethereum.permissioning.node.NodePermissioningController; import tech.pegasys.pantheon.util.bytes.BytesValue; import tech.pegasys.pantheon.util.enode.EnodeURL; @@ -139,13 +139,13 @@ public MockPeerDiscoveryAgent startDiscoveryAgent(final DiscoveryPeer... bootstr * Start a single discovery agent with the provided bootstrap peers. * * @param bootstrapPeers the list of bootstrap peers - * @param blacklist the peer blacklist + * @param peerPermissions peer permissions * @return a list of discovery agents. */ public MockPeerDiscoveryAgent startDiscoveryAgent( - final List bootstrapPeers, final PeerBlacklist blacklist) { + final List bootstrapPeers, final PeerPermissions peerPermissions) { final AgentBuilder agentBuilder = - agentBuilder().bootstrapPeers(bootstrapPeers).blacklist(blacklist); + agentBuilder().bootstrapPeers(bootstrapPeers).peerPermissions(peerPermissions); return startDiscoveryAgent(agentBuilder); } @@ -178,10 +178,10 @@ public static class AgentBuilder { private final Map agents; private final AtomicInteger nextAvailablePort; - private PeerBlacklist blacklist = new PeerBlacklist(); private Optional nodePermissioningController = Optional.empty(); private List bootnodes = Collections.emptyList(); private boolean active = true; + private PeerPermissions peerPermissions = PeerPermissions.noop(); private AgentBuilder( final Map agents, @@ -213,8 +213,8 @@ public AgentBuilder nodePermissioningController(final NodePermissioningControlle return this; } - public AgentBuilder blacklist(final PeerBlacklist blacklist) { - this.blacklist = blacklist; + public AgentBuilder peerPermissions(final PeerPermissions peerPermissions) { + this.peerPermissions = peerPermissions; return this; } @@ -230,7 +230,11 @@ public MockPeerDiscoveryAgent build() { config.setActive(active); return new MockPeerDiscoveryAgent( - SECP256K1.KeyPair.generate(), config, blacklist, nodePermissioningController, agents); + SECP256K1.KeyPair.generate(), + config, + peerPermissions, + nodePermissioningController, + agents); } } } diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTimestampsTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTimestampsTest.java index cc845ff247..2dd15356ca 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTimestampsTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTimestampsTest.java @@ -20,13 +20,11 @@ import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.BlockingAsyncExecutor; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.MockPeerDiscoveryAgent; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.MockTimerUtil; -import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.OutboundMessageHandler; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.Packet; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PacketType; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerDiscoveryController; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerTable; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PingPacketData; -import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; import tech.pegasys.pantheon.metrics.noop.NoOpMetricsSystem; import tech.pegasys.pantheon.util.Subscribers; @@ -53,21 +51,16 @@ public void lastSeenAndFirstDiscoveredTimestampsUpdatedOnMessage() { final KeyPair localKeyPair = keypairs.get(0); final PeerDiscoveryController controller = - new PeerDiscoveryController( - localKeyPair, - localPeer, - new PeerTable(agent.getAdvertisedPeer().get().getId()), - Collections.emptyList(), - OutboundMessageHandler.NOOP, - new MockTimerUtil(), - new BlockingAsyncExecutor(), - TimeUnit.HOURS.toMillis(1), - () -> true, - new PeerBlacklist(), - Optional.empty(), - new Subscribers<>(), - new Subscribers<>(), - new NoOpMetricsSystem()); + PeerDiscoveryController.builder() + .keypair(localKeyPair) + .localPeer(localPeer) + .peerTable(new PeerTable(agent.getAdvertisedPeer().get().getId())) + .timerUtil(new MockTimerUtil()) + .workerExecutor(new BlockingAsyncExecutor()) + .tableRefreshIntervalMs(TimeUnit.HOURS.toMillis(1)) + .peerBondedObservers(new Subscribers<>()) + .metricsSystem(new NoOpMetricsSystem()) + .build(); controller.start(); final PingPacketData ping = diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/MockPeerDiscoveryAgent.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/MockPeerDiscoveryAgent.java index 508bd4998d..477595420f 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/MockPeerDiscoveryAgent.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/MockPeerDiscoveryAgent.java @@ -17,7 +17,7 @@ import tech.pegasys.pantheon.ethereum.p2p.discovery.DiscoveryPeer; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryAgent; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerDiscoveryController.AsyncExecutor; -import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; +import tech.pegasys.pantheon.ethereum.p2p.permissions.PeerPermissions; import tech.pegasys.pantheon.ethereum.permissioning.node.NodePermissioningController; import tech.pegasys.pantheon.metrics.noop.NoOpMetricsSystem; import tech.pegasys.pantheon.util.bytes.BytesValue; @@ -39,10 +39,10 @@ public class MockPeerDiscoveryAgent extends PeerDiscoveryAgent { public MockPeerDiscoveryAgent( final KeyPair keyPair, final DiscoveryConfiguration config, - final PeerBlacklist peerBlacklist, + final PeerPermissions peerPermissions, final Optional nodePermissioningController, final Map agentNetwork) { - super(keyPair, config, peerBlacklist, nodePermissioningController, new NoOpMetricsSystem()); + super(keyPair, config, peerPermissions, nodePermissioningController, new NoOpMetricsSystem()); this.agentNetwork = agentNetwork; } diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java index 714fece115..217bf0dcd2 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java @@ -26,7 +26,6 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import tech.pegasys.pantheon.crypto.SECP256K1; @@ -35,12 +34,11 @@ import tech.pegasys.pantheon.ethereum.p2p.discovery.DiscoveryPeer; import tech.pegasys.pantheon.ethereum.p2p.discovery.Endpoint; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryEvent.PeerBondedEvent; -import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryEvent.PeerDroppedEvent; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryStatus; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryTestHelper; -import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerTable.EvictResult; import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; -import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; +import tech.pegasys.pantheon.ethereum.p2p.permissions.PeerPermissions; +import tech.pegasys.pantheon.ethereum.p2p.permissions.PeerPermissionsBlacklist; import tech.pegasys.pantheon.ethereum.permissioning.node.NodePermissioningController; import tech.pegasys.pantheon.metrics.noop.NoOpMetricsSystem; import tech.pegasys.pantheon.util.Subscribers; @@ -57,9 +55,7 @@ import java.util.Collections; import java.util.List; import java.util.Optional; -import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; import java.util.stream.Collectors; @@ -597,12 +593,12 @@ public void shouldNotAddNewPeerWhenReceivedPongFromBlacklistedPeer() { final DiscoveryPeer otherPeer = peers.get(1); final DiscoveryPeer otherPeer2 = peers.get(2); - final PeerBlacklist blacklist = new PeerBlacklist(); + final PeerPermissionsBlacklist blacklist = PeerPermissionsBlacklist.create(); final OutboundMessageHandler outboundMessageHandler = mock(OutboundMessageHandler.class); controller = getControllerBuilder() .peers(discoPeer) - .blacklist(blacklist) + .peerPermissions(blacklist) .outboundMessageHandler(outboundMessageHandler) .build(); @@ -675,12 +671,12 @@ public void shouldNotBondWithBlacklistedPeer() { final DiscoveryPeer otherPeer = peers.get(1); final DiscoveryPeer otherPeer2 = peers.get(2); - final PeerBlacklist blacklist = new PeerBlacklist(); + final PeerPermissionsBlacklist blacklist = PeerPermissionsBlacklist.create(); final OutboundMessageHandler outboundMessageHandler = mock(OutboundMessageHandler.class); controller = getControllerBuilder() .peers(discoPeer) - .blacklist(blacklist) + .peerPermissions(blacklist) .outboundMessageHandler(outboundMessageHandler) .build(); @@ -726,18 +722,15 @@ public void shouldNotBondWithBlacklistedPeer() { } @Test - public void shouldRespondToNeighborsRequestFromKnownPeer() - throws InterruptedException, ExecutionException, TimeoutException { + public void shouldRespondToNeighborsRequestFromKnownPeer() { final List peers = createPeersInLastBucket(localPeer, 1); final DiscoveryPeer discoPeer = peers.get(0); - final PeerBlacklist blacklist = new PeerBlacklist(); final OutboundMessageHandler outboundMessageHandler = mock(OutboundMessageHandler.class); controller = getControllerBuilder() .peers(discoPeer) - .blacklist(blacklist) .outboundMessageHandler(outboundMessageHandler) .build(); @@ -768,19 +761,16 @@ public void shouldRespondToNeighborsRequestFromKnownPeer() } @Test - public void shouldNotRespondToNeighborsRequestFromUnknownPeer() - throws InterruptedException, ExecutionException, TimeoutException { + public void shouldNotRespondToNeighborsRequestFromUnknownPeer() { final List peers = createPeersInLastBucket(localPeer, 2); final DiscoveryPeer discoPeer = peers.get(0); final DiscoveryPeer otherPeer = peers.get(1); - final PeerBlacklist blacklist = new PeerBlacklist(); final OutboundMessageHandler outboundMessageHandler = mock(OutboundMessageHandler.class); controller = getControllerBuilder() .peers(discoPeer) - .blacklist(blacklist) .outboundMessageHandler(outboundMessageHandler) .build(); @@ -816,12 +806,12 @@ public void shouldNotRespondToNeighborsRequestFromBlacklistedPeer() { final DiscoveryPeer discoPeer = peers.get(0); - final PeerBlacklist blacklist = new PeerBlacklist(); + final PeerPermissionsBlacklist blacklist = PeerPermissionsBlacklist.create(); final OutboundMessageHandler outboundMessageHandler = mock(OutboundMessageHandler.class); controller = getControllerBuilder() .peers(discoPeer) - .blacklist(blacklist) + .peerPermissions(blacklist) .outboundMessageHandler(outboundMessageHandler) .build(); @@ -990,8 +980,6 @@ public void shouldNotBondWithNonPermittedPeer() { final DiscoveryPeer notPermittedPeer = peers.get(1); final DiscoveryPeer permittedPeer = peers.get(2); - final PeerBlacklist blacklist = new PeerBlacklist(); - final NodePermissioningController nodePermissioningController = new NodePermissioningControllerTestHelper(localPeer) .withPermittedPeers(discoveryPeer, permittedPeer) @@ -1002,7 +990,6 @@ public void shouldNotBondWithNonPermittedPeer() { controller = getControllerBuilder() .peers(discoveryPeer) - .blacklist(blacklist) .nodePermissioningController(nodePermissioningController) .outboundMessageHandler(outboundMessageHandler) .build(); @@ -1065,27 +1052,6 @@ public void shouldNotRespondToPingFromNonWhitelistedDiscoveryPeer() { assertThat(controller.streamDiscoveredPeers()).doesNotContain(peers.get(0)); } - @Test - @SuppressWarnings({"unchecked", "rawtypes"}) - public void whenPeerIsNotEvictedDropFromTableShouldReturnFalseAndNotifyZeroObservers() { - final List peers = createPeersInLastBucket(localPeer, 1); - final DiscoveryPeer peer = peers.get(0); - final PeerTable peerTableSpy = spy(peerTable); - final Consumer peerDroppedEventConsumer = mock(Consumer.class); - final Subscribers> peerDroppedSubscribers = new Subscribers(); - peerDroppedSubscribers.subscribe(peerDroppedEventConsumer); - - doReturn(EvictResult.absent()).when(peerTableSpy).tryEvict(any()); - - controller = getControllerBuilder().peerDroppedObservers(peerDroppedSubscribers).build(); - - controller.start(); - final boolean dropped = controller.dropFromPeerTable(peer); - - assertThat(dropped).isFalse(); - verifyZeroInteractions(peerDroppedEventConsumer); - } - private static Packet mockPingPacket(final DiscoveryPeer from, final DiscoveryPeer to) { final Packet packet = mock(Packet.class); @@ -1147,7 +1113,6 @@ private PeerDiscoveryController startPeerDiscoveryController( static class ControllerBuilder { private Collection discoPeers = Collections.emptyList(); - private PeerBlacklist blacklist = new PeerBlacklist(); private Optional nodePermissioningController = Optional.empty(); private MockTimerUtil timerUtil = new MockTimerUtil(); private KeyPair keypair; @@ -1156,7 +1121,7 @@ static class ControllerBuilder { private OutboundMessageHandler outboundMessageHandler = OutboundMessageHandler.NOOP; private static final PeerDiscoveryTestHelper helper = new PeerDiscoveryTestHelper(); private Subscribers> peerBondedObservers = new Subscribers<>(); - private Subscribers> peerDroppedObservers = new Subscribers<>(); + private PeerPermissions peerPermissions = PeerPermissions.noop(); public static ControllerBuilder create() { return new ControllerBuilder(); @@ -1172,8 +1137,8 @@ ControllerBuilder peers(final DiscoveryPeer... discoPeers) { return this; } - ControllerBuilder blacklist(final PeerBlacklist blacklist) { - this.blacklist = blacklist; + ControllerBuilder peerPermissions(final PeerPermissions peerPermissions) { + this.peerPermissions = peerPermissions; return this; } @@ -1212,12 +1177,6 @@ ControllerBuilder peerBondedObservers(final Subscribers> observers) { - this.peerDroppedObservers = observers; - return this; - } - PeerDiscoveryController build() { checkNotNull(keypair); if (localPeer == null) { @@ -1227,21 +1186,21 @@ PeerDiscoveryController build() { peerTable = new PeerTable(localPeer.getId()); } return spy( - new PeerDiscoveryController( - keypair, - localPeer, - peerTable, - discoPeers, - outboundMessageHandler, - timerUtil, - new BlockingAsyncExecutor(), - TABLE_REFRESH_INTERVAL_MS, - PEER_REQUIREMENT, - blacklist, - nodePermissioningController, - peerBondedObservers, - peerDroppedObservers, - new NoOpMetricsSystem())); + PeerDiscoveryController.builder() + .keypair(keypair) + .localPeer(localPeer) + .peerTable(peerTable) + .bootstrapNodes(discoPeers) + .outboundMessageHandler(outboundMessageHandler) + .timerUtil(timerUtil) + .workerExecutor(new BlockingAsyncExecutor()) + .tableRefreshIntervalMs(TABLE_REFRESH_INTERVAL_MS) + .peerRequirement(PEER_REQUIREMENT) + .peerPermissions(peerPermissions) + .nodePermissioningController(nodePermissioningController) + .peerBondedObservers(peerBondedObservers) + .metricsSystem(new NoOpMetricsSystem()) + .build()); } } } diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryTableRefreshTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryTableRefreshTest.java index 7547c3d469..db1d8cd0f1 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryTableRefreshTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryTableRefreshTest.java @@ -12,7 +12,6 @@ */ package tech.pegasys.pantheon.ethereum.p2p.discovery.internal; -import static java.util.Collections.emptyList; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.atLeast; @@ -25,7 +24,6 @@ import tech.pegasys.pantheon.ethereum.p2p.discovery.DiscoveryPeer; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryStatus; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryTestHelper; -import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; import tech.pegasys.pantheon.metrics.noop.NoOpMetricsSystem; import tech.pegasys.pantheon.util.Subscribers; import tech.pegasys.pantheon.util.bytes.BytesValue; @@ -54,21 +52,17 @@ public void tableRefreshSingleNode() { final MockTimerUtil timer = new MockTimerUtil(); final PeerDiscoveryController controller = spy( - new PeerDiscoveryController( - localKeyPair, - localPeer, - new PeerTable(localPeer.getId()), - emptyList(), - outboundMessageHandler, - timer, - new BlockingAsyncExecutor(), - 0, - () -> true, - new PeerBlacklist(), - Optional.empty(), - new Subscribers<>(), - new Subscribers<>(), - new NoOpMetricsSystem())); + PeerDiscoveryController.builder() + .keypair(localKeyPair) + .localPeer(localPeer) + .peerTable(new PeerTable(localPeer.getId())) + .outboundMessageHandler(outboundMessageHandler) + .timerUtil(timer) + .workerExecutor(new BlockingAsyncExecutor()) + .tableRefreshIntervalMs(0) + .peerBondedObservers(new Subscribers<>()) + .metricsSystem(new NoOpMetricsSystem()) + .build()); controller.start(); // Send a PING, so as to add a Peer in the controller. diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/RecursivePeerRefreshStateTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/RecursivePeerRefreshStateTest.java index 4d1cc9f229..35fb727fea 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/RecursivePeerRefreshStateTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/RecursivePeerRefreshStateTest.java @@ -16,7 +16,6 @@ import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -28,23 +27,20 @@ import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryStatus; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.RecursivePeerRefreshState.BondingAgent; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.RecursivePeerRefreshState.FindNeighbourDispatcher; -import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; -import tech.pegasys.pantheon.ethereum.permissioning.LocalPermissioningConfiguration; -import tech.pegasys.pantheon.ethereum.permissioning.node.NodePermissioningController; +import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.RecursivePeerRefreshState.OutboundDiscoveryMessagingPermissions; import tech.pegasys.pantheon.util.bytes.BytesValue; import tech.pegasys.pantheon.util.enode.EnodeURL; -import java.nio.file.Files; -import java.nio.file.Path; import java.util.Collections; import java.util.List; -import java.util.Optional; +import org.junit.Before; import org.junit.Test; public class RecursivePeerRefreshStateTest { private static final BytesValue TARGET = createId(0); - private final PeerBlacklist peerBlacklist = mock(PeerBlacklist.class); + private final OutboundDiscoveryMessagingPermissions peerPermissions = + mock(OutboundDiscoveryMessagingPermissions.class); private final BondingAgent bondingAgent = mock(BondingAgent.class); private final FindNeighbourDispatcher neighborFinder = mock(FindNeighbourDispatcher.class); private final MockTimerUtil timerUtil = new MockTimerUtil(); @@ -57,16 +53,21 @@ public class RecursivePeerRefreshStateTest { private RecursivePeerRefreshState recursivePeerRefreshState = new RecursivePeerRefreshState( - peerBlacklist, - Optional.empty(), bondingAgent, neighborFinder, timerUtil, localPeer, new PeerTable(createId(999), 16), + peerPermissions, 5, 100); + @Before + public void setup() { + // Default peerPermissions to be permissive + when(peerPermissions.isPermitted(any())).thenReturn(true); + } + @Test public void shouldBondWithInitialNodesWhenStarted() { recursivePeerRefreshState.start(asList(peer1, peer2, peer3), TARGET); @@ -171,13 +172,12 @@ public void shouldStopWhenAllNodesHaveBeenQueried() { public void shouldStopWhenMaximumNumberOfRoundsReached() { recursivePeerRefreshState = new RecursivePeerRefreshState( - peerBlacklist, - Optional.empty(), bondingAgent, neighborFinder, timerUtil, localPeer, new PeerTable(createId(999), 16), + peerPermissions, 5, 1); @@ -431,22 +431,20 @@ public void shouldBondWithPeersInNeighboursResponseReceivedAfterTimeout() { } @Test - public void shouldNotBondWithNodesOnBlacklist() { + public void shouldNotBondWithNonPermittedNode() { final DiscoveryPeer peerA = createPeer(1, "127.0.0.1", 1, 1); final DiscoveryPeer peerB = createPeer(2, "127.0.0.2", 2, 2); - final PeerBlacklist blacklist = new PeerBlacklist(); - blacklist.add(peerB); + when(peerPermissions.isPermitted(peerB)).thenReturn(false); recursivePeerRefreshState = new RecursivePeerRefreshState( - blacklist, - Optional.empty(), bondingAgent, neighborFinder, timerUtil, localPeer, new PeerTable(createId(999), 16), + peerPermissions, 5, 100); recursivePeerRefreshState.start(singletonList(peerA), TARGET); @@ -481,48 +479,6 @@ public void shouldNotBondWithSelf() { verify(bondingAgent, never()).performBonding(localPeer); } - @Test - public void shouldNotBondWithNodesNotPermitted() throws Exception { - final DiscoveryPeer localPeer = createPeer(999, "127.0.0.9", 9, 9); - final DiscoveryPeer peerA = createPeer(1, "127.0.0.1", 1, 1); - final DiscoveryPeer peerB = createPeer(2, "127.0.0.2", 2, 2); - - final Path tempFile = Files.createTempFile("test", "test"); - tempFile.toFile().deleteOnExit(); - final LocalPermissioningConfiguration permissioningConfiguration = - LocalPermissioningConfiguration.createDefault(); - permissioningConfiguration.setNodePermissioningConfigFilePath( - tempFile.toAbsolutePath().toString()); - - final NodePermissioningController nodeWhitelistController = - mock(NodePermissioningController.class); - when(nodeWhitelistController.isPermitted(any(), eq(peerA.getEnodeURL()))).thenReturn(true); - when(nodeWhitelistController.isPermitted(any(), eq(peerB.getEnodeURL()))).thenReturn(false); - - recursivePeerRefreshState = - new RecursivePeerRefreshState( - peerBlacklist, - Optional.of(nodeWhitelistController), - bondingAgent, - neighborFinder, - timerUtil, - localPeer, - new PeerTable(createId(999), 16), - 5, - 100); - recursivePeerRefreshState.start(singletonList(peerA), TARGET); - - verify(bondingAgent).performBonding(peerA); - - completeBonding(peerA); - - verify(neighborFinder).findNeighbours(peerA, TARGET); - - recursivePeerRefreshState.onNeighboursReceived(peerA, Collections.singletonList(peerB)); - - verify(bondingAgent, never()).performBonding(peerB); - } - private static BytesValue createId(final int id) { return BytesValue.fromHexString(String.format("%0128x", id)); } diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/DefaultP2PNetworkTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/DefaultP2PNetworkTest.java index edf0c360cd..6526b70b56 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/DefaultP2PNetworkTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/DefaultP2PNetworkTest.java @@ -44,7 +44,6 @@ import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryStatus; import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer; import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; -import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; import tech.pegasys.pantheon.ethereum.p2p.wire.Capability; import tech.pegasys.pantheon.ethereum.p2p.wire.PeerInfo; import tech.pegasys.pantheon.ethereum.p2p.wire.SubProtocol; @@ -390,6 +389,7 @@ public void attemptPeerConnections_connectsToValidPeer() { final int maxPeers = 5; final DefaultP2PNetwork network = mockNetwork(() -> RlpxConfiguration.create().setMaxPeers(maxPeers)); + network.start(); doReturn(2).when(network).connectionCount(); DiscoveryPeer peer = createDiscoveryPeer(); @@ -411,6 +411,7 @@ public void attemptPeerConnections_ignoresUnbondedPeer() { final int maxPeers = 5; final DefaultP2PNetwork network = mockNetwork(() -> RlpxConfiguration.create().setMaxPeers(maxPeers)); + network.start(); doReturn(2).when(network).connectionCount(); DiscoveryPeer peer = createDiscoveryPeer(); @@ -427,6 +428,7 @@ public void attemptPeerConnections_ignoresConnectingPeer() { final int maxPeers = 5; final DefaultP2PNetwork network = mockNetwork(() -> RlpxConfiguration.create().setMaxPeers(maxPeers)); + network.start(); doReturn(2).when(network).connectionCount(); DiscoveryPeer peer = createDiscoveryPeer(); @@ -444,6 +446,7 @@ public void attemptPeerConnections_ignoresConnectedPeer() { final int maxPeers = 5; final DefaultP2PNetwork network = mockNetwork(() -> RlpxConfiguration.create().setMaxPeers(maxPeers)); + network.start(); doReturn(2).when(network).connectionCount(); DiscoveryPeer peer = createDiscoveryPeer(); @@ -461,6 +464,43 @@ public void attemptPeerConnections_withSlotsAvailable() { final int maxPeers = 5; final DefaultP2PNetwork network = mockNetwork(() -> RlpxConfiguration.create().setMaxPeers(maxPeers)); + network.start(); + + doReturn(2).when(network).connectionCount(); + final List peers = + Stream.iterate(1, n -> n + 1) + .limit(10) + .map( + (seed) -> { + DiscoveryPeer peer = createDiscoveryPeer(); + peer.setStatus(PeerDiscoveryStatus.BONDED); + return peer; + }) + .collect(Collectors.toList()); + + final List highestValuePeers = peers.subList(5, 8); + // Mark as high value by lowering the lastAttemptedConnection value + peers.forEach(p -> p.setLastAttemptedConnection(100)); + highestValuePeers.forEach(p -> p.setLastAttemptedConnection(1)); + + doReturn(peers.stream()).when(network).streamDiscoveredPeers(); + final ArgumentCaptor peerCapture = ArgumentCaptor.forClass(DiscoveryPeer.class); + doReturn(CompletableFuture.completedFuture(mock(PeerConnection.class))) + .when(network) + .connect(peerCapture.capture()); + + network.attemptPeerConnections(); + verify(network, times(3)).connect(any()); + assertThat(peers.containsAll(peerCapture.getAllValues())).isTrue(); + assertThat(peerCapture.getAllValues()).containsExactlyInAnyOrderElementsOf(highestValuePeers); + } + + @Test + public void attemptPeerConnections_withNonPermittedPeers() { + final int maxPeers = 5; + final DefaultP2PNetwork network = + mockNetwork(() -> RlpxConfiguration.create().setMaxPeers(maxPeers)); + network.start(); doReturn(2).when(network).connectionCount(); final List peers = @@ -474,6 +514,18 @@ public void attemptPeerConnections_withSlotsAvailable() { }) .collect(Collectors.toList()); + // Prioritize peers + final List highestValuePeers = peers.subList(5, 8); + peers.forEach(p -> p.setLastAttemptedConnection(100)); + highestValuePeers.forEach(p -> p.setLastAttemptedConnection(2)); + // Set up the highest value peer to lack permissions + DiscoveryPeer highestValueNonPermittedPeer = peers.get(0); + highestValueNonPermittedPeer.setLastAttemptedConnection(1); + when(nodePermissioningController.isPermitted(any(), any())).thenReturn(true); + when(nodePermissioningController.isPermitted( + any(), eq(highestValueNonPermittedPeer.getEnodeURL()))) + .thenReturn(false); + doReturn(peers.stream()).when(network).streamDiscoveredPeers(); final ArgumentCaptor peerCapture = ArgumentCaptor.forClass(DiscoveryPeer.class); doReturn(CompletableFuture.completedFuture(mock(PeerConnection.class))) @@ -483,6 +535,7 @@ public void attemptPeerConnections_withSlotsAvailable() { network.attemptPeerConnections(); verify(network, times(3)).connect(any()); assertThat(peers.containsAll(peerCapture.getAllValues())).isTrue(); + assertThat(peerCapture.getAllValues()).containsExactlyInAnyOrderElementsOf(highestValuePeers); } @Test @@ -490,6 +543,7 @@ public void attemptPeerConnections_withNoSlotsAvailable() { final int maxPeers = 5; final DefaultP2PNetwork network = mockNetwork(() -> RlpxConfiguration.create().setMaxPeers(maxPeers)); + network.start(); doReturn(maxPeers).when(network).connectionCount(); final List peers = @@ -542,6 +596,19 @@ public void connect_toNonListeningPeer() { "Attempt to connect to peer with no listening port: " + peer.getEnodeURLString()); } + @Test + public void connect_toDiscoveryPeerUpdatesStats() { + final DefaultP2PNetwork network = network(); + network.start(); + final DiscoveryPeer peer = createDiscoveryPeer(); + + assertThat(peer.getLastAttemptedConnection()).isEqualTo(0); + + final CompletableFuture result = network.connect(peer); + assertThat(result).isNotCompletedExceptionally(); + assertThat(peer.getLastAttemptedConnection()).isGreaterThan(0); + } + private DiscoveryPeer createDiscoveryPeer() { return createDiscoveryPeer(Peer.randomId(), 999); } @@ -564,6 +631,7 @@ private PeerConnection mockPeerConnection() { private PeerConnection mockPeerConnection(final Peer remotePeer) { final EnodeURL remoteEnode = remotePeer.getEnodeURL(); + final Peer peer = DefaultPeer.fromEnodeURL(remoteEnode); final PeerInfo peerInfo = new PeerInfo( 5, @@ -573,8 +641,9 @@ private PeerConnection mockPeerConnection(final Peer remotePeer) { remoteEnode.getNodeId()); final PeerConnection peerConnection = mock(PeerConnection.class); - when(peerConnection.getRemoteEnode()).thenReturn(remoteEnode); - when(peerConnection.getPeerInfo()).thenReturn(peerInfo); + lenient().when(peerConnection.getRemoteEnode()).thenReturn(remoteEnode); + lenient().when(peerConnection.getPeerInfo()).thenReturn(peerInfo); + lenient().when(peerConnection.getPeer()).thenReturn(peer); return peerConnection; } @@ -616,7 +685,6 @@ private DefaultP2PNetwork.Builder builder() { .vertx(vertx) .config(config) .keyPair(KeyPair.generate()) - .peerBlacklist(new PeerBlacklist()) .metricsSystem(new NoOpMetricsSystem()) .supportedCapabilities(Arrays.asList(Capability.create("eth", 63))); } diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/NetworkingServiceLifecycleTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/NetworkingServiceLifecycleTest.java index 0ac734a46f..bfe3b1615f 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/NetworkingServiceLifecycleTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/NetworkingServiceLifecycleTest.java @@ -23,7 +23,6 @@ import tech.pegasys.pantheon.ethereum.p2p.config.DiscoveryConfiguration; import tech.pegasys.pantheon.ethereum.p2p.config.NetworkingConfiguration; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryServiceException; -import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; import tech.pegasys.pantheon.ethereum.p2p.wire.Capability; import tech.pegasys.pantheon.metrics.noop.NoOpMetricsSystem; import tech.pegasys.pantheon.util.enode.EnodeURL; @@ -158,7 +157,6 @@ private DefaultP2PNetwork.Builder builder() { .vertx(vertx) .keyPair(keyPair) .config(config) - .peerBlacklist(new PeerBlacklist()) .metricsSystem(new NoOpMetricsSystem()) .supportedCapabilities(Arrays.asList(Capability.create("eth", 63))); } diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/P2PNetworkTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/P2PNetworkTest.java index b768dc2192..221bde8566 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/P2PNetworkTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/P2PNetworkTest.java @@ -28,7 +28,7 @@ import tech.pegasys.pantheon.ethereum.p2p.network.exceptions.IncompatiblePeerException; import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer; import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; -import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; +import tech.pegasys.pantheon.ethereum.p2p.permissions.PeerPermissionsBlacklist; import tech.pegasys.pantheon.ethereum.p2p.wire.Capability; import tech.pegasys.pantheon.ethereum.p2p.wire.SubProtocol; import tech.pegasys.pantheon.ethereum.p2p.wire.messages.DisconnectMessage.DisconnectReason; @@ -218,11 +218,10 @@ public void rejectPeerWithNoSharedCaps() throws Exception { @Test public void rejectIncomingConnectionFromBlacklistedPeer() throws Exception { - final PeerBlacklist localBlacklist = new PeerBlacklist(); - final PeerBlacklist remoteBlacklist = new PeerBlacklist(); + final PeerPermissionsBlacklist localBlacklist = PeerPermissionsBlacklist.create(); - try (final P2PNetwork localNetwork = builder().peerBlacklist(localBlacklist).build(); - final P2PNetwork remoteNetwork = builder().peerBlacklist(remoteBlacklist).build()) { + try (final P2PNetwork localNetwork = builder().peerPermissions(localBlacklist).build(); + final P2PNetwork remoteNetwork = builder().build()) { localNetwork.start(); remoteNetwork.start(); @@ -365,7 +364,6 @@ private DefaultP2PNetwork.Builder builder() { .vertx(vertx) .config(config) .keyPair(KeyPair.generate()) - .peerBlacklist(new PeerBlacklist()) .metricsSystem(new NoOpMetricsSystem()) .supportedCapabilities(Arrays.asList(Capability.create("eth", 63))); } diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/PeerReputationManagerTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/PeerReputationManagerTest.java new file mode 100644 index 0000000000..fab54dcfb7 --- /dev/null +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/PeerReputationManagerTest.java @@ -0,0 +1,107 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package tech.pegasys.pantheon.ethereum.p2p.network; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import tech.pegasys.pantheon.ethereum.p2p.api.PeerConnection; +import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer; +import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; +import tech.pegasys.pantheon.ethereum.p2p.permissions.PeerPermissionsBlacklist; +import tech.pegasys.pantheon.ethereum.p2p.wire.PeerInfo; +import tech.pegasys.pantheon.ethereum.p2p.wire.messages.DisconnectMessage.DisconnectReason; +import tech.pegasys.pantheon.util.bytes.BytesValue; +import tech.pegasys.pantheon.util.enode.EnodeURL; + +import org.junit.Test; + +public class PeerReputationManagerTest { + private final PeerReputationManager peerReputationManager; + private final PeerPermissionsBlacklist blacklist; + + public PeerReputationManagerTest() { + blacklist = PeerPermissionsBlacklist.create(); + peerReputationManager = new PeerReputationManager(blacklist); + } + + @Test + public void doesNotBlacklistPeerForNormalDisconnect() { + final PeerConnection peer = generatePeerConnection(); + + assertThat(blacklist.isPermitted(peer.getPeer())).isTrue(); + + peerReputationManager.onDisconnect(peer, DisconnectReason.TOO_MANY_PEERS, false); + + assertThat(blacklist.isPermitted(peer.getPeer())).isTrue(); + } + + @Test + public void blacklistPeerForBadBehavior() { + final PeerConnection peer = generatePeerConnection(); + + assertThat(blacklist.isPermitted(peer.getPeer())).isTrue(); + peerReputationManager.onDisconnect(peer, DisconnectReason.BREACH_OF_PROTOCOL, false); + assertThat(blacklist.isPermitted(peer.getPeer())).isFalse(); + } + + @Test + public void doesNotBlacklistPeerForOurBadBehavior() { + final PeerConnection peer = generatePeerConnection(); + + assertThat(blacklist.isPermitted(peer.getPeer())).isTrue(); + peerReputationManager.onDisconnect(peer, DisconnectReason.BREACH_OF_PROTOCOL, true); + assertThat(blacklist.isPermitted(peer.getPeer())).isTrue(); + } + + @Test + public void blacklistIncompatiblePeer() { + final PeerConnection peer = generatePeerConnection(); + + assertThat(blacklist.isPermitted(peer.getPeer())).isTrue(); + peerReputationManager.onDisconnect( + peer, DisconnectReason.INCOMPATIBLE_P2P_PROTOCOL_VERSION, false); + assertThat(blacklist.isPermitted(peer.getPeer())).isFalse(); + } + + @Test + public void blacklistIncompatiblePeerWhoIssuesDisconnect() { + final PeerConnection peer = generatePeerConnection(); + + assertThat(blacklist.isPermitted(peer.getPeer())).isTrue(); + peerReputationManager.onDisconnect( + peer, DisconnectReason.INCOMPATIBLE_P2P_PROTOCOL_VERSION, true); + assertThat(blacklist.isPermitted(peer.getPeer())).isFalse(); + } + + private PeerConnection generatePeerConnection() { + final BytesValue nodeId = Peer.randomId(); + final PeerConnection conn = mock(PeerConnection.class); + final PeerInfo peerInfo = mock(PeerInfo.class); + final Peer peer = + DefaultPeer.fromEnodeURL( + EnodeURL.builder() + .nodeId(Peer.randomId()) + .ipAddress("10.9.8.7") + .discoveryPort(65535) + .listeningPort(65534) + .build()); + + when(peerInfo.getNodeId()).thenReturn(nodeId); + when(conn.getPeerInfo()).thenReturn(peerInfo); + when(conn.getPeer()).thenReturn(peer); + + return conn; + } +} diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/peers/PeerBlacklistTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/peers/PeerBlacklistTest.java deleted file mode 100644 index 587d4a14ab..0000000000 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/peers/PeerBlacklistTest.java +++ /dev/null @@ -1,219 +0,0 @@ -/* - * Copyright 2018 ConsenSys AG. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ -package tech.pegasys.pantheon.ethereum.p2p.peers; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -import tech.pegasys.pantheon.ethereum.p2p.api.PeerConnection; -import tech.pegasys.pantheon.ethereum.p2p.wire.PeerInfo; -import tech.pegasys.pantheon.ethereum.p2p.wire.messages.DisconnectMessage.DisconnectReason; -import tech.pegasys.pantheon.util.bytes.BytesValue; -import tech.pegasys.pantheon.util.enode.EnodeURL; - -import java.util.Collections; - -import org.junit.Test; - -public class PeerBlacklistTest { - private int nodeIdValue = 1; - - @Test - public void directlyAddingPeerWorks() { - final PeerBlacklist blacklist = new PeerBlacklist(); - final Peer peer = generatePeer(); - - assertThat(blacklist.contains(peer)).isFalse(); - - blacklist.add(peer); - - assertThat(blacklist.contains(peer)).isTrue(); - } - - @Test - public void directlyAddingPeerByPeerIdWorks() { - final PeerBlacklist blacklist = new PeerBlacklist(); - final Peer peer = generatePeer(); - - assertThat(blacklist.contains(peer)).isFalse(); - - blacklist.add(peer.getId()); - - assertThat(blacklist.contains(peer)).isTrue(); - } - - @Test - public void banningPeerByPeerIdWorks() { - final Peer peer = generatePeer(); - final PeerBlacklist blacklist = new PeerBlacklist(Collections.singleton(peer.getId())); - - assertThat(blacklist.contains(peer)).isTrue(); - - blacklist.add(peer.getId()); - - assertThat(blacklist.contains(peer)).isTrue(); - } - - @Test - public void bannedNodesDoNotRollover() { - final Peer bannedPeer = generatePeer(); - final Peer peer1 = generatePeer(); - final Peer peer2 = generatePeer(); - final Peer peer3 = generatePeer(); - final PeerBlacklist blacklist = new PeerBlacklist(2, Collections.singleton(bannedPeer.getId())); - - assertThat(blacklist.contains(bannedPeer)).isTrue(); - assertThat(blacklist.contains(peer1)).isFalse(); - assertThat(blacklist.contains(peer2)).isFalse(); - assertThat(blacklist.contains(peer3)).isFalse(); - - // fill to the limit - blacklist.add(peer1.getId()); - blacklist.add(peer2.getId()); - assertThat(blacklist.contains(bannedPeer)).isTrue(); - assertThat(blacklist.contains(peer1)).isTrue(); - assertThat(blacklist.contains(peer2)).isTrue(); - assertThat(blacklist.contains(peer3)).isFalse(); - - // trigger rollover - blacklist.add(peer3.getId()); - assertThat(blacklist.contains(bannedPeer)).isTrue(); - assertThat(blacklist.contains(peer1)).isFalse(); - assertThat(blacklist.contains(peer2)).isTrue(); - assertThat(blacklist.contains(peer3)).isTrue(); - } - - @Test - public void doesNotBlacklistPeerForNormalDisconnect() { - final PeerBlacklist blacklist = new PeerBlacklist(); - final PeerConnection peer = generatePeerConnection(); - - assertThat(blacklist.contains(peer)).isFalse(); - - blacklist.onDisconnect(peer, DisconnectReason.TOO_MANY_PEERS, false); - - assertThat(blacklist.contains(peer)).isFalse(); - } - - @Test - public void blacklistPeerForBadBehavior() { - - final PeerBlacklist blacklist = new PeerBlacklist(); - final PeerConnection peer = generatePeerConnection(); - - assertThat(blacklist.contains(peer)).isFalse(); - - blacklist.onDisconnect(peer, DisconnectReason.BREACH_OF_PROTOCOL, false); - - assertThat(blacklist.contains(peer)).isTrue(); - } - - @Test - public void doesNotBlacklistPeerForOurBadBehavior() { - final PeerBlacklist blacklist = new PeerBlacklist(); - final PeerConnection peer = generatePeerConnection(); - - assertThat(blacklist.contains(peer)).isFalse(); - - blacklist.onDisconnect(peer, DisconnectReason.BREACH_OF_PROTOCOL, true); - - assertThat(blacklist.contains(peer)).isFalse(); - } - - @Test - public void blacklistIncompatiblePeer() { - final PeerBlacklist blacklist = new PeerBlacklist(); - final PeerConnection peer = generatePeerConnection(); - - assertThat(blacklist.contains(peer)).isFalse(); - - blacklist.onDisconnect(peer, DisconnectReason.INCOMPATIBLE_P2P_PROTOCOL_VERSION, false); - - assertThat(blacklist.contains(peer)).isTrue(); - } - - @Test - public void blacklistIncompatiblePeerWhoIssuesDisconnect() { - final PeerBlacklist blacklist = new PeerBlacklist(); - final PeerConnection peer = generatePeerConnection(); - - assertThat(blacklist.contains(peer)).isFalse(); - - blacklist.onDisconnect(peer, DisconnectReason.INCOMPATIBLE_P2P_PROTOCOL_VERSION, true); - - assertThat(blacklist.contains(peer)).isTrue(); - } - - @Test - public void capsSizeOfList() { - - final PeerBlacklist blacklist = new PeerBlacklist(2); - final PeerConnection peer1 = generatePeerConnection(); - final PeerConnection peer2 = generatePeerConnection(); - final PeerConnection peer3 = generatePeerConnection(); - - // Add first peer - blacklist.onDisconnect(peer1, DisconnectReason.BREACH_OF_PROTOCOL, false); - assertThat(blacklist.contains(peer1)).isTrue(); - assertThat(blacklist.contains(peer2)).isFalse(); - assertThat(blacklist.contains(peer3)).isFalse(); - - // Add second peer - blacklist.onDisconnect(peer2, DisconnectReason.BREACH_OF_PROTOCOL, false); - assertThat(blacklist.contains(peer1)).isTrue(); - assertThat(blacklist.contains(peer2)).isTrue(); - assertThat(blacklist.contains(peer3)).isFalse(); - - // Adding third peer should kick out least recently accessed peer - blacklist.onDisconnect(peer3, DisconnectReason.BREACH_OF_PROTOCOL, false); - assertThat(blacklist.contains(peer1)).isFalse(); - assertThat(blacklist.contains(peer2)).isTrue(); - assertThat(blacklist.contains(peer3)).isTrue(); - - // Adding peer1 back in should kick out peer2 - blacklist.onDisconnect(peer1, DisconnectReason.BREACH_OF_PROTOCOL, false); - assertThat(blacklist.contains(peer1)).isTrue(); - assertThat(blacklist.contains(peer2)).isFalse(); - assertThat(blacklist.contains(peer3)).isTrue(); - - // Adding peer2 back in should kick out peer3 - blacklist.onDisconnect(peer2, DisconnectReason.BREACH_OF_PROTOCOL, false); - assertThat(blacklist.contains(peer1)).isTrue(); - assertThat(blacklist.contains(peer2)).isTrue(); - assertThat(blacklist.contains(peer3)).isFalse(); - } - - private PeerConnection generatePeerConnection() { - final BytesValue nodeId = BytesValue.of(nodeIdValue++); - final PeerConnection peer = mock(PeerConnection.class); - final PeerInfo peerInfo = mock(PeerInfo.class); - - when(peerInfo.getNodeId()).thenReturn(nodeId); - when(peer.getPeerInfo()).thenReturn(peerInfo); - - return peer; - } - - private Peer generatePeer() { - final byte[] id = new byte[64]; - id[0] = (byte) nodeIdValue++; - return DefaultPeer.fromEnodeURL( - EnodeURL.builder() - .nodeId(id) - .ipAddress("10.9.8.7") - .discoveryPort(65535) - .listeningPort(65534) - .build()); - } -} diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/permissions/PeerPermissionsBlacklistTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/permissions/PeerPermissionsBlacklistTest.java new file mode 100644 index 0000000000..dbcba83a87 --- /dev/null +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/permissions/PeerPermissionsBlacklistTest.java @@ -0,0 +1,218 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package tech.pegasys.pantheon.ethereum.p2p.permissions; + +import static org.assertj.core.api.Assertions.assertThat; + +import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer; +import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; +import tech.pegasys.pantheon.util.enode.EnodeURL; + +import java.util.Collections; +import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.junit.Test; + +public class PeerPermissionsBlacklistTest { + + @Test + public void add_peer() { + PeerPermissionsBlacklist blacklist = PeerPermissionsBlacklist.create(); + Peer peer = createPeer(); + + final AtomicInteger callbackCount = new AtomicInteger(0); + blacklist.subscribeUpdate( + (restricted, affectedPeers) -> { + callbackCount.incrementAndGet(); + assertThat(restricted).isTrue(); + assertThat(affectedPeers).contains(Collections.singletonList(peer)); + }); + + assertThat(callbackCount).hasValue(0); + + blacklist.add(peer); + assertThat(callbackCount).hasValue(1); + } + + @Test + public void remove_peer() { + PeerPermissionsBlacklist blacklist = PeerPermissionsBlacklist.create(); + Peer peer = createPeer(); + blacklist.add(peer); + + final AtomicInteger callbackCount = new AtomicInteger(0); + blacklist.subscribeUpdate( + (restricted, affectedPeers) -> { + callbackCount.incrementAndGet(); + assertThat(restricted).isFalse(); + assertThat(affectedPeers).contains(Collections.singletonList(peer)); + }); + + assertThat(callbackCount).hasValue(0); + + blacklist.remove(peer); + assertThat(callbackCount).hasValue(1); + } + + @Test + public void add_id() { + PeerPermissionsBlacklist blacklist = PeerPermissionsBlacklist.create(); + Peer peer = createPeer(); + + final AtomicInteger callbackCount = new AtomicInteger(0); + blacklist.subscribeUpdate( + (restricted, affectedPeers) -> { + callbackCount.incrementAndGet(); + assertThat(restricted).isTrue(); + assertThat(affectedPeers).isEmpty(); + }); + + assertThat(callbackCount).hasValue(0); + + blacklist.add(peer.getId()); + assertThat(callbackCount).hasValue(1); + } + + @Test + public void remove_id() { + PeerPermissionsBlacklist blacklist = PeerPermissionsBlacklist.create(); + Peer peer = createPeer(); + blacklist.add(peer); + + final AtomicInteger callbackCount = new AtomicInteger(0); + blacklist.subscribeUpdate( + (restricted, affectedPeers) -> { + callbackCount.incrementAndGet(); + assertThat(restricted).isFalse(); + assertThat(affectedPeers).isEmpty(); + }); + + assertThat(callbackCount).hasValue(0); + + blacklist.remove(peer.getId()); + assertThat(callbackCount).hasValue(1); + } + + @Test + public void trackedPeerIsNotPermitted() { + PeerPermissionsBlacklist blacklist = PeerPermissionsBlacklist.create(); + + Peer peer = createPeer(); + assertThat(blacklist.isPermitted(peer)).isTrue(); + + blacklist.add(peer); + assertThat(blacklist.isPermitted(peer)).isFalse(); + + blacklist.remove(peer); + assertThat(blacklist.isPermitted(peer)).isTrue(); + } + + @Test + public void subscribeUpdate() { + PeerPermissionsBlacklist blacklist = PeerPermissionsBlacklist.create(); + final AtomicInteger callbackCount = new AtomicInteger(0); + final AtomicInteger restrictedCallbackCount = new AtomicInteger(0); + Peer peer = createPeer(); + + blacklist.subscribeUpdate( + (permissionsRestricted, affectedPeers) -> { + callbackCount.incrementAndGet(); + if (permissionsRestricted) { + restrictedCallbackCount.incrementAndGet(); + } + }); + + assertThat(blacklist.isPermitted(peer)).isTrue(); + assertThat(callbackCount).hasValue(0); + assertThat(restrictedCallbackCount).hasValue(0); + + blacklist.add(peer); + assertThat(callbackCount).hasValue(1); + assertThat(restrictedCallbackCount).hasValue(1); + + blacklist.add(peer); + assertThat(callbackCount).hasValue(1); + assertThat(restrictedCallbackCount).hasValue(1); + + blacklist.remove(peer); + assertThat(callbackCount).hasValue(2); + assertThat(restrictedCallbackCount).hasValue(1); + + blacklist.remove(peer); + assertThat(callbackCount).hasValue(2); + assertThat(restrictedCallbackCount).hasValue(1); + + blacklist.add(peer); + assertThat(callbackCount).hasValue(3); + assertThat(restrictedCallbackCount).hasValue(2); + } + + @Test + public void createWithLimitedCapacity() { + final PeerPermissionsBlacklist blacklist = PeerPermissionsBlacklist.create(2); + Peer peerA = createPeer(); + Peer peerB = createPeer(); + Peer peerC = createPeer(); + + // All peers are initially permitted + assertThat(blacklist.isPermitted(peerA)).isTrue(); + assertThat(blacklist.isPermitted(peerB)).isTrue(); + assertThat(blacklist.isPermitted(peerC)).isTrue(); + + // Add peerA + blacklist.add(peerA); + assertThat(blacklist.isPermitted(peerA)).isFalse(); + assertThat(blacklist.isPermitted(peerB)).isTrue(); + assertThat(blacklist.isPermitted(peerC)).isTrue(); + + // Add peerB + blacklist.add(peerB); + assertThat(blacklist.isPermitted(peerA)).isFalse(); + assertThat(blacklist.isPermitted(peerB)).isFalse(); + assertThat(blacklist.isPermitted(peerC)).isTrue(); + + // Add peerC + // Limit is exceeded and peerA should drop off of the list and be allowed + blacklist.add(peerC); + assertThat(blacklist.isPermitted(peerA)).isTrue(); + assertThat(blacklist.isPermitted(peerB)).isFalse(); + assertThat(blacklist.isPermitted(peerC)).isFalse(); + } + + @Test + public void createWithUnlimitedCapacity() { + final PeerPermissionsBlacklist blacklist = PeerPermissionsBlacklist.create(); + final int peerCount = 200; + final List peers = + Stream.generate(this::createPeer).limit(peerCount).collect(Collectors.toList()); + + peers.forEach(p -> assertThat(blacklist.isPermitted(p)).isTrue()); + peers.forEach(blacklist::add); + peers.forEach(p -> assertThat(blacklist.isPermitted(p)).isFalse()); + + peers.forEach(blacklist::remove); + peers.forEach(p -> assertThat(blacklist.isPermitted(p)).isTrue()); + } + + private Peer createPeer() { + return DefaultPeer.fromEnodeURL( + EnodeURL.builder() + .nodeId(Peer.randomId()) + .ipAddress("127.0.0.1") + .discoveryAndListeningPorts(EnodeURL.DEFAULT_LISTENING_PORT) + .build()); + } +} diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/permissions/PeerPermissionsTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/permissions/PeerPermissionsTest.java new file mode 100644 index 0000000000..74b7472323 --- /dev/null +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/permissions/PeerPermissionsTest.java @@ -0,0 +1,131 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package tech.pegasys.pantheon.ethereum.p2p.permissions; + +import static org.assertj.core.api.Assertions.assertThat; + +import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer; +import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; +import tech.pegasys.pantheon.util.enode.EnodeURL; + +import java.util.Optional; +import java.util.concurrent.atomic.AtomicInteger; + +import org.junit.Test; + +public class PeerPermissionsTest { + @Test + public void subscribeUpdate() { + TestPeerPermissions peerPermissions = new TestPeerPermissions(false); + final AtomicInteger callbackCount = new AtomicInteger(0); + + peerPermissions.subscribeUpdate( + (permissionsRestricted, affectedPeers) -> callbackCount.incrementAndGet()); + + peerPermissions.allowPeers(true); + assertThat(callbackCount).hasValue(1); + + peerPermissions.allowPeers(false); + assertThat(callbackCount).hasValue(2); + } + + @Test + public void subscribeUpdate_forCombinedPermissions() { + TestPeerPermissions peerPermissionsA = new TestPeerPermissions(false); + TestPeerPermissions peerPermissionsB = new TestPeerPermissions(false); + PeerPermissions combined = PeerPermissions.combine(peerPermissionsA, peerPermissionsB); + + final AtomicInteger callbackCount = new AtomicInteger(0); + final AtomicInteger restrictedCallbackCount = new AtomicInteger(0); + + combined.subscribeUpdate( + (permissionsRestricted, affectedPeers) -> { + callbackCount.incrementAndGet(); + if (permissionsRestricted) { + restrictedCallbackCount.incrementAndGet(); + } + }); + + peerPermissionsA.allowPeers(true); + assertThat(callbackCount).hasValue(1); + assertThat(restrictedCallbackCount).hasValue(0); + + peerPermissionsB.allowPeers(true); + assertThat(callbackCount).hasValue(2); + assertThat(restrictedCallbackCount).hasValue(0); + + peerPermissionsA.allowPeers(false); + assertThat(callbackCount).hasValue(3); + assertThat(restrictedCallbackCount).hasValue(1); + + peerPermissionsB.allowPeers(false); + assertThat(callbackCount).hasValue(4); + assertThat(restrictedCallbackCount).hasValue(2); + } + + @Test + public void isPermitted_forCombinedPermissions() { + final PeerPermissions allowPeers = new TestPeerPermissions(true); + final PeerPermissions disallowPeers = new TestPeerPermissions(false); + final PeerPermissions noop = PeerPermissions.NOOP; + final PeerPermissions combinedPermissive = PeerPermissions.combine(noop, allowPeers); + final PeerPermissions combinedRestrictive = PeerPermissions.combine(disallowPeers, allowPeers); + + Peer peer = + DefaultPeer.fromEnodeURL( + EnodeURL.builder() + .listeningPort(30303) + .discoveryPort(30303) + .nodeId(Peer.randomId()) + .ipAddress("127.0.0.1") + .build()); + + assertThat(PeerPermissions.combine(allowPeers, disallowPeers).isPermitted(peer)).isFalse(); + assertThat(PeerPermissions.combine(disallowPeers, disallowPeers).isPermitted(peer)).isFalse(); + assertThat(PeerPermissions.combine(disallowPeers, disallowPeers).isPermitted(peer)).isFalse(); + assertThat(PeerPermissions.combine(allowPeers, disallowPeers).isPermitted(peer)).isFalse(); + assertThat(PeerPermissions.combine(allowPeers, allowPeers).isPermitted(peer)).isTrue(); + + assertThat(PeerPermissions.combine(combinedPermissive, allowPeers).isPermitted(peer)).isTrue(); + assertThat(PeerPermissions.combine(combinedPermissive, disallowPeers).isPermitted(peer)) + .isFalse(); + assertThat(PeerPermissions.combine(combinedRestrictive, allowPeers).isPermitted(peer)) + .isFalse(); + assertThat(PeerPermissions.combine(combinedRestrictive, disallowPeers).isPermitted(peer)) + .isFalse(); + assertThat(PeerPermissions.combine(combinedRestrictive).isPermitted(peer)).isFalse(); + assertThat(PeerPermissions.combine(combinedPermissive).isPermitted(peer)).isTrue(); + + assertThat(PeerPermissions.combine(noop).isPermitted(peer)).isTrue(); + assertThat(PeerPermissions.combine().isPermitted(peer)).isTrue(); + } + + private static class TestPeerPermissions extends PeerPermissions { + + private boolean allowPeers; + + public TestPeerPermissions(final boolean allowPeers) { + this.allowPeers = allowPeers; + } + + public void allowPeers(final boolean doAllowPeers) { + this.allowPeers = doAllowPeers; + dispatchUpdate(!doAllowPeers, Optional.empty()); + } + + @Override + public boolean isPermitted(final Peer peer) { + return allowPeers; + } + } +} diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java b/pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java index c4c12899e1..a2a8ef56ea 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java @@ -58,7 +58,7 @@ import tech.pegasys.pantheon.ethereum.p2p.config.SubProtocolConfiguration; import tech.pegasys.pantheon.ethereum.p2p.network.DefaultP2PNetwork; import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer; -import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; +import tech.pegasys.pantheon.ethereum.p2p.permissions.PeerPermissionsBlacklist; import tech.pegasys.pantheon.ethereum.p2p.wire.Capability; import tech.pegasys.pantheon.ethereum.p2p.wire.SubProtocol; import tech.pegasys.pantheon.ethereum.permissioning.AccountLocalConfigPermissioningController; @@ -105,7 +105,7 @@ public class RunnerBuilder { private GraphQLRpcConfiguration graphQLRpcConfiguration; private WebSocketConfiguration webSocketConfiguration; private Path dataDir; - private Collection bannedNodeIds; + private Collection bannedNodeIds = new ArrayList<>(); private MetricsConfiguration metricsConfiguration; private MetricsSystem metricsSystem; private Optional permissioningConfiguration = Optional.empty(); @@ -178,8 +178,8 @@ public RunnerBuilder dataDir(final Path dataDir) { return this; } - public RunnerBuilder bannedNodeIds(final Collection bannedNodeIds) { - this.bannedNodeIds = bannedNodeIds; + public RunnerBuilder bannedNodeIds(final Collection bannedNodeIds) { + this.bannedNodeIds.addAll(bannedNodeIds); return this; } @@ -241,9 +241,8 @@ public Runner build() { .setClientId(PantheonInfo.version()) .setSupportedProtocols(subProtocols); - final PeerBlacklist peerBlacklist = - new PeerBlacklist( - bannedNodeIds.stream().map(BytesValue::fromHexString).collect(Collectors.toSet())); + final PeerPermissionsBlacklist bannedNodes = PeerPermissionsBlacklist.create(); + bannedNodeIds.forEach(bannedNodes::add); final List bootnodes = discoveryConfiguration.getBootnodes(); @@ -268,7 +267,7 @@ public Runner build() { .vertx(vertx) .keyPair(keyPair) .config(networkConfig) - .peerBlacklist(peerBlacklist) + .peerPermissions(bannedNodes) .metricsSystem(metricsSystem) .supportedCapabilities(caps) .nodePermissioningController(nodePermissioningController) diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java index 71ffa92cfd..8a4acbb8dc 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java @@ -217,7 +217,20 @@ void setBootnodes(final List values) { description = "A list of node IDs to ban from the P2P network.", split = ",", arity = "1..*") - private final Collection bannedNodeIds = new ArrayList<>(); + void setBannedNodeIds(final List values) { + try { + bannedNodeIds = + values.stream() + .filter(value -> !value.isEmpty()) + .map(EnodeURL::parseNodeId) + .collect(Collectors.toList()); + } catch (final IllegalArgumentException e) { + throw new ParameterException( + commandLine, "Invalid ids supplied to '--banned-node-ids'. " + e.getMessage()); + } + } + + private Collection bannedNodeIds = new ArrayList<>(); @Option( names = {"--sync-mode"}, diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/RunnerTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/RunnerTest.java index b4cb3d0b6b..e3c1419cee 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/RunnerTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/RunnerTest.java @@ -169,7 +169,6 @@ private void syncFromGenesis(final SyncMode mode) throws Exception { .p2pListenPort(0) .maxPeers(3) .metricsSystem(noOpMetricsSystem) - .bannedNodeIds(emptySet()) .staticNodes(emptySet()); Runner runnerBehind = null; diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java index b3edaa22cf..6f0249f637 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java @@ -35,6 +35,7 @@ import tech.pegasys.pantheon.plugins.internal.PantheonPluginContextImpl; import tech.pegasys.pantheon.services.kvstore.RocksDbConfiguration; import tech.pegasys.pantheon.util.BlockImporter; +import tech.pegasys.pantheon.util.bytes.BytesValue; import java.io.ByteArrayOutputStream; import java.io.File; @@ -87,6 +88,7 @@ public abstract class CommandTestAbstract { @Mock Logger mockLogger; @Mock PantheonPluginContextImpl mockPantheonPluginContext; + @Captor ArgumentCaptor> bytesValueCollectionCollector; @Captor ArgumentCaptor> stringListArgumentCaptor; @Captor ArgumentCaptor pathArgumentCaptor; @Captor ArgumentCaptor fileArgumentCaptor; diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java index cf8ecdb383..567e486a95 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java @@ -60,6 +60,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Arrays; import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -881,7 +882,9 @@ public void p2pEnabledOptionValueFalseMustBeUsed() { @Test public void p2pOptionsRequiresServiceToBeEnabled() { - final String[] nodes = {"0001", "0002", "0003"}; + final String[] nodes = { + "6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0" + }; parseCommand( "--p2p-enabled", @@ -981,15 +984,6 @@ public void callingWithInvalidBootnodeAndEqualSignMustDisplayErrorAndUsage() { assertThat(commandErrorOutput.toString()).startsWith(expectedErrorOutputStart); } - @Test - public void callingWithBannedNodeidsOptionButNoValueMustDisplayErrorAndUsage() { - parseCommand("--banned-node-ids"); - assertThat(commandOutput.toString()).isEmpty(); - final String expectedErrorOutputStart = - "Missing required parameter for option '--banned-node-ids' at index 0 ()"; - assertThat(commandErrorOutput.toString()).startsWith(expectedErrorOutputStart); - } - @Test public void bootnodesOptionMustBeUsed() { parseCommand("--bootnodes", String.join(",", validENodeStrings)); @@ -1007,18 +1001,48 @@ public void bootnodesOptionMustBeUsed() { @Test public void bannedNodeIdsOptionMustBeUsed() { - final String[] nodes = {"0001", "0002", "0003"}; - parseCommand("--banned-node-ids", String.join(",", nodes)); - - verify(mockRunnerBuilder).bannedNodeIds(stringListArgumentCaptor.capture()); + final BytesValue[] nodes = { + BytesValue.fromHexString( + "6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"), + BytesValue.fromHexString( + "7f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"), + BytesValue.fromHexString( + "0x8f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0") + }; + + final String nodeIdsArg = + Arrays.asList(nodes).stream() + .map(BytesValue::toUnprefixedString) + .collect(Collectors.joining(",")); + parseCommand("--banned-node-ids", nodeIdsArg); + + verify(mockRunnerBuilder).bannedNodeIds(bytesValueCollectionCollector.capture()); verify(mockRunnerBuilder).build(); - assertThat(stringListArgumentCaptor.getValue().toArray()).isEqualTo(nodes); + assertThat(bytesValueCollectionCollector.getValue().toArray()).isEqualTo(nodes); assertThat(commandOutput.toString()).isEmpty(); assertThat(commandErrorOutput.toString()).isEmpty(); } + @Test + public void callingWithBannedNodeidsOptionButNoValueMustDisplayErrorAndUsage() { + parseCommand("--banned-node-ids"); + assertThat(commandOutput.toString()).isEmpty(); + final String expectedErrorOutputStart = + "Missing required parameter for option '--banned-node-ids' at index 0 ()"; + assertThat(commandErrorOutput.toString()).startsWith(expectedErrorOutputStart); + } + + @Test + public void callingWithBannedNodeidsOptionWithInvalidValuesMustDisplayErrorAndUsage() { + parseCommand("--banned-node-ids", "0x10,20,30"); + assertThat(commandOutput.toString()).isEmpty(); + final String expectedErrorOutputStart = + "Invalid ids supplied to '--banned-node-ids'. Expected 64 bytes in 0x10"; + assertThat(commandErrorOutput.toString()).startsWith(expectedErrorOutputStart); + } + @Test public void p2pHostAndPortOptionMustBeUsed() { diff --git a/util/src/main/java/tech/pegasys/pantheon/util/LimitedSet.java b/util/src/main/java/tech/pegasys/pantheon/util/LimitedSet.java new file mode 100644 index 0000000000..7bb3573089 --- /dev/null +++ b/util/src/main/java/tech/pegasys/pantheon/util/LimitedSet.java @@ -0,0 +1,48 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package tech.pegasys.pantheon.util; + +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Set; + +/** Helper that creates a thread-safe set with a maximum capacity. */ +public final class LimitedSet { + public enum Mode { + DROP_LEAST_RECENTLY_ACCESSED, + DROP_OLDEST_ELEMENT + } + + private LimitedSet() {} + + /** + * @param initialCapacity The initial size to allocate for the set. + * @param maxSize The maximum number of elements to keep in the set. + * @param mode A mode that determines which element is evicted when the set exceeds its max size. + * @param The type of object held in the set. + * @return A thread-safe set that will evict elements when the max size is exceeded. + */ + public static final Set create( + final int initialCapacity, final int maxSize, final Mode mode) { + final boolean useAccessOrder = mode.equals(Mode.DROP_LEAST_RECENTLY_ACCESSED); + return Collections.synchronizedSet( + Collections.newSetFromMap( + new LinkedHashMap(initialCapacity, 0.75f, useAccessOrder) { + @Override + protected boolean removeEldestEntry(final Map.Entry eldest) { + return size() > maxSize; + } + })); + } +} diff --git a/util/src/main/java/tech/pegasys/pantheon/util/enode/EnodeURL.java b/util/src/main/java/tech/pegasys/pantheon/util/enode/EnodeURL.java index 2509676045..8461611dc5 100644 --- a/util/src/main/java/tech/pegasys/pantheon/util/enode/EnodeURL.java +++ b/util/src/main/java/tech/pegasys/pantheon/util/enode/EnodeURL.java @@ -133,6 +133,17 @@ public static boolean sameListeningEndpoint(final EnodeURL enodeA, final EnodeUR && Objects.equals(enodeA.listeningPort, enodeB.listeningPort); } + public static BytesValue parseNodeId(final String nodeId) { + int expectedSize = EnodeURL.NODE_ID_SIZE * 2; + if (nodeId.toLowerCase().startsWith("0x")) { + expectedSize += 2; + } + checkArgument( + nodeId.length() == expectedSize, + "Expected " + EnodeURL.NODE_ID_SIZE + " bytes in " + nodeId); + return BytesValue.fromHexString(nodeId, NODE_ID_SIZE); + } + public URI toURI() { final String uri = String.format( diff --git a/util/src/test/java/tech/pegasys/pantheon/util/LimitedSetTest.java b/util/src/test/java/tech/pegasys/pantheon/util/LimitedSetTest.java new file mode 100644 index 0000000000..390e6df6ad --- /dev/null +++ b/util/src/test/java/tech/pegasys/pantheon/util/LimitedSetTest.java @@ -0,0 +1,60 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package tech.pegasys.pantheon.util; + +import static org.assertj.core.api.Assertions.assertThat; + +import tech.pegasys.pantheon.util.LimitedSet.Mode; + +import java.util.Set; + +import org.junit.Test; + +public class LimitedSetTest { + + @Test + public void create_evictOldest() { + final Set set = LimitedSet.create(1, 2, Mode.DROP_OLDEST_ELEMENT); + set.add(1); + assertThat(set.size()).isEqualTo(1); + set.add(2); + assertThat(set.size()).isEqualTo(2); + + // Access element 1 then add a new element that will put us over the limit + set.add(1); + + set.add(3); + assertThat(set.size()).isEqualTo(2); + // Element 1 should have been evicted + assertThat(set.contains(3)).isTrue(); + assertThat(set.contains(2)).isTrue(); + } + + @Test + public void create_evictLeastRecentlyAccessed() { + final Set set = LimitedSet.create(1, 2, Mode.DROP_LEAST_RECENTLY_ACCESSED); + set.add(1); + assertThat(set.size()).isEqualTo(1); + set.add(2); + assertThat(set.size()).isEqualTo(2); + + // Access element 1 then add a new element that will put us over the limit + set.add(1); + + set.add(3); + assertThat(set.size()).isEqualTo(2); + // Element 2 should have been evicted + assertThat(set.contains(3)).isTrue(); + assertThat(set.contains(1)).isTrue(); + } +} diff --git a/util/src/test/java/tech/pegasys/pantheon/util/enode/EnodeURLTest.java b/util/src/test/java/tech/pegasys/pantheon/util/enode/EnodeURLTest.java index e021a5bbf5..f3bb88910f 100644 --- a/util/src/test/java/tech/pegasys/pantheon/util/enode/EnodeURLTest.java +++ b/util/src/test/java/tech/pegasys/pantheon/util/enode/EnodeURLTest.java @@ -16,6 +16,8 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.catchThrowable; +import tech.pegasys.pantheon.util.bytes.BytesValue; + import java.net.URI; import java.util.OptionalInt; @@ -724,4 +726,20 @@ public void sameListeningEndpoint_listeningDisabledForBoth() { assertThat(EnodeURL.sameListeningEndpoint(enodeA, enodeB)).isTrue(); } + + @Test + public void parseNodeId_invalid() { + final String invalidId = "0x10"; + assertThatThrownBy(() -> EnodeURL.parseNodeId(invalidId)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Expected 64 bytes in " + invalidId); + } + + @Test + public void parseNodeId_valid() { + final String validId = VALID_NODE_ID; + final BytesValue nodeId = EnodeURL.parseNodeId(validId); + assertThat(nodeId.size()).isEqualTo(EnodeURL.NODE_ID_SIZE); + assertThat(nodeId.toUnprefixedString()).isEqualTo(validId); + } }