fix(wg_mgr): retry the initial direct-path handshake instead of one-shot - #54
Merged
Merged
Conversation
Adapted from Csontikka/microlink@5bda1783 (issue #32), with the already-fixed active-clearing bug (PR #53, this same session) kept removed -- the source commit's diff still cleared peer->active right after wireguardif_connect(), which PR #53 (issues #26/#28) proved permanently blocks that peer's direct-path handshake since active is the master session-initiation gate, not a retry throttle. Porting this verbatim would have silently reintroduced a bug fixed two PRs ago. process_disco_pong() fired a one-shot direct-path handshake init gated by a single boolean latch (tried_initial_handshake): if that one init was lost or unanswered, the peer stayed permanently un-sessioned on the direct path even though further direct PONGs kept arriving. Replaced the latch with a retry timestamp (last_init_handshake_ms) so a dropped/unanswered init gets another try every 30s (INITIAL_HANDSHAKE_RETRY_MS) instead of giving up forever. Also documented in FORK_PRS.md that issue #36 (wake blocked sockets on stop) was investigated and isn't needed here: this fork's microlink_stop() (from issues #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 to bail out cooperatively -- the source commit's UAF doesn't exist in this fork's architecture. Closes #32 Co-authored-by: Adrian.Nguyen-Qualgo <nguyen.ndt@qualgo.net> Co-authored-by: Claude Sonnet 5 <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
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
Adapted from
Csontikka/microlink@5bda1783(issue #32), with the already-fixedactive-clearing bug (PR #53, this same session) kept removed — the source commit's diff still clearedpeer->activeright afterwireguardif_connect(), which PR #53 (issues #26/#28) proved permanently blocks that peer's direct-path handshake, sinceactiveis the master session-initiation gate, not a retry throttle. Porting this verbatim would have silently reintroduced a bug fixed two PRs ago.process_disco_pong()fired a one-shot direct-path handshake init gated by a single boolean latch (tried_initial_handshake): if that one init was lost or unanswered, the peer stayed permanently un-sessioned on the direct path even though further direct PONGs kept arriving. Replaced the latch with a retry timestamp (last_init_handshake_ms) so a dropped/unanswered init gets another try every 30s (INITIAL_HANDSHAKE_RETRY_MS) instead of giving up forever.FORK_PRS.mdrow 13 (issue #32) marked done. Also documented that issue #36 (wake blocked sockets on stop) was investigated as part of this batch and isn't needed here: this fork'smicrolink_stop()(from issues #21/#22's PR #47) never frees context untilml_join_tasks()proves every worker exited, andml_derp.c's blocking loops already callml_shutdown_pending()on every SO_RCVTIMEO-bounded iteration to bail out cooperatively — the source commit's UAF doesn't exist in this fork's architecture, so no port planned (row left open for now).Closes #32
Test plan
idf.py buildnot runnable in this sandbox — needs a real ESP-IDF 6.x build to confirm.grepconfirmstried_initial_handshakeis fully gone, replaced bylast_init_handshake_ms/INITIAL_HANDSHAKE_RETRY_MS.dev->peers[...].active = falsedid not reappear anywhere inml_wg_mgr.c(the otheractive = falsehits found by grep are unrelated — peer-table eviction and DISCOpending_probes, different structs).FORK_PRS.mddiff touches only rows 13 (Retry direct-handshake init every 30s instead of one-shot latch (mined from Csontikka/microlink) #32, done) and 17 (Shutdown derp.sockfd/coord_sock during microlink_stop() (mined from Csontikka/microlink) #36, investigation note).🤖 Generated with Claude Code