Improve banning logic by grouping Ipv6 /56 prefixes (#8199)

Co-Authored-By: Age Manning <Age@AgeManning.com>

Co-Authored-By: Daniel Knopik <daniel@dknopik.de>

Co-Authored-By: João Oliveira <hello@jxs.pt>

beacon_node/lighthouse_network/src/peer_manager/peerdb.rs

@@ -1419,13 +1419,33 @@ pub struct BannedPeersCount {
     banned_peers_per_ip: HashMap<IpAddr, usize>,
 }
 
+/// Normalizes an IP address for grouped ban counting.
+///
+/// For [`IpAddr::V4`] the address is returned unchanged.
+///
+/// For [`IpAddr::V6`] the address is masked to the /56 prefix (typical ISP
+/// allocation size for residential users), zeroing out the lower 72 host bits.
+/// This groups all addresses from the same ISP-allocated /56 subnet under a
+/// single key in the ban counter map, preventing attackers from evading bans
+/// by cycling through addresses in their allocation.
+fn normalize_ip_for_banning(ip: IpAddr) -> IpAddr {
+    match ip {
+        IpAddr::V4(_) => ip,
+        IpAddr::V6(ipv6) => {
+            const MASK: u128 = 0xFFFF_FFFF_FFFF_FF00_0000_0000_0000_0000;
+            IpAddr::V6(std::net::Ipv6Addr::from(u128::from(ipv6) & MASK))
+        }
+    }
+}
+
 impl BannedPeersCount {
     /// Removes the peer from the counts if it is banned. Returns true if the peer was banned and
     /// false otherwise.
     pub fn remove_banned_peer(&mut self, ip_addresses: impl Iterator<Item = IpAddr>) {
         self.banned_peers = self.banned_peers.saturating_sub(1);
         for address in ip_addresses {
-            if let Some(count) = self.banned_peers_per_ip.get_mut(&address) {
+            let normalized_ip = normalize_ip_for_banning(address);
+            if let Some(count) = self.banned_peers_per_ip.get_mut(&normalized_ip) {
                 *count = count.saturating_sub(1);
             }
         }
@@ -1434,7 +1454,8 @@ impl BannedPeersCount {
     pub fn add_banned_peer(&mut self, ip_addresses: impl Iterator<Item = IpAddr>) {
         self.banned_peers = self.banned_peers.saturating_add(1);
         for address in ip_addresses {
-            *self.banned_peers_per_ip.entry(address).or_insert(0) += 1;
+            let normalized_ip = normalize_ip_for_banning(address);
+            *self.banned_peers_per_ip.entry(normalized_ip).or_insert(0) += 1;
         }
     }
 
@@ -1453,8 +1474,9 @@ impl BannedPeersCount {
     /// An IP is considered banned if more than BANNED_PEERS_PER_IP_THRESHOLD banned peers
     /// exist with this IP
     pub fn ip_is_banned(&self, ip: &IpAddr) -> bool {
+        let normalized_ip = normalize_ip_for_banning(*ip);
         self.banned_peers_per_ip
-            .get(ip)
+            .get(&normalized_ip)
             .is_some_and(|count| *count > BANNED_PEERS_PER_IP_THRESHOLD)
     }
 }
@@ -2019,9 +2041,9 @@ mod tests {
         let mut pdb = get_db();
 
         let ip1 = Ipv4Addr::new(1, 2, 3, 4).into();
-        let ip2 = Ipv6Addr::new(1, 2, 3, 4, 5, 6, 7, 8).into();
+        let ip2 = Ipv6Addr::new(1, 2, 3, 0x0400, 5, 6, 7, 8).into();
         let ip3 = Ipv4Addr::new(1, 2, 3, 5).into();
-        let ip4 = Ipv6Addr::new(1, 2, 3, 4, 5, 6, 7, 9).into();
+        let ip4 = Ipv6Addr::new(1, 2, 3, 0x0500, 5, 6, 7, 9).into();
         let ip5 = Ipv4Addr::new(2, 2, 3, 4).into();
 
         let mut peers = Vec::new();
@@ -2304,4 +2326,287 @@ mod tests {
             Score::max_score().score()
         );
     }
+
+    #[test]
+    fn test_normalize_ipv4_unchanged() {
+        let ip = IpAddr::V4(Ipv4Addr::new(192, 168, 1, 1));
+        assert_eq!(normalize_ip_for_banning(ip), ip);
+
+        let ip2 = IpAddr::V4(Ipv4Addr::new(10, 0, 0, 1));
+        assert_eq!(normalize_ip_for_banning(ip2), ip2);
+    }
+
+    #[test]
+    fn test_normalize_ipv6_same_subnet() {
+        let ip1 = IpAddr::V6(Ipv6Addr::new(
+            0x2001, 0x0db8, 0x1234, 0x5678, 0xabcd, 0xef00, 0x0000, 0x0001,
+        ));
+        let ip2 = IpAddr::V6(Ipv6Addr::new(
+            0x2001, 0x0db8, 0x1234, 0x5678, 0xabcd, 0xef00, 0x0000, 0x0002,
+        ));
+
+        let normalized1 = normalize_ip_for_banning(ip1);
+        let normalized2 = normalize_ip_for_banning(ip2);
+
+        assert_eq!(normalized1, normalized2);
+    }
+
+    #[test]
+    fn test_normalize_ipv6_different_subnet() {
+        let ip1 = IpAddr::V6(Ipv6Addr::new(
+            0x2001, 0x0db8, 0x1234, 0x5600, 0x0000, 0x0000, 0x0000, 0x0001,
+        ));
+        let ip2 = IpAddr::V6(Ipv6Addr::new(
+            0x2001, 0x0db8, 0x1234, 0x5700, 0x0000, 0x0000, 0x0000, 0x0001,
+        ));
+
+        let normalized1 = normalize_ip_for_banning(ip1);
+        let normalized2 = normalize_ip_for_banning(ip2);
+
+        assert_ne!(normalized1, normalized2);
+    }
+
+    #[test]
+    fn test_normalize_ipv6_masks_correctly() {
+        let ip = IpAddr::V6(Ipv6Addr::new(
+            0x2001, 0x0db8, 0x1234, 0x5678, 0xabcd, 0xef01, 0x2345, 0x6789,
+        ));
+        let normalized = normalize_ip_for_banning(ip);
+
+        if let IpAddr::V6(ipv6) = normalized {
+            let segments = ipv6.segments();
+            assert_eq!(segments[0], 0x2001);
+            assert_eq!(segments[1], 0x0db8);
+            assert_eq!(segments[2], 0x1234);
+            assert_eq!(segments[3] & 0xFF00, 0x5600);
+            assert_eq!(segments[4], 0x0000);
+            assert_eq!(segments[5], 0x0000);
+            assert_eq!(segments[6], 0x0000);
+            assert_eq!(segments[7], 0x0000);
+        } else {
+            panic!("Expected IPv6 address");
+        }
+    }
+
+    #[test]
+    fn test_ipv6_ban_grouping() {
+        let mut pdb = get_db();
+
+        let base_segments = [0x2001, 0x0db8, 0x1234, 0x5600, 0, 0, 0, 0];
+
+        let mut peers = Vec::new();
+        for i in 0..BANNED_PEERS_PER_IP_THRESHOLD + 2 {
+            let segments = [
+                base_segments[0],
+                base_segments[1],
+                base_segments[2],
+                base_segments[3],
+                i as u16,
+                (i * 2) as u16,
+                (i * 3) as u16,
+                (i * 4) as u16,
+            ];
+            let ip = IpAddr::V6(Ipv6Addr::new(
+                segments[0],
+                segments[1],
+                segments[2],
+                segments[3],
+                segments[4],
+                segments[5],
+                segments[6],
+                segments[7],
+            ));
+            peers.push(connect_peer_with_ips(&mut pdb, vec![ip]));
+        }
+
+        for p in &peers[..BANNED_PEERS_PER_IP_THRESHOLD + 1] {
+            let _ = pdb.report_peer(p, PeerAction::Fatal, ReportSource::PeerManager, "");
+            pdb.inject_disconnect(p);
+        }
+
+        let different_subnet_ip =
+            IpAddr::V6(Ipv6Addr::new(0x2001, 0x0db8, 0x1234, 0x5700, 0, 0, 0, 1));
+        let p_different = connect_peer_with_ips(&mut pdb, vec![different_subnet_ip]);
+
+        assert!(
+            pdb.ban_status(&peers[BANNED_PEERS_PER_IP_THRESHOLD + 1])
+                .is_some()
+        );
+        assert!(pdb.ban_status(&p_different).is_none());
+    }
+
+    #[test]
+    fn test_ipv6_subnet_banning_with_different_addresses() {
+        let mut pdb = get_db();
+
+        let mut peers = Vec::new();
+        for i in 0..BANNED_PEERS_PER_IP_THRESHOLD + 1 {
+            let ip = IpAddr::V6(Ipv6Addr::new(
+                0x2001,
+                0x0db8,
+                0xabcd,
+                0x1200,
+                0x1111 * i as u16,
+                0x2222 * i as u16,
+                0x3333 * i as u16,
+                i as u16,
+            ));
+            peers.push(connect_peer_with_ips(&mut pdb, vec![ip]));
+        }
+
+        for p in &peers {
+            let _ = pdb.report_peer(p, PeerAction::Fatal, ReportSource::PeerManager, "");
+            pdb.inject_disconnect(p);
+        }
+
+        let new_peer_same_subnet = IpAddr::V6(Ipv6Addr::new(
+            0x2001, 0x0db8, 0xabcd, 0x1200, 0xffff, 0xeeee, 0xdddd, 0xcccc,
+        ));
+        let p_new = connect_peer_with_ips(&mut pdb, vec![new_peer_same_subnet]);
+
+        assert!(
+            pdb.ban_status(&p_new).is_some(),
+            "New peer from same /56 subnet should be banned"
+        );
+
+        let new_peer_different_subnet = IpAddr::V6(Ipv6Addr::new(
+            0x2001, 0x0db8, 0xabcd, 0x1300, 0xffff, 0xeeee, 0xdddd, 0xcccc,
+        ));
+        let p_different = connect_peer_with_ips(&mut pdb, vec![new_peer_different_subnet]);
+
+        assert!(
+            pdb.ban_status(&p_different).is_none(),
+            "Peer from different /56 subnet should not be banned"
+        );
+    }
+
+    #[test]
+    fn test_ipv6_vs_ipv4_banning_independence() {
+        let mut pdb = get_db();
+
+        let ipv6_base = [0x2001, 0x0db8, 0x1234, 0x5600];
+        let mut ipv6_peers = Vec::new();
+        for i in 0..BANNED_PEERS_PER_IP_THRESHOLD + 1 {
+            let ip = IpAddr::V6(Ipv6Addr::new(
+                ipv6_base[0],
+                ipv6_base[1],
+                ipv6_base[2],
+                ipv6_base[3],
+                i as u16,
+                0,
+                0,
+                i as u16,
+            ));
+            ipv6_peers.push(connect_peer_with_ips(&mut pdb, vec![ip]));
+        }
+
+        for p in &ipv6_peers {
+            let _ = pdb.report_peer(p, PeerAction::Fatal, ReportSource::PeerManager, "");
+            pdb.inject_disconnect(p);
+        }
+
+        let ipv4_peer = connect_peer_with_ips(&mut pdb, vec![Ipv4Addr::new(1, 2, 3, 4).into()]);
+        assert!(
+            pdb.ban_status(&ipv4_peer).is_none(),
+            "IPv4 peer should not be affected by IPv6 subnet ban"
+        );
+
+        let ipv6_peer_same_subnet = IpAddr::V6(Ipv6Addr::new(
+            ipv6_base[0],
+            ipv6_base[1],
+            ipv6_base[2],
+            ipv6_base[3],
+            0xdead,
+            0xbeef,
+            0xcafe,
+            0xbabe,
+        ));
+        let p6_new = connect_peer_with_ips(&mut pdb, vec![ipv6_peer_same_subnet]);
+        assert!(
+            pdb.ban_status(&p6_new).is_some(),
+            "New IPv6 peer from banned /56 subnet should be banned"
+        );
+    }
+
+    #[test]
+    fn test_ipv6_partial_segment_masking() {
+        let mut pdb = get_db();
+
+        let mut peers = Vec::new();
+        for i in 0..BANNED_PEERS_PER_IP_THRESHOLD + 1 {
+            let ip = IpAddr::V6(Ipv6Addr::new(
+                0x2001,
+                0x0db8,
+                0x5555,
+                0x12ab + i as u16,
+                i as u16,
+                0,
+                0,
+                i as u16,
+            ));
+            peers.push(connect_peer_with_ips(&mut pdb, vec![ip]));
+        }
+
+        for p in &peers {
+            let _ = pdb.report_peer(p, PeerAction::Fatal, ReportSource::PeerManager, "");
+            pdb.inject_disconnect(p);
+        }
+
+        let same_prefix = IpAddr::V6(Ipv6Addr::new(
+            0x2001, 0x0db8, 0x5555, 0x12ff, 0xaaaa, 0xbbbb, 0xcccc, 0xdddd,
+        ));
+        let p_same = connect_peer_with_ips(&mut pdb, vec![same_prefix]);
+        assert!(
+            pdb.ban_status(&p_same).is_some(),
+            "Peer with same /56 prefix should be banned (0x12ab/56 == 0x12ff/56)"
+        );
+
+        let different_prefix = IpAddr::V6(Ipv6Addr::new(
+            0x2001, 0x0db8, 0x5555, 0x13ab, 0xaaaa, 0xbbbb, 0xcccc, 0xdddd,
+        ));
+        let p_different = connect_peer_with_ips(&mut pdb, vec![different_prefix]);
+        assert!(
+            pdb.ban_status(&p_different).is_none(),
+            "Peer with different /56 prefix should not be banned (0x12ab/56 != 0x13ab/56)"
+        );
+    }
+
+    #[test]
+    fn test_ipv6_subnet_unban_clears_all_in_subnet() {
+        let mut pdb = get_db();
+
+        let mut peers = Vec::new();
+        for i in 0..BANNED_PEERS_PER_IP_THRESHOLD + 1 {
+            let ip = IpAddr::V6(Ipv6Addr::new(
+                0x2001, 0x0db8, 0xaaaa, 0xbb00, i as u16, i as u16, i as u16, i as u16,
+            ));
+            peers.push(connect_peer_with_ips(&mut pdb, vec![ip]));
+        }
+
+        for p in &peers {
+            let _ = pdb.report_peer(p, PeerAction::Fatal, ReportSource::PeerManager, "");
+            pdb.inject_disconnect(p);
+        }
+
+        let new_peer = IpAddr::V6(Ipv6Addr::new(
+            0x2001, 0x0db8, 0xaaaa, 0xbb00, 0xdead, 0xbeef, 0xcafe, 0xbabe,
+        ));
+        let p_new = connect_peer_with_ips(&mut pdb, vec![new_peer]);
+        assert!(pdb.ban_status(&p_new).is_some(), "Subnet should be banned");
+
+        for p in &peers {
+            reset_score(&mut pdb, p);
+            pdb.update_connection_state(p, NewConnectionState::Unbanned);
+            let _ = pdb.shrink_to_fit();
+        }
+
+        let another_peer = IpAddr::V6(Ipv6Addr::new(
+            0x2001, 0x0db8, 0xaaaa, 0xbb00, 0x1111, 0x2222, 0x3333, 0x4444,
+        ));
+        let p_another = connect_peer_with_ips(&mut pdb, vec![another_peer]);
+        assert!(
+            pdb.ban_status(&p_another).is_none(),
+            "Subnet should be unbanned after all peers are unbanned"
+        );
+    }
 }

beacon_node/lighthouse_network/tests/rpc_tests.rs

@@ -65,7 +65,7 @@ fn bellatrix_block_large(spec: &ChainSpec) -> BeaconBlock<E> {
 fn test_tcp_status_rpc() {
     // Set up the logging.
     let log_level = "debug";
-    let enable_logging = true;
+    let enable_logging = false;
     let _subscriber = build_tracing_subscriber(log_level, enable_logging);
 
     let rt = Arc::new(Runtime::new().unwrap());
@@ -167,7 +167,7 @@ fn test_tcp_status_rpc() {
 fn test_tcp_blocks_by_range_chunked_rpc() {
     // Set up the logging.
     let log_level = "debug";
-    let enable_logging = true;
+    let enable_logging = false;
     let _subscriber = build_tracing_subscriber(log_level, enable_logging);
 
     let messages_to_send = 6;
@@ -313,7 +313,7 @@ fn test_tcp_blocks_by_range_chunked_rpc() {
 fn test_blobs_by_range_chunked_rpc() {
     // Set up the logging.
     let log_level = "debug";
-    let enable_logging = true;
+    let enable_logging = false;
     let _subscriber = build_tracing_subscriber(log_level, enable_logging);
 
     let slot_count = 32;
@@ -439,7 +439,7 @@ fn test_blobs_by_range_chunked_rpc() {
 fn test_tcp_blocks_by_range_over_limit() {
     // Set up the logging.
     let log_level = "debug";
-    let enable_logging = true;
+    let enable_logging = false;
     let _subscriber = build_tracing_subscriber(log_level, enable_logging);
 
     let messages_to_send = 5;
@@ -543,7 +543,7 @@ fn test_tcp_blocks_by_range_over_limit() {
 fn test_tcp_blocks_by_range_chunked_rpc_terminates_correctly() {
     // Set up the logging.
     let log_level = "debug";
-    let enable_logging = true;
+    let enable_logging = false;
     let _subscriber = build_tracing_subscriber(log_level, enable_logging);
 
     let messages_to_send = 10;
@@ -679,7 +679,7 @@ fn test_tcp_blocks_by_range_chunked_rpc_terminates_correctly() {
 fn test_tcp_blocks_by_range_single_empty_rpc() {
     // Set up the logging.
     let log_level = "trace";
-    let enable_logging = true;
+    let enable_logging = false;
     let _subscriber = build_tracing_subscriber(log_level, enable_logging);
 
     let rt = Arc::new(Runtime::new().unwrap());
@@ -800,7 +800,7 @@ fn test_tcp_blocks_by_range_single_empty_rpc() {
 fn test_tcp_blocks_by_root_chunked_rpc() {
     // Set up the logging.
     let log_level = "debug";
-    let enable_logging = true;
+    let enable_logging = false;
     let _subscriber = build_tracing_subscriber(log_level, enable_logging);
 
     let messages_to_send = 6;
@@ -945,7 +945,7 @@ fn test_tcp_blocks_by_root_chunked_rpc() {
 fn test_tcp_columns_by_root_chunked_rpc_for_fork(fork_name: ForkName) {
     // Set up the logging.
     let log_level = "debug";
-    let enable_logging = true;
+    let enable_logging = false;
     let _subscriber = build_tracing_subscriber(log_level, enable_logging);
     let num_of_columns = E::number_of_columns();
     let messages_to_send = 32 * num_of_columns;
@@ -1135,7 +1135,7 @@ fn test_tcp_columns_by_root_chunked_rpc_gloas() {
 fn test_tcp_columns_by_range_chunked_rpc_for_fork(fork_name: ForkName) {
     // Set up the logging.
     let log_level = "debug";
-    let enable_logging = true;
+    let enable_logging = false;
     let _subscriber = build_tracing_subscriber(log_level, enable_logging);
 
     let messages_to_send = 32;
@@ -1297,7 +1297,7 @@ fn test_tcp_columns_by_range_chunked_rpc_gloas() {
 fn test_tcp_blocks_by_root_chunked_rpc_terminates_correctly() {
     // Set up the logging.
     let log_level = "debug";
-    let enable_logging = true;
+    let enable_logging = false;
     let _subscriber = build_tracing_subscriber(log_level, enable_logging);
 
     let messages_to_send: u64 = 10;
@@ -1511,7 +1511,7 @@ fn goodbye_test(log_level: &str, enable_logging: bool, protocol: Protocol) {
 #[allow(clippy::single_match)]
 fn tcp_test_goodbye_rpc() {
     let log_level = "debug";
-    let enabled_logging = true;
+    let enabled_logging = false;
     goodbye_test(log_level, enabled_logging, Protocol::Tcp);
 }
 
@@ -1520,7 +1520,7 @@ fn tcp_test_goodbye_rpc() {
 #[allow(clippy::single_match)]
 fn quic_test_goodbye_rpc() {
     let log_level = "debug";
-    let enabled_logging = true;
+    let enabled_logging = false;
     goodbye_test(log_level, enabled_logging, Protocol::Quic);
 }
 
@@ -1528,7 +1528,7 @@ fn quic_test_goodbye_rpc() {
 #[test]
 fn test_delayed_rpc_response() {
     // Set up the logging.
-    let _subscriber = build_tracing_subscriber("debug", true);
+    let _subscriber = build_tracing_subscriber("debug", false);
     let rt = Arc::new(Runtime::new().unwrap());
     let spec = Arc::new(spec_with_all_forks_enabled());
 
@@ -1664,7 +1664,7 @@ fn test_delayed_rpc_response() {
 #[test]
 fn test_active_requests() {
     // Set up the logging.
-    let _subscriber = build_tracing_subscriber("debug", true);
+    let _subscriber = build_tracing_subscriber("debug", false);
     let rt = Arc::new(Runtime::new().unwrap());
     let spec = Arc::new(spec_with_all_forks_enabled());