From aaea743e15a598913e29379490f3182932df3714 Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Tue, 28 Jul 2026 17:37:09 +0300 Subject: [PATCH] daemon: stop the KX rate limiter wedging key exchange permanently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit allowKxFromSource tracked one bucket per source IP in kxRateLim and never removed any. There was no delete(tm.kxRateLim, ...) anywhere in the package — the only deletes in tunnel.go are for relayKxLim, lastPendDropLog, lastRekeyReq, pending and peers. So once maxPerSrcKxEntries (4096) distinct source IPs had EVER sent a PILA/PILK frame, this branch: if len(tm.kxRateLim) >= maxPerSrcKxEntries { return false } refused every subsequent new source IP for the life of the process. Key exchange with any new peer was permanently dead, with no recovery short of a restart. A peer spraying spoofed source IPs fills all 4096 slots in a single burst, which turns a rate limiter into a remote, permanent denial of service. The correct pattern was already thirty lines away: the sibling relay limiter calls pruneRelayKxLocked before rejecting. This mirrors it exactly with pruneKxRateLimLocked, reusing relayKxPruneAge (10s). Pruning costs no rate-limiting accuracy. A bucket idle for 10s has long since refilled — perSourceKxLimit tokens accrue every second — so dropping it is equivalent to keeping it: the next frame from that IP recreates it with a full budget. The cap still holds when the table is genuinely active. Tests: a full table of stale entries must still admit a new peer (fails against the unfixed code); a full table of ACTIVE entries must still refuse, so the prune is a reclaim and not a hole in the cap; and the per-source budget is unchanged. The pre-existing TestKxRateLimiterMaxEntries still passes. Found during a codebase-wide sweep for safety mechanisms that silently fail to engage — the same class as the dead idle-timeout that OOM-killed the service fleet. Co-Authored-By: Claude Opus 5 --- pkg/daemon/tunnel.go | 30 ++++++++- pkg/daemon/zz_kx_ratelimit_prune_test.go | 84 ++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 pkg/daemon/zz_kx_ratelimit_prune_test.go diff --git a/pkg/daemon/tunnel.go b/pkg/daemon/tunnel.go index b8947c08..aa62b8a8 100644 --- a/pkg/daemon/tunnel.go +++ b/pkg/daemon/tunnel.go @@ -274,8 +274,20 @@ func (tm *TunnelManager) allowKxFromSource(addr *net.UDPAddr) bool { b, ok := tm.kxRateLim[key] now := time.Now() if !ok { + // Prune before rejecting. Without this the map only ever grows: + // there was no delete for kxRateLim anywhere, so once + // maxPerSrcKxEntries distinct source IPs had EVER sent a PILA/PILK + // frame, every subsequent new source IP was refused forever and key + // exchange with any new peer was permanently dead until restart. + // A peer spraying spoofed source IPs filled all 4096 slots in a + // single burst, turning a rate limiter into a self-inflicted + // denial of service. Mirrors pruneRelayKxLocked, which the sibling + // relay limiter has had all along. if len(tm.kxRateLim) >= maxPerSrcKxEntries { - return false + tm.pruneKxRateLimLocked(now) + if len(tm.kxRateLim) >= maxPerSrcKxEntries { + return false + } } tm.kxRateLim[key] = &srcKxBucket{tokens: perSourceKxLimit - 1, lastFill: now} return true @@ -336,6 +348,22 @@ func (tm *TunnelManager) allowKxFromRelaySrc(srcNodeID uint32) bool { return false } +// pruneKxRateLimLocked drops per-source-IP key-exchange buckets that have +// been idle longer than relayKxPruneAge. Caller must hold kxRateLimMu. +// +// A bucket idle that long has fully refilled (perSourceKxLimit tokens per +// second, so it refills in well under a second), which makes dropping it +// exactly equivalent to keeping it — the next frame from that IP +// re-creates it with a full budget. So pruning costs no rate-limiting +// accuracy and is purely a reclaim. +func (tm *TunnelManager) pruneKxRateLimLocked(now time.Time) { + for ip, b := range tm.kxRateLim { + if now.Sub(b.lastFill) >= relayKxPruneAge { + delete(tm.kxRateLim, ip) + } + } +} + func (tm *TunnelManager) pruneRelayKxLocked(now time.Time) { for id, b := range tm.relayKxLim { if now.Sub(b.lastFill) >= relayKxPruneAge { diff --git a/pkg/daemon/zz_kx_ratelimit_prune_test.go b/pkg/daemon/zz_kx_ratelimit_prune_test.go new file mode 100644 index 00000000..d18c5434 --- /dev/null +++ b/pkg/daemon/zz_kx_ratelimit_prune_test.go @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package daemon + +import ( + "fmt" + "net" + "testing" + "time" +) + +// TestKxRateLimitDoesNotWedgePermanently is the regression guard for a +// self-inflicted denial of service in allowKxFromSource. +// +// kxRateLim had no delete anywhere in the package: entries were inserted +// per source IP and never removed. Once maxPerSrcKxEntries (4096) distinct +// source IPs had EVER sent a PILA/PILK frame, the `len >= max` branch +// returned false for every new IP forever, so key exchange with any new +// peer was permanently dead until the process restarted. An attacker +// spraying spoofed source IPs filled all 4096 slots in a single burst. +// +// Against the unfixed code this test fails: the fresh IP is refused. +func TestKxRateLimitDoesNotWedgePermanently(t *testing.T) { + tm := &TunnelManager{kxRateLim: make(map[string]*srcKxBucket)} + + // Fill the table to capacity with stale entries, as a burst of spoofed + // source IPs would. lastFill is backdated past relayKxPruneAge so they + // are all reclaimable. + stale := time.Now().Add(-10 * relayKxPruneAge) + for i := 0; i < maxPerSrcKxEntries; i++ { + ip := fmt.Sprintf("10.%d.%d.%d", (i>>16)&0xFF, (i>>8)&0xFF, i&0xFF) + tm.kxRateLim[ip] = &srcKxBucket{tokens: perSourceKxLimit, lastFill: stale} + } + if len(tm.kxRateLim) != maxPerSrcKxEntries { + t.Fatalf("setup: got %d entries, want %d", len(tm.kxRateLim), maxPerSrcKxEntries) + } + + // A brand-new legitimate peer must still be able to key-exchange. + fresh := &net.UDPAddr{IP: net.ParseIP("203.0.113.7"), Port: 4000} + if !tm.allowKxFromSource(fresh) { + t.Fatal("new source IP refused while the table was full of stale entries — " + + "key exchange is permanently wedged (this is the DoS)") + } + + if len(tm.kxRateLim) >= maxPerSrcKxEntries { + t.Fatalf("table did not shrink after prune: %d entries", len(tm.kxRateLim)) + } +} + +// TestKxRateLimitStillRefusesWhenAllEntriesAreFresh confirms the prune is a +// reclaim of idle buckets, not a hole in the cap. When every slot is +// genuinely active, the limit must still hold. +func TestKxRateLimitStillRefusesWhenAllEntriesAreFresh(t *testing.T) { + tm := &TunnelManager{kxRateLim: make(map[string]*srcKxBucket)} + + now := time.Now() + for i := 0; i < maxPerSrcKxEntries; i++ { + ip := fmt.Sprintf("10.%d.%d.%d", (i>>16)&0xFF, (i>>8)&0xFF, i&0xFF) + tm.kxRateLim[ip] = &srcKxBucket{tokens: perSourceKxLimit, lastFill: now} + } + + fresh := &net.UDPAddr{IP: net.ParseIP("203.0.113.8"), Port: 4000} + if tm.allowKxFromSource(fresh) { + t.Fatal("cap not enforced: a new source was admitted while all 4096 buckets were active") + } +} + +// TestKxRateLimitPerSourceBudgetStillApplies guards the actual rate limit: +// a single source may not exceed perSourceKxLimit frames in one refill +// window. +func TestKxRateLimitPerSourceBudgetStillApplies(t *testing.T) { + tm := &TunnelManager{kxRateLim: make(map[string]*srcKxBucket)} + addr := &net.UDPAddr{IP: net.ParseIP("198.51.100.4"), Port: 4000} + + allowed := 0 + for i := 0; i < perSourceKxLimit*3; i++ { + if tm.allowKxFromSource(addr) { + allowed++ + } + } + if allowed > perSourceKxLimit { + t.Fatalf("per-source budget exceeded: %d allowed, limit %d", allowed, perSourceKxLimit) + } +}