From c676937c1f1597c8b533200156765005acd71986 Mon Sep 17 00:00:00 2001 From: David Mechler Date: Fri, 8 May 2020 08:43:31 -0400 Subject: [PATCH 01/11] Add check for spoofed IP in ping message Signed-off-by: David Mechler --- .../internal/PeerDiscoveryController.java | 11 +++++++ .../internal/PeerDiscoveryControllerTest.java | 30 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java index 3756c058717..17e4747ac54 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java @@ -292,6 +292,17 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { return; } + // Message from spoofed IP should be ignored + if (packet.getType().equals(PacketType.PING) + && packet + .getPacketData(PingPacketData.class) + .map( + pingPacketData -> + !sender.getEndpoint().getHost().equals(pingPacketData.getFrom().getHost())) + .orElse(false)) { + return; + } + // Load the peer from the table, or use the instance that comes in. final Optional maybeKnownPeer = peerTable.get(sender).filter(known -> known.discoveryEndpointMatches(sender)); diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java index 64e59438711..03581c454aa 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java @@ -585,6 +585,36 @@ public void shouldNotRemoveExistingPeerWhenReceivedPing() { assertThat(controller.streamDiscoveredPeers()).contains(peers.get(0)); } + @Test + public void shouldIgnorePeerWhenReceivedInvalidReplyToken() { + // test spoofs a ping from a different ip address('victim ip') + startPeerDiscoveryController(); + + final DiscoveryPeer peer = + spy( + DiscoveryPeer.fromEnode( + EnodeURL.builder() + .nodeId(Peer.randomId()) + .ipAddress("10.20.30.40") + .discoveryAndListeningPorts(30303) + .build())); + + final DiscoveryPeer victimPeer = + spy( + DiscoveryPeer.fromEnode( + EnodeURL.builder() + .nodeId(Peer.randomId()) + .ipAddress("10.20.30.41") + .discoveryAndListeningPorts(30303) + .build())); + + final Packet pingPacket = mockPingPacket(peer, this.localPeer); + controller.onMessage(pingPacket, victimPeer); + + assertThat(controller.streamDiscoveredPeers()).doesNotContain(victimPeer); + assertThat(controller.streamDiscoveredPeers()).doesNotContain(peer); + } + @Test public void shouldNotAddNewPeerWhenReceivedPongFromBlacklistedPeer() { final List peers = createPeersInLastBucket(localPeer, 3); From 26dc9388b381caed88b0c5467dd067ac916e12bd Mon Sep 17 00:00:00 2001 From: David Mechler Date: Fri, 8 May 2020 09:14:01 -0400 Subject: [PATCH 02/11] Fix formatting issues Signed-off-by: David Mechler --- .../discovery/internal/PeerDiscoveryController.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java index 17e4747ac54..d5310162089 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java @@ -295,11 +295,11 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { // Message from spoofed IP should be ignored if (packet.getType().equals(PacketType.PING) && packet - .getPacketData(PingPacketData.class) - .map( - pingPacketData -> - !sender.getEndpoint().getHost().equals(pingPacketData.getFrom().getHost())) - .orElse(false)) { + .getPacketData(PingPacketData.class) + .map( + pingPacketData -> + !sender.getEndpoint().getHost().equals(pingPacketData.getFrom().getHost())) + .orElse(false)) { return; } From de9b2dbad6ce3f79e257f9fec758ac8b6dc34472 Mon Sep 17 00:00:00 2001 From: David Mechler Date: Fri, 8 May 2020 10:14:32 -0400 Subject: [PATCH 03/11] Add logging message when ping request is rejected. Signed-off-by: David Mechler --- .../ethereum/p2p/discovery/internal/PeerDiscoveryController.java | 1 + 1 file changed, 1 insertion(+) diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java index d5310162089..bda1466231c 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java @@ -300,6 +300,7 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { pingPacketData -> !sender.getEndpoint().getHost().equals(pingPacketData.getFrom().getHost())) .orElse(false)) { + LOG.debug("Rejecting ping attempt from IP {}", sender.getEndpoint().getHost()); return; } From b99b7c94fb0852f0ca796d1df2002fbc37f552df Mon Sep 17 00:00:00 2001 From: David Mechler Date: Thu, 4 Jun 2020 16:38:41 -0400 Subject: [PATCH 04/11] Revert last changes Signed-off-by: David Mechler --- .../internal/PeerDiscoveryController.java | 25 ++++++++-------- .../internal/PeerDiscoveryControllerTest.java | 30 ------------------- 2 files changed, 12 insertions(+), 43 deletions(-) diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java index 266be8111f2..673aeef1eed 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java @@ -293,18 +293,6 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { return; } - // Message from spoofed IP should be ignored - if (packet.getType().equals(PacketType.PING) - && packet - .getPacketData(PingPacketData.class) - .map( - pingPacketData -> - !sender.getEndpoint().getHost().equals(pingPacketData.getFrom().getHost())) - .orElse(false)) { - LOG.debug("Rejecting ping attempt from IP {}", sender.getEndpoint().getHost()); - return; - } - // Load the peer from the table, or use the instance that comes in. final Optional maybeKnownPeer = peerTable.get(sender).filter(known -> known.discoveryEndpointMatches(sender)); @@ -328,6 +316,12 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { }); break; case NEIGHBORS: + if (peer.getStatus() != PeerDiscoveryStatus.BONDED) { + LOG.info( + "Rejecting NEIGHBORS request for unbonded nodes, sender: {}", sender.getEndpoint()); + break; + } + matchInteraction(packet) .ifPresent( interaction -> @@ -335,7 +329,12 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { peer, getPeersFromNeighborsPacket(packet))); break; case FIND_NEIGHBORS: - if (!peerKnown || !peerPermissions.allowInboundNeighborsRequest(peer)) { + if (!peerKnown + || !peerPermissions.allowInboundNeighborsRequest(peer) + || (peer.getStatus() != PeerDiscoveryStatus.BONDED)) { + LOG.info( + "Rejecting FIND_NEIGHBORS request for unbonded nodes, sender: {}", + sender.getEndpoint()); break; } diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java index 961c92806c7..e74736e25a8 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java @@ -645,36 +645,6 @@ public void shouldNotRemoveExistingPeerWhenReceivedPing() { assertThat(controller.streamDiscoveredPeers()).contains(peers.get(0)); } - @Test - public void shouldIgnorePeerWhenReceivedInvalidReplyToken() { - // test spoofs a ping from a different ip address('victim ip') - startPeerDiscoveryController(); - - final DiscoveryPeer peer = - spy( - DiscoveryPeer.fromEnode( - EnodeURL.builder() - .nodeId(Peer.randomId()) - .ipAddress("10.20.30.40") - .discoveryAndListeningPorts(30303) - .build())); - - final DiscoveryPeer victimPeer = - spy( - DiscoveryPeer.fromEnode( - EnodeURL.builder() - .nodeId(Peer.randomId()) - .ipAddress("10.20.30.41") - .discoveryAndListeningPorts(30303) - .build())); - - final Packet pingPacket = mockPingPacket(peer, this.localPeer); - controller.onMessage(pingPacket, victimPeer); - - assertThat(controller.streamDiscoveredPeers()).doesNotContain(victimPeer); - assertThat(controller.streamDiscoveredPeers()).doesNotContain(peer); - } - @Test public void shouldNotAddNewPeerWhenReceivedPongFromBlacklistedPeer() { final List peers = createPeersInLastBucket(localPeer, 3); From f2eddab4e11b151bcab242bf8a69d903e716f0ea Mon Sep 17 00:00:00 2001 From: David Mechler Date: Mon, 8 Jun 2020 10:43:00 -0400 Subject: [PATCH 05/11] Fix for hive SpoofAmplification test Signed-off-by: David Mechler --- .../internal/PeerDiscoveryController.java | 3 +- .../internal/PeerDiscoveryControllerTest.java | 66 +++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java index 673aeef1eed..2d79c87e2a0 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java @@ -301,7 +301,8 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { switch (packet.getType()) { case PING: - if (peerPermissions.allowInboundBonding(peer)) { + if (peerPermissions.allowInboundBonding(peer) + && (sender.getEnodeURL().getNodeId().equals(packet.getNodeId()))) { addToPeerTable(peer); final PingPacketData ping = packet.getPacketData(PingPacketData.class).get(); respondToPing(ping, packet.getHash(), peer); diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java index e74736e25a8..22d8796b98d 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java @@ -603,6 +603,72 @@ public void shouldAddNewPeerWhenReceivedPingAndPeerTableBucketIsNotFull() { assertThat(controller.streamDiscoveredPeers()).contains(peers.get(0)); } + @Test + public void shouldNotBondWhenReceivingSpoofedPing() { + final List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(2); + final List peers = helper.createDiscoveryPeers(nodeKeys); + + DiscoveryPeer victim = peers.get(0); + DiscoveryPeer badGuy = peers.get(1); + + final OutboundMessageHandler outboundMessageHandler = mock(OutboundMessageHandler.class); + controller = getControllerBuilder().outboundMessageHandler(outboundMessageHandler).build(); + + controller.start(); + + final PingPacketData pingPacketData = + PingPacketData.create(badGuy.getEndpoint(), localPeer.getEndpoint()); + final Packet pingPacket = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(1)); + controller.onMessage(pingPacket, victim); + assertThat(controller.streamDiscoveredPeers()).doesNotContain(victim); + assertThat(controller.streamDiscoveredPeers()).doesNotContain(badGuy); + } + + /* + udpPorts NOT equal + tcpPorts NOT equal + */ + @Test + public void shouldNotBondWhenReceivingSpoofedPing1() { + final List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(2); + final List peers = helper.createDiscoveryPeers(nodeKeys); + + DiscoveryPeer victim = peers.get(0); + DiscoveryPeer badGuy = peers.get(1); + + final OutboundMessageHandler outboundMessageHandler = mock(OutboundMessageHandler.class); + controller = getControllerBuilder().outboundMessageHandler(outboundMessageHandler).build(); + + controller.start(); + + final PingPacketData pingPacketData = + PingPacketData.create(badGuy.getEndpoint(), localPeer.getEndpoint()); + final Packet pingPacket = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); + controller.onMessage(pingPacket, victim); + assertThat(controller.streamDiscoveredPeers()).contains(victim); + } + + @Test + public void shouldNotBondWhenReceivingSpoofedPing2() { + final List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(2); + final List peers = helper.createDiscoveryPeers(nodeKeys); + + DiscoveryPeer victim = peers.get(0); + DiscoveryPeer badGuy = peers.get(1); + + final OutboundMessageHandler outboundMessageHandler = mock(OutboundMessageHandler.class); + controller = getControllerBuilder().outboundMessageHandler(outboundMessageHandler).build(); + + controller.start(); + + final PingPacketData pingPacketData = + PingPacketData.create(victim.getEndpoint(), localPeer.getEndpoint()); + final Packet pingPacket = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(1)); + controller.onMessage(pingPacket, victim); + assertThat(controller.streamDiscoveredPeers()).doesNotContain(victim); + assertThat(controller.streamDiscoveredPeers()).doesNotContain(badGuy); + } + @Test public void shouldNotAddSelfWhenReceivedPingFromSelf() { startPeerDiscoveryController(); From 4f1af986b12b2ec167efcd850de18a454dcbeab2 Mon Sep 17 00:00:00 2001 From: David Mechler Date: Fri, 8 May 2020 08:43:31 -0400 Subject: [PATCH 06/11] Add check for spoofed IP in ping message Signed-off-by: David Mechler --- .../internal/PeerDiscoveryController.java | 11 +++++++ .../internal/PeerDiscoveryControllerTest.java | 30 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java index 70823aa1d39..65e04baab8d 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java @@ -293,6 +293,17 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { return; } + // Message from spoofed IP should be ignored + if (packet.getType().equals(PacketType.PING) + && packet + .getPacketData(PingPacketData.class) + .map( + pingPacketData -> + !sender.getEndpoint().getHost().equals(pingPacketData.getFrom().getHost())) + .orElse(false)) { + return; + } + // Load the peer from the table, or use the instance that comes in. final Optional maybeKnownPeer = peerTable.get(sender).filter(known -> known.discoveryEndpointMatches(sender)); diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java index e74736e25a8..961c92806c7 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java @@ -645,6 +645,36 @@ public void shouldNotRemoveExistingPeerWhenReceivedPing() { assertThat(controller.streamDiscoveredPeers()).contains(peers.get(0)); } + @Test + public void shouldIgnorePeerWhenReceivedInvalidReplyToken() { + // test spoofs a ping from a different ip address('victim ip') + startPeerDiscoveryController(); + + final DiscoveryPeer peer = + spy( + DiscoveryPeer.fromEnode( + EnodeURL.builder() + .nodeId(Peer.randomId()) + .ipAddress("10.20.30.40") + .discoveryAndListeningPorts(30303) + .build())); + + final DiscoveryPeer victimPeer = + spy( + DiscoveryPeer.fromEnode( + EnodeURL.builder() + .nodeId(Peer.randomId()) + .ipAddress("10.20.30.41") + .discoveryAndListeningPorts(30303) + .build())); + + final Packet pingPacket = mockPingPacket(peer, this.localPeer); + controller.onMessage(pingPacket, victimPeer); + + assertThat(controller.streamDiscoveredPeers()).doesNotContain(victimPeer); + assertThat(controller.streamDiscoveredPeers()).doesNotContain(peer); + } + @Test public void shouldNotAddNewPeerWhenReceivedPongFromBlacklistedPeer() { final List peers = createPeersInLastBucket(localPeer, 3); From 00c69d9ef86f17e0805877fae12601c71a989b64 Mon Sep 17 00:00:00 2001 From: David Mechler Date: Fri, 8 May 2020 09:14:01 -0400 Subject: [PATCH 07/11] Fix formatting issues Signed-off-by: David Mechler --- .../discovery/internal/PeerDiscoveryController.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java index 65e04baab8d..5d1526f1b75 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java @@ -296,11 +296,11 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { // Message from spoofed IP should be ignored if (packet.getType().equals(PacketType.PING) && packet - .getPacketData(PingPacketData.class) - .map( - pingPacketData -> - !sender.getEndpoint().getHost().equals(pingPacketData.getFrom().getHost())) - .orElse(false)) { + .getPacketData(PingPacketData.class) + .map( + pingPacketData -> + !sender.getEndpoint().getHost().equals(pingPacketData.getFrom().getHost())) + .orElse(false)) { return; } From 05afcf1a103fb77fb2c45c833d3ceb0ff41d461c Mon Sep 17 00:00:00 2001 From: David Mechler Date: Fri, 8 May 2020 10:14:32 -0400 Subject: [PATCH 08/11] Add logging message when ping request is rejected. Signed-off-by: David Mechler --- .../ethereum/p2p/discovery/internal/PeerDiscoveryController.java | 1 + 1 file changed, 1 insertion(+) diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java index 5d1526f1b75..266be8111f2 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java @@ -301,6 +301,7 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { pingPacketData -> !sender.getEndpoint().getHost().equals(pingPacketData.getFrom().getHost())) .orElse(false)) { + LOG.debug("Rejecting ping attempt from IP {}", sender.getEndpoint().getHost()); return; } From cce51a048c2c00bcc66ca5bd242c2d21efd4230b Mon Sep 17 00:00:00 2001 From: David Mechler Date: Thu, 4 Jun 2020 16:38:41 -0400 Subject: [PATCH 09/11] Revert last changes Signed-off-by: David Mechler --- .../internal/PeerDiscoveryController.java | 25 ++++++++-------- .../internal/PeerDiscoveryControllerTest.java | 30 ------------------- 2 files changed, 12 insertions(+), 43 deletions(-) diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java index 266be8111f2..673aeef1eed 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java @@ -293,18 +293,6 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { return; } - // Message from spoofed IP should be ignored - if (packet.getType().equals(PacketType.PING) - && packet - .getPacketData(PingPacketData.class) - .map( - pingPacketData -> - !sender.getEndpoint().getHost().equals(pingPacketData.getFrom().getHost())) - .orElse(false)) { - LOG.debug("Rejecting ping attempt from IP {}", sender.getEndpoint().getHost()); - return; - } - // Load the peer from the table, or use the instance that comes in. final Optional maybeKnownPeer = peerTable.get(sender).filter(known -> known.discoveryEndpointMatches(sender)); @@ -328,6 +316,12 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { }); break; case NEIGHBORS: + if (peer.getStatus() != PeerDiscoveryStatus.BONDED) { + LOG.info( + "Rejecting NEIGHBORS request for unbonded nodes, sender: {}", sender.getEndpoint()); + break; + } + matchInteraction(packet) .ifPresent( interaction -> @@ -335,7 +329,12 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { peer, getPeersFromNeighborsPacket(packet))); break; case FIND_NEIGHBORS: - if (!peerKnown || !peerPermissions.allowInboundNeighborsRequest(peer)) { + if (!peerKnown + || !peerPermissions.allowInboundNeighborsRequest(peer) + || (peer.getStatus() != PeerDiscoveryStatus.BONDED)) { + LOG.info( + "Rejecting FIND_NEIGHBORS request for unbonded nodes, sender: {}", + sender.getEndpoint()); break; } diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java index 961c92806c7..e74736e25a8 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java @@ -645,36 +645,6 @@ public void shouldNotRemoveExistingPeerWhenReceivedPing() { assertThat(controller.streamDiscoveredPeers()).contains(peers.get(0)); } - @Test - public void shouldIgnorePeerWhenReceivedInvalidReplyToken() { - // test spoofs a ping from a different ip address('victim ip') - startPeerDiscoveryController(); - - final DiscoveryPeer peer = - spy( - DiscoveryPeer.fromEnode( - EnodeURL.builder() - .nodeId(Peer.randomId()) - .ipAddress("10.20.30.40") - .discoveryAndListeningPorts(30303) - .build())); - - final DiscoveryPeer victimPeer = - spy( - DiscoveryPeer.fromEnode( - EnodeURL.builder() - .nodeId(Peer.randomId()) - .ipAddress("10.20.30.41") - .discoveryAndListeningPorts(30303) - .build())); - - final Packet pingPacket = mockPingPacket(peer, this.localPeer); - controller.onMessage(pingPacket, victimPeer); - - assertThat(controller.streamDiscoveredPeers()).doesNotContain(victimPeer); - assertThat(controller.streamDiscoveredPeers()).doesNotContain(peer); - } - @Test public void shouldNotAddNewPeerWhenReceivedPongFromBlacklistedPeer() { final List peers = createPeersInLastBucket(localPeer, 3); From 96fc849497a7a445e66fef8e1a632729b305555b Mon Sep 17 00:00:00 2001 From: David Mechler Date: Mon, 8 Jun 2020 10:43:00 -0400 Subject: [PATCH 10/11] Fix for hive SpoofAmplification test Signed-off-by: David Mechler --- .../internal/PeerDiscoveryController.java | 3 +- .../internal/PeerDiscoveryControllerTest.java | 66 +++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java index 673aeef1eed..2d79c87e2a0 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java @@ -301,7 +301,8 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { switch (packet.getType()) { case PING: - if (peerPermissions.allowInboundBonding(peer)) { + if (peerPermissions.allowInboundBonding(peer) + && (sender.getEnodeURL().getNodeId().equals(packet.getNodeId()))) { addToPeerTable(peer); final PingPacketData ping = packet.getPacketData(PingPacketData.class).get(); respondToPing(ping, packet.getHash(), peer); diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java index e74736e25a8..22d8796b98d 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java @@ -603,6 +603,72 @@ public void shouldAddNewPeerWhenReceivedPingAndPeerTableBucketIsNotFull() { assertThat(controller.streamDiscoveredPeers()).contains(peers.get(0)); } + @Test + public void shouldNotBondWhenReceivingSpoofedPing() { + final List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(2); + final List peers = helper.createDiscoveryPeers(nodeKeys); + + DiscoveryPeer victim = peers.get(0); + DiscoveryPeer badGuy = peers.get(1); + + final OutboundMessageHandler outboundMessageHandler = mock(OutboundMessageHandler.class); + controller = getControllerBuilder().outboundMessageHandler(outboundMessageHandler).build(); + + controller.start(); + + final PingPacketData pingPacketData = + PingPacketData.create(badGuy.getEndpoint(), localPeer.getEndpoint()); + final Packet pingPacket = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(1)); + controller.onMessage(pingPacket, victim); + assertThat(controller.streamDiscoveredPeers()).doesNotContain(victim); + assertThat(controller.streamDiscoveredPeers()).doesNotContain(badGuy); + } + + /* + udpPorts NOT equal + tcpPorts NOT equal + */ + @Test + public void shouldNotBondWhenReceivingSpoofedPing1() { + final List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(2); + final List peers = helper.createDiscoveryPeers(nodeKeys); + + DiscoveryPeer victim = peers.get(0); + DiscoveryPeer badGuy = peers.get(1); + + final OutboundMessageHandler outboundMessageHandler = mock(OutboundMessageHandler.class); + controller = getControllerBuilder().outboundMessageHandler(outboundMessageHandler).build(); + + controller.start(); + + final PingPacketData pingPacketData = + PingPacketData.create(badGuy.getEndpoint(), localPeer.getEndpoint()); + final Packet pingPacket = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); + controller.onMessage(pingPacket, victim); + assertThat(controller.streamDiscoveredPeers()).contains(victim); + } + + @Test + public void shouldNotBondWhenReceivingSpoofedPing2() { + final List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(2); + final List peers = helper.createDiscoveryPeers(nodeKeys); + + DiscoveryPeer victim = peers.get(0); + DiscoveryPeer badGuy = peers.get(1); + + final OutboundMessageHandler outboundMessageHandler = mock(OutboundMessageHandler.class); + controller = getControllerBuilder().outboundMessageHandler(outboundMessageHandler).build(); + + controller.start(); + + final PingPacketData pingPacketData = + PingPacketData.create(victim.getEndpoint(), localPeer.getEndpoint()); + final Packet pingPacket = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(1)); + controller.onMessage(pingPacket, victim); + assertThat(controller.streamDiscoveredPeers()).doesNotContain(victim); + assertThat(controller.streamDiscoveredPeers()).doesNotContain(badGuy); + } + @Test public void shouldNotAddSelfWhenReceivedPingFromSelf() { startPeerDiscoveryController(); From f706cdecd733f3e341947b7bc10736def8c1d6dc Mon Sep 17 00:00:00 2001 From: David Mechler Date: Thu, 11 Jun 2020 09:34:47 -0400 Subject: [PATCH 11/11] Fix for hive SpoofAmplification test Signed-off-by: David Mechler --- .../internal/PeerDiscoveryController.java | 18 +-- .../p2p/discovery/PeerDiscoveryAgentTest.java | 4 +- .../internal/PeerDiscoveryControllerTest.java | 140 ++++-------------- 3 files changed, 36 insertions(+), 126 deletions(-) diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java index 2d79c87e2a0..1f3a0daad97 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java @@ -301,11 +301,10 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { switch (packet.getType()) { case PING: - if (peerPermissions.allowInboundBonding(peer) - && (sender.getEnodeURL().getNodeId().equals(packet.getNodeId()))) { - addToPeerTable(peer); + if (peerPermissions.allowInboundBonding(peer)) { final PingPacketData ping = packet.getPacketData(PingPacketData.class).get(); respondToPing(ping, packet.getHash(), peer); + bond(peer); } break; case PONG: @@ -317,12 +316,6 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { }); break; case NEIGHBORS: - if (peer.getStatus() != PeerDiscoveryStatus.BONDED) { - LOG.info( - "Rejecting NEIGHBORS request for unbonded nodes, sender: {}", sender.getEndpoint()); - break; - } - matchInteraction(packet) .ifPresent( interaction -> @@ -330,12 +323,7 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { peer, getPeersFromNeighborsPacket(packet))); break; case FIND_NEIGHBORS: - if (!peerKnown - || !peerPermissions.allowInboundNeighborsRequest(peer) - || (peer.getStatus() != PeerDiscoveryStatus.BONDED)) { - LOG.info( - "Rejecting FIND_NEIGHBORS request for unbonded nodes, sender: {}", - sender.getEndpoint()); + if (!peerKnown || !peerPermissions.allowInboundNeighborsRequest(peer)) { break; } diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java index d9761f893ac..cb455faebae 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java @@ -103,7 +103,7 @@ public void neighborsPacketLimited() { // hedge against missing one and duplicating another. assertThat(agent.streamDiscoveredPeers()).contains(otherPeers.toArray(new DiscoveryPeer[20])); assertThat(agent.streamDiscoveredPeers()) - .allMatch(p -> p.getStatus() == PeerDiscoveryStatus.BONDED); + .allMatch(p -> p.getStatus() == PeerDiscoveryStatus.BONDING); // Use additional agent to exchange messages with agent final MockPeerDiscoveryAgent testAgent = helper.startDiscoveryAgent(); @@ -353,7 +353,7 @@ public void simulatePeerRestartingOnDifferentEndpoint( // Remote agent should have bonded with agent assertThat(agent.streamDiscoveredPeers()).hasSize(1); - assertThat(agent.streamDiscoveredPeers()).contains(remoteAgent.getAdvertisedPeer().get()); + assertThat(agent.streamDiscoveredPeers()).contains(remotePeer); // Create a new remote agent with same id, and new endpoint remoteAgent.stop(); diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java index 22d8796b98d..2c25ccce548 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java @@ -147,16 +147,14 @@ public void bootstrapPeersRetriesSent() { } private void mockPingPacketCreation(final Packet mockPacket) { - mockPacketCreation(PacketType.PING, Optional.empty(), mockPacket); + mockPacketCreation(Optional.empty(), mockPacket); } - private void mockPacketCreation( - final PacketType type, final DiscoveryPeer peer, final Packet mockPacket) { - mockPacketCreation(type, Optional.of(peer), mockPacket); + private void mockPacketCreation(final DiscoveryPeer peer, final Packet mockPacket) { + mockPacketCreation(Optional.of(peer), mockPacket); } - private void mockPacketCreation( - final PacketType type, final Optional peer, final Packet mockPacket) { + private void mockPacketCreation(final Optional peer, final Packet mockPacket) { doAnswer( invocation -> { final Consumer handler = invocation.getArgument(2); @@ -164,7 +162,10 @@ private void mockPacketCreation( return null; }) .when(controller) - .createPacket(eq(type), peer.isPresent() ? matchPingDataForPeer(peer.get()) : any(), any()); + .createPacket( + eq(PacketType.PING), + peer.isPresent() ? matchPingDataForPeer(peer.get()) : any(), + any()); } @Test @@ -276,7 +277,7 @@ public void shouldRespondToPingRequest() { final PingPacketData pingPacketData = PingPacketData.create(localEndpoint, discoPeer.getEndpoint()); final Packet discoPeerPing = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); - mockPacketCreation(PacketType.PING, discoPeer, discoPeerPing); + mockPacketCreation(discoPeer, discoPeerPing); controller.onMessage(discoPeerPing, discoPeer); @@ -307,7 +308,7 @@ public void shouldNotRespondToExpiredPingRequest() { discoPeer.getEndpoint(), Instant.now().getEpochSecond() - PacketData.DEFAULT_EXPIRATION_PERIOD_SEC); final Packet discoPeerPing = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); - mockPacketCreation(PacketType.PING, discoPeer, discoPeerPing); + mockPacketCreation(discoPeer, discoPeerPing); controller.onMessage(discoPeerPing, discoPeer); @@ -471,6 +472,7 @@ public void findNeighborsSentAfterBondingFinished() { assertThat(data.getTarget()).isEqualTo(localPeer.getId()); assertThat(controller.streamDiscoveredPeers()).hasSize(1); + assertThat(controller.streamDiscoveredPeers().findFirst().isPresent()).isTrue(); assertThat(controller.streamDiscoveredPeers().findFirst().get().getStatus()) .isEqualTo(PeerDiscoveryStatus.BONDED); } @@ -594,79 +596,23 @@ public void stopTwice() { } @Test - public void shouldAddNewPeerWhenReceivedPingAndPeerTableBucketIsNotFull() { + public void shouldBondWithNewPeerWhenReceivedPing() { final List peers = createPeersInLastBucket(localPeer, 1); startPeerDiscoveryController(); final Packet pingPacket = mockPingPacket(peers.get(0), localPeer); controller.onMessage(pingPacket, peers.get(0)); - assertThat(controller.streamDiscoveredPeers()).contains(peers.get(0)); + verify(controller, times(1)).bond(peers.get(0)); } @Test - public void shouldNotBondWhenReceivingSpoofedPing() { - final List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(2); - final List peers = helper.createDiscoveryPeers(nodeKeys); - - DiscoveryPeer victim = peers.get(0); - DiscoveryPeer badGuy = peers.get(1); - - final OutboundMessageHandler outboundMessageHandler = mock(OutboundMessageHandler.class); - controller = getControllerBuilder().outboundMessageHandler(outboundMessageHandler).build(); - - controller.start(); - - final PingPacketData pingPacketData = - PingPacketData.create(badGuy.getEndpoint(), localPeer.getEndpoint()); - final Packet pingPacket = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(1)); - controller.onMessage(pingPacket, victim); - assertThat(controller.streamDiscoveredPeers()).doesNotContain(victim); - assertThat(controller.streamDiscoveredPeers()).doesNotContain(badGuy); - } - - /* - udpPorts NOT equal - tcpPorts NOT equal - */ - @Test - public void shouldNotBondWhenReceivingSpoofedPing1() { - final List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(2); - final List peers = helper.createDiscoveryPeers(nodeKeys); - - DiscoveryPeer victim = peers.get(0); - DiscoveryPeer badGuy = peers.get(1); - - final OutboundMessageHandler outboundMessageHandler = mock(OutboundMessageHandler.class); - controller = getControllerBuilder().outboundMessageHandler(outboundMessageHandler).build(); - - controller.start(); - - final PingPacketData pingPacketData = - PingPacketData.create(badGuy.getEndpoint(), localPeer.getEndpoint()); - final Packet pingPacket = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); - controller.onMessage(pingPacket, victim); - assertThat(controller.streamDiscoveredPeers()).contains(victim); - } - - @Test - public void shouldNotBondWhenReceivingSpoofedPing2() { - final List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(2); - final List peers = helper.createDiscoveryPeers(nodeKeys); - - DiscoveryPeer victim = peers.get(0); - DiscoveryPeer badGuy = peers.get(1); - - final OutboundMessageHandler outboundMessageHandler = mock(OutboundMessageHandler.class); - controller = getControllerBuilder().outboundMessageHandler(outboundMessageHandler).build(); - - controller.start(); + public void shouldNotAddNewPeerWhenReceivedPing() { + final List peers = createPeersInLastBucket(localPeer, 1); + startPeerDiscoveryController(); - final PingPacketData pingPacketData = - PingPacketData.create(victim.getEndpoint(), localPeer.getEndpoint()); - final Packet pingPacket = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(1)); - controller.onMessage(pingPacket, victim); - assertThat(controller.streamDiscoveredPeers()).doesNotContain(victim); - assertThat(controller.streamDiscoveredPeers()).doesNotContain(badGuy); + final Packet pingPacket = mockPingPacket(peers.get(0), localPeer); + controller.onMessage(pingPacket, peers.get(0)); + assertThat(controller.streamDiscoveredPeers()).doesNotContain(peers.get(0)); } @Test @@ -680,23 +626,6 @@ public void shouldNotAddSelfWhenReceivedPingFromSelf() { assertThat(controller.streamDiscoveredPeers()).doesNotContain(localPeer); } - @Test - public void shouldAddNewPeerWhenReceivedPingAndPeerTableBucketIsFull() { - final List peers = createPeersInLastBucket(localPeer, 17); - startPeerDiscoveryController(); - // Fill the last bucket. - for (int i = 0; i < 16; i++) { - peerTable.tryAdd(peers.get(i)); - } - - final Packet pingPacket = mockPingPacket(peers.get(16), localPeer); - controller.onMessage(pingPacket, peers.get(16)); - - assertThat(controller.streamDiscoveredPeers()).contains(peers.get(16)); - // The first peer added should have been evicted. - assertThat(controller.streamDiscoveredPeers()).doesNotContain(peers.get(0)); - } - @Test public void shouldNotRemoveExistingPeerWhenReceivedPing() { final List peers = createPeersInLastBucket(localPeer, 1); @@ -734,7 +663,7 @@ public void shouldNotAddNewPeerWhenReceivedPongFromBlacklistedPeer() { List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); PingPacketData pingPacketData = PingPacketData.create(localEndpoint, discoPeer.getEndpoint()); final Packet discoPeerPing = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); - mockPacketCreation(PacketType.PING, discoPeer, discoPeerPing); + mockPacketCreation(discoPeer, discoPeerPing); controller.start(); verify(outboundMessageHandler, times(1)) @@ -751,13 +680,13 @@ public void shouldNotAddNewPeerWhenReceivedPongFromBlacklistedPeer() { nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); pingPacketData = PingPacketData.create(localEndpoint, otherPeer.getEndpoint()); final Packet pingPacket = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); - mockPacketCreation(PacketType.PING, otherPeer, pingPacket); + mockPacketCreation(otherPeer, pingPacket); // Setup ping to be sent to otherPeer2 after neighbors packet is received nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); pingPacketData = PingPacketData.create(localEndpoint, otherPeer2.getEndpoint()); final Packet pingPacket2 = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); - mockPacketCreation(PacketType.PING, otherPeer2, pingPacket2); + mockPacketCreation(otherPeer2, pingPacket2); final Packet neighborsPacket = MockPacketDataFactory.mockNeighborsPacket(discoPeer, otherPeer, otherPeer2); @@ -812,7 +741,7 @@ public void shouldNotBondWithBlacklistedPeer() { List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); PingPacketData pingPacketData = PingPacketData.create(localEndpoint, discoPeer.getEndpoint()); final Packet discoPeerPing = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); - mockPacketCreation(PacketType.PING, discoPeer, discoPeerPing); + mockPacketCreation(discoPeer, discoPeerPing); controller.start(); verify(outboundMessageHandler, times(1)).send(any(), matchPacketOfType(PacketType.PING)); @@ -828,13 +757,13 @@ public void shouldNotBondWithBlacklistedPeer() { nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); pingPacketData = PingPacketData.create(localEndpoint, otherPeer.getEndpoint()); final Packet pingPacket = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); - mockPacketCreation(PacketType.PING, otherPeer, pingPacket); + mockPacketCreation(otherPeer, pingPacket); // Setup ping to be sent to otherPeer2 after neighbors packet is received nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); pingPacketData = PingPacketData.create(localEndpoint, otherPeer2.getEndpoint()); final Packet pingPacket2 = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); - mockPacketCreation(PacketType.PING, otherPeer2, pingPacket2); + mockPacketCreation(otherPeer2, pingPacket2); // Blacklist peer blacklist.add(otherPeer); @@ -867,7 +796,7 @@ public void shouldRespondToNeighborsRequestFromKnownPeer() { final PingPacketData pingPacketData = PingPacketData.create(localEndpoint, discoPeer.getEndpoint()); final Packet discoPeerPing = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); - mockPacketCreation(PacketType.PING, discoPeer, discoPeerPing); + mockPacketCreation(discoPeer, discoPeerPing); controller.start(); verify(outboundMessageHandler, times(1)).send(any(), matchPacketOfType(PacketType.PING)); @@ -907,7 +836,7 @@ public void shouldNotRespondToNeighborsRequestFromUnknownPeer() { final PingPacketData pingPacketData = PingPacketData.create(localEndpoint, discoPeer.getEndpoint()); final Packet discoPeerPing = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); - mockPacketCreation(PacketType.PING, discoPeer, discoPeerPing); + mockPacketCreation(discoPeer, discoPeerPing); controller.start(); verify(outboundMessageHandler, times(1)).send(any(), matchPacketOfType(PacketType.PING)); @@ -946,7 +875,7 @@ public void shouldNotRespondToExpiredNeighborsRequest() { final PingPacketData pingPacketData = PingPacketData.create(localEndpoint, discoPeer.getEndpoint()); final Packet discoPeerPing = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); - mockPacketCreation(PacketType.PING, discoPeer, discoPeerPing); + mockPacketCreation(discoPeer, discoPeerPing); controller.start(); verify(outboundMessageHandler, times(1)).send(any(), matchPacketOfType(PacketType.PING)); @@ -989,7 +918,7 @@ public void shouldNotRespondToNeighborsRequestFromBlacklistedPeer() { final PingPacketData pingPacketData = PingPacketData.create(localEndpoint, discoPeer.getEndpoint()); final Packet discoPeerPing = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); - mockPacketCreation(PacketType.PING, discoPeer, discoPeerPing); + mockPacketCreation(discoPeer, discoPeerPing); controller.start(); verify(outboundMessageHandler, times(1)).send(any(), matchPacketOfType(PacketType.PING)); @@ -1386,18 +1315,11 @@ private List createPeersInLastBucket(final Peer host, final int n return newPeers; } - private PeerDiscoveryController startPeerDiscoveryController( - final DiscoveryPeer... bootstrapPeers) { - return startPeerDiscoveryController(LONG_DELAY_FUNCTION, bootstrapPeers); - } - - private PeerDiscoveryController startPeerDiscoveryController( - final RetryDelayFunction retryDelayFunction, final DiscoveryPeer... bootstrapPeers) { + private void startPeerDiscoveryController(final DiscoveryPeer... bootstrapPeers) { // Create the controller. controller = getControllerBuilder().peers(bootstrapPeers).build(); - controller.setRetryDelayFunction(retryDelayFunction); + controller.setRetryDelayFunction(LONG_DELAY_FUNCTION); controller.start(); - return controller; } static class ControllerBuilder {