diff --git a/CHANGELOG.md b/CHANGELOG.md index 916fd84e..401222c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,52 @@ Version numbers follow [Semantic Versioning](https://semver.org/). 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). +- S7comm classic (`0x32`) header parsing and four-way `protocol_id` dispatch + (`src/analyzer/s7comm.rs`, STORY-187, ADR-014 Decisions 2/9): `S7commFlowState` + gains `session_established`, `cr_observed_dir: Option`, + `classified_protocol: Option`, and the + `malformed_header_reported_c2s`/`_s2c` dedup flags (BC-2.21.001). `on_data`'s + frame dispatch now branches on `CotpHeader::protocol_id` (BC-2.21.002): a + Connect Request (CR) records the pending direction, and a Connect Confirm (CC) + marks `session_established` only when observed in the direction OPPOSITE a + previously-recorded CR on the same flow — a bare CR, a CC with no prior CR, an + out-of-order CC, and a same-direction CC all leave it `false`. The first Data + Transfer (DT) frame carrying a `Some(byte)` `protocol_id` sets + `classified_protocol` exactly once (`Some(0x32)` -> `Classic`, `Some(0x72)` -> + `Plus`, any other `Some(byte)` -> `Unclassified`), sticky for the life of the + flow; a `protocol_id: None` DT frame carries no protocol evidence and never + classifies, deferring classification to a later `Some(byte)` DT frame if any. + A new pure-core free function, `parse_s7comm_header`, extracts the classic + S7comm common header (ROSCTR, PDU reference, parameter/data length, + big-endian `u16` fields) for Job/Userdata ROSCTR values (10-byte header), and + the 12-byte Ack/Ack_Data extension (error class/code) for BOTH the Ack AND + Ack_Data ROSCTR values (2026-09-24 canonical-frame holdout ruling, + DF-CANONICAL-FRAME-HOLDOUT-001 — a real-world Ack_Data/Setup-Communication- + response parameter block only aligns at byte 12, not byte 10) — returning + `None` for under-length input, a defensively-rechecked non-`0x32` + protocol-ID byte, an unrecognized ROSCTR byte, or a truncated Ack/Ack_Data + (BC-2.21.004-008). Classic dissection (`parse_s7comm_header` call) fires only + when the current DT frame's `protocol_id == Some(0x32)` AND the flow's + STICKY `classified_protocol == Some(Classic)` — a flow already + sticky-classified `Plus`/`Unclassified` by an earlier DT frame is never + dissected, even on a later `0x32`-leading frame, per ADR-014 Decision 2's + no-misattribution guarantee. The declared `param_length`/`data_length` are + bounds-checked against the bytes actually available via a new pure, + public (`pub fn`) helper, `s7comm_bounds_ok(header: &S7commHeader, data_len: + usize) -> bool` (BC-2.21.009), before any parameter/data-block slice is + attempted — the same helper the VP-051 Kani harness calls directly. + Malformed-header conditions (parse failure or bounds-check failure) emit one + T0814 (Anomaly/Possible/Medium) per flow direction, deduplicated via the new + dedup flags, with evidence text stating the specific reject reason (too + short, unrecognized ROSCTR, truncated Ack, truncated Ack_Data, or + declared-vs-available byte counts). The `Some(0x72)` (S7comm-plus) and + unrecognized/`None` + `protocol_id` branches remain panic-free structural no-ops; their observable + behavior is STORY-190's scope. Includes a `#[cfg(kani)]` VP-051 + bounds-safety skeleton and a VP-053 dispatch-totality proptest (runs under + `cargo test`; the full non-vacuous VP-051 Kani run (both harnesses, + `--fail-uncoverable`, deliberate-flip check) and the full VP-053 obligation + are deferred to STORY-194). ### Fixed 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 8b738066..71585470 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 @@ -188,6 +188,50 @@ the analyzer is load-bearing for correctness, not merely defense-in-depth. Contr with IEC-104's `is_valid_iec104_frame` (ADR-013 Decision 1), which only rejects garbage and never re-routes to a *different* named protocol. +> **RECONCILIATION NOTE (2026-09-24, STORY-187 adversarial pass 1; F-01/F-02/F-12):** +> Three human rulings refine this Decision's disambiguation table and dispatch gating, +> effective for BC-2.21.001/002 (product-owner has amended these two contracts to +> match; see each BC's `modified:` history; this note tracks the architectural +> ruling, not the BC text itself — do not treat this note as a substitute for the +> amended BCs): +> +> - **F-01 (session establishment is directional-paired, not any-CC).** +> `session_established` is set only by a CC TPDU observed in the direction *opposite* +> a previously-observed CR on the same flow — not by any CC regardless of CR history, +> as an unqualified reading of the table's `None (CR/CC TPDU)` row could suggest. A +> `pending-CR`-shaped field on `S7commFlowState` (tracking the direction of the most +> recently observed CR — overwritten by each subsequent CR, not cleared on a matching +> CC, since `session_established` transitions monotonically and no postcondition reads +> this field after that transition) is a permitted implementation detail to express +> this pairing; the field is not required to be named that, only to exist in some +> equivalent form. +> - **F-02 (`protocol_id: None` never classifies).** A DT-TPDU with `protocol_id: None` +> (an empty upper-layer payload — Decision 1's `CotpHeader::protocol_id` doc comment) +> does **not** participate in sticky first-classification. `classified_protocol` +> remains unset until the first DT-TPDU carrying `Some(byte)` is observed; only that +> frame's byte drives the three-way `0x32 -> Classic` / `0x72 -> Plus` / other +> `-> Unclassified` assignment, sticky thereafter. This narrows the table's `None +> (CR/CC TPDU)` row: "defer classification until the first DT frame arrives" must be +> read as "until the first DT frame *carrying a protocol-ID byte* arrives" — an +> empty-payload DT frame is treated the same as "not yet arrived," not as a +> classifying event in its own right. +> - **F-12 (classic dissection gated on sticky classification, not per-frame +> `protocol_id`).** Classic S7comm header dissection — `parse_s7comm_header`, its +> bounds check, and malformed-header T0814 emission — runs only when the flow's +> *sticky* `classified_protocol` is `Classic`, not merely when the current DT frame's +> `protocol_id == Some(0x32)`. This is this Decision's own no-misattribution guarantee +> ("a COTP DT-TPDU ... must never be misattributed to S7comm") applied symmetrically at +> the flow level: first-classification-wins (BC-2.21.002 postcondition 6) must govern +> *every* frame on the flow, not just the classifying one — a flow sticky-classified +> `Plus` or `Unclassified` must not fall into classic dissection even if a later, +> individual frame happens to carry a `0x32` protocol-ID byte (e.g. a malformed or +> adversarial frame on an already-classified flow). +> +> These rulings refine Decision 2's disambiguation table and analyzer-side gating; they +> do not change this Decision's dispatcher-level conclusion (single +> `DispatchTarget::S7comm` rule, in-analyzer disambiguation, VP-004 six-step obligation +> unaffected). + **VP-004 six-step atomic obligation** (mirrors ADR-013 Decision 9, ADR-010 Decision 1), to be executed in the same commit: @@ -377,6 +421,13 @@ behavioral* sources only: the Wireshark **wiki** page (prose, not the dissector Kleinmann & Wool 2014, and the Orange-Cyberdefense `awesome-industrial-protocols` catalog. +> [Note, 2026-09-24: the "only" above is read together with the "Permitted design +> references" list immediately below — those BSD/MIT-licensed sources also ground +> field-level structural detail (e.g. the Ack/Ack_Data 12-byte header and +> error_class/error_code byte split), design reference only, no code copied. See the +> F-40 reconciliation note and Decision 9's canonical-frame note below for the specific +> field this applies to.] + **Permitted design references (no verbatim code copy, design reference only):** - `cisagov/icsnpp-s7comm` (BSD-3-Clause) - `kprovost/libs7comm` (BSD-2-Clause) @@ -389,6 +440,34 @@ this is materially lower-risk than any effort touching S7comm-plus authenticatio TLS. No external S7/COTP/TPKT crate appears in `Cargo.toml`/`Cargo.lock` — original Rust parser only, zero lines borrowed, following the ADR-013 Decision 7 precedent exactly. +> **RECONCILIATION NOTE (2026-09-24, STORY-187 per-story adversarial pass 5, F-40, +> HUMAN RULING):** Decision 4's allowed-source list above is **amended, not rewritten** +> by this note — the original decision text and table stand as-is; this note adds a new, +> narrowly-scoped permission on top of it. Publicly posted **wire-capture byte examples** +> — observed on-the-wire frame bytes appearing in public documentation, blog posts, or +> vendor knowledge-base pages — are **PERMITTED AS TEST-VECTOR SOURCES ONLY**, satisfying +> `DF-CANONICAL-FRAME-HOLDOUT-001`. This permission is version-agnostic (it does not turn +> on any particular wirerust or ADR version) and strictly test-vector-scoped: parser +> design and field semantics continue to derive **only** from Decision 4's original prose +> sources (the Wireshark wiki page, Kleinmann & Wool 2014, the Orange-Cyberdefense +> catalog) and the "Permitted design references" list above — this ruling does not add to +> or relax that list. Code from the banned/GPL dissectors (Wireshark +> `packet-s7comm.c`/`packet-s7comm_plus.c`, Snap7, libnodave) and the AVOID-list crates +> (`s7`/`s7-comm`/`s7-client`, `rusty-cotp`/`rusty-tpkt`/`tpkt`/`copt`) remains excluded +> in full — this ruling does not touch the code-provenance ban. Rationale: observed wire +> bytes are protocol facts (what a real device put on the wire), not copyrighted source +> expression; using them as test-vector inputs/expected-outputs is categorically +> different from copying dissector code. Three sources are currently used under this +> test-vector-only permission (see BC-2.21.008's Canonical Test Vectors and ADR-014 +> Decision 9's canonical-frame holdout notes for concrete usage): +> 1. cnblogs, "西门子S7通讯协议引用整理" (primary): +> +> 2. Yiqisoft (2023-03-22): — the +> cited bytes are output **produced by** the `gos7` library (observed wire/library +> output only; no `gos7` source code was read, copied, or used as a design +> reference). +> 3. Inductive Automation Knowledge Base, "Loggers - Device Connections: Siemens". + ### Decision 5: MITRE ATT&CK for ICS technique set — 3 new IDs, 8 reused, tactic-variant ruling Per `.factory/research/s7comm-mitre-ics-tagging.md` (live technique-page verification, @@ -593,6 +672,94 @@ and directional carry-buffer isolation (mirrors VP-045/VP-046). cargo-fuzz P1 fo combined TPKT→COTP→S7comm parse chain's no-panic property under arbitrary byte input (mirrors VP-047). +> **RECONCILIATION NOTE (2026-09-24, STORY-187 adversarial pass 1; F-14):** +> VP-INDEX.md registers **VP-051** as a Kani P0 target for `parse_s7comm_header`, +> superseding item 3's original tool-selection text above ("cargo-fuzz candidate ... +> rather than Kani, mirroring VP-047's IEC-104 treatment of `parse_asdu`"). On review, +> `parse_s7comm_header` fits the same Kani-tractability profile as the two SS-20 +> header-parse functions (item 1/2 above) rather than `parse_asdu`'s profile: a fixed +> 10-or-12-byte header, `Option`-returning, no internal loop over caller-supplied +> length — the caller-side bounds check against `param_length`/`data_length` +> (BC-2.21.009) is `dispatch_classic_s7comm`'s responsibility, not +> `parse_s7comm_header`'s, which keeps the parse function itself loop-free and small +> enough for model checking. VP-051's Kani harness MUST be **non-vacuous** per +> DF-KANI-NONVACUITY-001 (a harness whose input constraints admit no cases is not a +> proof — it must be checked to actually exercise both the `Some` and `None` return +> paths) and MUST use **bounded input / unwind**: a fixed-size symbolic byte array +> (`kani::any()` over a bounded-length slice, not unbounded `Vec`) with an explicit +> unwind/loop bound, matching `parse_tpkt_header`/`parse_cotp_header`'s existing harness +> shape (item 1/2). cargo-fuzz remains in scope, unchanged, for the *combined* +> TPKT→COTP→S7comm parse chain's no-panic property (this item's closing sentence above); +> it is no longer the sole verification tool for `parse_s7comm_header` in isolation — +> Kani and cargo-fuzz are complementary here (proof of the isolated header parse plus +> fuzzing of the full chain), not alternatives. + +> **RECONCILIATION NOTE (2026-09-24, STORY-187 canonical-frame holdout):** A +> human-ratified ruling on real captured Setup Communication traffic corrects this +> item's "10-or-12-byte header" framing, which implicitly limited the 12-byte/ +> error-field case to `rosctr == Ack` alone. **Both** ROSCTR `0x02` (Ack) **and** `0x03` +> (Ack_Data) carry the 12-byte header with `error_class = data[10]`, +> `error_code = data[11]`; Job (`0x01`) and Userdata (`0x07`) remain the 10-byte +> header with no error fields. Field-semantics grounding (permitted sources per Decision +> 4 / the F-40 reconciliation note above — prose sources and design references only, no +> wire-capture bytes): Kleinmann & Wool 2014 (JDFSL 9(2), §3.2, Figure 2, p.41) documents +> a 2-byte error block labeled "Error Code - only for ROSCTR 3", establishing the +> Ack_Data (`0x03`) 12-byte header (the authors observed only ROSCTR 1 and 3 traffic, so +> Ack (`0x02`) is not independently attested there). The permitted design references +> corroborate this and extend it to Ack: cisagov/icsnpp-s7comm (BSD-3-Clause), +> `src/s7comm-protocol.pac`, declares separate records for `ROSCTR_ACK` (`0x02`) and +> `ROSCTR_ACK_Data` (`0x03`), each embedding an `S7Comm_Error` record of two `uint8` +> fields (`error_class`, `error_code`) immediately after the four 2-byte header fields, +> with no such record for Job/User_Data — as reviewed 2026-09-24; gijzelaerr/python-snap7 +> (MIT), `snap7/s7protocol.py`, function `parse_response`, documents in comment that ACK +> and ACK_DATA carry a 12-byte header with `error_class`/`error_code` as separate bytes +> at offsets 10 and 11, versus a 10-byte USERDATA header — as reviewed 2026-09-24. +> kprovost/libs7comm (BSD-2-Clause), `src/lib/s7comm_types.h` and `s7comm.c`, is +> consistent in aggregate: message types 2 and 3 carry 2 extra bytes, modeled there as a +> single 16-bit "result" field rather than the two discrete error_class/error_code bytes +> — as reviewed 2026-09-24. `parse_s7comm_header`'s `header_len` selection is therefore +> `12` when `rosctr ∈ {Ack, AckData}`, `10` otherwise, with `error_class = data[10]`, +> `error_code = data[11]`. +> +> Caveat: icsnpp-s7comm, python-snap7, and libs7comm are reverse-engineered, +> community-maintained implementations, not a Siemens specification — Siemens has not +> published the classic S7comm wire format. This grounding is version-agnostic; it does +> not turn on any particular wirerust or ADR version. +> +> Canonical-frame test vectors (PERMITTED AS TEST-VECTOR SOURCES ONLY per the F-40 +> Decision 4 reconciliation note above — not used for field-semantics grounding): a real +> Setup Communication Ack_Data frame — +> `32 03 00 00 FF FF 00 08 00 00 00 00 F0 00 00 01 00 01 00 F0` (cnblogs, +> ) — whose parameter block +> (`F0 00 ...`, Setup Communication function `0xF0`) begins at byte 12, not byte 10, is +> the frame that surfaced this defect; corroborated by independent secondary test-vector +> sources (Yiqisoft S7comm protocol notes; Inductive Automation Knowledge Base). +> +> This corrects VP-051's non-vacuity obligation (F-14, above) in lockstep: the +> harness's Some/None deliberate-flip check must assert +> `error_class.is_some() == (rosctr == Ack || rosctr == AckData)`, **not** +> `error_class.is_some() == (rosctr == Ack)` — a harness written against the latter, +> narrower formula would vacuously pass a `header_len`/`error_class` mismatch on every +> Ack_Data input (12-byte frame incorrectly parsed with a 10-byte, no-error-fields +> header). Mirrored in `.factory/specs/verification-properties/VP-INDEX.md`, +> `.factory/specs/architecture/verification-architecture.md`, and +> `.factory/specs/architecture/verification-coverage-matrix.md` (architect burst, same +> date). Product-owner amended BC-2.21.004, BC-2.21.006, BC-2.21.007 +> (length-conditional accept rule; BC-2.21.008 linked as the Ack/Ack_Data accept +> path), BC-2.21.008 and BC-2.21.009 to match; see each BC's `modified:` history. +> This note tracks the architectural ruling, not the BC text itself — do not treat +> this note as a substitute for the amended BCs. + +> **NOTE (2026-09-24, STORY-187 per-story adversarial pass 5, F-39 secondary):** this +> item's "proptest P1 for the protocol-ID branch totality" sentence (above, Decision 9's +> Tool-selection paragraph) is superseded on phase — VP-INDEX.md registers **VP-053** as +> a proptest **P0** target (not P1) for this property, load-bearing because a +> protocol-ID-branch classification defect would misattribute or force-fit traffic onto +> the wrong protocol (never-force-fit unclassified-gap handling, port-102 four-way +> collision). VP-INDEX.md is the phase-assignment source of truth; this note is +> version-agnostic and does not change VP-053's tool (proptest), module +> (`analyzer/s7comm.rs`), or property text — phase only. + **VP numbering is explicitly deferred to product-owner** at F2 BC/VP authoring (this ADR does not register new VP-NNN IDs; VP-004 and VP-007 are pre-existing obligations being extended, not new VPs). The last currently-registered VP is VP-047 (IEC-104); the F1 @@ -810,10 +977,13 @@ for `parse_tpkt_header`/`parse_cotp_header`/`parse_s7comm_header` at F2 BC/VP au - **RFC 1006** (IETF, STD 35) — TPKT framing structure and length-field semantics. - **ITU-T X.224 ≡ ISO/IEC 8073:1997** — COTP TPDU types (CR/CC/DT), Length Indicator field. -- **S7comm classic protocol structure** — free-to-read prose/behavioral sources only - (Wireshark S7comm wiki page, Kleinmann & Wool 2014, Orange-Cyberdefense - `awesome-industrial-protocols`); no GPL/LGPL source consulted as an implementation - template (Decision 4). +- **S7comm classic protocol structure** — prose/behavioral sources (Wireshark S7comm + wiki page, Kleinmann & Wool 2014, Orange-Cyberdefense `awesome-industrial-protocols`) + and the Decision 4 permitted design references (`cisagov/icsnpp-s7comm`, + `kprovost/libs7comm`, `python-snap7` — design reference only, no code copied); public + wire-capture bytes used as test vectors only, not field-semantics grounding (Decision 4 + F-40 reconciliation note, 2026-09-24); no GPL/LGPL source consulted as an + implementation template (Decision 4). - **Feature cycle:** `feature-s7comm` — this ADR governs the S7comm/ISO-on-TCP subsystems (SS-20, SS-21) delivered in that cycle. - **F1/F2 research (this cycle):** diff --git a/docs/demo-evidence/STORY-187/AC-001-003-flow-state-session.gif b/docs/demo-evidence/STORY-187/AC-001-003-flow-state-session.gif new file mode 100644 index 00000000..d9d9c492 Binary files /dev/null and b/docs/demo-evidence/STORY-187/AC-001-003-flow-state-session.gif differ diff --git a/docs/demo-evidence/STORY-187/AC-001-003-flow-state-session.tape b/docs/demo-evidence/STORY-187/AC-001-003-flow-state-session.tape new file mode 100644 index 00000000..77dfca11 --- /dev/null +++ b/docs/demo-evidence/STORY-187/AC-001-003-flow-state-session.tape @@ -0,0 +1,44 @@ +# STORY-187 AC-187-001/002/003: S7commFlowState field set, lazy creation, and +# CR/CC session_established matching (BC-2.21.001) +# +# Demonstrates: +# - test_BC_2_21_001_flow_state_field_set: S7commFlowState carries the full +# field set this story adds (session_established, cr_observed_dir, +# classified_protocol, malformed_header_reported_c2s/_s2c) (AC-187-001) +# - test_BC_2_21_001_lazy_flow_state_creation: state is created on first +# on_data call, not before (AC-187-002) +# - test_BC_2_21_001_cr_then_opposite_cc_sets_session_established: CR(A) then +# CC(B), B opposite A -> session_established becomes true (AC-187-003) +# - Five negative sequences (CR-only, CC-only, CC-before-CR, same-direction +# CC, repeated same-direction CR) that all leave session_established false +# (AC-187-003) +# - test_BC_2_21_001_most_recent_cr_direction_wins: cr_observed_dir is +# overwritten by every CR, not only the first (AC-187-003) +# - test_BC_2_21_001_session_established_is_monotonic: once true, the flag is +# never reverted by later frames (AC-187-003) +# +# Traces to: BC-2.21.001 postconditions 1-3, edge cases EC-004..EC-009 +# Test: cargo test --test s7comm_analyzer_tests BC_2_21_001 + +Output AC-001-003-flow-state-session.gif +Output AC-001-003-flow-state-session.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-187-001/002/003: S7commFlowState field set, lazy creation, CR/CC session tracking (BC-2.21.001)" +Enter +Sleep 300ms + +Type "cargo test --test s7comm_analyzer_tests BC_2_21_001 2>&1 | grep -E 'story_187::test|test result:'" +Enter +Wait +Sleep 2s diff --git a/docs/demo-evidence/STORY-187/AC-001-003-flow-state-session.webm b/docs/demo-evidence/STORY-187/AC-001-003-flow-state-session.webm new file mode 100644 index 00000000..ec58b181 Binary files /dev/null and b/docs/demo-evidence/STORY-187/AC-001-003-flow-state-session.webm differ diff --git a/docs/demo-evidence/STORY-187/AC-004-005-012-dispatch-classification.gif b/docs/demo-evidence/STORY-187/AC-004-005-012-dispatch-classification.gif new file mode 100644 index 00000000..0a01fbb2 Binary files /dev/null and b/docs/demo-evidence/STORY-187/AC-004-005-012-dispatch-classification.gif differ diff --git a/docs/demo-evidence/STORY-187/AC-004-005-012-dispatch-classification.tape b/docs/demo-evidence/STORY-187/AC-004-005-012-dispatch-classification.tape new file mode 100644 index 00000000..e55a14bf --- /dev/null +++ b/docs/demo-evidence/STORY-187/AC-004-005-012-dispatch-classification.tape @@ -0,0 +1,59 @@ +# STORY-187 AC-187-004/005/012: Some(0x32) DT dispatch, sticky first-classification- +# wins, and classic-dissection gating on sticky classified_protocol (BC-2.21.002) +# +# Demonstrates: +# - test_BC_2_21_002_classic_s7comm_dispatch_asserts_classified_protocol_classic: +# a Some(0x32) DT frame dispatches to classic dissection (AC-187-004) +# - test_BC_2_21_002_malformed_0x32_frame_on_classic_flow_emits_exactly_one_t0814: +# malformed error-path counterpart -- exactly one T0814, not zero, not more +# (AC-187-004) +# - test_BC_2_21_002_sticky_first_classification: first DT frame's protocol_id +# classifies the flow exactly once; later frames never overwrite it (AC-187-005) +# - test_BC_2_21_002_none_protocol_id_dt_first_then_0x32_dt_classifies_classic: +# protocol_id: None never classifies; classification stays deferred (AC-187-005) +# - test_BC_2_21_002_two_frames_one_delivery_first_write_wins / +# test_BC_2_21_002_empty_dt_followed_by_frame_same_delivery_stays_unclassified / +# test_BC_2_21_002_unparseable_cotp_does_not_classify (AC-187-005) +# - test_BC_2_21_002_0x32_dt_frame_not_dissected_when_sticky_classified_plus_or_unclassified / +# test_BC_2_21_002_non_0x32_dt_frame_on_classic_flow_not_dissected: the gate is a +# conjunction of BOTH the frame's own protocol_id byte AND the flow's sticky +# classified_protocol -- neither alone is sufficient (AC-187-012) +# - proptest_vp053_protocol_id_dispatch_totality: VP-053 property -- for any +# protocol_id (None or Some(byte)), the first DT frame classifies exactly per +# the four-way table and always creates flow state +# +# Note: this filter also picks up test_BC_2_21_002_setup_comm_fixture_pcap_well_formed_no_findings +# (AC-187-013's fixture-pcap regression test, also recorded in its own dedicated +# context in AC-013-canonical-frames) since it shares this BC's dispatch path. +# +# Traces to: BC-2.21.002 postconditions 1, 3, 6, invariant 4, edge cases EC-003..EC-005 +# Test: cargo test --test s7comm_analyzer_tests BC_2_21_002 +# Test: cargo test --test s7comm_analyzer_tests vp053 + +Output AC-004-005-012-dispatch-classification.gif +Output AC-004-005-012-dispatch-classification.webm + +Set FontFamily "Menlo" +Set Theme "Dracula" +Set Width 1200 +Set Height 620 +Set FontSize 14 +Set Padding 24 +Set Shell "bash" +Set TypingSpeed 50ms + +Require cargo + +Type "# AC-187-004/005/012: Some(0x32) DT dispatch + sticky first-classification-wins + gating (BC-2.21.002)" +Enter +Sleep 300ms + +Type "cargo test --test s7comm_analyzer_tests BC_2_21_002 2>&1 | grep -E 'story_187::test|test result:'" +Enter +Wait +Sleep 1s + +Type "cargo test --test s7comm_analyzer_tests vp053 2>&1 | grep -E 'story_187::vp053|test result:'" +Enter +Wait +Sleep 2s diff --git a/docs/demo-evidence/STORY-187/AC-004-005-012-dispatch-classification.webm b/docs/demo-evidence/STORY-187/AC-004-005-012-dispatch-classification.webm new file mode 100644 index 00000000..7788dc67 Binary files /dev/null and b/docs/demo-evidence/STORY-187/AC-004-005-012-dispatch-classification.webm differ diff --git a/docs/demo-evidence/STORY-187/AC-006-009-length-rosctr-parser.gif b/docs/demo-evidence/STORY-187/AC-006-009-length-rosctr-parser.gif new file mode 100644 index 00000000..278f0bb5 Binary files /dev/null and b/docs/demo-evidence/STORY-187/AC-006-009-length-rosctr-parser.gif differ diff --git a/docs/demo-evidence/STORY-187/AC-006-009-length-rosctr-parser.tape b/docs/demo-evidence/STORY-187/AC-006-009-length-rosctr-parser.tape new file mode 100644 index 00000000..76173e4d --- /dev/null +++ b/docs/demo-evidence/STORY-187/AC-006-009-length-rosctr-parser.tape @@ -0,0 +1,78 @@ +# STORY-187 AC-187-006/007/008/009: parse_s7comm_header 10-byte minimum, defensive +# 0x32 reject, Job/Userdata common-header extraction, and unrecognized-ROSCTR +# safe-reject totality (BC-2.21.004/005/006/007) +# +# Demonstrates: +# - test_BC_2_21_004_parse_returns_none_for_len_lt_10 / +# test_BC_2_21_004_len_shorter_than_10_returns_none_and_emits_t0814_once / +# test_BC_2_21_004_len_shorter_than_10_emits_t0814_once_s2c / +# test_BC_2_21_004_nine_byte_payload_on_data_too_short_evidence: too-short +# header (<10 bytes) returns None, no panic, one T0814 per direction, exact +# boundary evidence text (AC-187-006 -- ERROR PATH) +# - test_BC_2_21_005_defensive_reject_wrong_protocol_id_byte / +# test_BC_2_21_005_defensive_reject_zero_byte: data[0] != 0x32 returns None, +# no Finding -- caller-hygiene defense-in-depth (AC-187-007) +# - test_BC_2_21_006_common_header_field_extraction / +# test_BC_2_21_006_nonzero_reserved_bytes_do_not_reject / +# test_BC_2_21_006_byte_asymmetric_big_endian_decode / +# test_BC_2_21_006_canonical_setup_communication_job_frame_on_data: Job/Userdata +# common-header field extraction at the correct byte offsets (AC-187-008 -- +# SUCCESS PATH) +# - test_BC_2_21_007_unrecognized_rosctr_returns_none / +# test_BC_2_21_007_shares_dedup_flag_with_004_malformed_header / +# test_BC_2_21_007_unrecognized_rosctr_emits_t0814_once_s2c / +# proptest_bc_2_21_007_rosctr_byte_totality_over_all_256_values: unrecognized +# ROSCTR byte returns None for all 252 remaining u8 values, no panic, shared +# dedup flag with AC-187-006, s2c dedup verified independently (AC-187-009 -- +# ERROR PATH + exhaustive totality) +# +# Traces to: BC-2.21.004 postconditions 1-2/4, BC-2.21.005 postconditions 1/3, +# BC-2.21.006 postconditions 1-5, BC-2.21.007 postconditions 1-3 +# Test: cargo test --test s7comm_analyzer_tests BC_2_21_004 +# Test: cargo test --test s7comm_analyzer_tests BC_2_21_005 +# Test: cargo test --test s7comm_analyzer_tests BC_2_21_006 +# Test: cargo test --test s7comm_analyzer_tests BC_2_21_007 +# Test: cargo test --test s7comm_analyzer_tests bc_2_21_007 + +Output AC-006-009-length-rosctr-parser.gif +Output AC-006-009-length-rosctr-parser.webm + +Set FontFamily "Menlo" +Set Theme "Dracula" +Set Width 1200 +Set Height 780 +Set FontSize 13 +Set Padding 24 +Set Shell "bash" +Set TypingSpeed 50ms + +Require cargo + +Type "# AC-187-006/007/008/009: 10-byte minimum, defensive 0x32 reject, Job/Userdata extraction, ROSCTR totality" +Enter +Sleep 300ms + +Type "cargo test --test s7comm_analyzer_tests BC_2_21_004 2>&1 | grep -E 'story_187::test|test result:'" +Enter +Wait +Sleep 500ms + +Type "cargo test --test s7comm_analyzer_tests BC_2_21_005 2>&1 | grep -E 'story_187::test|test result:'" +Enter +Wait +Sleep 500ms + +Type "cargo test --test s7comm_analyzer_tests BC_2_21_006 2>&1 | grep -E 'story_187::test|test result:'" +Enter +Wait +Sleep 500ms + +Type "cargo test --test s7comm_analyzer_tests BC_2_21_007 2>&1 | grep -E 'story_187::test|test result:'" +Enter +Wait +Sleep 500ms + +Type "cargo test --test s7comm_analyzer_tests bc_2_21_007 2>&1 | grep -E 'story_187::proptest|test result:'" +Enter +Wait +Sleep 2s diff --git a/docs/demo-evidence/STORY-187/AC-006-009-length-rosctr-parser.webm b/docs/demo-evidence/STORY-187/AC-006-009-length-rosctr-parser.webm new file mode 100644 index 00000000..b7cf0fce Binary files /dev/null and b/docs/demo-evidence/STORY-187/AC-006-009-length-rosctr-parser.webm differ diff --git a/docs/demo-evidence/STORY-187/AC-010-011-ack-bounds-check.gif b/docs/demo-evidence/STORY-187/AC-010-011-ack-bounds-check.gif new file mode 100644 index 00000000..f33666a7 Binary files /dev/null and b/docs/demo-evidence/STORY-187/AC-010-011-ack-bounds-check.gif differ diff --git a/docs/demo-evidence/STORY-187/AC-010-011-ack-bounds-check.tape b/docs/demo-evidence/STORY-187/AC-010-011-ack-bounds-check.tape new file mode 100644 index 00000000..d4ae1d43 --- /dev/null +++ b/docs/demo-evidence/STORY-187/AC-010-011-ack-bounds-check.tape @@ -0,0 +1,77 @@ +# STORY-187 AC-187-010/011: ROSCTR=Ack/Ack_Data 12-byte header + error_class/ +# error_code extraction, truncated-Ack(_Data) rejection, and declared +# param_length/data_length bounds-checking before slice access (BC-2.21.008/009) +# +# Demonstrates: +# - test_BC_2_21_008_ack_rosctr_12_byte_minimum_and_error_fields / +# test_BC_2_21_008_ack_data_12_byte_header_and_error_fields / +# test_BC_2_21_008_canonical_ack_vector_verbatim / +# test_BC_2_21_008_error_fields_none_for_job_and_userdata_rosctr / +# test_BC_2_21_008_ack_data_nonzero_error_fields_with_parameter_block: Ack AND +# Ack_Data both require 12 bytes, error_class/error_code extracted, Some only +# for Ack/AckData (AC-187-010 -- SUCCESS PATH) +# - test_BC_2_21_008_truncated_ack_data_returns_none / +# test_BC_2_21_008_truncated_ack_on_data_emits_t0814_once / +# test_BC_2_21_008_truncated_ack_data_on_data_emits_t0814_once: 10/11-byte +# truncated Ack and Ack_Data both return None with reason-specific T0814 +# evidence, distinguishable from each other and from AC-187-006/009's reasons +# (AC-187-010 -- ERROR PATH) +# - test_BC_2_21_009_bounds_check_before_parameter_data_slice / +# test_BC_2_21_009_bounds_check_dedup_s2c / +# test_BC_2_21_009_ack_header_len_12_bounds_check / +# test_BC_2_21_009_data_length_overrun_on_data_emits_t0814 / +# test_BC_2_21_009_dissection_bounded_to_own_tpkt_frame / +# test_BC_2_21_009_bounds_failure_evidence_reports_declared_and_available / +# test_BC_2_21_009_bounds_failure_evidence_ack_data_header_len_12: declared +# param_length/data_length bounds-checked before any slice access; never +# borrows bytes from a trailing frame; per-direction dedup verified for BOTH +# c2s and s2c (AC-187-011 -- ERROR PATH) +# - test_BC_2_21_009_bounds_check_passes_exact_match / +# test_BC_2_21_009_empty_parameter_and_data_blocks_trivial_pass / +# test_BC_2_21_009_overflow_free_arithmetic_max_values / +# test_BC_2_21_009_s7comm_bounds_ok_data_length_only_overrun / +# test_BC_2_21_009_s7comm_bounds_ok_helper_matches_bounds_decision: bounds +# check passes on exact-fit data and never overflows (AC-187-011 -- SUCCESS +# PATH) +# - proptest_bc_2_21_006_008_some_iff_rosctr_and_length_conditional: joint +# length-conditional totality across BC-2.21.006/007/008, exhaustive over all +# 256 ROSCTR byte values and lengths [9,10,11,12,13] +# +# Traces to: BC-2.21.008 postconditions 1-4, edge case EC-007; BC-2.21.009 +# postconditions 1-3 +# Test: cargo test --test s7comm_analyzer_tests BC_2_21_008 +# Test: cargo test --test s7comm_analyzer_tests BC_2_21_009 +# Test: cargo test --test s7comm_analyzer_tests bc_2_21_006_008 + +Output AC-010-011-ack-bounds-check.gif +Output AC-010-011-ack-bounds-check.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 "# AC-187-010/011: Ack/Ack_Data 12-byte header extraction + bounds-check before slice access" +Enter +Sleep 300ms + +Type "cargo test --test s7comm_analyzer_tests BC_2_21_008 2>&1 | grep -E 'story_187::test|test result:'" +Enter +Wait +Sleep 500ms + +Type "cargo test --test s7comm_analyzer_tests BC_2_21_009 2>&1 | grep -E 'story_187::test|test result:'" +Enter +Wait +Sleep 500ms + +Type "cargo test --test s7comm_analyzer_tests bc_2_21_006_008 2>&1 | grep -E 'story_187::proptest|test result:'" +Enter +Wait +Sleep 2s diff --git a/docs/demo-evidence/STORY-187/AC-010-011-ack-bounds-check.webm b/docs/demo-evidence/STORY-187/AC-010-011-ack-bounds-check.webm new file mode 100644 index 00000000..4c102cb7 Binary files /dev/null and b/docs/demo-evidence/STORY-187/AC-010-011-ack-bounds-check.webm differ diff --git a/docs/demo-evidence/STORY-187/AC-013-canonical-frames.gif b/docs/demo-evidence/STORY-187/AC-013-canonical-frames.gif new file mode 100644 index 00000000..4e792782 Binary files /dev/null and b/docs/demo-evidence/STORY-187/AC-013-canonical-frames.gif differ diff --git a/docs/demo-evidence/STORY-187/AC-013-canonical-frames.tape b/docs/demo-evidence/STORY-187/AC-013-canonical-frames.tape new file mode 100644 index 00000000..8952584f --- /dev/null +++ b/docs/demo-evidence/STORY-187/AC-013-canonical-frames.tape @@ -0,0 +1,65 @@ +# STORY-187 AC-187-013: canonical, independently-sourced classic S7comm Setup +# Communication frame pair (Job request + Ack_Data response), and the committed +# fixture pcap end-to-end regression (BC-2.21.002/006/008, policy +# DF-CANONICAL-FRAME-HOLDOUT-001) +# +# Demonstrates: +# - test_BC_2_21_006_canonical_setup_communication_job_frame_on_data: canonical +# Setup Communication Job (function code 0xF0) PDU, wrapped in real RFC 1006 +# TPKT + class-0 COTP DT (02 F0 80) framing, dispatches and classifies the +# flow Classic with zero Findings +# - test_BC_2_21_008_canonical_ack_vector_verbatim: the BC-2.21.008 canonical Ack +# 12-byte vector taken verbatim (no byte substituted) +# - test_BC_2_21_008_canonical_ack_data_setup_communication_response_on_data: the +# canonical Ack_Data (0x03) response frame -- header_len == 12, error_class == +# Some(0), error_code == Some(0), parameter block begins at data[12] (corrected +# 2026-09-24 ruling, not the pre-ruling data[10] assumption) +# - test_BC_2_21_002_setup_comm_fixture_pcap_well_formed_no_findings: the +# committed tests/fixtures/s7comm-setup-comm.pcap capture (synthetic, generator- +# produced per ADR-014 Decision 7) read via PcapSource::from_file and fed +# through S7commAnalyzer::on_data end-to-end -- zero Findings, +# session_established == true, classified_protocol == Some(Classic) +# +# Canonical byte-sequence provenance (policy DF-CANONICAL-FRAME-HOLDOUT-001): +# primary source cnblogs, "Siemens S7 Communication Protocol Reference Compilation" +# (https://www.cnblogs.com/crcce-dncs/p/10659087.html), corroborated by two +# independent sources -- Yiqisoft (https://www.yiqisoft.cn/blogs/IoT-Gateway/363.html) +# and the Inductive Automation KB ("Loggers - Device Connections: Siemens") -- +# permitted as test-vector sources per ADR-014 Decision 4's 2026-09-24 +# reconciliation note (STORY-187 adversarial pass 5, F-40, human-ratified). Parser +# design and field semantics continue to derive from Decision 4's prose sources +# (Wireshark wiki, Kleinmann & Wool 2014, Orange-Cyberdefense catalog) and +# permitted design references (icsnpp-s7comm, libs7comm, python-snap7). +# +# Traces to: BC-2.21.002 postcondition 3, BC-2.21.006, BC-2.21.008 postconditions +# 1-2; policy DF-CANONICAL-FRAME-HOLDOUT-001 +# Test: cargo test --test s7comm_analyzer_tests canonical +# Test: cargo test --test s7comm_analyzer_tests fixture_pcap + +Output AC-013-canonical-frames.gif +Output AC-013-canonical-frames.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-187-013: canonical Setup Communication Job/Ack_Data frames + fixture pcap end-to-end" +Enter +Sleep 300ms + +Type "cargo test --test s7comm_analyzer_tests canonical 2>&1 | grep -E 'story_187::test|test result:'" +Enter +Wait +Sleep 500ms + +Type "cargo test --test s7comm_analyzer_tests fixture_pcap 2>&1 | grep -E 'story_187::test|test result:'" +Enter +Wait +Sleep 2s diff --git a/docs/demo-evidence/STORY-187/AC-013-canonical-frames.webm b/docs/demo-evidence/STORY-187/AC-013-canonical-frames.webm new file mode 100644 index 00000000..566eb1cc Binary files /dev/null and b/docs/demo-evidence/STORY-187/AC-013-canonical-frames.webm differ diff --git a/docs/demo-evidence/STORY-187/AC-ALL-story_187-green.gif b/docs/demo-evidence/STORY-187/AC-ALL-story_187-green.gif new file mode 100644 index 00000000..3662379b Binary files /dev/null and b/docs/demo-evidence/STORY-187/AC-ALL-story_187-green.gif differ diff --git a/docs/demo-evidence/STORY-187/AC-ALL-story_187-green.tape b/docs/demo-evidence/STORY-187/AC-ALL-story_187-green.tape new file mode 100644 index 00000000..34591aa7 --- /dev/null +++ b/docs/demo-evidence/STORY-187/AC-ALL-story_187-green.tape @@ -0,0 +1,36 @@ +# STORY-187 full module: all 63 story_187:: tests green, covering AC-187-001 +# through AC-187-013 (S7commFlowState completion, four-way protocol_id dispatch +# skeleton, and parse_s7comm_header pure-core parser) +# +# This is the top-level artifact showing the complete story_187 test module +# passing in one run -- every AC-187-001..013 test is represented in this output. +# (story_187 is one module inside tests/s7comm_analyzer_tests.rs, which also +# hosts STORY-186's story_186 module -- 83 tests total in the file; this +# recording filters to story_187's 63.) +# +# Traces to: BC-2.21.001, BC-2.21.002, BC-2.21.004, BC-2.21.005, BC-2.21.006, +# BC-2.21.007, BC-2.21.008, BC-2.21.009 +# Test: cargo test --test s7comm_analyzer_tests story_187:: + +Output AC-ALL-story_187-green.gif +Output AC-ALL-story_187-green.webm + +Set FontFamily "Menlo" +Set Theme "Dracula" +Set Width 1200 +Set Height 1080 +Set FontSize 11 +Set Padding 24 +Set Shell "bash" +Set TypingSpeed 30ms + +Require cargo + +Type "# STORY-187: full story_187 test module -- 63/63 green" +Enter +Sleep 300ms + +Type "cargo test --test s7comm_analyzer_tests story_187:: 2>&1 | grep -E 'running [0-9]+ tests|story_187::|test result:'" +Enter +Wait +Sleep 3s diff --git a/docs/demo-evidence/STORY-187/AC-ALL-story_187-green.webm b/docs/demo-evidence/STORY-187/AC-ALL-story_187-green.webm new file mode 100644 index 00000000..14b83ceb Binary files /dev/null and b/docs/demo-evidence/STORY-187/AC-ALL-story_187-green.webm differ diff --git a/docs/demo-evidence/STORY-187/evidence-report.md b/docs/demo-evidence/STORY-187/evidence-report.md new file mode 100644 index 00000000..5329d79c --- /dev/null +++ b/docs/demo-evidence/STORY-187/evidence-report.md @@ -0,0 +1,218 @@ +# Demo Evidence Report — STORY-187 + +**Story:** STORY-187: S7comm Flow State Completion, Four-Way protocol_id Dispatch +Skeleton, and parse_s7comm_header Pure-Core Parser +**Wave:** 90 +**Date:** 2026-09-25 +**Branch:** feature/STORY-187-s7comm-header-dispatch +**Product type:** Library (pure-core parser consumed by the effectful-shell +`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`, +`mod story_187` (63 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 story_187 Module: 63/63 PASS + +Command: +``` +cargo test --test s7comm_analyzer_tests story_187:: +``` + +Result: **63 passed; 0 failed; 0 ignored; 0 measured; 20 filtered out** (the 20 filtered +out are STORY-186's `story_186` module tests in the same file). The full file +(`story_186` + `story_187`) is **83 passed; 0 failed**, finishing in ~4.8s wall clock +(dominated by STORY-186's three VP-050 proptests; every `story_187` test finishes in +well under a millisecond). + +| Artifact | Description | +|----------|-------------| +| `AC-ALL-story_187-green.gif` / `.webm` | Full `story_187` module — 63/63 green | +| `AC-ALL-story_187-green.tape` | VHS script for the full-module run | + +--- + +## AC → Test → Artifact Coverage Map + +| AC | Title | BC | Test(s) | Artifact | Path | Verdict | +|----|-------|-----|---------|----------|------|---------| +| AC-187-001 | `S7commFlowState` carries the full field set required by this story's scope | BC-2.21.001 PC-1, PC-2 | `test_BC_2_21_001_flow_state_field_set`, `test_BC_2_21_001_fields_constructible_with_expected_types` | `AC-001-003-flow-state-session.gif/.webm` | Success | PASS | +| AC-187-002 | `S7commFlowState` is created lazily on first `on_data` call | BC-2.21.001 PC-3 | `test_BC_2_21_001_lazy_flow_state_creation`, `test_BC_2_21_001_never_touched_flow_state_never_created` | `AC-001-003-flow-state-session.gif/.webm` | Success | PASS | +| AC-187-003 | `session_established` is set only by an opposite-direction CC after a CR; five negative sequences + most-recent-CR-wins + monotonicity | BC-2.21.001 PC-1, EC-004..EC-009; BC-2.21.002 PC-2 | `test_BC_2_21_001_cr_then_opposite_cc_sets_session_established`, `test_BC_2_21_001_cr_only_session_not_established`, `test_BC_2_21_001_cc_only_no_prior_cr_session_not_established`, `test_BC_2_21_001_cc_before_cr_session_not_established`, `test_BC_2_21_001_same_direction_cc_session_not_established`, `test_BC_2_21_001_repeated_same_direction_cr_session_not_established`, `test_BC_2_21_001_most_recent_cr_direction_wins`, `test_BC_2_21_001_session_established_is_monotonic`, `test_BC_2_21_001_dedup_flags_independent_c2s` | `AC-001-003-flow-state-session.gif/.webm` | Success (all sequences are non-classifying; no error path — CR/CC session tracking has no malformed-input branch) | PASS | +| AC-187-004 | `Some(0x32)` DT frames dispatch to classic dissection when sticky `classified_protocol` is/becomes Classic; malformed too-short frame on a Classic flow emits exactly one T0814 | BC-2.21.002 PC-3; BC-2.21.004 PC-4 | `test_BC_2_21_002_classic_s7comm_dispatch_asserts_classified_protocol_classic` (success), `test_BC_2_21_002_malformed_0x32_frame_on_classic_flow_emits_exactly_one_t0814` (error) | `AC-004-005-012-dispatch-classification.gif/.webm` | Success + Error | PASS | +| AC-187-005 | First DT frame's `protocol_id: Some(byte)` sets `classified_protocol` exactly once (sticky first-write-wins); `protocol_id: None` never classifies | BC-2.21.002 PC-6, EC-003, EC-004 | `test_BC_2_21_002_sticky_first_classification`, `test_BC_2_21_002_none_protocol_id_dt_first_then_0x32_dt_classifies_classic`, `test_BC_2_21_002_two_frames_one_delivery_first_write_wins`, `test_BC_2_21_002_empty_dt_followed_by_frame_same_delivery_stays_unclassified`, `test_BC_2_21_002_unparseable_cotp_does_not_classify` | `AC-004-005-012-dispatch-classification.gif/.webm` | Success (classifying paths) + Error (unparseable-COTP non-classifying path) | PASS | +| AC-187-006 | `parse_s7comm_header` returns `None` for `data.len() < 10`; no panic; one T0814 per direction with boundary-specific evidence | BC-2.21.004 PC-1, PC-2, PC-4 | `test_BC_2_21_004_parse_returns_none_for_len_lt_10`, `test_BC_2_21_004_len_shorter_than_10_returns_none_and_emits_t0814_once`, `test_BC_2_21_004_len_shorter_than_10_emits_t0814_once_s2c`, `test_BC_2_21_004_nine_byte_payload_on_data_too_short_evidence` | `AC-006-009-length-rosctr-parser.gif/.webm` | Error (too-short header is definitionally the malformed condition) | PASS | +| AC-187-007 | `parse_s7comm_header` defensively rejects `data[0] != 0x32` — no `Finding`, caller-hygiene only | BC-2.21.005 PC-1, PC-3 | `test_BC_2_21_005_defensive_reject_wrong_protocol_id_byte`, `test_BC_2_21_005_defensive_reject_zero_byte` | `AC-006-009-length-rosctr-parser.gif/.webm` | Defensive-reject (no on-wire error path — this is a caller-hygiene contract, not an anomaly) | PASS | +| AC-187-008 | `parse_s7comm_header` extracts common header fields for Job/Userdata at the correct byte offsets; Reserved bytes never gate | BC-2.21.006 PC-1..5 | `test_BC_2_21_006_common_header_field_extraction`, `test_BC_2_21_006_nonzero_reserved_bytes_do_not_reject`, `test_BC_2_21_006_byte_asymmetric_big_endian_decode`, `test_BC_2_21_006_canonical_setup_communication_job_frame_on_data` | `AC-006-009-length-rosctr-parser.gif/.webm` | Success | PASS | +| AC-187-009 | `parse_s7comm_header` returns `None` for unrecognized ROSCTR; totality over all 256 `u8` values; shared dedup flag, s2c independence | BC-2.21.007 PC-1..3 | `test_BC_2_21_007_unrecognized_rosctr_returns_none`, `test_BC_2_21_007_shares_dedup_flag_with_004_malformed_header`, `test_BC_2_21_007_unrecognized_rosctr_emits_t0814_once_s2c`, `proptest_bc_2_21_007_rosctr_byte_totality_over_all_256_values` | `AC-006-009-length-rosctr-parser.gif/.webm` | Error (unrecognized ROSCTR is the malformed condition) + exhaustive totality | PASS | +| AC-187-010 | ROSCTR=Ack AND Ack_Data both require 12 bytes, extract `error_class`/`error_code`; truncated Ack/Ack_Data (10/11 bytes) returns `None` with distinguishable evidence | BC-2.21.008 PC-1..4, EC-007 | `test_BC_2_21_008_ack_rosctr_12_byte_minimum_and_error_fields`, `test_BC_2_21_008_canonical_ack_vector_verbatim`, `test_BC_2_21_008_ack_data_12_byte_header_and_error_fields`, `test_BC_2_21_008_error_fields_none_for_job_and_userdata_rosctr`, `test_BC_2_21_008_ack_data_nonzero_error_fields_with_parameter_block` (success); `test_BC_2_21_008_truncated_ack_data_returns_none`, `test_BC_2_21_008_truncated_ack_on_data_emits_t0814_once`, `test_BC_2_21_008_truncated_ack_data_on_data_emits_t0814_once` (error) | `AC-010-011-ack-bounds-check.gif/.webm` | Success + Error | PASS | +| AC-187-011 | Declared `param_length`/`data_length` bounds-checked before any slice access; dedup verified for both flow directions; never borrows bytes from a trailing frame | BC-2.21.009 PC-1..3 | `test_BC_2_21_009_bounds_check_before_parameter_data_slice`, `test_BC_2_21_009_bounds_check_dedup_s2c`, `test_BC_2_21_009_ack_header_len_12_bounds_check`, `test_BC_2_21_009_data_length_overrun_on_data_emits_t0814`, `test_BC_2_21_009_dissection_bounded_to_own_tpkt_frame`, `test_BC_2_21_009_bounds_failure_evidence_reports_declared_and_available`, `test_BC_2_21_009_bounds_failure_evidence_ack_data_header_len_12` (error); `test_BC_2_21_009_bounds_check_passes_exact_match`, `test_BC_2_21_009_empty_parameter_and_data_blocks_trivial_pass`, `test_BC_2_21_009_overflow_free_arithmetic_max_values`, `test_BC_2_21_009_s7comm_bounds_ok_data_length_only_overrun`, `test_BC_2_21_009_s7comm_bounds_ok_helper_matches_bounds_decision`, `proptest_bc_2_21_006_008_some_iff_rosctr_and_length_conditional` (success) | `AC-010-011-ack-bounds-check.gif/.webm` | Success + Error | PASS | +| AC-187-012 | Classic dissection is gated on sticky `classified_protocol == Classic` **AND** current frame `protocol_id == Some(0x32)` — a conjunction, neither alone sufficient | BC-2.21.002 PC-3, Inv-4, EC-005 | `test_BC_2_21_002_0x32_dt_frame_not_dissected_when_sticky_classified_plus_or_unclassified`, `test_BC_2_21_002_non_0x32_dt_frame_on_classic_flow_not_dissected` | `AC-004-005-012-dispatch-classification.gif/.webm` | Negative-gate proof (both are "must NOT dissect" assertions — the gate's own error/reject path) | PASS | +| AC-187-013 | Canonical, independently-sourced classic S7comm Setup Communication frame pair (Job + Ack_Data) and committed fixture pcap validate parser and dispatch | BC-2.21.002 PC-3; BC-2.21.006; BC-2.21.008 PC-1-2; policy DF-CANONICAL-FRAME-HOLDOUT-001 | `test_BC_2_21_006_canonical_setup_communication_job_frame_on_data`, `test_BC_2_21_008_canonical_ack_data_setup_communication_response_on_data`, `test_BC_2_21_002_setup_comm_fixture_pcap_well_formed_no_findings` | `AC-013-canonical-frames.gif/.webm` | Success (all three assert zero `Finding`s on well-formed canonical/fixture input — AC-187-013 has no malformed-input scenario of its own; the malformed-header error paths for the SAME byte layouts are covered under AC-187-006/009/010/011) | PASS | + +**All 13 acceptance criteria (AC-187-001..013) are covered by at least one recorded +artifact.** `proptest_vp053_protocol_id_dispatch_totality` (VP-053) is additionally +captured in `AC-004-005-012-dispatch-classification.gif/.webm` as supporting evidence +for AC-187-005's four-way classification table. + +### Note on artifact overlap + +`test_BC_2_21_002_setup_comm_fixture_pcap_well_formed_no_findings` (AC-187-013) shares +BC-2.21.002's dispatch path with AC-187-004/005/012, so the `BC_2_21_002` filter used +for `AC-004-005-012-dispatch-classification` also happens to catch it; likewise +`test_BC_2_21_006_canonical_setup_communication_job_frame_on_data` (AC-187-013) and +`test_BC_2_21_008_canonical_ack_vector_verbatim`/ +`test_BC_2_21_008_canonical_ack_data_setup_communication_response_on_data` appear +inside the broader `BC_2_21_006`/`BC_2_21_008` filters used for +`AC-006-009-length-rosctr-parser` and `AC-010-011-ack-bounds-check` respectively. This +is intentional context, not a substitute for AC-187-013's own dedicated recording — the +coverage map above cites `AC-013-canonical-frames.gif/.webm` as the primary artifact for +AC-187-013 because it is the only recording that isolates all three of that AC's tests +together with their source-provenance commentary. + +--- + +## Error-Path Coverage Summary + +Per-story convention (matching STORY-186), the five malformed-header conditions that +share the `malformed_header_reported_c2s`/`_s2c` dedup flag are each an explicit error +path, distinguished by reason-specific evidence text: + +| Reason | AC | Error-path test(s) | +|--------|----|--------------------| +| Header shorter than 10 bytes | AC-187-006 | `test_BC_2_21_004_len_shorter_than_10_returns_none_and_emits_t0814_once`, `test_BC_2_21_004_len_shorter_than_10_emits_t0814_once_s2c`, `test_BC_2_21_004_nine_byte_payload_on_data_too_short_evidence` | +| Unrecognized ROSCTR byte | AC-187-009 | `test_BC_2_21_007_unrecognized_rosctr_emits_t0814_once_s2c`, `test_BC_2_21_007_shares_dedup_flag_with_004_malformed_header` | +| Truncated Ack (10/11 bytes) | AC-187-010 | `test_BC_2_21_008_truncated_ack_on_data_emits_t0814_once` | +| Truncated Ack_Data (10/11 bytes) | AC-187-010 | `test_BC_2_21_008_truncated_ack_data_on_data_emits_t0814_once` | +| Bounds-check failure (`param_length`/`data_length` exceed available bytes) | AC-187-011 | `test_BC_2_21_009_bounds_check_before_parameter_data_slice`, `test_BC_2_21_009_bounds_check_dedup_s2c`, `test_BC_2_21_009_data_length_overrun_on_data_emits_t0814`, `test_BC_2_21_009_bounds_failure_evidence_reports_declared_and_available`, `test_BC_2_21_009_bounds_failure_evidence_ack_data_header_len_12` | + +Every one of these five reason classes is exercised for BOTH flow directions +independently (c2s and s2c dedup verified separately, per the story's explicit +"dedup verified for both flow directions" language in AC-187-006/009/011), and the +shared `assert_reason_specific_evidence` test helper confirms the five reasons remain +textually distinguishable from one another. + +AC-187-001/002/003/007/012/013 have no on-wire "anomaly" error path of their own (CR/CC +session bookkeeping, the `data[0] != 0x32` caller-hygiene defensive-reject, the +classification gate's negative case, and the canonical/fixture well-formed-input checks +are all non-`Finding`-emitting by design) — their negative/defensive assertions are +covered as noted in the coverage map above. + +--- + +## VP-051 Kani Obligation (executed locally, supporting evidence — no recording) + +Per the story's VP-051 obligation, two separate `#[kani::proof]` harnesses are anchored +in `tests/s7comm_analyzer_tests.rs`'s `#[cfg(kani)] mod vp051_kani`: + +- `verify_parse_s7comm_header_bounds_safety` — header-extraction half (bounded symbolic + `data: [u8; 16]` + `len`); `Some`/`None` totality and positive field-extraction + obligations. +- `verify_s7comm_bounds_ok_bounds_safety` — bounds-check half (independent symbolic + `data_len: usize` against `s7comm_bounds_ok`); exact-equality and no-panic + obligations. + +Both harnesses were executed locally with `cargo kani` and returned **VERIFICATION +SUCCESSFUL** for both proofs. This is text-only supporting evidence per the demo- +recording scope — Kani symbolic execution output is not a VHS-recordable interactive +CLI demo, and formal-verification artifacts are the `formal-verifier` agent's +deliverable, not the demo-recorder's. No recording was produced for this obligation. + +## cargo-mutants Results (supporting evidence — no recording) + +Mutation testing for this story's scope reported **43 mutants generated**: all viable, +non-equivalent mutants were killed by the test suite; 1 mutant was classified equivalent +(no test can distinguish it from correct behavior); 2 mutants were unviable (did not +compile). This is text-only supporting evidence per the demo-recording scope — mutation +testing summary tables are the `formal-verifier` agent's deliverable, not a VHS- +recordable interactive CLI demo. No recording was produced for this obligation. + +--- + +## Canonical-Frame Provenance (AC-187-013, policy DF-CANONICAL-FRAME-HOLDOUT-001) + +The TPKT length field, the `02 F0 80` COTP DT header bytes, and the S7comm common- +header/Setup-Communication-parameter bytes used in `test_BC_2_21_006_canonical_setup_communication_job_frame_on_data` +and `test_BC_2_21_008_canonical_ack_data_setup_communication_response_on_data` are NOT +derived from this story's own behavioral contracts, ADR-014, or any other project +artifact. Sourcing (STORY-187 per-story adversarial pass 5, F-40, human ruling +2026-09-24): + +- **Primary citation:** cnblogs, "西门子S7通讯协议引用整理" + (https://www.cnblogs.com/crcce-dncs/p/10659087.html) — Setup Communication + request/response byte sequence, including the full 27-byte Ack_Data response frame + `03 00 00 1B 02 F0 80 32 03 00 00 FF FF 00 08 00 00 00 00 F0 00 00 01 00 01 00 F0`. +- **Corroborating sources (test-vector sourcing only):** Yiqisoft + (https://www.yiqisoft.cn/blogs/IoT-Gateway/363.html) and the Inductive Automation KB + ("Loggers - Device Connections: Siemens"). + +Per ADR-014 Decision 4's 2026-09-24 reconciliation note, publicly posted wire-capture +byte examples are permitted as **test-vector sources only** — parser design and field +semantics for this story continue to derive exclusively from Decision 4's prose sources +(Wireshark wiki, Kleinmann & Wool 2014, Orange-Cyberdefense catalog) and permitted +design references (cisagov/icsnpp-s7comm BSD-3, kprovost/libs7comm BSD-2, +gijzelaerr/python-snap7 MIT). This satisfies DF-CANONICAL-FRAME-HOLDOUT-001, which +requires the canonical byte sequences themselves (not the parser's design) to be +sourced independently of the project's own specification artifacts. + +The committed `tests/fixtures/s7comm-setup-comm.pcap` capture is a distinct, synthetic +artifact — generated by `tests/fixtures/mk_s7comm_pcap.py` per ADR-014 Decision 7 — and +is NOT one of the independently-sourced canonical byte literals; it is a regression +fixture built to encode the corrected 12-byte Ack_Data header shape. + +--- + +## 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 (and following the STORY-186 +precedent in `docs/demo-evidence/STORY-186/`), 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_187::` 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 Path-Scrub Gate below). + +Six recordings were produced: five per behavior group (matching the task's suggested AC +groupings, with AC-187-006..011 split across two recordings to keep each recording +readable) plus one top-level full-`story_187`-module run. + +| Artifact | Behavior group | ACs covered | Tests | +|----------|-----------------|-------------|-------| +| `AC-001-003-flow-state-session` | Flow state field set + lazy creation + CR/CC session tracking | AC-187-001, 002, 003 | 13 | +| `AC-004-005-012-dispatch-classification` | protocol_id dispatch + sticky first-classification-wins + conjunctive gating | AC-187-004, 005, 012 | 14 | +| `AC-006-009-length-rosctr-parser` | 10-byte minimum + defensive 0x32 reject + Job/Userdata extraction + ROSCTR totality | AC-187-006, 007, 008, 009 | 14 | +| `AC-010-011-ack-bounds-check` | Ack/Ack_Data 12-byte header + error fields + param/data bounds-check | AC-187-010, 011 | 22 | +| `AC-013-canonical-frames` | Canonical Setup Communication frame pair + fixture pcap | AC-187-013 | 4 | +| `AC-ALL-story_187-green` | Full `story_187` module | AC-187-001..013 | 63 | + +(Per-recording test counts sum to more than 63 total for the five per-group recordings +because several tests are legitimately relevant to more than one recording's filter, as +noted in "Note on artifact overlap" above; the `AC-ALL-story_187-green` recording is the +authoritative de-duplicated 63-test count.) + +--- + +## Path-Scrub Gate (PG-W70-DEMO-SCRUB) + +Gate command run from the repo root, per +`.factory/maintenance/demo-evidence-scrub-gate.md` (the mandatory absolute-host-path / +tilde-form-home-path grep, applied to this story's evidence directory). + +**Result: zero matches.** All `.tape` files and this report use only repo-relative +paths (`tests/fixtures/...`, `src/analyzer/s7comm.rs`) and the `cargo test --test +s7comm_analyzer_tests 2>&1 | grep -E '...'` command form, which suppresses +cargo's own `Running tests/... (/target/debug/deps/...)` line (the only +place an absolute host path would otherwise leak into rendered GIF/webm text). (Note: +the gate command itself is not reproduced verbatim in this report, since its own regex +literal would trip the same grep it documents — see +`.factory/maintenance/demo-evidence-scrub-gate.md` for the exact command.) + +## Doc-Tense Check + +`python3 bin/check-green-doc-tense` was run against this report; see the commit-time +tool output for the pass/fail result recorded in this delivery's PR description. diff --git a/src/analyzer/s7comm.rs b/src/analyzer/s7comm.rs index 787e4d64..82058a62 100644 --- a/src/analyzer/s7comm.rs +++ b/src/analyzer/s7comm.rs @@ -23,14 +23,17 @@ //! `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) +//! ## Scope //! -//! 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`. +//! STORY-186 proved TPKT/COTP frame extraction, carry-buffer reassembly, and +//! 1-byte resync. STORY-187 builds on that: `on_data`'s frame-walk loop now +//! interprets `CotpHeader::protocol_id` via the four-way dispatch (BC-2.21.002) — +//! CR/CC opposite-direction session tracking (F-01), sticky first-`Some(byte)`-wins +//! protocol classification (F-02), and classic S7comm (`0x32`) header dissection +//! gated on the flow's sticky `classified_protocol == Classic` (F-12) are fully +//! wired. The `Some(0x72)` (S7comm-plus) and unrecognized/`None`-`protocol_id` +//! branches remain deliberate, panic-free structural no-ops — their observable +//! behavior is STORY-190's scope. //! //! ## Behavioral contracts //! - BC-2.20.013: TPKT frames spanning TCP segment boundaries are reassembled via @@ -42,8 +45,27 @@ //! - 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.001: `S7commFlowState` owns TPKT/COTP carry buffers, S7comm +//! classification state (`classified_protocol`, `cr_observed_dir`, +//! `session_established`), and per-direction malformed-header dedup flags. +//! - BC-2.21.002: `S7commAnalyzer::on_data` four-way dispatch on +//! `CotpHeader::protocol_id` — CR/CC session tracking, sticky +//! first-classification-wins, and the classic-dissection sticky-classification +//! gate (F-12). //! - 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. +//! - BC-2.21.004: `parse_s7comm_header` returns `None` for input shorter than 10 +//! bytes. +//! - BC-2.21.005: `parse_s7comm_header` defensively rejects `data[0] != 0x32`. +//! - BC-2.21.006: `parse_s7comm_header` extracts the common header fields (ROSCTR, +//! PDU reference, parameter/data length) for Job/Userdata (10-byte header). +//! - BC-2.21.007: `parse_s7comm_header` returns `None` for an unrecognized ROSCTR +//! byte, no force-fit. +//! - BC-2.21.008: `parse_s7comm_header` for ROSCTR ∈ {Ack, Ack_Data} requires 12 +//! bytes and extracts `error_class`/`error_code` (2026-09-24 canonical-frame +//! holdout ruling, DF-CANONICAL-FRAME-HOLDOUT-001). +//! - BC-2.21.009: declared `param_length`/`data_length` are bounds-checked (via the +//! pure [`s7comm_bounds_ok`] helper) before any parameter/data-block slice. use std::collections::HashMap; @@ -80,13 +102,14 @@ 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. +/// Per-flow state for [`S7commAnalyzer`] — carry-buffer fields (STORY-186) plus the +/// S7comm classification state and malformed-header dedup flags this story +/// (STORY-187) adds (BC-2.21.001). /// -/// 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). +/// Per BC-2.20.016 postcondition 3, the 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 @@ -103,6 +126,224 @@ pub struct S7commFlowState { /// 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, + /// Set when a COTP CR (BC-2.20.007) is followed by a matching CC (BC-2.20.008) on + /// this flow (BC-2.21.001 postcondition 1). Classification of the upper-layer + /// protocol is deferred to the first DT frame regardless of this flag's value + /// (BC-2.21.002 postcondition 2). + pub session_established: bool, + /// Records the direction of the most recently observed COTP CR on this flow + /// (overwritten by each subsequent CR), so a later CC can be tested for + /// direction-opposite-ness against it (BC-2.21.001 postcondition 1's + /// `cr_observed_dir` "at minimum" field permission, F-01 ruling, human-ratified + /// 2026-09-24). `None` until the first CR is observed on this flow. Once set, it + /// is never cleared by a matching opposite-direction CC — `dispatch_cotp_frame`'s + /// `ConnectConfirm` arm only sets `session_established` and never writes back to + /// this field, so it continues to reflect the most recent CR even after a + /// successful CR/CC match. + pub cr_observed_dir: Option, + /// Set at most once, on the first DT frame observed for this flow whose + /// `protocol_id` is `Some(byte)` — sticky first-classification-wins + /// (BC-2.21.002 postcondition 6, BC-2.21.001 edge case EC-002). A `protocol_id: + /// None` DT frame carries no protocol evidence and never consumes "first DT + /// frame" status (F-02 ruling, human-ratified 2026-09-24) — this field remains + /// `None` until a DT frame with `Some(byte)` is observed, however many + /// `protocol_id: None` DT frames precede it. + pub classified_protocol: Option, + /// Set once a malformed classic S7comm header (BC-2.21.004/007/008/009) has been + /// reported for the client-to-server direction on this flow, so repeated + /// malformed-header conditions in this direction do not each emit a new T0814 + /// finding. Distinct from `carry_overflow_reported_c2s` — the two dedup flags + /// track independent anomaly classes (BC-2.21.001 invariant 2). + pub malformed_header_reported_c2s: bool, + /// Set once a malformed classic S7comm header has been reported for the + /// server-to-client direction on this flow. Independent of + /// `malformed_header_reported_c2s` (BC-2.21.001 invariant 2). + pub malformed_header_reported_s2c: bool, +} + +// --------------------------------------------------------------------------- +// S7comm data model (STORY-187) +// --------------------------------------------------------------------------- + +/// Which upper-layer protocol a flow's COTP DT frames have been classified as, per +/// the `CotpHeader::protocol_id` four-way dispatch (BC-2.21.002, ADR-014 Decision 2). +/// +/// `Plus` and `Unclassified` are populated by this story's dispatch skeleton but are +/// not yet fully driven — full behavior for those two branches lands in STORY-190. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum S7Protocol { + /// `protocol_id == Some(0x32)` — classic S7comm. + Classic, + /// `protocol_id == Some(0x72)` — S7comm-plus. + Plus, + /// `protocol_id` is `Some(byte)` for a byte other than `0x32`/`0x72` on a DT + /// frame — unclassified gap (BC-2.21.027). A `protocol_id: None` DT frame never + /// classifies at all (F-02 ruling) and is never represented by this variant — + /// see [`S7commFlowState::classified_protocol`]. + Unclassified, +} + +/// The four classic-S7comm ROSCTR ("Remote Operating Service Control") values this +/// story models (BC-2.21.006/007/008), mirroring `CotpTpduType`'s +/// exhaustive-but-bounded design (BC-2.20.011) — this is not exhaustive over all 256 +/// `u8` values by design (BC-2.21.007 invariant 2). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Rosctr { + /// `0x01` — Job (request). + Job, + /// `0x02` — Ack (bare acknowledgment; requires the 12-byte extended header, + /// BC-2.21.008). + Ack, + /// `0x03` — Ack_Data (response carrying a parameter/data block; also requires + /// the 12-byte extended header, BC-2.21.008, 2026-09-24 canonical-frame holdout + /// ruling DF-CANONICAL-FRAME-HOLDOUT-001). + AckData, + /// `0x07` — Userdata. + Userdata, +} + +/// Parsed classic S7comm (protocol-ID `0x32`) common header, as extracted by +/// [`parse_s7comm_header`] (BC-2.21.004 through BC-2.21.008). +/// +/// Frozen per ADR-014 Decision 9 item 3's pure-core free-fn design — this is the +/// exact field set BC-2.21.006/008 define, no additional fields. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct S7commHeader { + /// The ROSCTR value extracted from `data[1]`. + pub rosctr: Rosctr, + /// `u16::from_be_bytes([data[4], data[5]])` (BC-2.21.006 postcondition 2). + pub pdu_reference: u16, + /// `u16::from_be_bytes([data[6], data[7]])` (BC-2.21.006 postcondition 3). Not yet + /// validated against the actual remaining bytes in `data` — that is the caller's + /// BC-2.21.009 obligation. + pub param_length: u16, + /// `u16::from_be_bytes([data[8], data[9]])` (BC-2.21.006 postcondition 4). Not yet + /// validated against the actual remaining bytes in `data` — that is the caller's + /// BC-2.21.009 obligation. + pub data_length: u16, + /// `Some(data[10])` only when `rosctr ∈ {Ack, AckData}`; `None` for Job/Userdata + /// (BC-2.21.008 postcondition 3, 2026-09-24 canonical-frame holdout ruling + /// DF-CANONICAL-FRAME-HOLDOUT-001). + pub error_class: Option, + /// `Some(data[11])` only when `rosctr ∈ {Ack, AckData}`; `None` for Job/Userdata + /// (BC-2.21.008 postcondition 3, 2026-09-24 canonical-frame holdout ruling + /// DF-CANONICAL-FRAME-HOLDOUT-001). + pub error_code: Option, + /// `10` for Job/Userdata (BC-2.21.006); `12` for Ack/Ack_Data (BC-2.21.008, + /// 2026-09-24 canonical-frame holdout ruling DF-CANONICAL-FRAME-HOLDOUT-001). + pub header_len: usize, +} + +// --------------------------------------------------------------------------- +// Pure-core parser (STORY-187) +// --------------------------------------------------------------------------- + +/// Parses the classic S7comm (protocol-ID `0x32`) common header from `data`, the COTP +/// DT payload slice beginning **at** the already-classified protocol-ID byte +/// (BC-2.21.004 precondition 1, `&tpkt_payload[payload_offset..]` from BC-2.20.009). +/// +/// Pure-core free function (ADR-014 Decision 9 item 3) — no I/O, no global state +/// mutation, no side effects, deterministic; MUST NOT read `S7commAnalyzer`'s flow +/// state (this story's Forbidden Dependencies). +/// +/// # Contract summary (BC-2.21.004 through BC-2.21.008; Ack_Data grouping per the +/// 2026-09-24 canonical-frame holdout ruling, DF-CANONICAL-FRAME-HOLDOUT-001) +/// +/// - `data.len() < 10` -> `None` (BC-2.21.004). +/// - `data[0] != 0x32` -> `None` (BC-2.21.005, defensive caller-hygiene guard). +/// - `data[1] ∈ {0x01, 0x07}` (Job/Userdata) -> `Some(S7commHeader { .., +/// error_class: None, error_code: None, header_len: 10 })` (BC-2.21.006). +/// - `data[1] ∈ {0x02, 0x03}` (Ack/Ack_Data) -> requires `data.len() >= 12`; +/// `Some(S7commHeader { .., error_class: Some(data[10]), error_code: +/// Some(data[11]), header_len: 12 })`, else `None` (BC-2.21.008). +/// - `data[1] ∉ {0x01, 0x02, 0x03, 0x07}` -> `None`, no force-fit (BC-2.21.007). +/// +pub fn parse_s7comm_header(data: &[u8]) -> Option { + // BC-2.21.004: 10-byte minimum guard. No bytes beyond this check are accessed + // for any data.len() in [0, 9]. + if data.len() < 10 { + return None; + } + // BC-2.21.005: defensive re-check of the protocol-ID byte (caller-hygiene, not + // a wire-observable anomaly — no Finding is emitted for this path). + if data[0] != 0x32 { + return None; + } + + let pdu_reference = u16::from_be_bytes([data[4], data[5]]); + let param_length = u16::from_be_bytes([data[6], data[7]]); + let data_length = u16::from_be_bytes([data[8], data[9]]); + + match data[1] { + // BC-2.21.006: Job / Userdata — common 10-byte header (Ack_Data moved to the + // 12-byte Ack/Ack_Data group below per the 2026-09-24 canonical-frame + // holdout ruling, DF-CANONICAL-FRAME-HOLDOUT-001). ROSCTR mapped directly in + // each outer match arm (F-16) — no inner re-match, no panic site anywhere in + // this pure parser. + 0x01 => Some(S7commHeader { + rosctr: Rosctr::Job, + pdu_reference, + param_length, + data_length, + error_class: None, + error_code: None, + header_len: 10, + }), + 0x07 => Some(S7commHeader { + rosctr: Rosctr::Userdata, + pdu_reference, + param_length, + data_length, + error_class: None, + error_code: None, + header_len: 10, + }), + // BC-2.21.008 (2026-09-24 canonical-frame holdout ruling, + // DF-CANONICAL-FRAME-HOLDOUT-001): Ack AND Ack_Data both require the + // 12-byte extended header (10-byte common header + error class/code) — a + // real-world Ack_Data (Setup Communication response) parameter block only + // aligns at byte 12, not byte 10. Ack_Data is grouped here with Ack, not + // with Job/Userdata's 10-byte group above. + 0x02 | 0x03 => { + if data.len() < 12 { + return None; + } + let rosctr = if data[1] == 0x02 { + Rosctr::Ack + } else { + Rosctr::AckData + }; + Some(S7commHeader { + rosctr, + pdu_reference, + param_length, + data_length, + error_class: Some(data[10]), + error_code: Some(data[11]), + header_len: 12, + }) + } + // BC-2.21.007: unrecognized ROSCTR byte — safe-reject, no force-fit. + _ => None, + } +} + +/// BC-2.21.009 / F-14: pure, public (`pub fn`) caller-side bounds check — `true` iff +/// `data_len >= header.header_len + header.param_length as usize + header.data_length +/// as usize` (checked addition; the sum cannot overflow `usize` on any wirerust +/// target, BC-2.21.009 invariant 1). Extracted as a standalone `pub fn` (rather than +/// inlined at the `on_data` call site only) so the VP-051 Kani harness can call it +/// directly, independent of the effectful `S7commAnalyzer::dispatch_classic_s7comm` +/// call site (human ruling, STORY-187 per-story adversarial pass 1, F-14, +/// 2026-09-24). +/// +/// Pure-core free function: no I/O, no global state, no side effects. +pub fn s7comm_bounds_ok(header: &S7commHeader, data_len: usize) -> bool { + let declared_total = header + .header_len + .checked_add(header.param_length as usize) + .and_then(|sum| sum.checked_add(header.data_length as usize)); + matches!(declared_total, Some(total) if data_len >= total) } // --------------------------------------------------------------------------- @@ -113,14 +354,17 @@ pub struct S7commFlowState { /// 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. +/// STORY-193's scope) — TPKT/COTP frame extraction, carry management, and resync +/// are proven in isolation, and `protocol_id` dispatch (session tracking, sticky +/// classification, and gated classic-header dissection) is wired on top of it. #[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). + /// Findings accumulated across all flows processed by this analyzer: the T0814 + /// carry-overflow finding (BC-2.20.014 postcondition 3) and the T0814 + /// malformed classic-S7comm-header finding (parse failure or bounds-check + /// failure, BC-2.21.004/007/008/009, F-11). pub findings: Vec, } @@ -152,8 +396,11 @@ impl S7commAnalyzer { /// [`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. + /// Each extracted frame's `CotpHeader` is then routed through the BC-2.21.002 + /// four-way dispatch (see [`Self::dispatch_cotp_frame`]): CR/CC frames update + /// session-tracking state, and Data Transfer frames drive sticky protocol + /// classification and, for classic (`0x32`) S7comm on a sticky-Classic flow, + /// header dissection via [`parse_s7comm_header`]. pub fn on_data(&mut self, flow_key: FlowKey, data: &[u8], ts: u32, direction: Direction) { use crate::findings::{Confidence, ThreatCategory, Verdict}; @@ -266,12 +513,21 @@ impl S7commAnalyzer { 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. + // Complete TPKT frame: dispatch to parse_cotp_header, then + // to this story's four-way protocol_id dispatch skeleton + // (BC-2.21.002), and advance past it (BC-2.20.013 + // postcondition 1a). let frame = &working[cursor..cursor + total]; - let _ = iso_on_tcp::parse_cotp_header(&frame[4..]); + let tpkt_payload = &frame[4..]; + let cotp = iso_on_tcp::parse_cotp_header(tpkt_payload); + Self::dispatch_cotp_frame( + state, + cotp, + tpkt_payload, + direction, + ts, + &mut local_findings, + ); cursor += total; } else { // Declared-but-incomplete: stash the entire partial frame @@ -331,4 +587,300 @@ impl S7commAnalyzer { } cursor } + + /// BC-2.21.002 four-way dispatch on `parse_cotp_header`'s return value — the + /// single integration point between SS-20's frame extraction and SS-21's + /// protocol-specific dissection. + /// + /// `tpkt_payload` is `frame[4..]` (the bytes `parse_cotp_header` was called on); + /// `cotp` is that call's result. + /// + /// The CR/CC session-tracking branch (BC-2.21.002 postcondition 2) updates + /// `session_established` only on a CC observed in the direction OPPOSITE a + /// previously-recorded CR on this flow (F-01 ruling, BC-2.21.001 + /// postcondition 1) — a bare CR, a CC with no prior CR, an out-of-order CC, and + /// a same-direction CC all leave it untouched. The sticky-first-classification + /// assignment (BC-2.21.002 postcondition 6) for DT frames is fully wired via + /// [`Self::classify_first_dt_frame`] (AC-187-005; a `protocol_id: None` DT + /// frame never classifies, F-02). The `Some(0x32)` classic branch is fully + /// wired to [`Self::dispatch_classic_s7comm`], but gated on the flow's STICKY + /// `classified_protocol == Some(Classic)` (F-12, BC-2.21.002 postcondition 3 / + /// invariant 4) — a flow already sticky-classified `Plus`/`Unclassified` is + /// never dissected, even on a later `0x32`-leading DT frame. The + /// `None`-from-`parse_cotp_header` branch (unclassified-gap, BC-2.21.028), the + /// `Some(0x72)` DT branch (S7comm-plus framing-only path, BC-2.21.024/025/026, + /// STORY-190 scope), and the `Some(other)`/`None`-protocol_id DT branches + /// (unclassified gap, BC-2.21.027) remain structural no-ops with no + /// divergent/panicking body — their observable behavior is STORY-190's scope. + fn dispatch_cotp_frame( + state: &mut S7commFlowState, + cotp: Option, + tpkt_payload: &[u8], + direction: Direction, + ts: u32, + findings: &mut Vec, + ) { + match cotp { + None => { + // Unclassified gap (BC-2.21.028): `parse_cotp_header` could not + // interpret this COTP payload. Routed to a STORY-190 placeholder + // no-op per this story's Tasks list — never counted as S7comm, never + // force-fit into a recognized branch. + } + Some(header) => match header.tpdu_type { + iso_on_tcp::CotpTpduType::ConnectRequest => { + // Session tracking only, no protocol classification + // (BC-2.21.002 postcondition 2). Records/updates the pending + // CR direction so a later CC on this flow can be tested for + // direction-opposite-ness (F-01 ruling, BC-2.21.001 + // postcondition 1) -- a bare CR alone never sets + // `session_established` itself. + state.cr_observed_dir = Some(direction); + } + iso_on_tcp::CotpTpduType::ConnectConfirm => { + // A CC observed on this flow marks the session established + // ONLY when a CR was previously recorded in the OPPOSITE + // direction (F-01 ruling, BC-2.21.001 postcondition 1, + // AC-187-003). A CC with no prior CR, a CC before any CR + // (out-of-order -- matching is forward-looking from the CR + // only, never retroactive from the CC), or a CC in the SAME + // direction as the recorded CR all leave `session_established` + // untouched. No protocol classification occurs here + // (BC-2.21.002 postcondition 2). + if let Some(cr_dir) = state.cr_observed_dir + && cr_dir != direction + { + state.session_established = true; + } + } + iso_on_tcp::CotpTpduType::DataTransfer => { + Self::classify_first_dt_frame(state, header.protocol_id); + match header.protocol_id { + Some(0x32) => { + // F-12 gate (BC-2.21.002 postcondition 3 / invariant 4): + // classic dissection fires only when the flow's STICKY + // classified_protocol is (or, by this very frame's own + // first-classification above, becomes) Classic — never + // merely because the current frame's raw protocol_id + // byte is 0x32. A flow already sticky-classified Plus or + // Unclassified by an earlier DT frame must never fall + // into classic dissection, even on a later 0x32-leading + // frame (ADR-014 Decision 2 no-misattribution guarantee, + // F-12 ruling, human-ratified 2026-09-24). + if state.classified_protocol == Some(S7Protocol::Classic) { + let payload = &tpkt_payload[header.payload_offset..]; + Self::dispatch_classic_s7comm( + state, payload, direction, ts, findings, + ); + } + } + Some(0x72) => { + // S7comm-plus framing-only path (BC-2.21.024/025/026) — + // completed structurally in STORY-190. Deliberate + // no-divergent-body placeholder no-op per this story's + // Tasks list. + } + _ => { + // Unrecognized protocol_id, or an empty DT payload + // (protocol_id: None, BC-2.20.010) — unclassified gap + // (BC-2.21.027). Deliberate no-divergent-body placeholder + // no-op per this story's Tasks list. + } + } + } + }, + } + } + + /// BC-2.21.002 postcondition 6 / BC-2.21.001 edge case EC-002: + /// sticky-first-classification-wins. On the first DT frame observed for a flow + /// whose `protocol_id` is `Some(byte)`, `classified_protocol` is set exactly + /// once, per ADR-014 Decision 2's four-row disambiguation table: `Some(0x32)` -> + /// `Classic`, `Some(0x72)` -> `Plus`, any other `Some(byte)` -> `Unclassified`. + /// Subsequent DT frames on the same flow never overwrite it, even if their + /// `protocol_id` differs. + /// + /// A `protocol_id: None` DT frame (empty payload, BC-2.20.010) carries NO + /// protocol evidence and never classifies -- it does not consume the flow's + /// "first DT frame" status, so classification remains deferred to a later DT + /// frame (if any) that carries `Some(byte)` (F-02 ruling, human-ratified + /// 2026-09-24, BC-2.21.002 postcondition 6 / edge case EC-004). + fn classify_first_dt_frame(state: &mut S7commFlowState, protocol_id: Option) { + let Some(byte) = protocol_id else { + // No protocol evidence; leaves classified_protocol (and "first DT + // frame" status) untouched regardless of its current value (F-02). + return; + }; + if state.classified_protocol.is_some() { + return; + } + let protocol = match byte { + 0x32 => S7Protocol::Classic, + 0x72 => S7Protocol::Plus, + _ => S7Protocol::Unclassified, + }; + state.classified_protocol = Some(protocol); + } + + /// BC-2.21.002 postcondition 3: classic S7comm (`protocol_id == Some(0x32)`) + /// dissection entry point. Calls [`parse_s7comm_header`] on `payload` (the DT + /// payload slice beginning at `payload_offset`) and applies the BC-2.21.009 + /// caller-side bounds check (`data.len() >= header_len + param_length + + /// data_length`) before any parameter/data-block slice — malformed headers and + /// bounds failures emit one T0814 per flow direction via + /// `malformed_header_reported_c2s`/`_s2c` (BC-2.21.001). + /// + fn dispatch_classic_s7comm( + state: &mut S7commFlowState, + payload: &[u8], + direction: Direction, + ts: u32, + findings: &mut Vec, + ) { + // F-15: this function is only ever reached from the `Some(0x32)` DT arm + // above, whose `payload` is `&tpkt_payload[header.payload_offset..]` with + // `header.protocol_id == Some(tpkt_payload[header.payload_offset])` + // (BC-2.20.009) -- i.e. `payload[0]` is always the already-classified + // protocol-ID byte. Debug-only self-check of that call-site invariant; + // never a production-reachable panic (release builds omit this entirely). + debug_assert_eq!( + payload.first(), + Some(&0x32u8), + "dispatch_classic_s7comm must only be called with a payload beginning \ + at the already-classified 0x32 protocol-ID byte" + ); + + let Some(header) = parse_s7comm_header(payload) else { + // BC-2.21.004/007/008: length-reject or unrecognized-ROSCTR — malformed + // header, dedup-guarded T0814. F-11: the specific reject reason (and, + // where applicable, declared-vs-available byte counts) is classified + // separately for evidence purposes only -- it never influences the + // accept/reject decision itself, which remains `parse_s7comm_header`'s + // sole responsibility. + let reason = Self::classify_malformed_header_reason(payload); + Self::report_malformed_header(state, direction, ts, findings, &reason); + return; + }; + + // BC-2.21.009 / F-14: the declared param_length/data_length are + // bounds-checked against the bytes actually remaining in `payload` before + // any parameter/data-block slice is ever constructed. Delegates to the + // extracted pure helper [`s7comm_bounds_ok`] rather than re-inlining the + // checked-arithmetic comparison here, so this call site and the VP-051 + // Kani harness share a single source of truth for the bounds decision. + if s7comm_bounds_ok(&header, payload.len()) { + // Bounds check passes. Function-code/Userdata classification + // (BC-2.21.010 onward) is out of this story's scope (STORY-188/189). + } else { + // Declared lengths exceed the bytes actually present (or, in the + // unreachable overflow case, the sum would have overflowed `usize`) — + // treated identically to a malformed header (BC-2.21.009 + // postcondition 2). F-11: report the declared-vs-available byte counts. + let declared = + header.header_len as u64 + header.param_length as u64 + header.data_length as u64; + let reason = format!( + "declared param_length/data_length exceed available bytes: \ + declared {declared} (header_len={} + param_length={} + data_length={}), \ + available {} (BC-2.21.009)", + header.header_len, + header.param_length, + header.data_length, + payload.len() + ); + Self::report_malformed_header(state, direction, ts, findings, &reason); + } + } + + /// F-11: classifies *why* [`parse_s7comm_header`] rejected `payload`, purely to + /// produce specific T0814 evidence text -- this classification never affects + /// the accept/reject decision itself, which remains `parse_s7comm_header`'s + /// sole responsibility (this function is only ever called after that function + /// has already returned `None` for the same `payload`). + fn classify_malformed_header_reason(payload: &[u8]) -> String { + if payload.len() < 10 { + return format!( + "header too short: {} byte(s) available, 10 required (BC-2.21.004)", + payload.len() + ); + } + if payload[0] != 0x32 { + // Unreachable via dispatch_classic_s7comm's only call site (guarded by + // the debug_assert_eq! in that function, F-15) -- retained as a + // defensive fallback so this classifier never panics or mis-labels a + // future, differently-guarded call site. Because this call site + // guarantees payload[0] == 0x32, this branch never actually executes + // today, so BC-2.21.005 postcondition 3 (no Finding emitted for a + // direct, non-dispatch-gated reject) is not violated in practice -- + // the string it would produce is unreachable, not observed. + return format!( + "unexpected protocol-ID byte 0x{:02x} (expected 0x32, BC-2.21.005)", + payload[0] + ); + } + match payload[1] { + 0x02 => format!( + "truncated Ack header: {} byte(s) available, 12 required (BC-2.21.008)", + payload.len() + ), + 0x03 => format!( + "truncated Ack_Data header: {} byte(s) available, 12 required \ + (BC-2.21.008, DF-CANONICAL-FRAME-HOLDOUT-001)", + payload.len() + ), + other => format!("unrecognized ROSCTR byte 0x{other:02x} (BC-2.21.007)"), + } + } + + /// Emits one T0814 (Anomaly/Possible/Medium) for a malformed classic S7comm + /// header/bounds condition, deduplicated per flow direction via + /// `malformed_header_reported_c2s`/`_s2c` (BC-2.21.001 postcondition 1) — shared + /// by BC-2.21.004 (too-short), BC-2.21.007 (unrecognized ROSCTR), BC-2.21.008 + /// (truncated Ack, truncated Ack_Data), and BC-2.21.009 (declared-length/ + /// available-bytes mismatch), since all five conditions answer the same + /// question: "was this frame's S7comm header/declared structure internally + /// consistent?" + /// + /// `reason` (F-11) is the specific, human-readable cause -- e.g. "header too + /// short: 4 byte(s) available, 10 required" or (for header_len=10, + /// param_length=3, data_length=5, available=12) "declared param_length/ + /// data_length exceed available bytes: declared 18 (header_len=10 + + /// param_length=3 + data_length=5), available 12 (BC-2.21.009)" -- computed + /// by the caller via [`Self::classify_malformed_header_reason`] (parse + /// failures) or inline (bounds-check failure). It never influences dedup or + /// the T0814 emission decision, only the finding's summary/evidence text. + fn report_malformed_header( + state: &mut S7commFlowState, + direction: Direction, + ts: u32, + findings: &mut Vec, + reason: &str, + ) { + use crate::findings::{Confidence, ThreatCategory, Verdict}; + + let reported = if direction == Direction::ClientToServer { + &mut state.malformed_header_reported_c2s + } else { + &mut state.malformed_header_reported_s2c + }; + if *reported { + return; + } + *reported = true; + + let timestamp = chrono::DateTime::from_timestamp(ts as i64, 0); + findings.push(Finding { + category: ThreatCategory::Anomaly, + verdict: Verdict::Possible, + confidence: Confidence::Medium, + summary: format!( + "Malformed classic S7comm header: {reason} (T0814; \ + BC-2.21.004/007/008/009)" + ), + evidence: vec![reason.to_string()], + mitre_techniques: vec!["T0814".to_string()], + source_ip: None, + timestamp, + direction: Some(direction), + }); + } } diff --git a/tests/fixtures/mk_s7comm_pcap.py b/tests/fixtures/mk_s7comm_pcap.py new file mode 100644 index 00000000..a79ca7fb --- /dev/null +++ b/tests/fixtures/mk_s7comm_pcap.py @@ -0,0 +1,445 @@ +#!/usr/bin/env python3 +""" +mk_s7comm_pcap.py — Generate tests/fixtures/s7comm-setup-comm.pcap + +Produces a minimal libpcap (.pcap) file with classic magic (0xa1b2c3d4, +little-endian) and link-type 1 (Ethernet) containing a classic S7comm (protocol-ID +0x32) session over ISO-on-TCP (TPKT/COTP) on TCP/102. + +STORY-187 scope (ADR-014 Decision 7 item 1, "first use in this story"): this +generator hand-crafts the COTP session-establishment handshake (Connect Request / +Connect Confirm) plus a Setup Communication (function 0xF0) Job/Ack_Data PDU pair +and one further minimal classic Job/Ack_Data PDU pair — deliberately synthetic, +dedicated CC0/MIT, following the `mk_modbus_large_pcap.py` precedent (per ADR-014 +Decision 7's own citation) (pure-Python libpcap/Ethernet/IPv4/TCP builders, no +external pcap-writing library). + +No official public Siemens specification exists for classic S7comm (ADR-014 +Decision 4) — the wire layout used here (TPKT/COTP framing, the classic S7comm +common header — 10 bytes for Job (0x01)/Userdata (0x07), 12 bytes for Ack +(0x02)/Ack_Data (0x03) per the 2026-09-24 canonical-frame holdout ruling, +DF-CANONICAL-FRAME-HOLDOUT-001 — and the Setup Communication parameter block) +has its field semantics and parser design derived from ADR-014 Decision 4's +PROSE sources (the Wireshark wiki page, Kleinmann & Wool 2014, and the +Orange-Cyberdefense `awesome-industrial-protocols` catalog) together with +Decision 4's PERMITTED DESIGN REFERENCES (cisagov/icsnpp-s7comm, +kprovost/libs7comm, gijzelaerr/python-snap7) — never from Wireshark's +dissector source, Snap7 (the C++ library), or libnodave (all GPL/LGPL-tainted). +Snap7 (the C++ library) is distinct from gijzelaerr/python-snap7 (a permitted +Python binding/wrapper listed above); the two are not interchangeable, and +this exclusion refers only to the former. Kleinmann & Wool 2014 attests the +Ack_Data (0x03) 12-byte header only; the Ack (0x02) +12-byte layout and the 1-byte error_class/error_code split it shares with +Ack_Data rest on cisagov/icsnpp-s7comm and gijzelaerr/python-snap7 +(kprovost/libs7comm consistent in aggregate) — see Decision 4/Decision 9's +notes for the full citation. The canonical byte examples used as test +vectors are, separately, corroborated by publicly posted wire-capture +sources (cnblogs, https://www.cnblogs.com/crcce-dncs/p/10659087.html — +primary; Yiqisoft, https://www.yiqisoft.cn/blogs/IoT-Gateway/363.html; and +the Inductive Automation KB) — permitted as test-vector sources only by the +ADR-014 Decision 4 reconciliation note (2026-09-24, DF-CANONICAL-FRAME- +HOLDOUT-001); Kleinmann & Wool 2014 is a prose field-semantics source, not a +test-vector source. Zero lines are borrowed from any external implementation. + +This fixture is consumed by +`test_BC_2_21_002_setup_comm_fixture_pcap_well_formed_no_findings` in +`tests/s7comm_analyzer_tests.rs` — an end-to-end regression check that reads the +committed capture, feeds every packet's TCP payload through +`S7commAnalyzer::on_data`, and asserts the well-formed session yields zero +findings, `session_established == true`, and `classified_protocol == +Some(S7Protocol::Classic)`. + +Packet sequence (all timestamps in seconds, realistic 2024-era epoch values): + 1. [t=1_717_100_000] Client→Server SYN (TCP handshake — no payload) + 2. [t=1_717_100_001] Server→Client SYN-ACK + 3. [t=1_717_100_002] Client→Server ACK (handshake complete) + 4. [t=1_717_100_003] Client→Server: COTP Connect Request (CR) + 5. [t=1_717_100_004] Server→Client: COTP Connect Confirm (CC) + 6. [t=1_717_100_005] Client→Server: S7comm Setup Communication request + (ROSCTR=Job 0x01, parameter function 0xF0) + 7. [t=1_717_100_006] Server→Client: S7comm Setup Communication response + (ROSCTR=Ack_Data 0x03, parameter function 0xF0, PDU-length negotiated) + 8. [t=1_717_100_007] Client→Server: minimal classic Job PDU (ROSCTR=Job 0x01, + empty parameter/data blocks — BC-2.21.006 EC-001 shape) + 9. [t=1_717_100_008] Server→Client: minimal classic Ack_Data PDU (ROSCTR=Ack_Data + 0x03, empty parameter/data blocks) + 10. [t=1_717_100_009] Client→Server FIN-ACK + +Usage: + python3 tests/fixtures/mk_s7comm_pcap.py + # Writes tests/fixtures/s7comm-setup-comm.pcap +""" + +import struct +import os + +# --------------------------------------------------------------------------- +# libpcap file format helpers (verbatim structure, mirrors mk_modbus_large_pcap.py) +# --------------------------------------------------------------------------- + +PCAP_MAGIC_LE = 0xA1B2C3D4 # little-endian native, microsecond timestamps +LINKTYPE_ETHERNET = 1 + + +def pcap_global_header() -> bytes: + """24-byte global header (little-endian).""" + return struct.pack( + " bytes: + """16-byte per-packet header (little-endian). ts_usec=0 (whole-second).""" + return struct.pack( + " bytes: + """14-byte Ethernet II header.""" + return dst_mac + src_mac + bytes([0x08, 0x00]) # EtherType = IPv4 + + +def ip_header(src_ip: bytes, dst_ip: bytes, total_len: int, proto: int = 6) -> bytes: + """20-byte IPv4 header (no options, TTL=64, identification=0x5678).""" + hdr = bytearray( + [ + 0x45, # version=4, IHL=5 (20 bytes) + 0x00, # DSCP/ECN + (total_len >> 8) & 0xFF, + total_len & 0xFF, + 0x56, 0x78, # identification + 0x40, 0x00, # flags=DF, fragment offset=0 + 64, # TTL + proto, # protocol (6=TCP) + 0x00, 0x00, # checksum placeholder + ] + ) + hdr += src_ip + dst_ip + words = struct.unpack("!10H", bytes(hdr)) + s = sum(words) + s = (s >> 16) + (s & 0xFFFF) + s += s >> 16 + checksum = (~s) & 0xFFFF + hdr[10] = (checksum >> 8) & 0xFF + hdr[11] = checksum & 0xFF + return bytes(hdr) + + +def tcp_header( + src_port: int, + dst_port: int, + seq: int, + ack: int, + flags: int, + payload_len: int, + src_ip: bytes, + dst_ip: bytes, +) -> bytes: + """20-byte TCP header. Checksum left as 0 — wirerust does not validate it.""" + offset_flags = (5 << 12) | flags + hdr = struct.pack( + "!HHIIHHH", + src_port, + dst_port, + seq, + ack, + offset_flags, + 65535, # window + 0, # checksum placeholder + ) + bytes([0, 0]) # urgent pointer + return bytes(hdr) + + +def build_frame( + src_mac: bytes, + dst_mac: bytes, + src_ip: bytes, + dst_ip: bytes, + src_port: int, + dst_port: int, + seq: int, + ack: int, + flags: int, + payload: bytes, +) -> bytes: + """Build a complete Ethernet+IPv4+TCP frame.""" + tcp_hdr = tcp_header(src_port, dst_port, seq, ack, flags, len(payload), src_ip, dst_ip) + ip_total_len = 20 + 20 + len(payload) # IP + TCP + payload + ip_hdr = ip_header(src_ip, dst_ip, ip_total_len) + eth_hdr = eth_header(src_mac, dst_mac) + return eth_hdr + ip_hdr + tcp_hdr + payload + + +# --------------------------------------------------------------------------- +# TPKT (RFC 1006) / COTP (ISO 8073) framing builders +# --------------------------------------------------------------------------- + + +def tpkt_frame(cotp_payload: bytes) -> bytes: + """Wrap a COTP payload in a 4-byte TPKT header (RFC 1006 §6).""" + total_len = 4 + len(cotp_payload) + assert total_len <= 0xFFFF, "TPKT length field is a u16 (RFC 1006 §6)" + return struct.pack("!BBH", 0x03, 0x00, total_len) + cotp_payload + + +def cotp_cr(dst_ref: int = 0x0000, src_ref: int = 0x0001) -> bytes: + """ + COTP Connect Request (CR), TPDU code high-nibble 0xE0. + + Fixed part: LI, code|credit, dst-ref(2), src-ref(2), class-option(1) = 6 bytes + (LI=6 counts everything after the LI byte itself, ISO 8073 §13.3). + """ + fixed = struct.pack("!BHHB", 0xE0, dst_ref, src_ref, 0x00) + li = len(fixed) + return bytes([li]) + fixed + + +def cotp_cc(dst_ref: int = 0x0001, src_ref: int = 0x0001) -> bytes: + """COTP Connect Confirm (CC), TPDU code high-nibble 0xD0 — same shape as CR.""" + fixed = struct.pack("!BHHB", 0xD0, dst_ref, src_ref, 0x00) + li = len(fixed) + return bytes([li]) + fixed + + +def cotp_dt(upper_payload: bytes, tpdu_number: int = 0x00) -> bytes: + """ + COTP Data Transfer (DT), standard ISO 8073 class-0 fixed part: LI (1) + + code (1, 0xF0) + TPDU-NR+EOT (1) — the code and TPDU-NR+EOT bytes are + SEPARATE octets (LI=2, counting both bytes that follow the LI byte itself), + not merged into a single byte. The end-of-TSDU (EOT) bit is bit 7 of the + TPDU-NR+EOT byte — set here, since every DT in this fixture is a complete, + unsegmented TSDU — with bits 0-6 carrying the 7-bit TPDU sequence number. + """ + return bytes([0x02, 0xF0, 0x80 | (tpdu_number & 0x7F)]) + upper_payload + + +# --------------------------------------------------------------------------- +# Classic S7comm (protocol-ID 0x32) PDU builders +# --------------------------------------------------------------------------- + +S7_PROTOCOL_ID = 0x32 +ROSCTR_JOB = 0x01 +ROSCTR_ACK = 0x02 +ROSCTR_ACK_DATA = 0x03 + + +def s7comm_pdu( + rosctr: int, + pdu_reference: int, + parameter: bytes, + data: bytes, + error_class: int = 0x00, + error_code: int = 0x00, +) -> bytes: + """ + Classic S7comm common header. Per the 2026-09-24 canonical-frame holdout + ruling (DF-CANONICAL-FRAME-HOLDOUT-001, BC-2.21.006/BC-2.21.008): Job (0x01) + and Userdata (0x07) use the 10-byte common header — Protocol ID (1) + ROSCTR + (1) + Reserved (2, always 0x0000 here) + PDU Reference (2 BE) + Parameter + Length (2 BE) + Data Length (2 BE); Ack (0x02) and Ack_Data (0x03) use a + 12-byte header — the same 10 bytes plus Error Class (1) + Error Code (1) — + with the parameter block starting at byte 12, not byte 10. `error_class`/ + `error_code` are ignored for Job/Userdata (no such fields exist there). + """ + header = struct.pack( + "!BBHHHH", + S7_PROTOCOL_ID, + rosctr, + 0x0000, # reserved + pdu_reference, + len(parameter), + len(data), + ) + if rosctr in (ROSCTR_ACK, ROSCTR_ACK_DATA): + header += bytes([error_class & 0xFF, error_code & 0xFF]) + return header + parameter + data + + +def setup_communication_request(pdu_reference: int) -> bytes: + """ + Setup Communication (function 0xF0) Job request. Parameter block: function + (1) + reserved (1) + max AMQ calling (2 BE) + max AMQ called (2 BE) + PDU + length (2 BE) — the conventional 8-byte Setup Communication parameter shape. + """ + parameter = struct.pack("!BBHHH", 0xF0, 0x00, 0x0001, 0x0001, 0x03C0) + return s7comm_pdu(ROSCTR_JOB, pdu_reference, parameter, b"") + + +def setup_communication_response(pdu_reference: int) -> bytes: + """ + Setup Communication Ack_Data response — same 8-byte parameter shape, now + correctly emitted with the 12-byte Ack_Data header (error_class=0x00, + error_code=0x00), matching the canonical cnblogs Ack_Data layout used by + `test_BC_2_21_008_canonical_ack_data_setup_communication_response_on_data` + (parameter block at byte 12, per DF-CANONICAL-FRAME-HOLDOUT-001). + """ + parameter = struct.pack("!BBHHH", 0xF0, 0x00, 0x0001, 0x0001, 0x01E0) + return s7comm_pdu(ROSCTR_ACK_DATA, pdu_reference, parameter, b"") + + +def minimal_job_pdu(pdu_reference: int) -> bytes: + """ + A minimal classic Job PDU with empty parameter/data blocks — the + BC-2.21.006 EC-001 shape (`param_length == 0`, `data_length == 0`). + Function-code classification (Groups 3/4) is out of this story's scope + (STORY-188/189); this PDU exists only to exercise the header-level + Job/Ack_Data pairing this fixture models. + """ + return s7comm_pdu(ROSCTR_JOB, pdu_reference, b"", b"") + + +def minimal_ack_data_pdu(pdu_reference: int) -> bytes: + """ + The matching minimal classic Ack_Data response, also empty parameter/data + blocks — a 12-byte Ack_Data header (error_class=0x00, error_code=0x00) per + DF-CANONICAL-FRAME-HOLDOUT-001; NOT a 10-byte header. + """ + return s7comm_pdu(ROSCTR_ACK_DATA, pdu_reference, b"", b"") + + +# --------------------------------------------------------------------------- +# TCP flag constants +# --------------------------------------------------------------------------- +SYN = 0x002 +ACK = 0x010 +FIN = 0x001 +SYN_ACK = SYN | ACK +FIN_ACK = FIN | ACK +PSH_ACK = 0x018 # PSH=0x008 | ACK=0x010 + + +# --------------------------------------------------------------------------- +# Build the complete packet capture +# --------------------------------------------------------------------------- + + +def build_pcap() -> bytes: + packets = [] + + # Base timestamp: 2024-05-30 20:13:20 UTC (Unix epoch 1717100000) + t0 = 1_717_100_000 + + client_seq = 5000 + server_seq = 9000 + + def send(direction_client_to_server, payload, t_offset): + nonlocal client_seq, server_seq + if direction_client_to_server: + frame = build_frame( + CLIENT_MAC, SERVER_MAC, CLIENT_IP, SERVER_IP, + CLIENT_PORT, SERVER_PORT, + client_seq, server_seq, PSH_ACK, payload, + ) + packets.append((t0 + t_offset, frame)) + client_seq += len(payload) + else: + frame = build_frame( + SERVER_MAC, CLIENT_MAC, SERVER_IP, CLIENT_IP, + SERVER_PORT, CLIENT_PORT, + server_seq, client_seq, PSH_ACK, payload, + ) + packets.append((t0 + t_offset, frame)) + server_seq += len(payload) + + # --- Packets 1-3: TCP handshake --- + packets.append( + ( + t0, + build_frame( + CLIENT_MAC, SERVER_MAC, CLIENT_IP, SERVER_IP, + CLIENT_PORT, SERVER_PORT, client_seq, 0, SYN, b"", + ), + ) + ) + client_seq += 1 + packets.append( + ( + t0 + 1, + build_frame( + SERVER_MAC, CLIENT_MAC, SERVER_IP, CLIENT_IP, + SERVER_PORT, CLIENT_PORT, server_seq, client_seq, SYN_ACK, b"", + ), + ) + ) + server_seq += 1 + packets.append( + ( + t0 + 2, + build_frame( + CLIENT_MAC, SERVER_MAC, CLIENT_IP, SERVER_IP, + CLIENT_PORT, SERVER_PORT, client_seq, server_seq, ACK, b"", + ), + ) + ) + + # --- Packet 4: COTP Connect Request --- + send(True, tpkt_frame(cotp_cr()), 3) + + # --- Packet 5: COTP Connect Confirm --- + send(False, tpkt_frame(cotp_cc()), 4) + + # --- Packet 6: Setup Communication request (Job) --- + send(True, tpkt_frame(cotp_dt(setup_communication_request(0x0001))), 5) + + # --- Packet 7: Setup Communication response (Ack_Data) --- + send(False, tpkt_frame(cotp_dt(setup_communication_response(0x0001))), 6) + + # --- Packet 8: minimal classic Job PDU (empty parameter/data blocks) --- + send(True, tpkt_frame(cotp_dt(minimal_job_pdu(0x0002))), 7) + + # --- Packet 9: minimal classic Ack_Data PDU (empty parameter/data blocks) --- + send(False, tpkt_frame(cotp_dt(minimal_ack_data_pdu(0x0002))), 8) + + # --- Packet 10: Client->Server FIN-ACK --- + packets.append( + ( + t0 + 9, + build_frame( + CLIENT_MAC, SERVER_MAC, CLIENT_IP, SERVER_IP, + CLIENT_PORT, SERVER_PORT, client_seq, server_seq, FIN_ACK, b"", + ), + ) + ) + + out = pcap_global_header() + for ts_sec, frame_bytes in packets: + out += pcap_packet_header(ts_sec, len(frame_bytes)) + out += frame_bytes + return out + + +if __name__ == "__main__": + script_dir = os.path.dirname(os.path.abspath(__file__)) + out_path = os.path.join(script_dir, "s7comm-setup-comm.pcap") + data = build_pcap() + with open(out_path, "wb") as f: + f.write(data) + print(f"Wrote {len(data)} bytes to {out_path}") + print(" - 10 packets total (3 handshake + CR/CC + Setup Comm req/resp +") + print(" minimal Job/Ack_Data pair + FIN-ACK)") + print(" - Port 102 (ISO-on-TCP / S7comm)") + print(f" - Timestamps: {1_717_100_000} .. {1_717_100_009} (Unix epoch, 2024-05-30)") diff --git a/tests/fixtures/s7comm-setup-comm.pcap b/tests/fixtures/s7comm-setup-comm.pcap new file mode 100644 index 00000000..fa635345 Binary files /dev/null and b/tests/fixtures/s7comm-setup-comm.pcap differ diff --git a/tests/s7comm_analyzer_tests.rs b/tests/s7comm_analyzer_tests.rs index b86e2182..499ceeac 100644 --- a/tests/s7comm_analyzer_tests.rs +++ b/tests/s7comm_analyzer_tests.rs @@ -1,9 +1,17 @@ //! 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. +//! Extraction, Resync, and the Frozen SS-20/SS-21 Module Boundary; and for STORY-187: +//! S7comm Flow State Completion, Four-Way `protocol_id` Dispatch Skeleton, and +//! `parse_s7comm_header` Pure-Core Parser. //! -//! Covers BC-2.20.013, BC-2.20.014, BC-2.20.015, BC-2.20.016, BC-2.21.003, and the +//! Covers BC-2.20.013, BC-2.20.014, BC-2.20.015, BC-2.20.016, BC-2.21.003, the //! VP-050 proptest obligation (walk-first residual bound, direction isolation, 1-byte -//! resync advance). +//! resync advance) (STORY-186); and BC-2.21.001, BC-2.21.002, BC-2.21.004 through +//! BC-2.21.009, the VP-051 Kani skeleton, the VP-053 proptest skeleton, the +//! independently-sourced canonical-frame holdout tests required by policy +//! DF-CANONICAL-FRAME-HOLDOUT-001, and the committed-fixture pcap end-to-end test +//! (`test_BC_2_21_002_setup_comm_fixture_pcap_well_formed_no_findings`) (STORY-187). +//! Each story's tests are grouped in their own `mod story_186` / `mod story_187` +//! wrapper below, each with its own module-level doc comment giving full detail. //! //! ## Contract coverage //! - BC-2.20.013: TPKT frames spanning TCP segment boundaries are reassembled via @@ -34,8 +42,8 @@ //! 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 +//! expected to be green immediately, since the frozen SS-20 module boundary is +//! already satisfied by the current implementation — 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) @@ -967,7 +975,7 @@ mod story_186 { // 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 + // already be green (satisfied by the frozen SS-20 module boundary) and // exist here as permanent drift guards against a future violation. // ========================================================================= @@ -1289,3 +1297,3923 @@ mod story_186 { } } } + +/// Tests for STORY-187: S7comm Flow State Completion, Four-Way `protocol_id` Dispatch +/// Skeleton, and `parse_s7comm_header` Pure-Core Parser. +/// +/// Covers BC-2.21.001, BC-2.21.002, BC-2.21.004 through BC-2.21.009, and the +/// VP-051 Kani skeleton / VP-053 proptest skeleton obligations (both partial per +/// STORY-187's own VP Obligation notes — full proof execution/non-vacuous run is +/// deferred to STORY-194, and VP-053's Some(0x72)/unclassified dissection completeness +/// is deferred to STORY-190). +/// +/// ## Contract coverage +/// - BC-2.21.001: `S7commFlowState` carries the full STORY-187 v1.1 field set +/// (`session_established`, `cr_observed_dir`, `classified_protocol`, +/// `malformed_header_reported_c2s`/`_s2c`), created lazily on first `on_data`, with +/// the carry-overflow and malformed-header dedup flags tracked independently. +/// - BC-2.21.002: `S7commAnalyzer::on_data`'s four-way dispatch on +/// `CotpHeader::protocol_id` — CR/CC session tracking (no classification), classic +/// (`Some(0x32)`) dissection entry gated on sticky classification, and sticky +/// first-classification-wins over `Some(byte)`-only DT frames. +/// - BC-2.21.004: `parse_s7comm_header` returns `None` for `data.len() < 10`. +/// - BC-2.21.005: `parse_s7comm_header` defensively rejects `data[0] != 0x32`. +/// - BC-2.21.006: `parse_s7comm_header` extracts the common header (Job/Userdata +/// happy path only, as of the round-4 canonical-frame holdout ruling below — +/// Ack_Data moved to BC-2.21.008). +/// - BC-2.21.007: `parse_s7comm_header` returns `None` for an unrecognized ROSCTR byte. +/// - BC-2.21.008: `parse_s7comm_header` for ROSCTR=Ack AND Ack_Data (0x02/0x03) both +/// require 12 bytes and extract `error_class`/`error_code`. +/// - BC-2.21.009: the caller-side `header_len + param_length + data_length` bounds +/// check precedes any parameter/data-block slice; extracted as the pure helper +/// `s7comm_bounds_ok` (F-14). +/// +/// ## Test naming convention +/// `test_BC_S_SS_NNN_xxx()`, matching `story_186`'s established convention in this +/// file. `#![allow(non_snake_case)]` at the file top already covers this module. +/// +/// ## Provenance +/// The original STORY-187 test suite originated Red-first (`tdd_mode: strict`) +/// against `todo!()` bodies in `src/analyzer/s7comm.rs`; that original scope landed +/// GREEN in commit 3be2730a and later commits. This module's round-2 addendum +/// (2026-09-24) added coverage for the human-ratified rulings from STORY-187's +/// per-story adversarial pass 1 (F-01 opposite-direction CR/CC matching, F-02 +/// `protocol_id: None` never classifies, F-08 256-value ROSCTR totality, F-12 sticky +/// classification gates re-dissection, F-14 bounded VP-051 Kani harness plus the +/// `s7comm_bounds_ok` pure helper) — see STORY-187.md v1.1's Changelog for the full +/// ruling list; ALL of that round-2 coverage (F-01/F-02/F-08/F-12/F-14) is now GREEN +/// against the current implementation (see `src/analyzer/s7comm.rs`'s +/// `dispatch_cotp_frame`/`classify_first_dt_frame`/`dispatch_classic_s7comm` for the +/// F-01/F-02/F-12 logic and `s7comm_bounds_ok` for F-14) — none of it is pending +/// follow-up implementation work. +/// +/// This module's round-3 addendum (per-story adversarial pass 2, STORY-187.md v1.2) +/// adds: the AC-187-013 canonical, independently-sourced classic S7comm frame test +/// required by policy DF-CANONICAL-FRAME-HOLDOUT-001 (F-19) — which originally +/// surfaced a genuine spec/implementation discrepancy over the Ack_Data (`0x03`) +/// header length (see the round-4 note below for its resolution); reason-specific +/// malformed-header evidence-text assertions and additional Ack/bounds +/// `on_data`-level coverage (F-21); a fifth AC-187-003 negative case, repeated +/// same-direction CR (F-22); and the VP-051 bounds-check-half Kani skeleton plus a +/// non-vacuity wording correction (F-18/F-24). +/// +/// This module's round-4 addendum (STORY-187.md v1.3, the 2026-09-24 canonical-frame +/// holdout human ruling, DF-CANONICAL-FRAME-HOLDOUT-001) resolves the round-3 +/// discrepancy: ROSCTR `0x02` (Ack) AND `0x03` (Ack_Data) BOTH require the 12-byte +/// header (`error_class = data[10]`, `error_code = data[11]`, `header_len == 12`); +/// Job (`0x01`)/Userdata (`0x07`) remain 10-byte with no error fields; a truncated +/// Ack_Data frame (10 or 11 bytes) returns `None`, exactly like a truncated Ack. The +/// former `test_BC_2_21_006_canonical_setup_communication_job_frame_on_data` +/// "DISCREPANCY -- EXPECTED TO FAIL" Ack_Data assertions are now split into their own +/// `test_BC_2_21_008_canonical_ack_data_setup_communication_response_on_data` test, +/// asserting the corrected (not discrepant) 12-byte shape as ordinary, non-vacuous +/// GREEN-gate assertions against the current implementation. Per DF-TEST-NAMESPACE-001, +/// all STORY-187 tests are grouped inside this dedicated `mod story_187` wrapper. +/// +/// Canonical test vectors from BC-2.21.004/005/006/007/008/009 are used verbatim +/// where the BCs themselves specify them, EXCEPT where a test's own doc comment +/// notes a deliberate substitution -- e.g. P12-F-2's non-zero error-byte +/// substitution in the BC-2.21.008 Ack/Ack_Data 12-byte happy-path assertions +/// (`test_BC_2_21_008_ack_rosctr_12_byte_minimum_and_error_fields`, +/// `test_BC_2_21_008_ack_data_12_byte_header_and_error_fields`), which use the +/// canonical vector's SHAPE with distinct error bytes 0x81/0x04 substituted for +/// the canonical `Some(0)`/`Some(0)` error fields so a mutation cannot hide behind +/// a same-value pair; `test_BC_2_21_008_canonical_ack_vector_verbatim` below covers +/// the canonical Ack vector's own bytes unmodified. These are ordinary, +/// internally-authored regression vectors and do NOT satisfy policy +/// DF-CANONICAL-FRAME-HOLDOUT-001's independent-sourcing requirement (their +/// byte values ARE derived from this +/// project's own BCs, by design — the opposite of what that policy requires). Only +/// `test_BC_2_21_006_canonical_setup_communication_job_frame_on_data` and +/// `test_BC_2_21_008_canonical_ack_data_setup_communication_response_on_data` (F-19, +/// AC-187-013) satisfy DF-CANONICAL-FRAME-HOLDOUT-001 — their byte sequences are +/// sourced independently of this project's BCs/ADR-014, per those tests' own doc +/// comments. +mod story_187 { + use chrono::DateTime; + use wirerust::analyzer::s7comm::{ + Rosctr, S7Protocol, S7commAnalyzer, S7commFlowState, S7commHeader, parse_s7comm_header, + s7comm_bounds_ok, + }; + use wirerust::findings::{Confidence, Finding, 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). Mirrors `story_186`'s helper + /// of the same shape (separate module scope, so redefined here). + fn flow_key_default() -> FlowKey { + FlowKey::new( + "127.0.0.1".parse().unwrap(), + 1234, + "127.0.0.2".parse().unwrap(), + 102, + ) + } + + /// Wraps a COTP payload in a 4-byte TPKT header (RFC 1006 §6): `[0x03, 0x00, + /// len_hi, len_lo]` where `len = 4 + cotp_payload.len()`. + fn tpkt_frame(cotp_payload: &[u8]) -> Vec { + let total_len = 4 + cotp_payload.len(); + assert!( + total_len <= u16::MAX as usize, + "test helper: frame too large for the TPKT u16 length field" + ); + let len = total_len as u16; + let mut frame = vec![0x03u8, 0x00, (len >> 8) as u8, (len & 0xFF) as u8]; + frame.extend_from_slice(cotp_payload); + frame + } + + /// A complete COTP Connect Request (CR) frame wrapped in its TPKT header: LI=1, + /// code=0xE0, one pad byte — mirrors `story_186::cr_frame_7()`'s exact shape (a + /// complete, minimal 7-byte TPKT/COTP frame; `payload_offset = 1 + LI = 2 <= + /// tpkt_payload.len() = 3`). + fn cr_frame() -> Vec { + tpkt_frame(&[0x01u8, 0xE0, 0x00]) + } + + /// A complete COTP Connect Confirm (CC) frame — same shape as `cr_frame()` but + /// with the CC high-nibble code (`0xD0`). + fn cc_frame() -> Vec { + tpkt_frame(&[0x01u8, 0xD0, 0x00]) + } + + /// A complete COTP Data Transfer (DT) frame carrying `upper_payload` as its + /// upper-layer payload: LI=1 (so `payload_offset = 2`), DT code `0xF0`, then + /// `upper_payload` verbatim (its first byte is `protocol_id` per BC-2.20.009, when + /// non-empty). + fn dt_frame(upper_payload: &[u8]) -> Vec { + let mut cotp = vec![0x01u8, 0xF0]; + cotp.extend_from_slice(upper_payload); + tpkt_frame(&cotp) + } + + /// A complete, genuinely constructible COTP DT frame with an EMPTY upper-layer + /// payload (`protocol_id: None`, BC-2.20.010): TPKT header `03 00 00 07` (version + /// 3, `length = 7`, exactly RFC 1006 §6's length-floor) followed by a 3-byte + /// class-0 COTP DT fixed part `02 F0 80` — `LI = 2`, code `0xF0`, TPDU-NR+EOT + /// `0x80` (EOT set, TPDU-NR 0). `payload_offset = 1 + LI = 3 == tpkt_payload.len()` + /// (`[0x02, 0xF0, 0x80]`, 3 bytes) exactly, so `parse_cotp_header` yields + /// `protocol_id: None` per BC-2.20.010's "no out-of-bounds index at + /// `tpkt_payload[payload_offset]`" rule — unlike the earlier revision of this + /// helper (which under-declared `LI = 1` and omitted the separate TPDU-NR+EOT + /// byte, making the resulting frame one byte short of the RFC 1006 §6 floor and + /// therefore rejected by `parse_tpkt_header` before ever reaching COTP dispatch), + /// this is a real, dispatchable minimum-legal DT frame (BC-187 round-2 F-03 fix). + fn dt_frame_empty_payload() -> Vec { + vec![0x03u8, 0x00, 0x00, 0x07, 0x02, 0xF0, 0x80] + } + + /// A minimal classic S7comm common header for Job/Userdata (10 bytes): + /// `[0x32, rosctr, 0x00, 0x00, pdu_ref(2 BE), param_len(2 BE), data_len(2 BE)]`. + fn classic_header_bytes(rosctr: u8, pdu_ref: u16, param_len: u16, data_len: u16) -> Vec { + let mut v = vec![0x32u8, rosctr, 0x00, 0x00]; + v.extend_from_slice(&pdu_ref.to_be_bytes()); + v.extend_from_slice(¶m_len.to_be_bytes()); + v.extend_from_slice(&data_len.to_be_bytes()); + v + } + + /// A minimal classic S7comm Ack (0x02) header (12 bytes): the 10-byte common + /// header plus `error_class`/`error_code`. + fn ack_header_bytes( + pdu_ref: u16, + param_len: u16, + data_len: u16, + error_class: u8, + error_code: u8, + ) -> Vec { + let mut v = classic_header_bytes(0x02, pdu_ref, param_len, data_len); + v.push(error_class); + v.push(error_code); + v + } + + /// A minimal classic S7comm Ack_Data (0x03) header (12 bytes): the 10-byte common + /// header plus `error_class`/`error_code` — same shape as `ack_header_bytes` + /// above, per the 2026-09-24 canonical-frame holdout ruling + /// (DF-CANONICAL-FRAME-HOLDOUT-001): Ack_Data requires the SAME 12-byte header as + /// Ack, not the plain 10-byte common header the pre-ruling (v1.0/v1.1) + /// implementation assumed. + fn ack_data_header_bytes( + pdu_ref: u16, + param_len: u16, + data_len: u16, + error_class: u8, + error_code: u8, + ) -> Vec { + let mut v = classic_header_bytes(0x03, pdu_ref, param_len, data_len); + v.push(error_class); + v.push(error_code); + v + } + + /// Asserts a single malformed-header T0814 finding (BC-2.21.004/007/008/009: the + /// shared `malformed_header_reported_c2s`/`_s2c` dedup class) has exactly the + /// expected category/verdict/confidence/technique/direction. + fn assert_malformed_header_t0814(finding: &Finding, expected_direction: Direction) { + assert_eq!( + finding.category, + ThreatCategory::Anomaly, + "malformed classic-S7comm-header T0814 must have ThreatCategory::Anomaly \ + (BC-2.21.004 postcondition 4)" + ); + assert_eq!( + finding.verdict, + Verdict::Possible, + "malformed classic-S7comm-header T0814 must have Verdict::Possible \ + (BC-2.21.004 postcondition 4)" + ); + assert_eq!( + finding.confidence, + Confidence::Medium, + "malformed classic-S7comm-header T0814 must have Confidence::Medium \ + (BC-2.21.004 postcondition 4)" + ); + assert_eq!( + finding.mitre_techniques, + vec!["T0814".to_string()], + "malformed classic-S7comm-header finding's mitre_techniques (Vec, \ + src/findings.rs) must be EXACTLY the single-element vec![\"T0814\"] -- not \ + merely contain it among other entries (P14-F-5a)" + ); + assert_eq!( + finding.direction, + Some(expected_direction), + "the finding must be attributed to the direction the malformed frame arrived on" + ); + } + + /// F-21: asserts a malformed classic-S7comm-header T0814 finding's evidence text + /// contains `expected_substring` -- the reason-specific evidence requirement that + /// distinguishes the five (too short; unrecognized ROSCTR; truncated Ack; + /// truncated Ack_Data; declared lengths exceed available, the BC-2.21.009 + /// bounds-check failure) malformed-header conditions sharing the + /// `malformed_header_reported_c2s`/`_s2c` + /// dedup flag from the emitted `Finding` alone. Matched against + /// `classify_malformed_header_reason`'s / `dispatch_classic_s7comm`'s exact + /// reason strings in `src/analyzer/s7comm.rs`. + fn assert_reason_specific_evidence(finding: &Finding, expected_substring: &str) { + assert!( + finding + .evidence + .iter() + .any(|e| e.contains(expected_substring)), + "expected the malformed classic-S7comm-header finding's evidence to \ + contain {expected_substring:?} (F-21 reason-specific evidence \ + requirement, distinguishing the BC-2.21.004/007/008/009 malformed-header \ + conditions from each other) -- got {:?}", + finding.evidence + ); + // P20-F-3: `report_malformed_header` (src/analyzer/s7comm.rs ~873-877) always + // formats `summary` as `format!("Malformed classic S7comm header: {reason} \ + // (T0814; BC-2.21.004/007/008/009)")` using the SAME `reason` string it pushes + // into `evidence` as `vec![reason.to_string()]` -- so `summary` is guaranteed + // non-empty and to contain each `evidence` entry verbatim. Assert only that + // guarantee, not any BC-specific wording beyond it. + assert!( + !finding.summary.is_empty(), + "malformed classic-S7comm-header finding's summary must not be empty" + ); + assert!( + finding + .evidence + .iter() + .all(|e| finding.summary.contains(e.as_str())), + "malformed classic-S7comm-header finding's summary must mention the same \ + reason text as its evidence (report_malformed_header formats summary as \ + \"Malformed classic S7comm header: {{reason}} ...\" using the identical \ + reason string as evidence[0], src/analyzer/s7comm.rs ~873-877) -- \ + summary={:?}, evidence={:?}", + finding.summary, + finding.evidence + ); + // P23-F-2 (NIT): `report_malformed_header` (src/analyzer/s7comm.rs + // ~868-871) always formats `summary` with the fixed literal prefix + // "Malformed classic S7comm header:" ahead of the interpolated + // `{reason}` -- pin that exact prefix so a wording drift or a + // reordering of the format string is caught here, not just the + // presence of the reason substring elsewhere. + assert!( + finding + .summary + .starts_with("Malformed classic S7comm header:"), + "malformed classic-S7comm-header finding's summary must start with the \ + fixed literal prefix \"Malformed classic S7comm header:\" \ + (report_malformed_header, src/analyzer/s7comm.rs) -- got {:?}", + finding.summary + ); + } + + // ========================================================================= + // BC-2.21.001: `S7commFlowState` owns TPKT/COTP carry buffers, S7comm + // classification state, and per-direction dedup flags. + // ========================================================================= + + /// AC-187-001: `S7commFlowState` carries the full STORY-187 field set — + /// `session_established: bool`, `cr_observed_dir: Option`, + /// `classified_protocol: Option`, `malformed_header_reported_c2s: bool`, + /// `malformed_header_reported_s2c: bool` — in addition to STORY-186's carry + /// fields, and the first `on_data` call for a newly classified flow with zero + /// bytes delivered leaves every field at its documented default (BC-2.21.001's + /// own canonical test vector). + /// + /// NOTE: this test exercises only the struct's field defaults + /// (`#[derive(Default)]`) and `on_data`'s lazy-creation/frame-walk logic on an + /// EMPTY delivery — the frame-walk loop breaks immediately for `data.len() < 4` + /// remaining, before any COTP/S7comm dispatch is reached — so this is a + /// regression guard for AC-187-001's field-set/default-value contract, + /// independent of the F-01/F-02/F-12 CR/CC-and-classification dispatch logic + /// other tests in this module exercise directly. + /// + /// Traces: BC-2.21.001 postcondition 1, Canonical Test Vectors row 1; AC-187-001. + #[test] + fn test_BC_2_21_001_flow_state_field_set() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + analyzer.on_data(flow_key.clone(), &[], 0, Direction::ClientToServer); + + let state = analyzer + .flows + .get(&flow_key) + .expect("S7commFlowState must be created on the first on_data call"); + assert!( + state.carry_c2s.is_empty(), + "carry_c2s must default to empty" + ); + assert!( + state.carry_s2c.is_empty(), + "carry_s2c must default to empty" + ); + assert!( + !state.carry_overflow_reported_c2s, + "carry_overflow_reported_c2s must default to false" + ); + assert!( + !state.carry_overflow_reported_s2c, + "carry_overflow_reported_s2c must default to false" + ); + assert!( + !state.session_established, + "session_established must default to false (BC-2.21.001 postcondition 1)" + ); + assert_eq!( + state.cr_observed_dir, None, + "cr_observed_dir (the F-01 pending-CR-direction tracking field) must \ + default to None -- no CR has been observed yet (BC-2.21.001 postcondition \ + 1, F-01 ruling, human-ratified 2026-09-24)" + ); + assert_eq!( + state.classified_protocol, None, + "classified_protocol must default to None (BC-2.21.001 postcondition 1)" + ); + assert!( + !state.malformed_header_reported_c2s, + "malformed_header_reported_c2s must default to false (BC-2.21.001 postcondition 1)" + ); + assert!( + !state.malformed_header_reported_s2c, + "malformed_header_reported_s2c must default to false (BC-2.21.001 postcondition 1)" + ); + } + + /// AC-187-002: `S7commFlowState` is created lazily on the first `on_data` call for + /// a newly classified flow and stored in the analyzer's per-flow map, keyed by + /// `FlowKey`. + /// + /// NOTE: same rationale as `test_BC_2_21_001_flow_state_field_set` above — lazy + /// creation via `self.flows.entry(flow_key).or_default()` originated in STORY-186 + /// and is exercised here on an empty delivery that never reaches the COTP/S7comm + /// dispatch path this round-2 addendum otherwise targets. + /// + /// Traces: BC-2.21.001 postcondition 3; AC-187-002. + #[test] + fn test_BC_2_21_001_lazy_flow_state_creation() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + assert!( + !analyzer.flows.contains_key(&flow_key), + "precondition: no S7commFlowState exists before the first on_data call" + ); + + analyzer.on_data(flow_key.clone(), &[], 0, Direction::ClientToServer); + + assert!( + analyzer.flows.contains_key(&flow_key), + "S7commFlowState must be created lazily on the first on_data call and stored \ + keyed by FlowKey (BC-2.21.001 postcondition 3)" + ); + } + + /// EC-001 (BC-2.21.001): a flow that never sends any bytes before close never gets + /// an `S7commFlowState` created — `on_flow_close` is a no-op for it. + /// + /// NOTE: a flow_key that `on_data` was never called for cannot appear in + /// `analyzer.flows` (a plain `HashMap` starts empty) — a structural guarantee + /// independent of any dispatch-logic behavior. + /// + /// Traces: BC-2.21.001 edge case EC-001. + #[test] + fn test_BC_2_21_001_never_touched_flow_state_never_created() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + assert!( + !analyzer.flows.contains_key(&flow_key), + "a flow_key never passed to on_data must never appear in the per-flow map \ + (BC-2.21.001 edge case EC-001)" + ); + analyzer.on_flow_close(flow_key.clone()); + assert!( + !analyzer.flows.contains_key(&flow_key), + "closing a flow that never received on_data must remain a no-op" + ); + assert!(analyzer.findings.is_empty()); + } + + /// EC-003 (BC-2.21.001) / Invariant 2: `carry_overflow_reported_c2s` and + /// `malformed_header_reported_c2s` are independent dedup flags — setting one does + /// not set or reset the other. `carry_overflow_reported_c2s` is set via SYNTHETIC + /// direct field injection (mirroring the `story_186` SYNTHETIC convention for the + /// carry-overflow guard, which is unreachable via real `on_data` traffic per + /// BC-2.20.014 v1.1); `malformed_header_reported_c2s` is then driven to `true` + /// via a genuine `on_data`-triggered malformed classic-S7comm-header condition + /// (a `Some(0x32)` DT frame with `data.len() < 10`, BC-2.21.004) on the SAME flow + /// direction, and both flags must end up independently `true`. + /// + /// Traces: BC-2.21.001 invariant 2, edge case EC-003. + #[test] + fn test_BC_2_21_001_dedup_flags_independent_c2s() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + { + let state = analyzer.flows.entry(flow_key.clone()).or_default(); + state.carry_overflow_reported_c2s = true; + } + + // A Some(0x32) DT frame whose payload is one byte (just the protocol-ID byte) + // -- data.len() == 1 < 10 inside parse_s7comm_header -- triggers the + // malformed-header T0814/dedup path (BC-2.21.004). + let frame = dt_frame(&[0x32]); + analyzer.on_data(flow_key.clone(), &frame, 0, Direction::ClientToServer); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + state.carry_overflow_reported_c2s, + "the SYNTHETIC pre-set carry_overflow_reported_c2s flag must remain true" + ); + assert!( + state.malformed_header_reported_c2s, + "malformed_header_reported_c2s must independently become true from the \ + genuine on_data-driven malformed-header condition (BC-2.21.001 invariant 2)" + ); + assert_eq!( + analyzer.findings.len(), + 1, + "the SYNTHETIC pre-set carry_overflow_reported_c2s flag must not itself \ + emit a finding, and the genuine malformed-header condition must emit \ + exactly one T0814 -- not zero, not more than one (P14-F-5c)" + ); + } + + // ========================================================================= + // BC-2.21.002: `S7commAnalyzer::on_data` four-way dispatch on + // `CotpHeader::protocol_id`. + // ========================================================================= + + /// AC-187-003: a COTP CR frame followed by a matching CC frame updates + /// `session_established`; no protocol classification occurs (classification is + /// deferred to the first DT frame). + /// + /// v1.1 F-01 consistency note: this test's CR (c2s) then CC (s2c) sequence is + /// already the F-01 opposite-direction "matching CC" case, so it does not + /// conflict with the v1.1 opposite-direction-CR->CC ruling; it remains a valid + /// positive case and is not renamed, alongside the new, more explicitly-named + /// `test_BC_2_21_001_cr_then_opposite_cc_sets_session_established` below (which + /// additionally covers the negative F-01 cases this test does not). + /// + /// Traces: BC-2.21.002 postcondition 2; AC-187-003. + #[test] + fn test_BC_2_21_002_cr_cc_updates_session_no_classification() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + analyzer.on_data(flow_key.clone(), &cr_frame(), 0, Direction::ClientToServer); + analyzer.on_data(flow_key.clone(), &cc_frame(), 1, Direction::ServerToClient); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + state.session_established, + "a CR followed by a matching CC must set session_established \ + (BC-2.21.002 postcondition 2, BC-2.21.001 postcondition 1)" + ); + assert_eq!( + state.classified_protocol, None, + "CR/CC frames must never trigger protocol classification -- classification \ + is deferred to the first DT frame regardless of session_established's value \ + (BC-2.21.002 postcondition 2)" + ); + assert!( + analyzer.findings.is_empty(), + "ordinary CR/CC session establishment must not emit any finding" + ); + } + + // ========================================================================= + // AC-187-003 (v1.1, F-01): session_established is set ONLY by a CC observed in + // the direction OPPOSITE a previously-observed CR on the same flow. CR-only, + // CC-only (no prior CR), CC-before-CR, and same-direction CC must all leave + // session_established false (human ruling, STORY-187 per-story adversarial pass + // 1, F-01, 2026-09-24; BC-2.21.001 postcondition 1, edge cases EC-004..EC-007). + // ========================================================================= + + /// AC-187-003 positive case: CR observed in direction A (c2s), then CC observed + /// in direction B (s2c), opposite of A -- session_established becomes true. + /// + /// Traces: BC-2.21.001 postcondition 1, Canonical Test Vectors row 2; AC-187-003. + #[test] + fn test_BC_2_21_001_cr_then_opposite_cc_sets_session_established() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + analyzer.on_data(flow_key.clone(), &cr_frame(), 0, Direction::ClientToServer); + analyzer.on_data(flow_key.clone(), &cc_frame(), 1, Direction::ServerToClient); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + state.session_established, + "a CC observed in the direction opposite a previously-observed CR must set \ + session_established (F-01 ruling, BC-2.21.001 postcondition 1)" + ); + } + + /// AC-187-003 negative case (a): a CR is observed, with no CC ever following on + /// this flow (CR-only) -- session_established must remain false. Scoped + /// EXCLUSIVELY to case (a) -- this test does NOT claim BC-2.21.001 EC-007 + /// (repeated same-direction CR), which is covered exclusively by + /// `test_BC_2_21_001_repeated_same_direction_cr_session_not_established` below + /// (F-22 ruling, human-ratified 2026-09-24). + /// + /// Traces: BC-2.21.001 postcondition 1; AC-187-003. + #[test] + fn test_BC_2_21_001_cr_only_session_not_established() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + analyzer.on_data(flow_key.clone(), &cr_frame(), 0, Direction::ClientToServer); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + !state.session_established, + "a CR with no CC ever following must leave session_established false \ + (F-01 ruling; a CR alone never sets the flag)" + ); + } + + /// AC-187-003 negative case (b): the flow's first observed COTP frame is a CC, + /// with no prior CR (CC-only, e.g. a mid-flow capture start) -- + /// session_established must remain false: a CC with no preceding CR cannot + /// "match" anything. + /// + /// Traces: BC-2.21.001 edge case EC-004; AC-187-003. + #[test] + fn test_BC_2_21_001_cc_only_no_prior_cr_session_not_established() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + analyzer.on_data(flow_key.clone(), &cc_frame(), 0, Direction::ServerToClient); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + !state.session_established, + "a CC observed with no prior CR on this flow must leave \ + session_established false (F-01 ruling, BC-2.21.001 edge case EC-004)" + ); + assert_eq!( + state.cr_observed_dir, None, + "a CC with no prior CR must never populate cr_observed_dir -- only a CR \ + frame writes this field, and none has been observed on this flow \ + (P11-F-3, BC-2.21.001 edge case EC-004)" + ); + } + + /// P12-F-1: `cr_observed_dir` reflects the MOST RECENT CR on the flow, not the + /// first -- a subsequent CR in a different direction overwrites the field + /// (`dispatch_cotp_frame`'s `ConnectRequest` arm always writes + /// `state.cr_observed_dir = Some(direction)`, unconditionally, never guarded by + /// "only if unset"). Verified both ways: a CC opposite the MOST RECENT CR sets + /// `session_established`, and a CC matching the MOST RECENT CR's direction (which + /// happens to be the SAME direction as the now-stale first CR) does not. + /// + /// Traces: BC-2.21.001 postcondition 1's "most recently observed COTP CR" + /// framing; F-01 ruling; BC-2.21.001 EC-008, EC-009 (P13-F-2). + #[test] + fn test_BC_2_21_001_most_recent_cr_direction_wins() { + // CR(c2s), CR(s2c), CC(c2s) -- the MOST RECENT CR is s2c, and c2s is opposite + // it, so session_established must become true. Under a (buggy) + // first-CR-wins implementation, cr_observed_dir would still read Some(c2s) + // after the second CR, making this CC SAME-direction (not opposite), + // leaving session_established false -- the two implementations disagree + // here. + { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + analyzer.on_data(flow_key.clone(), &cr_frame(), 0, Direction::ClientToServer); + analyzer.on_data(flow_key.clone(), &cr_frame(), 1, Direction::ServerToClient); + analyzer.on_data(flow_key.clone(), &cc_frame(), 2, Direction::ClientToServer); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + state.session_established, + "a CC opposite the MOST RECENT CR's direction (s2c) must set \ + session_established, even though it matches the FIRST CR's \ + direction (c2s) -- most-recent-CR-wins, not first-CR-wins \ + (P12-F-1, BC-2.21.001 postcondition 1)" + ); + } + + // CR(c2s), CR(s2c), CC(s2c) -- the MOST RECENT CR is s2c, and this CC is + // SAME-direction, so session_established must remain false. Under a + // first-CR-wins implementation, cr_observed_dir would still read Some(c2s), + // making this CC opposite-direction and incorrectly setting + // session_established true -- the two implementations disagree here too. + { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + analyzer.on_data(flow_key.clone(), &cr_frame(), 0, Direction::ClientToServer); + analyzer.on_data(flow_key.clone(), &cr_frame(), 1, Direction::ServerToClient); + analyzer.on_data(flow_key.clone(), &cc_frame(), 2, Direction::ServerToClient); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + !state.session_established, + "a CC in the SAME direction as the MOST RECENT CR (s2c) must leave \ + session_established false, even though it is OPPOSITE the FIRST \ + CR's direction (c2s) -- most-recent-CR-wins, not first-CR-wins \ + (P12-F-1, BC-2.21.001 postcondition 1)" + ); + } + } + + /// AC-187-003 negative case (c): a CC is observed BEFORE any CR (out-of-order + /// arrival) -- session_established must remain false when the CC arrives, and a + /// CR that subsequently arrives (in either direction) does not retroactively set + /// it from the earlier CC: matching is forward-looking from the CR only. + /// + /// Traces: BC-2.21.001 edge case EC-005; AC-187-003. + #[test] + fn test_BC_2_21_001_cc_before_cr_session_not_established() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + // CC arrives first (s2c), then a CR arrives afterward (c2s) -- opposite + // direction of the CC, which per F-01 is still not a retroactive match. + analyzer.on_data(flow_key.clone(), &cc_frame(), 0, Direction::ServerToClient); + { + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + !state.session_established, + "session_established must remain false immediately after an \ + out-of-order CC with no CR yet observed (BC-2.21.001 edge case EC-005)" + ); + } + + analyzer.on_data(flow_key.clone(), &cr_frame(), 1, Direction::ClientToServer); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + !state.session_established, + "a CR arriving AFTER an out-of-order CC must not retroactively set \ + session_established from that earlier CC -- matching is forward-looking \ + from the CR only, never backward-looking from the CC (F-01 ruling, \ + BC-2.21.001 edge case EC-005)" + ); + } + + /// AC-187-003 negative case (d): a CR is observed in direction A, then a CC is + /// observed also in direction A (same direction, not opposite) -- + /// session_established must remain false: a same-direction CC is never + /// "matching." + /// + /// Traces: BC-2.21.001 edge case EC-006, Canonical Test Vectors row 3; AC-187-003. + #[test] + fn test_BC_2_21_001_same_direction_cc_session_not_established() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + analyzer.on_data(flow_key.clone(), &cr_frame(), 0, Direction::ClientToServer); + analyzer.on_data(flow_key.clone(), &cc_frame(), 1, Direction::ClientToServer); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + !state.session_established, + "a CC observed in the SAME direction as the flow's already-observed CR \ + must leave session_established false (F-01 ruling, BC-2.21.001 edge case \ + EC-006, Canonical Test Vectors row 3)" + ); + } + + /// AC-187-003 negative case (e), F-22 (human-ratified 2026-09-24): a CR is + /// observed in direction A, then a SECOND CR is also observed in direction A, + /// with no intervening CC (repeated same-direction CR) -- session_established + /// must remain false, and `cr_observed_dir` must still equal `Some(A)`: the + /// repeated CR does not clear or corrupt the pending direction, and does not + /// itself set session_established. + /// + /// Traces: BC-2.21.001 edge case EC-007; AC-187-003. + #[test] + fn test_BC_2_21_001_repeated_same_direction_cr_session_not_established() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + analyzer.on_data(flow_key.clone(), &cr_frame(), 0, Direction::ClientToServer); + analyzer.on_data(flow_key.clone(), &cr_frame(), 1, Direction::ClientToServer); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + !state.session_established, + "a repeated CR in the same direction, with no intervening CC, must leave \ + session_established false (F-22 ruling, BC-2.21.001 edge case EC-007)" + ); + assert_eq!( + state.cr_observed_dir, + Some(Direction::ClientToServer), + "the repeated CR must not clear or corrupt the pending CR direction -- \ + cr_observed_dir must still reflect the (shared) direction A after the \ + second CR (F-22 ruling, BC-2.21.001 v1.4+ EC-007's deterministic \ + most-recent-CR-wins overwrite rule; both CRs share direction A, so the \ + overwrite is unobservable in this vector but the rule still applies)" + ); + } + + /// EC-001 (BC-2.21.002): a flow that observes only CR/CC frames, never a DT frame, + /// leaves `classified_protocol` as `None` for the flow's lifetime; no dissection of + /// any kind occurs. + /// + /// NOTE: `classify_first_dt_frame` is only ever invoked from the `DataTransfer` + /// arm of `dispatch_cotp_frame`'s match -- a flow that sends only CR/CC frames + /// never reaches that arm at all, structurally, independent of the F-01/F-02/F-12 + /// CR/CC-and-classification dispatch logic exercised elsewhere in this module. + /// This is a regression guard for AC-187's CR/CC-only edge case. + /// + /// Traces: BC-2.21.002 edge case EC-001. + #[test] + fn test_BC_2_21_002_cr_cc_only_never_classifies() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + analyzer.on_data(flow_key.clone(), &cr_frame(), 0, Direction::ClientToServer); + analyzer.on_data(flow_key.clone(), &cc_frame(), 1, Direction::ServerToClient); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.classified_protocol, None, + "a flow observing only CR/CC frames must never classify \ + (BC-2.21.002 edge case EC-001)" + ); + } + + /// AC-187-004 (v1.1): a DT frame with `protocol_id: Some(0x32)` dispatches to + /// classic S7comm dissection -- `parse_s7comm_header` is called on the slice + /// beginning at `payload_offset`, and after dispatch + /// `S7commFlowState.classified_protocol == Some(S7Protocol::Classic)`. Exercised + /// end-to-end via a minimal, fully valid Job PDU (empty parameter/data blocks, + /// BC-2.21.006 EC-001 shape): this produces no malformed-header finding at all + /// (the header parses cleanly and the BC-2.21.009 bounds check passes trivially + /// for `param_length == data_length == 0`). + /// + /// Traces: BC-2.21.002 postcondition 3; AC-187-004. + #[test] + fn test_BC_2_21_002_classic_s7comm_dispatch_asserts_classified_protocol_classic() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + let header = classic_header_bytes(0x01, 0x0001, 0x0000, 0x0000); // Job, empty blocks + let frame = dt_frame(&header); + analyzer.on_data(flow_key.clone(), &frame, 0, Direction::ClientToServer); + + assert!( + analyzer.findings.is_empty(), + "a well-formed, fully-parseable classic Job PDU with empty parameter/data \ + blocks must not emit any malformed-header finding (BC-2.21.002 \ + postcondition 3)" + ); + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + !state.malformed_header_reported_c2s, + "a well-formed classic Job PDU must not trip the malformed-header dedup flag" + ); + assert_eq!( + state.classified_protocol, + Some(S7Protocol::Classic), + "after a Some(0x32) DT frame is dispatched, classified_protocol must be \ + Some(S7Protocol::Classic) (AC-187-004, BC-2.21.002 postcondition 3)" + ); + } + + /// AC-187-004 (v1.1): a malformed (too-short, `data.len() < 10`) `0x32`-leading DT + /// frame arriving on a flow whose sticky `classified_protocol` is ALREADY + /// `Some(S7Protocol::Classic)` (established by an earlier, well-formed `0x32` DT + /// frame) emits exactly one T0814 for the malformed-header condition -- not more + /// than one, and not zero. Distinct from + /// `test_BC_2_21_004_len_shorter_than_10_returns_none_and_emits_t0814_once`, which + /// exercises the malformed frame as the flow's FIRST DT frame; here the flow is + /// already Classic-classified before the malformed frame arrives. + /// + /// Traces: BC-2.21.002 postcondition 3, BC-2.21.004 postcondition 4; AC-187-004. + #[test] + fn test_BC_2_21_002_malformed_0x32_frame_on_classic_flow_emits_exactly_one_t0814() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + // First: a well-formed classic Job PDU establishes Some(Classic). + let good_header = classic_header_bytes(0x01, 0x0001, 0x0000, 0x0000); + let good_frame = dt_frame(&good_header); + analyzer.on_data(flow_key.clone(), &good_frame, 0, Direction::ClientToServer); + { + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.classified_protocol, + Some(S7Protocol::Classic), + "precondition: the flow must already be sticky-classified Classic \ + before the malformed frame below arrives" + ); + } + assert!( + analyzer.findings.is_empty(), + "precondition: no findings yet" + ); + + // Second: a malformed (too-short) 0x32-leading DT frame on the SAME + // already-Classic-classified flow. + let malformed_frame = dt_frame(&[0x32]); + analyzer.on_data( + flow_key.clone(), + &malformed_frame, + 1, + Direction::ClientToServer, + ); + + assert_eq!( + analyzer.findings.len(), + 1, + "a malformed 0x32-leading DT frame on an already-Classic-classified flow \ + must emit exactly one T0814 -- not more than one, and not zero \ + (AC-187-004, BC-2.21.004 postcondition 4)" + ); + assert_malformed_header_t0814(&analyzer.findings[0], Direction::ClientToServer); + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.classified_protocol, + Some(S7Protocol::Classic), + "classified_protocol must remain Classic across the malformed frame -- \ + classification is sticky and unaffected by a later malformed-header \ + condition" + ); + } + + /// AC-187-005 / EC-002: on the first DT frame observed for a flow (protocol_id: + /// `Some(0x99)`, an unrecognized/unclassified value), `classified_protocol` is set + /// exactly once (to `Unclassified`); a LATER DT frame on the SAME flow carrying + /// `Some(0x32)` (a fully valid classic Job PDU) does NOT overwrite it. Per the + /// F-12 sticky-classification gate (AC-187-012), the classic dissection branch + /// does NOT run for this later frame either, since the flow's sticky + /// `classified_protocol` is already `Some(Unclassified)` -- BC-2.21.002's + /// dispatch match is per-frame, but BOTH the `classified_protocol` assignment AND + /// classic-dissection eligibility are gated on the STICKY value, never on the + /// current frame's raw `protocol_id` byte alone. + /// + /// Uses BC-2.21.001 EC-002's exact scenario ("First DT frame classifies + /// Unclassified, later frame on same flow carries Some(0x32)"). + /// + /// Traces: BC-2.21.002 postcondition 6; BC-2.21.001 edge case EC-002; AC-187-005. + #[test] + fn test_BC_2_21_002_sticky_first_classification() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + // First DT frame: protocol_id Some(0x99), unrecognized -> Unclassified. + let first = dt_frame(&[0x99]); + analyzer.on_data(flow_key.clone(), &first, 0, Direction::ClientToServer); + { + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.classified_protocol, + Some(S7Protocol::Unclassified), + "the first DT frame (protocol_id Some(0x99)) must classify the flow \ + Unclassified exactly once (BC-2.21.002 postcondition 6)" + ); + } + + // Second DT frame: protocol_id Some(0x32), a fully valid classic Job PDU. + let header = classic_header_bytes(0x01, 0x0002, 0x0000, 0x0000); + let second = dt_frame(&header); + analyzer.on_data(flow_key.clone(), &second, 1, Direction::ClientToServer); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.classified_protocol, + Some(S7Protocol::Unclassified), + "a later DT frame with a DIFFERENT protocol_id (Some(0x32)) must NOT \ + overwrite the sticky first classification (BC-2.21.002 postcondition 6, \ + BC-2.21.001 edge case EC-002)" + ); + } + + /// EC-002 (BC-2.21.002): a flow's very first observed frame is a DT frame with + /// `protocol_id: Some(0x32)` (no prior CR/CC observed). Classification proceeds + /// normally from the DT frame alone; `session_established` remains `false`. + /// + /// Traces: BC-2.21.002 edge case EC-002 (BC's own numbering; distinct from the + /// BC-2.21.001 EC-002 referenced above). + #[test] + fn test_BC_2_21_002_dt_first_frame_no_prior_cr_cc() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + let header = classic_header_bytes(0x01, 0x0001, 0x0000, 0x0000); + let frame = dt_frame(&header); + analyzer.on_data(flow_key.clone(), &frame, 0, Direction::ClientToServer); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.classified_protocol, + Some(S7Protocol::Classic), + "a DT frame with protocol_id Some(0x32) as the flow's very first observed \ + frame must classify Classic normally, without any prior CR/CC \ + (BC-2.21.002 edge case EC-002)" + ); + assert!( + !state.session_established, + "session_established must remain false when no CR/CC was ever observed \ + (BC-2.21.002 edge case EC-002) -- this must not block classic dissection" + ); + } + + // ========================================================================= + // AC-187-005 (v1.1, F-02): a DT frame whose protocol_id is None carries no + // protocol evidence and never classifies -- it does not consume the flow's + // "first DT frame" status. Classification remains deferred to a later DT frame + // (if any) that carries Some(byte) (human ruling, STORY-187 per-story + // adversarial pass 1, F-02, 2026-09-24; BC-2.21.002 postconditions 5-6, edge + // case EC-004; BC-2.21.001 postcondition 1). + // ========================================================================= + + /// AC-187-005: a flow's first DT frame carries `protocol_id: None` (empty + /// payload, via the genuinely-constructible `dt_frame_empty_payload()` helper) -- + /// `classified_protocol` remains `None` after it. A second DT frame on the same + /// flow then carries `protocol_id: Some(0x32)` (a fully valid classic Job PDU) -- + /// THIS frame is the one that sets `classified_protocol = + /// Some(S7Protocol::Classic)` and is dissected (since `classified_protocol` was + /// still unset at the moment this frame was dispatched). + /// + /// Traces: BC-2.21.002 postcondition 6, edge case EC-004; BC-2.21.001 edge case + /// EC-004 (parallel "no protocol evidence" framing); AC-187-005. + #[test] + fn test_BC_2_21_002_none_protocol_id_dt_first_then_0x32_dt_classifies_classic() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + // First DT frame: protocol_id None (empty upper-layer payload). + analyzer.on_data( + flow_key.clone(), + &dt_frame_empty_payload(), + 0, + Direction::ClientToServer, + ); + { + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.classified_protocol, None, + "a protocol_id: None DT frame carries no protocol evidence and must \ + NOT classify the flow, even as the flow's first DT frame (F-02 \ + ruling, BC-2.21.002 postcondition 6)" + ); + } + assert!( + analyzer.findings.is_empty(), + "a protocol_id: None DT frame must not itself emit any finding" + ); + + // Second DT frame: protocol_id Some(0x32), a fully valid classic Job PDU. + let header = classic_header_bytes(0x01, 0x0001, 0x0000, 0x0000); + let second = dt_frame(&header); + analyzer.on_data(flow_key.clone(), &second, 1, Direction::ClientToServer); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.classified_protocol, + Some(S7Protocol::Classic), + "the SECOND DT frame (the first to carry Some(byte)) must be the one that \ + sets classified_protocol -- classification was still deferred after the \ + first, None-protocol_id frame (F-02 ruling, BC-2.21.002 postcondition 6, \ + edge case EC-004)" + ); + assert!( + analyzer.findings.is_empty(), + "the well-formed classic Job PDU that classifies the flow must not itself \ + emit any malformed-header finding" + ); + } + + /// AC-187-005: two DT frames with different `protocol_id` values arrive + /// back-to-back within a SINGLE `on_data` call (mirrors BC-2.20.013's + /// multi-frame walk) -- `classified_protocol`'s first-write-wins rule applies + /// across the pair in arrival order: the first frame within the delivery sets + /// it, the second does not overwrite it, even though the frame-walk loop + /// dispatches both within the same `on_data` invocation. + /// + /// Traces: BC-2.21.002 edge case EC-003; AC-187-005. + #[test] + fn test_BC_2_21_002_two_frames_one_delivery_first_write_wins() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + // First frame: protocol_id Some(0x72) (Plus). Second frame, concatenated in + // the SAME delivery: protocol_id Some(0x32), a fully valid classic Job PDU. + let mut delivery = dt_frame(&[0x72]); + let classic_header = classic_header_bytes(0x01, 0x0001, 0x0000, 0x0000); + delivery.extend_from_slice(&dt_frame(&classic_header)); + + analyzer.on_data(flow_key.clone(), &delivery, 0, Direction::ClientToServer); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.classified_protocol, + Some(S7Protocol::Plus), + "within a single on_data delivery carrying two DT frames, the FIRST \ + frame's protocol_id (Some(0x72) -> Plus) must set classified_protocol; \ + the second frame (Some(0x32)) must not overwrite it -- first-write-wins \ + applies within a delivery, not merely across separate on_data calls \ + (BC-2.21.002 edge case EC-003)" + ); + } + + // ========================================================================= + // AC-187-012 (v1.1, F-12): classic S7comm dissection is gated on the flow's + // STICKY classified_protocol == Classic, never on the current frame's raw + // protocol_id byte alone. A 0x32-leading DT frame on a flow already + // sticky-classified Plus or Unclassified by an earlier DT frame is NOT + // dissected -- no parse_s7comm_header call, no finding (human ruling, STORY-187 + // per-story adversarial pass 1, F-12, 2026-09-24; BC-2.21.002 postcondition 3, + // invariant 4, edge case EC-005). + // ========================================================================= + + /// AC-187-012: a flow's first DT frame carries `protocol_id: Some(0x72)`, sticky + /// classifying it `Some(S7Protocol::Plus)`. A LATER DT frame on the same flow + /// carries `protocol_id: Some(0x32)` with a MALFORMED (too-short) classic-S7comm + /// payload -- per the F-12 sticky gate, it must NOT be dissected at all: no + /// `parse_s7comm_header` call is ever made, so NO finding of any kind is emitted + /// (deliberately not a well-formed payload here: a well-formed `0x32` payload + /// would emit zero findings regardless of whether the gate is correctly wired, + /// since a successful parse never raises a T0814 either way -- only a MALFORMED + /// payload distinguishes "correctly gated, never dissected" from "incorrectly + /// ungated, dissected and rejected as malformed"). `classified_protocol` remains + /// `Some(Plus)`. + /// + /// Also covers the parallel Unclassified case (first DT frame `Some(0x99)`, an + /// unrecognized byte) in the same test, per BC-2.21.002 edge case EC-005's + /// framing ("a flow sticky-classified Plus/Unclassified must never fall into + /// classic dissection"). + /// + /// Traces: BC-2.21.002 postcondition 3, invariant 4, edge case EC-005; AC-187-012. + #[test] + fn test_BC_2_21_002_0x32_dt_frame_not_dissected_when_sticky_classified_plus_or_unclassified() { + // Malformed classic-S7comm payload (too short: data.len() == 1 inside + // parse_s7comm_header) -- see the doc comment above for why the negative + // case must use a malformed, not well-formed, payload. + let malformed_classic = vec![0x32u8]; + + // --- Plus-classified flow. --- + { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + analyzer.on_data( + flow_key.clone(), + &dt_frame(&[0x72]), + 0, + Direction::ClientToServer, + ); + { + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!(state.classified_protocol, Some(S7Protocol::Plus)); + } + + let later = dt_frame(&malformed_classic); + analyzer.on_data(flow_key.clone(), &later, 1, Direction::ClientToServer); + + assert!( + analyzer.findings.is_empty(), + "a later, MALFORMED 0x32-leading DT frame on an already-Plus-classified \ + flow must emit NO finding at all -- the gate must prevent \ + parse_s7comm_header from ever being called for it, so the malformed \ + payload never reaches the malformed-header T0814 path either (F-12 \ + ruling, BC-2.21.002 postcondition 3, edge case EC-005)" + ); + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.classified_protocol, + Some(S7Protocol::Plus), + "classified_protocol must remain Some(Plus) -- a flow, once \ + classified, is never re-interpreted under a different protocol's \ + parser (F-12 ruling, ADR-014 Decision 2 no-misattribution guarantee)" + ); + assert!( + !state.malformed_header_reported_c2s, + "the malformed-header dedup flag must NOT be set -- the gated frame \ + was never passed to parse_s7comm_header at all (F-12 ruling)" + ); + } + + // --- Unclassified-classified flow. --- + { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + analyzer.on_data( + flow_key.clone(), + &dt_frame(&[0x99]), + 0, + Direction::ClientToServer, + ); + { + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!(state.classified_protocol, Some(S7Protocol::Unclassified)); + } + + let later = dt_frame(&malformed_classic); + analyzer.on_data(flow_key.clone(), &later, 1, Direction::ClientToServer); + + assert!( + analyzer.findings.is_empty(), + "a later, MALFORMED 0x32-leading DT frame on an already- \ + Unclassified-classified flow must emit NO finding at all (F-12 \ + ruling, BC-2.21.002 edge case \ + EC-005 / STORY-187 story-level EC-009)" + ); + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.classified_protocol, + Some(S7Protocol::Unclassified), + "classified_protocol must remain Some(Unclassified) (F-12 ruling)" + ); + } + } + + // ========================================================================= + // P12-F-3: dissection must be bounded to the CURRENT TPKT frame's own payload + // (`&frame[4..]`, `frame == &working[cursor..cursor + total]`) -- never to + // `&working[cursor + 4..]`, which would (with no upper bound) leak bytes from + // any SUBSEQUENT frame present later in the same `working` buffer/delivery into + // the current frame's `tpkt_payload`/COTP-payload/S7comm-payload slice. + // ========================================================================= + + /// P12-F-3: a single `on_data` delivery contains TWO complete, back-to-back TPKT + /// frames: first a DT frame whose classic S7comm Job header declares + /// `param_length == 2` but has ZERO parameter bytes actually present WITHIN ITS + /// OWN TPKT frame (the frame's declared TPKT `length` covers exactly the 10-byte + /// common header, nothing more); second, a complete COTP CR frame. + /// + /// Under the correct implementation, the DT frame's dissection is bounded to its + /// own frame -- `payload.len() == 10`, so `s7comm_bounds_ok` correctly fails + /// (`declared_total == 12 > 10`) and exactly one T0814 is emitted. Under the + /// `tpkt_payload = &working[cursor + 4..]` mutation (no upper bound), the DT + /// frame's `tpkt_payload` would instead extend all the way to the end of + /// `working`, absorbing the entire trailing CR frame's 7 bytes into what should + /// have been a 10-byte payload -- `payload.len()` would become 17, which + /// incorrectly PASSES the bounds check (`17 >= 12`), so the mutant emits ZERO + /// findings instead of one. + /// + /// Traces: BC-2.21.009 postcondition 2; BC-2.20.013 postcondition 1a (frame + /// boundary discipline). + #[test] + fn test_BC_2_21_009_dissection_bounded_to_own_tpkt_frame() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + // Job header (exactly 10 bytes, no trailing parameter/data bytes) declaring + // param_length=2, data_length=0 -- so the DT frame's own TPKT-declared + // length covers only these 10 bytes, none of the (falsely) declared 2 + // parameter bytes. + let header = classic_header_bytes(0x01, 0x0001, 0x0002, 0x0000); + assert_eq!(header.len(), 10); + let mut delivery = dt_frame(&header); + delivery.extend_from_slice(&cr_frame()); + + analyzer.on_data(flow_key.clone(), &delivery, 0, Direction::ClientToServer); + + assert_eq!( + analyzer.findings.len(), + 1, + "the DT frame's bounds-check failure (param_length=2 declared, 0 bytes \ + present WITHIN ITS OWN FRAME) must emit exactly one T0814, undiminished \ + by the trailing CR frame present later in the same delivery (P12-F-3)" + ); + assert_malformed_header_t0814(&analyzer.findings[0], Direction::ClientToServer); + assert_reason_specific_evidence(&analyzer.findings[0], "exceed available bytes"); + + // The trailing CR frame must still have been walked and dispatched normally + // (session-tracking state updated), confirming the cursor advanced correctly + // past the DT frame using its OWN declared TPKT length, not consumed by an + // over-wide tpkt_payload slice. + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.cr_observed_dir, + Some(Direction::ClientToServer), + "the trailing CR frame must still be walked and dispatched (its own \ + cr_observed_dir side effect observed), confirming the cursor advanced \ + past the DT frame by exactly its own declared TPKT length (P12-F-3)" + ); + } + + /// P12-F-3 (mirror case): a single `on_data` delivery contains a DT frame with an + /// EMPTY upper-layer payload (`protocol_id: None`, via `dt_frame_empty_payload()` + /// -- `payload_offset == tpkt_payload.len()` exactly, within its OWN frame), + /// immediately followed by a complete CR frame in the SAME delivery. + /// + /// Under the correct implementation, the DT frame's `tpkt_payload` is bounded to + /// its own 3-byte COTP fixed part, so `header.protocol_id` correctly evaluates to + /// `None` (no byte exists at `payload_offset` within this frame) and + /// `classified_protocol` stays `None` (F-02: a `None`-protocol_id DT frame never + /// classifies). Under the `tpkt_payload = &working[cursor + 4..]` mutation, the + /// DT frame's `tpkt_payload` would instead leak into the trailing CR frame's + /// bytes, so `tpkt_payload[payload_offset]` would read the CR frame's leading + /// TPKT version byte (`0x03`) instead of being out-of-bounds -- producing + /// `protocol_id: Some(0x03)`, which classifies the flow `Unclassified` instead of + /// leaving it `None`. + /// + /// Traces: BC-2.21.002 postcondition 6; BC-2.20.010 (`protocol_id: None` no + /// out-of-bounds read); BC-2.20.013 postcondition 1a (frame boundary discipline). + #[test] + fn test_BC_2_21_002_empty_dt_followed_by_frame_same_delivery_stays_unclassified() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + let mut delivery = dt_frame_empty_payload(); + delivery.extend_from_slice(&cr_frame()); + + analyzer.on_data(flow_key.clone(), &delivery, 0, Direction::ClientToServer); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.classified_protocol, None, + "a protocol_id: None DT frame (empty payload WITHIN ITS OWN FRAME) \ + followed by a trailing CR frame in the same delivery must leave \ + classified_protocol at None -- the DT frame's protocol_id evaluation \ + must never read into the trailing frame's bytes (P12-F-3, F-02)" + ); + assert!( + analyzer.findings.is_empty(), + "neither the empty-payload DT frame nor the trailing CR frame emits any \ + finding" + ); + } + + // ========================================================================= + // AC-187-013 (F-19, policy DF-CANONICAL-FRAME-HOLDOUT-001): a canonical, + // independently-sourced classic S7comm Setup Communication frame pair. + // ========================================================================= + + /// AC-187-013 / F-19 / policy DF-CANONICAL-FRAME-HOLDOUT-001: a canonical, + /// INDEPENDENTLY-SOURCED classic S7comm Setup Communication (function code + /// `0xF0`) Job request, wrapped in real ISO-on-TCP framing (RFC 1006 TPKT + /// header + standard class-0 COTP DT `02 F0 80`). Its companion Ack_Data + /// response frame is covered by + /// `test_BC_2_21_008_canonical_ack_data_setup_communication_response_on_data` + /// below (split out per STORY-187 v1.3's per-BC test naming). + /// + /// **Primary source** (byte sequence taken verbatim): cnblogs, + /// "西门子S7通讯协议引用整理" ("Siemens S7 Communication Protocol Reference + /// Compilation"), -- + /// Setup Communication request/response example. + /// + /// **Corroborating sources** (same field layout, different client-chosen PDU + /// reference/PDU-size values -- confirming the layout recurs across + /// independently-authored examples, not an artifact of the one primary source): + /// - Yiqisoft blog, "PLC｜Golang 连接西门子Siemens S7/TCP 协议读取数据" + /// (2023-03-22), + /// (gos7-generated log: PDU reference `0x0400`, PDU size `480`). + /// - Inductive Automation Support KB, "Loggers - Device Connections: Siemens", + /// + /// (PDU reference `0x0002`, PDU size `240`). + /// + /// The PDU reference value (`0xFFFF` in the primary source's example) is a + /// client-chosen, connection-scoped sequence value, not a protocol constant -- + /// so the corroborating sources' differing PDU-reference/PDU-size values do not + /// conflict with the primary source; they confirm the same FIELD LAYOUT + /// (rosctr / pdu_reference / param_length / data_length / [error_class / + /// error_code] / parameter block) recurs across independently-authored + /// real-world examples. + /// + /// Per policy DF-CANONICAL-FRAME-HOLDOUT-001: the TPKT length field, the + /// `02 F0 80` COTP DT header bytes, and the S7comm common-header/Setup- + /// Communication-parameter bytes below are NOT derived from this project's own + /// BCs, ADR-014, or any other project artifact -- they are copied verbatim from + /// the primary source cited above. + /// + /// These publicly posted wire-capture byte examples are used here strictly as + /// TEST-VECTOR SOURCES, as permitted by the ADR-014 Decision 4 reconciliation + /// note (2026-09-24, DF-CANONICAL-FRAME-HOLDOUT-001); parser design and field + /// semantics continue to derive from Decision 4's prose sources and permitted + /// design references, never from these wire-capture test vectors. + /// + /// Traces: BC-2.21.002 postcondition 3, BC-2.21.006; AC-187-013. + #[test] + fn test_BC_2_21_006_canonical_setup_communication_job_frame_on_data() { + // Canonical Setup Communication Job request (25 bytes total): RFC 1006 TPKT + // header (03 00 00 19) + class-0 COTP DT (02 F0 80) + classic S7comm Job + // header/params, verbatim from the cnblogs primary source cited above. + const JOB_FRAME: [u8; 25] = [ + 0x03, 0x00, 0x00, 0x19, 0x02, 0xF0, 0x80, 0x32, 0x01, 0x00, 0x00, 0xFF, 0xFF, 0x00, + 0x08, 0x00, 0x00, 0xF0, 0x00, 0x00, 0x01, 0x00, 0x01, 0x07, 0x80, + ]; + + // --- on_data-level assertions: the Job frame dispatches cleanly. --- + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + analyzer.on_data(flow_key.clone(), &JOB_FRAME, 0, Direction::ClientToServer); + + assert!( + analyzer.findings.is_empty(), + "the canonical Setup Communication Job request frame must produce no \ + findings -- it is well-formed under the BC-2.21.009 bounds check \ + (AC-187-013)" + ); + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.classified_protocol, + Some(S7Protocol::Classic), + "the canonical Job frame's protocol_id (0x32) must sticky-classify the \ + flow Classic (AC-187-013, BC-2.21.002 postcondition 3)" + ); + + // --- Direct parse_s7comm_header assertion on the isolated S7 slice. --- + // TPKT (4 bytes) + COTP DT fixed part (3 bytes) = 7-byte prefix. + let job_s7 = &JOB_FRAME[7..]; + let job_header = + parse_s7comm_header(job_s7).expect("canonical Job header must parse (AC-187-013)"); + assert_eq!(job_header.rosctr, Rosctr::Job); + assert_eq!( + job_header.pdu_reference, 0xFFFF, + "PDU reference is client-chosen (0xFFFF in the primary source's example) \ + -- not a protocol constant" + ); + assert_eq!(job_header.param_length, 8); + assert_eq!(job_header.data_length, 0); + assert_eq!(job_header.header_len, 10); + assert_eq!(job_header.error_class, None); + assert_eq!(job_header.error_code, None); + } + + /// AC-187-013 / F-19 / canonical-frame holdout ruling + /// (DF-CANONICAL-FRAME-HOLDOUT-001, human-ratified 2026-09-24): the companion + /// canonical, independently-sourced classic S7comm Setup Communication + /// **Ack_Data (`0x03`) response** frame, wrapped in the same RFC 1006 TPKT + + /// standard class-0 COTP DT (`02 F0 80`) framing as + /// `test_BC_2_21_006_canonical_setup_communication_job_frame_on_data` above. + /// + /// **Primary source** (byte sequence taken verbatim): cnblogs, + /// "西门子S7通讯协议引用整理" ("Siemens S7 Communication Protocol Reference + /// Compilation"), -- full + /// 27-byte frame `03 00 00 1B 02 F0 80 32 03 00 00 FF FF 00 08 00 00 00 00 F0 + /// 00 00 01 00 01 00 F0`. + /// + /// **Corroborating sources** (same field layout -- an Ack_Data Setup + /// Communication response ALSO carries Error Class (`data[10]`) / Error Code + /// (`data[11]`), with the parameter block starting at `data[12]`, exactly as + /// documented in BC-2.21.008's Canonical Test Vectors -- these are byte-level + /// TEST-VECTOR corroboration only): + /// - Yiqisoft blog (2023-03-22), + /// - Inductive Automation Support KB, "Loggers - Device Connections: Siemens" + /// + /// **Field-semantics grounding** (separate from the test-vector corroboration + /// above; see ADR-014 Decision 4 / Decision 9 notes for the full citations): + /// Kleinmann & Wool 2014 (JDFSL 9(2), Fig. 2) is a PROSE source attesting the + /// Ack_Data (`0x03`) 12-byte header only. The Ack (`0x02`) 12-byte layout and the + /// 1-byte `error_class`/`error_code` split rest on Decision 4's PERMITTED DESIGN + /// REFERENCES -- cisagov/icsnpp-s7comm and gijzelaerr/python-snap7 (kprovost/libs7comm + /// consistent in aggregate) -- not on Kleinmann & Wool, which does not attest Ack. + /// + /// ## Resolution of the round-3 discrepancy (DO NOT relitigate by relaxing + /// ## assertions) + /// + /// The primary source's Ack_Data response byte layout is only self-consistent + /// under a 12-byte Ack_Data header -- i.e. `rosctr == AckData` carrying + /// `error_class`/`error_code` at `data[10]`/`data[11]`, with the + /// Setup-Communication response parameter block (`F0 00 00 01 00 01 00 F0`: + /// function code `0xF0` + reserved + max-AMQ-calling + max-AMQ-called + + /// PDU-length, exactly matching the declared `param_length == 8`) beginning at + /// byte 12. This round-3 discrepancy (an earlier revision of this test asserted + /// this same 12-byte shape as "EXPECTED TO FAIL") is now the RATIFIED, + /// human-ratified behavior per DF-CANONICAL-FRAME-HOLDOUT-001 (2026-09-24): + /// BC-2.21.008 / `parse_s7comm_header` extends the 12-byte header + /// (`error_class`/`error_code`) to BOTH `rosctr == Ack` (`0x02`) AND + /// `rosctr == AckData` (`0x03`) -- the assertions below are ordinary, load-bearing + /// GREEN-gate assertions, not a documented discrepancy. + /// + /// These publicly posted wire-capture byte examples are used here strictly as + /// TEST-VECTOR SOURCES, as permitted by the ADR-014 Decision 4 reconciliation + /// note (2026-09-24, DF-CANONICAL-FRAME-HOLDOUT-001); parser design and field + /// semantics continue to derive from Decision 4's prose sources and permitted + /// design references (see the field-semantics grounding note above), never from + /// these wire-capture test vectors. + /// + /// Traces: BC-2.21.002 postcondition 3, BC-2.21.008 postconditions 1-2; + /// AC-187-013. + #[test] + fn test_BC_2_21_008_canonical_ack_data_setup_communication_response_on_data() { + // Canonical Setup Communication Ack_Data response (27 bytes total), + // verbatim from the cnblogs primary source cited above. + const ACK_DATA_FRAME: [u8; 27] = [ + 0x03, 0x00, 0x00, 0x1B, 0x02, 0xF0, 0x80, 0x32, 0x03, 0x00, 0x00, 0xFF, 0xFF, 0x00, + 0x08, 0x00, 0x00, 0x00, 0x00, 0xF0, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0xF0, + ]; + + // --- on_data-level assertions: the Ack_Data frame dispatches cleanly. --- + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + // Ack_Data is the server's response frame, so it is fed as ServerToClient + // (the response direction) -- corrected from an earlier revision that fed + // it as ClientToServer; classification and the findings assertion are + // direction-independent, so this does not change what the test proves. + analyzer.on_data( + flow_key.clone(), + &ACK_DATA_FRAME, + 0, + Direction::ServerToClient, + ); + + assert!( + analyzer.findings.is_empty(), + "the canonical Setup Communication Ack_Data response frame must produce \ + no findings -- it is well-formed under the corrected 12-byte-header \ + BC-2.21.009 bounds check (AC-187-013)" + ); + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.classified_protocol, + Some(S7Protocol::Classic), + "the canonical Ack_Data frame's protocol_id (0x32) must sticky-classify \ + the flow Classic (AC-187-013, BC-2.21.002 postcondition 3)" + ); + + // --- Direct parse_s7comm_header assertion on the isolated S7 slice. --- + // TPKT (4 bytes) + COTP DT fixed part (3 bytes) = 7-byte prefix. + let ack_data_s7 = &ACK_DATA_FRAME[7..]; + let ack_data_header = parse_s7comm_header(ack_data_s7) + .expect("canonical Ack_Data header must parse (AC-187-013)"); + assert_eq!(ack_data_header.rosctr, Rosctr::AckData); + assert_eq!( + ack_data_header.pdu_reference, 0xFFFF, + "PDU reference is client-chosen (0xFFFF in the primary source's example) \ + -- not a protocol constant" + ); + assert_eq!(ack_data_header.param_length, 8); + assert_eq!(ack_data_header.data_length, 0); + assert_eq!( + ack_data_header.header_len, 12, + "Ack_Data (0x03) requires the 12-byte header per the 2026-09-24 \ + canonical-frame holdout ruling (DF-CANONICAL-FRAME-HOLDOUT-001) -- the \ + real-world parameter block only aligns at byte 12, not byte 10 (the \ + superseded v1.0/v1.1 assumption)" + ); + assert_eq!( + ack_data_header.error_class, + Some(0x00), + "error_class must be extracted from data[10] for Ack_Data \ + (BC-2.21.008 postcondition 2, DF-CANONICAL-FRAME-HOLDOUT-001)" + ); + assert_eq!( + ack_data_header.error_code, + Some(0x00), + "error_code must be extracted from data[11] for Ack_Data \ + (BC-2.21.008 postcondition 2, DF-CANONICAL-FRAME-HOLDOUT-001)" + ); + } + + // ========================================================================= + // BC-2.21.004: `parse_s7comm_header` returns None for input shorter than 10 bytes. + // ========================================================================= + + /// BC-2.21.004 canonical test vectors: `[]` (0 bytes), an 8-byte input, and a + /// 9-byte input must all return `None` from `parse_s7comm_header` directly. + /// + /// Traces: BC-2.21.004 postconditions 1-2, Canonical Test Vectors rows 1-3. + #[test] + fn test_BC_2_21_004_parse_returns_none_for_len_lt_10() { + assert_eq!(parse_s7comm_header(&[]), None, "0 bytes must return None"); + assert_eq!( + parse_s7comm_header(&[0x32, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x02]), + None, + "8 bytes (canonical vector) must return None" + ); + assert_eq!( + parse_s7comm_header(&[0x32, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x02, 0x00]), + None, + "9 bytes (canonical vector, one byte short) must return None" + ); + } + + /// AC-187-006: the on_data-driven malformed-header path (a `Some(0x32)` DT frame + /// whose payload is shorter than the 10-byte common-header minimum) returns + /// `None` from `parse_s7comm_header` and causes `S7commAnalyzer` to emit exactly + /// one T0814 on first occurrence per flow direction; a second occurrence on the + /// SAME direction does not re-emit (BC-2.21.004 edge case EC-004 dedup). + /// + /// Traces: BC-2.21.004 postcondition 4, edge case EC-004; AC-187-006. + #[test] + fn test_BC_2_21_004_len_shorter_than_10_returns_none_and_emits_t0814_once() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + // Payload = just the protocol-ID byte -> data.len() == 1 inside + // parse_s7comm_header, well under the 10-byte minimum. A NON-ZERO ts + // (424242) is used so the timestamp assertion below cannot vacuously pass + // against chrono's zero-epoch default (P14-F-5b). + let frame = dt_frame(&[0x32]); + analyzer.on_data(flow_key.clone(), &frame, 424_242, Direction::ClientToServer); + + assert_eq!( + analyzer.findings.len(), + 1, + "the first malformed-length classic-S7comm-header condition on this \ + direction must emit exactly one T0814 (BC-2.21.004 postcondition 4)" + ); + assert_malformed_header_t0814(&analyzer.findings[0], Direction::ClientToServer); + assert_reason_specific_evidence(&analyzer.findings[0], "header too short"); + assert_eq!( + analyzer.findings[0].timestamp, + DateTime::from_timestamp(424_242i64, 0), + "the finding's timestamp must equal chrono::DateTime::from_timestamp(ts as \ + i64, 0) -- the exact conversion report_malformed_header performs in \ + src/analyzer/s7comm.rs (P14-F-5b) -- got {:?}", + analyzer.findings[0].timestamp + ); + { + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + state.malformed_header_reported_c2s, + "malformed_header_reported_c2s must be set after the first occurrence" + ); + } + + // A second occurrence of the SAME malformed condition, same direction, same + // flow -- must not re-emit (BC-2.21.004 edge case EC-004). + analyzer.on_data(flow_key.clone(), &frame, 1, Direction::ClientToServer); + assert_eq!( + analyzer.findings.len(), + 1, + "a second malformed-length occurrence on the same flow direction must NOT \ + emit an additional T0814 -- the dedup flag suppresses re-emission \ + (BC-2.21.004 edge case EC-004)" + ); + } + + /// AC-187-006 (v1.1): per-direction dedup verified independently for the s2c + /// direction -- a c2s malformed-length occurrence and an independent s2c + /// malformed-length occurrence on the SAME flow must each emit their own T0814 + /// (two total), and a REPEATED s2c occurrence must not emit a third. + /// + /// Traces: BC-2.21.004 postcondition 4, edge case EC-004; BC-2.21.001 invariant 2 + /// (per-direction dedup independence); AC-187-006. + #[test] + fn test_BC_2_21_004_len_shorter_than_10_emits_t0814_once_s2c() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + let frame = dt_frame(&[0x32]); // data.len() == 1 inside parse_s7comm_header + + // c2s malformed occurrence. + analyzer.on_data(flow_key.clone(), &frame, 0, Direction::ClientToServer); + assert_eq!( + analyzer.findings.len(), + 1, + "the first c2s malformed-length occurrence must emit one T0814" + ); + + // First s2c malformed occurrence -- independent of the c2s dedup flag. + analyzer.on_data(flow_key.clone(), &frame, 1, Direction::ServerToClient); + assert_eq!( + analyzer.findings.len(), + 2, + "the first s2c malformed-length occurrence must emit its OWN T0814, \ + independently of the c2s direction's dedup flag already being set \ + (BC-2.21.001 invariant 2)" + ); + assert_malformed_header_t0814(&analyzer.findings[1], Direction::ServerToClient); + assert_reason_specific_evidence(&analyzer.findings[1], "header too short"); + assert_eq!( + analyzer.findings[1].timestamp, + DateTime::from_timestamp(1i64, 0), + "the s2c finding's timestamp must equal chrono::DateTime::from_timestamp(ts \ + as i64, 0) for the NON-ZERO ts=1 this on_data call used -- the exact \ + conversion report_malformed_header performs in src/analyzer/s7comm.rs \ + (P14-F-5b) -- got {:?}", + analyzer.findings[1].timestamp + ); + { + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + state.malformed_header_reported_s2c, + "malformed_header_reported_s2c must be set after the first s2c \ + occurrence" + ); + } + + // A repeated s2c occurrence must not re-emit. + analyzer.on_data(flow_key.clone(), &frame, 2, Direction::ServerToClient); + assert_eq!( + analyzer.findings.len(), + 2, + "a repeated s2c malformed-length occurrence must NOT emit a third T0814 \ + (BC-2.21.004 edge case EC-004, s2c direction)" + ); + } + + // ========================================================================= + // BC-2.21.005: `parse_s7comm_header` defensively rejects `data[0] != 0x32`. + // ========================================================================= + + /// AC-187-007: `data[0] == 0x72` (would only occur via a caller defect) is + /// rejected with `None`; this is a pure-function hygiene contract exercised via a + /// direct call, never reachable via BC-2.21.002's real dispatch, and emits no + /// finding. + /// + /// Traces: BC-2.21.005 postconditions 1-3, Canonical Test Vectors; AC-187-007. + #[test] + fn test_BC_2_21_005_defensive_reject_wrong_protocol_id_byte() { + let data = [0x72u8, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x02, 0x00, 0x00]; + assert_eq!( + parse_s7comm_header(&data), + None, + "data[0] == 0x72 must be rejected defensively (BC-2.21.005 postcondition 1)" + ); + } + + /// BC-2.21.005 canonical test vector: `data[0] == 0x00`. + /// + /// Traces: BC-2.21.005 edge case EC-002, Canonical Test Vectors. + #[test] + fn test_BC_2_21_005_defensive_reject_zero_byte() { + let data = [0x00u8, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x02, 0x00, 0x00]; + assert_eq!( + parse_s7comm_header(&data), + None, + "data[0] == 0x00 must be rejected defensively (BC-2.21.005 edge case EC-002)" + ); + } + + // ========================================================================= + // BC-2.21.006: `parse_s7comm_header` extracts the common header (Job/Userdata + // happy path; Ack/Ack_Data -> BC-2.21.008). + // ========================================================================= + + /// AC-187-008: the two BC-2.21.006 canonical test vectors (Job, Userdata) each + /// extract the exact expected `S7commHeader` -- full structural equality, not + /// merely `is_some()`. Ack_Data (`0x03`) is NOT part of this BC's happy path as + /// of the 2026-09-24 canonical-frame holdout ruling + /// (DF-CANONICAL-FRAME-HOLDOUT-001) -- it now requires the 12-byte header + /// covered by BC-2.21.008 (see + /// `test_BC_2_21_008_ack_data_12_byte_header_and_error_fields`); a 10-byte + /// Ack_Data header is truncated and returns `None` (BC-2.21.006 v1.1's + /// Canonical Test Vectors note). + /// + /// Traces: BC-2.21.006 postconditions 1-5, Canonical Test Vectors; AC-187-008. + #[test] + fn test_BC_2_21_006_common_header_field_extraction() { + // Job, minimal: 32 01 00 00 00 01 00 02 00 00 + let job = [0x32u8, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x02, 0x00, 0x00]; + assert_eq!( + parse_s7comm_header(&job), + Some(S7commHeader { + rosctr: Rosctr::Job, + pdu_reference: 1, + param_length: 2, + data_length: 0, + error_class: None, + error_code: None, + header_len: 10, + }), + "canonical Job vector must extract the exact expected S7commHeader" + ); + + // Userdata: 32 07 00 00 00 05 00 08 00 00 + let userdata = [0x32u8, 0x07, 0x00, 0x00, 0x00, 0x05, 0x00, 0x08, 0x00, 0x00]; + assert_eq!( + parse_s7comm_header(&userdata), + Some(S7commHeader { + rosctr: Rosctr::Userdata, + pdu_reference: 5, + param_length: 8, + data_length: 0, + error_class: None, + error_code: None, + header_len: 10, + }), + "canonical Userdata vector must extract the exact expected S7commHeader" + ); + + // Ack_Data at 10 bytes (the pre-ruling v1.0/v1.1 "happy path" shape) is now + // truncated -- it requires the 12-byte header (BC-2.21.008). + let ack_data_10_bytes = [0x32u8, 0x03, 0x00, 0x00, 0x00, 0x01, 0x00, 0x02, 0x00, 0x04]; + assert_eq!( + parse_s7comm_header(&ack_data_10_bytes), + None, + "a 10-byte Ack_Data header must return None -- Ack_Data (0x03) is no \ + longer part of BC-2.21.006's 10-byte happy-path group as of the \ + 2026-09-24 canonical-frame holdout ruling (DF-CANONICAL-FRAME-HOLDOUT-001); \ + see BC-2.21.008 for the 12-byte Ack_Data shape" + ); + } + + /// EC-003 (BC-2.21.006): non-zero Reserved bytes (`data[2..4]`) are extracted and + /// discarded, never causing rejection -- the field is read-but-unvalidated. + /// + /// Traces: BC-2.21.006 postcondition 5, edge case EC-003. + #[test] + fn test_BC_2_21_006_nonzero_reserved_bytes_do_not_reject() { + let data = [0x32u8, 0x01, 0xAB, 0xCD, 0x00, 0x01, 0x00, 0x02, 0x00, 0x00]; + assert_eq!( + parse_s7comm_header(&data), + Some(S7commHeader { + rosctr: Rosctr::Job, + pdu_reference: 1, + param_length: 2, + data_length: 0, + error_class: None, + error_code: None, + header_len: 10, + }), + "non-zero Reserved bytes must be extracted-and-discarded, never causing \ + rejection or altering any other field (BC-2.21.006 postcondition 5, \ + edge case EC-003)" + ); + } + + // ========================================================================= + // BC-2.21.007: `parse_s7comm_header` returns None for an unrecognized ROSCTR byte. + // ========================================================================= + + /// AC-187-009 (test name per story): the three BC-2.21.007 canonical + /// unrecognized-ROSCTR test vectors (`0x00`, `0x04`, `0xFF`) each return `None`. + /// + /// Traces: BC-2.21.007 postconditions 1-2, Canonical Test Vectors; AC-187-009. + #[test] + fn test_BC_2_21_007_unrecognized_rosctr_returns_none() { + let base = |rosctr: u8| { + [ + 0x32u8, rosctr, 0x00, 0x00, 0x00, 0x01, 0x00, 0x02, 0x00, 0x00, + ] + }; + assert_eq!( + parse_s7comm_header(&base(0x00)), + None, + "ROSCTR 0x00 (canonical vector) must return None" + ); + assert_eq!( + parse_s7comm_header(&base(0x04)), + None, + "ROSCTR 0x04 (canonical vector -- not to be confused with FC 0x04) must \ + return None" + ); + assert_eq!( + parse_s7comm_header(&base(0xFF)), + None, + "ROSCTR 0xFF (canonical vector) must return None" + ); + } + + /// AC-187-009: an unrecognized-ROSCTR malformed condition shares the SAME + /// per-direction dedup flag as BC-2.21.004's too-short condition -- a + /// too-short frame followed by an unrecognized-ROSCTR frame on the SAME + /// direction/flow must emit only the FIRST T0814, not a second one. + /// + /// Traces: BC-2.21.007 postcondition 3. + #[test] + fn test_BC_2_21_007_shares_dedup_flag_with_004_malformed_header() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + // First malformed condition: too-short (BC-2.21.004). + let too_short = dt_frame(&[0x32]); + analyzer.on_data(flow_key.clone(), &too_short, 0, Direction::ClientToServer); + assert_eq!( + analyzer.findings.len(), + 1, + "the first malformed condition must emit one T0814" + ); + + // Second, DISTINCT malformed condition on the same direction: unrecognized + // ROSCTR (BC-2.21.007) -- must NOT emit a second finding, since both share the + // same malformed_header_reported_c2s dedup flag (BC-2.21.007 postcondition 3). + let unrecognized = dt_frame(&[0x32, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x02, 0x00, 0x00]); + analyzer.on_data( + flow_key.clone(), + &unrecognized, + 1, + Direction::ClientToServer, + ); + assert_eq!( + analyzer.findings.len(), + 1, + "an unrecognized-ROSCTR malformed condition sharing the dedup flag with an \ + already-reported too-short condition on the same direction must NOT emit a \ + second T0814 (BC-2.21.007 postcondition 3)" + ); + } + + /// AC-187-009 (v1.1): per-direction dedup verified independently for the s2c + /// direction, using an unrecognized-ROSCTR malformed condition (BC-2.21.007) + /// rather than BC-2.21.004's too-short condition -- mirrors + /// `test_BC_2_21_004_len_shorter_than_10_emits_t0814_once_s2c`'s c2s-then-s2c + /// pattern. + /// + /// Traces: BC-2.21.007 postcondition 3; AC-187-009. + #[test] + fn test_BC_2_21_007_unrecognized_rosctr_emits_t0814_once_s2c() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + let unrecognized = dt_frame(&[0x32, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x02, 0x00, 0x00]); + + // c2s malformed occurrence. + analyzer.on_data( + flow_key.clone(), + &unrecognized, + 0, + Direction::ClientToServer, + ); + assert_eq!( + analyzer.findings.len(), + 1, + "the first c2s unrecognized-ROSCTR occurrence must emit one T0814" + ); + // P20-F-2: assert the exact rendered byte, not just the generic substring -- + // `unrecognized` frame's data[1] == 0x00, and + // classify_malformed_header_reason's `other => format!("unrecognized ROSCTR + // byte 0x{other:02x} (BC-2.21.007)")` arm (src/analyzer/s7comm.rs) renders + // that as "unrecognized ROSCTR byte 0x00". + assert_reason_specific_evidence(&analyzer.findings[0], "unrecognized ROSCTR byte 0x00"); + + // First s2c malformed occurrence -- independent of the c2s dedup flag. + analyzer.on_data( + flow_key.clone(), + &unrecognized, + 1, + Direction::ServerToClient, + ); + assert_eq!( + analyzer.findings.len(), + 2, + "the first s2c unrecognized-ROSCTR occurrence must emit its OWN T0814, \ + independently of the c2s direction's dedup flag already being set" + ); + assert_malformed_header_t0814(&analyzer.findings[1], Direction::ServerToClient); + assert_reason_specific_evidence(&analyzer.findings[1], "unrecognized ROSCTR byte 0x00"); + assert_eq!( + analyzer.findings[1].timestamp, + DateTime::from_timestamp(1i64, 0), + "the s2c finding's timestamp must equal chrono::DateTime::from_timestamp(ts \ + as i64, 0) for the NON-ZERO ts=1 this on_data call used -- the exact \ + conversion report_malformed_header performs in src/analyzer/s7comm.rs \ + (P14-F-5b) -- got {:?}", + analyzer.findings[1].timestamp + ); + { + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!(state.malformed_header_reported_s2c); + } + + // A repeated s2c occurrence must not re-emit. + analyzer.on_data( + flow_key.clone(), + &unrecognized, + 2, + Direction::ServerToClient, + ); + assert_eq!( + analyzer.findings.len(), + 2, + "a repeated s2c unrecognized-ROSCTR occurrence must NOT emit a third \ + T0814" + ); + } + + /// AC-187-009 (v1.1, F-08): `parse_s7comm_header` returns `Some` iff `data[1]` is + /// one of the four recognized ROSCTR values (`0x01`/`0x02`/`0x03`/`0x07`), + /// exhaustively verified over all 256 possible `data[1]` byte values at a + /// length (12 bytes) sufficient for EVERY recognized ROSCTR, including the + /// Ack/Ack_Data 12-byte minimum -- i.e. this test isolates BC-2.21.007's own + /// "recognized vs. unrecognized ROSCTR" concern from BC-2.21.006/008's + /// length-conditional concern (covered separately by + /// `proptest_bc_2_21_006_008_some_iff_rosctr_and_length_conditional` below). No + /// value panics. + /// + /// Traces: BC-2.21.007 postconditions 1-2, Canonical Test Vectors; AC-187-009. + #[test] + fn proptest_bc_2_21_007_rosctr_byte_totality_over_all_256_values() { + for rosctr in 0u8..=255u8 { + let mut data = classic_header_bytes(rosctr, 1, 0, 0); + data.push(0x00); // error_class (only consumed when rosctr is Ack/AckData) + data.push(0x00); // error_code + assert_eq!(data.len(), 12); + let result = parse_s7comm_header(&data); + let expected_some = matches!(rosctr, 0x01 | 0x02 | 0x03 | 0x07); + assert_eq!( + result.is_some(), + expected_some, + "rosctr={rosctr:#04x}, len=12: parse_s7comm_header must return Some \ + iff rosctr is one of the four recognized ROSCTR values \ + (0x01/0x02/0x03/0x07) -- every other byte is unrecognized \ + regardless of length (BC-2.21.007 postconditions 1-2), verified \ + across all 256 possible u8 values" + ); + } + } + + /// AC-187-009 / F-08 (corrected 2026-09-24 per the canonical-frame holdout + /// ruling, DF-CANONICAL-FRAME-HOLDOUT-001): the joint, LENGTH-CONDITIONAL + /// totality across BC-2.21.006/007/008, exhaustively over all 256 possible + /// `data[1]` byte values AND a representative span of lengths + /// (`[9, 10, 11, 12, 13]`, straddling both the 10-byte Job/Userdata minimum and + /// the 12-byte Ack/Ack_Data minimum). `parse_s7comm_header(data)` returns + /// `Some` **iff** (`data[1] ∈ {0x01, 0x07}` and `data.len() >= 10`) **or** + /// (`data[1] ∈ {0x02, 0x03}` and `data.len() >= 12`) -- i.e. Ack_Data (`0x03`) + /// is grouped with Ack (`0x02`) under the 12-byte minimum, NOT with Job/Userdata + /// under the 10-byte minimum (the superseded v1.0/v1.1/v1.2 assumption). Every + /// other `data[1]` byte value returns `None` at every length, and no value + /// panics. + /// + /// Traces: BC-2.21.006 postcondition 1, BC-2.21.007 postconditions 1-2, + /// BC-2.21.008 postconditions 1-2; AC-187-009. + #[test] + fn proptest_bc_2_21_006_008_some_iff_rosctr_and_length_conditional() { + for rosctr in 0u8..=255u8 { + for len in 9usize..=13usize { + let mut data = classic_header_bytes(rosctr, 1, 0, 0); // 10 bytes + if len < data.len() { + data.truncate(len); + } else { + while data.len() < len { + data.push(0x00); + } + } + assert_eq!(data.len(), len); + + let result = parse_s7comm_header(&data); + let job_or_userdata = matches!(rosctr, 0x01 | 0x07); + let ack_or_ack_data = matches!(rosctr, 0x02 | 0x03); + let expected_some = + (len >= 10 && job_or_userdata) || (len >= 12 && ack_or_ack_data); + assert_eq!( + result.is_some(), + expected_some, + "rosctr={rosctr:#04x}, len={len}: parse_s7comm_header must return \ + Some iff (rosctr in {{0x01, 0x07}} and len>=10) or (rosctr in \ + {{0x02, 0x03}} and len>=12) -- length-conditional joint totality \ + across BC-2.21.006/007/008, corrected 2026-09-24 per the \ + canonical-frame holdout ruling (DF-CANONICAL-FRAME-HOLDOUT-001): \ + Ack_Data (0x03) is grouped with Ack (0x02) under the 12-byte \ + minimum, not with Job/Userdata under the 10-byte minimum" + ); + } + } + } + + // ========================================================================= + // BC-2.21.008: `parse_s7comm_header` for ROSCTR=Ack (0x02) AND Ack_Data (0x03) + // both require 12 bytes (2026-09-24 canonical-frame holdout ruling, + // DF-CANONICAL-FRAME-HOLDOUT-001). + // ========================================================================= + + /// AC-187-010: the three BC-2.21.008 canonical Ack test vectors -- 10 bytes + /// (`None`, truncated) and 11 bytes (`None`, truncated) use the BC's canonical + /// bytes verbatim; the 12-byte case uses the BC-2.21.008 canonical-vector SHAPE + /// with distinct error bytes 0x81/0x04 substituted (P12-F-2) for the canonical + /// vector's `Some(0)`/`Some(0)` error fields -- not the verbatim canonical bytes + /// -- so a mutation cannot hide behind a same-value pair (`Some`, exact + /// structural equality including `error_class`/`error_code`). + /// `test_BC_2_21_008_canonical_ack_vector_verbatim` below covers the canonical + /// vector's own bytes unmodified. + /// + /// Traces: BC-2.21.008 postconditions 1-2, Canonical Test Vectors; AC-187-010. + #[test] + fn test_BC_2_21_008_ack_rosctr_12_byte_minimum_and_error_fields() { + // 10 bytes: common header only, no error class/code -- truncated Ack. + let ten = [0x32u8, 0x02, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00]; + assert_eq!( + parse_s7comm_header(&ten), + None, + "10-byte Ack (canonical vector) must return None -- truncated" + ); + + // 11 bytes: one byte short of the 12-byte Ack minimum. + let eleven = [ + 0x32u8, 0x02, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, + ]; + assert_eq!( + parse_s7comm_header(&eleven), + None, + "11-byte Ack (canonical vector) must return None -- one byte short" + ); + + // 12 bytes exactly: complete Ack (built via the ack_header_bytes helper). + // Uses DISTINCT, non-zero error_class/error_code values (0x81/0x04, per + // P12-F-2) rather than Some(0)/Some(0) -- a same-value pair cannot + // distinguish a correct data[10]/data[11] extraction from a mutation that + // swaps the two offsets, or from a mutation that hard-codes `Some(0)` for + // either field. + let twelve = ack_header_bytes(1, 0, 0, 0x81, 0x04); + assert_eq!(twelve.len(), 12); + assert_eq!( + parse_s7comm_header(&twelve), + Some(S7commHeader { + rosctr: Rosctr::Ack, + pdu_reference: 1, + param_length: 0, + data_length: 0, + error_class: Some(0x81), + error_code: Some(0x04), + header_len: 12, + }), + "12-byte Ack (BC-2.21.008 canonical-vector SHAPE with distinct error bytes \ + 0x81/0x04 substituted (P12-F-2) -- not the verbatim canonical bytes -- \ + minimal happy path) must extract the exact expected S7commHeader with \ + error_class == data[10] == Some(0x81) and error_code == data[11] == \ + Some(0x04) -- distinct values (P12-F-2) so an offset swap or a \ + hard-coded Some(0) is caught" + ); + } + + /// P19-F-1 remediation: `test_BC_2_21_008_ack_rosctr_12_byte_minimum_and_error_fields` + /// above substitutes distinct, non-zero error bytes (0x81/0x04, per P12-F-2) into + /// the 12-byte Ack happy-path vector rather than using the BC-2.21.008 canonical + /// vector's own `Some(0)`/`Some(0)` error fields verbatim -- a deliberate + /// mutation-catching improvement, but it leaves no test in this file that directly + /// parses BC-2.21.008's canonical Ack row (Canonical Test Vectors table, line 174 + /// of the BC file) with its bytes unmodified. This test closes that gap: it parses + /// the exact canonical-vector bytes `32 02 00 00 00 01 00 00 00 00 00 00` verbatim + /// -- no byte substituted -- and asserts the exact `S7commHeader` the BC specifies, + /// including `error_class: Some(0)` and `error_code: Some(0)`. + /// + /// Traces: BC-2.21.008 postcondition 2, Canonical Test Vectors (Ack 12-byte happy- + /// path row); AC-187-010. + #[test] + fn test_BC_2_21_008_canonical_ack_vector_verbatim() { + // BC-2.21.008 Canonical Test Vectors table, Ack 12-byte happy-path row, + // verbatim -- no byte substituted. + let bytes = [ + 0x32u8, 0x02, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + ]; + assert_eq!( + parse_s7comm_header(&bytes), + Some(S7commHeader { + rosctr: Rosctr::Ack, + pdu_reference: 1, + param_length: 0, + data_length: 0, + error_class: Some(0), + error_code: Some(0), + header_len: 12, + }), + "BC-2.21.008 canonical Ack vector (verbatim, no byte substituted) must \ + produce Some({{rosctr: Ack, error_class: Some(0), error_code: Some(0), \ + header_len: 12}}) exactly as the BC's Canonical Test Vectors table \ + specifies" + ); + } + + /// AC-187-010 (2026-09-24 canonical-frame holdout ruling, + /// DF-CANONICAL-FRAME-HOLDOUT-001): the mirror-image test of + /// `test_BC_2_21_008_ack_rosctr_12_byte_minimum_and_error_fields` above, for + /// Ack_Data (`0x03`) instead of Ack (`0x02`) -- both ROSCTR values require the + /// SAME 12-byte header shape. Uses BC-2.21.008's own Ack_Data canonical test + /// vectors: 10 bytes (`None`, truncated) and 11 bytes (`None`, truncated) use the + /// canonical bytes verbatim; the 12-byte case uses the BC-2.21.008 canonical-vector + /// SHAPE (the cnblogs-sourced Setup Communication response header slice) with + /// distinct error bytes 0x81/0x04 substituted (P12-F-2) for the canonical vector's + /// `Some(0)`/`Some(0)` error fields -- not the verbatim canonical/cnblogs bytes -- + /// so a mutation cannot hide behind a same-value pair (`Some`, exact structural + /// equality including `error_class`/`error_code`). + /// + /// Traces: BC-2.21.008 postconditions 1-2, Canonical Test Vectors; AC-187-010. + #[test] + fn test_BC_2_21_008_ack_data_12_byte_header_and_error_fields() { + // 10 bytes: common header only, no error class/code -- truncated Ack_Data. + let ten = [0x32u8, 0x03, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00]; + assert_eq!( + parse_s7comm_header(&ten), + None, + "10-byte Ack_Data (BC-2.21.008 canonical vector) must return None -- \ + truncated (DF-CANONICAL-FRAME-HOLDOUT-001: Ack_Data is NOT a \ + 10-byte-only ROSCTR)" + ); + + // 11 bytes: one byte short of the 12-byte Ack_Data minimum. + let eleven = [ + 0x32u8, 0x03, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, + ]; + assert_eq!( + parse_s7comm_header(&eleven), + None, + "11-byte Ack_Data (BC-2.21.008 canonical vector) must return None -- one \ + byte short" + ); + + // 12 bytes exactly: the BC-2.21.008 canonical-vector SHAPE (the cnblogs Setup + // Communication response header slice) with distinct error bytes 0x81/0x04 + // substituted (P12-F-2) -- not the verbatim canonical/cnblogs bytes, which + // carry Some(0)/Some(0) -- built via the ack_data_header_bytes helper for + // structural equality with a distinct set of field values (pdu_reference/ + // param_length) than the 10/11-byte vectors above. + // Distinct, non-zero error_class/error_code (0x81/0x04, per P12-F-2) -- + // mirrors the Ack-rosctr test's rationale above. + let twelve = ack_data_header_bytes(0xFFFF, 8, 0, 0x81, 0x04); + assert_eq!(twelve.len(), 12); + assert_eq!( + parse_s7comm_header(&twelve), + Some(S7commHeader { + rosctr: Rosctr::AckData, + pdu_reference: 0xFFFF, + param_length: 8, + data_length: 0, + error_class: Some(0x81), + error_code: Some(0x04), + header_len: 12, + }), + "12-byte Ack_Data (BC-2.21.008 canonical-vector SHAPE, Setup Communication \ + response shape, with distinct error bytes 0x81/0x04 substituted \ + (P12-F-2) -- not the verbatim canonical/cnblogs bytes) must extract the \ + exact expected S7commHeader with error_class == data[10] == Some(0x81), \ + error_code == data[11] == Some(0x04), and header_len == 12 \ + (DF-CANONICAL-FRAME-HOLDOUT-001; P12-F-2: distinct values catch an \ + offset swap or a hard-coded Some(0))" + ); + } + + /// AC-187-010 (2026-09-24 canonical-frame holdout ruling, + /// DF-CANONICAL-FRAME-HOLDOUT-001), BC-2.21.008 edge cases EC-005/EC-006: a + /// dedicated, direct-call regression test isolating the truncated-Ack_Data + /// `None` behavior at exactly 10 and 11 bytes -- distinct from the full + /// canonical-vector-plus-happy-path coverage in + /// `test_BC_2_21_008_ack_data_12_byte_header_and_error_fields` above. + /// + /// Traces: BC-2.21.008 postcondition 1, edge cases EC-005/EC-006; AC-187-010. + #[test] + fn test_BC_2_21_008_truncated_ack_data_returns_none() { + // EC-005: data[1] == 0x03 (Ack_Data), data.len() == 10 (only the common + // header present). + let ten = ack_data_header_bytes(1, 0, 0, 0x00, 0x00); + let ten_truncated = &ten[..10]; + assert_eq!( + parse_s7comm_header(ten_truncated), + None, + "a 10-byte Ack_Data header (only the common header present) must return \ + None (BC-2.21.008 edge case EC-005)" + ); + + // EC-006: data[1] == 0x03 (Ack_Data), data.len() == 11 (one byte short of + // the 12-byte minimum). + let eleven = ack_data_header_bytes(1, 0, 0, 0x00, 0x00); + let eleven_truncated = &eleven[..11]; + assert_eq!( + parse_s7comm_header(eleven_truncated), + None, + "an 11-byte Ack_Data header (one byte short of the 12-byte minimum) must \ + return None (BC-2.21.008 edge case EC-006)" + ); + } + + /// BC-2.21.008 postcondition 3: `error_class`/`error_code` are `Some` ONLY when + /// `rosctr ∈ {Ack, AckData}` -- cross-checked here against Job AND Userdata + /// headers, both of which must have both fields `None` (the Job case is already + /// asserted structurally within `test_BC_2_21_006_common_header_field_extraction`; + /// restated here, alongside Userdata, as a dedicated, BC-2.21.008-scoped + /// assertion for direct traceability). Renamed from + /// `test_BC_2_21_008_error_fields_none_for_non_ack_rosctr` per the 2026-09-24 + /// canonical-frame holdout ruling (DF-CANONICAL-FRAME-HOLDOUT-001) -- the old + /// name's "non-Ack" framing is now misleading, since Ack_Data (also + /// "non-Ack") no longer has `None` error fields. + /// + /// Traces: BC-2.21.008 postcondition 3, invariant 2. + #[test] + fn test_BC_2_21_008_error_fields_none_for_job_and_userdata_rosctr() { + let job = [0x32u8, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x02, 0x00, 0x00]; + let job_header = parse_s7comm_header(&job).expect("valid Job header must parse"); + assert_eq!( + job_header.error_class, None, + "error_class must be None for Job (BC-2.21.008 postcondition 3)" + ); + assert_eq!( + job_header.error_code, None, + "error_code must be None for Job (BC-2.21.008 postcondition 3)" + ); + + let userdata = [0x32u8, 0x07, 0x00, 0x00, 0x00, 0x05, 0x00, 0x08, 0x00, 0x00]; + let userdata_header = + parse_s7comm_header(&userdata).expect("valid Userdata header must parse"); + assert_eq!( + userdata_header.error_class, None, + "error_class must be None for Userdata (BC-2.21.008 postcondition 3)" + ); + assert_eq!( + userdata_header.error_code, None, + "error_code must be None for Userdata (BC-2.21.008 postcondition 3)" + ); + } + + /// AC-187-010 (F-21b): an `on_data`-delivered classic-S7comm DT frame whose + /// ROSCTR byte (`data[1]`) is `0x02` (Ack) and whose total S7comm payload length + /// is exactly 10 bytes, and separately a variant whose total payload length is + /// exactly 11 bytes (both one and two bytes short of the 12-byte Ack minimum), + /// must each cause `parse_s7comm_header` to return `None` and emit exactly one + /// T0814 `Finding` with evidence text identifying the reason as "truncated Ack + /// header" -- the exact string `classify_malformed_header_reason` produces for + /// `data[1] == 0x02` (distinct from the "header too short", "unrecognized + /// ROSCTR", and "truncated Ack_Data header" reasons; per pass-3 F-29, the + /// assertion below uses the full "truncated Ack header" substring rather than + /// the shorter "truncated Ack" prefix, which is also a substring of + /// "truncated Ack_Data header" and so would not actually distinguish the two + /// reasons). Each case uses a fresh analyzer/flow, so no priming frame is needed: the + /// frame's own `protocol_id: Some(0x32)` sticky-classifies the flow Classic on + /// this very frame (AC-187-004's "or, by this very frame's own + /// first-classification, becomes" clause), and dissection then proceeds. + /// + /// Traces: BC-2.21.008 postcondition 1; AC-187-010. + #[test] + fn test_BC_2_21_008_truncated_ack_on_data_emits_t0814_once() { + // 10-byte case: the bare common header only (rosctr = 0x02, Ack) -- two + // bytes short of the 12-byte Ack minimum. + { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + let ten = classic_header_bytes(0x02, 1, 0, 0); + assert_eq!(ten.len(), 10); + let frame = dt_frame(&ten); + analyzer.on_data(flow_key, &frame, 0, Direction::ClientToServer); + + assert_eq!( + analyzer.findings.len(), + 1, + "a 10-byte Ack (rosctr=0x02) DT frame must emit exactly one T0814 \ + (AC-187-010, BC-2.21.008 postcondition 1)" + ); + assert_malformed_header_t0814(&analyzer.findings[0], Direction::ClientToServer); + assert_reason_specific_evidence(&analyzer.findings[0], "truncated Ack header"); + } + + // 11-byte case: one byte short of the 12-byte Ack minimum. + { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + let mut eleven = classic_header_bytes(0x02, 1, 0, 0); + eleven.push(0x00); + assert_eq!(eleven.len(), 11); + let frame = dt_frame(&eleven); + analyzer.on_data(flow_key, &frame, 0, Direction::ClientToServer); + + assert_eq!( + analyzer.findings.len(), + 1, + "an 11-byte Ack (rosctr=0x02) DT frame must emit exactly one T0814 \ + (AC-187-010, BC-2.21.008 postcondition 1)" + ); + assert_malformed_header_t0814(&analyzer.findings[0], Direction::ClientToServer); + assert_reason_specific_evidence(&analyzer.findings[0], "truncated Ack header"); + } + } + + /// AC-187-010 (2026-09-24 canonical-frame holdout ruling, + /// DF-CANONICAL-FRAME-HOLDOUT-001): the mirror-image `on_data`-level test of + /// `test_BC_2_21_008_truncated_ack_on_data_emits_t0814_once` above, for + /// Ack_Data (`0x03`) instead of Ack (`0x02`) -- a 10-byte and, separately, an + /// 11-byte Ack_Data DT frame must each emit exactly one T0814 `Finding` with + /// evidence text identifying the reason as "truncated Ack_Data", DISTINCT from + /// the "truncated Ack" reason (the assertion below requires the evidence to + /// contain the full "truncated Ack_Data" substring, so an implementation that + /// mistakenly reused the plain "truncated Ack" string for this ROSCTR would + /// fail this test). Each case uses a fresh analyzer/flow: the frame's own + /// `protocol_id: Some(0x32)` sticky-classifies the flow Classic on this very + /// frame (AC-187-004's "or, by this very frame's own first-classification, + /// becomes" clause), and dissection then proceeds. + /// + /// Traces: BC-2.21.008 postcondition 1, edge cases EC-005/EC-006; AC-187-010. + #[test] + fn test_BC_2_21_008_truncated_ack_data_on_data_emits_t0814_once() { + // 10-byte case: the bare common header only (rosctr = 0x03, Ack_Data) -- + // two bytes short of the 12-byte Ack_Data minimum. + { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + let ten = classic_header_bytes(0x03, 1, 0, 0); + assert_eq!(ten.len(), 10); + let frame = dt_frame(&ten); + analyzer.on_data(flow_key, &frame, 0, Direction::ClientToServer); + + assert_eq!( + analyzer.findings.len(), + 1, + "a 10-byte Ack_Data (rosctr=0x03) DT frame must emit exactly one \ + T0814 (AC-187-010, BC-2.21.008 postcondition 1)" + ); + assert_malformed_header_t0814(&analyzer.findings[0], Direction::ClientToServer); + assert_reason_specific_evidence(&analyzer.findings[0], "truncated Ack_Data"); + } + + // 11-byte case: one byte short of the 12-byte Ack_Data minimum. + { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + let mut eleven = classic_header_bytes(0x03, 1, 0, 0); + eleven.push(0x00); + assert_eq!(eleven.len(), 11); + let frame = dt_frame(&eleven); + analyzer.on_data(flow_key, &frame, 0, Direction::ClientToServer); + + assert_eq!( + analyzer.findings.len(), + 1, + "an 11-byte Ack_Data (rosctr=0x03) DT frame must emit exactly one \ + T0814 (AC-187-010, BC-2.21.008 postcondition 1)" + ); + assert_malformed_header_t0814(&analyzer.findings[0], Direction::ClientToServer); + assert_reason_specific_evidence(&analyzer.findings[0], "truncated Ack_Data"); + } + } + + // ========================================================================= + // BC-2.21.009: declared param_length/data_length are bounds-checked against + // remaining bytes before parameter/data block access. + // ========================================================================= + + /// AC-187-011: a header whose declared `param_length`/`data_length` exceed the + /// bytes actually remaining (canonical vector `2 / 0 / 1` -- one byte short) is + /// treated as malformed: one T0814 per flow direction (sharing the dedup flag with + /// BC-2.21.004/007/008), and no out-of-bounds slice is ever attempted (proven + /// indirectly here by the absence of a panic). + /// + /// Traces: BC-2.21.009 postconditions 1-2, Canonical Test Vectors; AC-187-011. + #[test] + fn test_BC_2_21_009_bounds_check_before_parameter_data_slice() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + // Job header declaring param_length=2, data_length=0, but only 1 byte follows + // the 10-byte header (canonical vector "2 / 0 / 1"). + let mut header = classic_header_bytes(0x01, 0x0001, 0x0002, 0x0000); + header.push(0xAA); // only 1 of the declared 2 parameter bytes present + let frame = dt_frame(&header); + + analyzer.on_data(flow_key.clone(), &frame, 0, Direction::ClientToServer); + + assert_eq!( + analyzer.findings.len(), + 1, + "declared param_length exceeding the actually-available bytes must emit \ + exactly one malformed-header T0814 (BC-2.21.009 postcondition 2)" + ); + assert_malformed_header_t0814(&analyzer.findings[0], Direction::ClientToServer); + assert_reason_specific_evidence(&analyzer.findings[0], "exceed available bytes"); + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + state.malformed_header_reported_c2s, + "malformed_header_reported_c2s must be set (BC-2.21.009 postcondition 2 \ + shares the dedup flag with BC-2.21.004/007/008)" + ); + + // P19-F-2 (AC-187-011): a second c2s delivery of the SAME malformed frame + // must NOT re-emit -- the malformed_header_reported_c2s flag set above must + // suppress a repeat occurrence of this specific bounds-check-failure reason + // on the same direction, exactly like the other four malformed-header + // conditions (BC-2.21.004/007/008) already verify for themselves elsewhere. + analyzer.on_data(flow_key.clone(), &frame, 1, Direction::ClientToServer); + assert_eq!( + analyzer.findings.len(), + 1, + "a repeated c2s bounds-check-failure occurrence of the same malformed \ + frame must NOT emit a second T0814 (BC-2.21.009 postcondition 2 dedup)" + ); + } + + /// AC-187-011 (v1.1): per-direction dedup verified independently for the s2c + /// direction, using a bounds-check (declared-length-exceeds-available-bytes) + /// malformed condition (BC-2.21.009) -- mirrors the c2s-then-s2c pattern used by + /// `test_BC_2_21_004_len_shorter_than_10_emits_t0814_once_s2c` and + /// `test_BC_2_21_007_unrecognized_rosctr_emits_t0814_once_s2c`. + /// + /// Traces: BC-2.21.009 postcondition 2; AC-187-011. + #[test] + fn test_BC_2_21_009_bounds_check_dedup_s2c() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + let mut header = classic_header_bytes(0x01, 0x0001, 0x0002, 0x0000); + header.push(0xAA); // only 1 of the declared 2 parameter bytes present + let frame = dt_frame(&header); + + // c2s bounds-check-failure occurrence. + analyzer.on_data(flow_key.clone(), &frame, 0, Direction::ClientToServer); + assert_eq!( + analyzer.findings.len(), + 1, + "the first c2s bounds-check-failure occurrence must emit one T0814" + ); + + // First s2c bounds-check-failure occurrence -- independent of the c2s dedup + // flag. + analyzer.on_data(flow_key.clone(), &frame, 1, Direction::ServerToClient); + assert_eq!( + analyzer.findings.len(), + 2, + "the first s2c bounds-check-failure occurrence must emit its OWN T0814, \ + independently of the c2s direction's dedup flag already being set" + ); + assert_malformed_header_t0814(&analyzer.findings[1], Direction::ServerToClient); + assert_reason_specific_evidence(&analyzer.findings[1], "exceed available bytes"); + assert_eq!( + analyzer.findings[1].timestamp, + DateTime::from_timestamp(1i64, 0), + "the s2c finding's timestamp must equal chrono::DateTime::from_timestamp(ts \ + as i64, 0) for the NON-ZERO ts=1 this on_data call used -- the exact \ + conversion report_malformed_header performs in src/analyzer/s7comm.rs \ + (P14-F-5b) -- got {:?}", + analyzer.findings[1].timestamp + ); + { + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!(state.malformed_header_reported_s2c); + } + + // A repeated s2c occurrence must not re-emit. + analyzer.on_data(flow_key.clone(), &frame, 2, Direction::ServerToClient); + assert_eq!( + analyzer.findings.len(), + 2, + "a repeated s2c bounds-check-failure occurrence must NOT emit a third \ + T0814" + ); + } + + /// F-14 / AC-187-011: the extracted pure helper `s7comm_bounds_ok` (BC-2.21.009) + /// must agree with `dispatch_classic_s7comm`'s bounds decision: it returns `true` + /// exactly when `data_len >= header.header_len + header.param_length as usize + + /// header.data_length as usize`, callable directly (independent of the effectful + /// `on_data` call site) so the VP-051 Kani harness can exercise it too. + /// + /// Traces: BC-2.21.009 postcondition 1, invariant 1; STORY-187 VP-051 Kani + /// Obligation note (F-14). + #[test] + fn test_BC_2_21_009_s7comm_bounds_ok_helper_matches_bounds_decision() { + // Exact match: header_len=10, param_length=2, data_length=0, data_len=12. + let header = parse_s7comm_header(&classic_header_bytes(0x01, 1, 2, 0)) + .expect("10-byte common header must parse"); + assert!( + s7comm_bounds_ok(&header, 12), + "data_len == header_len + param_length + data_length exactly must pass \ + (BC-2.21.009 Canonical Test Vectors row 1)" + ); + assert!( + !s7comm_bounds_ok(&header, 11), + "data_len one byte short of the declared total must fail \ + (BC-2.21.009 Canonical Test Vectors row 2)" + ); + + // Empty parameter/data blocks: trivially passes at data_len == header_len. + let empty_header = parse_s7comm_header(&classic_header_bytes(0x01, 1, 0, 0)) + .expect("10-byte common header with empty blocks must parse"); + assert!( + s7comm_bounds_ok(&empty_header, 10), + "param_length == data_length == 0 with data_len == header_len exactly \ + must pass trivially (BC-2.21.009 edge case EC-001)" + ); + + // Overflow-free at maximum u16 values: must return false, never panic. + let max_header = parse_s7comm_header(&classic_header_bytes(0x01, 1, 0xFFFF, 0xFFFF)) + .expect("10-byte common header must parse regardless of declared lengths"); + assert!( + !s7comm_bounds_ok(&max_header, 10), + "maximum-representable declared param_length/data_length against a bare \ + 10-byte data_len must cleanly fail, never panic (BC-2.21.009 invariant 1, \ + STORY-187 Edge Case EC-007)" + ); + } + + /// BC-2.21.009 canonical test vector: `param_length=2, data_length=0`, exactly 2 + /// bytes remaining -- bounds check passes, no finding. + /// + /// Traces: BC-2.21.009 Canonical Test Vectors row 1 (happy-path). + #[test] + fn test_BC_2_21_009_bounds_check_passes_exact_match() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + let mut header = classic_header_bytes(0x01, 0x0001, 0x0002, 0x0000); + header.extend_from_slice(&[0xAA, 0xBB]); // exactly the declared 2 param bytes + let frame = dt_frame(&header); + + analyzer.on_data(flow_key.clone(), &frame, 0, Direction::ClientToServer); + + assert!( + analyzer.findings.is_empty(), + "an exact declared-length-to-available-bytes match must pass the bounds \ + check cleanly, with no malformed-header finding (BC-2.21.009 Canonical \ + Test Vectors row 1)" + ); + } + + /// EC-001 (BC-2.21.009): `param_length == 0`, `data_length == 0`, and + /// `data.len() == header_len` exactly -- the bounds check passes trivially (an + /// empty parameter block is legitimate, not itself an error). + /// + /// Traces: BC-2.21.009 edge case EC-001. + #[test] + fn test_BC_2_21_009_empty_parameter_and_data_blocks_trivial_pass() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + let header = classic_header_bytes(0x01, 0x0001, 0x0000, 0x0000); + let frame = dt_frame(&header); + + analyzer.on_data(flow_key.clone(), &frame, 0, Direction::ClientToServer); + + assert!( + analyzer.findings.is_empty(), + "param_length == data_length == 0 with data.len() == header_len exactly \ + must pass the bounds check trivially, with no finding \ + (BC-2.21.009 edge case EC-001)" + ); + } + + /// EC-007 (story-level edge case table): `param_length == 0xFFFF`, `data_length == + /// 0xFFFF`, `data.len() == 10` (i.e. only the bare 10-byte header, no + /// parameter/data bytes at all) -- the bounds check must fail cleanly, with no + /// arithmetic overflow in the `header_len + param_length + data_length` sum and no + /// slice ever attempted (no panic). + /// + /// Traces: BC-2.21.009 invariant 1, STORY-187 Edge Case EC-007. + #[test] + fn test_BC_2_21_009_overflow_free_arithmetic_max_values() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + // Job header declaring the maximum representable param_length/data_length, + // with NO trailing bytes at all beyond the bare 10-byte header. + let header = classic_header_bytes(0x01, 0x0001, 0xFFFF, 0xFFFF); + assert_eq!(header.len(), 10); + let frame = dt_frame(&header); + + // Must not panic (no arithmetic overflow, no out-of-bounds slice). + analyzer.on_data(flow_key.clone(), &frame, 0, Direction::ClientToServer); + + assert_eq!( + analyzer.findings.len(), + 1, + "maximum-representable declared param_length/data_length against a bare \ + 10-byte header must cleanly fail the bounds check and emit exactly one \ + malformed-header T0814, never panic via arithmetic overflow \ + (BC-2.21.009 invariant 1)" + ); + assert_malformed_header_t0814(&analyzer.findings[0], Direction::ClientToServer); + } + + /// AC-187-011 (F-21c): the Ack-header (`header.header_len == 12`) bounds case, + /// verified both via `on_data` (12 bytes total -- the declared 1-byte parameter + /// is ABSENT -> T0814; 13 bytes -- the declared byte PRESENT -> clean) and via + /// direct `s7comm_bounds_ok` assertions, to verify the check correctly + /// incorporates the Ack/Ack_Data-specific `header_len == 12` base rather than + /// assuming the 10-byte Job/Userdata default (corrected 2026-09-24 per the + /// canonical-frame holdout ruling, DF-CANONICAL-FRAME-HOLDOUT-001 -- Ack_Data is + /// no longer assumed to use the 10-byte default; this test exercises the + /// `header_len == 12` mechanism via `rosctr == Ack`, which is shared identically + /// with `rosctr == AckData` since both flow through the same + /// `parse_s7comm_header` match arm and the same `s7comm_bounds_ok` helper). + /// + /// Traces: BC-2.21.009 postcondition 2; AC-187-011. + #[test] + fn test_BC_2_21_009_ack_header_len_12_bounds_check() { + // 12 bytes total: Ack header (header_len=12) declaring param_length=1, + // data_length=0, but the declared parameter byte is ABSENT. + { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + let header_bytes = ack_header_bytes(1, 1, 0, 0x00, 0x00); + assert_eq!(header_bytes.len(), 12); + let frame = dt_frame(&header_bytes); + analyzer.on_data(flow_key, &frame, 0, Direction::ClientToServer); + + assert_eq!( + analyzer.findings.len(), + 1, + "a 12-byte Ack header declaring param_length=1 with the parameter \ + byte ABSENT must fail the bounds check and emit one T0814 \ + (AC-187-011)" + ); + assert_malformed_header_t0814(&analyzer.findings[0], Direction::ClientToServer); + assert_reason_specific_evidence(&analyzer.findings[0], "exceed available bytes"); + } + + // 13 bytes total: same Ack header, this time WITH the declared parameter + // byte present -- bounds check passes cleanly. + { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + let mut header_bytes = ack_header_bytes(1, 1, 0, 0x00, 0x00); + header_bytes.push(0xAA); // the declared parameter byte + assert_eq!(header_bytes.len(), 13); + let frame = dt_frame(&header_bytes); + analyzer.on_data(flow_key, &frame, 0, Direction::ClientToServer); + + assert!( + analyzer.findings.is_empty(), + "a 13-byte Ack header declaring param_length=1 WITH the parameter \ + byte present must pass the bounds check cleanly, no finding \ + (AC-187-011)" + ); + } + + // Direct s7comm_bounds_ok assertions incorporating the Ack-specific + // header_len == 12 base rather than assuming the 10-byte default. + let header = parse_s7comm_header(&ack_header_bytes(1, 1, 0, 0x00, 0x00)) + .expect("12-byte Ack header must parse"); + assert_eq!(header.header_len, 12); + assert!( + !s7comm_bounds_ok(&header, 12), + "header_len(12) + param_length(1) + data_length(0) == 13 > data_len(12) \ + must fail the bounds check (AC-187-011)" + ); + assert!( + s7comm_bounds_ok(&header, 13), + "header_len(12) + param_length(1) + data_length(0) == 13 == data_len(13) \ + must pass the bounds check exactly (AC-187-011)" + ); + } + + /// P12-F-1: unlike every other BC-2.21.009 `on_data` test in this module (which + /// all use `data_length == 0` and vary `param_length`), this test holds + /// `param_length == 0` fixed and overruns exclusively on `data_length` -- the + /// declared 2 data bytes are only partially (1 byte) or exactly (2 bytes) + /// present, with NO parameter bytes at all. A mutation that drops + /// `header.data_length` from `s7comm_bounds_ok`'s summed total (leaving only + /// `header_len + param_length`) would compute `declared_total == 10` here + /// instead of the correct `12`, so the 1-trailing-byte case (`data_len == 11 >= + /// 10`) would incorrectly PASS the bounds check and emit zero findings -- + /// diverging from this test's expectation of exactly one T0814. + /// + /// Traces: BC-2.21.009 postcondition 1 (declared total includes data_length). + #[test] + fn test_BC_2_21_009_data_length_overrun_on_data_emits_t0814() { + // 1 of the declared 2 data bytes present -> bounds check fails, one T0814. + { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + let mut header = classic_header_bytes(0x01, 0x0001, 0x0000, 0x0002); + header.push(0xAA); // only 1 of the declared 2 data bytes present + let frame = dt_frame(&header); + analyzer.on_data(flow_key.clone(), &frame, 0, Direction::ClientToServer); + + assert_eq!( + analyzer.findings.len(), + 1, + "declared data_length exceeding the actually-available bytes (with \ + param_length == 0) must emit exactly one malformed-header T0814 \ + (P12-F-1, BC-2.21.009 postcondition 1)" + ); + assert_malformed_header_t0814(&analyzer.findings[0], Direction::ClientToServer); + assert_reason_specific_evidence(&analyzer.findings[0], "exceed available bytes"); + } + + // Both declared data bytes present -> bounds check passes, no finding. + { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + let mut header = classic_header_bytes(0x01, 0x0001, 0x0000, 0x0002); + header.extend_from_slice(&[0xAA, 0xBB]); // exactly the declared 2 data bytes + let frame = dt_frame(&header); + analyzer.on_data(flow_key.clone(), &frame, 0, Direction::ClientToServer); + + assert!( + analyzer.findings.is_empty(), + "an exact declared-data_length-to-available-bytes match (with \ + param_length == 0) must pass the bounds check cleanly, with no \ + malformed-header finding (P12-F-1)" + ); + } + } + + /// P12-F-1: direct `s7comm_bounds_ok` assertions isolating `data_length`'s + /// contribution to the summed bound -- `header_len == 10`, `param_length == 0`, + /// `data_length == 2`. Kills the same "drop data_length from the sum" mutation + /// as the `on_data`-level test above, without going through `on_data` or + /// `parse_s7comm_header` at all. Also covers a MIXED `param_length == + /// 3`/`data_length == 2` case, so a mutation that reads `data_length` but drops + /// `param_length` (or vice versa) is caught too. + /// + /// Traces: BC-2.21.009 postcondition 1, invariant 1. + #[test] + fn test_BC_2_21_009_s7comm_bounds_ok_data_length_only_overrun() { + let header = S7commHeader { + rosctr: Rosctr::Job, + pdu_reference: 0, + param_length: 0, + data_length: 2, + error_class: None, + error_code: None, + header_len: 10, + }; + assert!( + !s7comm_bounds_ok(&header, 11), + "header_len(10) + param_length(0) + data_length(2) == 12 > data_len(11) \ + must fail -- if data_length were dropped from the sum, declared_total \ + would be 10 and data_len(11) would incorrectly pass (P12-F-1)" + ); + assert!( + s7comm_bounds_ok(&header, 12), + "header_len(10) + param_length(0) + data_length(2) == 12 == data_len(12) \ + must pass the bounds check exactly (P12-F-1)" + ); + + // Mixed case: both param_length and data_length are non-zero and distinct. + let mixed = S7commHeader { + param_length: 3, + data_length: 2, + ..header + }; + assert!( + !s7comm_bounds_ok(&mixed, 14), + "header_len(10) + param_length(3) + data_length(2) == 15 > data_len(14) \ + must fail (P12-F-1 mixed case)" + ); + assert!( + s7comm_bounds_ok(&mixed, 15), + "header_len(10) + param_length(3) + data_length(2) == 15 == data_len(15) \ + must pass exactly (P12-F-1 mixed case)" + ); + } + + /// cargo-mutants (27.1.0) survivor kill: `dispatch_classic_s7comm`'s + /// declared-vs-available evidence computation (`declared = header.header_len as + /// u64 + header.param_length as u64 + header.data_length as u64;`, F-11) had no + /// test asserting the exact numeric `declared`/`available` values it renders + /// into the T0814 evidence text, so three arithmetic mutations at that line + /// survived: `+` -> `*` on the first `+` (`header_len * param_length + + /// data_length`, giving 35 for this test's inputs instead of 18), `+` -> `-` on + /// the second `+` (`header_len + param_length - data_length`, giving 8), and `+` + /// -> `*` on the second `+` (`header_len + param_length * data_length`, giving + /// 25). All three mutants still fail the bounds check and still emit one T0814 + /// (the boolean accept/reject decision comes from `s7comm_bounds_ok`, which is + /// untouched by these mutations) -- only the evidence text's numbers differ, so + /// asserting the exact `declared 18 (header_len=10 + param_length=3 + + /// data_length=5)` / `available 12` substrings is required to distinguish + /// correct behavior from all three surviving mutants (35, 8, 25). + /// + /// Job header (`rosctr = 0x01`, `header_len = 10`): `param_length = 3`, + /// `data_length = 5` -> declared total `10 + 3 + 5 = 18`; the frame's classic + /// S7comm payload (the bytes passed to `dispatch_classic_s7comm`) is exactly 12 + /// bytes (the 10-byte header plus 2 trailing bytes), 6 short of the declared 18, + /// so the bounds check fails and exactly one T0814 is emitted with `available == + /// 12`. + /// + /// Traces: BC-2.21.009 postcondition 2, F-11; AC-187-011. + #[test] + fn test_BC_2_21_009_bounds_failure_evidence_reports_declared_and_available() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + // Job header: header_len=10, param_length=3, data_length=5 -> declared 18. + let mut header = classic_header_bytes(0x01, 0x0001, 0x0003, 0x0005); + header.extend_from_slice(&[0xAA, 0xBB]); // only 2 trailing bytes -> 12 total + assert_eq!( + header.len(), + 12, + "test setup: 10-byte header + 2 trailing bytes" + ); + let frame = dt_frame(&header); + + analyzer.on_data(flow_key.clone(), &frame, 0, Direction::ClientToServer); + + assert_eq!( + analyzer.findings.len(), + 1, + "a Job header declaring a total of 18 bytes against only 12 available \ + must emit exactly one malformed-header T0814 (BC-2.21.009 postcondition 2)" + ); + assert_malformed_header_t0814(&analyzer.findings[0], Direction::ClientToServer); + assert!( + analyzer.findings[0] + .evidence + .iter() + .any(|e| e.contains( + "declared 18 (header_len=10 + param_length=3 + data_length=5)" + )), + "evidence must report the correct declared total (18) and its exact \ + header_len/param_length/data_length breakdown -- distinguishes the \ + correct `+`/`+` computation from the surviving `*`/`+` mutant (35), got \ + {:?}", + analyzer.findings[0].evidence + ); + assert!( + analyzer.findings[0] + .evidence + .iter() + .any(|e| e.contains("available 12 (BC-2.21.009)")), + "evidence must report the correct available byte count (12) -- \ + distinguishes the correct `+`/`+` computation from the surviving \ + `+`/`-` mutant (8) and `+`/`*` mutant (25), got {:?}", + analyzer.findings[0].evidence + ); + } + + /// Companion to + /// `test_BC_2_21_009_bounds_failure_evidence_reports_declared_and_available` + /// above, using the Ack_Data (`header_len = 12`) variant rather than Job + /// (`header_len = 10`) -- confirms the declared/available evidence text is + /// correct across BOTH `header_len` bases (2026-09-24 canonical-frame holdout + /// ruling, DF-CANONICAL-FRAME-HOLDOUT-001), not merely coincidentally correct for + /// the 10-byte common-header case. + /// + /// Ack_Data header (`rosctr = 0x03`, `header_len = 12`): `param_length = 3`, + /// `data_length = 5` -> declared total `12 + 3 + 5 = 20`; the frame's classic + /// S7comm payload is exactly 14 bytes (the 12-byte header plus 2 trailing + /// bytes), 6 short of the declared 20, so the bounds check fails and exactly one + /// T0814 is emitted with `available == 14`. + /// + /// Traces: BC-2.21.009 postcondition 2, F-11; AC-187-011. + #[test] + fn test_BC_2_21_009_bounds_failure_evidence_ack_data_header_len_12() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + // Ack_Data header: header_len=12, param_length=3, data_length=5 -> declared 20. + let mut header = ack_data_header_bytes(0x0001, 0x0003, 0x0005, 0x00, 0x00); + header.extend_from_slice(&[0xAA, 0xBB]); // only 2 trailing bytes -> 14 total + assert_eq!( + header.len(), + 14, + "test setup: 12-byte header + 2 trailing bytes" + ); + let frame = dt_frame(&header); + + analyzer.on_data(flow_key.clone(), &frame, 0, Direction::ClientToServer); + + assert_eq!( + analyzer.findings.len(), + 1, + "an Ack_Data header declaring a total of 20 bytes against only 14 \ + available must emit exactly one malformed-header T0814 (BC-2.21.009 \ + postcondition 2)" + ); + assert_malformed_header_t0814(&analyzer.findings[0], Direction::ClientToServer); + assert!( + analyzer.findings[0] + .evidence + .iter() + .any(|e| e.contains( + "declared 20 (header_len=12 + param_length=3 + data_length=5)" + )), + "evidence must report the correct declared total (20) and its exact \ + header_len/param_length/data_length breakdown for the header_len=12 \ + (Ack_Data) case, got {:?}", + analyzer.findings[0].evidence + ); + assert!( + analyzer.findings[0] + .evidence + .iter() + .any(|e| e.contains("available 14 (BC-2.21.009)")), + "evidence must report the correct available byte count (14) for the \ + header_len=12 (Ack_Data) case, got {:?}", + analyzer.findings[0].evidence + ); + } + + /// P12-F-1: `pdu_reference`, `param_length`, and `data_length` are each decoded + /// via `u16::from_be_bytes` (big-endian) -- never `from_le_bytes`. Uses + /// byte-asymmetric values (each field's high byte != its low byte, and all three + /// fields are pairwise distinct) so that a mutation swapping big-endian for + /// little-endian decoding on ANY one of the three fields produces a different + /// value than expected, and is caught. + /// + /// `0x0102` decoded little-endian would read as `0x0201`; `0x0304` + /// little-endian as `0x0403`; `0x0506` little-endian as `0x0605` -- all three + /// diverge from the correct big-endian values asserted below. + /// + /// Traces: BC-2.21.006 postconditions 2-4. + #[test] + fn test_BC_2_21_006_byte_asymmetric_big_endian_decode() { + let data = classic_header_bytes(0x01, 0x0102, 0x0304, 0x0506); + assert_eq!(data.len(), 10); + + let parsed = parse_s7comm_header(&data).expect("valid Job header must parse"); + assert_eq!( + parsed.pdu_reference, 0x0102, + "pdu_reference (data[4..6]) must be decoded big-endian (P12-F-1)" + ); + assert_eq!( + parsed.param_length, 0x0304, + "param_length (data[6..8]) must be decoded big-endian (P12-F-1)" + ); + assert_eq!( + parsed.data_length, 0x0506, + "data_length (data[8..10]) must be decoded big-endian (P12-F-1)" + ); + } + + // ========================================================================= + // VP-051 (Kani P0, skeleton, v1.1 / F-14): S7comm Header Bounds-Before-Slice + // Safety. Traces BC-2.21.004, BC-2.21.006, BC-2.21.007, BC-2.21.008, BC-2.21.009. + // Full proof execution deferred to STORY-194 (formal-hardening) per this + // story's VP-051 Kani Obligation note. + // ========================================================================= + + /// `#[cfg(kani)]` skeleton, compiled only under `cargo kani` -- under a normal + /// `cargo test`/`cargo check` build this module compiles to nothing (mirrors the + /// `tests/kani_proofs.rs` VP-025/VP-027 pattern and the in-source + /// `#[cfg(kani)] mod kani_proofs` pattern used by `src/analyzer/iso_on_tcp.rs`'s + /// VP-048/VP-049 harnesses). Located in this test file per STORY-187's own File + /// Structure Requirements table (`tests/s7comm_analyzer_tests.rs` MODIFY: "+ + /// VP-051 Kani skeleton"), rather than inside `src/analyzer/s7comm.rs`, since + /// `parse_s7comm_header` and `s7comm_bounds_ok` are both `pub` and fully + /// exercisable from the test crate. + /// + /// v1.1 (F-14, human-ratified 2026-09-24): rewritten to use BOUNDED symbolic + /// input -- a fixed-size `[u8; 16]` array via `kani::any()` plus an + /// assumed-bounded `len <= 16` -- rather than an unbounded `Vec`, and to + /// assert the non-vacuous properties VP-051's Kani Obligation note requires + /// (`len < 10` implies `None`; `Some` implies `header_len ∈ {10, 12}` and + /// `data.len() >= header_len`; `error_class.is_some() == (rosctr == Ack || + /// rosctr == AckData)`, identically for `error_code`). Also exercises + /// `s7comm_bounds_ok`, the pure, public (`pub fn`) helper BC-2.21.009's caller-side + /// bounds check was extracted into for exactly this purpose. + /// + /// v1.3 (round 4, human-ratified 2026-09-24, canonical-frame holdout ruling + /// DF-CANONICAL-FRAME-HOLDOUT-001): the non-vacuous assertion 3 below was + /// corrected from the narrower `error_class.is_some() == (rosctr == Ack)` to + /// `(rosctr == Ack || rosctr == AckData)` -- the narrower form would vacuously + /// PASS a `header_len`/`error_class` mismatch on every Ack_Data input (an + /// Ack_Data header with `header_len == 12` but `error_class == None` would + /// satisfy the narrower equation, since `rosctr == Ack` is `false` for + /// `AckData` too), silently hiding exactly the kind of bug this harness exists + /// to catch. `header_len` selection is `12` when `rosctr ∈ {Ack, AckData}` and + /// `10` otherwise (Job, Userdata). + #[cfg(kani)] + mod vp051_kani { + use wirerust::analyzer::s7comm::{Rosctr, parse_s7comm_header, s7comm_bounds_ok}; + + /// VP-051: `parse_s7comm_header` must not panic for any input up to a bounded + /// length (a fixed-size `[u8; 16]` array, `len <= 16`), including every + /// `data.len() < 10` case (BC-2.21.004) and the `data[0] != 0x32` + /// defensive-reject case (BC-2.21.005). + /// + /// `Some(header)` positive assertions (pass-6 F-47): beyond the header_len + /// shape check, the `Some` branch now asserts the full field-mapping contract + /// so header_len is linked to ROSCTR by an `assert_eq!`, not merely by a + /// `kani::cover!`: + /// - `header.header_len == 12` iff `rosctr ∈ {Ack, AckData}`, else `10` + /// (BC-2.21.006 postcondition 1 / BC-2.21.008 postcondition 2). + /// - When `rosctr ∈ {Ack, AckData}`: `error_class == Some(slice[10])` and + /// `error_code == Some(slice[11])` (BC-2.21.008 postcondition 2 -- exact + /// byte values; presence alone is postcondition 3). + /// - `rosctr` maps exactly from `slice[1]`: `0x01 -> Job`, `0x02 -> Ack`, + /// `0x03 -> AckData`, `0x07 -> Userdata` (BC-2.21.006 postcondition 1 / + /// BC-2.21.007 postcondition 1 -- the recognized-ROSCTR set). + /// - `pdu_reference`, `param_length`, `data_length` equal the big-endian reads + /// of `slice[4..6]`, `slice[6..8]`, `slice[8..10]` respectively + /// (BC-2.21.006 postcondition 2/3/4). + /// + /// Non-vacuous per DF-KANI-NONVACUITY-001 (F-24 wording correction, + /// human-ratified 2026-09-24): the `kani::cover!` checks below record whether + /// BOTH the `None` and `Some` return paths, and both the `header_len == 10` + /// and `header_len == 12` shapes, are reachable from the bounded symbolic + /// input. `kani::cover!` does NOT, by itself, fail verification when a + /// covered condition turns out unreachable -- it is reported as an + /// UNSATISFIABLE cover in the Kani output, not a hard verification failure. + /// STORY-194 (the full non-vacuous proof run) MUST invoke `cargo kani` with + /// `--fail-uncoverable` (or otherwise inspect each cover's + /// satisfied/unsatisfied status in the Kani report) so that a vacuous + /// harness is actually caught as a failure, not silently accepted. + #[kani::proof] + fn verify_parse_s7comm_header_bounds_safety() { + let data: [u8; 16] = kani::any(); + let len: usize = kani::any(); + kani::assume(len <= 16); + let slice = &data[..len]; + + // Must not panic for any bounded input (BC-2.21.004/005/006/007/008 + // guards). + let result = parse_s7comm_header(slice); + + // Non-vacuous assertion 1 (BC-2.21.004 postcondition 1): len < 10 implies + // None. + if len < 10 { + assert!( + result.is_none(), + "VP-051 / BC-2.21.004 postcondition 1: data.len() < 10 must always \ + return None" + ); + } + + if let Some(header) = result { + // Non-vacuous assertion 2: header_len is always 10 or 12, and the + // input slice is always at least that long. + assert!( + header.header_len == 10 || header.header_len == 12, + "VP-051: header_len must be exactly 10 or 12" + ); + assert!( + slice.len() >= header.header_len, + "VP-051 / BC-2.21.004 postcondition 1 / BC-2.21.008 postcondition 1: \ + a Some(header) result's own header_len must never exceed the input \ + slice's length -- this is the parser's own length guard (>= 10 for \ + Job/Userdata, >= 12 for Ack/Ack_Data), not BC-2.21.009's caller-side \ + param_length/data_length bounds check" + ); + + // Non-vacuous assertion 3 (BC-2.21.008 postcondition 3, corrected + // 2026-09-24 per DF-CANONICAL-FRAME-HOLDOUT-001): error_class/ + // error_code are Some iff rosctr is Ack OR AckData, identically for + // both fields -- NOT the narrower (rosctr == Ack) alone, which would + // vacuously pass a header_len/error_class mismatch on every Ack_Data + // input. + let is_ack_or_ack_data = + header.rosctr == Rosctr::Ack || header.rosctr == Rosctr::AckData; + assert_eq!( + header.error_class.is_some(), + is_ack_or_ack_data, + "VP-051 / BC-2.21.008 postcondition 3: error_class.is_some() must \ + equal (rosctr == Ack || rosctr == AckData)" + ); + assert_eq!( + header.error_code.is_some(), + is_ack_or_ack_data, + "VP-051 / BC-2.21.008 postcondition 3: error_code.is_some() must \ + equal (rosctr == Ack || rosctr == AckData)" + ); + + // Positive assertion (pass-6 F-47, BC-2.21.006 postcondition 1 / + // BC-2.21.008 postcondition 2): header_len is LINKED to ROSCTR by an + // assert_eq!, not merely observed via kani::cover! below -- 12 iff + // Ack/AckData, else 10. + assert_eq!( + header.header_len, + if is_ack_or_ack_data { 12 } else { 10 }, + "VP-051 / BC-2.21.006 postcondition 1 / BC-2.21.008 postcondition 2: \ + header_len must be 12 iff rosctr is Ack or AckData, else 10" + ); + + // Positive assertion (pass-6 F-47, BC-2.21.008 postcondition 2): when + // the extended header is present, error_class/error_code carry the + // EXACT byte values from slice[10]/slice[11], not merely Some(_) -- + // postcondition 3 covers presence alone; the exact-value equality + // asserted here is postcondition 2. + if is_ack_or_ack_data { + assert_eq!( + header.error_class, + Some(slice[10]), + "VP-051 / BC-2.21.008 postcondition 2: error_class must equal \ + Some(slice[10]) for Ack/AckData" + ); + assert_eq!( + header.error_code, + Some(slice[11]), + "VP-051 / BC-2.21.008 postcondition 2: error_code must equal \ + Some(slice[11]) for Ack/AckData" + ); + } + + // Positive assertion (pass-6 F-47, BC-2.21.006 postcondition 1 / + // BC-2.21.007 postcondition 1): rosctr must map EXACTLY from slice[1] + // -- 0x01 -> Job, 0x02 -> Ack, 0x03 -> AckData, 0x07 -> Userdata (the + // BC-2.21.007 postcondition 1 recognized-ROSCTR set). No other + // slice[1] value reaches this Some(header) branch (BC-2.21.007 + // safe-reject). + let expected_rosctr = match slice[1] { + 0x01 => Rosctr::Job, + 0x02 => Rosctr::Ack, + 0x03 => Rosctr::AckData, + 0x07 => Rosctr::Userdata, + other => panic!( + "VP-051 / BC-2.21.007 postcondition 1: unexpected slice[1] = \ + {other:#x} reached a Some(header) result; only \ + 0x01/0x02/0x03/0x07 may produce Some" + ), + }; + assert_eq!( + header.rosctr, expected_rosctr, + "VP-051 / BC-2.21.006 postcondition 1 / BC-2.21.007 postcondition 1: \ + rosctr must map exactly from slice[1]" + ); + + // Positive assertion (pass-6 F-47, BC-2.21.006 postcondition 2/3/4): + // pdu_reference/param_length/data_length are exactly the big-endian + // reads of slice[4..6]/[6..8]/[8..10] -- the common fields are always + // present and unaffected by header_len shape. + assert_eq!( + header.pdu_reference, + u16::from_be_bytes([slice[4], slice[5]]), + "VP-051 / BC-2.21.006 postcondition 2: pdu_reference must equal \ + u16::from_be_bytes([slice[4], slice[5]])" + ); + assert_eq!( + header.param_length, + u16::from_be_bytes([slice[6], slice[7]]), + "VP-051 / BC-2.21.006 postcondition 3: param_length must equal \ + u16::from_be_bytes([slice[6], slice[7]])" + ); + assert_eq!( + header.data_length, + u16::from_be_bytes([slice[8], slice[9]]), + "VP-051 / BC-2.21.006 postcondition 4: data_length must equal \ + u16::from_be_bytes([slice[8], slice[9]])" + ); + + // F-14: also exercise the extracted BC-2.21.009 pure helper directly, + // independent of the effectful on_data call site -- must not panic for + // any bounded data_len. + let _ = s7comm_bounds_ok(&header, slice.len()); + + // NON-VACUITY: both header_len shapes, and both Ack and AckData, must + // be reachable. + kani::cover!(header.header_len == 10); + kani::cover!(header.header_len == 12); + kani::cover!(header.rosctr == Rosctr::Ack); + kani::cover!(header.rosctr == Rosctr::AckData); + } + + // NON-VACUITY: both the None and Some return paths must be reachable from + // the bounded symbolic input. + kani::cover!(result.is_none()); + kani::cover!(result.is_some()); + } + + /// VP-051 bounds-check half (F-18, human-ratified 2026-09-24): a SECOND, + /// INDEPENDENT symbolic input `data_len: usize` -- decoupled from the small + /// fixed-size symbolic array used for header-field extraction above -- + /// representing the length of the hypothetical full delivery the header's + /// declared `param_length`/`data_length` are checked against. Proves + /// `s7comm_bounds_ok` returns EXACTLY the right answer (the equality + /// asserted directly, both directions, not merely one direction of an + /// implication) and that a `true` result implies the resulting + /// parameter/data sub-slice access is always `Some(..)`, never `None` and + /// never a construct that could panic. + /// + /// Bounded per the VP-051 Kani Obligation note's own justification: + /// `data_len <= u16::MAX as usize * 3` represents any hypothetical + /// full-delivery length up to three times the maximum single `u16` field, + /// comfortably covering `header_len (<= 12) + param_length (<= u16::MAX) + + /// data_length (<= u16::MAX)` without requiring an unbounded symbolic value. + #[kani::proof] + fn verify_s7comm_bounds_ok_bounds_safety() { + let data: [u8; 16] = kani::any(); + let len: usize = kani::any(); + kani::assume(len <= 16); + let slice = &data[..len]; + + let Some(header) = parse_s7comm_header(slice) else { + return; + }; + + let data_len: usize = kani::any(); + kani::assume(data_len <= u16::MAX as usize * 3); + + let declared_total = + header.header_len as u64 + header.param_length as u64 + header.data_length as u64; + let expected = data_len as u64 >= declared_total; + + // Exact equality (both directions): no false-accept, no false-reject. + assert_eq!( + s7comm_bounds_ok(&header, data_len), + expected, + "VP-051 F-18: s7comm_bounds_ok must return EXACTLY (data_len >= \ + header_len + param_length + data_length) -- no false-accept and no \ + false-reject" + ); + + if expected { + // A symbolic buffer of length data_len -- the hypothetical full + // delivery. When the bounds check passes, the parameter/data + // sub-slice access must always succeed. + let full: Vec = vec![0u8; data_len]; + let param_data_end = + header.header_len + header.param_length as usize + header.data_length as usize; + assert!( + full.get(header.header_len..param_data_end).is_some(), + "VP-051 F-18: when s7comm_bounds_ok is true, the parameter/data \ + sub-slice access data.get(header_len..header_len+param_length+ \ + data_length) must be Some(..), never None, never a panic" + ); + } + + // NON-VACUITY (F-18, DF-KANI-NONVACUITY-001): both the bounds-ok-true and + // bounds-ok-false outcomes must be reachable under the symbolic inputs, + // independent of the header-extraction half's None/Some cover + // obligations above. + kani::cover!(expected); + kani::cover!(!expected); + } + } + + // ========================================================================= + // VP-053 (proptest P0, PARTIAL skeleton, v1.1 / F-02): `protocol_id` Four-Way + // Dispatch Totality and Unclassified Never-Force-Fit. Traces BC-2.21.002, + // BC-2.21.027, BC-2.21.028. This story wires the CR/CC and Some(0x32) branches + // fully; Some(0x72)/Some(other) route to a structural no-op completed in + // STORY-190. The full non-vacuous run is deferred to STORY-194. + // + // v1.1 (F-14/VP-INDEX.md v2.51's "Sticky-Classification Gating" reword, + // human-ratified 2026-09-24): the strategy now generates `protocol_id: None` as + // its OWN distinct case (`Option`, not a bare `u8`), rather than only + // implicitly via absence -- the property under test ("protocol_id: None does not + // participate in sticky first-classification", F-02) is untestable unless the + // strategy can generate it explicitly alongside Some(0x32)/Some(0x72)/Some(other). + // ========================================================================= + + mod vp053 { + use proptest::prelude::*; + use wirerust::analyzer::s7comm::{S7Protocol, S7commAnalyzer}; + use wirerust::reassembly::flow::FlowKey; + use wirerust::reassembly::handler::Direction; + + fn flow_key_for(salt: u16) -> FlowKey { + FlowKey::new( + "127.0.0.1".parse().unwrap(), + 20_000u16.wrapping_add(salt), + "127.0.0.2".parse().unwrap(), + 102, + ) + } + + fn tpkt_frame(cotp_payload: &[u8]) -> Vec { + let total_len = 4 + cotp_payload.len(); + let len = total_len as u16; + let mut frame = vec![0x03u8, 0x00, (len >> 8) as u8, (len & 0xFF) as u8]; + frame.extend_from_slice(cotp_payload); + frame + } + + /// Generates `protocol_id` as `Option`, an explicit, equally-weighted + /// case split between `None` (empty DT payload, no protocol evidence, F-02) + /// and `Some(byte)` (any of the 256 possible protocol-ID byte values) -- + /// rather than `None` arising only implicitly/by absence. + fn protocol_id_strategy() -> impl Strategy> { + prop_oneof![Just(None), any::().prop_map(Some)] + } + + /// Builds a complete DT frame carrying `protocol_id`. When `protocol_id == + /// Some(0x32)`, a fully valid minimal classic Job header (empty parameter/data + /// blocks) follows, so that VP-053's dispatch-TOTALITY property is exercised + /// independently of BC-2.21.004-009's header-parse REJECTION paths (those are + /// this file's dedicated `test_BC_2_21_004/007/008/009_*` tests). When + /// `protocol_id == None`, the COTP fixed part uses the genuinely-constructible + /// class-0 DT shape (`LI=2`, code `0xF0`, TPDU-NR+EOT `0x80`, no upper-layer + /// bytes at all) -- `payload_offset == tpkt_payload.len()` exactly, matching + /// BC-2.20.010's `protocol_id: None` case (mirrors the fix applied to this + /// file's top-level `dt_frame_empty_payload()` helper, F-03). + fn dt_frame_for_protocol_id(protocol_id: Option) -> Vec { + let cotp = match protocol_id { + Some(0x32) => { + let mut cotp = vec![0x01u8, 0xF0, 0x32]; + cotp.extend_from_slice(&[0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00]); + cotp + } + Some(byte) => vec![0x01u8, 0xF0, byte], + None => vec![0x02u8, 0xF0, 0x80], + }; + tpkt_frame(&cotp) + } + + proptest! { + /// VP-053 (partial skeleton, v1.1): for any `protocol_id` (`None` or + /// `Some(byte)`), the first DT frame observed on a fresh flow must + /// classify it exactly per BC-2.21.002's four-way table: `Some(0x32)` -> + /// `Classic`, `Some(0x72)` -> `Plus`, any other `Some(byte)` -> + /// `Unclassified`, and `None` -> NO classification at all (`None` stays + /// `None`, F-02) -- and a `S7commFlowState` must always be created + /// (BC-2.21.001 postcondition 3), regardless of `protocol_id`. + /// + /// Sticky-gating (F-12) is NOT re-exercised in this single-frame-per-flow + /// skeleton -- it is covered by the dedicated + /// `test_BC_2_21_002_0x32_dt_frame_not_dissected_when_sticky_classified_plus_or_unclassified` + /// test above; extending this proptest to a multi-frame-per-flow + /// sequence is deferred to STORY-194's full non-vacuous run alongside the + /// Some(0x72)/Some(other) dissection completeness deferred to STORY-190. + #[test] + fn proptest_vp053_protocol_id_dispatch_totality( + protocol_id in protocol_id_strategy(), + salt in any::(), + ) { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_for(salt); + + let frame = dt_frame_for_protocol_id(protocol_id); + analyzer.on_data(flow_key.clone(), &frame, 0, Direction::ClientToServer); + + let state = analyzer.flows.get(&flow_key).expect( + "VP-053: S7commFlowState must be created lazily on the first \ + on_data call regardless of protocol_id (BC-2.21.001 postcondition 3)" + ); + + let expected = match protocol_id { + Some(0x32) => Some(S7Protocol::Classic), + Some(0x72) => Some(S7Protocol::Plus), + Some(_) => Some(S7Protocol::Unclassified), + None => None, + }; + prop_assert_eq!( + state.classified_protocol, + expected, + "VP-053 LOAD-BEARING property (v1.1, F-02): the first DT frame's \ + protocol_id must classify the flow exactly per BC-2.21.002's \ + four-way table, with protocol_id: None carrying NO protocol \ + evidence and never classifying -- protocol_id={:?}", + protocol_id + ); + + // P11-F-4: `dt_frame_for_protocol_id` never generates a MALFORMED + // classic-S7comm payload -- the `Some(0x32)` branch always builds a + // complete, well-formed 10-byte Job header with empty + // parameter/data blocks (verified above the `proptest!` block), and + // every other `protocol_id` value (`None`, `Some(0x72)`, + // `Some(other)`) never reaches `dispatch_classic_s7comm` at all (the + // F-12 gate only fires for `Some(0x32)`). So across every value this + // strategy can generate, zero findings must ever be emitted -- + // scoped to this proptest's single-frame-per-flow generator only, + // not a general claim about all possible classic S7comm frames. + prop_assert!( + analyzer.findings.is_empty(), + "VP-053: this strategy generates only a well-formed classic Job \ + header (protocol_id Some(0x32)) or frames that never reach \ + classic dissection at all -- no scenario this generator can \ + produce should ever emit a finding, but got {:?} for \ + protocol_id={:?}", + analyzer.findings, + protocol_id + ); + } + } + } + + /// A trivial compile-time/type-level sanity check that `S7commFlowState`'s + /// STORY-187 field set is exactly what BC-2.21.001 postcondition 1 requires -- + /// this is a construction-only check (no `on_data` call), kept separate from + /// `test_BC_2_21_001_flow_state_field_set` above for documentation clarity. + #[test] + fn test_BC_2_21_001_fields_constructible_with_expected_types() { + let state = S7commFlowState { + carry_c2s: Vec::new(), + carry_s2c: Vec::new(), + carry_overflow_reported_c2s: false, + carry_overflow_reported_s2c: false, + session_established: false, + cr_observed_dir: None, + classified_protocol: None, + malformed_header_reported_c2s: false, + malformed_header_reported_s2c: false, + }; + assert_eq!(state.classified_protocol, None); + } + + // ========================================================================= + // Pass-3 F-25: the committed `tests/fixtures/s7comm-setup-comm.pcap` fixture + // (generated by `tests/fixtures/mk_s7comm_pcap.py`) must actually be + // exercised by a test, not merely generated and left unread. This also + // regression-guards the F-25 fix: `mk_s7comm_pcap.py`'s Ack_Data (`0x03`) + // PDU builder previously emitted a 10-byte common header for + // `setup_communication_response`/`minimal_ack_data_pdu`, which is malformed + // under the 2026-09-24 canonical-frame holdout ruling + // (DF-CANONICAL-FRAME-HOLDOUT-001, BC-2.21.008): Ack_Data requires the same + // 12-byte header as Ack (`error_class`/`error_code` at bytes 10/11, parameter + // block starting at byte 12). + // ========================================================================= + + /// Minimal, stdlib-only Ethernet II + IPv4 + TCP walk sufficient to pull a + /// packet's source/destination ports and TCP payload slice out of a raw + /// captured frame (`RawPacket.data` — a full link-layer frame, since the + /// fixture pcap's link-type is Ethernet, `LINKTYPE_ETHERNET = 1`). Returns + /// `None` for anything that isn't a well-formed IPv4/TCP frame (not expected + /// to occur in this fixture, since every packet is Ethernet/IPv4/TCP by + /// construction). Deliberately narrow — just enough to drive + /// `S7commAnalyzer::on_data` from the committed fixture below, not a + /// general-purpose packet parser. + fn extract_tcp_payload( + frame: &[u8], + ) -> Option<(std::net::IpAddr, std::net::IpAddr, u16, u16, &[u8])> { + use std::net::{IpAddr, Ipv4Addr}; + + const ETH_HEADER_LEN: usize = 14; + if frame.len() < ETH_HEADER_LEN + 20 { + return None; + } + let ethertype = u16::from_be_bytes([frame[12], frame[13]]); + if ethertype != 0x0800 { + // Not IPv4 — none of this fixture's frames take this branch. + return None; + } + + let ip_start = ETH_HEADER_LEN; + let version_ihl = frame[ip_start]; + if version_ihl >> 4 != 4 { + return None; + } + let ihl_bytes = usize::from(version_ihl & 0x0F) * 4; + if ihl_bytes < 20 || frame.len() < ip_start + ihl_bytes { + return None; + } + let protocol = frame[ip_start + 9]; + if protocol != 6 { + // Not TCP — none of this fixture's frames take this branch. + return None; + } + let src_ip = IpAddr::V4(Ipv4Addr::new( + frame[ip_start + 12], + frame[ip_start + 13], + frame[ip_start + 14], + frame[ip_start + 15], + )); + let dst_ip = IpAddr::V4(Ipv4Addr::new( + frame[ip_start + 16], + frame[ip_start + 17], + frame[ip_start + 18], + frame[ip_start + 19], + )); + let total_len = u16::from_be_bytes([frame[ip_start + 2], frame[ip_start + 3]]) as usize; + let ip_end = (ip_start + total_len).min(frame.len()); + + let tcp_start = ip_start + ihl_bytes; + if frame.len() < tcp_start + 20 || tcp_start > ip_end { + return None; + } + let src_port = u16::from_be_bytes([frame[tcp_start], frame[tcp_start + 1]]); + let dst_port = u16::from_be_bytes([frame[tcp_start + 2], frame[tcp_start + 3]]); + let data_offset_bytes = usize::from(frame[tcp_start + 12] >> 4) * 4; + let payload_start = tcp_start + data_offset_bytes; + if payload_start > ip_end { + return None; + } + Some(( + src_ip, + dst_ip, + src_port, + dst_port, + &frame[payload_start..ip_end], + )) + } + + /// Reads the committed `tests/fixtures/s7comm-setup-comm.pcap` capture via + /// [`wirerust::reader::PcapSource::from_file`] (the same public entry point + /// `tests/e2e_corpus_smoke_tests.rs` uses for its committed-fixture walk), + /// extracts each packet's TCP payload with [`extract_tcp_payload`] above, and + /// feeds every non-empty payload through [`S7commAnalyzer::on_data`] in the + /// correct direction (destination port 102 -- the ISO-on-TCP/S7comm server + /// port, ADR-014 -- is `ClientToServer`; the reverse is `ServerToClient`). + /// + /// Asserts the fixture is entirely well-formed under the corrected 12-byte + /// Ack/Ack_Data header (pass-3 F-25): zero findings, `session_established == + /// true` (from the fixture's COTP CR/CC handshake), and `classified_protocol + /// == Some(S7Protocol::Classic)` (from the fixture's `0x32` protocol-ID + /// frames). + #[test] + fn test_BC_2_21_002_setup_comm_fixture_pcap_well_formed_no_findings() { + let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures/s7comm-setup-comm.pcap"); + let source = wirerust::reader::PcapSource::from_file(&path) + .expect("the committed s7comm-setup-comm.pcap fixture must parse cleanly"); + + const SERVER_PORT: u16 = 102; + + let mut analyzer = S7commAnalyzer::new(); + let mut flow_key: Option = None; + let mut payload_frames_seen = 0usize; + + for packet in &source.packets { + let Some((src_ip, dst_ip, src_port, dst_port, payload)) = + extract_tcp_payload(&packet.data) + else { + continue; + }; + if payload.is_empty() { + // TCP handshake (SYN/SYN-ACK/ACK) and the trailing FIN-ACK carry no + // upper-layer payload -- nothing to feed to `on_data`. + continue; + } + payload_frames_seen += 1; + + let direction = if dst_port == SERVER_PORT { + Direction::ClientToServer + } else { + Direction::ServerToClient + }; + // FlowKey::new canonicalizes its (ip, port) pairs internally, so + // passing this packet's own src/dst (in either direction) yields the + // same key for the whole bidirectional flow. + let key = flow_key + .get_or_insert_with(|| FlowKey::new(src_ip, src_port, dst_ip, dst_port)) + .clone(); + + analyzer.on_data(key, payload, packet.timestamp_secs, direction); + } + + assert_eq!( + payload_frames_seen, 6, + "expected exactly 6 TCP-payload-bearing packets in the fixture (COTP CR, \ + COTP CC, Setup Comm Job, Setup Comm Ack_Data, minimal Job, minimal \ + Ack_Data) -- got {payload_frames_seen}; the fixture's packet sequence may \ + have drifted from mk_s7comm_pcap.py's documented Packet sequence" + ); + + assert!( + analyzer.findings.is_empty(), + "the committed s7comm-setup-comm.pcap fixture must be entirely well-formed \ + under the corrected 12-byte Ack_Data header (pass-3 F-25) -- got {:?}", + analyzer.findings + ); + + let flow_key = + flow_key.expect("the fixture must contain at least one TCP-payload-bearing packet"); + let state = analyzer + .flows + .get(&flow_key) + .expect("S7commFlowState must exist for the fixture's single flow"); + assert!( + state.session_established, + "the fixture's COTP Connect Request / Connect Confirm handshake must set \ + session_established (BC-2.21.001 postcondition 1, BC-2.21.002 \ + postcondition 2)" + ); + assert_eq!( + state.classified_protocol, + Some(S7Protocol::Classic), + "the fixture's classic S7comm (protocol_id 0x32) DT frames must \ + sticky-classify the flow Classic (BC-2.21.002 postcondition 3)" + ); + } + + // ========================================================================= + // Pass-14 test-adequacy additions (P14-F-1 through P14-F-4, P13-F-4b). + // ========================================================================= + + /// P14-F-1: a flow already sticky-classified Classic (via a well-formed Job DT + /// frame) receives THREE further DT frames whose `protocol_id` is NOT + /// `Some(0x32)` -- `Some(0x72)` (Plus-shaped), `Some(0x99)` carrying bytes that + /// would otherwise parse as a well-formed classic Job header if mis-dissected + /// starting at their own first byte (proving the gate keys on `protocol_id`, not + /// on payload shape), and a `protocol_id: None` empty-payload DT frame. None of + /// the three may reach `dispatch_classic_s7comm` -- the `Some(0x32)` match arm in + /// `dispatch_cotp_frame` (BC-2.21.002) is the ONLY route into classic dissection, + /// so a non-`0x32` `protocol_id` must never emit a finding, must never flip + /// `malformed_header_reported_c2s`, and must never disturb the flow's sticky + /// `classified_protocol`. + /// + /// Kills a wrong implementation that widens the classic-dissection gate to fire + /// on every DT frame once a flow is sticky-classified Classic (i.e. hoists the + /// `if state.classified_protocol == Some(S7Protocol::Classic)` check outside the + /// `Some(0x32)` match arm instead of leaving it inside that arm) -- under that + /// mutant, the `Some(0x72)`/`Some(0x99)`/`None`-protocol_id frames below would + /// each be handed to `dispatch_classic_s7comm`, whose `payload.first() == + /// Some(&0x32)` `debug_assert_eq!` (F-15, src/analyzer/s7comm.rs) panics for + /// every one of them under `cargo test`'s debug profile -- itself a test failure. + /// + /// Traces: BC-2.21.002 postcondition 3, invariant 4. + #[test] + fn test_BC_2_21_002_non_0x32_dt_frame_on_classic_flow_not_dissected() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + // Establish sticky Classic via a well-formed Job PDU. + let good_header = classic_header_bytes(0x01, 0x0001, 0x0000, 0x0000); + let good_frame = dt_frame(&good_header); + analyzer.on_data(flow_key.clone(), &good_frame, 0, Direction::ClientToServer); + { + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.classified_protocol, + Some(S7Protocol::Classic), + "precondition: the flow must already be sticky-classified Classic" + ); + } + assert!( + analyzer.findings.is_empty(), + "precondition: no findings yet" + ); + + // Three further DT frames, none with protocol_id Some(0x32). + analyzer.on_data( + flow_key.clone(), + &dt_frame(&[0x72]), + 1, + Direction::ClientToServer, + ); + analyzer.on_data( + flow_key.clone(), + &dt_frame(&[0x99, 0x01, 0, 0, 0, 1, 0, 0, 0, 0]), + 2, + Direction::ClientToServer, + ); + analyzer.on_data( + flow_key.clone(), + &dt_frame_empty_payload(), + 3, + Direction::ClientToServer, + ); + + assert!( + analyzer.findings.is_empty(), + "no non-0x32 DT frame on an already-Classic flow may ever emit a finding \ + (P14-F-1, BC-2.21.002 postcondition 3 / invariant 4) -- got {:?}", + analyzer.findings + ); + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.classified_protocol, + Some(S7Protocol::Classic), + "classified_protocol must remain Classic -- untouched by non-0x32 DT \ + frames (P14-F-1)" + ); + assert!( + !state.malformed_header_reported_c2s, + "malformed_header_reported_c2s must remain false -- no non-0x32 DT frame \ + may ever reach dispatch_classic_s7comm (P14-F-1)" + ); + } + + /// P14-F-2: a COTP TPDU whose code high-nibble is `0x80` -- not in `{0xE0, 0xD0, + /// 0xF0}` -- causes `parse_cotp_header` to return `None` (its `_ => None` catch- + /// all arm, src/analyzer/iso_on_tcp.rs). Routed through `dispatch_cotp_frame`'s + /// `None` arm (the unclassified-gap placeholder): no CR/CC session-tracking + /// write, no protocol classification, no finding. A subsequent, well-formed + /// classic Job DT frame on the SAME flow then classifies normally -- the earlier + /// unparseable COTP frame leaves no residual state behind. + /// + /// Traces: BC-2.21.002 postcondition 1 (`None`-from-`parse_cotp_header` + /// unclassified gap). + #[test] + fn test_BC_2_21_002_unparseable_cotp_does_not_classify() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + // TPDU code 0x80 -- high nibble 0x80 & 0xF0 == 0x80, not in {0xE0, 0xD0, + // 0xF0} -- parse_cotp_header returns None. + let unparseable = tpkt_frame(&[0x01, 0x80, 0x32, 0x01, 0, 0, 0, 1, 0, 0, 0, 0]); + analyzer.on_data(flow_key.clone(), &unparseable, 0, Direction::ClientToServer); + + { + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.classified_protocol, None, + "an unparseable COTP frame must never classify the flow (P14-F-2, \ + BC-2.21.002 postcondition 1)" + ); + assert!( + !state.session_established, + "an unparseable COTP frame must never set session_established (P14-F-2)" + ); + assert_eq!( + state.cr_observed_dir, None, + "an unparseable COTP frame must never populate cr_observed_dir \ + (P14-F-2)" + ); + } + assert!( + analyzer.findings.is_empty(), + "an unparseable COTP frame must never emit any finding (P14-F-2)" + ); + + // A well-formed classic Job DT frame on the SAME flow, afterward, classifies + // normally -- the unparseable frame left no residual state. + let header = classic_header_bytes(0x01, 0x0001, 0x0000, 0x0000); + let good_frame = dt_frame(&header); + analyzer.on_data(flow_key.clone(), &good_frame, 1, Direction::ClientToServer); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert_eq!( + state.classified_protocol, + Some(S7Protocol::Classic), + "a subsequent well-formed classic Job DT frame must classify Classic \ + normally after an earlier unparseable COTP frame (P14-F-2)" + ); + assert!( + analyzer.findings.is_empty(), + "the well-formed classic Job PDU must not itself emit any finding" + ); + } + + /// P14-F-3: once `session_established` becomes true, no subsequent CR/CC + /// activity on the flow may ever set it back to false -- monotonic latch + /// semantics. Two sequences, each establishing the flag via an opposite- + /// direction CC and then exercising further CR/CC traffic that must leave it + /// unchanged: (a) CR(c2s), CC(s2c) [establishes], CC(c2s) [same-direction as the + /// still-recorded CR, per F-01 a non-match -- must not clear the flag]; (b) + /// CR(c2s), CC(s2c) [establishes], CR(s2c) [overwrites `cr_observed_dir` -- + /// `dispatch_cotp_frame`'s `ConnectRequest` arm never reads or writes + /// `session_established`, so this must not clear it either]. + /// + /// Kills a wrong implementation that uses assignment-form + /// (`state.session_established = cr_dir != direction;`) in the `ConnectConfirm` + /// arm instead of the guarded `if cr_dir != direction { state.session_established + /// = true; }` form -- under that mutant, sequence (a)'s final same-direction CC + /// (`cr_dir == c2s == direction`) evaluates `cr_dir != direction` to `false` and + /// UNCONDITIONALLY assigns `session_established = false`, flipping the flag back + /// off and failing this test. + /// + /// Traces: BC-2.21.001 postcondition 1. + #[test] + fn test_BC_2_21_001_session_established_is_monotonic() { + // (a) CR(c2s), CC(s2c), CC(c2s) -- still true. + { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + analyzer.on_data(flow_key.clone(), &cr_frame(), 0, Direction::ClientToServer); + analyzer.on_data(flow_key.clone(), &cc_frame(), 1, Direction::ServerToClient); + { + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + state.session_established, + "precondition: the opposite-direction CC must establish the \ + session first" + ); + // P23-F-1 (BC-2.21.001 PC1): the `ConnectConfirm` arm only ever + // READS `cr_observed_dir` to decide whether to set + // `session_established` -- it never clears it, even on the + // establishing (matching) CC. Kills a mutant that adds + // `state.cr_observed_dir = None;` inside the `ConnectConfirm` + // match arm. + assert_eq!( + state.cr_observed_dir, + Some(Direction::ClientToServer), + "cr_observed_dir must still record the original CR direction \ + after the establishing CC -- it is never cleared on a \ + matching CC (P23-F-1, BC-2.21.001 PC1)" + ); + } + + analyzer.on_data(flow_key.clone(), &cc_frame(), 2, Direction::ClientToServer); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + state.session_established, + "a same-direction CC arriving AFTER session_established is already \ + true must leave it true -- monotonic latch, not a re-evaluated \ + condition (P14-F-3)" + ); + } + + // (b) CR(c2s), CC(s2c), CR(s2c) -- still true. + { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + analyzer.on_data(flow_key.clone(), &cr_frame(), 0, Direction::ClientToServer); + analyzer.on_data(flow_key.clone(), &cc_frame(), 1, Direction::ServerToClient); + { + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + state.session_established, + "precondition: the opposite-direction CC must establish the \ + session first" + ); + } + + analyzer.on_data(flow_key.clone(), &cr_frame(), 2, Direction::ServerToClient); + + let state = analyzer.flows.get(&flow_key).unwrap(); + assert!( + state.session_established, + "a further CR arriving AFTER session_established is already true \ + must leave it true -- the ConnectRequest arm never reads or writes \ + session_established (P14-F-3)" + ); + // P23-F-1 (BC-2.21.001 PC1): the `ConnectRequest` arm unconditionally + // overwrites `cr_observed_dir` with the new frame's direction -- it is + // NOT guarded on `session_established`. Kills a mutant that guards the + // arm with `if !state.session_established { state.cr_observed_dir = + // Some(direction); }`: under that mutant, this later CR(s2c) would be + // silently dropped (session_established is already true) and + // cr_observed_dir would remain stuck at the original + // `Some(ClientToServer)` instead of being overwritten. + assert_eq!( + state.cr_observed_dir, + Some(Direction::ServerToClient), + "cr_observed_dir must be overwritten by the later CR(s2c) even \ + though session_established is already true -- the ConnectRequest \ + arm is unconditional (P23-F-1, BC-2.21.001 PC1)" + ); + } + } + + /// P14-F-4 (P13-F-4a): a classic S7comm payload of EXACTLY 9 bytes (`32 01 00 00 + /// 00 01 00 00 00`) -- one byte short of the 10-byte common-header minimum -- on + /// an already-Classic-classified flow must emit exactly one T0814 whose evidence + /// contains the exact string "header too short: 9" (the specific byte count + /// `classify_malformed_header_reason` reports, distinguishing this from the + /// existing 1-byte-payload malformed-header tests elsewhere in this module). + /// + /// Kills a wrong implementation that changes `classify_malformed_header_reason`'s + /// `payload.len() < 10` guard to `payload.len() < 9` (F-11's evidence-text + /// classifier, src/analyzer/s7comm.rs) -- under that mutant, this exact 9-byte + /// payload no longer takes the "header too short" branch (`9 < 9` is `false`); it + /// falls through to `match payload[1]` with `payload[1] == 0x01` (Job), which is + /// not one of the `0x02`/`0x03` arms, so it takes the `other =>` catch-all and + /// reports "unrecognized ROSCTR byte 0x01" instead -- the evidence-substring + /// assertion below then fails. (The reject DECISION itself is unaffected by this + /// mutant -- `parse_s7comm_header`'s own `data.len() < 10` guard is untouched -- + /// so only the evidence TEXT changes, which is exactly what this test targets.) + /// + /// Traces: BC-2.21.004 postcondition 4; F-11. + #[test] + fn test_BC_2_21_004_nine_byte_payload_on_data_too_short_evidence() { + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + + // Establish sticky Classic via a well-formed Job PDU first. + let good_header = classic_header_bytes(0x01, 0x0001, 0x0000, 0x0000); + let good_frame = dt_frame(&good_header); + analyzer.on_data(flow_key.clone(), &good_frame, 0, Direction::ClientToServer); + assert!( + analyzer.findings.is_empty(), + "precondition: no findings yet" + ); + + // Exactly 9 bytes: [0x32, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00]. + let nine: [u8; 9] = [0x32, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00]; + let frame = dt_frame(&nine); + analyzer.on_data(flow_key.clone(), &frame, 1, Direction::ClientToServer); + + assert_eq!( + analyzer.findings.len(), + 1, + "a 9-byte classic-S7comm payload must emit exactly one T0814 (P14-F-4, \ + BC-2.21.004 postcondition 4)" + ); + assert_malformed_header_t0814(&analyzer.findings[0], Direction::ClientToServer); + assert_reason_specific_evidence(&analyzer.findings[0], "header too short: 9"); + } + + /// P13-F-4b (BC-2.21.008 EC-007): an Ack_Data (`0x03`) header with NON-ZERO + /// `error_class`/`error_code` (`0x81`/`0x04`) AND a non-empty parameter block -- + /// `param_length == 2` with exactly 2 trailing parameter bytes present -- + /// verified two ways: (1) a direct `parse_s7comm_header` call returns `Some` with + /// `header_len == 12` and the exact expected error bytes (structural equality, + /// mirroring `test_BC_2_21_008_ack_data_12_byte_header_and_error_fields` above but + /// additionally exercising a non-empty trailing parameter block); (2) fed through + /// `on_data` on a (self-classifying, `protocol_id: Some(0x32)`) flow, the SAME + /// bytes -- with bounds satisfied (`header_len(12) + param_length(2) + + /// data_length(0) == 14 == payload.len()`) -- emit NO finding at all, since a + /// non-zero `error_class`/`error_code` and a present parameter block are both + /// fully within BC-2.21.008/BC-2.21.009's contract, not malformed-header + /// conditions. + /// + /// Traces: BC-2.21.008 postconditions 1-2, edge case EC-007. + #[test] + fn test_BC_2_21_008_ack_data_nonzero_error_fields_with_parameter_block() { + // 12-byte Ack_Data header + 2 parameter bytes = 14 bytes total. + let mut payload = ack_data_header_bytes(0x0007, 2, 0, 0x81, 0x04); + assert_eq!(payload.len(), 12); + payload.push(0xAA); + payload.push(0xBB); + assert_eq!(payload.len(), 14); + + // (1) Direct parse. + let header = parse_s7comm_header(&payload).expect( + "a well-formed 12-byte Ack_Data header with a trailing parameter block \ + must parse (BC-2.21.008 EC-007)", + ); + assert_eq!( + header, + S7commHeader { + rosctr: Rosctr::AckData, + pdu_reference: 0x0007, + param_length: 2, + data_length: 0, + error_class: Some(0x81), + error_code: Some(0x04), + header_len: 12, + }, + "the exact expected S7commHeader (BC-2.21.008 EC-007) -- header_len must \ + remain 12 regardless of the trailing parameter block's presence, since \ + header_len describes the FIXED header only" + ); + + // (2) Via on_data on a (self-classifying) flow -- bounds satisfied, so no + // finding. + let mut analyzer = S7commAnalyzer::new(); + let flow_key = flow_key_default(); + let frame = dt_frame(&payload); + analyzer.on_data(flow_key, &frame, 0, Direction::ClientToServer); + + assert!( + analyzer.findings.is_empty(), + "a well-formed Ack_Data header with non-zero error fields and a bounds- \ + satisfying trailing parameter block must not emit any finding (P13-F-4b, \ + BC-2.21.008 EC-007) -- got {:?}", + analyzer.findings + ); + } +}