Userspace SYN cookie core (#1374)#1393
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces the userspace-dp SYN-cookie codec core (mint/validate) needed for #1374, along with focused unit tests and a documentation note clarifying that the Go capability gate must remain closed until the full SYN-cookie datapath is wired.
Changes:
- Added a deterministic SYN-cookie ISN codec in
screen.rswith a[5-bit epoch][3-bit MSS index][24-bit MAC]layout and per-zone/per-epoch secret derivation. - Added unit tests covering tuple binding, MSS index encoding, epoch window validation, low-bit wrap rejection, and stale-secret rejection.
- Updated
FEATURES.mdto document the partial nature of the implementation and the remaining wiring requirements.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
userspace-dp/src/screen.rs |
Adds SYN-cookie codec types, a SipHash-2-4 implementation, and minor formatting cleanup in IPv6 fragment parsing. |
userspace-dp/src/screen_tests.rs |
Adds SYN-cookie core unit tests and minor assertion formatting. |
userspace-dp/src/FEATURES.md |
Documents presence of SYN-cookie core and notes that the Go gate must remain closed until full plumbing exists. |
| const SYN_COOKIE_MSS_SHIFT: u32 = SYN_COOKIE_MAC_BITS; | ||
|
|
| // ================================================================ | ||
| // SYN cookie core (#1374) | ||
| // ================================================================ | ||
|
|
||
| fn syn_cookie_codec() -> SynCookieCodec { | ||
| SynCookieCodec::new([ | ||
| 0x10, 0x21, 0x32, 0x43, 0x54, 0x65, 0x76, 0x87, 0x98, 0xa9, 0xba, 0xcb, 0xdc, 0xed, 0xfe, | ||
| 0x0f, | ||
| ]) | ||
| } |
| let secret = self.epoch_secret(zone_id, full_epoch); | ||
| let mut sip = SipHash24::new(secret[0], secret[1]); | ||
| sip.write_u64(0x7870_662d_7379_6e63); | ||
| sip.write_u16(zone_id); | ||
| sip.write_u64(full_epoch); | ||
| sip.write_u8(mss_index); | ||
| sip.write_ip(tuple.src_ip); | ||
| sip.write_ip(tuple.dst_ip); | ||
| sip.write_u16(tuple.src_port); | ||
| sip.write_u16(tuple.dst_port); | ||
| (sip.finish() as u32) & SYN_COOKIE_MAC_MASK | ||
| } | ||
|
|
||
| fn epoch_secret(&self, zone_id: u16, full_epoch: u64) -> [u64; 2] { | ||
| let k0 = u64::from_le_bytes(self.master_key[0..8].try_into().expect("fixed slice")); | ||
| let k1 = u64::from_le_bytes(self.master_key[8..16].try_into().expect("fixed slice")); | ||
|
|
||
| let mut left = SipHash24::new(k0, k1); | ||
| left.write_u64(0x7870_662d_7363_6b30); | ||
| left.write_u16(zone_id); | ||
| left.write_u64(full_epoch); | ||
|
|
||
| let mut right = SipHash24::new(k0, k1); | ||
| right.write_u64(0x7870_662d_7363_6b31); | ||
| right.write_u16(zone_id); | ||
| right.write_u64(full_epoch); |
Claude round-1 review on
|
Round-1 quad-review consolidated synthesis on
|
| Reviewer | Verdict |
|---|---|
| Claude | MERGE-READY (foundation) |
| Codex | MERGE-NEEDS-MINOR |
| Gemini Pro 3 | MERGE-READY |
| Copilot | (no review yet on this HEAD) |
Codex MINOR — HMAC vs SipHash naming + missing wiring
Codex's main finding is a naming-vs-implementation mismatch: the contract says "per-zone HMAC" but the code uses SipHash-2-4. My read: SipHash with secret keys is the standard cryptographic primitive for SYN cookies — Linux kernel does the same. The "HMAC" wording in #1374 was aspirational nomenclature; SipHash24 keyed by per-zone secret is the practical equivalent. Recommend updating the issue body to say "keyed SipHash" or "PRF (HMAC-equivalent)" rather than rewriting the code.
Codex also notes the explicit deferrals:
- No SYN-ACK frame builder yet (deferred per FEATURES.md:23)
- No returning-ACK integration (deferred)
- No cluster-consistent secret publication (deferred)
These are correctly deferred — the capability gate stays closed.
Codec correctness — Codex and Gemini both verified
Both reviewers independently verified:
- ✓ Full epoch in MAC, 5-bit transmitted
- ✓ 32-epoch wrap rejection (validation only tries current and previous full epochs)
- ✓ Per-zone secret derivation via SipHash with domain tags
- ✓ Zero hot-path allocations
- ✓ Tests cover epoch wrap, MAC integrity, stale secret, MSS tamper
Recommendation
Block on: nothing structural. Math core is correct and capability gate stays closed.
Strongly consider: update #1374 issue body to clarify "PRF/keyed SipHash" rather than "HMAC", which more accurately describes the implementation choice. SipHash-2-4 keyed by per-zone secret IS the right primitive for SYN cookies; HMAC-SHA256 would be overkill for a 24-bit MAC.
Defer to follow-up PRs: SYN-ACK frame construction, returning-ACK validation, cluster-consistent master-key publication, validated-client cache, status counters, capability gate removal.
Codex task: task-mp955afe-04hhgd. Gemini Pro 3 task: task-mp955yhc-1lbbg8. Not merging — author's decision.
|
Round-1 naming nit resolved in the source issue: #1374 now describes the implementation as RFC 4987-style keyed-MAC/SipHash handling rather than HMAC. No code change needed; the codec math remains unchanged. |
Copilot hygiene folded inPushed
Validation run:
Both passed. Cargo still emits the existing warning set. |
|
@copilot review\n\nPlease re-review current HEAD after the latest fix-forward/rebase push. |
Re-reviewed current HEAD ( |
451b7a2 to
a8077a7
Compare
|
Rebased on current New head: Light verification of the folded Copilot maintenance fixes:
Validation:
PR remains mergeable. @copilot review |
|
|
||
| const SYN_COOKIE_EPOCH_BITS: u32 = 5; | ||
| const SYN_COOKIE_MSS_BITS: u32 = 3; | ||
| const SYN_COOKIE_MAC_BITS: u32 = 24; | ||
| const SYN_COOKIE_ISN_BITS: u32 = 32; | ||
| const SYN_COOKIE_LAYOUT_BITS: u32 = | ||
| SYN_COOKIE_EPOCH_BITS + SYN_COOKIE_MSS_BITS + SYN_COOKIE_MAC_BITS; | ||
| const SYN_COOKIE_EPOCH_MASK: u32 = (1 << SYN_COOKIE_EPOCH_BITS) - 1; | ||
| const SYN_COOKIE_MSS_MASK: u32 = (1 << SYN_COOKIE_MSS_BITS) - 1; | ||
| const SYN_COOKIE_MAC_MASK: u32 = (1 << SYN_COOKIE_MAC_BITS) - 1; | ||
| const SYN_COOKIE_EPOCH_SHIFT: u32 = SYN_COOKIE_MSS_BITS + SYN_COOKIE_MAC_BITS; | ||
| const SYN_COOKIE_MSS_SHIFT: u32 = SYN_COOKIE_MAC_BITS; | ||
| const SYN_COOKIE_MAC_DOMAIN: u64 = u64::from_be_bytes(*b"xpf-sync"); | ||
| const SYN_COOKIE_SECRET_LEFT_DOMAIN: u64 = u64::from_be_bytes(*b"xpf-sck0"); | ||
| const SYN_COOKIE_SECRET_RIGHT_DOMAIN: u64 = u64::from_be_bytes(*b"xpf-sck1"); | ||
| const _: [(); SYN_COOKIE_ISN_BITS as usize] = [(); SYN_COOKIE_LAYOUT_BITS as usize]; | ||
|
|
||
| /// Three-bit MSS table encoded in userspace SYN cookies. | ||
| /// | ||
| /// The index, not the raw MSS, is transmitted in the ISN. Values are sorted so | ||
| /// selection can choose the largest value not exceeding the peer-advertised MSS. |
| #[derive(Debug, Clone, Copy)] | ||
| struct SipHash24 { | ||
| v0: u64, | ||
| v1: u64, | ||
| v2: u64, | ||
| v3: u64, | ||
| tail: [u8; 8], | ||
| tail_len: usize, | ||
| len: u64, |
| fn syn_cookie_tuple() -> SynCookieTuple { | ||
| SynCookieTuple { | ||
| src_ip: IpAddr::V4(Ipv4Addr::new(192, 0, 2, 10)), | ||
| dst_ip: IpAddr::V4(Ipv4Addr::new(198, 51, 100, 20)), | ||
| src_port: 49152, | ||
| dst_port: 443, | ||
| } |
Refs #1374.
Summary
[5-bit epoch][3-bit MSS index][24-bit MAC]ISN layout.syn-cookiecapability gate must remain closed until SYN-ACK TX replies, returning-ACK handling, HA-secret publication, bounded validated-client state, and counters are wired.Validation
cargo test syn_cookie --manifest-path userspace-dp/Cargo.tomlcargo test screen --manifest-path userspace-dp/Cargo.tomlgit diff --check