vprox: report binary server health in static IP heartbeats - #17
Merged
Conversation
bruce-y
marked this pull request as ready for review
August 18, 2026 00:39
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
bruce-y
changed the title
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d04c59e. Configure here.
bruce-y
changed the title
pbardea
approved these changes
Aug 18, 2026
taha-au
approved these changes
Aug 18, 2026
shreyas-blacksmith
added a commit
that referenced
this pull request
Sep 1, 2026
… rekey (#18) * lib: add /connect-ipip endpoint for IPIP tunnel peers Squash of codesmith/connect-ipip-endpoint (15 commits, tip 35d57ab), rebased onto main across the iptables-rules refactor (#15/#17). Serves the macOS static-IP path: Mac hosts POST /connect-ipip, vprox allocates an inner IP from WgCidr, creates a kernel ipip tunnel keyed by the client's source address, and installs a per-peer FORWARD accept/drop filter plus wildcard MSS clamping. The agent-side half (pf route-to + gif, fa#3824/#3841) has been deployed since May; this server half was never merged, so every /connect-ipip returned 405 fleet-wide. Co-authored-by: Cursor <cursoragent@cursor.com> * lib: mark readiness fatal if the stale IPIP sweep fails If LinkList fails, HTTPS never listens but heartbeats previously stayed on starting/stale. Match WireGuard and iptables setup failures so the box reports unhealthy. Co-authored-by: Cursor <cursoragent@cursor.com> * lib: guard IPIP map and kernel tunnels with one mutex Drop ipipCreateMu. ipipMu now covers peer state and LinkAdd/LinkDel so idle teardown cannot delete the map entry while the (local, remote) pair still exists. HTTP writes and idle stats probes stay off the lock. Co-authored-by: Cursor <cursoragent@cursor.com> * lib: do not hold ipipMu across the startup sweep Sweep runs before HTTPS listen and never touches the peer map, so a lock around LinkList plus every leftover LinkDel only delays bringing the box up. Log create/teardown that block on netlink/xtables for 5s so a pile of static-IP setup failures is diagnosable. Co-authored-by: Cursor <cursoragent@cursor.com> * lib: drop the IPIP kernel-op hang timer Observability is VM static-IP setup duration/failures. A 5s side goroutine did not fail faster and was extra machinery on create/teardown. Co-authored-by: Cursor <cursoragent@cursor.com> * lib: adopt leftover IPIP tunnels across restarts Replace the startup sweep with RestoreIpipFromKernel: valid vp* ifaces are adopted into Go state (Mac clients cache gif with no keepalive, so deleting them blacks peers out), and only invalid leftovers are deleted. Teardown becomes honest to make that safe: tearDownIpipLink returns an error, inner IPs are Freed only after LinkNotFound confirms the iface is gone, and the idle reaper is now a vanished-only reaper so a living tunnel is never deleted for quietness. Co-authored-by: Cursor <cursoragent@cursor.com> * lib: add opt-in ESP to the IPIP outer path A client may send {"esp": true} to /connect-ipip; the server mints one AES-GCM (rfc4106) SA per direction with crypto/rand, installs transport-mode xfrm state plus require-ESP policies scoped to proto-4 between the two hosts (so plaintext IPIP from a spoofer is dropped), sets the tunnel MTU to fit the ESP overhead, and returns SPIs and keys in the TLS-protected response. Old clients send {} and get the exact pre-ESP behavior, including removal of any leftover require-ESP policy on rollback. Re-connecting re-mints and replaces the pair's SAs; there is no rekey machinery. Restart adoption leaves kernel xfrm untouched so ESP keeps flowing while the process is down. Co-authored-by: Cursor <cursoragent@cursor.com> * lib: use aes-cbc+hmac-sha256 for IPIP ESP Staging E2E on the real boxes showed macOS setkey's PF_KEY grammar has no AEAD tokens at all ("syntax error at [aes-gcm]"), so rfc4106 AES-GCM cannot be installed on the Mac side. Switch to AES-128-CBC with HMAC-SHA-256 truncated to 128 bits: macOS xnu implements RFC 4868 truncation, not the legacy 96-bit KAME truncation that Linux defaults to for hmac(sha256), so the TruncateLen must be explicit (the mismatch shows up as XfrmInStateProtoError with zero decrypted packets). CBC+HMAC worst-case overhead is larger than GCM's, so the ESP tunnel MTU drops to 1424. Co-authored-by: Cursor <cursoragent@cursor.com> * lib: add rekey mode to /connect-ipip for ESP SA rotation A client can now POST {"esp": true, "rekey": true} to rotate SA generations without tearing down the tunnel. vprox mints a new SPI/key pair, installs the new inbound SA alongside the old ones, and returns the material; its outbound switches to the new generation once the new inbound SA's packet counter first ticks (500ms poll, 60s timeout, on timeout switch anyway and log). Two minutes after a counter-confirmed switch, older generations are GC'd, keeping the newest previous inbound so a client that rolled back after a failed health check keeps working; timeout-confirmed switches skip GC entirely and leave the sweep to the next confirmed rekey, which also collects generations orphaned by a vprox restart. Fresh connects keep the existing destructive replace. Co-authored-by: Cursor <cursoragent@cursor.com> * lib: keep adopted peers' ESP when deleting duplicate leftovers RestoreIpipFromKernel removed ESP state for every deleted leftover, but ESP objects are keyed by the address pair: a duplicate-remote leftover shares its pair's live SAs/policies with the adopted tunnel, so deleting the duplicate silently stripped the adopted tunnel's encryption. Skip pair-level ESP removal for remotes owned by adopted peers. Co-authored-by: Cursor <cursoragent@cursor.com> * lib: delete the IPIP link before removing its peer filters tearDownIpipLink removed the per-peer FORWARD filters before LinkDel, so a failed delete left a live tunnel forwarding without spoof protection. Delete the link first (LinkNotFound counts as gone) and remove filters only once the link is confirmed absent; the error contract is unchanged (an error still means the link may remain, so the caller must not Free). Co-authored-by: Cursor <cursoragent@cursor.com> * lib: unwind ESP install when it fails after the xfrm objects land installIpipEsp could return an error from the iface lookup / MTU step after the SAs and require-ESP policies were already installed. The handler then returns 500, the client never receives the minted keys, and the pair is left require-ESP'd in the kernel with keys nobody holds -- a blackhole until a successful retry. Unwind the freshly installed xfrm objects (and log the unwind) before returning. Co-authored-by: Cursor <cursoragent@cursor.com> * docs: add ESP rollback runbook and accepted threat model Co-authored-by: Cursor <cursoragent@cursor.com> * lib: make ESP rekey forward-only with prepare/activate/abandon steps Rollback-as-deletion-and-recreation is unsound: re-adding an outbound SA resets its sequence counter while the peer's inbound anti-replay high-water mark survives, so on a mature tunnel every rolled-back packet is dropped as a replay. Generations now only move forward, and each step is driven explicitly by the client: - PREPARE ({"esp":true,"rekey":true}) installs BOTH new-generation states; the outbound rides a per-generation reqid (derived from SpiToClient) that the outbound policy does not select yet, so the wire keeps flowing on the active generation. Late failures unwind the freshly added states (orphan-SPI fix). - ACTIVATE flips the outbound policy template to the new reqid. The old state is retained, so the flip is reversible and replay-safe (verified on staging: xfrm resumes the retained state's oseq). Idempotent by SpiToClient. - ABANDON walks a failed rotation back: a pending generation's states are deleted; an activated one has the policy flipped back to the pre-activation reqid, states left for the next rotation's sweep. Inbound states and the in-policy keep reqid 0 so every installed inbound generation stays acceptable at once (xfrm requires exact reqid matches between template and state, verified on staging including reqid 0 selecting only legacy states). GC is gated on dataplane evidence: after ACTIVATE a poll watches the new inbound SA's packet counter and only sweeps old generations (plus strays) once it ticks; on timeout nothing is deleted. The old timeout-activate ("switch anyway") is gone. A pending generation the client never claims is reaped after a timeout, or replaced by the next PREPARE, without touching active states. A rekey against a pair with no live SAs falls back to the destructive full install and reports Fresh=true so the client treats the tunnel as lost, not rotated. Also from the review: peer teardown now supersedes in-flight GC/reap goroutines via a process-global monotonic epoch (a stale goroutine can never act on a re-created peer for the same client IP), and createIpipLink deletes stale per-peer FORWARD rules for the interface name before appending fresh ones, so a leftover DROP can never sit ahead of the new ACCEPT after iface-name reuse. Co-authored-by: Cursor <cursoragent@cursor.com> * docs: update ESP runbook for forward-only rekey and deploy backups The rollback contract changed: a rolled-back (pre-ESP) server does NOT keep traffic flowing on its own -- live pairs hold require-ESP policies on both sides, so the kernel flushes are mandatory, not an optimization. Point the binary-downgrade step at the vprox.bak-<date> copies the deploy playbook now retains next to the checkout, and restate the forward-only invariant (no SA is ever deleted and re-added; retirement is gated on kernel counters). Co-authored-by: Cursor <cursoragent@cursor.com> * lib: harden forward-only rekey and make ESP mandatory on /connect-ipip Rekey hardening: an ACTIVATE is now provisional -- if no packet arrives on the new generation's inbound SA within the 2-minute activation gate, the server flips the outbound policy back to the previous generation's reqid on its own (idempotent with a late ABANDON via epoch and current-reqid guards). Rekey mutations are version-gated: every request with "rekey" must carry espRekeyV=2 or it is rejected with a 400 before any state is touched. Restore reconciliation verifies adopted IPIP links (up, /32 route, correct local endpoint) and demotes irreparable ones to deletion, freeing their allocated IPs. ESP mandatory: /connect-ipip rejects plaintext bodies ({}, empty, or esp:false) with a 400 "ESP required" before any peer is looked up or created; the {}-strips-ESP rollback path is deleted. /connect-ipip has never carried production traffic (prod vproxes 405 it), so there are no deployed plaintext clients to stay compatible with. The WG /connect path is unaffected. Docs: runbook updated -- ESP is mandatory, the emergency brake is stopping static-IP admission (not a plaintext downgrade), and the rollout gate is registering no us-central orgs until both vprox and agents are deployed from matching revisions. Co-authored-by: Cursor <cursoragent@cursor.com> * lib: lock the forward-only ESP contract: fenced v1 wire + one housekeeping sweep /connect-ipip is now version 1 with an explicit op (connect | prepare | activate); missing or unknown version/op -- including the empty body and every legacy shape -- is a 400 before any peer lookup, so no request can imply a destructive install. ACTIVATE is a compare-and-swap fenced on kernel truth: the outbound policy flips only if it currently selects expectedActive's generation, a superseded/replayed activate gets a 409 naming the real active generation and mutates nothing, and re-activating the active target is idempotent. The rollback machinery is deleted entirely: the ABANDON op, the activation auto-revert, the prev-generation/revert-reqid bookkeeping, the espRekeyV version gate (the versioned op is the gate), and every per-rekey goroutine plus the espRekeyEpoch counter that guarded them. Failure handling is one housekeeping sweep extending the 5s vanished-peer reaper: pending generations reaped after a deadline (confirmed absent before bookkeeping clears), counter-gated GC of superseded generations after switch proof plus grace, orphan aging by kernel AddTime, one xfrm dump per pass under a snapshot-unlock-revalidate discipline, unreadable counters treated as UNKNOWN, and log-based sweep health. Restore reconstructs the active generation from the outbound policy's reqid (kernel truth); no transition state is persisted across restarts. Co-authored-by: Cursor <cursoragent@cursor.com> * lib: treat xfrm AddTime as the absolute install timestamp it is Staging M9 showed the housekeeping sweep deleting a restart-orphaned pending generation on its first pass: the orphan-aging check multiplied AddTime (kernel curlft.add_time, seconds since the EPOCH) as if it were an age, so every orphan trivially exceeded the 5-minute protection window -- racing exactly the transition the deadline exists to protect. The policy heal's newest-generation pick had the same inversion. Age is now computed as Now - AddTime and newest means the largest timestamp; verified on staging: the orphan survives the +1min check after a restart and is swept at the deadline (journal: "esp sweep reaped/deleted" at +5-6min). Co-authored-by: Cursor <cursoragent@cursor.com> * docs: update TERMINAL runbook for terminal-owned guards and confirmed holder death The Mac agent's TERMINAL procedure now installs live holders' drop guards in a terminal-owned pf anchor that per-VM cleanup cannot flush, confirms each holder VM's death before removing its guard (unconfirmed death quarantines with guards intact; resume is idempotent), and adds the headroom-unknown deadline as a terminal trigger. All holder-bearing teardowns funnel through the same procedure. Co-authored-by: Cursor <cursoragent@cursor.com> * lib: bound SPI-to-int conversions for netlink fields Co-authored-by: Cursor <cursoragent@cursor.com> * lib: convert SPIs to int in provably in-range halves CodeQL cannot credit a math.MaxInt guard (legitimate SPIs exceed MaxInt32), so split the conversion into two 16-bit halves that fit int on any platform, and add a compile-time assertion that fails 32-bit builds outright. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.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.
Summary
Adds a cached, read-only readiness monitor per bind IP for the HTTPS listener, WireGuard configuration, bind interface and address, IPv4 routing and forwarding, iptables rules, and peer capacity. It centralizes the networking invariants used by setup and validation and applies failure and recovery hysteresis before changing routing eligibility. Heartbeats send only
ip,region, andhealthy; classified and detailed failures stay in local logs, and terminal setup failures reporthealthy: falsebefore exit. The companion backend contract is https://github.com/useblacksmith/web/pull/11701, and the change passes focused Go tests, race detection, vet, a Linux build, and GitHub checks.