diff --git a/CHANGELOG.md b/CHANGELOG.md index 2194d9b3..e129bfa7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,29 @@ Version numbers follow [Semantic Versioning](https://semver.org/). no S7comm-specific interpretation of the extracted protocol-ID byte. Includes a `#[cfg(kani)]` no-panic safety proof harness (VP-049; execution deferred to STORY-194). +- `S7commAnalyzer` (SS-21, `src/analyzer/s7comm.rs`, new module): the + effectful shell built on SS-20's stateless TPKT/COTP parsing library, + proving directional carry-buffer TPKT reassembly across TCP segment + boundaries. `S7commFlowState` holds the per-flow `carry_c2s`/`carry_s2c` + buffers (never merged) plus per-direction overflow-reported latches. + `on_data` implements walk-first, residual-bound frame extraction: it + appends incoming bytes to the directional carry, repeatedly calls + `iso_on_tcp::parse_tpkt_header`/`parse_cotp_header` to extract and dispatch + complete frames, advances the cursor, and stashes only the leftover + partial-frame residual back to carry — never an aggregate + `carry.len() + data.len()` pre-check (BC-2.20.013, STORY-186, ADR-014 + Decision 8). A bad TPKT version byte triggers the shared 1-byte resync + sub-routine (BC-2.20.015), reused verbatim for both an ordinary mid-stream + reject and post-overflow resync. The residual carry is bounded by + `MAX_S7_ISO_ON_TCP_CARRY_BYTES = 65,535` (derived from TPKT's own `u16` + length maximum); exceeding it clears the carry and emits one T0814 finding + per direction (BC-2.20.014) — retained as a defense-in-depth guard against + future design regressions, since it is unreachable via `on_data` under the + current walk-first/resync design (BC-2.20.014 v1.1 Invariant 5). + `on_flow_close` removes a flow's `S7commFlowState` and discards any + carry bytes with no finding emitted (BC-2.21.003). Protocol-specific + dispatch on the extracted `protocol_id` is out of scope for this story + (STORY-187). ## [0.13.3] - 2026-09-05 diff --git a/docs/adr/0014-s7comm-iso-on-tcp-stream-dispatch-and-parser-design.md b/docs/adr/0014-s7comm-iso-on-tcp-stream-dispatch-and-parser-design.md index 109fb330..8b738066 100644 --- a/docs/adr/0014-s7comm-iso-on-tcp-stream-dispatch-and-parser-design.md +++ b/docs/adr/0014-s7comm-iso-on-tcp-stream-dispatch-and-parser-design.md @@ -421,9 +421,10 @@ T0836 (Modify Parameter, `Write Var 0x05` → `0x84`/`0x83`), T0858 (Change Oper restart PI-service string), T0888 (Remote System Information Discovery, Userdata `0x07`/CPU-group `0x04`/subfn `0x01` Read SZL, or Block-group `0x03`), T0846 (Remote System Discovery, multi-host TCP/102 sweep evidence only, not single-PDU), T0814 (Denial -of Service, connection-flood/malformed-length burst thresholds), T1692.001 (Unauthorized -Message: Command Message, successor to revoked T0855, any command from an unauthorized -source). +of Service, connection-flood/malformed-length burst thresholds — see Decision 8's +carry-overflow reconciliation note for the carry-overflow trigger specifically), +T1692.001 (Unauthorized Message: Command Message, successor to revoked T0855, any +command from an unauthorized source). **Group-`0x03` block-function correction:** the Userdata (ROSCTR `0x07`) subfunction group table must read group `0x03` = **Block functions** (`0x01` List blocks, `0x02` @@ -553,6 +554,21 @@ to the next `0x03` candidate (drop-and-rescan, not a permanent desync latch), an carry-overflow dedup flag, distinct from the malformed-length dedup flag used for in-range TPKT-length validation failures. +> **RECONCILIATION NOTE (2026-09-07, STORY-186 adversarial finding F-02; BC-2.20.014 +> v1.1):** A two-independent-pass STORY-186 adversarial gate found this carry-overflow +> → T0814 branch **unreachable via the real `on_data` path**: BC-2.20.013's walk-first +> frame extraction and BC-2.20.015's 1-byte resync both consume at least one byte of +> input on every call, and the TPKT `length` field is u16-capped — so the residual +> carry stashed back into `carry_c2s`/`carry_s2c` can never exceed 65,534 bytes, one +> byte below `MAX_S7_ISO_ON_TCP_CARRY_BYTES = 65,535`. Per human ruling (Option B), this +> paragraph describes a **defense-in-depth, unreachable-by-construction guard** — +> retained against a future design regression that removed the walk-first/resync +> per-call consumption guarantee — **not a live, observable runtime detection** under +> the current design. Product-owner has amended BC-2.20.014 → v1.1 and BC-2.20.013 → +> v1.1 accordingly. The paragraph above is left as-is as the guard's specified reaction +> *if* it were ever reached; it must not be read as evidence that T0814 fires from carry +> overflow in practice today. + ### Decision 9: Pure-core free-fn design for verification amenability Three functions are pure-core free `fn`s (module scope, not `impl` methods), following @@ -725,7 +741,7 @@ critical caveat) remains out of scope until F4 (Decision 10). | T0816 | Device Restart/Shutdown | Decoded `0x28` restart PI-service | Pre-existing EMITTED (ENIP); add S7comm call-site | | T0888 | Remote System Information Discovery | Userdata `0x07`/`0x04`/`0x01` Read SZL; `0x07`/`0x03`/* block-list | Pre-existing EMITTED (Modbus); add S7comm call-site | | T0846 | Remote System Discovery | Multi-host TCP/102 sweep evidence only | Pre-existing EMITTED (ENIP); emit only on sweep evidence | -| T0814 | Denial of Service | Connection flood; malformed-length burst threshold | Pre-existing EMITTED; add S7comm call-site | +| T0814 | Denial of Service | Connection flood; malformed-length burst threshold | Pre-existing EMITTED; add S7comm call-site (the carry-overflow call-site specifically is defense-in-depth / unreachable-by-construction — see Decision 8's reconciliation note and BC-2.20.014 v1.1) | | T1692.001 | Unauthorized Message: Command Message | Any command from a source outside an allowlist | Pre-existing EMITTED; co-tag only with positive unauthorized-source evidence | CWE set: CWE-306 (no authentication — classic S7comm has none), CWE-319 (cleartext diff --git a/docs/demo-evidence/STORY-186/AC-001-003-carry-reassembly.gif b/docs/demo-evidence/STORY-186/AC-001-003-carry-reassembly.gif new file mode 100644 index 00000000..a135c424 Binary files /dev/null and b/docs/demo-evidence/STORY-186/AC-001-003-carry-reassembly.gif differ diff --git a/docs/demo-evidence/STORY-186/AC-001-003-carry-reassembly.tape b/docs/demo-evidence/STORY-186/AC-001-003-carry-reassembly.tape new file mode 100644 index 00000000..d3d0c9e8 --- /dev/null +++ b/docs/demo-evidence/STORY-186/AC-001-003-carry-reassembly.tape @@ -0,0 +1,36 @@ +# STORY-186 AC-186-001/002/003: carry-buffer reassembly, walk-first extraction (BC-2.20.013) +# +# Demonstrates: +# - test_BC_2_20_013_walk_first_no_aggregate_precheck: no aggregate carry+incoming +# pre-check exists anywhere -- the walk always runs first (AC-186-001) +# - test_BC_2_20_013_adversarial_burst_head_frame_not_dropped: a complete 7-byte CR +# frame followed by 60,000 bytes of trailing garbage in ONE on_data call is still +# extracted (anti-evasion property, AC-186-002) +# - test_BC_2_20_013_split_frame_across_two_calls: a 4-byte TPKT header delivered in +# call 1 is stashed to carry; call 2 completes the 10-byte frame and empties carry +# (AC-186-003) +# +# Traces to: BC-2.20.013 postconditions 1-2, invariant 1, edge case EC-002 + +Output AC-001-003-carry-reassembly.gif +Output AC-001-003-carry-reassembly.webm + +Set FontFamily "Menlo" +Set Theme "Dracula" +Set Width 1200 +Set Height 500 +Set FontSize 14 +Set Padding 24 +Set Shell "bash" +Set TypingSpeed 50ms + +Require cargo + +Type "# AC-186-001/002/003: walk-first carry-buffer reassembly (BC-2.20.013)" +Enter +Sleep 300ms + +Type "cargo test --test s7comm_analyzer_tests BC_2_20_013 2>&1 | grep -E 'story_186::test|test result:'" +Enter +Wait +Sleep 2s diff --git a/docs/demo-evidence/STORY-186/AC-001-003-carry-reassembly.webm b/docs/demo-evidence/STORY-186/AC-001-003-carry-reassembly.webm new file mode 100644 index 00000000..7db3dddf Binary files /dev/null and b/docs/demo-evidence/STORY-186/AC-001-003-carry-reassembly.webm differ diff --git a/docs/demo-evidence/STORY-186/AC-004-006-defense-in-depth.gif b/docs/demo-evidence/STORY-186/AC-004-006-defense-in-depth.gif new file mode 100644 index 00000000..90ca003a Binary files /dev/null and b/docs/demo-evidence/STORY-186/AC-004-006-defense-in-depth.gif differ diff --git a/docs/demo-evidence/STORY-186/AC-004-006-defense-in-depth.tape b/docs/demo-evidence/STORY-186/AC-004-006-defense-in-depth.tape new file mode 100644 index 00000000..51df628c --- /dev/null +++ b/docs/demo-evidence/STORY-186/AC-004-006-defense-in-depth.tape @@ -0,0 +1,48 @@ +# STORY-186 AC-186-004/005/006: carry bound + defense-in-depth overflow guard (BC-2.20.014) +# +# Demonstrates: +# - test_BC_2_20_014_at_bound_residual_no_overflow: residual == 65,535 (the u16::MAX +# ceiling) is LIVE, reachable via real on_data traffic -- no overflow fires, strict +# '>' not '>=' (AC-186-004, the only reachable-via-on_data case in this group) +# - test_BC_2_20_014_overflow_clear_resync_one_t0814_per_direction: [SYNTHETIC] guard +# mechanics via direct flow-state injection -- clear-not-truncate, resync, exactly +# one T0814 (AC-186-005) +# - test_BC_2_20_014_repeated_overflow_dedup_same_direction: [SYNTHETIC] second +# injected overflow in the same direction does not re-emit (AC-186-005) +# - test_BC_2_20_014_overflow_dedup_independent_per_direction: [SYNTHETIC] c2s and +# s2c dedup flags are independent (AC-186-006) +# - test_BC_2_20_014_overflow_unreachable_via_on_data: positive on_data-path proof -- +# a real 200,000-byte non-anchored garbage flood emits NO T0814 and carry stays +# bounded <= 65,534 (AC-186-005 positive assertion, VP-050 reachability property) +# +# Reclassification: BC-2.20.014 v1.1 (human ruling 2026-09-07, Option B) -- the +# residual.len() > 65,535 guard is a structural defense-in-depth safety net, provably +# unreachable via the real on_data data path under BC-2.20.013 walk-first + BC-2.20.015 +# 1-byte-resync (TPKT length is u16-capped, so carry is bounded <= 65,534 by +# construction). AC-186-004 (at-bound) remains live; AC-186-005/006 (over-bound) are +# exercised only via direct flow-state injection, named SYNTHETIC below. +# +# Traces to: BC-2.20.014 invariant 1, postconditions 1/3/4, edge cases EC-001/EC-004/EC-005 + +Output AC-004-006-defense-in-depth.gif +Output AC-004-006-defense-in-depth.webm + +Set FontFamily "Menlo" +Set Theme "Dracula" +Set Width 1200 +Set Height 560 +Set FontSize 14 +Set Padding 24 +Set Shell "bash" +Set TypingSpeed 50ms + +Require cargo + +Type "# AC-186-004 (LIVE at-bound) + AC-186-005/006 (SYNTHETIC guard mechanics) (BC-2.20.014)" +Enter +Sleep 300ms + +Type "cargo test --test s7comm_analyzer_tests BC_2_20_014 2>&1 | grep -E 'story_186::test|test result:'" +Enter +Wait +Sleep 2s diff --git a/docs/demo-evidence/STORY-186/AC-004-006-defense-in-depth.webm b/docs/demo-evidence/STORY-186/AC-004-006-defense-in-depth.webm new file mode 100644 index 00000000..5be43717 Binary files /dev/null and b/docs/demo-evidence/STORY-186/AC-004-006-defense-in-depth.webm differ diff --git a/docs/demo-evidence/STORY-186/AC-007-009-resync.gif b/docs/demo-evidence/STORY-186/AC-007-009-resync.gif new file mode 100644 index 00000000..58a90213 Binary files /dev/null and b/docs/demo-evidence/STORY-186/AC-007-009-resync.gif differ diff --git a/docs/demo-evidence/STORY-186/AC-007-009-resync.tape b/docs/demo-evidence/STORY-186/AC-007-009-resync.tape new file mode 100644 index 00000000..65e1e6b5 --- /dev/null +++ b/docs/demo-evidence/STORY-186/AC-007-009-resync.tape @@ -0,0 +1,37 @@ +# STORY-186 AC-186-007/008/009: 1-byte resync, never 2 (BC-2.20.015) +# +# Demonstrates: +# - test_BC_2_20_015_resync_advances_exactly_one_byte: bytes +# [0x01, 0x03, 0x00, 0x00, 0x07] -- a spurious 0x01 immediately followed by a valid +# length=7 frame at offset 1; a 2-byte advance would skip it entirely (AC-186-007) +# - test_BC_2_20_015_single_resync_implementation_shared: the same 1-byte-advance +# sub-routine is invoked for both bad-version-byte and post-overflow conditions -- +# exactly one resync implementation, not two (AC-186-008) +# - test_BC_2_20_015_resync_terminates_no_valid_anchor: 200 bytes of non-0x03 garbage +# with no valid frame anywhere -- resync advances to the end without an infinite +# loop (AC-186-009) +# +# Traces to: BC-2.20.015 postcondition 1, invariants 1-3 + +Output AC-007-009-resync.gif +Output AC-007-009-resync.webm + +Set FontFamily "Menlo" +Set Theme "Dracula" +Set Width 1200 +Set Height 500 +Set FontSize 14 +Set Padding 24 +Set Shell "bash" +Set TypingSpeed 50ms + +Require cargo + +Type "# AC-186-007/008/009: resync advances exactly 1 byte, never 2 (BC-2.20.015)" +Enter +Sleep 300ms + +Type "cargo test --test s7comm_analyzer_tests BC_2_20_015 2>&1 | grep -E 'story_186::test|test result:'" +Enter +Wait +Sleep 2s diff --git a/docs/demo-evidence/STORY-186/AC-007-009-resync.webm b/docs/demo-evidence/STORY-186/AC-007-009-resync.webm new file mode 100644 index 00000000..1d302599 Binary files /dev/null and b/docs/demo-evidence/STORY-186/AC-007-009-resync.webm differ diff --git a/docs/demo-evidence/STORY-186/AC-010-011-module-boundary.gif b/docs/demo-evidence/STORY-186/AC-010-011-module-boundary.gif new file mode 100644 index 00000000..ce0079e9 Binary files /dev/null and b/docs/demo-evidence/STORY-186/AC-010-011-module-boundary.gif differ diff --git a/docs/demo-evidence/STORY-186/AC-010-011-module-boundary.tape b/docs/demo-evidence/STORY-186/AC-010-011-module-boundary.tape new file mode 100644 index 00000000..389fda01 --- /dev/null +++ b/docs/demo-evidence/STORY-186/AC-010-011-module-boundary.tape @@ -0,0 +1,34 @@ +# STORY-186 AC-186-010/011: frozen SS-20/SS-21 module boundary (BC-2.20.016) +# +# Demonstrates: +# - test_BC_2_20_016_iso_on_tcp_has_no_stream_analyzer_impl: static regression-guard +# confirms src/analyzer/iso_on_tcp.rs contains zero `impl StreamAnalyzer` blocks and +# zero DispatchTarget::IsoOnTcp-shaped references (AC-186-010) +# - test_BC_2_20_016_no_iso_on_tcp_flow_state_type_exists: static regression-guard +# confirms no `IsoOnTcpFlowState` type exists anywhere in the tree -- carry buffers +# live exclusively on S7commFlowState (SS-21) (AC-186-011) +# +# Traces to: BC-2.20.016 postconditions 1 and 3 + +Output AC-010-011-module-boundary.gif +Output AC-010-011-module-boundary.webm + +Set FontFamily "Menlo" +Set Theme "Dracula" +Set Width 1200 +Set Height 460 +Set FontSize 14 +Set Padding 24 +Set Shell "bash" +Set TypingSpeed 50ms + +Require cargo + +Type "# AC-186-010/011: iso_on_tcp.rs frozen module boundary regression guards (BC-2.20.016)" +Enter +Sleep 300ms + +Type "cargo test --test s7comm_analyzer_tests BC_2_20_016 2>&1 | grep -E 'story_186::test|test result:'" +Enter +Wait +Sleep 2s diff --git a/docs/demo-evidence/STORY-186/AC-010-011-module-boundary.webm b/docs/demo-evidence/STORY-186/AC-010-011-module-boundary.webm new file mode 100644 index 00000000..c6440c7d Binary files /dev/null and b/docs/demo-evidence/STORY-186/AC-010-011-module-boundary.webm differ diff --git a/docs/demo-evidence/STORY-186/AC-012-flow-close.gif b/docs/demo-evidence/STORY-186/AC-012-flow-close.gif new file mode 100644 index 00000000..31c2124e Binary files /dev/null and b/docs/demo-evidence/STORY-186/AC-012-flow-close.gif differ diff --git a/docs/demo-evidence/STORY-186/AC-012-flow-close.tape b/docs/demo-evidence/STORY-186/AC-012-flow-close.tape new file mode 100644 index 00000000..e4828630 --- /dev/null +++ b/docs/demo-evidence/STORY-186/AC-012-flow-close.tape @@ -0,0 +1,34 @@ +# STORY-186 AC-186-012: on_flow_close teardown + double-close idempotency (BC-2.21.003) +# +# Demonstrates: +# - test_s7comm_on_flow_close_removes_state_discards_carry: on_flow_close removes +# S7commFlowState from the per-flow map and discards carry bytes with no finding +# emitted (AC-186-012) +# - test_BC_2_21_003_double_close_same_flow_key_is_idempotent_no_op: calling +# on_flow_close a second time for the same flow_key (or an unknown flow_key) is a +# no-op -- does not panic, does not re-emit +# +# Traces to: BC-2.21.003 postconditions 1-4 + +Output AC-012-flow-close.gif +Output AC-012-flow-close.webm + +Set FontFamily "Menlo" +Set Theme "Dracula" +Set Width 1200 +Set Height 460 +Set FontSize 14 +Set Padding 24 +Set Shell "bash" +Set TypingSpeed 50ms + +Require cargo + +Type "# AC-186-012: on_flow_close removes state, discards carry, double-close is a no-op (BC-2.21.003)" +Enter +Sleep 300ms + +Type "cargo test --test s7comm_analyzer_tests close 2>&1 | grep -E 'story_186::test|test result:'" +Enter +Wait +Sleep 2s diff --git a/docs/demo-evidence/STORY-186/AC-012-flow-close.webm b/docs/demo-evidence/STORY-186/AC-012-flow-close.webm new file mode 100644 index 00000000..fe136fc3 Binary files /dev/null and b/docs/demo-evidence/STORY-186/AC-012-flow-close.webm differ diff --git a/docs/demo-evidence/STORY-186/AC-ALL-18-green.gif b/docs/demo-evidence/STORY-186/AC-ALL-18-green.gif new file mode 100644 index 00000000..f0a476aa Binary files /dev/null and b/docs/demo-evidence/STORY-186/AC-ALL-18-green.gif differ diff --git a/docs/demo-evidence/STORY-186/AC-ALL-18-green.tape b/docs/demo-evidence/STORY-186/AC-ALL-18-green.tape new file mode 100644 index 00000000..7f20c5c8 --- /dev/null +++ b/docs/demo-evidence/STORY-186/AC-ALL-18-green.tape @@ -0,0 +1,28 @@ +# STORY-186 full suite: all 18 tests green (15 unit/regression-guard tests + 3 VP-050 +# proptests) across BC-2.20.013/014/015/016 and BC-2.21.003 +# +# This is the top-level artifact showing the complete s7comm_analyzer_tests.rs suite +# passing in one run -- every AC-186-001..012 test is represented in this output. + +Output AC-ALL-18-green.gif +Output AC-ALL-18-green.webm + +Set FontFamily "Menlo" +Set Theme "Dracula" +Set Width 1200 +Set Height 900 +Set FontSize 13 +Set Padding 24 +Set Shell "bash" +Set TypingSpeed 40ms + +Require cargo + +Type "# STORY-186: full s7comm_analyzer_tests suite -- 18/18 green" +Enter +Sleep 300ms + +Type "cargo test --test s7comm_analyzer_tests 2>&1 | grep -E 'running [0-9]+ tests|story_186::|test result:'" +Enter +Wait +Sleep 3s diff --git a/docs/demo-evidence/STORY-186/AC-ALL-18-green.webm b/docs/demo-evidence/STORY-186/AC-ALL-18-green.webm new file mode 100644 index 00000000..2267e0b6 Binary files /dev/null and b/docs/demo-evidence/STORY-186/AC-ALL-18-green.webm differ diff --git a/docs/demo-evidence/STORY-186/VP-050-proptests.gif b/docs/demo-evidence/STORY-186/VP-050-proptests.gif new file mode 100644 index 00000000..1801a40d Binary files /dev/null and b/docs/demo-evidence/STORY-186/VP-050-proptests.gif differ diff --git a/docs/demo-evidence/STORY-186/VP-050-proptests.tape b/docs/demo-evidence/STORY-186/VP-050-proptests.tape new file mode 100644 index 00000000..b930eb45 --- /dev/null +++ b/docs/demo-evidence/STORY-186/VP-050-proptests.tape @@ -0,0 +1,36 @@ +# STORY-186 VP-050: proptest obligation (direction isolation, walk-first residual bound, +# resync one-byte-advance) over randomized inputs +# +# Demonstrates: +# - proptest_vp050_walk_first_residual_bound: carry stays <= MAX_S7_ISO_ON_TCP_CARRY_BYTES +# across randomized delivery patterns +# - proptest_vp050_direction_isolation: carry_c2s only ever contains C2S-routed bytes, +# carry_s2c only ever contains S2C-routed bytes, across 0..300-byte random payloads +# - proptest_vp050_resync_one_byte_advance: resync never advances by more than 1 byte +# per iteration, across randomized garbage-length inputs +# +# Method: proptest (randomized property-based testing, anchored in this story; +# the full walk-first equivalence property lands in STORY-194) + +Output VP-050-proptests.gif +Output VP-050-proptests.webm + +Set FontFamily "Menlo" +Set Theme "Dracula" +Set Width 1200 +Set Height 460 +Set FontSize 14 +Set Padding 24 +Set Shell "bash" +Set TypingSpeed 50ms + +Require cargo + +Type "# VP-050: proptest harnesses -- direction isolation, walk-first bound, resync advance" +Enter +Sleep 300ms + +Type "cargo test --test s7comm_analyzer_tests vp050 2>&1 | grep -E 'story_186::vp050|test result:'" +Enter +Wait +Sleep 2s diff --git a/docs/demo-evidence/STORY-186/VP-050-proptests.webm b/docs/demo-evidence/STORY-186/VP-050-proptests.webm new file mode 100644 index 00000000..e5d90e4a Binary files /dev/null and b/docs/demo-evidence/STORY-186/VP-050-proptests.webm differ diff --git a/docs/demo-evidence/STORY-186/evidence-report.md b/docs/demo-evidence/STORY-186/evidence-report.md new file mode 100644 index 00000000..f4c23689 --- /dev/null +++ b/docs/demo-evidence/STORY-186/evidence-report.md @@ -0,0 +1,191 @@ +# Demo Evidence Report — STORY-186 + +**Story:** STORY-186: S7comm ISO-on-TCP Carry-Buffer Reassembly, Walk-First Frame +Extraction, Resync, and the Frozen SS-20/SS-21 Module Boundary +**Wave:** 89 +**Date:** 2026-09-07 +**Branch:** feature/STORY-186-iso-on-tcp-reassembly +**Product type:** Library (pure-core parser consumed by a new effectful-shell analyzer, +`S7commAnalyzer` in `src/analyzer/s7comm.rs`) — there is no CLI subcommand or web UI +surface for S7comm yet (dispatcher wiring to the CLI is deferred to STORY-193). The +demonstration vehicle is this story's own test harness: `tests/s7comm_analyzer_tests.rs` +(18 tests) is the executable proof of each acceptance criterion. +**Recording tool:** VHS 0.11.0 (terminal recordings of `cargo test --test +s7comm_analyzer_tests`, filtered per behavior group) + +--- + +## Full Test Suite: 18/18 PASS + +Command: +``` +cargo test --test s7comm_analyzer_tests +``` + +Result: **18 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in ~4.6s** +(the 3 VP-050 proptests account for essentially all of the wall-clock time; the 15 +unit/regression-guard tests each finish in under a millisecond). + +Top-level artifact (all 18 tests, grouped by module, shown in one recording): + +| Artifact | Description | +|----------|-------------| +| `AC-ALL-18-green.gif` / `AC-ALL-18-green.webm` | Full `s7comm_analyzer_tests` suite — 18/18 green | +| `AC-ALL-18-green.tape` | VHS script for the master suite run | + +--- + +## AC → Test → Artifact Coverage Map + +| AC | Title | BC | Test(s) | Artifact | Verdict | +|----|-------|-----|---------|----------|---------| +| AC-186-001 | Frame-walk loop extracts every complete TPKT frame before any byte-count bound is applied; no aggregate carry+incoming pre-check exists | BC-2.20.013 PC-1, PC-2, Inv-1 | `test_BC_2_20_013_walk_first_no_aggregate_precheck` | `AC-001-003-carry-reassembly.gif/.webm` | PASS | +| AC-186-002 | Adversarial burst with a complete frame at the head is never dropped despite 60,000 bytes of trailing garbage (anti-evasion) | BC-2.20.013 Inv-1 | `test_BC_2_20_013_adversarial_burst_head_frame_not_dropped` | `AC-001-003-carry-reassembly.gif/.webm` | PASS | +| AC-186-003 | Split-frame reassembly across two `on_data` calls — header-only partial stashed, then completed and carry emptied | BC-2.20.013 EC-002 | `test_BC_2_20_013_split_frame_across_two_calls` | `AC-001-003-carry-reassembly.gif/.webm` | PASS | +| AC-186-004 | Carry buffer bounded at 65,535 bytes; at-bound residual (`== 65,535`) is legitimate, not overflow — **LIVE, reachable via real `on_data` traffic** (comparison is strict `>`, not `>=`) | BC-2.20.014 Inv-1, EC-001 | `test_BC_2_20_014_at_bound_residual_no_overflow` | `AC-004-006-defense-in-depth.gif/.webm` | PASS | +| AC-186-005 | **[DEFENSE-IN-DEPTH, SYNTHETIC]** Carry-overflow guard mechanics — clear-not-truncate, resync, exactly one T0814 per direction, dedup on repeat — exercised via direct flow-state injection, not via `on_data`; **plus** the positive on_data-driven unreachability proof (200,000-byte garbage flood emits no T0814, carry stays ≤ 65,534) | BC-2.20.014 PC-1, PC-3, PC-4, EC-004 | `test_BC_2_20_014_overflow_clear_resync_one_t0814_per_direction`, `test_BC_2_20_014_repeated_overflow_dedup_same_direction`, `test_BC_2_20_014_overflow_unreachable_via_on_data` | `AC-004-006-defense-in-depth.gif/.webm` | PASS | +| AC-186-006 | **[DEFENSE-IN-DEPTH, SYNTHETIC]** Overflow dedup flags are independent per direction (c2s dedup has no bearing on s2c) — guard mechanics via direct flow-state injection | BC-2.20.014 PC-4, EC-005 | `test_BC_2_20_014_overflow_dedup_independent_per_direction` | `AC-004-006-defense-in-depth.gif/.webm` | PASS | +| AC-186-007 | Resync advances exactly 1 byte per iteration on a bad TPKT version byte, never 2 | BC-2.20.015 PC-1, Inv-1 | `test_BC_2_20_015_resync_advances_exactly_one_byte` | `AC-007-009-resync.gif/.webm` | PASS | +| AC-186-008 | Resync sub-routine is reused verbatim for both bad-version-byte and post-overflow conditions — exactly one implementation | BC-2.20.015 Inv-3 | `test_BC_2_20_015_single_resync_implementation_shared` | `AC-007-009-resync.gif/.webm` | PASS | +| AC-186-009 | Resync always terminates for finite input (200 bytes of non-anchored garbage, no infinite loop) | BC-2.20.015 Inv-2 | `test_BC_2_20_015_resync_terminates_no_valid_anchor` | `AC-007-009-resync.gif/.webm` | PASS | +| AC-186-010 | `iso_on_tcp.rs` contains zero `impl StreamAnalyzer` blocks / `DispatchTarget::IsoOnTcp`-shaped references (frozen module boundary, static regression guard) | BC-2.20.016 PC-1 | `test_BC_2_20_016_iso_on_tcp_has_no_stream_analyzer_impl` | `AC-010-011-module-boundary.gif/.webm` | PASS | +| AC-186-011 | TPKT/COTP carry buffers live on `S7commFlowState` only; no `IsoOnTcpFlowState` type exists anywhere in the tree (static regression guard) | BC-2.20.016 PC-3 | `test_BC_2_20_016_no_iso_on_tcp_flow_state_type_exists` | `AC-010-011-module-boundary.gif/.webm` | PASS | +| AC-186-012 | `on_flow_close` removes `S7commFlowState` and discards carry bytes with no finding; double-close (or unknown flow_key) is a no-op | BC-2.21.003 PC-1..4 | `test_s7comm_on_flow_close_removes_state_discards_carry`, `test_BC_2_21_003_double_close_same_flow_key_is_idempotent_no_op` | `AC-012-flow-close.gif/.webm` | PASS | + +**All 12 acceptance criteria (AC-186-001..012) are covered by at least one recorded +artifact.** + +--- + +## VP-050 Proptest Obligation + +| Harness | Property | Test | Artifact | +|---------|----------|------|----------| +| `proptest_vp050_walk_first_residual_bound` | Carry stays `<= MAX_S7_ISO_ON_TCP_CARRY_BYTES` across randomized delivery patterns | `story_186::vp050::proptest_vp050_walk_first_residual_bound` | `VP-050-proptests.gif/.webm` | +| `proptest_vp050_direction_isolation` | `carry_c2s` only ever contains C2S-routed bytes, `carry_s2c` only ever contains S2C-routed bytes | `story_186::vp050::proptest_vp050_direction_isolation` | `VP-050-proptests.gif/.webm` | +| `proptest_vp050_resync_one_byte_advance` | Resync never advances by more than 1 byte per iteration, across randomized garbage-length inputs | `story_186::vp050::proptest_vp050_resync_one_byte_advance` | `VP-050-proptests.gif/.webm` | + +All 3 proptest harnesses pass green (default proptest case count). Full walk-first +equivalence property (splitting a byte sequence into `carry + incoming` yields the +identical result as running the walk once on the concatenated bytes) is deferred to +STORY-194 per the story's own VP-050 obligation note. + +--- + +## Defense-in-Depth Reclassification Note (AC-186-004/005/006) + +Per BC-2.20.014 v1.1 (STORY-186 adversarial gate F-02/F-03, two independent passes, +human ruling 2026-09-07, Option B — Defense-in-Depth): `residual.len() > 65,535` is +provably unreachable via the real `on_data` data path under the current BC-2.20.013 +walk-first + BC-2.20.015 1-byte-resync design, because the TPKT `length` field is +u16-capped — the directional carry is bounded `≤ 65,534` bytes by construction for both +conformant and adversarial input. Consequently: + +- **AC-186-004** (the at-bound case, `residual.len() == 65,535`) is **LIVE** — + reachable via real `on_data` traffic — and is recorded exercising the actual + `on_data` walk-first path (`test_BC_2_20_014_at_bound_residual_no_overflow`). +- **AC-186-005/006** (the over-bound guard mechanics — clear-not-truncate, resync, + one-T0814-per-direction, per-direction dedup independence) are recorded as + **SYNTHETIC**: the tests directly construct/inject an oversized `carry_c2s`/`carry_s2c` + on `S7commFlowState`, bypassing the normal `on_data` walk-first/resync path entirely. + These remain the binding specification for the guard's behavior *if* it is ever + reached (structural defense-in-depth against a future design regression), but are not + scenarios exercised by feeding bytes through `on_data` today. +- The **positive on_data-driven unreachability proof** + (`test_BC_2_20_014_overflow_unreachable_via_on_data`) is the counterpart assertion: + a real 200,000-byte non-anchored garbage flood fed through `on_data` emits **no** + T0814 for either direction and keeps carry bounded `≤ 65,534` at every observation + point — confirming the guard's precondition is not reached by real traffic. + +All three recordings for this group are captured together in +`AC-004-006-defense-in-depth.gif/.webm`, with the comment line in the recording itself +distinguishing the LIVE case from the SYNTHETIC cases. + +--- + +## Recording Method + +This is a pure-core/effectful-shell library story (no CLI binary registered yet — SS-21 +`S7commAnalyzer` dispatch wiring to the CLI is STORY-193's obligation per ADR-014). +Per the demo-recording skill's library/test-harness mode, evidence is captured as VHS +terminal recordings of `cargo test --test s7comm_analyzer_tests`, filtered per behavior +group and piped through `grep` to show only the relevant `story_186::` test lines and +the `test result:` summary line (this also avoids the `Running tests/... (/target/debug/deps/...)` line that `cargo test` otherwise prints, which would +leak an absolute local filesystem path into committed evidence — see the Path-Scrub +Gate section below). + +Seven recordings were produced, one per behavior group named in the task plus one +top-level full-suite run: + +| Artifact | Behavior group | ACs covered | +|----------|----------------|-------------| +| `AC-001-003-carry-reassembly.gif/.webm` | Walk-first carry-buffer reassembly, adversarial-burst anti-evasion, split-frame reassembly (BC-2.20.013) | AC-186-001, 002, 003 | +| `AC-004-006-defense-in-depth.gif/.webm` | Carry bound + defense-in-depth overflow guard (live at-bound + synthetic guard mechanics + positive unreachability) (BC-2.20.014) | AC-186-004, 005, 006 | +| `AC-007-009-resync.gif/.webm` | 1-byte resync, never 2; shared implementation; termination (BC-2.20.015) | AC-186-007, 008, 009 | +| `AC-010-011-module-boundary.gif/.webm` | Frozen SS-20/SS-21 module boundary static regression guards (BC-2.20.016) | AC-186-010, 011 | +| `AC-012-flow-close.gif/.webm` | Flow-close teardown + double-close idempotency (BC-2.21.003) | AC-186-012 | +| `VP-050-proptests.gif/.webm` | VP-050 proptest obligation (3 harnesses) | VP-050 | +| `AC-ALL-18-green.gif/.webm` | Full suite, all 18 tests | All 12 ACs + VP-050 | + +VHS recording settings: `FontFamily "Menlo"`, `Theme "Dracula"`, `Shell "bash"`. No +absolute filesystem path or custom shell prompt is ever typed into any recording — VHS's +own default minimal `>` prompt is used throughout, and every `cargo test` invocation is +piped through `grep` to strip the `Running tests/...` line that would otherwise echo the +worktree's absolute path. + +--- + +## Artifact List + +| File | AC / VP Coverage | +|------|-------------------| +| `AC-001-003-carry-reassembly.gif` | AC-186-001, AC-186-002, AC-186-003 | +| `AC-001-003-carry-reassembly.webm` | AC-186-001, AC-186-002, AC-186-003 | +| `AC-001-003-carry-reassembly.tape` | VHS source for the above | +| `AC-004-006-defense-in-depth.gif` | AC-186-004 (live), AC-186-005 (synthetic + positive unreachability), AC-186-006 (synthetic) | +| `AC-004-006-defense-in-depth.webm` | AC-186-004, AC-186-005, AC-186-006 | +| `AC-004-006-defense-in-depth.tape` | VHS source for the above | +| `AC-007-009-resync.gif` | AC-186-007, AC-186-008, AC-186-009 | +| `AC-007-009-resync.webm` | AC-186-007, AC-186-008, AC-186-009 | +| `AC-007-009-resync.tape` | VHS source for the above | +| `AC-010-011-module-boundary.gif` | AC-186-010, AC-186-011 | +| `AC-010-011-module-boundary.webm` | AC-186-010, AC-186-011 | +| `AC-010-011-module-boundary.tape` | VHS source for the above | +| `AC-012-flow-close.gif` | AC-186-012 | +| `AC-012-flow-close.webm` | AC-186-012 | +| `AC-012-flow-close.tape` | VHS source for the above | +| `VP-050-proptests.gif` | VP-050 (3 proptest harnesses) | +| `VP-050-proptests.webm` | VP-050 | +| `VP-050-proptests.tape` | VHS source for the above | +| `AC-ALL-18-green.gif` | All 12 ACs + VP-050 (top-level, full suite) | +| `AC-ALL-18-green.webm` | All 12 ACs + VP-050 | +| `AC-ALL-18-green.tape` | VHS source for the above | +| `evidence-report.md` | Index (this file) | + +--- + +## Demo-Evidence Path-Scrub Gate (PG-W70-DEMO-SCRUB) + +Gate defined in: `.factory/maintenance/demo-evidence-scrub-gate.md` + +Gate command run from the repo root before commit, exactly as specified in +`.factory/maintenance/demo-evidence-scrub-gate.md`: a recursive extended-regex search +under this directory for absolute macOS/Linux home-directory path prefixes and +tilde-form home references. The literal pattern is intentionally not reproduced in this +report, since the report itself lives under the directory the gate scans and would +otherwise self-match. + +Result: **zero content matches** across all `.tape` script sources and this +`evidence-report.md` file. No absolute host path or tilde-form home reference is present +in any text file in this directory. + +Note on the binary `.gif`/`.webm` recordings: every `cargo test` invocation captured in +these recordings is piped through `grep -E 'story_186::...|test result:'`, which +deliberately filters out the `Running tests/... (/target/debug/deps/...)` line +`cargo test` prints by default — this is the one line in raw `cargo test` output that +would otherwise echo an absolute local filesystem path. No custom shell prompt (which +could echo a working-directory path) was configured in any tape; VHS's own default +minimal `>` prompt is used throughout instead. + +Gate status: **PASSED** (2026-09-07). diff --git a/src/analyzer/mod.rs b/src/analyzer/mod.rs index 9c4e9b19..f8339b83 100644 --- a/src/analyzer/mod.rs +++ b/src/analyzer/mod.rs @@ -19,6 +19,7 @@ pub mod http; pub mod iec104; pub mod iso_on_tcp; pub mod modbus; +pub mod s7comm; pub mod tls; use std::collections::BTreeMap; diff --git a/src/analyzer/s7comm.rs b/src/analyzer/s7comm.rs new file mode 100644 index 00000000..5f28e370 --- /dev/null +++ b/src/analyzer/s7comm.rs @@ -0,0 +1,321 @@ +//! S7comm PDU analyzer — the SS-21 effectful shell built on SS-20's stateless +//! ISO-on-TCP (TPKT/COTP) parsing library. +//! +//! Subsystem SS-21, CAP-21 — `analyzer/s7comm.rs` (created for the first time in +//! STORY-186). +//! +//! ## Architecture (ADR-014 Decisions 1, 2, 8) +//! +//! - **ADR-014 Decision 1**: `src/analyzer/iso_on_tcp.rs` (SS-20) is deliberately +//! stateless — it exports pure free functions only (`parse_tpkt_header`, +//! `parse_cotp_header`) and owns no per-flow state of its own. The directional +//! TPKT/COTP carry buffers required for reassembly live here, on +//! [`S7commFlowState`] (SS-21), never on a hypothetical `IsoOnTcpFlowState`. +//! - **ADR-014 Decision 2**: no `DispatchTarget::IsoOnTcp` variant is introduced at +//! any point — SS-20 is a parsing library consumed by [`S7commAnalyzer`], not an +//! independent dispatch target. (`S7commAnalyzer` is not yet registered with the +//! dispatcher in this story — that wiring is STORY-193's scope.) +//! - **ADR-014 Decision 8 (WALK-FIRST-RESIDUAL-BOUND)**: [`MAX_S7_ISO_ON_TCP_CARRY_BYTES`] +//! is derived from the TPKT `length` field's own maximum (`u16::MAX`), not COTP's +//! 254-byte LI maximum. The frame-walk loop in [`S7commAnalyzer::on_data`] runs +//! unconditionally on `carry ++ incoming_data`; the byte bound applies only to the +//! leftover partial-frame residual stashed back into carry. No aggregate +//! `carry.len() + incoming_data.len()` pre-check may exist anywhere (anti-evasion, +//! mirrors IEC-104 F-172-001 / DNP3 F-B-002). +//! +//! ## Scope of this story (STORY-186) +//! +//! This story proves TPKT/COTP frame extraction, carry-buffer reassembly, and +//! 1-byte resync only. Protocol-specific dispatch on the extracted +//! `CotpHeader::protocol_id` (the four-way protocol-ID dispatch contract) is +//! **not** built here — that is STORY-187's scope. `on_data`'s frame-walk loop +//! dispatches each extracted frame to `iso_on_tcp::parse_cotp_header` and then stops; +//! it does not yet interpret `protocol_id`. +//! +//! ## Behavioral contracts +//! - BC-2.20.013: TPKT frames spanning TCP segment boundaries are reassembled via +//! directional carry buffers using walk-first, residual-bound semantics. +//! - BC-2.20.014: carry buffer bounded at `MAX_S7_ISO_ON_TCP_CARRY_BYTES = 65,535`; +//! overflow triggers clear-and-resync with one T0814 per direction. +//! - BC-2.20.015: resync anchor advances exactly 1 byte per iteration on a bad TPKT +//! version byte (never 2). +//! - BC-2.20.016: frozen `iso_on_tcp.rs` module boundary — verified by this module's +//! consumer relationship (SS-21 imports SS-20's pure functions; SS-20 gains no +//! knowledge of SS-21). +//! - BC-2.21.003: `on_flow_close` removes `S7commFlowState` and discards all carry +//! bytes; no finding is emitted for a flow closing with non-empty carry buffers. + +use std::collections::HashMap; + +use crate::analyzer::iso_on_tcp; +use crate::findings::Finding; +use crate::reassembly::flow::FlowKey; +use crate::reassembly::handler::Direction; + +// --------------------------------------------------------------------------- +// Constants +// --------------------------------------------------------------------------- + +/// Maximum bytes retained in a single direction's carry buffer between `on_data` +/// calls before the residual is treated as adversarial overflow (BC-2.20.014). +/// +/// Derived exactly from the TPKT `length` field's own maximum representable value +/// (`u16::MAX`, RFC 1006 §6) — not from COTP's single-byte Length Indicator (max +/// 254). The overflow comparison is strict `>`, never `>=` (BC-2.20.014 Edge Case +/// EC-001 / Invariant 1): a residual of exactly 65,535 bytes from a still-incomplete, +/// conformant `length = 65,535` frame is legitimate, not overflow. +pub const MAX_S7_ISO_ON_TCP_CARRY_BYTES: usize = 65_535; + +// --------------------------------------------------------------------------- +// Per-flow state +// --------------------------------------------------------------------------- + +/// Minimal per-flow state for [`S7commAnalyzer`] — carry-buffer fields only. +/// +/// STORY-187 extends this struct with the classification/dedup fields its own +/// scope requires. Per BC-2.20.016 postcondition 3, these carry buffers live here +/// (SS-21) and nowhere else — no `IsoOnTcpFlowState` type exists anywhere in the +/// tree, and `carry_c2s`/`carry_s2c` are never merged into a single shared buffer +/// (directional isolation, BC-2.20.013 invariant 3). +#[derive(Debug, Clone, Default)] +pub struct S7commFlowState { + /// Directional carry buffer for client-to-server bytes not yet resolved into a + /// complete TPKT frame (BC-2.20.013). + pub carry_c2s: Vec, + /// Directional carry buffer for server-to-client bytes not yet resolved into a + /// complete TPKT frame (BC-2.20.013). + pub carry_s2c: Vec, + /// Set once a carry-buffer overflow (BC-2.20.014) has been reported for the + /// client-to-server direction on this flow, so repeated overflow events in this + /// direction do not each emit a new T0814 finding (BC-2.20.014 postcondition 3). + pub carry_overflow_reported_c2s: bool, + /// Set once a carry-buffer overflow (BC-2.20.014) has been reported for the + /// server-to-client direction on this flow. Independent of + /// `carry_overflow_reported_c2s` (BC-2.20.014 edge case EC-005). + pub carry_overflow_reported_s2c: bool, +} + +// --------------------------------------------------------------------------- +// Analyzer +// --------------------------------------------------------------------------- + +/// S7comm (SS-21) effectful shell: owns per-flow state and drives the TPKT/COTP +/// frame-walk loop built on SS-20's pure parse functions. +/// +/// Not yet registered with the dispatcher (`DispatchTarget::S7comm` wiring is +/// STORY-193's scope) — this story creates the analyzer and proves extraction, +/// carry management, and resync in isolation. +#[derive(Debug, Default)] +pub struct S7commAnalyzer { + /// Per-flow state, keyed by the canonical [`FlowKey`]. + pub flows: HashMap, + /// Findings accumulated across all flows processed by this analyzer (e.g. the + /// T0814 carry-overflow finding, BC-2.20.014 postcondition 3). + pub findings: Vec, +} + +impl S7commAnalyzer { + /// Construct a new, empty `S7commAnalyzer`. + pub fn new() -> Self { + Self { + flows: HashMap::new(), + findings: Vec::new(), + } + } + + /// Process a chunk of reassembled TCP stream data for `flow_key`, in `direction`. + /// + /// Implements the BC-2.20.013 walk-first, residual-bound frame-walk loop: + /// + /// 1. Overflow check at entry on the directional carry (before appending the new + /// delivery or walking) per BC-2.20.014's walk-first-residual-bound semantics — + /// never an aggregate `carry.len() + data.len()` pre-check (BC-2.20.013 + /// postcondition 2, invariant 1). + /// 2. `working = carry[direction] ++ data`; repeatedly call + /// `iso_on_tcp::parse_tpkt_header(&working[cursor..])`: a complete frame + /// (`Some(header)` and enough trailing bytes) is extracted and dispatched to + /// `iso_on_tcp::parse_cotp_header`, `cursor` advances by `header.length`, and + /// the loop continues; a declared-but-incomplete frame or a `None` result + /// breaks the loop. + /// 3. On a bad-version-byte reject (or immediately after a carry-overflow clear), + /// the shared 1-byte resync sub-routine (BC-2.20.015; see + /// [`Self::resync_one_byte`]) advances the cursor and retries. + /// 4. Whatever remains after the loop terminates is stashed to `carry[direction]`. + /// + /// This story's dispatch on `CotpHeader::protocol_id` is a no-op placeholder — + /// classification lands in STORY-187. + pub fn on_data(&mut self, flow_key: FlowKey, data: &[u8], ts: u32, direction: Direction) { + use crate::findings::{Confidence, ThreatCategory, Verdict}; + + // Collect frame-walk findings locally to avoid a borrow conflict between the + // per-flow state entry (below) and `self.findings`. + let mut local_findings: Vec = Vec::new(); + + { + let state = self.flows.entry(flow_key).or_default(); + + // BC-2.20.014 precondition 2 / postconditions 1-4: overflow check on the + // directional carry ALONE, before the current delivery is appended and the + // walk begins (walk-first, residual-bound semantics — BC-2.20.013 + // postcondition 2, invariant 1: no aggregate carry+delivery pre-check). + // + // DEFENSE-IN-DEPTH, NOT LIVE DETECTION (human ruling, Option B, + // 2026-09-07): under the current walk-first + 1-byte-resync + u16 + // length-cap design, this branch is unreachable via `on_data`. The + // frame-walk loop below stashes at most a declared-but-incomplete TPKT + // frame to carry, and a TPKT `length` field is a `u16` (max 65,535 — + // `MAX_S7_ISO_ON_TCP_CARRY_BYTES`), so the residual can equal but never + // exceed the bound; a bad-version-byte reject resyncs 1 byte at a time + // rather than accumulating carry. BC-2.20.014 v1.1 formalizes this as + // Invariant 5: the directional carry is provably `<= 65,534` bytes + // on entry to `on_data` (strictly less than `MAX_S7_ISO_ON_TCP_CARRY_BYTES`, + // since a carry of exactly 65,535 would itself have been a complete, + // dispatchable frame on the walk that produced it), so `carry.len() > + // MAX_S7_ISO_ON_TCP_CARRY_BYTES` never evaluates true by construction. + // + // The check — and its placement at call-entry on the directional carry, + // reconciled as correct/equivalent per BC-2.20.014 v1.1 Invariant 5 — is + // retained anyway, guarding only against a *future* design regression + // (e.g. a change that stashes more than one incomplete frame's worth of + // bytes to carry, or relaxes the resync step size). It is intentionally + // not deleted: removing it would silently drop the safety net the next + // time this module's invariants change. Do not read `local_findings` + // ever containing a T0814 in this story's test suite as evidence the + // branch is live; it is not exercised by `on_data` today. + { + let (carry, reported) = if direction == Direction::ClientToServer { + (&mut state.carry_c2s, &mut state.carry_overflow_reported_c2s) + } else { + (&mut state.carry_s2c, &mut state.carry_overflow_reported_s2c) + }; + if carry.len() > MAX_S7_ISO_ON_TCP_CARRY_BYTES { + // Clear, not truncate (BC-2.20.014 invariant 2) — the oversized + // residual has no reliable frame boundary to preserve. + carry.clear(); + if !*reported { + // T0814-emission branch — DEFENSE-IN-DEPTH, NOT LIVE DETECTION + // (see the enclosing overflow-check comment above and + // BC-2.20.014 v1.1 Invariant 5): unreachable via `on_data` + // under the current walk-first/1-byte-resync/u16-length-cap + // design, retained only against a future design regression. + // The `chrono::DateTime` timestamp conversion is deferred to + // this rare branch (rather than computed unconditionally at + // the top of `on_data`) since it is otherwise-unreachable and + // its only consumer is this `Finding`. + *reported = true; + let timestamp = chrono::DateTime::from_timestamp(ts as i64, 0); + local_findings.push(Finding { + category: ThreatCategory::Anomaly, + verdict: Verdict::Possible, + confidence: Confidence::Medium, + summary: format!( + "S7comm/ISO-on-TCP directional carry residual overflow: carry \ + buffer exceeded MAX_S7_ISO_ON_TCP_CARRY_BYTES={MAX_S7_ISO_ON_TCP_CARRY_BYTES} \ + — adversarial or non-conformant byte sequence; carry cleared \ + and the walk resyncs on this delivery (T0814; BC-2.20.014)" + ), + evidence: vec![format!( + "carry overflow (>{MAX_S7_ISO_ON_TCP_CARRY_BYTES}); carry cleared" + )], + mitre_techniques: vec!["T0814".to_string()], + source_ip: None, + timestamp, + direction: Some(direction), + }); + } + // Carry is now cleared; the walk proceeds on the delivery alone + // (fresh-start resync, not a permanent desync latch — BC-2.20.014 + // postcondition 2). + } + } + + // BC-2.20.013 precondition 3: working = carry[direction] ++ incoming_data. + let mut working: Vec = if direction == Direction::ClientToServer { + std::mem::take(&mut state.carry_c2s) + } else { + std::mem::take(&mut state.carry_s2c) + }; + working.extend_from_slice(data); + + // Frame-walk loop (BC-2.20.013 postcondition 1). Runs unconditionally on + // the full working buffer — no aggregate byte-count bound is ever applied + // here; only the leftover residual stashed back to carry is bounded + // (BC-2.20.014), checked above at call entry. + let mut cursor = 0usize; + loop { + if working.len() - cursor < 4 { + // Fewer than 4 bytes remain: cannot even attempt a TPKT header + // read. Stash the remainder to carry below (BC-2.20.015 + // postcondition 3(b)). + break; + } + match iso_on_tcp::parse_tpkt_header(&working[cursor..]) { + Some(header) => { + let total = header.length as usize; + if working.len() - cursor >= total { + // Complete TPKT frame: dispatch to parse_cotp_header and + // advance past it (BC-2.20.013 postcondition 1a). This + // story's protocol_id dispatch is a no-op placeholder — + // classification lands in STORY-187. + let frame = &working[cursor..cursor + total]; + let _ = iso_on_tcp::parse_cotp_header(&frame[4..]); + cursor += total; + } else { + // Declared-but-incomplete: stash the entire partial frame + // (including its parsed header) to carry (BC-2.20.013 + // postcondition 1b). + break; + } + } + None => { + // Bad version byte (or a rejected length field, EC-004): resync + // via the shared 1-byte-advance sub-routine (BC-2.20.015), + // reused verbatim whether reached from an ordinary mid-stream + // reject or the post-carry-overflow fresh-start walk above + // (BC-2.20.015 invariant 3 / AC-186-008 — there is exactly one + // resync implementation). + cursor = Self::resync_one_byte(&working, cursor); + } + } + } + + let remainder = working[cursor..].to_vec(); + if direction == Direction::ClientToServer { + state.carry_c2s = remainder; + } else { + state.carry_s2c = remainder; + } + } + + self.findings.extend(local_findings); + } + + /// Remove `flow_key`'s [`S7commFlowState`], discarding any carry bytes with no + /// finding emitted (BC-2.21.003). A no-op if no state exists for `flow_key`. + pub fn on_flow_close(&mut self, flow_key: FlowKey) { + self.flows.remove(&flow_key); + } + + /// Shared 1-byte resync sub-routine (BC-2.20.015). + /// + /// Reused verbatim for both an ordinary bad-version-byte reject encountered + /// mid-stream and the post-carry-overflow resync (BC-2.20.014) — there is exactly + /// one resync implementation, not two (BC-2.20.015 invariant 3 / AC-186-008). + /// + /// Advances `cursor` by exactly 1 byte per iteration (never 2, BC-2.20.015 + /// invariant 1) until either `iso_on_tcp::parse_tpkt_header` succeeds at the new + /// offset, or fewer than 4 bytes remain (BC-2.20.015 postcondition 3(b)), at which + /// point the caller stashes the remainder to carry per the ordinary + /// incomplete-frame path (BC-2.20.013). + /// + /// Returns the new cursor position. + fn resync_one_byte(working: &[u8], cursor: usize) -> usize { + let mut cursor = cursor; + while working.len() - cursor >= 4 + && iso_on_tcp::parse_tpkt_header(&working[cursor..]).is_none() + { + cursor += 1; + } + cursor + } +} diff --git a/tests/s7comm_analyzer_tests.proptest-regressions b/tests/s7comm_analyzer_tests.proptest-regressions new file mode 100644 index 00000000..340a0881 --- /dev/null +++ b/tests/s7comm_analyzer_tests.proptest-regressions @@ -0,0 +1,7 @@ +# Seeds for failure cases proptest has generated in the past. It is +# automatically read and these particular cases re-run before any +# novel cases are generated. +# +# It is recommended to check this file in to source control so that +# everyone who runs the test benefits from these saved cases. +cc 25125c4164a36b5c294faa80825b197886150b4c8213df65a11d8f7c28bb71fe # shrinks to c2s_data = [], s2c_data = [] diff --git a/tests/s7comm_analyzer_tests.rs b/tests/s7comm_analyzer_tests.rs new file mode 100644 index 00000000..fec513c2 --- /dev/null +++ b/tests/s7comm_analyzer_tests.rs @@ -0,0 +1,1108 @@ +//! Tests for STORY-186: S7comm ISO-on-TCP Carry-Buffer Reassembly, Walk-First Frame +//! Extraction, Resync, and the Frozen SS-20/SS-21 Module Boundary. +//! +//! Covers BC-2.20.013, BC-2.20.014, BC-2.20.015, BC-2.20.016, BC-2.21.003, and the +//! VP-050 proptest obligation (walk-first residual bound, direction isolation, 1-byte +//! resync advance). +//! +//! ## Contract coverage +//! - BC-2.20.013: TPKT frames spanning TCP segment boundaries are reassembled via +//! directional carry buffers using walk-first, residual-bound semantics. No aggregate +//! `carry.len() + incoming.len()` pre-check exists anywhere. +//! - BC-2.20.014: carry buffer bounded at `MAX_S7_ISO_ON_TCP_CARRY_BYTES = 65,535`; +//! overflow triggers clear-and-resync with exactly one T0814 per direction, guarded by +//! a per-direction dedup flag. +//! - BC-2.20.015: resync anchor advances exactly 1 byte per iteration on a bad TPKT +//! version byte (never 2); the same resync sub-routine is reused for both an ordinary +//! bad-version-byte reject and a post-carry-overflow resync. +//! - BC-2.20.016: frozen `iso_on_tcp.rs` module boundary — no `impl StreamAnalyzer`, no +//! `IsoOnTcpFlowState` type anywhere in the tree. +//! - BC-2.21.003: `on_flow_close` removes `S7commFlowState` and discards all carry bytes +//! with no finding emitted; a no-op for an unknown `flow_key`. +//! +//! ## Test naming convention +//! Tests follow `test_BC_S_SS_NNN_xxx()` for BC-traceable tests. The non_snake_case +//! lint fires on uppercase BC IDs — suppressed intentionally, mirroring the +//! STORY-184/185/167-173 precedent in `iso_on_tcp_tests.rs` / `iec104_analyzer_tests.rs`. +//! +//! ## Provenance +//! Authored Red-first as TDD stubs (STORY-186 `tdd_mode: strict`) against the `todo!()` +//! bodies of `S7commAnalyzer::on_data`, `S7commAnalyzer::on_flow_close`, and +//! `S7commAnalyzer::resync_one_byte` in `src/analyzer/s7comm.rs`. The `todo!()` bodies +//! were replaced by the STORY-186 implementation (commit 34e9b435); these tests are +//! now GREEN against the landed STORY-186 implementation. +//! The two BC-2.20.016 static regression-guard tests are architectural/structural checks +//! (per BC-2.20.016's own "Verification Properties" note: "verified by code-review +//! inspection and the regression-guard greps ... not by a runtime proof harness") and are +//! expected to be green immediately, since the frozen module boundary is already +//! satisfied by the current `iso_on_tcp.rs` / `s7comm.rs` stub — they exist here as +//! permanent drift guards, not as Red Gate behavioral tests. +//! +//! Canonical test vectors from the BCs are used verbatim (DF-CANONICAL-FRAME-HOLDOUT-001) +//! where given; one clarification was applied per "BC is the source of truth" guidance +//! (see the `test_BC_2_20_015_resync_advances_exactly_one_byte` doc comment below) where +//! the story's inline AC-186-007 example byte sequence disagreed with BC-2.20.015's own +//! canonical vector table. + +#![allow(non_snake_case)] + +// Per DF-TEST-NAMESPACE-001: all STORY-186 tests are grouped inside a dedicated +// `mod story_186` wrapper to prevent test-function name collisions with other stories' +// BC-prefixed names. +mod story_186 { + use wirerust::analyzer::s7comm::{MAX_S7_ISO_ON_TCP_CARRY_BYTES, S7commAnalyzer}; + use wirerust::findings::{Confidence, ThreatCategory, Verdict}; + use wirerust::reassembly::flow::FlowKey; + use wirerust::reassembly::handler::Direction; + + /// Canonical default flow key for these tests: an arbitrary client port against + /// TCP/102, the registered ISO-on-TCP port (ADR-014). + fn flow_key_default() -> FlowKey { + FlowKey::new( + "127.0.0.1".parse().unwrap(), + 1234, + "127.0.0.2".parse().unwrap(), + 102, + ) + } + + /// A minimal, complete 7-byte TPKT/COTP CR (Connect Request) frame: + /// TPKT header `[0x03, 0x00, 0x00, 0x07]` (version=3, length=7, the RFC 1006 §6 + /// minimum) followed by a 3-byte COTP payload `[LI=1, code=0xE0 (CR), pad]`. + /// `payload_offset = 1 + LI = 2 <= tpkt_payload.len() = 3`, so `parse_cotp_header` + /// succeeds — content beyond that is irrelevant to this story's no-op dispatch. + fn cr_frame_7() -> Vec { + vec![0x03, 0x00, 0x00, 0x07, 0x01, 0xE0, 0x00] + } + + /// A complete TPKT frame declaring the maximum representable length + /// (`u16::MAX = 65,535`): 4-byte header + 65,531 zero payload bytes. + fn max_length_frame() -> Vec { + let mut frame = vec![0u8; 65_535]; + frame[0] = 0x03; + frame[1] = 0x00; + frame[2] = 0xFF; + frame[3] = 0xFF; + frame + } + + // ========================================================================= + // BC-2.20.013: TPKT frames spanning TCP segment boundaries are reassembled via + // directional carry buffers using walk-first, residual-bound semantics. + // ========================================================================= + + /// AC-186-001: the frame-walk loop extracts every complete TPKT frame before any + /// byte-count bound is applied — there is no aggregate + /// `carry[direction].len() + incoming_data.len()` pre-check anywhere. + /// + /// Setup: `carry_c2s` is seeded (directly, simulating a prior `on_data` call) with + /// the first 65,000 bytes of a complete 65,535-byte max-length TPKT frame. The + /// current `on_data` call then delivers the remaining 535 bytes of that frame PLUS + /// a second complete 7-byte CR frame appended immediately after — total aggregate + /// `carry.len() + incoming.len() = 65,000 + 542 = 65,542`, which is *greater* than + /// `MAX_S7_ISO_ON_TCP_CARRY_BYTES = 65,535`. + /// + /// An implementation with an aggregate pre-check (the rejected + /// PRE-CHECK-DISCARD-ALL alternative BC-2.20.013 invariant 1 explicitly forbids) + /// would treat this as overflow: clear the carry, emit a T0814, and never dispatch + /// either frame. The correct WALK-FIRST implementation extracts both complete + /// frames (the 65,535-byte frame, then the 7-byte CR frame) leaving zero residual + /// and zero findings, because the *directional carry alone* (65,000 bytes) never + /// exceeded the bound at the start of this call (BC-2.20.014 precondition 2). + /// + /// Traces: BC-2.20.013 postconditions 1-2, invariant 1; AC-186-001. + #[test] + fn test_BC_2_20_013_walk_first_no_aggregate_precheck() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + let big_frame = max_length_frame(); // 65,535 bytes total + { + let state = analyzer.flows.entry(flow_key.clone()).or_default(); + state.carry_c2s = big_frame[..65_000].to_vec(); + } + // Sanity: 65,000 <= MAX_S7_ISO_ON_TCP_CARRY_BYTES (65,535), so the seeded carry + // alone must not itself be an overflow. + + let mut delivery = big_frame[65_000..].to_vec(); // 535 completing bytes + delivery.extend_from_slice(&cr_frame_7()); // + a second complete 7-byte frame + assert_eq!( + 65_000 + delivery.len(), + 65_542, + "aggregate carry+delivery must exceed MAX_S7_ISO_ON_TCP_CARRY_BYTES (65,535) \ + to exercise the anti-aggregate-precheck property" + ); + + analyzer.on_data(flow_key.clone(), &delivery, 0, Direction::ClientToServer); + + assert!( + analyzer.findings.is_empty(), + "walk-first semantics: an aggregate carry+incoming total that exceeds the \ + bound must NOT trigger the overflow reaction when the directional carry \ + ALONE never exceeded it at call entry (BC-2.20.013 postcondition 2, \ + invariant 1; BC-2.20.014 precondition 2) — a wrongly aggregate-pre-checked \ + implementation would emit a T0814 here" + ); + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + state.carry_c2s.is_empty(), + "both complete frames (65,535-byte + 7-byte CR) must be fully extracted by \ + the walk, leaving no residual (BC-2.20.013 postcondition 3) — a wrongly \ + aggregate-pre-checked implementation would clear the carry via the overflow \ + path instead of consuming it via ordinary extraction, or would drop the \ + delivery outright" + ); + } + + /// AC-186-002: an adversarial burst with a complete frame at the head is never + /// dropped, regardless of the size of the trailing garbage that follows it + /// (Ptacek/Newsham-class evasion channel; mirrors IEC-104 F-172-001 / DNP3 F-B-002). + /// + /// One `on_data` call delivers `[complete 7-byte CR frame][60,000 bytes of 0xAA + /// garbage]`. The walk must extract the head frame (cursor advances past its 7 + /// bytes) and then resync through the garbage 1 byte at a time (no `0x03` byte + /// appears anywhere in the 0xAA run). Since the resync loop stops precisely when + /// fewer than 4 bytes remain (BC-2.20.015 postcondition 3(b)), and + /// `60,000 mod 1 == 0` bytes are consumed one at a time from a starting remainder + /// of exactly 60,000, the final residual is deterministically the last 3 garbage + /// bytes. + /// + /// Traces: BC-2.20.013 postcondition 1, invariant 1; AC-186-002. + #[test] + fn test_BC_2_20_013_adversarial_burst_head_frame_not_dropped() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + let mut data = cr_frame_7(); + data.extend(std::iter::repeat_n(0xAAu8, 60_000)); + assert_eq!(data.len(), 60_007); + + analyzer.on_data(flow_key.clone(), &data, 0, Direction::ClientToServer); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.carry_c2s, + vec![0xAAu8; 3], + "the head 7-byte CR frame must be extracted (cursor advances past it, not \ + dropped by any evasion-style aggregate check) and the walk must then \ + resync through all 60,000 trailing garbage bytes down to the deterministic \ + 3-byte remainder (BC-2.20.013 postcondition 1, invariant 1; AC-186-002)" + ); + } + + /// AC-186-003: split-frame reassembly across two `on_data` calls (BC-2.20.013 + /// canonical vector, EC-002). + /// + /// Call 1 delivers `[0x03, 0x00, 0x00, 0x0A]` (a 4-byte TPKT header declaring + /// `length=10`) with no trailing payload bytes — declared-but-incomplete. Call 2 + /// delivers the remaining 6 bytes. After call 1, `carry_c2s` must hold exactly the + /// 4-byte header-only stash. After call 2, `working = carry ++ new_bytes` contains + /// the complete 10-byte frame, which is extracted, leaving `carry_c2s` empty. + /// + /// Traces: BC-2.20.013 postcondition 1 (sub-clause b), edge case EC-002; AC-186-003. + #[test] + fn test_BC_2_20_013_split_frame_across_two_calls() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + let call_1 = [0x03u8, 0x00, 0x00, 0x0A]; + analyzer.on_data(flow_key.clone(), &call_1, 0, Direction::ClientToServer); + { + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.carry_c2s, + call_1.to_vec(), + "call 1: the 4-byte header-only partial must be stashed verbatim to \ + carry_c2s (declared-but-incomplete; BC-2.20.013 postcondition 1b)" + ); + } + assert!( + analyzer.findings.is_empty(), + "call 1: a declared-but-incomplete stash must not emit any finding" + ); + + let call_2 = [0x01u8, 0xE0, 0x00, 0x00, 0x00, 0x00]; // remaining 6 bytes -> total 10 + analyzer.on_data(flow_key.clone(), &call_2, 0, Direction::ClientToServer); + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + state.carry_c2s.is_empty(), + "call 2: carry ++ new_bytes forms the complete 10-byte frame; it must be \ + extracted, leaving carry_c2s empty (BC-2.20.013 EC-002; AC-186-003)" + ); + assert!( + analyzer.findings.is_empty(), + "call 2: ordinary split-frame completion must not emit any finding" + ); + } + + // ========================================================================= + // BC-2.20.014: carry buffer bounded at MAX_S7_ISO_ON_TCP_CARRY_BYTES=65,535; + // overflow triggers clear-and-resync with one T0814 per direction. + // ========================================================================= + + /// AC-186-004: a residual at exactly the 65,535-byte bound is legitimate, not + /// overflow — the comparison is strict `>`, never `>=` (BC-2.20.014 edge case + /// EC-001, invariant 1). + /// + /// `carry_c2s` is seeded with a complete, conformant 65,535-byte max-length TPKT + /// frame (the largest frame the TPKT `length` field can ever represent). `on_data` + /// is then called with an empty delivery: since `65,535 > 65,535` is false, the + /// overflow check on entry does not fire, so the walk proceeds and extracts the + /// frame in full — `carry_c2s` ends this call EMPTY, not retained unchanged. What + /// the test actually verifies is the strict-`>` at-bound boundary itself: a + /// complete, at-bound input must never trip the overflow reaction (clear + resync + /// + T0814), which it confirms via empty findings and an unset overflow dedup flag. + /// + /// Traces: BC-2.20.014 precondition 2, invariant 1, edge case EC-001; AC-186-004. + #[test] + fn test_BC_2_20_014_at_bound_residual_no_overflow() { + assert_eq!( + MAX_S7_ISO_ON_TCP_CARRY_BYTES, 65_535, + "MAX_S7_ISO_ON_TCP_CARRY_BYTES must be exactly u16::MAX (ADR-014 Decision 8)" + ); + + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + { + let state = analyzer.flows.entry(flow_key.clone()).or_default(); + state.carry_c2s = max_length_frame(); // exactly 65,535 bytes + } + + analyzer.on_data(flow_key.clone(), &[], 0, Direction::ClientToServer); + + assert!( + analyzer.findings.is_empty(), + "a residual of exactly 65,535 bytes must NOT trigger the overflow reaction \ + (comparison is strict '>', not '>='; BC-2.20.014 invariant 1, EC-001)" + ); + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + !state.carry_overflow_reported_c2s, + "the carry-overflow dedup flag must remain unset when the bound is merely \ + met, not exceeded (BC-2.20.014 EC-001)" + ); + } + + /// AC-186-005: carry overflow clears the direction's carry, resyncs, and emits + /// exactly one T0814 (`ThreatCategory::Anomaly`, `Verdict::Possible`, + /// `Confidence::Medium`) for this direction. + /// + /// `carry_c2s` is seeded (simulating prior accumulation) with 65,536 bytes of 0xAA + /// garbage — one byte over the bound. This `on_data` call's own delivery is a + /// complete 7-byte CR frame. Per BC-2.20.014 precondition 2, the overflow check + /// examines the residual carry ALONE, before the current delivery is appended: the + /// 65,536-byte garbage carry is cleared and exactly one T0814 finding is emitted; + /// the walk then proceeds on the (now-empty-carry ++ CR-frame) working buffer, + /// extracting the CR frame normally (BC-2.20.014 postcondition 2's "fresh-start + /// resync, not a permanent desync latch"). + /// + /// **BC-2.20.014 v1.1 (defense-in-depth reclassification, STORY-186 adversarial + /// gate F-02/F-03, human ruling 2026-09-07 — Option B):** this test exercises the + /// guard's mechanics via SYNTHETIC direct flow-state field injection + /// (`state.carry_c2s = vec![...]` above), **not** via `on_data` — the + /// `> 65,535`-byte residual seeded here is a state that is NOT reachable through + /// the real `on_data` data path. Under the current BC-2.20.013 walk-first + + /// BC-2.20.015 1-byte-resync design, the directional carry is bounded `<= 65,534` + /// bytes by construction for all traffic (BC-2.20.014 v1.1 Invariant 1) — this is + /// intentional and specified: the guard is retained as a structural + /// defense-in-depth safety net against a future design regression, and its + /// mechanics (clear-not-truncate, resync, one-T0814-per-direction) remain the + /// binding specification IF the guard is ever reached, which this direct-injection + /// harness exists solely to exercise in isolation (BC-2.20.014 v1.1 Canonical Test + /// Vectors, "over-bound, guard-mechanics (SYNTHETIC ...)"). See + /// `test_BC_2_20_014_overflow_unreachable_via_on_data` below for the positive proof + /// that this precondition is never reached by feeding bytes through `on_data`. + /// + /// Traces: BC-2.20.014 postconditions 1, 3, 4; AC-186-005. + #[test] + fn test_BC_2_20_014_overflow_clear_resync_one_t0814_per_direction() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + { + let state = analyzer.flows.entry(flow_key.clone()).or_default(); + state.carry_c2s = vec![0xAAu8; MAX_S7_ISO_ON_TCP_CARRY_BYTES + 1]; + } + + analyzer.on_data( + flow_key.clone(), + &cr_frame_7(), + 0, + Direction::ClientToServer, + ); + + assert_eq!( + analyzer.findings.len(), + 1, + "exactly one T0814 must be emitted for this direction on overflow \ + (BC-2.20.014 postcondition 3)" + ); + let finding = &analyzer.findings[0]; + assert_eq!( + finding.category, + ThreatCategory::Anomaly, + "T0814 carry-overflow finding must have ThreatCategory::Anomaly \ + (BC-2.20.014 postcondition 3)" + ); + assert_eq!( + finding.verdict, + Verdict::Possible, + "T0814 carry-overflow finding must have Verdict::Possible \ + (BC-2.20.014 postcondition 3)" + ); + assert_eq!( + finding.confidence, + Confidence::Medium, + "T0814 carry-overflow finding must have Confidence::Medium \ + (BC-2.20.014 postcondition 3)" + ); + assert!( + finding.mitre_techniques.iter().any(|t| t == "T0814"), + "the carry-overflow finding must cite T0814 (BC-2.20.014 postcondition 3)" + ); + assert_eq!( + finding.direction, + Some(Direction::ClientToServer), + "the finding must be attributed to the overflowing direction (C2S)" + ); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + state.carry_overflow_reported_c2s, + "carry_overflow_reported_c2s must be set after the first overflow \ + (BC-2.20.014 postcondition 3 dedup guard)" + ); + assert!( + !state.carry_overflow_reported_s2c, + "carry_overflow_reported_s2c must remain false — independent per-direction \ + dedup flags (BC-2.20.014 postcondition 4)" + ); + assert!( + state.carry_c2s.is_empty(), + "after the overflow clears carry_c2s, the fresh-start walk on the CR-frame \ + delivery alone must fully extract it, leaving no residual (BC-2.20.014 \ + postcondition 2: 'fresh-start resync, not a permanent desync latch')" + ); + } + + /// AC-186-005: a second overflow event in the same direction on the same flow does + /// not re-emit T0814 (BC-2.20.014 edge case EC-004) — the dedup flag suppresses + /// repeated emission, though the carry is still cleared and resync still occurs + /// each time. + /// + /// **BC-2.20.014 v1.1 (defense-in-depth reclassification):** like + /// `test_BC_2_20_014_overflow_clear_resync_one_t0814_per_direction` above, both + /// overflow events in this test are SYNTHETIC — directly injected onto + /// `state.carry_c2s` — not reachable via the real `on_data` data path. Under the + /// current walk-first (BC-2.20.013) + 1-byte-resync (BC-2.20.015) design the + /// directional carry is bounded `<= 65,534` bytes for all traffic (BC-2.20.014 v1.1 + /// Invariant 1), so this `> 65,535` condition never arises through `on_data`. This + /// is intentional per the reconciled spec (STORY-186 v1.1, human ruling + /// 2026-09-07 — Option B: Defense-in-Depth): the dedup mechanics tested here remain + /// the binding specification for the guard's behavior IF it is ever reached under a + /// future design regression (BC-2.20.014 v1.1 Canonical Test Vectors, "repeated + /// over-bound, same direction (SYNTHETIC ...)"). + /// + /// Traces: BC-2.20.014 postcondition 3 dedup guard, edge case EC-004; AC-186-005. + #[test] + fn test_BC_2_20_014_repeated_overflow_dedup_same_direction() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + // First overflow event. + { + let state = analyzer.flows.entry(flow_key.clone()).or_default(); + state.carry_c2s = vec![0xAAu8; MAX_S7_ISO_ON_TCP_CARRY_BYTES + 1]; + } + analyzer.on_data(flow_key.clone(), &[], 0, Direction::ClientToServer); + assert_eq!( + analyzer.findings.len(), + 1, + "first overflow event must emit exactly one T0814" + ); + { + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!(state.carry_overflow_reported_c2s); + assert!( + state.carry_c2s.is_empty(), + "carry must be cleared on the first overflow (empty delivery leaves \ + nothing to walk afterward)" + ); + } + + // Second overflow event, same direction, same flow. + { + let state = analyzer.flows.entry(flow_key.clone()).or_default(); + state.carry_c2s = vec![0xAAu8; MAX_S7_ISO_ON_TCP_CARRY_BYTES + 1]; + } + analyzer.on_data(flow_key.clone(), &[], 0, Direction::ClientToServer); + + assert_eq!( + analyzer.findings.len(), + 1, + "a second overflow event in the same direction must NOT emit an additional \ + T0814 — the dedup flag suppresses re-emission (BC-2.20.014 EC-004)" + ); + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + state.carry_c2s.is_empty(), + "the carry must still be cleared on the second overflow event even though \ + no new finding is emitted (BC-2.20.014 EC-004: 'carry is still cleared and \ + resync still occurs each time')" + ); + } + + /// AC-186-006: overflow dedup flags are independent per direction (BC-2.20.014 + /// edge case EC-005). A C2S overflow does not suppress a subsequent, independent + /// S2C overflow on the same flow. + /// + /// **BC-2.20.014 v1.1 (defense-in-depth reclassification):** both overflow events + /// in this test (C2S and S2C) are SYNTHETIC — directly injected onto + /// `state.carry_c2s`/`state.carry_s2c` — not reachable via the real `on_data` data + /// path. Under the current walk-first (BC-2.20.013) + 1-byte-resync (BC-2.20.015) + /// design the directional carry is bounded `<= 65,534` bytes for all traffic in + /// both directions (BC-2.20.014 v1.1 Invariant 1), so this `> 65,535` condition + /// never arises through `on_data` in either direction. This is intentional per the + /// reconciled spec (STORY-186 v1.1, human ruling 2026-09-07 — Option B: + /// Defense-in-Depth): the per-direction independence tested here remains the + /// binding specification for the guard's dedup mechanics IF it is ever reached + /// under a future design regression (BC-2.20.014 v1.1 Canonical Test Vectors). + /// + /// Traces: BC-2.20.014 postcondition 4, edge case EC-005; AC-186-006. + #[test] + fn test_BC_2_20_014_overflow_dedup_independent_per_direction() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + // C2S overflow. + { + let state = analyzer.flows.entry(flow_key.clone()).or_default(); + state.carry_c2s = vec![0xAAu8; MAX_S7_ISO_ON_TCP_CARRY_BYTES + 1]; + } + analyzer.on_data(flow_key.clone(), &[], 0, Direction::ClientToServer); + assert_eq!(analyzer.findings.len(), 1); + + // Independent S2C overflow on the same flow. + { + let state = analyzer.flows.entry(flow_key.clone()).or_default(); + state.carry_s2c = vec![0xAAu8; MAX_S7_ISO_ON_TCP_CARRY_BYTES + 1]; + } + analyzer.on_data(flow_key.clone(), &[], 0, Direction::ServerToClient); + + assert_eq!( + analyzer.findings.len(), + 2, + "the S2C overflow must emit its own T0814 independently of the C2S dedup \ + flag having already been set (BC-2.20.014 EC-005)" + ); + let s2c_finding = &analyzer.findings[1]; + assert_eq!( + s2c_finding.direction, + Some(Direction::ServerToClient), + "the second finding must be attributed to the S2C direction" + ); + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + state.carry_overflow_reported_c2s, + "carry_overflow_reported_c2s must remain true from the earlier C2S event" + ); + assert!( + state.carry_overflow_reported_s2c, + "carry_overflow_reported_s2c must now be set after the independent S2C \ + overflow (BC-2.20.014 EC-005)" + ); + } + + /// **NEW (BC-2.20.014 v1.1 / STORY-186 v1.1 AC-186-005 new positive assertion, + /// F-02 closure):** the positive, `on_data`-driven counterpart to the three + /// SYNTHETIC direct-injection tests above. Proves that the overflow precondition + /// (`residual.len() > 65,535`) is never reached when bytes are fed exclusively + /// through the real `on_data` entry point — never touching the `pub carry_c2s` / + /// `carry_s2c` fields directly — closing the adversarial F-02 concern that the + /// SYNTHETIC guard-mechanics tests could mask an unreachable/untested real-traffic + /// path. + /// + /// Two scenarios, both driven through `on_data` only: + /// + /// 1. **Garbage flood, single call:** one `on_data` call delivers 200,000 bytes of + /// non-`0x03`-anchored `0xAA` garbage. The frame-walk's resync sub-routine + /// (BC-2.20.015) drains this 1 byte at a time down to the deterministic 3-byte + /// remainder before the call returns — it never accumulates toward the + /// 65,535-byte bound within a single call. + /// 2. **Garbage flood, split across many calls:** the same total garbage volume is + /// redelivered in four separate 50,000-byte `on_data` calls on the same flow and + /// direction. Because BC-2.20.015's resync drains un-anchored garbage below 4 + /// remaining bytes before *each* call's walk terminates, the ~3-byte remainder + /// from call N is not compounded by call N+1's fresh 50,000 bytes into anything + /// exceeding the bound — garbage never accumulates carry-to-carry across calls + /// (BC-2.20.013 Reconciliation Note). + /// 3. **Dribbled, incomplete max-length frame:** a conformant TPKT frame declaring + /// `length = 65,535` (the maximum representable value) is delivered one byte + /// short of complete (65,534 of its 65,535 bytes), split across many small + /// `on_data` calls (500 bytes at a time) so the legitimate residual grows + /// incrementally, call by call, all the way up to the maximum legitimate + /// single-frame residual (65,534 bytes) without ever exceeding + /// `MAX_S7_ISO_ON_TCP_CARRY_BYTES` (65,535) — the walk-first residual bound + /// (BC-2.20.013 Reconciliation Note) holds at every observation point along the + /// way, not merely at the start and end. + /// + /// After every call in all three scenarios: zero T0814 findings are emitted, and + /// `carry_c2s.len() < MAX_S7_ISO_ON_TCP_CARRY_BYTES` (i.e. `<= 65,534`) holds — + /// the overflow guard's `> 65,535` precondition is never satisfied via the real + /// `on_data` data path (BC-2.20.014 v1.1 Invariant 1 / VP-050 reachability + /// property). + /// + /// Traces: BC-2.20.014 v1.1 Invariant 1, AC-186-005 (new positive assertion). + #[test] + fn test_BC_2_20_014_overflow_unreachable_via_on_data() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + // --- Scenario 1: garbage flood delivered in one on_data call. --- + let flood = vec![0xAAu8; 200_000]; + analyzer.on_data(flow_key.clone(), &flood, 0, Direction::ClientToServer); + + assert!( + analyzer.findings.is_empty(), + "a 200,000-byte non-0x03-anchored garbage flood delivered through on_data \ + alone must never emit a T0814 — the resync sub-routine (BC-2.20.015) \ + drains it to a sub-4-byte remainder within the same call, never \ + approaching the 65,535-byte bound (BC-2.20.014 v1.1 Invariant 1)" + ); + { + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + state.carry_c2s.len() < MAX_S7_ISO_ON_TCP_CARRY_BYTES, + "carry_c2s ({} bytes) must stay < MAX_S7_ISO_ON_TCP_CARRY_BYTES \ + (i.e. <= 65,534) after a single-call garbage flood driven via on_data \ + only", + state.carry_c2s.len() + ); + } + + // --- Scenario 2: the same total garbage volume redelivered across four + // separate on_data calls, to prove garbage never accumulates carry-to-carry + // across calls (BC-2.20.013 Reconciliation Note). --- + for _ in 0..4 { + let chunk = vec![0xAAu8; 50_000]; + analyzer.on_data(flow_key.clone(), &chunk, 0, Direction::ClientToServer); + + assert!( + analyzer.findings.is_empty(), + "no T0814 must ever be emitted while redelivering non-0x03-anchored \ + garbage across multiple on_data calls on the same flow/direction \ + (BC-2.20.014 v1.1 Invariant 1)" + ); + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + state.carry_c2s.len() < MAX_S7_ISO_ON_TCP_CARRY_BYTES, + "carry_c2s ({} bytes) must stay < 65,535 (i.e. <= 65,534) after each \ + dribbled garbage on_data call — garbage never accumulates \ + carry-to-carry across calls", + state.carry_c2s.len() + ); + } + + // --- Scenario 3: a conformant length=65,535 TPKT frame delivered one byte + // short of complete, dribbled across many small on_data calls on a fresh flow, + // so the legitimate residual grows incrementally all the way up to the maximum + // single-frame residual (65,534 bytes) without ever exceeding the bound. --- + let flow_key_2 = FlowKey::new( + "127.0.0.1".parse().unwrap(), + 1235, + "127.0.0.2".parse().unwrap(), + 102, + ); + let mut analyzer_2 = S7commAnalyzer::new(); + let full_frame = max_length_frame(); // 65,535 bytes total (length field = 0xFFFF) + let incomplete = &full_frame[..full_frame.len() - 1]; // 65,534 bytes: one short + assert_eq!(incomplete.len(), MAX_S7_ISO_ON_TCP_CARRY_BYTES - 1); + + for chunk in incomplete.chunks(500) { + analyzer_2.on_data(flow_key_2.clone(), chunk, 0, Direction::ClientToServer); + + assert!( + analyzer_2.findings.is_empty(), + "dribbling a still-incomplete, conformant max-length-frame residual via \ + on_data must never emit a T0814, even as the residual grows \ + incrementally toward the maximum legitimate single-frame size \ + (BC-2.20.014 v1.1 Invariant 1 / EC-002)" + ); + let state = analyzer_2.flows.get(&flow_key_2).unwrap(); + assert!( + state.carry_c2s.len() < MAX_S7_ISO_ON_TCP_CARRY_BYTES, + "carry_c2s ({} bytes) must stay < 65,535 (i.e. <= 65,534) at every \ + observation point while dribbling the incomplete max-length frame via \ + on_data", + state.carry_c2s.len() + ); + } + let final_state = analyzer_2.flows.get(&flow_key_2).unwrap(); + assert_eq!( + final_state.carry_c2s.len(), + MAX_S7_ISO_ON_TCP_CARRY_BYTES - 1, + "after dribbling all 65,534 available bytes of the still-incomplete \ + max-length frame, carry_c2s must hold exactly the maximum legitimate \ + single-frame residual (65,534 bytes) with no overflow ever triggered" + ); + } + + // ========================================================================= + // BC-2.20.015: resync anchor advances exactly 1 byte per iteration on a bad TPKT + // version byte (never 2). + // ========================================================================= + + /// AC-186-007: the resync sub-routine advances exactly 1 byte per iteration, never + /// 2, on a bad TPKT version byte. + /// + /// Canonical vector taken from **BC-2.20.015's own table** (not the story's inline + /// AC-186-007 example, which restates the byte sequence with a typo — `length=0x04` + /// instead of `0x07` — that would make the trailing header fail BC-2.20.003's + /// length-floor check and therefore never be a "valid frame" in the first place; + /// per "the BC is the source of truth", the BC-2.20.015 canonical vector + /// `[0x01, 0x03, 0x00, 0x00, 0x07]` is used here). + /// + /// Bytes: `[0x01, 0x03, 0x00, 0x00, 0x07]` — a spurious `0x01` immediately followed + /// by a valid TPKT header at offset 1 declaring `length=7`. Only 4 bytes are + /// available from offset 1 onward, so the header is declared-but-incomplete (not + /// yet a complete frame) — but the anchor itself must be found. A correct 1-byte + /// resync skips only the single spurious byte and stashes the anchored 4-byte + /// partial header to carry. A buggy 2-byte advance would land on offset 2 (`0x00`, + /// not `0x03`), fail to recognize the header, and never recover it. + /// + /// Traces: BC-2.20.015 postconditions 1-3, invariant 1; AC-186-007. + #[test] + fn test_BC_2_20_015_resync_advances_exactly_one_byte() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + let data = [0x01u8, 0x03, 0x00, 0x00, 0x07]; + analyzer.on_data(flow_key.clone(), &data, 0, Direction::ClientToServer); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.carry_c2s, + vec![0x03u8, 0x00, 0x00, 0x07], + "the 1-byte resync must skip only the single leading spurious byte (0x01) \ + and find + stash the anchored 4-byte partial TPKT header starting at \ + offset 1 (BC-2.20.015 postconditions 1-3); a 2-byte advance would have \ + landed on offset 2 (0x00) and missed the anchor entirely" + ); + assert!( + analyzer.findings.is_empty(), + "an ordinary bad-version-byte resync (not an overflow condition) must not \ + emit any finding" + ); + } + + /// AC-186-008: the resync sub-routine is reused verbatim for both the ordinary + /// bad-version-byte condition and the post-carry-overflow condition — there is + /// exactly one resync implementation, not two (BC-2.20.015 invariant 3). + /// + /// `carry_c2s` is seeded with an oversized (65,536-byte) garbage residual to force + /// the carry-overflow path (BC-2.20.014). This call's own delivery is + /// `[0x01, 0x03, 0x00, 0x00, 0x07]` — the exact same "spurious-byte-then-anchor" + /// pattern used by `test_BC_2_20_015_resync_advances_exactly_one_byte` above, but + /// now reached via the post-overflow code path (carry cleared, walk continues on + /// the delivery alone). If the two conditions invoked different resync logic (e.g. + /// a 2-byte advance for the post-overflow case), the anchor at offset 1 would be + /// missed here even though the ordinary-path test above finds it — demonstrating a + /// second, divergent implementation. Observing byte-for-byte identical resync + /// behavior in both conditions is exactly the behavioral signature of BC-2.20.015 + /// invariant 3's "exactly one resync implementation, not two" claim. + /// + /// Traces: BC-2.20.015 invariant 3; AC-186-008. + #[test] + fn test_BC_2_20_015_single_resync_implementation_shared() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + { + let state = analyzer.flows.entry(flow_key.clone()).or_default(); + state.carry_c2s = vec![0xAAu8; MAX_S7_ISO_ON_TCP_CARRY_BYTES + 1]; + } + + let data = [0x01u8, 0x03, 0x00, 0x00, 0x07]; + analyzer.on_data(flow_key.clone(), &data, 0, Direction::ClientToServer); + + // Exactly one finding — the overflow T0814 itself. The post-overflow resync + // over `data` must not itself emit any additional finding. + assert_eq!( + analyzer.findings.len(), + 1, + "only the carry-overflow T0814 must be emitted; the subsequent resync over \ + `data` is an ordinary (non-overflow) resync and must not itself add a \ + finding" + ); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.carry_c2s, + vec![0x03u8, 0x00, 0x00, 0x07], + "the post-overflow resync must use the exact same exactly-1-byte-advance \ + sub-routine as the ordinary bad-version-byte resync (BC-2.20.015 invariant \ + 3): it must find and stash the anchored 4-byte partial header at offset 1 \ + within `data`, identically to test_BC_2_20_015_resync_advances_exactly_one_byte" + ); + } + + /// AC-186-009: the resync sub-routine always terminates for finite input, even + /// with no valid anchor anywhere in the remaining bytes (BC-2.20.015 invariant 2). + /// + /// 200 bytes of `0xAA` garbage (never `0x03`) contain no valid frame boundary. + /// The resync walk must advance to the end without an infinite loop, and the + /// (now sub-4-byte) remainder is stashed to carry via the ordinary incomplete-frame + /// path. Since the resync loop stops exactly when fewer than 4 bytes remain, and + /// starts with exactly 200 bytes at cursor 0, the deterministic final residual is + /// the last 3 garbage bytes. + /// + /// Traces: BC-2.20.015 postcondition 3(b), invariant 2; AC-186-009. + #[test] + fn test_BC_2_20_015_resync_terminates_no_valid_anchor() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + let data = vec![0xAAu8; 200]; + analyzer.on_data(flow_key.clone(), &data, 0, Direction::ClientToServer); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.carry_c2s, + vec![0xAAu8; 3], + "with no valid 0x03 anchor anywhere in 200 garbage bytes, the resync walk \ + must terminate (not infinite-loop) at the deterministic 3-byte remainder \ + (BC-2.20.015 postcondition 3(b), invariant 2)" + ); + assert!( + analyzer.findings.is_empty(), + "an ordinary no-anchor-found resync must not emit any finding" + ); + } + + // ========================================================================= + // BC-2.20.016: frozen `iso_on_tcp.rs` module boundary — pure free functions only, + // no StreamAnalyzer impl, no per-flow state of its own. + // + // These are static/structural regression-guard tests (grep-equivalent), per + // BC-2.20.016's own "Verification Properties" note: verified by code-review + // inspection and static greps, not a runtime proof harness. They are expected to + // already be green (the frozen boundary is satisfied by the current stub) and + // exist here as permanent drift guards against a future violation. + // ========================================================================= + + /// Strips `//`, `///`, and `//!` line-comment content from Rust source text before + /// a static grep-equivalent scan, so that architectural doc comments *describing* + /// the absence of a construct (e.g. "no `impl StreamAnalyzer` block of any kind") + /// are not themselves mistaken for the construct they document the absence of. + /// A literal `grep -c` over the raw file (as BC-2.20.016's canonical vector table + /// spells it) would false-positive on exactly this kind of self-documenting + /// frozen-boundary prose; this scan targets the actual code, matching the BC's + /// substantive intent (postconditions 1/3: no such *code construct* exists). + fn strip_line_comments(content: &str) -> String { + content + .lines() + .map(|line| match line.find("//") { + Some(idx) => &line[..idx], + None => line, + }) + .collect::>() + .join("\n") + } + + /// AC-186-010: `iso_on_tcp.rs` contains zero `impl StreamAnalyzer` blocks and zero + /// `DispatchTarget::IsoOnTcp`-shaped references. + /// + /// Traces: BC-2.20.016 postconditions 1-2; AC-186-010. + #[test] + fn test_BC_2_20_016_iso_on_tcp_has_no_stream_analyzer_impl() { + let path = concat!(env!("CARGO_MANIFEST_DIR"), "/src/analyzer/iso_on_tcp.rs"); + let raw = + std::fs::read_to_string(path).unwrap_or_else(|e| panic!("failed to read {path}: {e}")); + let content = strip_line_comments(&raw); + + assert!( + !content.contains("impl StreamAnalyzer"), + "src/analyzer/iso_on_tcp.rs must contain zero `impl StreamAnalyzer` blocks in \ + actual code (BC-2.20.016 postcondition 1) — SS-20 is a stateless \ + pure-function parsing library, not a dispatcher-registered analyzer" + ); + assert!( + !content.contains("DispatchTarget::IsoOnTcp"), + "src/analyzer/iso_on_tcp.rs must never reference a `DispatchTarget::IsoOnTcp` \ + variant in actual code (BC-2.20.016 postcondition 2) — the only new \ + dispatcher variant for this feature is `DispatchTarget::S7comm`" + ); + } + + /// AC-186-011: no `IsoOnTcpFlowState` type exists anywhere in the tree — TPKT/COTP + /// carry buffers live exclusively on `S7commFlowState` (SS-21). + /// + /// Traces: BC-2.20.016 postcondition 3; AC-186-011. + #[test] + fn test_BC_2_20_016_no_iso_on_tcp_flow_state_type_exists() { + fn collect_rs_files(dir: &std::path::Path, out: &mut Vec) { + let entries = std::fs::read_dir(dir) + .unwrap_or_else(|e| panic!("failed to read dir {}: {e}", dir.display())); + for entry in entries { + let entry = entry.expect("directory entry read failure"); + let path = entry.path(); + if path.is_dir() { + collect_rs_files(&path, out); + } else if path.extension().and_then(|s| s.to_str()) == Some("rs") { + out.push(path); + } + } + } + + let src_dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); + let mut files = Vec::new(); + collect_rs_files(&src_dir, &mut files); + assert!( + !files.is_empty(), + "sanity: expected to find at least one .rs file under src/" + ); + + for file in &files { + let raw = std::fs::read_to_string(file) + .unwrap_or_else(|e| panic!("failed to read {}: {e}", file.display())); + let content = strip_line_comments(&raw); + assert!( + !content.contains("IsoOnTcpFlowState"), + "found a reference to `IsoOnTcpFlowState` in actual code in {} — no such \ + type may exist anywhere in the tree; TPKT/COTP carry buffers live \ + exclusively on `S7commFlowState` (BC-2.20.016 postcondition 3)", + file.display() + ); + } + } + + // ========================================================================= + // BC-2.21.003: on_flow_close removes S7commFlowState and discards all carry bytes. + // ========================================================================= + + /// AC-186-012: `on_flow_close` removes the flow's `S7commFlowState` (including + /// non-empty carry buffers) with no finding emitted; calling it for an unknown + /// `flow_key` is a no-op. + /// + /// Traces: BC-2.21.003 postconditions 1-4; AC-186-012. + #[test] + fn test_s7comm_on_flow_close_removes_state_discards_carry() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + { + let state = analyzer.flows.entry(flow_key.clone()).or_default(); + state.carry_c2s = vec![0x03, 0x00, 0x00, 0x0A]; + state.carry_s2c = vec![0xAA, 0xAA]; + } + assert!( + analyzer.flows.contains_key(&flow_key), + "precondition: flow state must exist before on_flow_close" + ); + + analyzer.on_flow_close(flow_key.clone()); + + assert!( + !analyzer.flows.contains_key(&flow_key), + "on_flow_close must remove the flow's S7commFlowState from the analyzer's \ + per-flow map (BC-2.21.003 postcondition 1)" + ); + assert!( + analyzer.findings.is_empty(), + "on_flow_close must not emit any finding, even with non-empty carry buffers \ + at closure (BC-2.21.003 postconditions 2-3)" + ); + + // Closing an unknown flow_key must be a no-op (BC-2.21.003 postcondition 4) — + // must not panic, and must not disturb any other flow's state. + // + // NOTE: `flow_key` (above) was just removed by the on_flow_close call, so + // re-inserting state under the *same* 4-tuple (`flow_key_default()`) here is + // deliberately re-creating a flow that shares `flow_key`'s identity — this is + // NOT a "different" flow for FlowKey-equality purposes, only a flow that + // happens to be tracked *after* the original's closure. It is named + // `reopened_flow_key` (not `other_key`) to make that byte-identity explicit and + // avoid the false impression that it exercises a distinct, second flow. + let reopened_flow_key = flow_key_default(); + { + let state = analyzer.flows.entry(reopened_flow_key.clone()).or_default(); + state.carry_c2s = vec![0x11, 0x22]; + } + let unknown_key = FlowKey::new( + "10.0.0.1".parse().unwrap(), + 9999, + "10.0.0.2".parse().unwrap(), + 102, + ); + analyzer.on_flow_close(unknown_key); + assert_eq!( + analyzer.flows.get(&reopened_flow_key).unwrap().carry_c2s, + vec![0x11, 0x22], + "closing an unknown flow_key must not disturb any other tracked flow's state \ + (BC-2.21.003 postcondition 4 no-op)" + ); + } + + /// EC-002 (BC-2.21.003): `on_flow_close` is called twice for the SAME `FlowKey` + /// (defensive double-close, adversarial finding F-06). The first call removes the + /// flow's `S7commFlowState` and emits no finding (BC-2.21.003 postconditions 1-3); + /// the second call — against the same, now-already-removed `flow_key` — must be a + /// harmless no-op per postcondition 4's "does not exist for the FlowKey" path: it + /// must not panic, must not re-create state, and must not emit any finding. This is + /// distinct from `test_s7comm_on_flow_close_removes_state_discards_carry`'s + /// unknown-key no-op case above, which exercises a `flow_key` that was *never* + /// tracked in the first place, rather than one tracked-then-closed-then-closed-again. + /// + /// Traces: BC-2.21.003 postcondition 4, edge case EC-002. + #[test] + fn test_BC_2_21_003_double_close_same_flow_key_is_idempotent_no_op() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + { + let state = analyzer.flows.entry(flow_key.clone()).or_default(); + state.carry_c2s = vec![0x03, 0x00, 0x00, 0x0A]; + state.carry_s2c = vec![0xAA, 0xAA]; + } + assert!( + analyzer.flows.contains_key(&flow_key), + "precondition: flow state must exist before the first on_flow_close" + ); + + // First close: removes the state, emits no finding. + analyzer.on_flow_close(flow_key.clone()); + assert!( + !analyzer.flows.contains_key(&flow_key), + "first on_flow_close must remove the flow's S7commFlowState \ + (BC-2.21.003 postcondition 1)" + ); + assert!( + analyzer.findings.is_empty(), + "first on_flow_close must not emit any finding (BC-2.21.003 postconditions \ + 2-3), even with non-empty carry buffers at closure" + ); + + // Second close on the SAME flow_key: idempotent no-op (BC-2.21.003 EC-002). + analyzer.on_flow_close(flow_key.clone()); + assert!( + !analyzer.flows.contains_key(&flow_key), + "second on_flow_close on the same, already-removed flow_key must remain a \ + no-op — state stays absent, it is not re-created (BC-2.21.003 \ + postcondition 4 / EC-002 'second call is a no-op')" + ); + assert!( + analyzer.findings.is_empty(), + "the second (double) close must not emit any finding either — a harmless \ + no-op, not an error condition (BC-2.21.003 EC-002)" + ); + } + + // ========================================================================= + // VP-050: TPKT/COTP Carry-Buffer Residual-Bound Reassembly, Overflow Isolation, + // and 1-Byte Resync (proptest P1; traces BC-2.20.013..015). + // ========================================================================= + + mod vp050 { + use proptest::prelude::*; + use wirerust::analyzer::s7comm::{MAX_S7_ISO_ON_TCP_CARRY_BYTES, S7commAnalyzer}; + use wirerust::reassembly::flow::FlowKey; + use wirerust::reassembly::handler::Direction; + + fn flow_key_default() -> FlowKey { + FlowKey::new( + "127.0.0.1".parse().unwrap(), + 1234, + "127.0.0.2".parse().unwrap(), + 102, + ) + } + + proptest! { + /// VP-050 sub-property: walk-first residual bound (BC-2.20.014 invariant + /// 1). For any pre-existing directional carry (including deliberately + /// oversized, adversarial values well past the bound) and any incoming + /// delivery, the residual left in `carry_c2s` after `on_data` returns must + /// never exceed `MAX_S7_ISO_ON_TCP_CARRY_BYTES` — an overflow always clears + /// the carry back down; it can never grow unbounded. + #[test] + fn proptest_vp050_walk_first_residual_bound( + preexisting_carry in prop::collection::vec(any::(), 0..70_000), + incoming in prop::collection::vec(any::(), 0..500), + ) { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + { + let state = analyzer.flows.entry(flow_key.clone()).or_default(); + state.carry_c2s = preexisting_carry; + } + + analyzer.on_data(flow_key.clone(), &incoming, 0, Direction::ClientToServer); + + let state = analyzer.flows.get(&flow_key).unwrap(); + prop_assert!( + state.carry_c2s.len() <= MAX_S7_ISO_ON_TCP_CARRY_BYTES, + "carry_c2s residual ({}) must never exceed MAX_S7_ISO_ON_TCP_CARRY_BYTES \ + ({}) for any input (BC-2.20.014 invariant 1 / VP-050)", + state.carry_c2s.len(), + MAX_S7_ISO_ON_TCP_CARRY_BYTES + ); + } + + /// VP-050 sub-property: direction isolation (BC-2.20.013 invariant 3). An + /// `on_data` call in one direction must never mutate the other direction's + /// carry buffer — `carry_c2s` after a C2S delivery followed by an + /// unrelated S2C delivery must be byte-for-byte identical to `carry_c2s` + /// immediately after the C2S delivery alone. + #[test] + fn proptest_vp050_direction_isolation( + c2s_data in prop::collection::vec(any::(), 0..300), + s2c_data in prop::collection::vec(any::(), 0..300), + ) { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + analyzer.on_data(flow_key.clone(), &c2s_data, 0, Direction::ClientToServer); + let carry_c2s_after_c2s_only = analyzer + .flows + .get(&flow_key) + .map(|s| s.carry_c2s.clone()) + .unwrap_or_default(); + + analyzer.on_data(flow_key.clone(), &s2c_data, 0, Direction::ServerToClient); + let state = analyzer.flows.get(&flow_key).unwrap(); + + prop_assert_eq!( + &state.carry_c2s, + &carry_c2s_after_c2s_only, + "an S2C delivery must never mutate carry_c2s (BC-2.20.013 invariant 3 \ + directional isolation / VP-050)" + ); + prop_assert!(state.carry_c2s.len() <= MAX_S7_ISO_ON_TCP_CARRY_BYTES); + prop_assert!(state.carry_s2c.len() <= MAX_S7_ISO_ON_TCP_CARRY_BYTES); + } + + /// VP-050 sub-property: 1-byte resync advance (BC-2.20.015 postconditions + /// 1-3, invariant 1). For any length of leading non-`0x03` garbage + /// followed by a complete, valid 7-byte TPKT/COTP frame, the resync walk + /// must find and fully extract that trailing frame — a resync that ever + /// advanced by more than 1 byte could skip the anchor and leave the frame + /// unextracted. + #[test] + fn proptest_vp050_resync_one_byte_advance(garbage_len in 0usize..50) { + let mut data = vec![0xAAu8; garbage_len]; + data.extend_from_slice(&[0x03, 0x00, 0x00, 0x07, 0x01, 0xE0, 0x00]); + + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + analyzer.on_data(flow_key.clone(), &data, 0, Direction::ClientToServer); + + let state = analyzer.flows.get(&flow_key).unwrap(); + prop_assert!( + state.carry_c2s.is_empty(), + "a 1-byte resync must find the anchor at offset {} (past the leading \ + garbage) and fully extract the trailing complete 7-byte frame, \ + leaving carry_c2s empty (BC-2.20.015 postconditions 1-3 / VP-050); \ + carry_c2s = {:?}", + garbage_len, + state.carry_c2s + ); + } + } + } +}