diff --git a/server.go b/server.go index d041812..434ed2b 100644 --- a/server.go +++ b/server.go @@ -1311,6 +1311,24 @@ func (s *Server) reapStaleNodes() { } return true }) + + // dstNodes had no Delete anywhere: entries were Stored in the discover + // handler and only ever Loaded on the relay path. The node id comes + // straight off the wire (data[0:4]) in an unauthenticated datagram, so + // the key space is the full uint32 range and an attacker chose it + // freely — steady, unbounded growth on a process that is embedded in + // the production registry. + // + // Same treatment as nodePubKeys directly above: an entry is only + // meaningful while the node is still known, so drop it once the node + // has aged out of s.nodes. + s.dstNodes.Range(func(k, _ interface{}) bool { + id, ok := k.(uint32) + if ok && !s.nodes.Has(id) { + s.dstNodes.Delete(k) + } + return true + }) } // --- Gossip --- diff --git a/zz_relay_source_limit_test.go b/zz_relay_source_limit_test.go index f71303e..68c63b7 100644 --- a/zz_relay_source_limit_test.go +++ b/zz_relay_source_limit_test.go @@ -49,14 +49,37 @@ func TestRelayBudgetIsPerDatagramSource(t *testing.T) { // Three times the per-source budget, every packet claiming a // different sender id. const attempts = maxRelaysPerSourcePerSecond * 3 + start := time.Now() for i := 0; i < attempts; i++ { s.dispatchRelay(relayFrame(uint32(i+1), dest, "x"), relaySourceForUDP(src)) } + elapsed := time.Since(start) got := drainRelayCh(s) - if got > maxRelaysPerSourcePerSecond { - t.Fatalf("one datagram source enqueued %d relays with rotating sender ids; budget is %d per second", - got, maxRelaysPerSourcePerSecond) + + // The budget is a 1-second window (allow() rolls the window when + // nowNano-windowStart >= 1s), so the ceiling has to account for how + // long the loop actually took. dispatchRelay reads the real clock — + // allow() takes nowNano but the caller supplies time.Now() — so a + // loaded CI runner that spends 2.3s in this loop legitimately opens + // three windows and passes ~3x the per-window budget. + // + // Asserting a flat budget assumed the whole burst fit inside one + // window. That held on a fast dev box (~0.25s) and failed on CI, + // where this test has been red on main since it landed. The property + // under test is that rotating the SENDER ID does not multiply the + // budget — not how fast the machine is. + windows := int(elapsed/time.Second) + 1 + ceiling := maxRelaysPerSourcePerSecond * windows + if got > ceiling { + t.Fatalf("one datagram source enqueued %d relays with rotating sender ids in %v; "+ + "budget is %d per second and at most %d window(s) elapsed (ceiling %d)", + got, elapsed, maxRelaysPerSourcePerSecond, windows, ceiling) + } + // Independently of timing, the limiter must actually limit: without + // per-source keying every one of the attempts would be enqueued. + if got >= attempts { + t.Fatalf("all %d relays enqueued — rotating the sender id bypassed the per-source budget entirely", got) } if got == 0 { t.Fatalf("no relays enqueued at all; the limiter rejected everything")