Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 ---
Expand Down
29 changes: 26 additions & 3 deletions zz_relay_source_limit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Loading