daemon: reliability + policy hardening (WSS half-open, watchdog restart-loop, port-policy fail-closed)#388
Open
TeoSlayer wants to merge 4 commits into
Open
daemon: reliability + policy hardening (WSS half-open, watchdog restart-loop, port-policy fail-closed)#388TeoSlayer wants to merge 4 commits into
TeoSlayer wants to merge 4 commits into
Conversation
The idle keepalive called conn.Ping(lifetimeCtx) — unbounded — while holding writeMu. On a silently half-open conn (stale NAT/relay mapping, half-open TCP), the pong never arrives, so the ping blocked forever: the keepalive loop wedged, every Send froze behind writeMu, and the dead beacon link was never detected. WSS-path analogue of the L4 rx-watchdog. Bound each ping with IdlePingTimeout (default 10s, well under the 30s interval). On timeout, force the conn closed so the supervisor's drainReads unblocks and reconnect fires; clear t.conn so Send fails fast. Test: TestIdlePing_HalfOpenConnTriggersReconnect drives a beacon that goes silent after auth (never pongs); asserts reconnect. wss green -race. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
loadNetworkPolicies rebuilt the port-policy cache from scratch each call and 'continue'd past any GetNetworkPolicy error, dropping that network's entry. isPortAllowed reads a missing entry as allow-all (empty list == no restriction), so a network that DOES restrict ports went silently unrestricted on any transient registry hiccup at load — a fail-OPEN hole. It also discarded a successful-empty response identically, making 'registry failed' indistinguishable from 'no restriction'. Now: snapshot the prior cache; on a failed fetch retain the last-known policy (fail-closed) with a bounded retry + Warn + network.policy_load_ failed event; on success store the result verbatim, including an empty list (the only thing that legitimately relaxes a restriction). The merge decision is extracted as the pure mergeNetworkPolicies so the fail-closed invariant is unit-tested (TestMergeNetworkPolicies_FailClosed) without a live registry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The hard exit (code 86 → launchd/systemd respawn) assumes a restart CLEARS the inbound wedge (stale NAT/relay mapping). When it can't — a NAT/firewall/topology change no restart fixes — the daemon boot-loops forever: wedge → 3 soft attempts → exit → respawn → wedge → …, every ~30-40 min, dropping every live tunnel each cycle while the registry heartbeat keeps the unit 'active' so nothing alerts. Add a cross-restart circuit breaker. Hard exits are recorded to a sidecar file next to the identity (survives the respawn). Once rxWedgeLoopMax (3) exits have occurred within rxWedgeLoopWindow (2h), the breaker OPENS: withhold further exits, stay up, keep soft-recovering, and emit tunnel.rx_wedge_restart_loop so an operator sees the stuck host. Empty IdentityPath (no persistence) disables the breaker — behaviour unchanged there. Tests: breaker opens at the threshold (no exit), stays closed below it (exit fires + is recorded), ignores exits outside the window, is off without persistence; plus the sidecar parser's window/malformed handling. pkg/daemon green under -race. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
The path comes from the daemon's own IdentityPath, not peer input; annotate so the diff-scoped gosec gate passes. Co-Authored-By: Claude Opus 4.8 <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
3 participants
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.
Three independent daemon fixes surfaced by a liveness/fail-open sweep, each verified against current `main` (post-#387) with tests.
1. WSS idle-ping bounded + reconnect on half-open beacon conn
`idlePing` called `conn.Ping(lifetimeCtx)` — unbounded — while holding `writeMu`. On a silently half-open conn (stale NAT/relay mapping, half-open TCP), the pong never arrives, so the ping blocked forever: the keepalive loop wedged, every `Send` froze behind `writeMu`, and the dead beacon link was never detected (`drainReads` sits on a Read that never returns; `Send` only errors once there's traffic). This is the WSS-transport analogue of the L4 rx-watchdog's silent-inbound wedge.
Now each ping is bounded by `IdlePingTimeout` (default 10s, well under the 30s interval); on timeout the conn is force-closed so the supervisor's `drainReads` unblocks and reconnect fires. Test: `TestIdlePing_HalfOpenConnTriggersReconnect`.
2. rx-watchdog restart-loop circuit breaker
The hard exit (code 86 → supervisor respawn) assumes a restart CLEARS the wedge. When it can't (a NAT/firewall/topology change no restart fixes), the daemon boot-loops every ~30-40 min forever, dropping every live tunnel each cycle, while the TCP registry heartbeat keeps the unit "active" so nothing alerts.
Now hard exits are recorded to a sidecar file next to the identity (survives respawn). After `rxWedgeLoopMax` (3) exits within `rxWedgeLoopWindow` (2h) the breaker opens: stay up, keep soft-recovering, emit `tunnel.rx_wedge_restart_loop`. No persistence ⇒ breaker off (unchanged behaviour). Tests cover open/closed/stale-window/no-persistence + the sidecar parser.
3. Network port policy fails CLOSED on registry error
`loadNetworkPolicies` `continue`d past any `GetNetworkPolicy` error, dropping that network's entry; `isPortAllowed` reads a missing entry as allow-all — so a restricted network went silently unrestricted on any transient registry hiccup (fail-OPEN). It also discarded a successful-empty response identically.
Now: snapshot the prior cache; a failed fetch retains the last-known policy (fail-closed) with bounded retry + `network.policy_load_failed` event; a successful fetch is authoritative including an empty list (the only thing that relaxes a restriction). Decision extracted as the pure `mergeNetworkPolicies`, unit-tested by `TestMergeNetworkPolicies_FailClosed`.
Testing
`pkg/daemon/...` green under `-race`; the two changed transport/daemon suites verified with the race detector. No behaviour change on the happy path.
Ships toward v1.12.7 (v1.12.6 already released with #381/#374/#387).
🤖 Generated with Claude Code