Skip to content

Commit 9a44092

Browse files
committed
fix(daemon): asymmetric-crypto + outside-window self-recovery
Two related rc3 bugs surfaced live against the list-agents public agent on 2026-05-11. Both have the same shape: one side of an established tunnel ends up with state the other side can't help it recover from, and the only path back was a manual daemon restart. After these fixes the daemon self-heals in under 5 s without operator action. Bug 1 — asymmetric crypto loss has no in-band recovery When a peer drops its Crypto for us (e.g. via DecryptFailDropGrace after a relay-buffered stale-frame burst) but keeps the same X25519 ephemeral, every PILA it retransmits arrives at us with keyChanged=false. The duplicate-PILA branch in HandleAuthFrame cleared pendingRekey and returned silently; the peer's loop never got our reply and was stuck until its X25519 changed (i.e. a daemon restart). Fix: in that branch, also call SendKeyExchangeToNode when InboundDecryptStale(peerNodeID) is true. The constant KeyExchangeReplyStaleThreshold (6 s) was already present with a doc comment describing exactly this behaviour — the wiring was missing. Pinned by TestAsymmetricRecoveryRepliesOnDuplicatePILA WhenStale and TestAsymmetricRecoveryRateLimitsRepliesWhenHealthy. Bug 2 — outside-replay-window divergence has no recovery Sustained ErrOutsideWindow rejections (peer's send counter has drifted far behind our window's high-water-mark) used to be logged and ignored. Triggering rekey on every occurrence storms because the relay re-delivers early-counter frames after every rotate, so the original code deliberately did nothing — at the cost of leaving the receiver wedged. Fix mirrors the existing AEAD-fail-grace pattern: - new OutsideWindowCount field on Crypto, incremented in envelope.DecryptFrame on each ErrOutsideWindow, reset on any successful decrypt; - new OutsideWindowDropThreshold (30) and OutsideWindowDropGrace (3 s) constants; - new Store.ShouldDropOnOutsideWindow(peerNodeID, c) gate; - tunnel.go's ErrOutsideWindow case now calls the gate and on trip does CompareAndDrop + maybeRequestRekey. Combined with the Bug 1 fix, both sides reset cleanly. Pinned by TestOutsideWindowCountIncrementsAndResets, TestShouldDropOnOutside WindowGatesOnThresholdAndGrace, and TestOutsideWindowDropEndToEnd. Also adds TestDialRetryBudgetCleansUpOrphanSynSent pinning the existing dial-retry-budget cleanup contract, useful when triaging "stuck SYN_SENT" reports.
1 parent 0a15e5b commit 9a44092

8 files changed

Lines changed: 558 additions & 10 deletions

File tree

