Investigate REQ-CRC-015/REQ-CRC-016 against RC4/RC5 TC18.txt: old supporting spec text appears removed, not just relocated #169

Activity

1. SoundMatt commented on Aug 20, 2026

   

   SoundMatt

   on Aug 20, 2026

   OwnerAuthor

   More actions

   Investigation results

   Read the current TC18.txt (6095 lines, RC4/RC5-revised) §11.1/§11.2 (AVTPDU/NTSCF/TSCF headers, requests, fragmentation via the ms bit) and §13.6 (End-to-End data protection) in full, cross-referenced against src/e2e.rs, src/avtp.rs, src/fragment.rs, and src/request.rs in a fresh clone. Summary up front: REQ-CRC-016 is moot (two independent reasons, both in the spec text and in this crate's own code). REQ-CRC-015 is not moot — it's a real, unresolved design question, and investigating it surfaced a second, more concrete defect in the fragment-train coverage-buffer construction that's arguably more serious than the "first vs. final fragment" framing the issue posed. Flagging that as high-priority below.

   Also worth noting up front: REQ-CRC-015 and REQ-CRC-016 are not actually present in .fusa-reqs.json. I grepped the full requirements list (628 entries) and both IDs are absent — REQ-CRC-001..014,017,019 exist, 015/016/018 do not. The two IDs exist only as prose mentions inside src/e2e.rs's doc comments, never as //fusa:req///fusa:test tags, so scripts/fusa-gap-check.sh's traceability gate (which greps for those exact tag patterns) doesn't see them either way — it currently reports a clean "Coverage: 628/628 (100%)". This matters for the recommendation below: there is no formal requirement entry to flip to not-implemented/resolved/delete — only prose to rewrite.

   ---

   REQ-CRC-016 (ntscf_data_length/stream_data_length +4-octet adjustment): MOOT

   Confirmed by full-text search that ntscf_data_length and stream_data_length each appear exactly once in current TC18.txt outside this one CRC section — as the field labels in the Figure 20/21 worked-example header diagrams (TC18.txt lines 4222, 4251) — and the surviving CRC-adjustment sentence (lines 4272-4273) only covers acf_msg_length:

   "When calculating the CRC, it needs to be considered that adding the CRC to the byte_msg_payload increases the acf_msg_length by 1." (TC18.txt L4272-4273)

   No adjustment rule for the AVTPDU-level length fields survives anywhere under any wording. But the interesting finding is why that's fine, from two independent angles:

   1. Spec-theoretic: §11.1 (TC18.txt L1116-1140) introduces the NTSCF/TSCF headers only via image figures (Figure 5/6) with no accompanying field-by-field text table (unlike every ACF message type, which gets an explicit "This field indicates the number of quadlets…" table). ntscf_data_length/stream_data_length are IEEE 1722-lineage fields TC18 doesn't redefine; by construction they describe "how many octets of payload actually follow this header," a directly-measured quantity, not a separately-maintained counter. A "+4 octets when you add a CRC" sentence on top of that is redundant hand-holding, not a distinct normative rule — consistent with the RC4/RC5 editors cutting it as unnecessary rather than dropping a real requirement.

   2. Concretely, in this crate's own code (src/avtp.rs L646-662, encode_ntscf_frame):

   pub fn encode_ntscf_frame(stream_id: StreamId, sequence_num: u8, acf_payload: &[u8]) -> Result<Vec<u8>, RcpError> {
       let ntscf_data_length = u16::try_from(acf_payload.len()).map_err(|_| RcpError::InvalidSize)?;
       ...

   ntscf_data_length is derived by directly measuring acf_payload.len() — whatever byte slice the caller hands in. If the caller has already appended a CRC trailer (via e2e::finalize_crc_trailer, src/e2e.rs L531-545) before calling this, the length field is automatically, structurally correct; there is no separately-tracked value that could desync from the true payload length, so the failure mode the old TC18 sentence warned about ("forgot to bump the length field") is architecturally impossible here — it's safe by construction, not by a rule anyone has to remember to apply.

   And for the second half of the concern (stream_data_length/TSCF): there is no encode_tscf_frame function in this crate at all — src/avtp.rs L632-640 states this deliberately ("There is deliberately no encode_tscf_frame counterpart… nothing in this crate assembles a TSCF-headed frame"), consistent with TC18 L2354 ("The RC Server always uses NTSCF header"). There's no code path where stream_data_length could be wrong, because there's no code path that sets it from a live payload at all.

   One more relevant fact: finalize_crc_trailer (the function that would append a CRC before a caller calls encode_ntscf_frame) has zero call sites anywhere in src/ outside its own tests — confirmed by grepping every transport module (l2.rs, udp.rs, shmem.rs, tlstransport.rs, mock.rs). The live E2E safe-mode encode path isn't wired into any transport yet, so even setting the "safe by construction" argument aside, there is currently no code anywhere in this crate that could exhibit the bug REQ-CRC-016 worries about.

   Verdict: moot. No code change needed. Recommend closing this out.

   ---

   REQ-CRC-015 (fragmented-request CRC header coverage): real, unresolved — and the investigation found something more concrete than "first vs. final"

   Is "first vs. final" even a real distinction? Yes — contrary to the possibility the issue raised (that TC18's per-fragment header fields might be identical across a train, making "first vs. final" a non-issue). Checked against TC18's own per-field definitions, repeated verbatim at every ACF message-type table (e.g. TC18.txt L1219-1235):

   • acf_msg_length — "the number of quadlets of data contained in this [ABB/GBB] message" (singular, per-message/per-fragment; TC18.txt L1220 and ~20 repeats). Necessarily differs between fragments — each carries only its own segment's quadlet count.
   • ms — "0b – the fragment sequence is completed with this message; 1b – more data to follow" (TC18.txt L1233). By definition true on every fragment except the last.
   • read_size/segment_num — the segment counter (TC18.txt L1235, L5860 "supported by the fields 'ms' and 'segment_num'"). Strictly increments per fragment by this crate's own reassembly logic (src/fragment.rs L211-214).

   So a coverage buffer built from the first fragment's header (acf_msg_length = first segment's own small quadlet count, ms = true, segment_num = 0) and one built from the final fragment's header (acf_msg_length = last segment's own quadlet count, ms = false, segment_num = N-1) are genuinely different byte sequences for any train with ≥2 segments — not just symbolically different, materially different, so they produce different CRC32 values. "First vs. final" was never a non-issue, under either the old or current text.

   What current TC18.txt actually says (L4274-4275):

   "Fragmentation, if applied to a message, happens after CRC calculation. So, the CRC is only calculated once and will be transmitted in the last segment."

   Read literally, this describes a third model, not quite "first fragment" or "final fragment": CRC calculation happens before fragmentation, i.e. conceptually over one whole, not-yet-split ACF message — which would mean the coverage buffer's header fields (especially acf_msg_length) should reflect the total combined/reassembled length, with ms = false, and no fragment-specific segment_num at all, since fragmentation hasn't happened yet at CRC-computation time. This is neither "first fragment's real wire header" nor "final fragment's real wire header" — it's a hypothetical, never-transmitted "whole message" header.

   This crate does neither. build_crc32_coverage_buffer_for_fragment_train (src/e2e.rs L645-668) takes final_fragment: &AcfCoverageMessage, replaces its payload with the train's combined payload, but reuses final_fragment's own info (including its own real, per-fragment acf_msg_length) unchanged except for the existing +1 quadlet bump:

   let combined_msg = AcfAbbMessage { info: msg.info, payload: combined.0 };
   build_crc32_coverage_buffer(header, &AcfCoverageMessage::Abb(&combined_msg))
   // (inside build_crc32_coverage_buffer: adjusted_info.acf_msg_length = adjusted_info.acf_msg_length.saturating_add(1))

   acf_msg_length is never recomputed from combined.0.len(). So the coverage buffer's header claims a small, single-fragment quadlet count while the bytes actually being hashed after it are the full multi-fragment combined payload — an internal header/payload length mismatch inside the CRC input, independent of whether "final fragment" or "first fragment" is the right header source. This is the more concrete, more actionable finding: regardless of how the first-vs-final question gets resolved, this crate's fragment-train coverage buffer likely does not match what a strictly-conformant peer following the "calculated once, as if whole" reading would compute, for any real multi-segment (≥2 fragment) safe-mode train.

   Does this crate enforce fragment-train identity at all? No. FragmentReassemblyBuffer::accept_fragment (src/fragment.rs L202-230) only stores each fragment's raw payload bytes and validates segment_num's strictly-incrementing sequence; it discards every other field of the passed-in ByteMessageInfo (transaction_num, byte_bus_id, acf_msg_type, …) after that one check. Nothing in this crate verifies that those fields — which TC18 implies must stay constant across one fragmented request's train (transaction_num "identifies the request in the context of the stream", TC18.txt L1228/L2144) — actually are constant. That's a related, independent gap worth a mention but is separate from REQ-CRC-015 itself.

   Is this a currently-shipping, active defect? Not in the sense of a live dispatch loop failing today — grepped every transport module (l2.rs, udp.rs, shmem.rs, tlstransport.rs, mock.rs) and src/request.rs's check_rx_enforce_e2e (L2679-2689, the one place that consumes a computed CRC): zero call sites for build_crc32_coverage_buffer_for_fragment_train, crc32_tc18_for_fragment_train, or verify_reassembled_train_crc anywhere outside e2e.rs/fragment.rs's own tests. Nothing in this crate currently wires fragment-train CRC computation into a real send or receive path.

   But e2e and fragment are both pub mod (src/lib.rs L88, L94) and all three functions are pub fn — they're live, documented, callable public API today. Any downstream consumer of this crate calling them for a real multi-segment safe-mode CRC today would get a CRC value that (a) doesn't match a hypothetical strictly-conformant "whole-message" reading of the current TC18 text, and (b) is not verified against any real second implementation. This is a latent defect ready to become a real interop failure the moment it's exercised — either by a future dispatch-loop wiring inside this crate, or by any external caller already using the public API directly.

   Verdict: real, unresolved — not moot, and not a simple "known divergence from clear TC18 text" either, since current TC18 no longer states a clear rule to diverge from. It's a genuine spec ambiguity TC18 itself needs to resolve, compounded by an internal implementation inconsistency (the un-recomputed acf_msg_length) that exists independent of which header-source interpretation wins.

   ---

   Recommendation

   1. Close REQ-CRC-016 as moot. Small, safe follow-up: rewrite src/e2e.rs's "Flag, deliberately not resolved here" section (L68-82, and the "original reasoning" block L84-98) to record the resolution above (safe-by-construction encode_ntscf_frame, no encode_tscf_frame, zero live callers of finalize_crc_trailer) instead of leaving it as an open flag. I did not make this edit myself this pass — the doc block is long, cross-referenced from multiple places, and touches phrasing other engineers may be relying on for context, so it's better done deliberately rather than as a drive-by from an investigation pass. Since REQ-CRC-016 isn't a real .fusa-reqs.json entry, there's nothing to update there — just the prose.

   2. Keep REQ-CRC-015 flagged — do not silently resolve it. This needs a human/engineering decision, not a guess, for two reasons: (a) current TC18 text is genuinely silent on which header a fragment-train CRC coverage buffer should draw from, and (b) fixing it means picking one of at least three candidate models (first-fragment header, final-fragment header, or a synthesized "whole reassembled message" header with a recomputed acf_msg_length) — exactly the "changes function contracts" situation the issue itself flagged as out of scope for a guessed fix. Recommend filing/keeping this as an explicit open engineering question, ideally cross-checked against cpp-RCP/c-RCP's actual fragment-train CRC behavior (mentioned elsewhere in this crate's comments as a cross-implementation reference) or raised with OPEN Alliance directly, since the spec text itself is ambiguous.

   3. High-priority, separate from the "first vs. final" framing: regardless of which header-source model wins, build_crc32_coverage_buffer_for_fragment_train (src/e2e.rs L645-668) never recomputes acf_msg_length from the combined payload's actual length — it always reuses whichever single fragment's own (necessarily much smaller) acf_msg_length, plus 1. This looks like it will produce a non-interoperable CRC for essentially every real ≥2-segment safe-mode fragment train under any reading of "the CRC is calculated once… as if whole." It's not actively firing in production today (no live caller in this crate), but it's reachable right now via public API (pub mod e2e/pub mod fragment, src/lib.rs L88/94), so it should be treated as a real defect-in-waiting, not a cosmetic citation issue. Recommend whoever picks up REQ-CRC-015 resolve this acf_msg_length question in the same pass, since it's part of the same coverage-buffer contract.

   4. Minor, incidental: src/fragment.rs's FragmentReassemblyBuffer doesn't validate that transaction_num/byte_bus_id/acf_msg_type stay constant across a fragment train's segments — worth a small hardening follow-up, independent of the CRC question, but not blocking.

   Happy to open a small doc-only PR for item 1 if that's wanted — held off this pass per the task's "don't touch code/contracts without high confidence and low risk" instruction, since even the doc-only rewrite is a fairly large, multi-paragraph, cross-referenced edit rather than a one-liner.
2. SoundMatt commented on Aug 20, 2026

   

   SoundMatt

   on Aug 20, 2026

   OwnerAuthor

   More actions

   Independently re-verified the investigation agent's finding by reading src/e2e.rs/src/fragment.rs directly (not trusting the report at face value): the REQ-CRC-016 "moot" conclusion checks out — encode_ntscf_frame derives ntscf_data_length from a live payload measurement, no separately-tracked counter exists to desync, and there's no encode_tscf_frame at all. Confirmed the "zero call sites outside tests" claim for the fragment-train CRC chain via a full-repo grep myself.

   Split the concrete acf_msg_length-never-recomputed defect out to its own issue, #170, since it's a real code bug distinct from (and unconditional on) this issue's own header-model question — worth tracking and fixing separately from the citation-drift/spec-interpretation work this issue is actually about.