feat(analyzer): TLS ServerHello handshake reassembly across records (STORY-145, closes S2C SNI/JA3S fragmentation gap)#343
Merged
Conversation
Files created: none (tests/tls_analyzer_tests.rs modified) todo!() functions: 0 (no src changes; stub is structural — ServerToClient carry path in try_parse_records retains the legacy parse_tls_plaintext body per AC-144-005 preservation; the server_hs_carry drain loop is NOT added here) ## GREEN-BY-DESIGN | Function | Justification | |----------|--------------| | `make_test_flow_key(seed: u8)` | Pure constructor; IpAddr::from + wrapping_add; zero branching, no I/O, no helpers, 4 lines. | | `build_server_hello()` | Single delegation to flat-root build_server_hello(0x002f) with slice strip; zero branching, no I/O, 1 line. | | `build_client_hello_with_sni(sni: &str)` | Single delegation to flat-root build_client_hello; zero branching, no I/O, 1 line. | | `wrap_as_tls_record(content_type, payload)` | Pure byte-framing constructor; no branching on domain state, no I/O, 4 lines. | ## WIRING-EXEMPT none Red Gate: cargo check + clippy -D warnings clean; 136 existing tests GREEN; 2 new story_145 tests FAIL (server_hello_seen not set for fragmented S2C — ServerToClient carry drain path not yet wired). No real server reassembly logic added.
…ss-flow isolation) Replace stub shells with non-vacuous proptest_vp039_direction_isolation and test_BC_2_07_041_cross_flow_isolation per VP-039 Sub-E spec skeletons. proptest_vp039_direction_isolation: saturating clamp (not prop_assume), three independent analyzers (interleaved / c2s_only / s2c_only), equivalence assertions plus explicit truth guards (server_hello_seen==true, ja3s_counts non-empty) to prevent vacuous pass before STORY-145 wires the S2C carry drain. test_BC_2_07_041_cross_flow_isolation: renamed from lowercase stub name per BC-traceability convention; #[allow(non_snake_case)] added; body unchanged. Red Gate: both tests FAIL (server_hello_seen stays false on fragmented S2C). 136 existing tests GREEN. Clippy -Dwarnings + fmt clean.
…embly (STORY-145, S-145.01) Replaces the pre-existing parse_tls_plaintext path in the ServerToClient arm with a direction-parameterized cursor-based carry drain loop symmetric to the ClientToServer path introduced in STORY-144. Key changes in src/analyzer/tls.rs: - Direction::ServerToClient arm: append record_payload to server_hs_carry, then drain complete handshake messages via parse_tls_message_handshake. msg_type 0x02 (ServerHello) dispatches handle_server_hello and sets server_hello_seen; other types consumed silently (BC-2.07.038 Inv-1). - Same SEC-001 O(carry_len) cursor design, MAX_BUF overflow guard (Step 1), Decision-4 body_len-spoof guard, and single post-loop drain as C2S. - Remove unused imports: parse_tls_plaintext, Err as NomErr. - Update block-header comment and per-import comment. Tests: 138 passed / 0 failed (2 previously failing Red Gate tests now green: proptest_vp039_direction_isolation, test_BC_2_07_041_cross_flow_isolation). Clippy: clean. fmt: clean.
…(S-145, OBS-145-001)
…t carry-empty postcond (S-145, F-145-001/002) Add two missing unit tests inside mod story_145 that cover the ServerToClient carry-buffer DoS guards introduced by STORY-145: - test_vp039_server_carry_overflow_clear_and_recover: exercises the Step-1 pre-append overflow guard (tls.rs:987-994) by accumulating server_hs_carry past MAX_BUF=65536 across multiple valid records, then verifies clear-and-recover semantics. - test_vp039_server_body_len_spoof: exercises the Decision-4 body_len spoof guard (tls.rs:1025-1033) by sending a ServerHello header with body_len=65537 > MAX_BUF. Also strengthens proptest_vp039_direction_isolation with two additional AC-145-001 postcondition assertions: both client_hs_carry and server_hs_carry must be zero-length after complete fragmented delivery.
…-byte fragmentation edge case)
Owner
Author
Review Cycle 1 Triage — STORY-145
pr-reviewer findings (all NON-BLOCKING)
security-reviewer findings
All other security checks (SEC-001 through SEC-005, SEC-007): CONFIRMED SOUND. DoS guards confirmed sound
Convergence: 0 blocking findings → proceeding to CI gate. |
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.
Merged
8 tasks
Zious11
added a commit
that referenced
this pull request
Jun 30, 2026
…rops counter (STORY-146) (#344) # [STORY-146] TLS buffer-saturation telemetry — buffer_saturation_drops counter **Epic:** TLS analyzer observability hardening — defense-in-depth for silent tail-drop (F-EV-001) **Mode:** feature **Convergence:** CONVERGED after 3 adversarial passes (fresh-context, bb29117)     Adds a `buffer_saturation_drops: u64` aggregate telemetry counter to `TlsAnalyzer`. When a per-direction TCP-segment buffer (`client_buf` or `server_buf`) reaches `MAX_BUF = 65,536` and tail-drops incoming bytes, the counter increments once per `on_data` drop event (both directions share the same aggregate). The counter is surfaced in `summarize()` under the key `"buffer_saturation_drops"` — always present even at 0 (EC-008). This makes the previously-silent tail-drop non-silent and constitutes defense-in-depth for F-EV-001. Byte-drop semantics (BC-2.07.005) are UNCHANGED — this is a telemetry/observability change only. Implemented with `saturating_add(1)` (SEC-003 sibling consistency, avoids theoretical overflow-check panic under `overflow-checks = true` in the release profile). The `fill_buf_for_testing` seam is `#[doc(hidden)]` and test-only; it is not reachable from production input paths. --- ## Architecture Changes ```mermaid graph TD TlsAnalyzer["TlsAnalyzer<br/>(src/analyzer/tls.rs)"] TlsFlowState["TlsFlowState<br/>(client_buf / server_buf)"] StreamHandler["StreamHandler::on_data"] summarize["StreamAnalyzer::summarize()"] newField["buffer_saturation_drops: u64<br/>(NEW — aggregate on TlsAnalyzer)"] newAccessor["buffer_saturation_drop_count()<br/>(NEW — public accessor)"] newSeam["fill_buf_for_testing()<br/>(NEW — #[doc(hidden)] test seam)"] newKey["'buffer_saturation_drops' key<br/>(NEW — in summarize() detail map)"] TlsAnalyzer -->|owns| TlsFlowState TlsAnalyzer -->|contains| newField StreamHandler -->|reads client_buf/server_buf| TlsFlowState StreamHandler -->|increments via saturating_add| newField summarize -->|inserts| newKey newAccessor -->|reads| newField newSeam -->|fills for test| TlsFlowState style newField fill:#90EE90 style newAccessor fill:#90EE90 style newSeam fill:#90EE90 style newKey fill:#90EE90 ``` <details> <summary><strong>Architecture Decision Record</strong></summary> ### ADR: Aggregate counter on TlsAnalyzer (not TlsFlowState) **Context:** The per-direction buffer tail-drop path was previously silent — no caller could observe that bytes were dropped, making F-EV-001 (silent drop) undetectable via telemetry. **Decision:** Add `buffer_saturation_drops: u64` as an aggregate counter on `TlsAnalyzer` (not on `TlsFlowState`), initialized to 0, not reset on `on_flow_close`. Increment with `saturating_add(1)` after the `&mut state` borrow block closes (borrow-constraint mandated by Rust's aliasing rules). Mirror the existing `truncated_records` / `handshake_reassembly_overflows` aggregate counter pattern. **Rationale:** Aggregate counters on the analyzer struct (vs per-flow state) are the established pattern in this codebase. Post-block placement is required by Rust borrow rules: the mutable borrow on `self.flows` must be released before accessing `self.buffer_saturation_drops`. A `did_drop: bool` flag captures the condition inside the block and is read after. **Alternatives Considered:** 1. Per-flow counter on `TlsFlowState` — rejected: inconsistent with `truncated_records` / `handshake_reassembly_overflows` pattern; adds complexity without benefit. 2. `checked_add` with panic — rejected: release profile sets `overflow-checks = true`; `saturating_add` is SEC-003 compliant and matches sibling counter implementations. **Consequences:** - Counter is always present in `summarize()` output (even at 0), closing the F-EV-001 silent-drop gap. - `fill_buf_for_testing` seam is `#[doc(hidden)]` and gated by `debug_assert!` on `n <= MAX_BUF`; not reachable from production input paths. </details> --- ## Story Dependencies ```mermaid graph LR S144["STORY-144<br/>✅ MERGED<br/>TLS C2C reassembly"] --> S146["STORY-146<br/>🟡 this PR<br/>buffer saturation telemetry"] S145["STORY-145<br/>✅ MERGED<br/>TLS S2C reassembly"] --> S146 S146 --> future["(no downstream blocker)"] style S146 fill:#FFD700 style S144 fill:#90EE90 style S145 fill:#90EE90 ``` **depends_on:** STORY-144 (merged #341), STORY-145 (merged #343) **blocks:** none --- ## Spec Traceability ```mermaid flowchart LR BC043["BC-2.07.043 v1.3<br/>buffer_saturation_drops<br/>telemetry"] --> AC001["AC-146-001<br/>fill_buf seam exists"] BC043 --> AC002["AC-146-002<br/>counter increments on drop"] BC043 --> AC004["AC-146-004<br/>accessor buffer_saturation_drop_count"] BC043 --> AC005["AC-146-005<br/>summarize() key always present"] BC043 --> AC006["AC-146-006<br/>both directions, same aggregate"] BC005["BC-2.07.005 v1.7 (amended)<br/>byte-drop semantics UNCHANGED"] --> AC003["AC-146-003<br/>byte semantics preserved"] AC001 --> T1["test_BC_2_07_043_buffer_saturation_observable"] AC002 --> T2["test_BC_2_07_043_buffer_saturation_full_drop"] AC004 --> T3["test_BC_2_07_043_counter_persists_across_flows"] AC005 --> T4["test_BC_2_07_043_summarize_value_equals_drop_count"] AC006 --> T5["test_BC_2_07_043_both_directions_increment_same_counter"] T1 --> SRC["src/analyzer/tls.rs"] T2 --> SRC T3 --> SRC T4 --> SRC T5 --> SRC ``` **VP-040:** buffer_saturation_drops telemetry counter (BC-2.07.043 v1.3 + amended BC-2.07.005 v1.7) --- ## Test Evidence ### Coverage Summary | Metric | Value | Threshold | Status | |--------|-------|-----------|--------| | Unit tests (tls_analyzer_tests) | 148/148 pass | 100% | PASS | | New story_146 tests | 8/8 pass | 100% | PASS | | `overflow-checks = true` (release) | saturating_add verified | N/A | PASS | | Regression suite | 0 regressions | 0 | PASS | ### Test Flow ```mermaid graph LR Unit["8 New Unit Tests<br/>(mod story_146)"] Suite["148 Total<br/>(tls_analyzer_tests)"] Clippy["Clippy -D warnings"] Fmt["cargo fmt --check"] Unit -->|8/8 pass| Pass1["PASS"] Suite -->|148/148 pass| Pass2["PASS"] Clippy --> Pass3["PASS"] Fmt --> Pass4["PASS"] style Pass1 fill:#90EE90 style Pass2 fill:#90EE90 style Pass3 fill:#90EE90 style Pass4 fill:#90EE90 ``` | Metric | Value | |--------|-------| | **New tests** | 8 added (mod story_146), 0 modified | | **Total suite** | 148 tests PASS (tls_analyzer_tests) | | **Coverage delta** | positive (new drop path fully covered by 8 tests) | | **Mutation kill rate** | EC-C1 / EC-C3 edge cases explicitly pinned (partial_drop_boundary, full_buffer_empty_data_no_count) | | **Regressions** | 0 | <details> <summary><strong>Detailed Test Results</strong></summary> ### New Tests (mod story_146) | Test | Result | What It Covers | |------|--------|---------------| | `test_BC_2_07_043_buffer_saturation_observable()` | PASS | AC-146-001/002: fill_buf seam + drop detection | | `test_BC_2_07_043_buffer_saturation_full_drop()` | PASS | AC-146-002: full-drop path (remaining==0) | | `test_BC_2_07_043_no_drop_no_counter()` | PASS | no-drop path stays at 0 | | `test_BC_2_07_043_counter_persists_across_flows()` | PASS | AC-146-004: counter not reset on flow close | | `test_BC_2_07_043_summarize_value_equals_drop_count()` | PASS | AC-146-005: summarize() key matches accessor | | `test_BC_2_07_043_both_directions_increment_same_counter()` | PASS | AC-146-006: both directions aggregate | | `test_BC_2_07_043_partial_drop_boundary()` | PASS | EC-C1: partial-drop boundary (remaining>0, data overflows) | | `test_BC_2_07_043_full_buffer_empty_data_no_count()` | PASS | EC-C3: empty data on full buffer does not increment | </details> --- ## Holdout Evaluation N/A — evaluated at wave gate. --- ## Adversarial Review | Pass | Context | Findings | Critical | High | Status | |------|---------|----------|----------|------|--------| | 1 | Fresh-context on bb29117 | 4 | 0 | 1 (F-146-001: stale RED prose) | Fixed (b6545fa) | | 2 | Fresh-context post-fix | 3 | 0 | 1 (F-146-02: saturating_add) | Fixed (6a57eaa) | | 3 | Fresh-context post-fix | 2 | 0 | 0 (LOW only: accessor visibility, test-count header) | Fixed (bb29117) | **Convergence:** CONVERGED — pass 3 produced 0 blocking findings; residuals were cosmetic (doc OBS-146-01 corrected in 2f561c3). <details> <summary><strong>High-Severity Findings & Resolutions</strong></summary> ### Finding F-146-001: Stale RED/todo prose in comments/docs - **Location:** `src/analyzer/tls.rs` (multiple comment blocks) - **Category:** doc-quality - **Problem:** Comments from the Red Gate stub phase referenced `todo!()` and incomplete state. - **Resolution:** Green-tense doc sweep; all stale prose removed (commit b6545fa). - **Test added:** N/A — documentation fix. ### Finding F-146-02: Missing saturating_add for overflow safety - **Location:** `src/analyzer/tls.rs:on_data` - **Category:** code-correctness / SEC-003 - **Problem:** Initial implementation used `+= 1`; release profile sets `overflow-checks = true`, theoretical panic at u64::MAX. - **Resolution:** Changed to `saturating_add(1)` matching sibling `handshake_reassembly_overflows` pattern (commit 6a57eaa). - **Test added:** EC-C1 / EC-C3 edge-case tests (commit bb29117). ### Finding F-146-ADV-01: Accessor visibility and test-count header mismatch - **Location:** `src/analyzer/tls.rs`, README/docs - **Category:** spec-fidelity / doc-quality (LOW) - **Problem:** `buffer_saturation_drop_count` visibility parity with siblings; test-count header said 6 (should be 8). - **Resolution:** Accessor confirmed public (sibling parity); header corrected to 8 (commit bb29117). </details> --- ## Security Review ```mermaid graph LR Critical["Critical: 0"] High["High: 0"] Medium["Medium: 0"] Low["Low: 0 unresolved"] style Critical fill:#90EE90 style High fill:#90EE90 style Medium fill:#90EE90 style Low fill:#87CEEB ``` **Result: CLEAR — 0 CRITICAL, 0 HIGH, 0 MEDIUM, 0 LOW findings.** Security review completed (STORY-146 is a SECURITY-CORRECTNESS cycle component — defense-in-depth telemetry for F-EV-001 silent tail-drop). All 5 assertions confirmed safe: 1. **No new DoS surface (CWE-400, CWE-770):** counter is an 8-byte scalar; no allocation on drop path; fill_buf_for_testing not callable from network input. CONFIRMED SAFE. 2. **Overflow safety (CWE-190):** `saturating_add(1)` clamps at u64::MAX without panic; mirrors sibling `handshake_reassembly_overflows`; correct for `overflow-checks=true`. CONFIRMED SAFE. 3. **Test seam isolation (CWE-489):** `fill_buf_for_testing` is `#[doc(hidden)]`, has zero call sites outside test code; binary crate so external crate usage not a concern. CONFIRMED SAFE. 4. **Byte-drop semantics unchanged (CWE-20):** `did_drop` is read-only comparison; does not alter `to_copy`, `extend_from_slice`, or `try_parse_records` invocation. CONFIRMED SAFE. 5. **No injection in summarize() key (CWE-74):** key is a compile-time literal; value is u64 serialized as JSON number; no shadowing in BTreeMap. CONFIRMED SAFE. <details> <summary><strong>Security Scan Details</strong></summary> ### Dependency Audit - `cargo audit`: run by CI (`cargo-audit` job); expected CLEAN. - `cargo deny`: run by CI; expected CLEAN. ### Formal Verification | Property | Method | Status | |----------|--------|--------| | `saturating_add` overflow safety | `overflow-checks = true` in release profile | VERIFIED by build | | `fill_buf_for_testing` n <= MAX_BUF | `debug_assert!` | VERIFIED in test builds | | Byte-drop semantics unchanged | 148 existing tests green | VERIFIED | </details> --- ## Risk Assessment & Deployment ### Blast Radius - **Systems affected:** `src/analyzer/tls.rs` (TlsAnalyzer struct + on_data + summarize), `tests/tls_analyzer_tests.rs` - **User impact:** `summarize()` output gains one new key (`buffer_saturation_drops`); additive, not breaking. - **Data impact:** No persistent state change; in-memory counter only, not written to disk. - **Risk Level:** LOW — telemetry-only, additive output change, byte-drop semantics unchanged. ### Performance Impact | Metric | Before | After | Delta | Status | |--------|--------|-------|-------|--------| | on_data hot path | baseline | +1 comparison + conditional saturating_add | negligible | OK | | Memory (TlsAnalyzer) | baseline | +8 bytes (u64 field) | negligible | OK | | summarize() output | N keys | N+1 keys | additive | OK | <details> <summary><strong>Rollback Instructions</strong></summary> **Immediate rollback (< 5 min):** ```bash git revert b76ac70 git push origin develop ``` **Verification after rollback:** - `cargo test --all-targets` passes - `summarize()` output no longer contains `buffer_saturation_drops` key </details> ### Feature Flags | Flag | Controls | Default | |------|----------|---------| | N/A | No feature flag — always-on telemetry counter | — | --- ## Demo Evidence | AC | Shows | Recording | |----|-------|-----------| | AC-146-005 | `wirerust analyze --tls` summary shows `buffer_saturation_drops: 0` key present | `docs/demo-evidence/STORY-146/AC-146-005-summarize-key-present.webm` | | AC-146-001/002/004/006 | All 8 `story_146` tests pass (partial/full drop, both directions, persistence, value-equality) | `docs/demo-evidence/STORY-146/AC-146-001-002-004-006-test-suite.webm` | --- ## Traceability | Requirement | Story AC | Test | Verification | Status | |-------------|---------|------|-------------|--------| | BC-2.07.043 Inv.1 (field exists) | AC-146-001 | `test_BC_2_07_043_buffer_saturation_observable` | unit test | PASS | | BC-2.07.043 Inv.2 (observable drop) | AC-146-002 | `test_BC_2_07_043_buffer_saturation_full_drop` | unit test | PASS | | BC-2.07.043 Inv.3 (no false-positive) | AC-146-002 | `test_BC_2_07_043_no_drop_no_counter` | unit test | PASS | | BC-2.07.043 Inv.4 (persists) | AC-146-004 | `test_BC_2_07_043_counter_persists_across_flows` | unit test | PASS | | BC-2.07.043 Post.4 (always in summarize) | AC-146-005 | `test_BC_2_07_043_summarize_value_equals_drop_count` | unit test | PASS | | BC-2.07.043 Post.3 (both dirs aggregate) | AC-146-006 | `test_BC_2_07_043_both_directions_increment_same_counter` | unit test | PASS | | BC-2.07.005 v1.7 (byte semantics unchanged) | AC-146-003 | 148 existing tests green | regression | PASS | | VP-040 | all ACs above | mod story_146 (8 tests) | adversarial CONVERGED | PASS | <details> <summary><strong>Full VSDD Contract Chain</strong></summary> ``` BC-2.07.043 v1.3 -> VP-040 -> test_BC_2_07_043_buffer_saturation_observable -> src/analyzer/tls.rs:buffer_saturation_drops -> ADV-PASS-3-OK BC-2.07.043 v1.3 -> VP-040 -> test_BC_2_07_043_buffer_saturation_full_drop -> src/analyzer/tls.rs:on_data(did_drop) -> ADV-PASS-3-OK BC-2.07.005 v1.7 -> amended -> byte-drop semantics preserved -> 148 tests green -> regression clean ``` </details> --- ## AI Pipeline Metadata <details> <summary><strong>Pipeline Details</strong></summary> ```yaml ai-generated: true pipeline-mode: feature factory-version: "1.0.0" pipeline-stages: spec-crystallization: completed story-decomposition: completed tdd-implementation: completed holdout-evaluation: N/A - evaluated at wave gate adversarial-review: completed (3 passes, CONVERGED) formal-verification: skipped (telemetry-only, LOW risk) convergence: achieved convergence-metrics: adversarial-passes: 3 blocking-findings-at-convergence: 0 models-used: builder: claude-sonnet-4-6 adversary: claude-sonnet-4-6 (fresh-context) generated-at: "2026-06-30T00:00:00Z" ``` </details> --- ## Pre-Merge Checklist - [ ] All CI status checks passing - [x] Coverage delta is positive (8 new tests covering previously-uncovered drop path) - [x] No critical/high security findings unresolved (security review CLEAR; adversarial CONVERGED) - [x] Rollback procedure validated (git revert + push) - [x] No feature flag needed (always-on telemetry) - [ ] Human review / merge authorization (provided by dispatching human separately) - [x] Demo evidence committed at docs/demo-evidence/STORY-146/ (2 recordings, all ACs covered) - [x] Adversarial convergence: 3 passes, 0 blocking findings
Zious11
added a commit
that referenced
this pull request
Jun 30, 2026
…tegration gate PASS; resume→F4 holdout STORY-146 (TLS buffer-saturation telemetry, PR #344 squash 8b52046) merged. Multi-pass convergence (doc-tense, saturating_add SEC-003 sibling-parity, EC-C1/EC-C3 coverage, 6→8 test-count header drift, accessor visibility parity, EC-C1 docstring attribution; 3 clean on bb29117). pr-reviewer APPROVE + security-reviewer CLEAR; 11/11 CI green. stories_delivered 93→94. Wave-66 integration gate PASS (2220/0 on 8b52046). Pre-F4 input-hash drift scan: MATCH=95 STALE=0 ERROR=3 (pre-existing structural issues: STORY-001 retired BC path, STORY-091+STORY-121 no inputs block). STORY-145=88e29c9 MATCH, STORY-146=6d9da65 MATCH. STATE.md: develop_head→8b52046, stories_delivered=94, story_index_version=v3.8, current_step→F4 holdout, EXACT RESUME POINT rewritten for holdout entry, D-310 added, TLS-SILENT-COMMENT-001 + TLS-SUMMARIZE-MAPTYPE-001 added to backlog, session checkpoint updated (Wave 66 COMPLETE; F4 holdout next). STORY-146.md: status draft→merged (v1.1 edits already present: saturating_add, 8-test documentation, BC-2.07.005 title fix, EC-C1 docstring attribution, revision history). STORY-INDEX.md: v3.7→v3.8; STORY-146 status draft→merged; wave-66 delivery row DELIVERED & CLOSED (#343, #344; develop head 8b52046). Count-propagation sweep: stories_delivered=93 remaining only in historical delta notations (92→93 for STORY-145) and D-309 decision row — intentional immutable audit trail. develop_head d3d2e19 remaining only in historical context (D-309, STORY-145 delta rows, v3.7 changelog line) — intentional.
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.
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:
MAX_BUF=65536, per-record limit 18432)saturating_addaggregates overflows4 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
Verification Property: VP-039 Sub-E (direction isolation proptest + cross-flow isolation unit test)
Story Dependencies
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 theClientToServerdrain loop being in place.Architecture Changes
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 --> HMFiles changed:
src/analyzer/tls.rs— +128/-35 lines (ServerToClient 0x16 carry drain path wired)tests/tls_analyzer_tests.rs— +535 lines (4 new tests inmod story_145)docs/demo-evidence/STORY-145/— 3 WebM recordings + 3 GIFs + evidence-report.mdtests/tls_analyzer_tests.proptest-regressions— 1-byte fragmentation regression seedAcceptance Criteria → Test → Demo Evidence
proptest_vp039_direction_isolationproptest_vp039_direction_isolationtest_BC_2_07_041_cross_flow_isolationserver_hello_seen=trueproptest_vp039_direction_isolation(interleaved)test_parse_server_hello+ CLI smokeDemo Evidence
server_hello_seen=true,ja3s_countsnon-empty,parse_errors=0, carries drain to 0docs/demo-evidence/STORY-145/AC-001-002-direction-carry-drain.webmsni_counts==2, no cross-flow bleeddocs/demo-evidence/STORY-145/AC-003-cross-flow-isolation.webmdocs/demo-evidence/STORY-145/AC-005-single-record-regression.webmAC-145-004 (server overflow/spoof guards) covered by
test_vp039_server_carry_overflow_clear_and_recoverandtest_vp039_server_body_len_spoofunit tests — overflow/error-path tests produce no user-observable CLI output distinguishable from a normal pass.Test Evidence
cargo test --test tls_analyzer_tests story_145cargo test --all-targetscargo clippy --all-targets -- -D warningscargo fmt --checkAll tests pass locally on stable Rust (Rust 2024 edition,
rust-version = "1.91").Security Review
Reviewed by
vsdd-factory:security-revieweragainst PR #343 diff (STORY-145). No CRITICAL or HIGH findings.consumed <= carry_lenat all times.record_bytes[5..]slice safetytotal_record_len = 5 + payload_lenguaranteed by upstream guards.saturating_addon overflow counteroverflow-checks = true.match directionarms; no cross-contamination path exists.>— carry can reach exactly MAX_BUF (65,536 bytes)DoS guards confirmed sound:
carry_len_before + record_payload.len() > MAX_BUF (65,536). Bounded by per-record cap (MAX_RECORD_PAYLOAD = 18,432).body_len > MAX_BUF; clears carry, increments counter, post-loop drain skipped viadecision4_firedflag.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
src/analyzer/tls.rs) only. No new struct fields, no new files, no new dependencies.MAX_BUF=65536. No quadratic growth.saturating_addoverflow counter is O(1).AI Pipeline Metadata
Pre-Merge Checklist
cargo clippy --all-targets -- -D warningscleancargo fmt --checkpasses