Skip to content

fix(derp,coord): absorb bugfix/hardening subset of upstream PR #22 - #44

Merged
fudio101 merged 2 commits into
mainfrom
fix/derp-tls-leak
Aug 18, 2026
Merged

fix(derp,coord): absorb bugfix/hardening subset of upstream PR #22#44
fudio101 merged 2 commits into
mainfrom
fix/derp-tls-leak

Conversation

@fudio101

@fudio101 fudio101 commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Started as the narrow DERP TLS-context-leak fix (issue #14); grown, per discussion, into absorbing
the full bugfix/hardening subset of upstream CamM2325/microlink#22
("Support custom (non-Tailscale) control planes end-to-end") — everything in that PR that's useful
regardless of control plane, leaving out the headscale-specific feature work.

DERP (ml_derp.c):

  • mbedTLS context leak on every failed ml_derp_connect() (~3-8 KB/attempt) — derp_free_tls_state() + fail_tls goto path
  • Region fallback when HomeDERP isn't in the DERPMap (falls back to first usable non-avoid region instead of a dead-end connect)
  • TLS 1.2 pin — some relays' Let's Encrypt ECDSA cert trips the ESP-IDF mbedTLS TLS 1.3 signature-algorithm OID path even under VERIFY_NONE
  • Return-code checking on every mbedTLS setup call, routed through the same fail_tls cleanup
  • BIO rework to a plain blocking f_recv bounded by SO_RCVTIMEO, dropping mbedtls_ssl_conf_read_timeout (avoids a BAD_INPUT_DATA failure the old scheme hit on some relays' TLS 1.3 path)

Coord (ml_coord.c / microlink.h):

  • Hostinfo OS field corrected to "esp32" (was "linux")
  • New ML_STATE_AUTH_FAILED state — do_register() now actually fails registration on a control-plane Error/MachineAuthorized rejection instead of silently "succeeding" into a broken state (Node absent, confusing downstream "node not found"); same state raised on expired-key-with-no-auth_key
  • state_cb now fires on the RECONNECTING transition (it never did before) and isn't clobbered by it while AUTH_FAILED is active

Deliberately excluded (see UPSTREAM_PRS.md's updated "Deliberately skipped" table):

Attribution

Adapted (not literal cherry-picks — base has diverged too far from both sources) from:

Related issues

Closes #14
Refs #43 (FORK_PRS.md dj-oyu bundle, TAI64N item superseded — not closed, rest of that bundle is still open)
Refs CamM2325/microlink#22 (upstream, partial absorption — see UPSTREAM_PRS.md)

Test plan

  • idf.py fullclean && idf.py build for examples/basic_connect (ESP-IDF wasn't available in the sandbox this PR was written in — please build before merging)
  • Manual diff review: region-fallback loop only fires when node_selected is false; every new mbedTLS setup-call failure routes through fail_tls:; all 4 Hostinfo OS occurrences updated (grep -n '"OS", "linux"' returns nothing); ML_STATE_AUTH_FAILED is appended (not inserted) into the enum; COORD_RECONNECTING's ML_CMD_DISCONNECT path unaffected by the new guard
  • Hardware re-verification recommended, especially the DERP TLS 1.2 pin / BIO rework and the new AUTH_FAILED state transitions — none of this was hardware-tested in this session

🤖 Generated with Claude Code

nguyenndt-qualgo and others added 2 commits August 18, 2026 22:30
ml_derp_connect()'s failure paths after the TLS phase begins only
closed the raw socket — they never freed the mbedtls_ssl_context /
mbedtls_ssl_config initialized just above. Each failed handshake
leaked ~3-8 KB of internal heap, and microlink_rebind() reconnects
DERP on every WiFi reconnect, so this compounds fast under
captive-portal/bad-network conditions until no handshake can even
allocate buffers.

Extract derp_free_tls_state() (ssl/ssl_conf free + socket close) and
route every post-init failure through a single `fail_tls` label via
goto, reusing the same helper from ml_derp_disconnect()'s existing
graceful-teardown path so the two call sites can't drift apart.

Adapted from cplewes/microlink@38602ab0 and cplewes/microlink@b25b1eee.
Not a literal cherry-pick: our ml_derp_conn_t already dropped the
entropy/ctr_drbg fields as part of the mbedTLS 4.x / TF-PSA-Crypto
migration (RNG is PSA-owned now), so derp_free_tls_state() frees only
ssl/ssl_conf.

Closes #14

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Extends the DERP TLS-leak fix already on this branch with the rest of
what upstream shipped as one verified commit (125b529):

- DERP region fallback when HomeDERP isn't in the DERPMap (falls back
  to the first usable non-avoid region instead of a dead-end connect
  to the default host)
- Pin DERP TLS to 1.2 — some relays' Let's Encrypt ECDSA cert trips
  the ESP-IDF mbedTLS TLS 1.3 signature-algorithm OID path even under
  VERIFY_NONE
- Check every mbedTLS setup call's return value (config_defaults,
  ssl_setup, set_hostname) and abort via the existing fail_tls path
  instead of proceeding into handshake with a half-initialized config
- Rework the DERP BIO to a plain blocking f_recv bounded by
  SO_RCVTIMEO, dropping mbedtls_ssl_conf_read_timeout entirely —
  avoids a BAD_INPUT_DATA failure the recv_timeout scheme hit on some
  relays' TLS 1.3 path

Plus the other generic (non-headscale) fixes from the same upstream
PR:

- Hostinfo "OS" field corrected to "esp32" (was "linux")
- New ML_STATE_AUTH_FAILED state: do_register() now actually fails
  registration on a control-plane Error/MachineAuthorized rejection
  instead of silently "succeeding" into a broken state (Node absent,
  confusing downstream "node not found"); the expired-key-with-no-
  auth_key path raises the same state
- state_cb now fires on the RECONNECTING transition (it never did
  before) and doesn't get clobbered by it while AUTH_FAILED is active

Adapted from cplewes/microlink@38602ab0/@b25b1eee (already on this
branch) and CamM2325#22 commits 125b529, fcdc8d9,
ae3d438, 8367c1e, 9ef10bb — not a literal cherry-pick, base has
diverged too far, and our mbedTLS 4.x/PSA migration already dropped
the entropy/ctr_drbg fields upstream's version still frees.

Deliberately excluded (see UPSTREAM_PRS.md): the 3 headscale/custom-
control-plane feature commits from CamM2325#22 (this fork
targets Tailscale's own SaaS control plane), and the netif->input RX
fix (5c8d60c), already absorbed via CamM2325#20.

Refs #14, #43 (FORK_PRS.md dj-oyu bundle's TAI64N item superseded),
CamM2325#22

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@fudio101 fudio101 changed the title fix(derp): free the mbedTLS context on failed DERP connect fix(derp,coord): absorb bugfix/hardening subset of upstream PR #22 Aug 18, 2026
@fudio101
fudio101 merged commit 2aedd0a into main Aug 18, 2026
13 checks passed
fudio101 added a commit that referenced this pull request Aug 19, 2026
…de backoff (#52)

Adapted from cplewes/microlink@b9636816 (not a literal cherry-pick: git apply
conflicted only on microlink_internal.h's ml_derp_conn_t, which lacks the
entropy/ctr_drbg fields the source commit anchors near since this fork uses
PSA crypto post-mbedTLS-4.x migration -- same divergence already handled for
issue #14/PR #44. ml_derp.c and ml_wg_mgr.c applied cleanly; hand-ported with
identical logic/comments throughout).

- DERP TLS session resumption: save the negotiated mbedTLS session after each
  successful handshake and resume it on the next reconnect, skipping the full
  ECDHE handshake (~7.5s -> sub-second). Best-effort: falls back to a full
  handshake transparently if the ticket expired.
- DERP_CONNECT_TIMEOUT_MS 10s -> 25s: the TLS handshake was observed taking
  ~7.5s+ and occasionally exceeding 10s on a lossy/high-latency captive
  network, tripping spurious "TLS handshake failed: timed out" -> full-retry
  thrash.
- Per-peer exponential backoff on direct-path upgrade probes: on a
  P2P-hostile network (blocks peer-to-peer UDP) direct-path upgrades never
  succeed, so re-probing every 15s forever wastes CPU + DERP TX. Doubles the
  interval on each unanswered probe up to a 300s cap, resets to the base
  interval once a direct path is established.

Closes #25

Co-authored-by: Adrian.Nguyen-Qualgo <nguyen.ndt@qualgo.net>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(derp): free the mbedTLS context on failed DERP connect (leaks ~20 KB per attempt)

2 participants