fix(wg): a safety peer's remote re-establishes its WireGuard session on its own - #155
iliabaranov wants to merge 6 commits into
Conversation
… source while DERP-only Bench 2026-09-19: a USB-tethered remote sat DERP-only for 7+ minutes with no valid keypair for the machine host — its handshake initiations went via the relay only and never completed — while the host pinged it DIRECTLY every few seconds (disco both ways on the tether). One host-initiated handshake healed it instantly; an idle wireguard-go never initiates on its own, so the remote must be able to re-establish the session by itself. Now the source of the most recent direct disco ping/pong from a safety peer is kept as a handshake candidate (60 s freshness). While the peer has no direct WG endpoint, each initiation is sent both via DERP and to that candidate: one handshake, two legs, either response completes it, and a direct response re-adopts the direct endpoint (update_peer_addr). Data frames are unchanged. Diag: wg_hs_cand_sends in the monitor.
…ery diagnostics A bare WireGuard initiation never re-adds a peer that the far side's wireguard-go has trimmed for inactivity — only an inbound disco PING does (Tailscale lazy peer configuration; already noted in ml_wg_mgr.c). So the second handshake leg alone cannot recover a remote that stopped pinging its machine host: disco_send_ping_to_peer() now also fans the ping out to the handshake candidate (the peer's last direct disco source) for safety peers, which keeps the host's peer configured and yields the pong the normal direct regain needs even when the candidate table lacks it. /admin/api/peers gains wg_up, eps, best, hs_cand, ping_age_ms, backoff_ms per peer so the next stuck-DERP-only event is attributable from a snapshot; monitor gains disco_hs_cand_pings.
…nects with retries The pong-promotion path stores a regained/first direct endpoint with wireguardif_update_endpoint() (connect_ip only) and calls wireguardif_connect() — which re-points the LIVE endpoint peer->ip and retries the handshake — only when some keypair is still valid. Without one it fell to a deliberate one-shot (then active=false, 'wait for them to initiate'), or to nothing at all for an already-known peer. Right for bulk peers that may have trimmed us; a deadlock for the machine host, whose wireguard-go initiates only when it has data — and it has nothing to send while we are silent. The demote's connect_derp() had left peer->ip at 0.0.0.0, so the remote kept initiating relay-only while disco reported direct: indefinitely with the relay dead or slow. A stale-but- valid keypair took the other branch and recovered in seconds — a lottery (bench 2026-09-19: 10 s, 21 s, 169 s, never). Safety peers now connect() with retries in that branch. Diag: wg_safety_reconnects.
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 5 advisory finding(s) below merit a look before merge.
Graphify review — findings
Adds a WireGuard handshake "second leg" so an idle DERP-only peer can be healed: when a direct disco packet arrives, its source is remembered via wireguardif_set_hs_candidate, and wireguardif_peer_output mirrors handshake initiations to that fresh candidate over UDP alongside the relay, treating the send as successful if either leg lands so a direct response re-adopts the direct endpoint. Exposes the recovery state — candidate endpoint, keypair/handshake/initiation status, ping age, and upgrade backoff — through microlink_get_peer_info and the /peers and /monitor HTTP endpoints (including a wireguardif_hs_cand_sends counter) for after-the-fact path forensics.
Worth a look
- Public peer info struct was enlarged without size negotiation —
components/microlink/include/microlink.h:62· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Handshake initiation mirrored to unvalidated disco source enables reflection/amplification abuse —
components/microlink/components/wireguard_lwip/src/wireguardif.c:179· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Direct discovery ping source can steer WireGuard handshake candidate —
components/microlink/src/ml_wg_mgr.c:2935· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- New peer diagnostics can be returned uninitialized —
components/microlink/src/microlink.c:850· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Handshake candidate is read and written without synchronization —
components/microlink/components/wireguard_lwip/src/wireguardif.c:1039· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 414 functions depend on the 359 functions this change touches.
Health — this change adds coupling hotspots:
- new:
disco_send_ping_to_peer()— 8 callers, 8 callees - new:
dcs_support_init()— 2 callers, 30 callees - new:
add_peer()— 2 callers, 22 callees - new:
wireguardif_network_rx()— 3 callers, 11 callees - new:
disco_send_call_me_maybe()— 6 callers, 4 callees - new:
neg_apply_target()— 2 callers, 10 callees - new:
microlink_init()— 1 callers, 17 callees - new:
disco_periodic_probes()— 1 callers, 16 callees - …and 39 more — each is listed as a finding
Verification — 414 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 413 function(s) in the blast radius were not formally verified this run
· 47 more finding(s) on lines outside this diff (see the check run).
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline finding on uninitialized diagnostic fields, I also looked at the unconditional wireguardif_connect() call for safety peers with a direct endpoint but no session (ml_wg_mgr.c around line 3182): it only fires from the per-pong "no session" branch, and wireguardif_connect() re-points peer->ip/sets active=true so once a session forms it stops re-firing on this path — the retry cadence is governed by WireGuard's own REKEY_TIMEOUT handshake logic rather than pong frequency, so the higher initiation volume this introduces looks intentional rather than a bug.
Extended reasoning...
A confirmed inline finding already exists for this PR (uninitialized wg_active/wg_send_hs/wg_hs_pending/wg_init_age_ms fields in microlink_get_peer_info() when the netif/peer-index/dev checks fail), so this run does not approve or issue a plain zero-findings defer. I additionally examined the previously-flagged-and-ruled-out concern about wireguardif_connect() being called unconditionally on every safety-peer pong with a direct endpoint and no session, to see whether it independently warranted escalation. Reading the surrounding code in ml_wg_mgr.c, the call sits inside the "no session" branch only; once wireguardif_connect() establishes active=true and a handshake succeeds, subsequent pongs for that peer take the earlier is_up branch instead, and retry cadence is bounded by WireGuard's internal REKEY_TIMEOUT logic rather than being re-triggered per pong. This matches the PR's stated intent (unlimited retries for the machine peer are deliberate) and does not by itself justify a separate finding, so I'm noting it only as something checked and ruled out rather than restating it as new.
Pongs arrive several times a second and wireguardif_connect() resets the pending handshake state; without pacing a slow path could keep answering an already-superseded initiation. One connect per 5 s per peer.
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 4 advisory finding(s) below merit a look before merge.
Graphify review — findings
Adds a WireGuard handshake "second leg" so a DERP-only safety peer can still complete a handshake: wireguard_peer now remembers the source of the most recent direct disco packet (hs_cand_ip/port/ms), and wireguardif_peer_output mirrors handshake initiations to that fresh candidate over the direct UDP path in addition to the relay, treating either leg succeeding as success (counted in wireguardif_hs_cand_sends, gated to a 60s freshness window). Adds wireguardif_set_hs_candidate/wireguardif_get_hs_candidate to record and read back that candidate, and paces a keyless-regain wireguardif_connect via last_safety_connect_ms. Surfaces per-peer path-recovery diagnostics (endpoint count, chosen direct endpoint, handshake candidate, ping/init ages, backoff, and the WG session flags wg_up/wg_active/wg_send_hs/wg_hs_pending) through microlink_get_peer_info and the /peers HTTP handler.
Worth a look
- hs_cand second-leg send can direct WG initiation to unvalidated attacker-controlled endpoint —
components/microlink/components/wireguard_lwip/src/wireguardif.c:178· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- hs_cand IP passed to udp_output_fn in host order without ntohl unlike getter —
components/microlink/components/wireguard_lwip/src/wireguardif.c:184· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Peer diagnostics read WireGuard peer internals without core locking —
components/microlink/src/microlink.c:871· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- New peer-info diagnostics are left uninitialized when no WG peer is available —
components/microlink/src/microlink.c:862· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 426 functions depend on the 364 functions this change touches.
Health — this change adds coupling hotspots:
- new:
disco_send_ping_to_peer()— 8 callers, 8 callees - new:
dcs_support_init()— 2 callers, 30 callees - new:
add_peer()— 2 callers, 22 callees - new:
wireguardif_network_rx()— 3 callers, 11 callees - new:
disco_send_call_me_maybe()— 6 callers, 4 callees - new:
noise_send()— 7 callers, 3 callees - new:
neg_apply_target()— 2 callers, 10 callees - new:
microlink_init()— 1 callers, 17 callees - …and 42 more — each is listed as a finding
Verification — 426 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 425 function(s) in the blast radius were not formally verified this run
· 50 more finding(s) on lines outside this diff (see the check run).
There was a problem hiding this comment.
Beyond the existing inline finding, I also checked the new 5s pacing gate added in the latest commit (b11c9592a38f, process_disco_pong) for a race where a second direct pong arriving within that window could fall through to the bulk-peer one-shot branch and flip a safety peer's active flag back to false, silently reproducing the deadlock this PR fixes. It self-heals: last_safety_connect_ms is only updated by the branch that fires the real retry, so it is untouched by that stray path, and the safety peer resumes retrying within one REKEY_TIMEOUT (5s) regardless.
Extended reasoning...
This run's bug hunter reported no new findings and my own prior inline comment on microlink.c (uninitialized wg_active/wg_send_hs/wg_hs_pending/wg_init_age_ms when the peer isn't yet WG-mapped) remains unaddressed by the latest commit — I verified b11c9592a38f only touched microlink_internal.h and ml_wg_mgr.c (the pacing fix), not microlink.c, so that thread stays open and I am not restating it here. The one new thing worth recording is that I independently traced the harness's ruled-out candidate (a second pong within 5s of the safety-peer wireguardif_connect() undoing the retry by hitting the bulk one-shot branch and setting active=false) through process_disco_pong in ml_wg_mgr.c around line 3168-3207, and confirmed it is a real but transient blip, not a reproduction of the original deadlock, because the periodic 5s gate (last_safety_connect_ms) that drives ongoing retries is set only by the safety branch itself and is unaffected by the erroneous one-shot path — so retries resume automatically within one REKEY_TIMEOUT. Since an unaddressed prior-review finding still stands, I am not approving; this defer note only adds the one new, concretely-verified item beyond what's already on the thread.
|
Soak result (2026-09-19 12:06–15:47 PDT, 3.7 h, this build + #148–#154): two Ethernet remotes + one USB-tethered remote bonded to a ROS 2 software machine on the tether host, DUT E-stop pressed/released/re-armed every 5 min (42/42 cycles, stop 0.25–0.50 s, re-arm ≤0.75 s). Zero bond drops, zero rebonds, zero resets, 100 % bonded samples on all three (12 638 one-second samples each). Reply age p99.9: 367 / 361 / 391 ms; one 1070 ms excursion on the Ethernet remote 7a60 (a ~1.2 s receive-side stall, TX unaffected — the W5500 RX class, bond held). Machine-side capture: 194 207 requests, 194 170 answered; the 37 unanswered are all late DERP-mirror copies rejected as out-of-order (median 1.3 s late), not losses. |
…rite Review: microlink_get_peer_info() left wg_active/wg_send_hs/wg_hs_pending/ wg_init_age_ms unset when there was no WG peer to read; the candidate tuple is written from the disco task and read on the TCPIP thread — write port last (0 first) so a reader seeing a non-zero port sees the matching ip. Lock-free scalar reads for telemetry documented as deliberate.
|
Graphify escalations, addressed or answered (f0da736):
|
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 4 advisory finding(s) below merit a look before merge.
Graphify review — findings
Adds a WireGuard "handshake second leg": when a DERP-only safety peer has a fresh direct disco source recorded via wireguardif_set_hs_candidate, wireguardif_peer_output mirrors handshake initiations to that direct address as well as the relay, so an idle peer that never re-initiates can still be healed direct, with each mirrored send counted in wireguardif_hs_cand_sends. Exposes per-peer path-recovery diagnostics through microlink_get_peer_info (chosen endpoint, handshake candidate, ping/backoff/init ages, and WG session flags), reading the TCPIP-thread scalars lock-free on purpose to avoid stalling the safety loop, and surfaces them in the peers HTTP endpoint. Adds last_safety_connect_ms to pace keyless-regain wireguardif_connect() calls to the rekey timeout.
Worth a look
- Public peer-info struct ABI expanded without size/version negotiation —
components/microlink/include/microlink.h:65· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Handshake initiation mirrored to unvalidated disco source (spoofable direct leg) —
components/microlink/components/wireguard_lwip/src/wireguardif.c:179· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- hs_cand written from disco task but read on TCPIP thread without any memory barrier —
components/microlink/components/wireguard_lwip/src/wireguardif.c:178· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Non-atomic reset of hs_cand fields loses candidate when writer clears while reader reads —
components/microlink/components/wireguard_lwip/src/wireguardif.c:1050· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 426 functions depend on the 364 functions this change touches.
Health — this change adds coupling hotspots:
- new:
disco_send_ping_to_peer()— 8 callers, 8 callees - new:
dcs_support_init()— 2 callers, 30 callees - new:
add_peer()— 2 callers, 22 callees - new:
wireguardif_network_rx()— 3 callers, 11 callees - new:
disco_send_call_me_maybe()— 6 callers, 4 callees - new:
noise_send()— 7 callers, 3 callees - new:
neg_apply_target()— 2 callers, 10 callees - new:
microlink_init()— 1 callers, 17 callees - …and 42 more — each is listed as a finding
Verification — 426 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 425 function(s) in the blast radius were not formally verified this run
· 50 more finding(s) on lines outside this diff (see the check run).
|
Superseded by #156 (consolidated remote-connectivity PR, same commits cherry-picked linearly; all review findings from this thread are addressed there). Closing. |
There was a problem hiding this comment.
Beyond the inline findings, I also checked whether a safety peer's second direct pong arriving within the new 5s pace window (ml_wg_mgr.c:3168) falls through to the legacy one-shot branch and clears peer->active — it does, but the next pong once the pace window elapses re-arms wireguardif_connect() with retries again, so the dip is bounded to the same 5s window rather than sticking the peer in the one-shot "wait for them to initiate" state.
Extended reasoning...
This run's inline findings (stale last_safety_connect_ms surviving slot reuse in ml_wg_mgr.c, and the racy p->wg_peer_index re-read enabling a dev->peers[-1] OOB read in microlink.c) already signal that human review is warranted, so per the narrow exception for findings-present runs I am only adding one concrete extra check that was performed and ruled out this round: the interaction between the new 5s safety-connect pacer and the pre-existing tried_initial_handshake one-shot branch at ml_wg_mgr.c:3190. Tracing the control flow confirms that a safety peer's rapid successive direct pongs can indeed fall into the legacy one-shot branch (since the safety branch never sets tried_initial_handshake), which explicitly zeroes peer->active — but because last_safety_connect_ms is only updated when the safety branch actually fires, the very next pong past the 5s pace boundary takes the safety branch again and re-enables retries, so the regression is self-limited to a sub-5s window rather than a lasting stuck state like the bug the PR fixes. This was not previously reported by me and had not surfaced in the visible prior-review text, so it is new information worth recording briefly without restating the inline findings.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🔴
components/microlink/src/ml_wg_mgr.c— A safety peer landing in a reused peer-table slot can have its new unlimited-retry handshake connect suppressed for up to 5s right when a bond is being re-established. The new pacer field p->last_safety_connect_ms (microlink_internal.h:617) is never cleared in the!existingslot-reuse block here, unlike every sibling throttle it sits next to (direct_backoff_until, last_derp_reconnect_ms, relay_retry_next_ms, last_cmm_sent_ms) which this same block explicitly resets "on slot REUSE too" for this exact staleness hazard. …Extended reasoning...
…With a stale recent timestamp inherited from the slot's prior occupant, the paced branch at ml_wg_mgr.c:3168 is skipped and control falls to the legacy one-shot branch at ml_wg_mgr.c:3190, which fires once then deliberately clears active — reproducing the stuck-DERP-only bond this PR is meant to fix, for slots reused during exactly the coord-reconnect/re-sync churn it targets. Fix: reset last_safety_connect_ms = 0 alongside the other throttle fields in this block.
wireguardif's paced safety-connect at ml_wg_mgr.c:3168 guards with
now - p->last_safety_connect_ms >= 5000u. last_safety_connect_ms lives in the fixed ml_peer_t table slot and is stamped at ml_wg_mgr.c:3186 whenever wireguardif_connect() fires for a safety peer with a direct pong and no session. When a peer is removed (wireguardif_remove_peer at ml_wg_mgr.c:2105/2172, or eviction) its slot is later reused by ML_PEER_ADD's!existingbranch at ml_wg_mgr.c:2319-2345. That branch resets last_ping_sent_ms, last_pong_recv_ms, direct_backoff_until, last_derp_reconnect_ms, relay_retry_next_ms/count,…Verification: Severity: nit. The claim is factually correct and reachable, but bounded and non-blocking. Verified:
last_safety_connect_ms(microlink_internal.h:617) is read only at ml_wg_mgr.c:3168 and written only at ml_wg_mgr.c:3186 — never cleared elsewhere (Grep: exactly those 3 sites). Peer removal/eviction/re-key-retire only set active=false (2114, 2174) and do NOT zero the slot struct, so a reused…
| if (ml->wg_netif != NULL && p->wg_peer_index >= 0) { | ||
| struct netif * netif = (struct netif *)ml->wg_netif; | ||
| u16_t cport = 0; | ||
| (void)wireguardif_get_hs_candidate(netif, (u8_t)p->wg_peer_index, &info->hs_cand_ip, &cport); | ||
| info->hs_cand_port = cport; | ||
| ip_addr_t cur_ip; | ||
| u16_t cur_port; | ||
| info->wg_up = (wireguardif_peer_is_up(netif, (u8_t)p->wg_peer_index, &cur_ip, &cur_port) == ERR_OK); | ||
| /* Diagnostic snapshot of plain scalars owned by the TCPIP thread, read | ||
| * without the core lock on purpose: a torn read can only yield a stale | ||
| * bool or age for one JSON sample, never a fault, and taking the lock from | ||
| * the HTTP task for telemetry is what stalled the safety loop before. */ | ||
| struct wireguard_device * dev = (struct wireguard_device *)netif->state; | ||
| if (dev != NULL && p->wg_peer_index < WIREGUARD_MAX_PEERS) { | ||
| const struct wireguard_peer * wp = &dev->peers[p->wg_peer_index]; |
There was a problem hiding this comment.
🔴 An HTTP client polling /peers during a concurrent peer remove/re-add can make microlink_get_peer_info() index dev->peers[-1], an out-of-bounds read, which base code never did (this whole block is new). p->wg_peer_index is re-read from the racy ml_peer_t field at lines 866, 869, 873, 879 and 880 instead of being cached once after the >=0 check at 866. If ml_wg_mgr.c:2328 sets p->wg_peer_index=-1 between those reads, the signed compare at line 879 (-1 < WIREGUARD_MAX_PEERS) still passes, and line 880 computes dev->peers[-1], reading backward into wireguard.h:218-234's private_key/cookie_secret region and reporting bits of it as wg_active/wg_send_hs/wg_hs_pending/wg_init_age_ms in the JSON response. …
Extended reasoning...
…Fix: read wg_peer_index into a local int once, validate 0<=idx<WIREGUARD_MAX_PEERS once, and use only that local for every subsequent wireguardif_* call and array index.
handler_get_peers (ml_config_httpd.c) runs on the HTTP task and calls microlink_get_peer_info() with no lock against the wg_mgr task. Step 1: line 866 reads p->wg_peer_index (say 5), passes the >= 0 guard, enters the block. Step 2: concurrently the wg_mgr task removes this peer and ml_wg_mgr.c:2328 sets p->wg_peer_index = -1. Step 3: line 879 re-reads p->wg_peer_index, now -1; -1 < WIREGUARD_MAX_PEERS (128, a plain #define) is a signed comparison and is true, so the guard does not reject it. Step 4: line 880 re-reads p->wg_peer_index again as -1 and evaluates dev->peers[p->wg_peer_index] = dev->peers[-1], stepping backward by sizeof(struct wireguard_peer) bytes from peers[0]. Step 5: struct wireguard_device (wireguard.h:218-234) places label_mac1_key, label_cookie_key, cookie_secret_millis, cookie_secret, private_key and public_key immediately before peers[], so this OOB read lands in that region. Step 6:…
Verification: nit. The scenario is real and reachable but does not fault on this platform. microlink_get_peer_info() (microlink.c:841-889) is deliberately lock-free (comment at 874-877) and re-dereferences p->wg_peer_index fresh at 866, 869, 873, 879 and 880 with no caching. wg_peer_index is a signed int (microlink_internal.h:687) that the wg_mgr task writes to -1 on peer reset/removal (ml_wg_mgr.c:1939 and…
…rd re-establishment, coord deadline (#156) Consolidates #149, #152, #151, #155 and #153 (closed as superseded; their review threads and bot findings were addressed there and the fixes are included here). 12 linear commits, 409+/29−. Every commit was in the build that passed the 3.7 h soak below. ## What is fixed | area | commit(s) | defect | |---|---|---| | USB tether TX | `de335e8` `1b5fe82` | `tinyusb_net_send_sync(…, 0)` reclaimed the packet before the equal-priority TinyUSB task ran the deferred send — ~95 % of device→host frames silently dropped (regression from #135). Now a 20 ms wait; TIMEOUT-after-send duplicate accepted and documented. | | USB tether ring | `3a8a527` | 16-slot ring lost ~1 safety frame/min on a hub-chain host (burst-induced). 64 slots (96 KB PSRAM), 400 ms lifetime. | | Tether link state | `f9ec4cb` | netif never went link-down when the host detached/suspended, so a dead tether stayed the preferred uplink. esp_tinyusb events now drive it; device netif gets its own MAC. | | WiFi failover crash | `9823785` | first time the failover path ever ran it aborted: `dcs_wifi_set_enabled` reaches flash from the supervisor's PSRAM stack. Toggle moved to an internal-stack worker. | | WireGuard re-establishment | `0440ad2` `5b86d21` `e020c86` `cae7e94` `54149eb` `d7a335f` | **root cause of the multi-minute/indefinite bond drops**: on direct regain with no keypair left, the live endpoint stayed 0.0.0.0 (relay-only) and a safety peer fell into the bulk-peer "one-shot, wait for them to initiate" — the machine host's wireguard-go never initiates without data. Safety peers now connect with paced retries; initiations get a second leg to the peer's authenticated direct disco source while DERP-only; disco pings fan out to it. Per-peer WG diagnostics in `/admin/api/peers`. | | Coord partial frame | `8cf5104` | `coord_recv` bounded by a retry count × 2 s socket timeout (≈10 min); now a 10 s wall-clock deadline, `ETIMEDOUT` so callers reconnect (#127). 88-check host test. | ## Evidence - Deterministic WG repro (block direct+DERP, reboot remote mid-outage, unblock direct only, no host help): **v1.3-14 never recovers (>300 s); this branch bonds in 22 s.** - Soak 2026-09-19 12:06–15:47 PDT, 3.7 h, this code + #148/#150/#154, two Ethernet remotes + one USB remote bonded to a ROS 2 machine on the tether host, DUT E-stop cycled every 5 min (42/42, stop ≤0.5 s, re-arm ≤0.75 s): **zero bond drops / rebonds / resets, 100 % bonded samples on all three** (12 638 × 1 Hz each); reply age p99.9 361–391 ms; USB ring 566 916 frames, 0 expired, 0 dropped; machine-side 194 207 requests, 194 170 answered (the rest late DERP-mirror copies, correctly rejected). One 1.07 s receive-side excursion on an Ethernet remote (W5500 RX class, tracked separately). - Baseline for the same class of USB remote on v1.3: 252 ring drops + 55 expiries per 4 h, and 2–7 min bond outages. ## Not in this PR #148 (release guard), #150 (name scrub), #154 (lockstep mismatch diagnostics — SIL comparator, reviewed separately). DERP-mirror latency (1.3 s median) and the W5500 RX stall are follow-ups.
1 participant
The failure
A USB-tethered remote bonded to the machine node on its tether host sat unbonded for 7+ minutes (bench 2026-09-19 09:09): every BOND attempt
ENOTCONN(peer known, no keypair),wg_tx_no_valid_keysclimbing, disco pings flowing both ways on the direct path, no WireGuard initiation toward the host's direct endpoint, initiations relayed only. One host-initiated handshake (tailscale ping --tsmp) healed it instantly — the host's wireguard-go only initiates when it has data, and it has none while the remote is silent. Reproduced deterministically (block direct + DERP, reboot the remote mid-outage so it is keyless and the host trims it, unblock direct only, DERP dead, no TSMP): v1.3-14 never recovers (>300 s).Root cause (
ml_wg_mgr.cpong-promotion path)The demote (
wireguardif_connect_derp) sets the live endpointpeer->ipto 0.0.0.0 so initiations go via DERP. On direct regain the handler stores the address withwireguardif_update_endpoint()— which only setsconnect_ip— and callswireguardif_connect()(which re-pointspeer->ipand fires a handshake) only if some keypair is still valid. Without one it fell to a deliberate one-shot ("wait for them to initiate",active=false) or, for an already-known peer, to nothing. So the remote kept initiating relay-only while disco reported direct. Whether a stale keypair survived decided between seconds and never: measured 10 s, 21 s, 169 s, ∞.Fix (three parts, safety peers only)
wireguardif_connect()with retries (the decisive change), paced to one connect perREKEY_TIMEOUT(5 s) per peer so pongs arriving several times a second cannot keep resetting a pending handshake. Unlimited duration is correct for the machine peer; bulk peers keep the one-shot.Diagnostics:
/admin/api/peersgainswg_up/wg_active/wg_send_hs/wg_hs_pending/wg_init_age_ms/eps/best/hs_cand/ping_age_ms/backoff_ms; monitor gainswg_hs_cand_sends,disco_hs_cand_pings,wg_safety_reconnects.Scope of the evidence
The deterministic recipe validates the package (all three changes were present in the build); change 1 is the clearest code defect and the one the recipe's stuck state exercises by construction (keyless peer, direct regained), but no change-1-only build was run. Changes 2 and 3 target the relay-up-but-slow variant and are not individually verified. My mechanism for that variant — initiations sitting in the DERP queue longer than
REKEY_TIMEOUTso each response matches a superseded handshake index — is a hypothesis consistent with the measured 1.3 s-median / 13 s-max mirror lateness, not a verified fact (peer review). Both extra legs are bounded (candidate freshness 60 s, same initiation bytes, safety peers only).Verification
Deterministic recipe above: baseline stuck >300 s; this branch bonded 22 s after the direct path returned (
wg_safety_reconnects=3), no TSMP. Two Ethernet units + the USB unit are in a 4 h armed soak on this build as I write.Also confirmed along the way: the machine side never causes a long outage (a timed-out entry is
PSTOP_REMOTE_UNKNOWN, skipped bypstop_remote_get, so a re-BOND is answered at once), andpriority_peer_ipis not the differentiator (recovers either way from a plain direct outage).