daemon: path probes and legacy keepalives prove liveness (stop spurious path resets) - #469
Merged
Merged
Conversation
…us path resets) On a v1.13.9 laptop the path watchdog reset healthy relay peers (16392, 242944, 179172) about 90s into every process. Nothing those peers sent was allowed to count as inbound liveness, so a quiet peer always looked dead. Two authenticated frame types were dropped by the Src identity binding in handleEncrypted before recordInboundDecrypt ran: 1. The path probe's pong. SendPathProbe left Dst unset. Every deployed handleControlPacket (checked v1.10.0, v1.12.0, v1.13.9) builds the pong with Src = ping.Dst, so the pong claimed node 0 and was dropped as spoofed. No probe could prove a path alive. Fix: set Dst to the peer. No wire change: the pong now names the peer, which it always should have. 2. The NAT keepalive from peers older than v1.12.1, which carries a zero Src (the Src stamp arrived in v1.12.1, one release after the binding). A quiet old peer looked inbound-silent 55s after every handshake, and PktsRecv never moved for the rx watchdog. Fix: a narrow exemption for an authenticated, empty ProtoControl/PortPing frame with Src.Node 0. It claims no node, is never delivered (isTunnelKeepalive drops it before recvCh) and still runs the liveness side effects. Any other frame whose Src differs from the authenticated peer is still dropped and still publishes security.src_spoofed. This is the trigger half of #465 only. It does not change what a path reset does to the session (keys, reset probe, replay windows, responder); that redesign continues in #465. Tests (zz_rekey_desync_recovery_test.go, two real daemons over loopback plus a fake registry where needed). All fail on main and pass here: - TestPathProbePongRefreshesLiveness - TestLegacyUnstampedKeepaliveCountsAsLiveness (includes a zero-Src frame with a payload that is still dropped) - TestSpoofedSourceStillDroppedNextToLegacyKeepaliveExemption: the one exempt shape passes; keepalive shape naming another node, zero-Src ping with payload, zero-Src control to another port, zero-Src stream and datagram frames to the ping port are all dropped, with no liveness, no PktsRecv, no delivery and a src_spoofed event - TestPathWatchdogDoesNotResetHealthyQuietPeer: a current peer that answers the probe goes probe -> healthy with no reset (main: probe, probe, probe, reset); a v1.12.0-style peer sending zero-Src keepalives stays healthy on every tick with no probe and no reset (main: probe, probe, probe, reset). Each subtest fails when only its own fix is removed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Path probes from v1.13.0-v1.13.9 (and v1.13.10-rc.1) leave Dst unset. handleControlPacket answered with pong.Src = ping.Dst, so a patched node still sent those probers a pong claiming node 0. The old prober's identity binding dropped it as spoofed, and each pong also refreshed our lastOutboundSend toward the prober, which held back our own keepalive. The old prober's watchdog therefore reset its path to a healthy patched node about 30s after any 55s gap in inbound, the same spurious reset the previous commit stops for patched probers. When the ping's Dst.Node is 0, the pong now names us (the tunnel's node ID, which is what the prober's binding compares against). A Dst that names a node is echoed unchanged. The only ping senders that leave Dst unset are those old path probes, so nothing else changes. No wire change. Tests (zz_rekey_desync_recovery_test.go): - TestPongToLegacyPathProbeRefreshesProberLiveness (new): a probe built exactly as v1.13.9's SendPathProbe goes to a peer running this build's real handler. The pong names the peer, passes the prober's identity binding and refreshes its lastInboundDecrypt. A ping whose Dst names a node gets that Dst back unchanged. Fails when the responder fix is removed. - TestPathProbePongRefreshesLiveness now also asserts the probe's Dst names the peer, since this build's responder would otherwise mask a missing Dst. - TestPathWatchdogDoesNotResetHealthyQuietPeer gains a "v1.13.9 peer answers the probe" subtest whose responder builds the pong exactly as every deployed handleControlPacket does (pong.Src = ping.Dst). It pins the probe fix on its own: without it, [probe probe probe reset]. The v1.12.0 subtest now uses that deployed responder too. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
TeoSlayer
pushed a commit
that referenced
this pull request
Sep 24, 2026
…roxy Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR contains only the two trigger fixes from #465 (its first commit, e9d2301), plus a third, responder-side fix for mixed fleets (067794d). They stop the path watchdog from resetting healthy, quiet peers. The recovery redesign in #465 (keeping keys across a reset, the reset probe, per-epoch replay windows, responder changes) is not included. That work continues in #465, which is now a draft.
The incident is written up in #465. On a v1.13.9 laptop, the path watchdog reset relay peers 16392, 242944 and 179172 about 90s into every process. The log showed
tunnel: dropping frame with spoofed source node ... claimed_src=0about 180ms after each watchdog probe (the pongs) and every ~30s (the old peers' keepalives). Nothing those peers sent counted as inbound liveness, so a quiet peer always looked dead. On main, a reset also drops our session keys, and that is what made each reset permanent. This PR removes the reason the healthy peers were reset. It does not change what a reset does.The fixes
1. Path probes name the peer (
SendPathProbe,pkg/daemon/tunnel.go)SendPathProbeleftDstunset. Every deployedhandleControlPacketbuilds the pong withSrc: pkt.Dst. I checked the v1.10.0, v1.12.0 and v1.13.9 tags, and main does the same. So the pong claimed node 0, and the Src identity binding inhandleEncrypteddropped it as spoofed beforerecordInboundDecryptran. No probe could ever prove a path alive. The fix setsDst: {Node: peerNodeID}.Dst(handlePacket/handleControlPacketonly echo it into the pong), so old peers handle the probe exactly as before.2. Legacy zero-Src keepalives count as liveness (
isLegacyUnstampedKeepalive,handleEncrypted)Peers older than v1.12.1 send their NAT keepalive with a zero Src. The binding arrived in v1.12.0 (#294) and the sender-side Src stamp in v1.12.1 (#303). Those keepalives were dropped as spoofed, so a quiet old peer looked inbound-silent 55s after every handshake, and
PktsRecvnever moved for the rx watchdog.The exemption is deliberately narrow. It covers only an authenticated frame (AEAD already verified) that is an empty
ProtoControlframe toPortPingwithSrc.Node == 0. That frame:isTunnelKeepalivedrops it beforerecvCh, so it never reacheshandlePacket/AddPeereither);Every other frame whose Src differs from the authenticated peer is still dropped and still publishes
security.src_spoofed. That includes a keepalive-shaped frame that names a different node.3. The pong to a Dst-less probe names the responder (
handleControlPacket,pkg/daemon/daemon.go)Fix 1 only helps when the prober runs this build. Nodes on v1.13.0–v1.13.9 (and v1.13.10-rc.1) still send probes with no
Dst, so a patched responder still answered them with a pong claiming node 0. The old prober dropped it as spoofed, and each pong also refreshed the responder'slastOutboundSendtoward the prober, which held back the responder's own keepalive. After any 55s inbound gap (for example two keepalives lost to a relay hiccup), the old node's watchdog reset its path to a healthy patched node.When the ping's
Dst.Nodeis 0, the pong now uses our own tunnel node ID as itsSrc.Node. That is the ID the prober's identity binding compares against. ADstthat names a node is echoed unchanged, as before. The only ping senders that leaveDstunset are those old path probes: keepalives are empty and never reachhandleControlPacket, the relay and direct-upgrade probes carryFlagACKand get no reply, andpilotctl pinguses a stream to the echo port. v1.10–v1.12 have no path watchdog. No wire change.What this PR does not change
resetPeerPath/RemovePeerare unchanged: a reset still drops our session keys. daemon: path reset keeps the session so rekey desync can recover without a restart #465 continues that work.Tests
The tests are in
pkg/daemon/zz_rekey_desync_recovery_test.goand use two real daemons over loopback (realreadLoop, and either this build's real ping handler or a responder that builds the pong exactly as every deployed version does,pong.Src = ping.Dst). All of them fail on main and pass here:TestPathProbePongRefreshesLivenessSrc), and the peer's real pong refresheslastInboundDecryptprobe Dst.Node = 0TestPongToLegacyPathProbeRefreshesProberLivenessSendPathProbe(noDst) gets a pong from this build's real handler that names the peer, passes the prober's identity binding and refreshes itslastInboundDecrypt; a ping whoseDstnames a node gets thatDstback unchangedTestLegacyUnstampedKeepaliveCountsAsLivenessPktsRecvand is not delivered; a zero-Src frame with a payload is still droppedTestSpoofedSourceStillDroppedNextToLegacyKeepaliveExemptionPktsRecv, no delivery and asrc_spoofedevent: a keepalive shape naming another node, a zero-Src ping with a payload, a zero-Src control frame to another port, and zero-Src stream and datagram frames to the ping portTestPathWatchdogDoesNotResetHealthyQuietPeer/current peer answers the probe[probe probe probe reset]TestPathWatchdogDoesNotResetHealthyQuietPeer/v1.13.9 peer answers the probepong.Src = ping.Dst)[probe probe probe reset]TestPathWatchdogDoesNotResetHealthyQuietPeer/v1.12.0 peer sending zero-Src keepalives[probe probe probe reset]Each fix has a test that fails when only that fix is removed:
Dst):TestPathProbePongRefreshesLivenessand the v1.13.9 watchdog subtest ([probe probe probe reset]) fail. The current-peer subtest still passes, because fix 3 covers a patched pair;[probe healthy probe healthy];TestPongToLegacyPathProbeRefreshesProberLivenessfails;[probe probe probe reset].Runs (all with
GOWORK=off):go test -race ./pkg/daemon/...: all 7 packages pass.go test -race -count=10on the tests in this file, and-count=5together with the existingTestPathWatch*,TestHandleControlPacket*andTestHandlePacket*tests: pass, no races.go vet ./...andgofmt -s -lare clean; the pre-commit hooks pass.The existing tests
TestHandleEncryptedDropsSpoofedSrc,TestPathWatch*,TestPathProbeIsNotSwallowedByOldKeepaliveFilterandTestKeepalivePacketStampsOwnSourceare unchanged and pass.Note for #465
The test file uses the same name and the same helpers and test names as #465's copy. When #465 is rebased onto a main that includes this PR, the file conflicts in one place (add/add). Keep #465's version, which is a superset, plus
TestSpoofedSourceStillDroppedNextToLegacyKeepaliveExemption,TestPongToLegacyPathProbeRefreshesProberLiveness,TestPathWatchdogDoesNotResetHealthyQuietPeerand their helpers (answerPings,answerPingsLikeV1139,sendV1120Keepalive,sendV1139PathProbe,sendOverSession,waitForInboundAfter), and this PR'sprobe.Dst.Nodecheck inTestPathProbePongRefreshesLiveness. Thetunnel.gochanges here are identical to the matching hunks in e9d2301. #465 does not touchhandleControlPacket, so fix 3 merges cleanly.Not merged. This is intended for the next stable release.
🤖 Generated with Claude Code