feat(kad_dht): enforce IP subnet diversity in k-buckets (#1383)

[yashksaini-coder]

Summary

Closes #1383.

py-libp2p's k-buckets accept peers with no constraint on network origin. An attacker can grind many valid peer IDs (peer ID = multihash(pubkey), free to mint) that land closest to a target key in XOR space, then run them all from a small number of IPs/subnets and occupy the closest-K slots for that key in every honest node's routing table. Because the messages are authentically signed, record-level defenses don't catch this — the attack lives in the routing layer.

This PR enforces IP/subnet diversity in KBucket.add_peer: before admitting a new peer, it is rejected if its globally-routable subnet already holds MAX_PEERS_PER_SUBNET peers in that bucket.

Design

Constant	Default	Meaning
MAX_PEERS_PER_SUBNET	2	max peers sharing a subnet per bucket; <= 0 disables the check
SUBNET_PREFIX_LEN_V4	24	IPv4 grouping prefix
SUBNET_PREFIX_LEN_V6	48	IPv6 grouping prefix

• Only globally-routable addresses are grouped (ipaddress.is_global). Loopback, private (RFC1918/ULA), CGNAT (100.64.0.0/10), link-local, and documentation ranges are all exempt — CI and local testnets are unaffected. Using is_global (rather than enumerating private ranges) keeps behaviour stable across CPython versions.
• DNS-named and relayed (p2p-circuit) peers are exempt. A circuit address exposes the relay's IP, not the peer's, so it is skipped to avoid grouping distinct peers behind a shared relay. DNS names are not resolved in the routing hot path.
• Multi-homed peers are grouped by their first globally-routable address.
• Reject-only, no eviction — matches go-libp2p's TryAddPeer and avoids a churn/DoS vector where an attacker forces out established peers.
• _should_split_bucket now only splits genuinely full buckets, so a subnet rejection (add_peer returning False on a non-full bucket) can no longer trigger a spurious bucket split.

No protocol or wire-format change. Self-contained routing-layer change.

Test plan

• uv run python -m pytest tests/core/kad_dht/test_unit_routing_table.py → 23 passed (16 existing + 7 new)
• New tests cover: subnet saturation rejection, loopback/private exemption, opt-out flag, distinct-subnet acceptance, IPv6 /48 grouping, multi-homed grouping-by-first-global, and the _subnet_key helper (incl. DNS + relay exemption)
• ruff check / ruff format --check clean; mypy clean on changed modules

Open questions for maintainers

1. IPv4 prefix: this ships /24 (stricter than go-libp2p's /16). /24 matches the realistic granularity an attacker rents; /16 over-blocks large shared ISP blocks less aggressively. Prefer /24, or widen to /16?
2. Table-wide cap: go-libp2p also has a table-wide MaxPeersPerIPGroup=3 (needs RoutingTable-level counter bookkeeping). Deferred here as a follow-up — acceptable, or wanted now?
3. Opt-out surface: disable is a module-level constant (MAX_PEERS_PER_SUBNET <= 0) rather than a RoutingTable/KadDHT constructor kwarg. Want a threaded kwarg instead/in addition?
4. Multi-homed posture: grouped by first global address (lenient), the opposite of go-libp2p's reject-if-any-group-full. Confirm this posture.

Non-goals

No crypto-puzzle peer IDs or reputation scoring (spec/ecosystem-level). Path-steering hardening is tracked separately in #1384.

Notes

• Background: [Stretch] Network attack simulation #57, and the eclipse-attack write-up linked from the issue.
• IPv6 uses a fixed /48 (site-allocation boundary); go-libp2p's ASN-based IPv6 grouping is intentionally not ported (it needs a bundled ASN dataset py-libp2p doesn't ship).

[yashksaini-coder]

📖 Context / reasoning: This PR is Layer 1 of a two-part eclipse-hardening effort for the kad-dht. If you'd like the full thought process behind the approach — why subnet diversity here, and why the related #1384 is moving to disjoint lookup paths rather than randomized candidate selection — it's written up in plain language here: #1400 (discussion). Happy to discuss any of the open questions above either here or there.

[acul71]

@seetadev
Thanks @yashksaini-coder for the thorough write-up, the tests, and the context in #1400. I went through the open questions against go-libp2p's kad-dht / kbucket implementation. Here are my maintainer decisions and the reasoning.

---

How go-libp2p does it (reference)

Diversity lives in go-libp2p-kbucket/peerdiversity, wired into kad-dht via RoutingTablePeerDiversityFilter — not in core go-libp2p itself.

Aspect	go-libp2p (when enabled)	This PR
Scope	Routing-table filter at add time	Per k-bucket in KBucket.add_peer
Per-group limit	maxPerCpl = 2 (per CPL)	MAX_PEERS_PER_SUBNET = 2 (per bucket)
Table-wide limit	maxForTable = 3	Not implemented
IPv4 grouping	/16 (legacy Class A → /8)	/24
IPv6 grouping	ASN-based	Fixed /48
Multi-address peer	All addresses checked	First global only
Private / no addr	Still grouped / rejected if no addrs	Exempt via is_global
Configuration	DHT constructor option	Module constants
Default on?	Dual DHT WAN only; plain DHT opt-in	Always on (unless <= 0)

Defaults in go dual DHT:

// go-libp2p-kad-dht/dual/dual.go
maxPrefixCountPerCpl = 2  // per CPL (common prefix length with local node)
maxPrefixCount       = 3  // table-wide per IP group

---

1. IPv4 /24 or /16?

Decision: /24 LGTM for py-libp2p.

go's IPv4 grouping (go-libp2p-kbucket/peerdiversity/filter.go):

// IPv4: legacy Class A → /8, otherwise → /16
rs, _ := f.legacyCidrs.ContainingNetworks(ip)
if len(rs) != 0 {
    key := ip.Mask(net.IPv4Mask(255, 0, 0, 0)).String()
    return PeerIPGroupKey(key)
}
// otherwise -> /16 prefix
key := ip.Mask(net.IPv4Mask(255, 255, 0, 0)).String()
return PeerIPGroupKey(key)

This PR uses /24 via SUBNET_PREFIX_LEN_V4 = 24. That matches realistic attacker economics (a rented cloud subnet is usually a /24, not a /16). go's /16 trades some security for fewer false rejections on large ISP blocks.

Please document the divergence from go in the PR or module comments. Legacy Class A /8 special-casing (go handles 12.0.0.0/8, 17.0.0.0/8, etc.) can be a follow-up if we care about parity — not a blocker for this PR.

---

2. Table-wide cap — now or follow-up?

Decision: merge per-bucket now; please open a follow-up for the table-wide cap.

go has two limits (go-libp2p-kad-dht/rt_diversity_filter.go):

func (r *rtPeerIPGroupFilter) Allow(g peerdiversity.PeerGroupInfo) bool {
    key := g.IPGroupKey
    cpl := g.Cpl

    // TABLE-WIDE: max 3 peers from same IP group anywhere in routing table
    if r.tableIpGroupCount[key] >= r.maxForTable {
        return false
    }

    // PER-CPL: max 2 peers from same IP group at each common-prefix-length
    c, ok := r.cplIpGroupCount[cpl]
    allow := !ok || c[key] < r.maxPerCpl
    return allow
}

Per-bucket limits alone still let an attacker grind peer IDs into many buckets and place 2 peers per bucket from the same /24. go's maxForTable = 3 caps that globally across the whole routing table. That's the bigger gap vs go — more important than the /24 vs /16 debate.

Happy to merge the per-bucket enforcement here; please file a follow-up for table-wide maxForTable=3 at RoutingTable level.

---

3. Module constants vs constructor kwarg?

Decision: module constants in common.py are fine for v1.

go exposes runtime configuration:

// go-libp2p-kad-dht/dht_options.go
func RoutingTablePeerDiversityFilter(pg peerdiversity.PeerIPGroupFilter) Option

// Usage (dual DHT WAN):
dht.RoutingTablePeerDiversityFilter(
    dht.NewRTPeerDiversityFilter(h, maxPrefixCountPerCpl, maxPrefixCount),
)

This PR:

MAX_PEERS_PER_SUBNET = 2
SUBNET_PREFIX_LEN_V4 = 24
SUBNET_PREFIX_LEN_V6 = 48
# Set MAX_PEERS_PER_SUBNET <= 0 to disable

That matches our existing pattern (BUCKET_SIZE, etc. in common.py). A runtime kwarg on KadDHT / RoutingTable would be nice for production tuning without editing source — fine as a small follow-up, not a merge blocker.

---

4. Multi-homed — first global or all addresses?

Decision: first globally-routable address is acceptable for this PR.

go checks every address — reject if any group is full (go-libp2p-kbucket/peerdiversity/filter.go):

peerGroups := make([]PeerGroupInfo, 0, len(addrs))
for _, a := range addrs {
    ip, err := manet.ToIP(a)
    // ...
    key := f.ipGroupKey(ip)
    group := PeerGroupInfo{Id: p, Cpl: cpl, IPGroupKey: key}

    if !f.pgm.Allow(group) {
        return false  // ANY saturated group → reject entire peer
    }
    peerGroups = append(peerGroups, group)
}

This PR (routing_table.py) — first global wins:

def _subnet_key(peer_info: PeerInfo) -> str | None:
    for addr in peer_info.addrs:
        if "p2p-circuit" in str(addr):
            continue
        for proto, prefix_len in (("ip4", SUBNET_PREFIX_LEN_V4), ("ip6", SUBNET_PREFIX_LEN_V6)):
            # ...
            if not ip.is_global:
                continue
            return str(ip_network(f"{ip}/{prefix_len}", strict=False))
    return None

First-global is simpler and avoids false rejects for legit multi-homed peers. Please document that go checks all addresses. A stricter all-global-addresses check is a reasonable follow-up once address ordering is well-defined.

---

Other notes (not blockers, but worth acknowledging)

Always-on vs opt-in: go only enables diversity on dual DHT WAN by default; plain kad-dht is opt-in. Making this always-on in py-libp2p (unless MAX_PEERS_PER_SUBNET <= 0) is a reasonable security default.

Peers with no addresses: go rejects them:

addrs := f.pgm.PeerAddresses(p)
if len(addrs) == 0 {
    return false
}

This PR exempts them (_subnet_key → None). Good for CI/local testnets; slightly looser for production — acceptable for v1 given our exemption story.

IPv6: go uses ASN (asnutil.AsnForIPv6); this PR uses fixed /48 without bundling an ASN dataset — pragmatic and already well documented.

---

Before merge (small fixes)

1. Remove unused import multihash in routing_table.py (ruff flags it).
2. Rebase onto latest main (branch is quite far behind).
3. Add a regression test for _should_split_bucket when subnet rejection happens on a non-full bucket (the fix you describe in the PR description).
4. Optional cleanup: unreachable return False at end of KBucket.add_peer; clearer debug log when rejection is subnet vs bucket-full.

---

Summary: Approve the core design — /24, first-global, module constants. Follow-up issues for table-wide cap and optional runtime config. Thanks again for picking up #1383 and for the clear design write-up.

[yashksaini-coder]

Thanks for going through the go side properly, the comparison table made this easy to act on.

Pushed the fixes:

• dropped the unused multihash import
• removed the dead return False at the tail of add_peer (both branches already return)
• added a regression test for the non-full-bucket case — it splits without the guard and doesn't with it, so it actually pins the fix
• documented the /24-vs-/16 and first-global-vs-all-addresses divergences right where the constants and _subnet_key live

Branch is already on latest main.

And filed the two you flagged:

• table-wide cap (maxForTable): kad-dht: add table-wide IP-group cap to subnet diversity (maxForTable) #1421
• runtime-configurable limits: kad-dht: make subnet-diversity limits runtime-configurable #1422

Agree the table-wide cap is the bigger gap of the two — I'll take that one next once this lands.

[yashksaini-coder]

@acul71 this is ready for another look whenever you have time — all the before-merge items from your review are pushed (unused import, split-guard test, the go-divergence docs) and CI is green. No rush, just flagging it's unblocked on my end.