Add broadcast address to PingPacketData from filter (#6456)

* make list of ping packet data 'from' address which we ignore
* add test coverage

Signed-off-by: garyschulte <garyschulte@gmail.com>

ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgent.java

@@ -74,6 +74,8 @@ public abstract class PeerDiscoveryAgent {
   // 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 List<String> PING_PACKET_SOURCE_IGNORED =
+      List.of("127.0.0.1", "255.255.255.255");
 
   protected final List<DiscoveryPeer> bootstrapPeers;
   private final List<PeerRequirement> peerRequirements = new CopyOnWriteArrayList<>();
@@ -286,29 +288,7 @@ public abstract class PeerDiscoveryAgent {
             .flatMap(Endpoint::getTcpPort)
             .orElse(udpPort);
 
-    // If the host is present in the P2P PING packet itself, use that as the endpoint. If the P2P
-    // PING packet specifies 127.0.0.1 (the default if a custom value is not specified with
-    // --p2p-host or via a suitable --nat-method) we ignore it in favour of the UDP source address.
-    // The likelihood is that the UDP source will be 127.0.0.1 anyway, but this reduces the chance
-    // of an unexpected change in behaviour as a result of
-    // https://github.com/hyperledger/besu/issues/6224 being fixed.
-    final String host =
-        packet
-            .getPacketData(PingPacketData.class)
-            .flatMap(PingPacketData::getFrom)
-            .map(Endpoint::getHost)
-            .filter(
-                fromAddr ->
-                    (!fromAddr.equals("127.0.0.1") && InetAddresses.isInetAddress(fromAddr)))
-            .stream()
-            .peek(
-                h ->
-                    LOG.trace(
-                        "Using \"From\" endpoint {} specified in ping packet. Ignoring UDP source host {}",
-                        h,
-                        sourceEndpoint.getHost()))
-            .findFirst()
-            .orElseGet(sourceEndpoint::getHost);
+    final String host = deriveHost(sourceEndpoint, packet);
 
     // Notify the peer controller.
     final DiscoveryPeer peer =
@@ -323,6 +303,38 @@ public abstract class PeerDiscoveryAgent {
     controller.ifPresent(c -> c.onMessage(packet, peer));
   }
 
+  /**
+   * method to derive the host from the source endpoint and the P2P PING packet. If the host is
+   * present in the P2P PING packet itself, use that as the endpoint. If the P2P PING packet
+   * specifies 127.0.0.1 (the default if a custom value is not specified with --p2p-host or via a
+   * suitable --nat-method) we ignore it in favour of the UDP source address. Some implementations
+   * send 127.0.0.1 or 255.255.255.255 anyway, but this reduces the chance of an unexpected change
+   * in behaviour as a result of https://github.com/hyperledger/besu/issues/6224 being fixed.
+   *
+   * @param sourceEndpoint source endpoint of the packet
+   * @param packet P2P PING packet
+   * @return host address as string
+   */
+  static String deriveHost(final Endpoint sourceEndpoint, final Packet packet) {
+    return packet
+        .getPacketData(PingPacketData.class)
+        .flatMap(PingPacketData::getFrom)
+        .map(Endpoint::getHost)
+        .filter(
+            fromAddr ->
+                (!PING_PACKET_SOURCE_IGNORED.contains(fromAddr)
+                    && InetAddresses.isInetAddress(fromAddr)))
+        .stream()
+        .peek(
+            h ->
+                LOG.trace(
+                    "Using \"From\" endpoint {} specified in ping packet. Ignoring UDP source host {}",
+                    h,
+                    sourceEndpoint.getHost()))
+        .findFirst()
+        .orElseGet(sourceEndpoint::getHost);
+  }
+
   /**
    * Send a packet to the given recipient.
    *

ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java

@@ -36,6 +36,7 @@ import org.hyperledger.besu.ethereum.p2p.discovery.internal.MockPeerDiscoveryAge
 import org.hyperledger.besu.ethereum.p2p.discovery.internal.NeighborsPacketData;
 import org.hyperledger.besu.ethereum.p2p.discovery.internal.Packet;
 import org.hyperledger.besu.ethereum.p2p.discovery.internal.PacketType;
+import org.hyperledger.besu.ethereum.p2p.discovery.internal.PingPacketData;
 import org.hyperledger.besu.ethereum.p2p.peers.DefaultPeer;
 import org.hyperledger.besu.ethereum.p2p.peers.EnodeURLImpl;
 import org.hyperledger.besu.ethereum.p2p.peers.Peer;
@@ -53,6 +54,7 @@ import com.google.common.base.Supplier;
 import com.google.common.base.Suppliers;
 import org.apache.tuweni.bytes.Bytes;
 import org.apache.tuweni.bytes.Bytes32;
+import org.apache.tuweni.units.bigints.UInt64;
 import org.ethereum.beacon.discovery.schema.NodeRecord;
 import org.junit.jupiter.api.Test;
 
@@ -834,6 +836,47 @@ public class PeerDiscoveryAgentTest {
     assertThat(agent.isActive()).isFalse();
   }
 
+  @Test
+  public void assertHostCorrectlyRevertsOnIgnoredPacketFrom() {
+    final String sourceHost = "UDP_SOURCE_ORIGIN_HOST";
+    final String localHost = "127.0.0.1";
+    final String broadcastDefaultHost = "255.255.255.255";
+    final String routableHost = "50.50.50.50";
+
+    Endpoint source = new Endpoint(sourceHost, 30303, Optional.empty());
+    Endpoint endpointLocal = new Endpoint(localHost, 30303, Optional.empty());
+    Endpoint endpointBroadcast = new Endpoint(broadcastDefaultHost, 30303, Optional.empty());
+    Endpoint endpointRoutable = new Endpoint(routableHost, 30303, Optional.empty());
+
+    Packet mockLocal =
+        when(mock(Packet.class).getPacketData(any()))
+            .thenReturn(
+                Optional.of(
+                    PingPacketData.create(Optional.of(endpointLocal), endpointLocal, UInt64.ONE)))
+            .getMock();
+    Packet mockBroadcast =
+        when(mock(Packet.class).getPacketData(any()))
+            .thenReturn(
+                Optional.of(
+                    PingPacketData.create(
+                        Optional.of(endpointBroadcast), endpointLocal, UInt64.ONE)))
+            .getMock();
+    Packet mockWellFormed =
+        when(mock(Packet.class).getPacketData(any()))
+            .thenReturn(
+                Optional.of(
+                    PingPacketData.create(
+                        Optional.of(endpointRoutable), endpointLocal, UInt64.ONE)))
+            .getMock();
+
+    // assert a pingpacketdata from address of 127.0.0.1 reverts to the udp source host
+    assertThat(PeerDiscoveryAgent.deriveHost(source, mockLocal)).isEqualTo(sourceHost);
+    // assert that 255.255.255.255 reverts to the udp source host
+    assertThat(PeerDiscoveryAgent.deriveHost(source, mockBroadcast)).isEqualTo(sourceHost);
+    // assert that a well-formed routable address in the ping packet data is used
+    assertThat(PeerDiscoveryAgent.deriveHost(source, mockWellFormed)).isEqualTo(routableHost);
+  }
+
   protected void bondViaIncomingPing(
       final MockPeerDiscoveryAgent agent, final MockPeerDiscoveryAgent otherNode) {
     final Packet pingPacket = helper.createPingPacket(otherNode, agent);