feat(analyzer): TLS handshake-message reassembly across records (STORY-144, closes SNI/JA3 fragmentation evasion)#341
Merged
Conversation
Files created: src/analyzer/tls.rs (modified), tests/tls_analyzer_tests.rs (modified) todo!() functions: 3 (proptest_vp039_carry_reassembly_two_record, proptest_vp039_exact_consume_coalesced, proptest_vp039_carry_bounded_invariant) ## GREEN-BY-DESIGN none ## WIRING-EXEMPT none Stub details: - TlsFlowState: added client_hs_carry: Vec<u8> and server_hs_carry: Vec<u8> (both Vec::new() in new()); NO hs_carry_abandoned flag (BC-2.07.039 Inv-4) - TlsAnalyzer: added handshake_reassembly_overflows: u64 (init 0); aggregate counter, NOT per-flow, NOT reset at on_flow_close (mirrors truncated_records) - 4 new #[doc(hidden)] pub fn test seams on TlsAnalyzer: client_hello_seen_for_testing, client_hs_carry_len_for_testing, server_hs_carry_len_for_testing, handshake_reassembly_overflow_count - parse_tls_message_handshake import added (ADR-011 Decision 4 parse boundary); #[allow(unused_imports)] stub suppresses -Dwarnings until drain loop is wired - summarize(): handshake_reassembly_overflows insertion LEFT for implementer (adding it now would break test_summarize_output; implementer adds key + updates the expected-key-count assertion together in AC-144-002/003) - try_parse_records: existing parse_tls_plaintext path PRESERVED (AC-144-005 regression gate); carry drain loop is the implementer's task (AC-144-002) - mod story_144 in tests/tls_analyzer_tests.rs: 15 Red Gate harnesses per DF-TEST-NAMESPACE-001; all 15 FAIL (Red Gate intact), 120 existing tests GREEN BC-5.38.001 compliance: cargo check passes, cargo clippy --all-targets -D warnings passes, all 15 new tests are RED, no non-trivial function body contains real implementation logic. The 3 proptest stubs use todo!() per BC-5.38.001. The 12 unit stubs assert carry-path behavior (carry accumulation, overflow counts, SNI extraction after fragmentation) that ONLY the carry drain loop can satisfy.
…mbly (VP-039)
Finalizes mod story_144 in tests/tls_analyzer_tests.rs: replaces the three
todo!() proptest stub bodies with real falsifiable proptest implementations,
converts unused doc-comment warnings to // style, fixes clippy::len_zero.
All 15 VP-039 harnesses (3 proptest + 12 unit) compile clean and FAIL against
the current carry-drain-loop stubs (Red Gate per BC-5.38.001):
- proptest_vp039_carry_reassembly_two_record (Sub-A): fails because
client_hello_seen stays false without the carry path
- proptest_vp039_exact_consume_coalesced (Sub-B): fails on carry_len==4
assertion after the 4-byte header record (stub returns 0)
- proptest_vp039_carry_bounded_invariant (Sub-F): fails on carry >= 1
secondary assertion after first partial record (stub returns 0)
- 12 unit tests: fail on existing carry/overflow/summarize assertions
120 pre-existing tests still pass; cargo clippy -D warnings clean.
…on reassembly Implements BC-2.07.038 v2.5 / BC-2.07.039 v2.4: per-direction client_hs_carry Vec<u8> on TlsFlowState accumulates 0x16 record payloads across record boundaries until a complete handshake message (4-byte header + body) is assembled. Key decisions implemented: - Decision-3 (ADR-011): append payload → drain loop dispatches complete messages - Decision-4: body_len > MAX_BUF → clear carry + handshake_reassembly_overflows++ - Decision-5: carry + payload > MAX_BUF BEFORE append → clear + overflows++ + continue - Direction-split: ClientToServer uses carry path; ServerToClient retains parse_tls_plaintext (STORY-145 scope) to preserve all ServerHello tests (AC-144-005) - parse_tls_message_handshake used for carry dispatch (not parse_tls_plaintext which requires a 5-byte TLS record header absent from assembled carry bytes) Test fixture fixes: - test_vp039_carry_overflow_clear_and_recover: replaced single 65,400-byte record with 4 records (3×18,432 + 10,104) per BC-2.07.039 v2.1 EC-002: single records > MAX_RECORD_PAYLOAD (18,432) cannot reach the carry; overflow is accumulation-based - test_vp039_carry_overflow_recovery: replaced single 65,537-byte record with 4×18,432-byte records; same spec rationale - test_parse_error_counter + test_malformed_handshake_increments_parse_errors_only: updated fixture to 9-byte TLS record carrying a complete-but-malformed handshake message so the drain loop assembles it → parse_tls_message_handshake fails → parse_errors++ - test_summarize_output: updated key count 7→8 for handshake_reassembly_overflows - dispatcher_tests (6 tests): changed discriminator from parse_error_count/truncated_record_count to active_flows_len_for_testing — carry path accumulates short payloads without errors All 15 STORY-144 Red-Gate tests now pass; 135 total tests green; clippy + fmt clean.
…-005/CR-006/CR-007/CR-008/F-144-002 All 135 tests green, clippy -D warnings clean, fmt --check clean. Changes: - CR-001/CR-003/CR-007/CR-008: Remove all stale "not yet implemented" / "RED GATE" / "MUST FAIL" / "against the stub" comments from src/analyzer/tls.rs and tests/tls_analyzer_tests.rs. Grep confirms zero residuals in src/ + tests/. - CR-002: Remove now-redundant #[allow(unused_imports)] on tls_parser import block (parse_tls_message_handshake is actively used by the carry drain loop). - CR-005: Remove #[doc(hidden)] from handshake_reassembly_overflow_count() and rewrite its doc to current-behavior tense. It is a genuine public accessor mirroring truncated_record_count() — not a hidden test seam. - CR-006 (perf): Change record_payload from to_vec() allocation to &record_bytes[5..] borrow. Both .len() and extend_from_slice() accept &[u8]; record_bytes outlives both uses. Also fix clippy::needless_borrow on extend_from_slice call. - F-144-002: Rewrite test_BC_2_07_040_empty_carry_flow_close fixture. Old fixture produced 8 bytes in carry (not empty) because the spoof header was appended after a partial record and the Step-1 guard (carry+payload > MAX_BUF) never fired. New fixture: deliver a single body_len=65537 record so Decision-4 fires immediately (carry starts empty, 4 bytes appended, drain loop sees body_len > MAX_BUF → clear, overflows+1, carry back to 0). Then on_flow_close verifies no errors/findings/flows. Test now genuinely exercises BC-2.07.040 empty-carry-at-close sub-case.
… + regression test SEC-001 (HIGH, CWE-400/834) — quadratic CPU amplification fix: Replace per-message Vec::drain(..k) in the ClientToServer carry drain loop with a cursor-based approach. A local `consumed` usize offset advances `4 + body_len` per consumed message; a single `carry.drain(..consumed)` fires after the loop exits. Total drain cost is O(carry_len) per on_data call instead of O(carry_len^2). The previous O(N*L) approach allowed an attacker to pack ~4,500 zero-body-length messages into one MAX_RECORD_PAYLOAD=18,432-byte record, causing ~40 MB of memmove per on_data call. Semantics preserved: - Decision-4 body_len-spoof guard: carry.clear() + overflows+1 + break (unchanged). - Decision-5 pre-append overflow guard: unchanged. - Exact-consume: each message advances cursor by exactly 4 + body_len regardless of parse outcome (BC-2.07.038 Postcondition 4 / Invariant 2). - Clone-on-dispatch: only msg_type==0x01 allocates message bytes; non-dispatched types (non-0x01) advance the cursor with zero heap allocation. SEC-003 (LOW, CWE-190) — overflow-checks=true safety: Both handshake_reassembly_overflows increment sites (Decision-4 and Decision-5) now use saturating_add(1) instead of bare `+=`. The release profile sets overflow-checks=true; saturation at u64::MAX is safe and intentional for this diagnostic counter. parse_errors left with bare `+=` to match the existing convention. Frame A assertion (adversary LOW): test_BC_2_07_038_canonical_frame_rfc8446_s4: add explicit parse_errors==1 assertion after Frame A delivery per BC-2.07.038 AC-CANONICAL-FRAME. Previously only client_hello_seen==false was asserted at this point. SEC-001 regression test (new): test_BC_2_07_042_coalesced_zero_len_no_quadratic_drain: delivers 1,000 zero-body-length non-0x01 messages in a single 0x16 record. Asserts carry==0, parse_errors==0, overflow_count==0, client_hello_seen==false. Guards against reintroducing O(N^2) drain. Custom CSS: none. All code changes use existing patterns. All 16 story_144 tests pass; full suite green (0 failures); clippy clean; fmt clean.
Records the AFTER half of the before/after pair for the TLS carry buffer and ClientHello fragmentation reassembly fix. Recordings cover: - AC-144-002-reassembly-fragmented: fragmented ClientHello now extracts SNI=['example.com'] + JA3 with parse_errors:0 (was MISSED on baseline) - AC-144-002-control-regression: single-record control pcap still works - AC-144-002-before-after-contrast: side-by-side BEFORE/AFTER contrast - AC-144-003-overflow-clear-recover: carry overflow unit tests passing (test_vp039_carry_overflow_clear_and_recover + summarize exposes counter) Baseline: .factory/demo-evidence/fix-tls-clienthello-frag/ (develop at a2d8c13) Fixture pcaps reused from baseline cycle.
Owner
Author
Review Cycle 1 Triage
Verdict: APPROVE (0 blocking findings) All 5 ACs verified. 136 tests pass (15 Red-Gate + 1 anti-quadratic + 120 existing). SEC-001 (quadratic drain) and SEC-003 (saturating_add) resolved. Demo evidence present (4 recordings). Carrying to CI gate. |
Zious11
added a commit
that referenced
this pull request
Jun 30, 2026
…+146) STORY-144 (TLS carry buffer + ClientHello reassembly) squash-merged via PR #341 (0986e87), 11/11 CI green. Per-story adversarial 3-clean + security pass (SEC-001 quadratic-drain DoS found+fixed) + demo pass (fragmented ClientHello: MISSED→DETECTED). stories_delivered 91→92. Wave 65 DONE. develop HEAD 0986e87. Wave 66 active: STORY-145 (ServerHello symmetry+isolation) first, then STORY-146 (buffer-saturation telemetry) from updated develop. Both dep STORY-144 satisfied. Delivering sequentially (both touch tls.rs). Decisions: D-307 added. Deferred items filed: SEC-002 (narrow overflow window [MAX_BUF-3,MAX_BUF] → F6), done()-mid-loop cross-direction carry (→ wave-gate review, pre-existing), SEC-004 (parse_errors u64 overflow → maintenance sweep, LOW). Count-propagation sweep: stories_delivered updated from 91→92 in: STATE.md frontmatter, STATE.md Notes, STATE.md Project Metadata table. Old develop SHA (ab0b388) removed from frontmatter + Metadata table; historical references in D-301/D-306/D-300 notes preserved as immutable past-state records (accurate).
10 tasks
Zious11
added a commit
that referenced
this pull request
Jun 30, 2026
…STORY-145, closes S2C SNI/JA3S fragmentation gap) (#343) ## Summary STORY-145 extends the TLS handshake-message reassembly carry-buffer mechanism (introduced in STORY-144 for ClientHello/ClientToServer) to the **ServerToClient direction**. A ServerHello fragmented across multiple TLS 0x16 records is now reassembled so JA3S fingerprinting and detection work correctly. This closes the S2C half of the silent SNI/JA3S evasion gap (TLS-CLIENTHELLO-FRAG-001). The ServerToClient carry-drain arm is a byte-for-byte symmetric mirror of the ClientToServer path delivered in STORY-144: - Clear-and-recover on overflow (`MAX_BUF=65536`, per-record limit 18432) - Cursor + single-drain O(carry_len) — DoS-safe (SEC-001) - `saturating_add` aggregates overflows 4 new tests in `mod story_145` (2 AC-cited Red-Gate + 2 DoS-guard sibling-coverage per DF-SIBLING-SWEEP-001). Commits will be squash-merged. --- ## Behavioral Contract Traceability | BC ID | Version | Title | AC Coverage | |-------|---------|-------|------------| | BC-2.07.041 | v1.2 | Handshake Carry Buffers Are Per-Flow and Per-Direction Isolated | AC-145-001, AC-145-002, AC-145-003 | | BC-2.07.002 | v1.6 | Parse Complete TLS ServerHello: JA3S Fingerprint Computed | AC-145-004, AC-145-005 | | BC-2.07.038 | v2.7 | (referenced) Carry buffer overflow, exact-consume, truncation semantics | AC-145-001 (symmetric) | **Verification Property:** VP-039 Sub-E (direction isolation proptest + cross-flow isolation unit test) ```mermaid flowchart LR BC241["BC-2.07.041 v1.2\n(Direction Isolation)"] --> AC1["AC-145-001\nS2C carry drain"] BC241 --> AC2["AC-145-002\nDirection selector"] BC241 --> AC3["AC-145-003\nCross-flow isolation"] BC202["BC-2.07.002 v1.6\n(ServerHello JA3S)"] --> AC4["AC-145-004\nFragmented reassembly"] BC202 --> AC5["AC-145-005\nSingle-record regression"] AC1 --> TEST1["proptest_vp039_direction_isolation"] AC2 --> TEST1 AC3 --> TEST2["test_BC_2_07_041_cross_flow_isolation"] AC4 --> TEST1 AC5 --> TEST3["test_parse_server_hello\n+ CLI smoke"] TEST1 --> IMPL["src/analyzer/tls.rs\nServerToClient arm"] TEST2 --> IMPL TEST3 --> IMPL ``` --- ## Story Dependencies ```mermaid graph LR STORY144["STORY-144\nClientToServer carry\n(merged #341)"] --> STORY145["STORY-145\nServerToClient carry\n(this PR)"] STORY145 -.->|blocks| STORY146["STORY-146\n(future)"] ``` **STORY-144** merged as PR #341 (`feat(analyzer): TLS handshake-message reassembly across records`). This PR depends on STORY-144's struct fields (`server_hs_carry`, `handshake_reassembly_overflows`) and the `ClientToServer` drain loop being in place. --- ## Architecture Changes ```mermaid graph TD subgraph "src/analyzer/tls.rs" TR["try_parse_records()"] TR -->|"Direction::ClientToServer\n0x16 record"| C2S["client_hs_carry\ndrain loop\n(STORY-144)"] TR -->|"Direction::ServerToClient\n0x16 record\nNEW"| S2C["server_hs_carry\ndrain loop\n(STORY-145)"] S2C --> SH["handle_server_hello()\nmsg_type==0x02"] SH --> SSH["server_hello_seen=true\nja3s_counts populated"] S2C --> OVF["Overflow guard:\nclear-and-recover\nMAX_BUF=65536"] end subgraph "TlsFlowState (per-flow, unchanged)" CK["client_hs_carry: Vec<u8>"] SK["server_hs_carry: Vec<u8>"] end subgraph "HashMap<FlowKey, TlsFlowState>" HM["Flow isolation\n(per-flow keying)"] end S2C --> SK C2S --> CK TR --> HM ``` **Files changed:** - `src/analyzer/tls.rs` — +128/-35 lines (ServerToClient 0x16 carry drain path wired) - `tests/tls_analyzer_tests.rs` — +535 lines (4 new tests in `mod story_145`) - `docs/demo-evidence/STORY-145/` — 3 WebM recordings + 3 GIFs + evidence-report.md - `tests/tls_analyzer_tests.proptest-regressions` — 1-byte fragmentation regression seed --- ## Acceptance Criteria → Test → Demo Evidence | AC | BC Traces To | Test | Demo | |----|-------------|------|------| | AC-145-001: S2C carry drain symmetric | BC-2.07.041 v1.2 Inv 2; BC-2.07.038 v2.7 PC3b | `proptest_vp039_direction_isolation` | [AC-001-002-direction-carry-drain.webm](docs/demo-evidence/STORY-145/AC-001-002-direction-carry-drain.webm) | | AC-145-002: Direction selector → no cross-dir bleed | BC-2.07.041 v1.2 Inv 2; PC 1–2 | `proptest_vp039_direction_isolation` | Same as above | | AC-145-003: Cross-flow isolation | BC-2.07.041 v1.2 Inv 1, 4; PC 1, 4–5 | `test_BC_2_07_041_cross_flow_isolation` | [AC-003-cross-flow-isolation.webm](docs/demo-evidence/STORY-145/AC-003-cross-flow-isolation.webm) | | AC-145-004: Fragmented S2C → `server_hello_seen=true` | BC-2.07.002 v1.6 PC7, Inv 4 | `proptest_vp039_direction_isolation` (interleaved) | Covered by AC-001/002 recording | | AC-145-005: Single-record ServerHello regression | BC-2.07.002 v1.6 Inv 4; BC-2.07.038 v2.7 EC-007 | `test_parse_server_hello` + CLI smoke | [AC-005-single-record-regression.webm](docs/demo-evidence/STORY-145/AC-005-single-record-regression.webm) | --- ## Demo Evidence | AC | Shows | Recording | |----|-------|-----------| | AC-145-001/002 | Fragmented ServerHello reassembled; `server_hello_seen=true`, `ja3s_counts` non-empty, `parse_errors=0`, carries drain to 0 | `docs/demo-evidence/STORY-145/AC-001-002-direction-carry-drain.webm` | | AC-145-003 | Two concurrent flows, fragmented ServerHellos, `sni_counts==2`, no cross-flow bleed | `docs/demo-evidence/STORY-145/AC-003-cross-flow-isolation.webm` | | AC-145-005 | Single-record ServerHello regression-free | `docs/demo-evidence/STORY-145/AC-005-single-record-regression.webm` | AC-145-004 (server overflow/spoof guards) covered by `test_vp039_server_carry_overflow_clear_and_recover` and `test_vp039_server_body_len_spoof` unit tests — overflow/error-path tests produce no user-observable CLI output distinguishable from a normal pass. --- ## Test Evidence | Suite | Pass | Fail | Notes | |-------|------|------|-------| | `cargo test --test tls_analyzer_tests story_145` | 4 | 0 | All STORY-145 ACs covered | | `cargo test --all-targets` | 140 | 0 | Full suite including STORY-144 regressions | | `cargo clippy --all-targets -- -D warnings` | CLEAN | 0 | Zero warnings | | `cargo fmt --check` | CLEAN | — | Formatting gate passes | All tests pass locally on stable Rust (Rust 2024 edition, `rust-version = "1.91"`). --- ## Security Review Reviewed by `vsdd-factory:security-reviewer` against PR #343 diff (STORY-145). No CRITICAL or HIGH findings. | ID | Severity | CWE | Finding | Disposition | |----|----------|-----|---------|-------------| | SEC-001 | INFO | CWE-191 | Carry subtraction arithmetic — underflow evaluated | Confirmed sound. Loop invariant guarantees `consumed <= carry_len` at all times. | | SEC-002 | INFO | CWE-125 | `record_bytes[5..]` slice safety | Confirmed safe. `total_record_len = 5 + payload_len` guaranteed by upstream guards. | | SEC-003 | INFO | CWE-190 | `saturating_add` on overflow counter | Correct idiom; prevents panic under `overflow-checks = true`. | | SEC-004 | INFO | CWE-664 | Cross-direction isolation | Structurally enforced by exhaustive `match direction` arms; no cross-contamination path exists. | | SEC-005 | INFO | CWE-668 | Cross-flow isolation | HashMap keying enforces isolation; no global mutable carry state. | | SEC-006 | LOW | CWE-400 | Step-1 guard uses strict `>` — carry can reach exactly MAX_BUF (65,536 bytes) | Pre-existing property of the symmetric design inherited from STORY-144 (ClientToServer arm accepted with same condition). Not a regression. Carry is still bounded. | | SEC-007 | INFO | CWE-668 | Aggregate counters shared across flows | By design; documented. No security exploit path. | **DoS guards confirmed sound:** - Step-1 overflow: clear-and-recover fires when `carry_len_before + record_payload.len() > MAX_BUF (65,536)`. Bounded by per-record cap (`MAX_RECORD_PAYLOAD = 18,432`). - Decision-4 body_len spoof: guard fires when parsed `body_len > MAX_BUF`; clears carry, increments counter, post-loop drain skipped via `decision4_fired` flag. - Incomplete-message guard: `carry_len - consumed < 4 + body_len` — arithmetic safe per SEC-001 analysis. **Overall: APPROVE.** No CRITICAL/HIGH findings. SEC-006 (LOW) is pre-existing symmetric property, not a regression. --- ## Risk Assessment | Dimension | Assessment | |-----------|------------| | Blast radius | Narrow: SS-07 (`src/analyzer/tls.rs`) only. No new struct fields, no new files, no new dependencies. | | Regression risk | Low: ServerToClient drain path was previously a single-record dispatch; new path is a carry-buffer drain that degrades gracefully to single-record fast path on first invocation. 140 existing tests unaffected. | | Performance impact | O(carry_len) drain loop is bounded by `MAX_BUF=65536`. No quadratic growth. `saturating_add` overflow counter is O(1). | | DoS surface | Mitigated: Step-1 overflow guard (clear-and-recover at 65536 bytes) prevents unbounded accumulation. Decision-4 body_len spoof guard prevents false-completion on spoofed length field. | | Security correctness | Positive: closes S2C half of TLS-CLIENTHELLO-FRAG-001 silent evasion gap. Improves detection coverage. | --- ## AI Pipeline Metadata | Field | Value | |-------|-------| | Pipeline mode | Feature (f-sequence) | | Story wave | 66 | | Story phase | f3 (TDD implementation) | | TDD mode | strict | | Model | claude-sonnet-4-6 | | Story points | 5 | | Squash merge | Yes | --- ## Pre-Merge Checklist - [x] PR description populated with traceability - [x] Demo evidence present (3 WebM recordings, evidence-report.md) - [x] All STORY-145 tests pass locally (4/4) - [x] Full test suite passes locally (140/0) - [x] `cargo clippy --all-targets -- -D warnings` clean - [x] `cargo fmt --check` passes - [x] STORY-144 (depends_on) merged as PR #341 - [x] Security review complete (APPROVE — no CRITICAL/HIGH; SEC-006 LOW pre-existing) - [x] PR review approved (pr-reviewer — APPROVE, 0 blocking, 3 non-blocking nits) - [x] CI green — all 11 checks pass (action-pin-gate, audit, clippy, deny, fmt, fuzz-build, green-doc-tense-gate, help-provenance-gate, semantic-PR, test, trust-boundary)
Zious11
added a commit
that referenced
this pull request
Jun 30, 2026
… log SEC-006/dup/doc deferrals STATE.md: - current_step: STORY-145 MERGED, NEXT: STORY-146 - develop_head: 0986e87→d3d2e1989bc9ca5ecfc7b4eb3d3fa9fd80dccaa2 - stories_delivered: 92→93 - story_index_version: v3.6→v3.7 - story_index_note: STORY-145 MERGED wave 66 PR #343 d3d2e19; STORY-146 pending - Open worktrees: removed story-145 ACTIVE line (worktree removed) - EXACT RESUME POINT: rewritten to wave-66 STORY-146 NOT STARTED - RESUME PROCEDURE: updated to develop d3d2e19, new worktree creation for STORY-146 - Project Metadata table: develop HEAD + stories count updated - Phase Progress F4 row: STORY-145 MERGED - Current Phase Steps: added DONE row for STORY-145 TDD delivery; NEXT→STORY-146 - Decisions Log: added D-309 (STORY-145 MERGED PR #343, convergence, CI, human merge) - Open Items: TLS-CLIENTHELLO-FRAG-001 status updated; added SEC-006/TLS-DRAIN-DUP-001/TLS-STALE-COMMENT-001 - Session Resume Checkpoint: updated to STORY-145 MERGED; STORY-146 next - Notes footer: updated story-index version, develop HEAD, STORY-145 MERGED stories/STORY-145.md: - status: draft→merged (v1.1 doc already in place; input-hash 88e29c9 MATCH) stories/STORY-INDEX.md: - version: 3.6→3.7; timestamp updated - v3.7 changelog entry added - STORY-144 status: draft→merged (wave 65 PR #341 0986e87) - STORY-145 status: draft→merged (wave 66 PR #343 d3d2e19) - Wave 65 delivery row: DELIVERED & CLOSED #341 0986e87 2026-06-29 - Wave 66 delivery row: STORY-145 DELIVERED #343 d3d2e19; STORY-146 pending Count-propagation sweep: stories_delivered 92→93 updated in STATE.md frontmatter, metadata table, Notes footer, and STORY-INDEX changelog. Old count (92) remains only in immutable historical rows (D-307 past decision, 91→92 arrow-notation in step history) — intentional.
Open
8 tasks
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
TLS carry buffer and ClientHello fragmentation reassembly (STORY-144, Wave 65, E-5). An adversary who splits a TLS ClientHello across multiple TLS 0x16 records could evade SNI-based detection and JA3 fingerprinting on
develop(baseline: SNI=MISSED, JA3=MISSED, parse_errors=2). This PR closes that evasion gap by implementing a per-direction carry buffer inTlsFlowStatethat accumulates record payloads and dispatcheshandle_client_helloonly when the full handshake message is assembled.Evasion closed: fragmented ClientHello → SNI: ['example.com'], JA3: 6169fabc98e3e6c9..., parse_errors: 0.
Architecture Changes
graph TD A["TLS Record (0x16)"] --> B{direction} B -->|ClientToServer| C["client_hs_carry: Vec<u8>"] B -->|ServerToClient| D["server_hs_carry: Vec<u8>"] C --> E{overflow check} E -->|len + payload > MAX_BUF| F["clear + handshake_reassembly_overflows += 1"] E -->|ok| G["carry.extend_from_slice(payload)"] G --> H{drain loop} H -->|carry < 4| I["break: wait for more data"] H -->|body_len > MAX_BUF| J["clear + overflow += 1, break"] H -->|carry < 4 + body_len| I H -->|complete message| K{msg_type} K -->|0x01 ClientHello| L["parse_tls_message_handshake → handle_client_hello"] K -->|other| M["consume silently"] L -->|Ok| N["carry.drain(..4+body_len), loop"] L -->|Err| O["parse_errors += 1, carry.drain(..4+body_len), loop"] M --> NFiles changed:
src/analyzer/tls.rs(+330/-69 lines),tests/tls_analyzer_tests.rs(+1076 lines),tests/dispatcher_tests.rs(+76/-0 lines),docs/demo-evidence/STORY-144/(4 recordings + evidence-report.md).No changes to
src/reassembly/,src/dispatcher.rs,src/findings.rs,src/reporter/, ortests/tls_integration_tests.rs. SS-07 (analyzer/tls.rs) only.Story Dependencies
graph LR STORY144["STORY-144 (this PR)"] STORY145["STORY-145 (ServerHello carry)"] STORY146["STORY-146 (buffer_saturation_drops counter)"] STORY144 --> STORY145 STORY144 --> STORY146depends_on: []— no upstream story PRs required before this merge.blocks: [STORY-145, STORY-146]— STORY-145 (ServerHello direction carry) and STORY-146 (buffer_saturation_drops counter) depend on this PR merging first.Spec Traceability
flowchart LR BC038["BC-2.07.038 v2.7\nHandshake Reassembly\nAcross Record Boundaries"] BC039["BC-2.07.039 v2.4\nCarry Buffer Bounded\nat MAX_BUF"] BC040["BC-2.07.040 v1.3\nTruncated Carry\nSilent Discard"] BC042["BC-2.07.042 v1.4\nCoalesced Messages\nDrain Loop"] BC001["BC-2.07.001 v1.9\nParse Complete\nClientHello"] VP039["VP-039\nCarry Reassembly\nVerification Property"] BC038 --> AC144001["AC-144-001\nStruct fields +\ncounters"] BC038 --> AC144002["AC-144-002\nDrain loop +\ndispatch"] BC039 --> AC144003["AC-144-003\nOverflow clear-\nand-recover"] BC040 --> AC144004["AC-144-004\non_flow_close\nsilent discard"] BC001 --> AC144005["AC-144-005\nSingle-record\nregression"] BC042 --> AC144002 VP039 --> AC144002 AC144001 --> T001["test_BC_2_07_038_canonical_frame_rfc8446_s4\ntest_BC_2_07_038_malformed_assembled_body\ntest_vp039_sni_boundary_deterministic"] AC144002 --> T002["proptest_vp039_carry_reassembly_two_record\ntest_vp039_n_record_reassembly\ntest_vp039_large_valid_hello_reassembly\nproptest_vp039_exact_consume_coalesced\ntest_BC_2_07_042_exact_consume_no_double_dispatch\nproptest_vp039_carry_bounded_invariant"] AC144003 --> T003["test_vp039_carry_overflow_clear_and_recover\ntest_vp039_carry_overflow_recovery\ntest_vp039_body_len_spoof\ntest_BC_2_07_039_summarize_exposes_handshake_reassembly_overflows_key"] AC144004 --> T004["test_vp039_truncated_carry_no_error\ntest_BC_2_07_040_empty_carry_flow_close"] AC144005 --> T005["all 120 existing tests"] T001 --> IMPL["src/analyzer/tls.rs\n(TlsFlowState.client_hs_carry,\nserver_hs_carry;\nTlsAnalyzer.handshake_reassembly_overflows)"] T002 --> IMPL T003 --> IMPL T004 --> IMPL T005 --> IMPLTest Evidence
15 new test harnesses (all in
mod story_144 {}per DF-TEST-NAMESPACE-001):proptest_vp039_carry_reassembly_two_recordtest_BC_2_07_038_canonical_frame_rfc8446_s4test_BC_2_07_038_malformed_assembled_bodytest_vp039_sni_boundary_deterministictest_vp039_n_record_reassemblytest_vp039_large_valid_hello_reassemblyproptest_vp039_exact_consume_coalescedtest_BC_2_07_042_exact_consume_no_double_dispatchtest_vp039_carry_overflow_clear_and_recovertest_vp039_carry_overflow_recoverytest_vp039_body_len_spooftest_BC_2_07_039_summarize_exposes_handshake_reassembly_overflows_keytest_vp039_truncated_carry_no_errortest_BC_2_07_040_empty_carry_flow_closeproptest_vp039_carry_bounded_invariantDemo Evidence
Headline: Before / After (AC-144-002)
The fragmented ClientHello pcap was MISSED on
developbefore this fix:Recording:
docs/demo-evidence/STORY-144/AC-144-002-before-after-contrast.gifAC-144-002: Fragmented ClientHello Reassembly
AC-144-002-reassembly-fragmented.gif—tls-clienthello-fragmented.pcapagainst STORY-144 binary. Shows SNI=['example.com'], JA3 hash, parse_errors:0.AC-144-002: Single-Record Regression (Control)
AC-144-002-control-regression.gif—tls-clienthello-control.pcapagainst STORY-144 binary. SNI still extracted; no regression on single-record path.AC-144-003: Carry Overflow Clear-and-Recover
AC-144-003-overflow-clear-recover.gif—cargo testrun oftest_vp039_carry_overflow_clear_and_recover+test_BC_2_07_039_summarize_exposes_handshake_reassembly_overflows_key. Both pass. Shows the Decision-5 guard fires andhandshake_reassembly_overflowsis surfaced insummarize().All demo evidence is in
docs/demo-evidence/STORY-144/evidence-report.md.Holdout Evaluation
N/A — evaluated at wave gate (Phase F4 holdout scenarios HS-F4-001 through HS-F4-006 are gated at the wave level, not per-PR).
Adversarial Review
3 clean adversarial passes (BC-5.39.001) completed during implementation. All findings from the 3 cycles were triaged and resolved before PR creation.
Security Review
extend+drainon each loop iteration caused O(n²) on large coalesced messagestest_sec001_no_quadratic_drain)==MAX_BUFcondition edge (non-RFC exact boundary)saturating_addmissing onhandshake_reassembly_overflowscountersaturating_add(1)to prevent u64 overflowNo CRITICAL or HIGH unresolved findings.
Risk Assessment
AI Pipeline Metadata
Deferred Items
==MAX_BUFvs>MAX_BUFdone()-mid-loop cross-direction carry interactionPre-Merge Checklist