diff --git a/FORK_PRS.md b/FORK_PRS.md index fe0d46c..b63e57d 100644 --- a/FORK_PRS.md +++ b/FORK_PRS.md @@ -35,7 +35,7 @@ become its own scoped PR. Issue numbers filled in once created. | 15 | ✅ [#34](https://github.com/fugo101/microlink/issues/34) (done) | `Csontikka/microlink` | `b7442d0f` | Handshake retries throttled to 1/tick round-robin instead of firing to every eligible peer in one tick (was blocking the caller ~280ms with 7 peers). Adapted and landed in the `wireguard_lwip` submodule (not this repo — the fix lives entirely in `wireguardif.c`): [`fugo101/wireguard-lwip#15`](https://github.com/fugo101/wireguard-lwip/pull/15), merged and released as v1.0.2. Submodule pointer bumped here; `idf_component.yml`'s `fugo101/wireguard_lwip: "^1.0.0"` pin already covered 1.0.2 (verified live on the ESP Component Registry), so no manifest edit needed — same reasoning as issue #43's TAI64N bump, `#46`. | 2 | | 16 | ✅ [#35](https://github.com/fugo101/microlink/issues/35) (done) | `Csontikka/microlink` | `7a24a9b3`, `4d3b3add` | DERP client backpressure/reconnect-storm fixes: don't tear down on TLS write backpressure, TCP_NODELAY + coalesced writes, fix a hot-spinning I/O loop, split into concurrent reader/writer tasks — check overlap against cplewes's DERP session-resumption fix (#6) before scoping. **3/4 pieces landed in `ml_derp.c`**: (1) `derp_tls_write_all`'s WANT_WRITE/WANT_READ/TIMEOUT retry budget 50→300 (500ms→3s) — the caller treated any write failure as fatal and forced a full reconnect, so transient TLS backpressure was triggering reconnect storms; genuine mbedtls errors still bail immediately, unchanged. (2) `TCP_NODELAY` set on the DERP socket post-upgrade (was unset — Nagle was adding up to 500ms of latency coalescing our own already-small frames) plus `derp_send_packet` rewritten to build header+dest_key+payload as one buffer/one TLS write instead of two, via the existing `ml_psram_malloc`. (3) `ml_derp_tx_task`'s end-of-loop `vTaskDelay(pdMS_TO_TICKS(1))` rounded to 0 ticks at this project's 100Hz tick rate, so it never actually paced an idle loop — replaced with a `did_work` flag (`taskYIELD()` when busy, real `vTaskDelay(10ms)` when idle) and raised the TX/RX per-loop batch caps 8→32 / 4→32 to match. Not literal cherry-picks — this fork's `ml_derp.c` has diverged (PSA crypto, different comment style, `ml_setsockopt`/`ml_psram_malloc` wrappers not present upstream) — hand-ported with equivalent logic. No overlap with the already-landed #14 (DERP TLS leak, `derp_free_tls_state`/teardown path) or #25 (TLS session resumption, `ml_derp_connect`'s handshake phase) fixes — different functions/line ranges, verified via `git log -- ml_derp.c`. **(4) reader/writer task split — investigated further, decided not to port** (distinct from "deferred, not yet done"): re-checked against PR #62's restructured `ml_derp_tx_task` (`did_work`/Phase 1 TX/Phase 2 RX/batch caps 32) and every `ml->derp.*` touch point fork-wide — `ssl`/`ssl_conf`/`saved_session`/`sockfd` are only ever touched inside `ml_derp.c`, no cross-task race exists today. Splitting would require real mbedTLS-layer synchronization (`mbedtls_ssl_read`/`_write` are not safe to call concurrently on the same `ssl` context from two threads, even one-reader/one-writer — shared internal buffers, record sequence numbers, session state) — either a dedicated mutex around every `mbedtls_ssl_*` call (reintroducing exactly the cost this fork's file header says it deliberately avoids: "eliminates the need for a TLS mutex since only one task touches the SSL context") or two independent BIO/session objects, plus a new queue to route the PING→PONG echo (`dispatch_derp_frame()`) through a writer task instead of calling it inline. The starvation this fix targets in `Csontikka/microlink`'s architecture is real there, but here it's bounded to the 3s write-retry budget under genuine backpressure only (rarer and non-fatal since piece 1 landed) — not the unbounded/hot-spinning case their fix addresses. Crediting `Csontikka/microlink` for a correct fix for *their* architecture; it doesn't transfer cleanly to this fork's diverged single-task DERP design. Confirmed no downstream impact: `/Users/nguyen.ndt/Projects/zen-clock` (pulls `fugo101/microlink ^3.0.0` from the registry) only uses `microlink_init/start/rebind`, no DERP-internal API — public surface unchanged either way. The "also touches `components/wireguard_lwip/`" note in the original scoping was a copy-paste artifact from #31/#34/#42's rows — `7a24a9b3`'s submodule changes are diagnostics-only (unrelated to these fixes) and `4d3b3add` doesn't touch the submodule at all; no submodule involvement here. | 2 | | 17 | ✅ [#36](https://github.com/fugo101/microlink/issues/36) (done) | `Csontikka/microlink` | `27806be3` | `microlink_stop()` never closed `derp.sockfd`/`coord_sock`, letting `derp_tx` outlive the teardown wait → UAF — reconcile against cplewes's teardown UAF fix (#2), may be the same root cause. **Investigated, not needed here**: this fork's `microlink_stop()` (from issue #21/#22's PR #47) never frees context until `ml_join_tasks()` proves every worker exited, and `ml_derp.c`'s blocking loops already call `ml_shutdown_pending()` on every SO_RCVTIMEO-bounded iteration (100-200ms) to bail out cooperatively — the source commit's "shutdown() the socket to force-wake a blocked task" fixes a UAF that doesn't exist here. Leaving open in case future review disagrees, but no port planned. | 2 | -| 18 | [#37](https://github.com/fugo101/microlink/issues/37) | `Csontikka/microlink` | `cbdf1603`, `aad403af`, `533f1f88`, `46e34917`, `017b3588`, `372ca277` | H2 frame reassembly across `noise_recv()` read boundaries — large MapResponses spanning reads deterministically corrupt the stream ("implausible message size"); genuine protocol-correctness bug independent of control-plane backend. **Triaged into 4 pieces — 1/4 landed**: (1) `cbdf1603` (`https://` login_server + Headscale Noise key fetch), `aad403af` ("deliver the netmap from the long-poll stream, Headscale ≥0.26"), and `372ca277` (Headscale v0.28 single-stream compat) are all pure Headscale-control-plane compatibility, out of scope per `CLAUDE.md` (Tailscale-only) — excluded, no port. (2) `533f1f88` bundles two fixes: its stream-liveness watchdog (a second clock fed only by genuine stream-5 DATA frames, so a front end that keeps ACKing our PINGs can't hide a dead server-side mapSession) is real, backend-independent, and **landed** — this fork's existing watchdog (`last_activity_ms`) resets on any inbound *or even outbound-send* activity, so it had exactly this blind spot. Its second half (reorder `ML_EVT_COORD_REGISTERED` to fire after the streamed netmap, not before) is **not applicable**: that race only exists on Csontikka's Headscale ≥0.26 empty-initial-fetch path; this fork's `do_fetch_peers()` always populates `ml->vpn_ip` synchronously before returning, so `ML_EVT_COORD_REGISTERED` already fires after the VPN IP is known — skipped. (3) `46e34917` (DERP relay liveness + peer sweep, 6 sub-fixes) reconciled against #14/#25/#35's landed DERP fixes: TLS-leak-on-failed-connect and DERP-region-fallback are **already covered** (this fork's #14 fix is a superset — also validates `mbedtls_ssl_config_defaults`/`set_hostname`, and its region fallback additionally skips `avoid`-flagged regions and re-derives fresh each connect instead of permanently committing to the first fallback found — no port needed, source version would actually regress both). The zero-conflict pieces **landed** (`ml_coord.c`/`ml_peer_nvs.c`/`ml_wg_mgr.c`, none overlapping #14/#25/PR #62's line ranges): authoritative-peer-list sweep (a full `Peers`/`peers` list is authoritative — entries absent from it are queued for removal, since ACL revocation was previously only reachable via an explicit `PeersRemoved` array), peer removal now also drops the NVS cache entry (`ml_peer_nvs_remove()`, new) so revoked peers stay gone across reboots, and a DISCO decrypt-fail log naming the claimed sender + arrival path. **Retry-forever DERP backoff and RX-liveness watchdog now landed** (session with real hardware access): new `derp_wanted`/`derp_next_retry_ms`/`derp_backoff_ms` state in `ml_derp_tx_task()`'s disconnected branch — the bounded 3-attempt/2s retries in the `CONNECT_REQ`/`RECONNECT` event handlers are unchanged, but once those are exhausted this now keeps retrying forever with exponential backoff (`ML_DERP_RETRY_MIN_MS` 5s → `ML_DERP_RETRY_MAX_MS` 60s, doubling on failure, reset on success) instead of the old bare `vTaskDelay(100ms); continue;` that nothing external ever re-armed. The RX-liveness watchdog reuses a field that already existed half-wired: `ml->derp.last_recv_ms` was already being written on every received frame and on connect (comment: "For keepalive watchdog") but nothing ever read it — added the read side, forcing `ML_EVT_DERP_RECONNECT` when `ML_DERP_STALE_MS` (90s) of RX silence elapses while still marked connected. Mirrors the already-landed control-plane watchdog pattern (`ctrl_stream_rx_ms`/`ML_CTRL_STREAM_STALE_MS`) exactly. The commit's bundled TLS-state-cleanup half (`fail_tls` goto + free on every `ml_derp_connect()` failure path) was **not ported** — checked `ml_derp.c` first and confirmed issue #14's landed fix already has this exact `fail_tls`/`derp_free_tls_state()` path on every failure branch, including the `mbedtls_ssl_setup()` check the source commit added; porting it again would be a no-op. **Hardware-verified** on the same LilyGo T-Display-S3/real tailnet as #33/#38: ~150s of steady-state boot log, 15/15 `DERP status` lines showed `connected=1`, zero `DERP retry-forever`/`DERP RX silent` false-positives, zero crashes — confirms the new code doesn't misfire during normal operation. Could not induce a real DERP-side outage from this sandbox (no router/relay access) to directly observe the recovery path firing; that's the one open gap, noted rather than silently assumed. (4) `017b3588`, the actual H2-reassembly fix this row was originally filed for, still deferred — investigated further this session and confirmed it targets `poll_map_update()` (`ml_coord.c`, the `COORD_LONG_POLL` incremental-update handler), a **different, still-unmodified** function from `do_fetch_peers()` (which issue #38 already rewrote and which already reassembles safely across `noise_recv()` boundaries — no overlap). `poll_map_update()` genuinely has the bug today (single `noise_recv()` call per invocation, discards partial frames, keeps only the last stream-5 DATA frame per read) but reproducing it needs a long-poll delta large enough to span a read boundary — no evidence this triggers on the current 5-peer test tailnet (only the unaffected initial-fetch path has been observed spanning multiple frames, per issue #38's boot log). Large diff (287±101 lines in source) rewriting live parsing logic — still deferred to a dedicated session, now specifically pending either a larger tailnet or a way to force a large incremental delta. | 2 | +| 18 | 🚫 [#37](https://github.com/fugo101/microlink/issues/37) (closed, deprioritized) | `Csontikka/microlink` | `cbdf1603`, `aad403af`, `533f1f88`, `46e34917`, `017b3588`, `372ca277` | H2 frame reassembly across `noise_recv()` read boundaries — large MapResponses spanning reads deterministically corrupt the stream ("implausible message size"); genuine protocol-correctness bug independent of control-plane backend. **Triaged into 4 pieces — 1/4 landed**: (1) `cbdf1603` (`https://` login_server + Headscale Noise key fetch), `aad403af` ("deliver the netmap from the long-poll stream, Headscale ≥0.26"), and `372ca277` (Headscale v0.28 single-stream compat) are all pure Headscale-control-plane compatibility, out of scope per `CLAUDE.md` (Tailscale-only) — excluded, no port. (2) `533f1f88` bundles two fixes: its stream-liveness watchdog (a second clock fed only by genuine stream-5 DATA frames, so a front end that keeps ACKing our PINGs can't hide a dead server-side mapSession) is real, backend-independent, and **landed** — this fork's existing watchdog (`last_activity_ms`) resets on any inbound *or even outbound-send* activity, so it had exactly this blind spot. Its second half (reorder `ML_EVT_COORD_REGISTERED` to fire after the streamed netmap, not before) is **not applicable**: that race only exists on Csontikka's Headscale ≥0.26 empty-initial-fetch path; this fork's `do_fetch_peers()` always populates `ml->vpn_ip` synchronously before returning, so `ML_EVT_COORD_REGISTERED` already fires after the VPN IP is known — skipped. (3) `46e34917` (DERP relay liveness + peer sweep, 6 sub-fixes) reconciled against #14/#25/#35's landed DERP fixes: TLS-leak-on-failed-connect and DERP-region-fallback are **already covered** (this fork's #14 fix is a superset — also validates `mbedtls_ssl_config_defaults`/`set_hostname`, and its region fallback additionally skips `avoid`-flagged regions and re-derives fresh each connect instead of permanently committing to the first fallback found — no port needed, source version would actually regress both). The zero-conflict pieces **landed** (`ml_coord.c`/`ml_peer_nvs.c`/`ml_wg_mgr.c`, none overlapping #14/#25/PR #62's line ranges): authoritative-peer-list sweep (a full `Peers`/`peers` list is authoritative — entries absent from it are queued for removal, since ACL revocation was previously only reachable via an explicit `PeersRemoved` array), peer removal now also drops the NVS cache entry (`ml_peer_nvs_remove()`, new) so revoked peers stay gone across reboots, and a DISCO decrypt-fail log naming the claimed sender + arrival path. **Retry-forever DERP backoff and RX-liveness watchdog now landed** (session with real hardware access): new `derp_wanted`/`derp_next_retry_ms`/`derp_backoff_ms` state in `ml_derp_tx_task()`'s disconnected branch — the bounded 3-attempt/2s retries in the `CONNECT_REQ`/`RECONNECT` event handlers are unchanged, but once those are exhausted this now keeps retrying forever with exponential backoff (`ML_DERP_RETRY_MIN_MS` 5s → `ML_DERP_RETRY_MAX_MS` 60s, doubling on failure, reset on success) instead of the old bare `vTaskDelay(100ms); continue;` that nothing external ever re-armed. The RX-liveness watchdog reuses a field that already existed half-wired: `ml->derp.last_recv_ms` was already being written on every received frame and on connect (comment: "For keepalive watchdog") but nothing ever read it — added the read side, forcing `ML_EVT_DERP_RECONNECT` when `ML_DERP_STALE_MS` (90s) of RX silence elapses while still marked connected. Mirrors the already-landed control-plane watchdog pattern (`ctrl_stream_rx_ms`/`ML_CTRL_STREAM_STALE_MS`) exactly. The commit's bundled TLS-state-cleanup half (`fail_tls` goto + free on every `ml_derp_connect()` failure path) was **not ported** — checked `ml_derp.c` first and confirmed issue #14's landed fix already has this exact `fail_tls`/`derp_free_tls_state()` path on every failure branch, including the `mbedtls_ssl_setup()` check the source commit added; porting it again would be a no-op. **Hardware-verified** on the same LilyGo T-Display-S3/real tailnet as #33/#38: ~150s of steady-state boot log, 15/15 `DERP status` lines showed `connected=1`, zero `DERP retry-forever`/`DERP RX silent` false-positives, zero crashes — confirms the new code doesn't misfire during normal operation. Could not induce a real DERP-side outage from this sandbox (no router/relay access) to directly observe the recovery path firing; that's the one open gap, noted rather than silently assumed. (4) `017b3588`, the actual H2-reassembly fix this row was originally filed for, still deferred — investigated further this session and confirmed it targets `poll_map_update()` (`ml_coord.c`, the `COORD_LONG_POLL` incremental-update handler), a **different, still-unmodified** function from `do_fetch_peers()` (which issue #38 already rewrote and which already reassembles safely across `noise_recv()` boundaries — no overlap). `poll_map_update()` genuinely has the bug today (single `noise_recv()` call per invocation, discards partial frames, keeps only the last stream-5 DATA frame per read) but reproducing it needs a long-poll delta large enough to span a read boundary — no evidence this triggers on the current 5-peer test tailnet (only the unaffected initial-fetch path has been observed spanning multiple frames, per issue #38's boot log). Large diff (287±101 lines in source) rewriting live parsing logic — closed without a fix rather than left open indefinitely with no path to verification; reopen if a tailnet large enough to trigger it shows up, or a way to force a large incremental delta is found. | 2 | | 19 | ✅ [#38](https://github.com/fugo101/microlink/issues/38) (done), ✅ [#39](https://github.com/fugo101/microlink/issues/39) (done) | [`djorr5/microlink`](https://github.com/djorr5/microlink) | `67b230b2` | Two independent changes, split into two issues: (a) dynamic H2 RX window sizing based on free heap for RAM-constrained boards, roughly halving MapResponse-parsing PSRAM footprint; (b) `ip4_route_src_hook` to force tailnet-range traffic onto the WG netif directly. **(a) landed, hardware-verified via `zen-clock`** (a real downstream consumer at `/Users/nguyen.ndt/Projects/zen-clock`, LilyGo T-Display-S3/ESP32-S3, tested with `override_path` pointing at this working tree): `do_fetch_peers()` now allocates a single PSRAM buffer sized to a runtime window (`ml->h2_rx_window_size`, chosen by new `choose_h2_rx_window_size()` in `ml_coord.c` — largest free block across SPIRAM/internal, minus a 32KB margin, clamped to [64KB, `ML_H2_BUFFER_SIZE_KB`]) instead of two fixed 512KB buffers; the extracted MapResponse JSON is compacted in place via `memmove` rather than copied into a second buffer, halving peak footprint (~1MB → ~512KB at defaults, less under heap pressure). `ml_h2_build_preface()`'s signature now takes the window size instead of hardcoding `ML_H2_BUFFER_SIZE`. `CONFIG_ML_JSON_BUFFER_SIZE_KB` removed (now unused — the merged buffer only has one size knob). **Adapted, not a literal port**: kept the existing per-iteration `frame_buf` scratch-then-copy pattern in the initial H2 receive loop instead of reading `noise_recv()` straight into `h2_recv + h2_total` as the source commit does — this fork's `noise_recv()` returns -1 without draining the ciphertext off the socket when a frame doesn't fit the caller's buffer, which would desync the stream if the destination window shrinks near the tail; the source fork's `noise_recv()` may handle that case differently, but porting the direct-read change here would introduce a stream-desync class of bug this fork didn't have before. **Flashed and confirmed working on real hardware** (LilyGo T-Display-S3, real tailnet `husky-firefighter.ts.net`) via `pio run -t upload` against zen-clock with `override_path` pointed at this working tree. Boot log against a real, non-trivial MapResponse (5 peers, spanning 13 Noise frames): `H2 rx window: 128KB (largest free block 6144KB, ceiling 128KB)` → `H2 preface sent (61 bytes, conn window=128KB)` → `H2 END_STREAM detected after 13 Noise frames (22KB, 1189ms)` → `Accumulated 22KB of H2 data from Noise frames` → `MapResponse JSON: 22592 bytes` → JSON parsed cleanly, `Peers (array, size=5)` — no truncation warnings, no crashes, no heap corruption. All 5 real tailnet peers then completed WireGuard handshakes (direct + DERP) normally, confirming the consolidated single-buffer + in-place `memmove` compaction didn't corrupt the accumulated H2/JSON data. The adaptive clamp-under-pressure branch itself (choosing *below* the compiled ceiling) was **not** exercised — this board's free PSRAM (6144KB largest block) is far above the 128KB ceiling it's configured with, so the window always resolved to the full ceiling; only the default/no-pressure path was observed running for real. **(b) decided not to port** (distinct from "not yet done") — re-investigated by reading `microlink_rebind()` directly (`microlink.c:669-730`): it never touches the WG netif's link state at all (only closes/reopens DISCO/STUN sockets and signals coord+DERP to reconnect); the WG netif is only ever `netif_remove()`'d once, in `ml_wg_mgr_task`'s shutdown path (`microlink_stop()`/`destroy()`, not rebind). So the "narrow edge case (netif up/link-up flapping during rebind)" the earlier assessment left open as a maybe doesn't actually exist in this fork — the netif stays continuously registered and up in `netif_list` for its entire lifetime outside of full teardown, meaning lwIP's own `ip4_route()` netmask scan (which already runs before ever consulting `LWIP_HOOK_IP4_ROUTE_SRC`, per the original investigation) is always correct. No scenario left where the hook would change behavior — closing with no port, no new `ml_lwip_hooks.c` file, no build wiring. | 2 | | 20 | ✅ [#40](https://github.com/fugo101/microlink/issues/40) (done) | [`AELovelace/LAIN-MicrolinkRouter`](https://github.com/AELovelace/LAIN-MicrolinkRouter) | `e1239460` | `microlink_set_exit_node()`. **Scoped down from the row's original framing after investigation, approved by the user**: this is not a Tailscale-style default-route exit node (all traffic incl. general internet through a peer) and not a LAN-wide NAPT exit node (other LAN devices' traffic via SoftAP) — neither exists anywhere in the source commit or this fork. What it actually is: a **tailnet-range fallback peer** — the peer table is bounded by `CONFIG_ML_MAX_PEERS`, so a designated peer acts as a fallback router for `100.64.0.0/10` destinations not present in the local table; the WG netif's mask stays narrow always (never widens to `0.0.0.0/0` — the source commit's own comment documents this as itself a fix for an earlier, more dangerous version that caused a real outage). General internet traffic is unaffected either way. `microlink_get_netif_impl()` is a hook for a future real NAPT exit-node feature, not included here. `microlink_set_exit_node()`/`microlink_get_netif_impl()`, `wg_program_peer_route()`/`wg_apply_exit_node()`, and the 5s peer-liveness safety net landed in `microlink.h`/`microlink_internal.h`/`microlink.c`/`ml_wg_mgr.c`. The hard prerequisite — a longest-prefix-match fix in `peer_lookup_by_allowed_ip()` (without it, a `0.0.0.0/0` fallback route can shadow a more specific peer's `/32` depending on array order) — landed and released in the submodule: [`fugo101/wireguard-lwip#17`](https://github.com/fugo101/wireguard-lwip/pull/17), merged and released as v1.0.3. Submodule pointer bumped here; `idf_component.yml`'s `fugo101/wireguard_lwip: "^1.0.0"` pin already covered 1.0.3 (verified live on the ESP Component Registry), so no manifest edit needed — same reasoning as issues #34/#42/#43. Explicitly **not** ported: the WiFi-STA outbound socket binding (solves a problem that only exists with a widened netmask, which this design never does — porting it would regress cellular failover), `nacl_box.c` changes (unrelated crypto refactor bundled in by accident, includes a bounds-check removal that reads as a regression), `ml_stun.c` changes (cosmetic no-op). | 2 | | 21 | ✅ [#41](https://github.com/fugo101/microlink/issues/41) (done) | [`caslavskola/microlink`](https://github.com/caslavskola/microlink) | `0265718e` | PSA crypto init migration for mbedTLS 4.x. **Investigated, not needed**: ESP-IDF 6.x's own `mbedtls` component already calls `psa_crypto_init()` automatically at boot (`ESP_SYSTEM_INIT_FN`, priority 104, `components/mbedtls/port/esp_psa_crypto_init.c`) — an explicit call in `microlink_init()` would be pure redundancy. The commit also rewrites `ml_noise.c`'s ChaCha20-Poly1305 from `mbedtls_chachapoly_*` to raw PSA `psa_aead_encrypt/decrypt` calls, but this fork already has a working, cleaner solution via `mbedtls/private/chachapoly.h` (documented in `ESP_IDF_6X_COMPAT.md`) — the source commit's version is messier (leftover `//to remove` debug logging, commented-out ChatGPT-added diagnostics) and fixes nothing we're missing. No port planned. | 2 |