From 539a26d0da2c746a320aff4cd0ca2ca2ee59e91e Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Wed, 22 Jul 2026 18:58:28 +0300 Subject: [PATCH] daemon: path watchdog resets a peer the moment rekey gives up (faster T2 recovery) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 2026-07-20 director probe (T2) documented the worst reliability failure: after a session-key desync, peers exchange 'encrypted packet but no key' + 'rekey retransmit gave up after maxRekeyAttempts' in a loop, and specialist replies arrive undecryptable and are dropped — nothing surfaces to the caller (a silent 'no reply'). Plain rekey-retransmit can't recover it: it resends the key exchange to the same stale cached endpoint. The per-peer path watchdog CAN (resetPeerPath re-resolves a fresh endpoint + re-punches), but only fired after the ~85s inbound-silence probe budget. Wire the rekey-gave-up state (keyexchange.PeerInRekeyGaveUp) as a fast reset trigger in pathWatchPeer: a gave-up peer is an unambiguous dead session, so reset it on the current ~10s tick instead of waiting out the probe budget. Still honours the post-reset cooldown so a genuinely offline peer can't cause a reset storm. Cuts T2 recovery from ~85s to one tick. Test: TestPathWatchRekeyGaveUpResetsImmediately (resets on gave-up before the probe budget; cooldown suppresses a second reset). pkg/daemon + keyexchange green under -race. Co-Authored-By: Claude Opus 4.8 --- pkg/daemon/keyexchange/keyexchange.go | 8 ++++++ pkg/daemon/pathwatch.go | 28 +++++++++++++++++++- pkg/daemon/tunnel.go | 10 +++++++ pkg/daemon/zz_pathwatch_test.go | 38 +++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 1 deletion(-) diff --git a/pkg/daemon/keyexchange/keyexchange.go b/pkg/daemon/keyexchange/keyexchange.go index 87263757..5f685e19 100644 --- a/pkg/daemon/keyexchange/keyexchange.go +++ b/pkg/daemon/keyexchange/keyexchange.go @@ -554,6 +554,14 @@ func (m *Manager) SetLastInboundDecryptForTest(peerNodeID uint32, t time.Time) { m.rkPendingMu.Unlock() } +// MarkRekeyGaveUpForTest stamps the give-up map so PeerInRekeyGaveUp +// returns true. Test-only. +func (m *Manager) MarkRekeyGaveUpForTest(peerNodeID uint32) { + m.rkPendingMu.Lock() + m.rekeyGaveUp[peerNodeID] = time.Now() + m.rkPendingMu.Unlock() +} + // LastInboundDecryptHas reports whether we've recorded any successful // decrypt timestamp for the peer. func (m *Manager) LastInboundDecryptHas(peerNodeID uint32) bool { diff --git a/pkg/daemon/pathwatch.go b/pkg/daemon/pathwatch.go index 53ef2a99..d4dfba4b 100644 --- a/pkg/daemon/pathwatch.go +++ b/pkg/daemon/pathwatch.go @@ -152,6 +152,32 @@ func (d *Daemon) pathWatchPeer(nodeID uint32, st *pathPeerState, now time.Time) return pathActionSkip } silence := now.Sub(last) + + // Fast path for the T2 desync (2026-07-20 probe): when the rekey + // machinery has GIVEN UP on a peer, the session is unambiguously dead + // — both sides hold mismatched keys and every inbound frame is + // dropped undecryptable. Plain rekey-retransmit can't recover it (it + // resends the key exchange to the same stale cached endpoint); a path + // reset can, because it re-resolves a fresh endpoint and re-punches. + // Reset immediately rather than waiting out the ~85s inbound-silence + // probe budget — but still honour the post-reset cooldown so a truly + // offline peer can't cause a reset storm. + inCooldown := !st.lastResetAt.IsZero() && now.Sub(st.lastResetAt) < pathResetCooldown + if d.tunnels.PeerInRekeyGaveUp(nodeID) && !inCooldown { + slog.Warn("path watchdog: peer rekey gave up (session desync) — resetting path now", + "peer_node_id", nodeID, + "silent_for", silence.Truncate(time.Second).String()) + d.publishEvent("tunnel.path_suspect", map[string]any{ + "peer_node_id": nodeID, + "silent_for_seconds": int64(silence.Seconds()), + "reason": "rekey_gave_up", + }) + pathWatchResetPeer(d, nodeID) + st.probesSent = 0 + st.lastResetAt = now + return pathActionReset + } + if silence < pathSilenceThreshold { if st.probesSent > 0 { slog.Info("path watchdog: peer path recovered", @@ -170,7 +196,7 @@ func (d *Daemon) pathWatchPeer(nodeID uint32, st *pathPeerState, now time.Time) // Inside the post-reset cooldown: don't re-probe/re-reset a peer // that is likely just offline. - if !st.lastResetAt.IsZero() && now.Sub(st.lastResetAt) < pathResetCooldown { + if inCooldown { return pathActionCooldown } diff --git a/pkg/daemon/tunnel.go b/pkg/daemon/tunnel.go index e11701e8..9e24d19e 100644 --- a/pkg/daemon/tunnel.go +++ b/pkg/daemon/tunnel.go @@ -428,6 +428,16 @@ func (tm *TunnelManager) ClearRekeyGaveUp(peerNodeID uint32) { tm.kx.ClearRekeyGaveUp(peerNodeID) } +// PeerInRekeyGaveUp reports whether the rekey machinery has given up on +// this peer (retransmitted the key-exchange to a stale cached endpoint +// MaxRekeyAttempts times without success). It is the fast, unambiguous +// "session is dead" signal the per-peer path watchdog uses to reset a +// desynced peer immediately, instead of waiting out the inbound-silence +// probe budget. Thin shim over keyexchange.Manager. +func (tm *TunnelManager) PeerInRekeyGaveUp(peerNodeID uint32) bool { + return tm.kx.PeerInRekeyGaveUp(peerNodeID) +} + // ClearLastRekeyReq drops the per-peer rate-limit timestamp recorded by // maybeRequestRekey. After a forced reset (pilotctl prefer-direct) the // next "encrypted packet but no key" event must be allowed to fire a diff --git a/pkg/daemon/zz_pathwatch_test.go b/pkg/daemon/zz_pathwatch_test.go index 38c89aa1..f3e53294 100644 --- a/pkg/daemon/zz_pathwatch_test.go +++ b/pkg/daemon/zz_pathwatch_test.go @@ -225,3 +225,41 @@ func TestResetPeerPathPreservesRelayActive(t *testing.T) { t.Fatal("expected resolve error with no registry connection") } } + +// TestPathWatchRekeyGaveUpResetsImmediately pins the T2 fast-path: a peer +// whose rekey machinery has given up is a dead session, so the watchdog +// resets it on the current tick instead of waiting out the inbound- +// silence probe budget — but still honours the post-reset cooldown. +func TestPathWatchRekeyGaveUpResetsImmediately(t *testing.T) { + const peer = 91 + d := newPathWatchTestDaemon(t, peer) + resets := swapPathResetForTest(t) + now := time.Now() + + // A gave-up peer is inbound-silent (that's why it desynced). Set + // silence PAST the threshold but leave probesSent=0: without the fast + // path this tick would only send the first probe (probesSent 0->1); + // with it, the gave-up signal resets immediately. Asserting Reset with + // probesSent still 0 proves the fast path beats the probe budget. + d.tunnels.kx.SetLastInboundDecryptForTest(peer, now.Add(-2*pathSilenceThreshold)) + d.tunnels.kx.MarkRekeyGaveUpForTest(peer) + + st := &pathPeerState{} + if got := d.pathWatchPeer(peer, st, now); got != pathActionReset { + t.Fatalf("rekey-gave-up action = %q, want %q (should reset now, not probe)", got, pathActionReset) + } + if len(*resets) != 1 || (*resets)[0] != peer { + t.Fatalf("resets = %v, want [%d]", *resets, peer) + } + if st.lastResetAt.IsZero() { + t.Fatal("reset must stamp cooldown") + } + + // Immediately after, still gave-up but inside cooldown → no second reset. + if got := d.pathWatchPeer(peer, st, now.Add(time.Second)); got != pathActionCooldown { + t.Fatalf("in-cooldown action = %q, want %q (no reset storm)", got, pathActionCooldown) + } + if len(*resets) != 1 { + t.Fatalf("cooldown must suppress a second reset, resets=%v", *resets) + } +}