daemon: blackholed peers stay on the relay; the next packet no longer overwrites a cached peer key - #505
Open
TeoSlayer wants to merge 1 commit into
Open
daemon: blackholed peers stay on the relay; the next packet no longer overwrites a cached peer key#505TeoSlayer wants to merge 1 commit into
TeoSlayer wants to merge 1 commit into
Conversation
…writing a cached peer key
Two bugs found while tracing why a node kept losing its path to one
peer (list-agents) on 2026-10-01.
Relay flag cleared on unpin. tryDirectUpgrade runs every
RelayProbeInterval (15 s) for each relay peer. To let a working direct
path win it must unpin a peer the blackhole heuristic pinned, and it
called SetRelayPeerPinned(id, false), which clears the relay flag as
well as the pin. Every blackholed peer was therefore back on its dead
direct path within 15 s and stayed there until three more silent sends
flipped it again: about 75 s of every 90 s spent sending to an address
that never answers. A new routing.UnpinRelayPeer removes only the pin;
ClearRelayOnDirect still moves the peer back after DirectClearsRequired
direct packets.
Cached peer key aliased the receive buffer. HandleAuthFrame sliced the
peer's Ed25519 key out of the frame it was handed and cached that slice.
The frame is the socket's single reused read buffer, so the next packet
overwrote the cached key. The next PILA from the peer mismatched the
cache and triggered a registry lookup on the UDP read loop ("auth key
exchange: peer pubkey updated from registry"). The key is now copied
when parsed, and SetPeerPubKey copies what it is given.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
4 of 5 tasks
TeoSlayer
marked this pull request as ready for review
October 1, 2026 21:57
This branch has not been deployed
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
Two daemon bugs found while tracing why one node lost its path to
list-agentsfor over half an hour on 2026-10-01. Both are onmainand in every release since the code they touch was added.1. A blackholed peer was put back on its dead direct path every 15 s.
tryDirectUpgraderuns eachRelayProbeIntervalfor every relay peer. To let a working direct path win it unpins a peer the blackhole heuristic pinned, and it did so withSetRelayPeerPinned(id, false), which clears the relay flag as well as the pin. Traffic went direct again until three more silent sends tripped the heuristic. In the live log this isdirect path silent, flipping to relayevery ~90 s withsilent_forgrowing past 29 minutes and norelay→direct auto-clearedin between: about 75 s of every 90 s spent sending to an address that never answered.2. A peer's cached Ed25519 key was overwritten by the next packet.
HandleAuthFramecacheddata[36:68], a slice of the frame it was handed. That frame is the socket's single reused receive buffer (udpio.Socket.Recv: "valid only until the next Recv"), so the following packet replaced the cached key with its own bytes. The next PILA from that peer mismatched the cache and forced a registry lookup on the UDP read loop, stalling all packet processing for that round trip. The log lineauth key exchange: peer pubkey updated from registryappeared 97 times in a six-hour session, for peers whose registry key never changed.Changes
routing.Manager.UnpinRelayPeerremoves only the pin.tryDirectUpgradeuses it, so the peer stays on the relay untilClearRelayOnDirecthas seenDirectClearsRequireddirect packets from it.HandleAuthFramecopies the key out of the frame when parsing;SetPeerPubKeycopies what it is given, so no other caller can reintroduce the alias.Related, not changed here
The key-bound trust check on the data plane (
Daemon.handshakeTrusts→IsTrustedWithKey, #424) reads this same cache. It never runs in a shipped daemon:runtime.NewHandshakeServiceAdapterdoes not forwardIsTrustedWithKey, so the daemon falls back toIsTrusted(nodeID). Had the adapter forwarded it while the cache held overwritten bytes, every trust record would have been dropped on the peer's first inbound SYN. The adapter gap is a separate decision; this PR removes the hazard so it can be closed safely.Test Plan
TestDirectUpgradeUnpinsWithoutLeavingTheRelay,TestBlackholedPeerStaysOnRelayAcrossProbeTicksandTestCachedPeerKeySurvivesReceiveBufferReusefail onmainand pass hereGOWORK=off go build ./...,go vet ./pkg/daemon/...,go test ./pkg/... ./cmd/... ./internal/... -shortgo test -parallel 4 -count=1 ./tests/: pass (274 s)send-messagecalls 8 s apart:mainThe two early sends on this branch fail at once with
sendto: operation not permitted: a local send error on the direct path fails the dial instead of trying the relay. That is a separate gap, not changed here; it is also whytest_force_relay_*still fail (they send 10 s after the partition).Also visible in that run on
main: the receiving node logsauth key exchange: peer pubkey updated from registrytwice within 5 ms of the first tunnel coming up, with two nodes and a registry that has held one key per node since boot.Checklist
go.mod/go.sumunchanged🤖 Generated with Claude Code