fix(rekey): complete relay-path session rekey (#91)#92
Merged
Conversation
… handling Two-epoch (current/previous, +next on responder) overlap, no wire change (conn_tag rotates per epoch = linkability fix + epoch discriminator); WireGuard confirmed-switch (initiator on completion, responder on first new-epoch inbound); rekey-mechanics-only scope + one-rekey-in-flight DoS guard; loser-fallback for winner-silent. PQ-hybrid=9b, #34 separate. Issue #9.
EpochSet state machine (owned EpochInbound sidesteps the multi-epoch borrow- return; steady state = one try) / PeerState::Established carries EpochSet / tick rekey scheduling (winner-initiates, one-in-flight, loser-fallback) / completion (initiator promote on Resp, responder install_next on Init) / netns loss-free-rotation + conn_tag-rotation money test. No wire change; yip-crypto + handshake.rs unchanged.
…ard confirmed-switch)
… in/out through current epoch
…t+fate for relay peers) EpochInbound::Send/TunThenSend previously carried Vec<Vec<u8>> (bytes only), discarding each EgressDatagram's dst and fate. dispatch_established and the roamed-peer fallback reconstructed dst from self.peers[idx].endpoint, which is wrong for relay-established peers (DataPlane::peer_addr is a server_addr() placeholder there; endpoint holds an unconfirmed candidate or None) -- silently dropping or misdirecting Send outcomes for relay peers. Carry the full EgressDatagram instead, so dst/fate are always correct and GSO fate-coalescing works again for free.
… one-in-flight, loser-fallback)
… install_next on Init
…rect-only; avoid never-completing churn)
…tag rotation
run-netns-rekey.sh forks run-netns-tunnel.sh: two yipd peers held to
YIP_REKEY_INTERVAL_MS=2000 (~10 rotations over a ~20s ping -i 0.2 -c 100
stream), run under both the poll and YIP_USE_URING=1 drivers, asserting
(1) rekey_continuity: <=1% ping loss across every rotation, and
(2) conn_tag rotation: >=3 distinct completed Noise-IK rekey rounds
observed in a tcpdump capture of the veth (obf off, so the PacketType
prefixes are visible).
Assertion 2 went through two failed designs before landing on a sound one,
documented in both the script and bin/yipd/examples/rekey_epoch_witness.rs:
- Diffing the raw masked dg[1..9] header bytes is vacuous: yip-wire's
Codec re-seeds the header mask from each frame's own auth tag, so the
visible bytes differ on every Data datagram regardless of epoch (verified
empirically) -- reported as RAW_DISTINCT_HEADER_PREFIXES, non-gating.
- Replaying captured [HandshakeInit] through a fresh responder-role
Handshake to re-derive the real (auth_key, hp_key, conn_tag) is
cryptographically unsound, not just impractical: Noise_IK's responder
draws its own fresh ephemeral while writing message 2, so a local replay
computes an unrelated session every time. Failed 100% of the time in
testing, exactly as predicted once the mechanism is understood -- this is
Noise's forward secrecy working as intended.
- rekey_epoch_witness instead counts DISTINCT CLEARTEXT Noise-IK ephemeral
public keys (the first message token of both HandshakeInit and
HandshakeResp is sent unencrypted, by spec) across the capture. N
distinct ephemerals is direct on-wire evidence of N independently
completed handshake rounds, and since conn_tag is a deterministic
function of the channel binding that mixes in both ephemerals' DH
product, N distinct rounds implies N distinct conn_tags with
cryptographic-strength probability -- without ever needing to (or being
able to) recover what those conn_tag values actually are.
Confirmed both poll and uring runs are real: strace on a throwaway netns
process shows genuine io_uring_enter syscalls under YIP_USE_URING=1.
Confirmed no regression on run-netns-tunnel{,-loss}.sh / run-netns-triangle.sh
at the production (unset) rekey interval.
CI: wires both driver runs into the existing netns-tunnel-test job
alongside the sibling netns steps.
Deferred: a rekey_under_loss (netem) variant, per the task brief's
"include it if it's a clean addition, else note it deferred" — the
continuity + rotation proof above already required significant redesign
work; adding combined netem+rekey is left for a follow-up.
Pre-commit `cargo test` blocked only by the known yip-io::uring flake
(uring_in_flight_send_table_reuses_slots_after_completions /
uring_loopback_roundtrip_recycles_recv_buffers, both timing-sensitive and
unrelated to this change — this diff touches no files under crates/yip-io).
fmt and clippy both passed. --no-verify used per that carve-out.
… of panicking (review)
…ion under Init retransmit/Resp reorder) Critical (9a final review): handle_rekey_init minted a FRESH session (fresh start_responder + install_next) on every rekey Init it saw, including retransmits, discarding the previous `next`. The initiator locks onto the FIRST HandshakeResp it reads, so on high-RTT or reordering links (RTT > retry_ms, or a reordered/duplicated Resp) the two sides diverged onto different epochs and the tunnel black-holed. Fix: EpochSet::next now carries the initiator's Noise ephemeral (init_eph, the unencrypted `e` in msg1) alongside the cached Resp bytes (NextEpoch). A retransmitted rekey Init (same ephemeral) is answered idempotently — resend the cached Resp, never rebuild `next`. A genuinely new round (new ephemeral) still builds a fresh `next`. Important-2: HANDSHAKE_TOTAL_MS (90s) exceeds REKEY_INTERVAL_MS/2 (60s), so a very-late retransmit of the ORIGINAL cold-start Init could land past accept_rekey_init's age gate. Peer::cached_resp_init_eph (set alongside cached_resp) lets handle_rekey_init recognize that case first and resend the cold-start reply instead of misclassifying it as a rekey round. Important-1: handle_rekey_resp still abandons an in-flight rekey on any Resp that fails read_response (rekey-liveness DoS only, current is never touched). snow::HandshakeState is not Clone (owns Box<dyn Dh>/Box<dyn Random>), so the clean clone-and-try fix isn't available; documented as a known limitation instead, riding with #34's authenticated-endpoint work. Minor: dropped the dead `if relay {...}` arms in drive_rekey_schedule that were unreachable after its early `if relay { return; }`. New tests: idempotent-retransmit unit test, an end-to-end Init-retransmit-before-Resp convergence test, a cold-start-dedup regression, and an init_ephemeral helper test.
…it framing (review)
Extract rekey_init_core/rekey_resp_core parameterized by RekeyEgress{Direct,Relay}
(single-source the 9a idempotent-ephemeral Critical fix); wire relayed_handshake_
init/resp to them; remove drive_rekey_schedule relay gate + restore relay-wrap Init
emit. Fail-closed; idempotency covers relay reordering. netns relay-forced money
test. Stacks on 9a (PR #90). Issue #91.
Extract rekey_init_core/rekey_resp_core (via_relay bool) behavior-preserving / wire relayed_handshake_init/resp to them / remove drive_rekey_schedule gate + relay-wrap Init emit / netns relay-forced money test. Single-sourced idempotent convergence; fail-closed relay_wrap-None no-op.
…y) — Direct path unchanged
…eaves egress untouched
…er the relay Task 4: prove relay-only sessions rotate loss-free over the rendezvous relay. run-netns-rekey-relay.sh forks run-netns-relay.sh's RELAY-FORCED topology (A/B mutually unreachable except via R's blind relay) with run-netns-rekey.sh's YIP_REKEY_INTERVAL_MS=2000 cadence, asserting (1) relay-forwarded=<N> N>0, (2) <=1% loss across ~10 rotations, (3) rekey_epoch_witness reports >=3 distinct completed rekey rounds. rekey_epoch_witness.rs grows an opt-in YIP_WITNESS_UNWRAP_RELAY=1 mode that strips the RelaySend (offset 33) / RelayDeliver (offset 17) rendezvous envelope before applying the existing distinct-ephemeral logic, so the direct-path 9a run-netns-rekey.sh is byte-for-byte unaffected (verified by re-running it). Wired into netns-tunnel-test in .github/workflows/integration.yml, both drivers, alongside the sibling 9a rekey steps.
…stent (final review)
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
1 participant
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.
Closes #91. Stacked on #90 (base =
feat/rekey-9a-session-rotation) — review #90 first.What this does
9a rotates each peer's Noise-IK session ~120s for forward secrecy, but relay-reached peers were gated out of rekey scheduling (
drive_rekey_schedulereturned early onrelay): rekey completion was only wired into the direct handshake handlers, so scheduling a relay rekey would have churned a never-completing ~1 Hz handshake (a DPI fingerprint). #91 wires relay-path completion, removes the gate, and proves it on the wire — so relay-only sessions now get forward-secrecy rotation too.The security-critical idempotent logic is single-sourced, not duplicated: relay paths reorder more than direct, which makes 9a's ephemeral-keyed convergence fix more load-bearing there, and a copy would be free to drift from it.
Changes
c388cb4,b459dde) —rekey_init_core/rekey_resp_core(.., via_relay: bool)+push_rekey_egress;handle_rekey_init/handle_rekey_respbecome thinvia_relay=falsewrappers. The direct path is byte-identical.push_rekey_egresstakes the fullEgressDatagramso the prime-emit preserves each packet's FECfate(review fix — a collapsedfatewould defeat GSO coalescing).e60b7ba) —relayed_handshake_init's Established arm andrelayed_handshake_resproute to the cores withvia_relay=true. The cold-startcached_resp_init_ephdedup is preserved, so a cold-start Init retransmit still resends the cached Resp instead of being misread as a rekey.2fbe21e) — relay peers now schedule rekeys, emitting the Init viarelay_wrap. Arelay_wrapNoneskips that send only; the round stays in flight and retries.b163430) —run-netns-rekey-relay.sh: three namespaces with no direct A↔B path, so traffic must traverse the blind relay.rekey_epoch_witnessgained an opt-inYIP_WITNESS_UNWRAP_RELAY=1mode that strips theRelaySend/RelayDeliverenvelope before counting distinct on-wire ephemerals (env-gated, so the 9a direct test is unaffected). Wired into CI, both drivers.004c138) — see below.Defect caught by the final review
Removing 9a's gate silently dropped an invariant it had been providing: relay and direct rekey completion were implicitly mutually exclusive. Without that, a direct peer completing a rekey via a relayed Init/Resp built the new epoch with
peer_addr = server_addr()whilepeers[idx].relaystayedfalse— andon_tundecides relay-wrapping frompeers[idx].relay, not the stamped address. Result: bare datagrams to the relay placeholder, dropped by the server, outbound black-holed for a full rekey interval. Reachable via a source-spoofed server address or a malicious relay, and with no attacker at all under asymmetric reachability (peer relays to us while we reach it directly).Fixed by gating completion on
peers[idx].relayat all four sites (fail-closedDispatchOut::Noneon mismatch). Therelayflag only flips on aHandshaking → Establishedtransition, never mid-session, so this cannot block a legitimate same-path rekey. Regression testdirect_peer_ignores_relayed_rekey_respwas confirmed failing before the fix.Verification
clippy -D warnings,cargo fmt— clean. Full workspace: 0 failures.relay-forwarded= 3147/3151 (the blind relay carried it),COMPLETED_ROUNDS= 10/10 distinct rekey rounds observed on the relayed wire.Known deferred
[HandshakeResp]can abandon an in-flight rekey — rekey-liveness only (currentuntouched, session survives, rotation delayed). Rides with Handshake anti-replay (timestamp) + authenticated endpoint learning #34 (authenticated endpoint).No wire-format change;
yip-crypto/yip-wire/handshake.rsuntouched apart from reusing the read-onlyhandshake::init_ephemeralhelper.