pkg/daemon/envelope/envelope.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,15 @@ func DecryptFrame(store *keyexchange.Store, data []byte) DecryptResult {
155155
err := ErrReplay
156156
if recvCounter < maxN && maxN-recvCounter >= keyexchange.ReplayWindowSize {
157157
err = ErrOutsideWindow
158+
// Track consecutive outside-window rejections. A sustained
159+
// burst means the peer's send counter has diverged from our
160+
// window's high-water-mark too far for in-band recovery —
161+
// only a fresh key exchange resets both sides. The caller
162+
// (L7) consults ShouldDropOnOutsideWindow to enforce the
163+
// threshold + grace gate. Mirrors DecryptFailCount.
164+
c.ReplayMu.Lock()
165+
c.OutsideWindowCount++
166+
c.ReplayMu.Unlock()
158167
}
159168
return DecryptResult{
160169
PeerNodeID: peerNodeID,
@@ -183,11 +192,14 @@ func DecryptFrame(store *keyexchange.Store, data []byte) DecryptResult {
183192
}
184193
}
185194

186-
// Successful decrypt — reset the AEAD failure counter.
195+
// Successful decrypt — reset both fault counters under one lock.
187196
c.ReplayMu.Lock()
188197
if c.DecryptFailCount != 0 {
189198
c.DecryptFailCount = 0
190199
}
200+
if c.OutsideWindowCount != 0 {
201+
c.OutsideWindowCount = 0
202+
}
191203
c.ReplayMu.Unlock()
192204

193205
return DecryptResult{
Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+
package envelope_test
4+
5+
// Regression tests for the outside-replay-window stuck-state bug
6+
// observed in v1.10.0-rc3 against list-agents on 2026-05-11:
7+
//
8+
// list-agents' MaxRecvNonce for our node stayed at 8518 while our
9+
// outbound counter sat at ~400 after a fresh key exchange. Every frame
10+
// we sent was rejected as ErrOutsideWindow. The current code path at
11+
// tunnel.go ErrOutsideWindow case logs and returns — no recovery.
12+
// Tunnel half-dead until a daemon restart on either side.
13+
//
14+
// The fix tracks consecutive outside-window rejections on the Crypto
15+
// (mirroring DecryptFailCount) and surfaces a ShouldDropOnOutsideWindow
16+
// gate that the L7 handler uses to drop + rekey. These tests exercise
17+
// the keyexchange/envelope half of the fix; the tunnel.go wiring is
18+
// covered by integration tests on the daemon.
19+
20+
import (
21+
"errors"
22+
"testing"
23+
"time"
24+
25+
"github.com/TeoSlayer/pilotprotocol/pkg/daemon/envelope"
26+
"github.com/TeoSlayer/pilotprotocol/pkg/daemon/keyexchange"
27+
)
28+
29+
// TestOutsideWindowCountIncrementsAndResets pins the bookkeeping: each
30+
// ErrOutsideWindow bumps OutsideWindowCount; a successful decrypt
31+
// resets it to 0. This is the input ShouldDropOnOutsideWindow consumes.
32+
func TestOutsideWindowCountIncrementsAndResets(t *testing.T) {
33+
t.Parallel()
34+
s := newPeerSetup(t)
35+
36+
// Stash a frame at counter=1.
37+
stale, err := envelope.EncryptFrame(s.localStore, s.peerID, []byte("stale"))
38+
if err != nil {
39+
t.Fatalf("encrypt stale: %v", err)
40+
}
41+
42+
// Advance peer MaxRecvNonce past ReplayWindowSize+1 by encrypting and
43+
// delivering fresh frames so stale (counter=1) lands outside the window.
44+
advance := keyexchange.ReplayWindowSize + 8
45+
for i := 0; i < advance; i++ {
46+
f, err := envelope.EncryptFrame(s.localStore, s.peerID, []byte("advance"))
47+
if err != nil {
48+
t.Fatalf("encrypt advance #%d: %v", i, err)
49+
}
50+
if r := envelope.DecryptFrame(s.peerStore, f[4:]); r.Err != nil {
51+
t.Fatalf("advance decrypt #%d: %v", i, r.Err)
52+
}
53+
}
54+
55+
c := s.peerStore.Get(s.localID)
56+
c.ReplayMu.Lock()
57+
pre := c.OutsideWindowCount
58+
c.ReplayMu.Unlock()
59+
if pre != 0 {
60+
t.Fatalf("OutsideWindowCount=%d before any outside-window event; want 0", pre)
61+
}
62+
63+
// Now deliver the stale frame.
64+
if r := envelope.DecryptFrame(s.peerStore, stale[4:]); !errors.Is(r.Err, envelope.ErrOutsideWindow) {
65+
t.Fatalf("stale decrypt err=%v; want ErrOutsideWindow", r.Err)
66+
}
67+
c.ReplayMu.Lock()
68+
post := c.OutsideWindowCount
69+
c.ReplayMu.Unlock()
70+
if post != 1 {
71+
t.Fatalf("OutsideWindowCount=%d after one outside-window event; want 1", post)
72+
}
73+
74+
// Successful decrypt resets to 0.
75+
f, _ := envelope.EncryptFrame(s.localStore, s.peerID, []byte("fresh"))
76+
if r := envelope.DecryptFrame(s.peerStore, f[4:]); r.Err != nil {
77+
t.Fatalf("fresh decrypt: %v", r.Err)
78+
}
79+
c.ReplayMu.Lock()
80+
post2 := c.OutsideWindowCount
81+
c.ReplayMu.Unlock()
82+
if post2 != 0 {
83+
t.Fatalf("OutsideWindowCount=%d after successful decrypt; want 0", post2)
84+
}
85+
}
86+
87+
// TestShouldDropOnOutsideWindowGatesOnThresholdAndGrace pins the
88+
// recovery trigger: the gate must require BOTH the count threshold
89+
// AND the grace age to elapse. This is what the L7 handler uses to
90+
// decide whether to drop crypto and request a rekey.
91+
func TestShouldDropOnOutsideWindowGatesOnThresholdAndGrace(t *testing.T) {
92+
t.Parallel()
93+
s := newPeerSetup(t)
94+
95+
c := s.peerStore.Get(s.localID)
96+
if c == nil {
97+
t.Fatalf("no Crypto for local on peer side")
98+
}
99+
100+
// Below threshold: never drop, regardless of age.
101+
c.ReplayMu.Lock()
102+
c.OutsideWindowCount = keyexchange.OutsideWindowDropThreshold - 1
103+
c.ReplayMu.Unlock()
104+
// Force age past grace so the only failing check is the threshold.
105+
backdateCreatedAt(c, time.Now().Add(-2*keyexchange.OutsideWindowDropGrace))
106+
if s.peerStore.ShouldDropOnOutsideWindow(s.localID, c) {
107+
t.Fatalf("dropped at count<threshold")
108+
}
109+
110+
// At threshold but within grace: don't drop yet.
111+
c.ReplayMu.Lock()
112+
c.OutsideWindowCount = keyexchange.OutsideWindowDropThreshold
113+
c.ReplayMu.Unlock()
114+
backdateCreatedAt(c, time.Now()) // fresh
115+
if s.peerStore.ShouldDropOnOutsideWindow(s.localID, c) {
116+
t.Fatalf("dropped within grace period")
117+
}
118+
119+
// Threshold met AND grace elapsed: drop.
120+
backdateCreatedAt(c, time.Now().Add(-2*keyexchange.OutsideWindowDropGrace))
121+
if !s.peerStore.ShouldDropOnOutsideWindow(s.localID, c) {
122+
t.Fatalf("did not drop at threshold + past grace")
123+
}
124+
}
125+
126+
// TestOutsideWindowDropEndToEnd is the smoking-gun reproduction of the
127+
// list-agents bug: simulate 30+ consecutive outside-window rejections
128+
// over the grace period, then assert the gate signals drop. Prior to
129+
// the fix the count never moved (no recovery path); now it climbs and
130+
// the gate trips, letting the L7 handler tear down the diverged Crypto.
131+
func TestOutsideWindowDropEndToEnd(t *testing.T) {
132+
t.Parallel()
133+
s := newPeerSetup(t)
134+
135+
// Capture N+1 stale frames before advancing peer's MaxRecvNonce so
136+
// that each one lands outside the window when delivered later.
137+
const stales = keyexchange.OutsideWindowDropThreshold + 1
138+
staleFrames := make([][]byte, stales)
139+
for i := 0; i < stales; i++ {
140+
f, err := envelope.EncryptFrame(s.localStore, s.peerID, []byte("stale"))
141+
if err != nil {
142+
t.Fatalf("encrypt stale #%d: %v", i, err)
143+
}
144+
staleFrames[i] = f
145+
}
146+
147+
// Advance peer's window past all stale counters.
148+
advance := keyexchange.ReplayWindowSize + stales + 8
149+
for i := 0; i < advance; i++ {
150+
f, err := envelope.EncryptFrame(s.localStore, s.peerID, []byte("advance"))
151+
if err != nil {
152+
t.Fatalf("encrypt advance #%d: %v", i, err)
153+
}
154+
if r := envelope.DecryptFrame(s.peerStore, f[4:]); r.Err != nil {
155+
t.Fatalf("advance decrypt #%d: %v", i, r.Err)
156+
}
157+
}
158+
159+
// Backdate Crypto so the grace gate is satisfied at the moment we
160+
// hit the count threshold below.
161+
c := s.peerStore.Get(s.localID)
162+
backdateCreatedAt(c, time.Now().Add(-2*keyexchange.OutsideWindowDropGrace))
163+
164+
// Deliver the stale frames. Each one should produce ErrOutsideWindow
165+
// and bump the counter. ShouldDropOnOutsideWindow should flip true
166+
// once threshold is reached.
167+
for i, f := range staleFrames {
168+
r := envelope.DecryptFrame(s.peerStore, f[4:])
169+
if !errors.Is(r.Err, envelope.ErrOutsideWindow) {
170+
t.Fatalf("stale #%d err=%v; want ErrOutsideWindow", i, r.Err)
171+
}
172+
}
173+
174+
if !s.peerStore.ShouldDropOnOutsideWindow(s.localID, c) {
175+
c.ReplayMu.Lock()
176+
count := c.OutsideWindowCount
177+
c.ReplayMu.Unlock()
178+
t.Fatalf("BUG-2: ShouldDropOnOutsideWindow=false after %d "+
179+
"consecutive ErrOutsideWindow (count=%d, threshold=%d)",
180+
stales, count, keyexchange.OutsideWindowDropThreshold)
181+
}
182+
}
183+
184+
// backdateCreatedAt rewrites the CreatedAt timestamp on a Crypto so
185+
// age-gated tests don't have to sleep through the real grace period.
186+
func backdateCreatedAt(c *keyexchange.Crypto, when time.Time) {
187+
c.CreatedAt = when
188+
}

pkg/daemon/keyexchange/crypto.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,22 @@ const SalvageMaxAge = 5 * time.Second
4444
// L5 concept) and Crypto.DecryptFailCount (per-peer state).
4545
const DecryptFailDropThreshold = 5
4646

47+
// OutsideWindowDropThreshold is how many consecutive outside-replay-window
48+
// rejections cause us to drop the peer's Crypto and trigger a fresh key
49+
// exchange. Tuned conservatively: ReplayWindowSize=256 means an in-spec
50+
// burst of late relay-buffered frames can produce a few outside-window
51+
// rejections normally. 30 consecutive without any successful decrypt is
52+
// strong evidence the peer's send counter is far behind our window
53+
// max-water-mark and no in-band recovery exists. Reset to 0 on success.
54+
const OutsideWindowDropThreshold = 30
55+
56+
// OutsideWindowDropGrace mirrors DecryptFailDropGrace but for the
57+
// outside-window path. Reuses the same 3-second value: a freshly
58+
// installed Crypto can legitimately see a small flurry of stale
59+
// in-flight frames at low counters; only after the new state has had
60+
// time to drain those should we treat the persistence as divergence.
61+
const OutsideWindowDropGrace = 3 * time.Second
62+
4763
// DecryptFailDropGrace is the minimum age a Crypto must reach before
4864
// repeated decrypt failures can drop it. Set above the typical relay RTT
4965
// (≤200 ms) plus a comfortable margin: stale ciphertext from before the
@@ -97,6 +113,18 @@ type Crypto struct {
97113
// decrypt.
98114
DecryptFailCount int
99115

116+
// OutsideWindowCount tracks consecutive frames rejected because their
117+
// counter was more than ReplayWindowSize behind MaxRecvNonce. A few
118+
// of these are normal (late relay-buffered frames). A sustained burst
119+
// means the peer's send counter and our receive window have diverged
120+
// far enough that no in-band recovery is possible — the only signal
121+
// the peer has to know there's a problem is silence, and the only
122+
// fix is a fresh key exchange. Once this counter exceeds
123+
// OutsideWindowDropThreshold AND the Crypto is older than
124+
// OutsideWindowDropGrace, the daemon drops this Crypto and requests
125+
// a rekey. Reset to 0 on any successful decrypt.
126+
OutsideWindowCount int
127+
100128
// CreatedAt is when this Crypto was installed. Used by L5's
101129
// handleAuthKeyExchange to decide between preserving existing state
102130
// (fresh handshake retransmit/reply within seconds) and resetting it

pkg/daemon/keyexchange/handle.go

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,20 @@ func (m *Manager) HandleAuthFrame(data []byte, from *net.UDPAddr, fromRelay bool
141141
m.SendKeyExchangeToNode(peerNodeID)
142142
m.ClearPendingRekey(peerNodeID)
143143
} else {
144-
// Already had a session and peer didn't rotate — clear any
145-
// outstanding pending state so we don't keep retrying.
144+
// Already had a session and peer didn't rotate. If we have NO
145+
// recent successful decrypt from this peer, our previous reply
146+
// PILA may have been lost OR the peer dropped its crypto for us
147+
// (e.g. via the DecryptFailDropGrace path after a relay-buffered
148+
// stale-frame burst). In either case the peer needs our PILA to
149+
// re-derive the shared secret — silence here is the asymmetric-
150+
// crypto deadlock observed against list-agents on 2026-05-11.
151+
// KeyExchangeReplyStaleThreshold (6 s) gates this so an active
152+
// session with regular inbound traffic cannot trigger a reply
153+
// loop. Don't reinstall locally — preserves our nonce counter
154+
// and replay window for in-flight encrypted traffic.
155+
if m.InboundDecryptStale(peerNodeID) {
156+
m.SendKeyExchangeToNode(peerNodeID)
157+
}
146158
m.ClearPendingRekey(peerNodeID)
147159
}
148160
return true

pkg/daemon/keyexchange/store.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,36 @@ func (s *Store) ShouldDropOnDecryptFail(peerNodeID uint32, c *Crypto) bool {
179179
return current == c
180180
}
181181

182+
// ShouldDropOnOutsideWindow mirrors ShouldDropOnDecryptFail for the
183+
// outside-replay-window divergence path. Returns true when the peer's
184+
// OutsideWindowCount has reached OutsideWindowDropThreshold AND the
185+
// Crypto is older than OutsideWindowDropGrace AND it is still the
186+
// currently-installed entry for peerNodeID.
187+
//
188+
// The caller (L7 handleEncrypted) is responsible for dropping the
189+
// Crypto via CompareAndDrop and triggering a fresh key exchange when
190+
// this returns true. This is the symmetric-state recovery for the
191+
// case where our receive-window's high-water-mark has drifted so far
192+
// past the peer's actual send counter that no in-band signal exists.
193+
// Reproduces the rc3 list-agents bug on 2026-05-11 where MaxRecvNonce
194+
// stayed at 8518 while the sender's outbound counter sat at ~400.
195+
func (s *Store) ShouldDropOnOutsideWindow(peerNodeID uint32, c *Crypto) bool {
196+
if c == nil {
197+
return false
198+
}
199+
c.ReplayMu.Lock()
200+
rejections := c.OutsideWindowCount
201+
c.ReplayMu.Unlock()
202+
if rejections < OutsideWindowDropThreshold {
203+
return false
204+
}
205+
if time.Since(c.CreatedAt) < OutsideWindowDropGrace {
206+
return false
207+
}
208+
current := s.Get(peerNodeID)
209+
return current == c
210+
}
211+
182212
// RecordSalvage stashes a plaintext send into the per-peer ring buffer.
183213
// On a subsequent peer-initiated rekey, DrainSalvage will hand back the
184214
// entries; the caller re-encrypts with the new key and re-sends —

0 commit comments

Comments
 (0)