diff --git a/bpa/src/dispatcher/forward.rs b/bpa/src/dispatcher/forward.rs index 7865cbbcb..6109022c8 100644 --- a/bpa/src/dispatcher/forward.rs +++ b/bpa/src/dispatcher/forward.rs @@ -273,12 +273,10 @@ impl Dispatcher { .. } = hardy_bpv7::parse::parse(source_data).map_err(hardy_bpv7::editor::Error::from)?; - // RFC 9171 §4.2.3-4/-5: report_on_failure MUST NOT be set on any block - // of an admin-record or anonymous bundle — the receiver has nowhere - // meaningful to report to, and a conformant parser (ours included) - // rejects the combination. - let report_on_failure = - !bundle.primary().flags.is_admin_record && !bundle.id().source.is_null(); + // RFC 9171 §4.2.3-4/-5: an admin-record or anonymous bundle's blocks + // may not request a report on failure, and a conformant parser (ours + // included) rejects the combination. + let report_on_failure = !bundle.primary().forbids_report_on_failure(); // The per-hop blocks below are replaced through `insert_block`, which // strips a replaced block from any plaintext BIB that covers it. That diff --git a/bpa/src/filter/validity.rs b/bpa/src/filter/validity.rs index 6876f1ec3..f91234733 100644 --- a/bpa/src/filter/validity.rs +++ b/bpa/src/filter/validity.rs @@ -21,10 +21,11 @@ pub struct BundleValidityFilter; #[async_trait] impl ReadFilter for BundleValidityFilter { async fn filter(&self, bundle: &Bundle, _data: &[u8]) -> Result { - if let Some(u) = bundle.primary().flags.unrecognised { + let unrecognised = bundle.primary().flags.unrecognised; + if unrecognised != 0 { debug!( bundle_id = %bundle.id(), - "Bundle primary block has unrecognised flag bits set: {u:#x}" + "Bundle primary block has unrecognised flag bits set: {unrecognised:#x}" ); } diff --git a/bpv7/CHANGELOG.md b/bpv7/CHANGELOG.md index 007f7e96a..4ff71c9af 100644 --- a/bpv7/CHANGELOG.md +++ b/bpv7/CHANGELOG.md @@ -8,13 +8,20 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Added - `bpsec::DecryptingReader`: a `reader::Reader` that decrypts BCB-covered blocks on demand and memoises each block's outcome — plaintext, no-key, or decrypt-failure — for the reader's lifetime, so a chain of consumers sharing one reader through the trait costs one decrypt attempt per covered block (the give doors re-run a cached failure to report its exact cause). The trait impl lends (cache borrows, zeroized when the reader drops); the inherent `block_data()` gives (owned plaintext, typed errors carrying the diagnostic cause, and `Ok(None)` for non-resident extents), preserving the free `bpsec::block_data` contract; `into_block_data()` is its consuming form, which moves a covered block's plaintext out instead of cloning it, for callers that read one block and drop the reader. -- `extension_editor::ExtensionEditor`: scoped extension-block editing over a parsed bundle — insert/replace/remove of extension blocks only. Some owner privileges are absent from the type (the primary-field setters, BIB/BCB creation and management, coverage stripping); target gates refuse the rest at runtime with a typed error — the primary and payload blocks, the reserved block types, security blocks as targets, and covered targets (unprovable coverage refuses conservatively). Blocks inserted through the editor are valid targets for its own replace/remove; `finish()` materialises the accumulated edits, or `None` when untouched. +- `extension_editor::ExtensionEditor`: scoped extension-block editing over a parsed bundle — insert/replace/remove of extension blocks only. Some owner privileges are absent from the type (the primary-field setters, BIB/BCB creation and management, coverage stripping); target gates refuse the rest at runtime with a typed error — the primary and payload blocks, the reserved block types, security blocks as targets, and covered targets (unprovable coverage refuses conservatively). Blocks inserted through the editor are valid targets for its own replace/remove; `finish()` materialises the accumulated edits, or `None` when untouched. An edit that would make a malformed bundle is refused at call time instead of failing at `finish()` or at a receiver's decoders: what the structural parser rejects, as `Error::Invalid` with the parser's own error (`InvalidFlags` for a `report_on_failure` flag the bundle forbids, `InvalidCrc` for an unrecognised CRC type), and Previous Node, Bundle Age, or Hop Count data that does not decode as its type (on `insert` and `replace`), as `Error::UndecodableBody` carrying the type and its decode error — data the parser never decodes but a receiving BPA does. Receiver policy over a well-formed bundle — RFC 9171 §4.4.2's Bundle Age requirement on a bundle without a clock, hop limits, a receiver's handling of unsupported blocks — is the caller's decision and is not refused. +- `PrimaryBlock::forbids_report_on_failure()`: the one statement of RFC 9171 §4.2.3-4/-5's block-flag rule — an administrative-record or null-source bundle forbids the `report_on_failure` flag on every block. The parser rejects the combination through it, `Builder::build` clears the flag, `ExtensionEditor` refuses it at call time, and the owner `Editor` refuses it when it rebuilds, once the primary is final. - `reader::ReaderExt`, blanket-implemented for every `Reader` (trait objects included): `extract()` CBOR-decodes a block's payload, with `Ok(None)` for absent-or-unavailable and `Err` only for decode failures. +- `block::Flags` and `bundle::Flags` implement `Hash`, as `rfc9173::ScopeFlags` does; hashing goes by the encoding (see Changed). +- `canonicalize()` on `block::Flags`, `bundle::Flags`, and `rfc9173::ScopeFlags`: it folds each bit of `unrecognised` that names a flag into that flag's field, mirroring `block::Type::canonicalize()`. `rfc9173::ScopeFlags` gains `From` and `From<&ScopeFlags> for u64`, the conversions its CBOR codec delegates to. +- `is_canonical()` on `block::Flags`, `bundle::Flags`, `rfc9173::ScopeFlags`, and `block::Type`: whether a value is the form `canonicalize()` returns — the one test that tells an alias from its canonical form, which equality no longer does. ### Removed - **BREAKING:** the free `bpsec::block_data()` function — `bpsec::DecryptingReader::block_data` is the same contract (typed errors carrying the cause; owned decrypted plaintext) plus outcome memoisation and an explicit `Ok(None)` for non-resident extents. A one-shot caller migrates to `DecryptingReader::new(..).into_block_data(n)`, which, like the removed function, returns the decrypted plaintext without an extra copy. ### Changed +- **BREAKING:** the `unrecognised` field of `block::Flags`, `bundle::Flags`, and `rfc9173::ScopeFlags` is a plain `u64`, zero when no unrecognised bits are set (was `Option`, where `None` and `Some(0)` meant the same). Migration: `None` becomes `0` and `Some(bits)` becomes `bits`, so `if let Some(bits) = flags.unrecognised` becomes `if flags.unrecognised != 0`. The serde shape of `block::Flags` and `bundle::Flags` is unchanged for every value a parse or builder produces — the field is omitted when zero — except that an explicit `null` no longer deserializes. +- **BREAKING (behaviour):** equality, hashing, and ordering compare what a value encodes. `block::Flags`, `bundle::Flags`, and `rfc9173::ScopeFlags` compare and hash their wire `u64`, and `block::Type` compares, orders, and hashes its type code, so an alias equals its canonical form (`Type::Unrecognised(1) == Type::Payload`) where it previously compared unequal, and `block::Type`'s order is the code order (it was declaration order, with every `Unrecognised` code last). Pattern matching and the derived `Debug` stay structural, so code that matches on named variants or reads named fields canonicalizes first, and two equal values can print differently; `is_canonical()` tells an alias from its canonical form. Serde canonicalizes `block::Flags`, `bundle::Flags`, and `block::Type` in both directions, with the stored shape unchanged. A security scope written as an alias is the same context as its canonical form, so a context whose targets share a security block shares it across both: the `Signer` puts such targets in one BIB, while BCB-AES-GCM gives every target its own BCB. +- **BREAKING:** the owner `Editor`'s `rebuild()` and `rebuild_bundle()` refuse a block carrying `report_on_failure` on a bundle whose final primary forbids it (RFC 9171 §4.2.3-4/-5) — whether an edit set the flag or a kept block carried it under the primary the edits replaced — with the new `editor::Error::ReportOnFailureForbidden(block_number)` variant, which can break exhaustive `match` arms. They previously emitted bytes the parser rejects. - **BREAKING:** the block read abstraction is renamed and lifted to the crate root, joining the `Builder`/`Editor`/`Signer`/`Encryptor` role-noun family: trait `bpsec::BlockSet` is now `reader::Reader`, and `bpsec::PlainBlockSet` is now `reader::PlainReader`. No behavioural change — signatures and semantics are otherwise identical, and plain block reading no longer requires the `bpsec` module path. `PlainReader`, two shared references, also derives `Clone`, `Copy` and `Debug`. - **BREAKING:** `reader::Reader::block` reports the payload slot as a four-state `reader::Availability` instead of `Option`: `Available(Payload)`, `NotResident` (extents beyond the resident bytes), `NoKey` (BCB-covered, no usable key), and `NotDecryptable` (BCB-covered, and this node cannot produce the plaintext despite trying — the ciphertext failed to authenticate, or the security context or parameters are unsupported; the give doors report which). A present block's unavailable payload no longer conflates "not held in memory" with "not decryptable by this node", so a caller can respond to each state differently; callers to whom every unavailable state is equivalent use `Availability::available()`. - **BREAKING (behaviour):** the structural parse marks `BibCoverage::Maybe` only on BCB-covered blocks. As built under RFC 9172 §3.9 ("when adding a BCB"), an encrypted BIB targets only blocks a BCB also covers, so on a bundle carrying an encrypted BIB, a block no BCB covers — typically the per-hop PreviousNode, HopCount and BundleAge blocks, and always the primary block — now reads `None` where it previously read `Maybe`. Editing, signing or encrypting such a block (`Editor`, `ExtensionEditor`, `Signer`, `Encryptor`) no longer refuses with `MaybeHasBib`, so a keyless relay performs its RFC 9171 per-hop rewrites on a signed-then-encrypted bundle instead of refusing them. Residual: a security acceptor that decrypts a target but not the BIB over it (`bpsec::edit::remove_encryption` does, for one target), or a non-conformant sender, leaves an encrypted BIB over a block no BCB covers; an edit to that block breaks the hidden signature, which fails closed at the downstream key-holder. @@ -24,6 +31,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), - CBOR tags at grammar positions that permit none — every scalar and structured field, and the block-array head — are now rejected from the first byte of the tag run, without reading it (decoded via the new `hardy-cbor` `Untagged` wrapper; its cbor-level `UnexpectedTag` is translated to each error domain's `NotCanonical`, so the observable classification is unchanged from previous releases, which read the entire run before rejecting). The one position permitting a tag (`#6.24` on block data) enforces the same fixed-byte bound by hand. This keeps ingress rejection of adversarial tag runs free of per-tag work and per-tag allocation (a scalar-field reject still boxes its one constant field-label error). ### Fixed +- `Builder::build` no longer makes bundles the parser rejects through the `report_on_failure` block flag when the bundle is an administrative record or has a null source: the flag is cleared on every block (RFC 9171 §4.2.3-4/-5), normalised like the fragment flag. Previously `with_hop_count`, which sets the flag, made any such bundle with a hop limit unparseable. +- A named flag bit carried in the `unrecognised` field of `block::Flags`, `bundle::Flags`, or `rfc9173::ScopeFlags` is now the flag it encodes. A hand-built `unrecognised` encodes bit for bit, so a named bit carried there set the flag on the wire while its field read false: it slipped past `ExtensionEditor`'s `report_on_failure` refusal and `Builder::build`'s normalisation into bytes the parser rejects, and in a BPSec scope it left the primary block out of the IPPT/AAD the security source computed while the emitted parameter told the receiver to include it. Every method that takes flags now canonicalizes them (the new `canonicalize()`, above) — `Builder::with_flags`, both `BlockBuilder::with_flags`s, `Editor::with_bundle_flags`, `ExtensionEditor::insert`, and the BIB-HMAC-SHA2 and BCB-AES-GCM operations' scope, with the builder's block template as the backstop — so an alias is refused, normalised, or covered as the flag it encodes, and the `Bundle` a builder returns matches its bytes. Parsed values are already canonical, so parsed round-trips are unchanged. Serde canonicalizes in both directions (see Changed); direct field writes are not canonicalized. - `Editor::push_block`, `Editor::insert_block`, and `Builder::add_extension_block` canonicalize their block-type argument through the new `block::Type::canonicalize()`, which folds an `Unrecognised` alias of a known type code back to its named variant. A hand-built `Type::Unrecognised(v)` encodes as the raw code `v`, so `Unrecognised(11)` or `Unrecognised(12)` previously bypassed `push_block`'s security-block refusal and emitted a block the next node parses as a real BIB/BCB without going through the Signer/Encryptor; `Unrecognised(0)` bypassed both primary-block refusals, and `Unrecognised(1 | 6 | 7 | 10)` bypassed the singleton-duplicate rules (producing bundles the receiving parser rejects). All three caller-supplied-type doors canonicalize, and `BlockTemplate::new` canonicalizes as the constructor invariant, so a stored template never holds an alias and replace-by-type matches are sound. Scope of the refusals is per door: the editor doors refuse the reserved codes and enforce the singleton rules whatever variant carries them; the builder door refuses only wire code 0 — its contract deliberately permits security blocks and duplicates, as the test-crafting surface. Any other policy that matches on named variants can canonicalize first. - `Editor::insert_block`'s replace-by-type path handles BPSec coverage exactly like `update_block`: the replaced block is stripped from its covering BIB/BCB target lists, and the replace refuses (`BibIsEncrypted`, `MaybeHasBib`) where coverage cannot be safely updated. Previously it built a fresh template with no BIB handling, so replacing a BIB-covered block (e.g. the forwarding path's hop-count update on a bundle whose HopCount a peer had signed) emitted a wire bundle whose BIB still targeted the changed body — failing verification downstream — while the in-memory rebuilt `Bundle` reported the block uncovered. - A CBOR tag on the status flag of a status-report assertion was silently accepted — the bare `bool` decode folds tag presence into a canonical flag the caller discarded. It is now rejected (`InvalidField("status")` wrapping `NotCanonical`). diff --git a/bpv7/docs/TODO.md b/bpv7/docs/TODO.md index 8d9f02cd3..34fdf8bbe 100644 --- a/bpv7/docs/TODO.md +++ b/bpv7/docs/TODO.md @@ -394,6 +394,14 @@ Open items from the `refactor/bpv7-parse` deep review (`references/reviews/bpv7- ## bpv7-reader-editor review ledger (2026-09-28) -**Pre-existing broken intra-doc links, and no `cargo doc` gate (review 2.9).** `RUSTDOCFLAGS="-D warnings" cargo doc -p hardy-bpv7 --no-deps --all-features` fails on two links older than the reader-editor work: `Block::payload`'s doc links `parse::parse`, which `block.rs` does not import, and `Builder::build`'s doc links `Bundle`, which `builder.rs` does not import (qualify both, e.g. `crate::parse::parse`). No workflow runs `cargo doc` (`docs.yml` builds only the mkdocs site), so nothing catches a new broken link either. Fix the two links, then add a workspace `cargo doc --no-deps --all-features` step with `RUSTDOCFLAGS="-D warnings"` to the CI checks job. +**No `cargo doc` gate (review 2.9).** No workflow runs `cargo doc` (`docs.yml` builds only the mkdocs site), so nothing catches a broken intra-doc link. `RUSTDOCFLAGS="-D warnings" cargo doc -p hardy-bpv7 --no-deps --all-features` passes; other workspace crates still carry broken links. Fix those, then add a workspace `cargo doc --no-deps --all-features` step with `RUSTDOCFLAGS="-D warnings"` to the CI checks job. **`remove_encryption` leaves an encrypted BIB over a decrypted target (re-review R-3).** With BCB-AES-GCM the Encryptor emits one BCB per target, so a signed-then-encrypted block X carries one BCB over X and another over the BIB that covers X. `bpsec::edit::remove_encryption(X)` decrypts under X's BCB only: the same-BCB BIB handling it documents is unreachable with a context that cannot share a BCB. The result is conformant and verifiable by a key-holder, but X is no longer BCB-covered, so a keyless parse reads its coverage as `None` rather than `Maybe`, and an editor (a relay's per-hop rewrite, or a Stage 2 Rewriter through `ExtensionEditor`) modifies it; the key-holder's check of the hidden result then fails closed. Fix direction: when the decrypted target's BIB sits under a different BCB, `remove_encryption` also decrypts and strips that BCB if it holds the key, restoring a plaintext BIB (which a relay's per-hop rewrite then strips by policy), and refuses otherwise; the `bundle remove-encryption` CLI inherits the change. Partial acceptors in other implementations can produce the same state, so the parser-side residual remains, as the `BibCoverage::Maybe` doc and the CHANGELOG record. + +## ExtensionEditor accept-set invariant (external #712 review, 2026-09-30) + +`ExtensionEditor` refuses at call time every edit that would make a malformed bundle: what the structural parser rejects — the forbidden `report_on_failure` flag (`PrimaryBlock::forbids_report_on_failure`) and unrecognised CRC types — and Previous Node / Bundle Age / Hop Count data that does not decode as its type (`UndecodableBody`), which the parser never inspects but a receiving BPA decodes with the same decoders. Receiver policy over a well-formed bundle is the caller's decision and outside the invariant: RFC 9171 §4.4.2's Bundle Age requirement on a bundle without a clock (removing that block is allowed; the BPA's egress path is to insert Bundle Age on an unclocked bundle, from the age decoded at ingress), hop limits, and how a receiver treats an unsupported block's processing flags. One residual of the invariant itself: a truncated `source_data` is not detected at insert or `finish()`; `Altered` fires only when an extent is read. The refusal list is kept in step by hand, so a future parser or decode rule can widen the gap again, and a caller that treats a failed `finish()` or an unreadable result as fatal aborts on it. Pin the invariant with a fuzz target beside `random_bundles`: for every parseable bundle and every `ExtensionEditor` operation sequence, `finish()` succeeding implies the flattened result parses and its Previous Node, Bundle Age, and Hop Count blocks decode as their types (`Block::extract`), skipping block 0, whose flags are nominal. + +**Write doors take the parsed-only `CrcType::Unrecognised`.** `Builder::with_crc_type`, both `BlockBuilder::with_crc_type`s (builder and editor), `Editor::with_bundle_crc_type`, and `BundleTemplate.crc_type` take the wire `CrcType`, whose `Unrecognised(code)` only a parse produces meaningfully; the write fails late, at `crc::append_crc_value`, as `builder::Error::InternalError`. `ExtensionEditor::insert`'s call-time refusal is the stopgap. Fix direction: a write-side `crc::Kind` (`None`, `Crc16`, `Crc32`) taken by the write doors, with `From for CrcType` and `TryFrom for Kind`, while `CrcType` stays the read type; the tools' `ArgCrcType` (`bpv7/tools/src/flags.rs`) already spells that set. + +**`Builder::build` does not apply the primary-level half of RFC 9171 §4.2.3-4/-5.** The parser rejects a null-source bundle without `do_not_fragment`, or with the fragment flag or any status-report flag, and an administrative record with any status-report flag (inline in `parse.rs`). `Builder::build` neither normalises nor refuses these, so `Builder::new("dtn:none", ..)` with default flags builds a bundle the parser rejects; no production code builds null-source bundles. Fix direction: a second `PrimaryBlock` predicate for the primary-level rule, consumed by the parser and by `Builder::build`, which normalises like the fragment flag (setting `do_not_fragment`, clearing the report bits) or refuses with a typed `builder::Error`. diff --git a/bpv7/src/block.rs b/bpv7/src/block.rs index 9f302f9f6..fa8f1b82a 100644 --- a/bpv7/src/block.rs +++ b/bpv7/src/block.rs @@ -5,7 +5,12 @@ and the generic `Block` struct that represents all extension blocks. */ use alloc::boxed::Box; -use core::{fmt, ops::Range}; +use core::{ + cmp::Ordering, + fmt, + hash::{Hash, Hasher}, + ops::Range, +}; use hardy_cbor::{ decode::{FromCbor, parse_exact}, @@ -15,47 +20,134 @@ use hardy_cbor::{ use crate::{Error, crc}; /// Represents the processing control flags for a BPv7 block. /// -/// These flags, defined in RFC 9171 Section 4.2.2, control how a node should +/// These flags, defined in RFC 9171 Section 4.2.4, control how a node should /// process the block, especially in cases of failure or fragmentation. -#[derive(Default, Debug, Clone, PartialEq, Eq)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +/// +/// A bit carried in [`unrecognised`](Self::unrecognised) that names a flag +/// is an alias of that flag: it encodes as the flag's bit, so the next +/// parser reads the flag set. Equality and hashing compare what the value +/// encodes, so an alias equals its named flag; code that reads the named +/// fields must [`canonicalize`](Self::canonicalize) first. Parsed values +/// are canonical, serde canonicalizes in both directions, and the methods +/// that take block flags canonicalize them: +/// [`builder::BlockBuilder::with_flags`](crate::builder::BlockBuilder::with_flags), +/// [`editor::BlockBuilder::with_flags`](crate::editor::BlockBuilder::with_flags), +/// and [`ExtensionEditor::insert`](crate::extension_editor::ExtensionEditor::insert). +/// Direct field writes are not canonicalized. +#[derive(Default, Debug, Clone)] +#[cfg_attr( + feature = "serde", + derive(serde::Serialize, serde::Deserialize), + serde(from = "FlagsRepr", into = "FlagsRepr") +)] pub struct Flags { /// If set, the block must be replicated in every fragment of the bundle. - #[cfg_attr( - feature = "serde", - serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not") - )] pub must_replicate: bool, /// If set, a status report should be generated if block processing fails. - #[cfg_attr( - feature = "serde", - serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not") - )] pub report_on_failure: bool, /// If set, the entire bundle should be deleted if block processing fails. - #[cfg_attr( - feature = "serde", - serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not") - )] pub delete_bundle_on_failure: bool, /// If set, this block should be deleted if its processing fails. - #[cfg_attr( - feature = "serde", - serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not") - )] pub delete_block_on_failure: bool, - #[cfg_attr( - feature = "serde", - serde(default, skip_serializing_if = "Option::is_none") - )] - /// A bitmask of any unrecognized flags encountered during parsing. - pub unrecognised: Option, + /// A bitmask of the flag bits this implementation does not name; zero + /// when there are none. + /// + /// Encoding carries every bit set here; a parsed `Flags` never holds a + /// named bit in it. + pub unrecognised: u64, +} + +impl PartialEq for Flags { + fn eq(&self, other: &Self) -> bool { + u64::from(self) == u64::from(other) + } +} + +impl Eq for Flags {} + +impl Hash for Flags { + fn hash(&self, state: &mut H) { + u64::from(self).hash(state); + } +} + +// The serde form: the same fields as `Flags`, so the stored shape is the +// plain field list; `Flags` converts through it, canonicalizing both ways. +#[cfg(feature = "serde")] +#[derive(serde::Serialize, serde::Deserialize)] +struct FlagsRepr { + #[serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not")] + must_replicate: bool, + #[serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not")] + report_on_failure: bool, + #[serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not")] + delete_bundle_on_failure: bool, + #[serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not")] + delete_block_on_failure: bool, + #[serde(default, skip_serializing_if = "is_zero")] + unrecognised: u64, +} + +#[cfg(feature = "serde")] +impl From for Flags { + fn from(repr: FlagsRepr) -> Self { + Self { + must_replicate: repr.must_replicate, + report_on_failure: repr.report_on_failure, + delete_bundle_on_failure: repr.delete_bundle_on_failure, + delete_block_on_failure: repr.delete_block_on_failure, + unrecognised: repr.unrecognised, + } + .canonicalize() + } +} + +#[cfg(feature = "serde")] +impl From for FlagsRepr { + fn from(flags: Flags) -> Self { + let flags = flags.canonicalize(); + Self { + must_replicate: flags.must_replicate, + report_on_failure: flags.report_on_failure, + delete_bundle_on_failure: flags.delete_bundle_on_failure, + delete_block_on_failure: flags.delete_block_on_failure, + unrecognised: flags.unrecognised, + } + } +} + +// serde's `skip_serializing_if` predicate for the `unrecognised` fields. +#[cfg(feature = "serde")] +pub(crate) const fn is_zero(bits: &u64) -> bool { + *bits == 0 +} + +impl Flags { + /// Folds every bit of [`unrecognised`](Self::unrecognised) that names a + /// flag into its named field; genuinely unrecognised bits are kept. + /// + /// A hand-built `unrecognised` encodes bit for bit, so `1 << 1` + /// *is* `report_on_failure` to the next parser. Policy that reads the + /// named fields must canonicalize first, or it misreads the flags the + /// bytes carry. + #[must_use] + pub fn canonicalize(self) -> Self { + Self::from(u64::from(&self)) + } + + /// Whether no bit of [`unrecognised`](Self::unrecognised) names a flag — + /// the form [`canonicalize`](Self::canonicalize) returns. Equality + /// cannot tell an alias from its canonical form; this can. + #[must_use] + pub fn is_canonical(&self) -> bool { + Self::from(self.unrecognised).unrecognised == self.unrecognised + } } impl From<&Flags> for u64 { fn from(value: &Flags) -> Self { - let mut flags = value.unrecognised.unwrap_or(0); + let mut flags = value.unrecognised; if value.must_replicate { flags |= 1 << 0; } @@ -77,26 +169,24 @@ impl From for Flags { let mut flags = Self::default(); let mut unrecognised = value; - if (value & 1) != 0 { + if (value & (1 << 0)) != 0 { flags.must_replicate = true; - unrecognised &= !1; + unrecognised &= !(1 << 0); } - if (value & 2) != 0 { + if (value & (1 << 1)) != 0 { flags.report_on_failure = true; - unrecognised &= !2; + unrecognised &= !(1 << 1); } - if (value & 4) != 0 { + if (value & (1 << 2)) != 0 { flags.delete_bundle_on_failure = true; - unrecognised &= !4; + unrecognised &= !(1 << 2); } - if (value & 16) != 0 { + if (value & (1 << 4)) != 0 { flags.delete_block_on_failure = true; - unrecognised &= !16; + unrecognised &= !(1 << 4); } - if unrecognised != 0 { - flags.unrecognised = Some(unrecognised); - } + flags.unrecognised = unrecognised; flags } } @@ -121,20 +211,37 @@ impl FromCbor for Flags { impl Flags { /// The processing-control flags for a primary block (RFC 9171 §4.2.3): /// must-replicate, report-on-failure, and delete-bundle-on-failure set. + /// + /// Nominal: the primary block has no flags field on the wire, so these + /// describe block 0's entry in a bundle's block map and are never + /// encoded — a bundle that forbids `report_on_failure` on its blocks + /// (see [`PrimaryBlock::forbids_report_on_failure`](crate::primary_block::PrimaryBlock::forbids_report_on_failure)) + /// still shows it here. pub fn primary() -> Self { Self { must_replicate: true, report_on_failure: true, delete_bundle_on_failure: true, delete_block_on_failure: false, - unrecognised: None, + unrecognised: 0, } } } /// The type of a BPv7 block, as defined in RFC 9171 Section 4.2.1. -#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +/// +/// An [`Unrecognised`](Type::Unrecognised) code that names a known type is +/// an alias of that type: it encodes as the type's code. Equality, ordering +/// and hashing compare the code, so an alias equals its named variant; +/// pattern matching does not, so code that matches on named variants must +/// [`canonicalize`](Type::canonicalize) first. Serde canonicalizes in both +/// directions. +#[derive(Debug, Copy, Clone)] +#[cfg_attr( + feature = "serde", + derive(serde::Serialize, serde::Deserialize), + serde(from = "TypeRepr", into = "TypeRepr") +)] pub enum Type { /// Primary Block (type code 0). Primary, @@ -162,11 +269,96 @@ impl Type { /// A hand-built `Unrecognised(v)` encodes as the raw code `v` on the /// wire, so `Unrecognised(11)` *is* a Block Integrity Block to the next /// parser. Policy that matches on named variants must canonicalize - /// first, or the alias walks straight past it. + /// first, or it misreads the type the bytes carry. #[must_use] pub fn canonicalize(self) -> Self { Self::from(u64::from(self)) } + + /// Whether this is not an [`Unrecognised`](Type::Unrecognised) alias of + /// a known code — the form [`canonicalize`](Type::canonicalize) returns. + /// Equality cannot tell an alias from its named variant; this can. + #[must_use] + pub fn is_canonical(&self) -> bool { + match self { + Self::Unrecognised(code) => matches!(Self::from(*code), Self::Unrecognised(_)), + _ => true, + } + } +} + +impl PartialEq for Type { + fn eq(&self, other: &Self) -> bool { + u64::from(*self) == u64::from(*other) + } +} + +impl Eq for Type {} + +impl PartialOrd for Type { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for Type { + fn cmp(&self, other: &Self) -> Ordering { + u64::from(*self).cmp(&u64::from(*other)) + } +} + +impl Hash for Type { + fn hash(&self, state: &mut H) { + u64::from(*self).hash(state); + } +} + +// The serde form: the same variants as `Type`, so the stored shape is +// unchanged; `Type` converts through it, canonicalizing both ways. +#[cfg(feature = "serde")] +#[derive(serde::Serialize, serde::Deserialize)] +enum TypeRepr { + Primary, + Payload, + PreviousNode, + BundleAge, + HopCount, + BlockIntegrity, + BlockSecurity, + Unrecognised(u64), +} + +#[cfg(feature = "serde")] +impl From for Type { + fn from(repr: TypeRepr) -> Self { + match repr { + TypeRepr::Primary => Self::Primary, + TypeRepr::Payload => Self::Payload, + TypeRepr::PreviousNode => Self::PreviousNode, + TypeRepr::BundleAge => Self::BundleAge, + TypeRepr::HopCount => Self::HopCount, + TypeRepr::BlockIntegrity => Self::BlockIntegrity, + TypeRepr::BlockSecurity => Self::BlockSecurity, + TypeRepr::Unrecognised(code) => Self::Unrecognised(code), + } + .canonicalize() + } +} + +#[cfg(feature = "serde")] +impl From for TypeRepr { + fn from(block_type: Type) -> Self { + match block_type.canonicalize() { + Type::Primary => Self::Primary, + Type::Payload => Self::Payload, + Type::PreviousNode => Self::PreviousNode, + Type::BundleAge => Self::BundleAge, + Type::HopCount => Self::HopCount, + Type::BlockIntegrity => Self::BlockIntegrity, + Type::BlockSecurity => Self::BlockSecurity, + Type::Unrecognised(code) => Self::Unrecognised(code), + } + } } impl From for u64 { @@ -355,7 +547,7 @@ impl Block { /// /// `source` MUST be the complete, contiguous bundle byte stream the /// block's offsets were parsed against (the `Bytes` returned by - /// [`parse::parse`], or the + /// [`parse::parse`](crate::parse::parse), or the /// buffer a `Builder`/`Editor` produced) — the offsets are /// bundle-absolute. Returns `None` if they fall outside `source`. /// diff --git a/bpv7/src/bpsec/rfc9173/bcb_aes_gcm.rs b/bpv7/src/bpsec/rfc9173/bcb_aes_gcm.rs index 78c07d077..af6bbbe18 100644 --- a/bpv7/src/bpsec/rfc9173/bcb_aes_gcm.rs +++ b/bpv7/src/bpsec/rfc9173/bcb_aes_gcm.rs @@ -163,6 +163,9 @@ impl ToCbor for Results { fn build_data(flags: &ScopeFlags, args: &bcb::OperationArgs) -> Result, Error> { let mut encoder = Encoder::new(); + // RFC 9173 §4.7.2 step 1: the AAD starts with the scope flags with + // reserved and unassigned bits set to 0, so only the named flags are + // carried over. encoder.emit(&ScopeFlags { include_primary_block: flags.include_primary_block, include_target_header: flags.include_target_header, @@ -237,6 +240,9 @@ impl Operation { scope_flags: ScopeFlags, args: bcb::OperationArgs, ) -> Result<(Self, Box<[u8]>), Error> { + // The emitted parameter and the IPPT/AAD below must agree on what an + // alias bit in `unrecognised` covers. + let scope_flags = scope_flags.canonicalize(); let payload = args .blocks .block(args.target) diff --git a/bpv7/src/bpsec/rfc9173/bib_hmac_sha2.rs b/bpv7/src/bpsec/rfc9173/bib_hmac_sha2.rs index 2940692cc..2c57e61e4 100644 --- a/bpv7/src/bpsec/rfc9173/bib_hmac_sha2.rs +++ b/bpv7/src/bpsec/rfc9173/bib_hmac_sha2.rs @@ -150,7 +150,9 @@ where let mut mac = hmac::Hmac::::new_from_slice(key).map_err(|e| Error::Algorithm(e.to_string()))?; - // Build IPT + // Build IPT. RFC 9173 §3.7 step 1: the IPPT starts with the scope flags + // with reserved and unassigned bits set to 0, so only the named flags + // are carried over. mac.update( &emit(&ScopeFlags { include_primary_block: flags.include_primary_block, @@ -259,6 +261,9 @@ impl Operation { scope_flags: ScopeFlags, args: bib::OperationArgs, ) -> Result { + // The emitted parameter and the IPPT/AAD below must agree on what an + // alias bit in `unrecognised` covers. + let scope_flags = scope_flags.canonicalize(); if let Some(ops) = &jwk.operations && !ops.contains(&key::Operation::Sign) { diff --git a/bpv7/src/bpsec/rfc9173/mod.rs b/bpv7/src/bpsec/rfc9173/mod.rs index 4ac250da4..bc5b63b20 100644 --- a/bpv7/src/bpsec/rfc9173/mod.rs +++ b/bpv7/src/bpsec/rfc9173/mod.rs @@ -1,4 +1,5 @@ use alloc::{borrow::Cow, boxed::Box, string::ToString, vec}; +use core::hash::{Hash, Hasher}; use hardy_cbor::{ decode::{FromCbor, parse_exact}, @@ -50,7 +51,15 @@ fn rand_array() -> Result<[u8; N], Error> { // inline-tests-vs-tests/ split convention). /// Scope flags controlling which bundle fields are included in the IPPT (RFC 9173 Section 3.3/4.3). -#[derive(Debug, Hash, Clone, PartialEq, Eq)] +/// +/// A bit carried in [`unrecognised`](Self::unrecognised) that names a flag +/// is an alias of that flag: it encodes as the flag's bit, so the verifier +/// includes what the bit names. Equality and hashing compare what the value +/// encodes, so an alias equals its named flag. Signing and encryption +/// [`canonicalize`](Self::canonicalize) the scope before computing the +/// IPPT or AAD, so both sides agree on what it covers. Direct field writes +/// are not canonicalized. +#[derive(Debug, Clone)] pub struct ScopeFlags { /// Include the primary block in the Integrity-Protected Plaintext (bit 0). pub include_primary_block: bool, @@ -58,17 +67,54 @@ pub struct ScopeFlags { pub include_target_header: bool, /// Include the security block header in the IPPT (bit 2). pub include_security_header: bool, - /// Any unrecognized scope flag bits, preserved for forward compatibility. - pub unrecognised: Option, + /// Any unrecognised scope flag bits, preserved for forward + /// compatibility; zero when there are none. + pub unrecognised: u64, } impl ScopeFlags { + /// The empty scope: every flag clear, no unrecognised bits. The + /// [`Default`] scope is RFC 9173's, all three flags set. pub const NONE: Self = Self { include_primary_block: false, include_target_header: false, include_security_header: false, - unrecognised: None, + unrecognised: 0, }; + + /// Folds every bit of [`unrecognised`](Self::unrecognised) that names a + /// flag into its named field; genuinely unrecognised bits are kept. + /// + /// A hand-built `unrecognised` encodes bit for bit, so `1 << 0` + /// *is* `include_primary_block` to the verifier. Code that reads the + /// named fields must canonicalize first, or it misreads the scope the + /// bytes carry. + #[must_use] + pub fn canonicalize(self) -> Self { + Self::from(u64::from(&self)) + } + + /// Whether no bit of [`unrecognised`](Self::unrecognised) names a flag — + /// the form [`canonicalize`](Self::canonicalize) returns. Equality + /// cannot tell an alias from its canonical form; this can. + #[must_use] + pub fn is_canonical(&self) -> bool { + Self::from(self.unrecognised).unrecognised == self.unrecognised + } +} + +impl PartialEq for ScopeFlags { + fn eq(&self, other: &Self) -> bool { + u64::from(self) == u64::from(other) + } +} + +impl Eq for ScopeFlags {} + +impl Hash for ScopeFlags { + fn hash(&self, state: &mut H) { + u64::from(self).hash(state); + } } impl Default for ScopeFlags { @@ -77,22 +123,14 @@ impl Default for ScopeFlags { include_primary_block: true, include_target_header: true, include_security_header: true, - unrecognised: None, + unrecognised: 0, } } } -impl FromCbor for ScopeFlags { - type Error = Error; - - fn from_cbor(data: &[u8]) -> Result<(Self, bool, usize), Self::Error> { - let (value, len) = crate::error::parse_canonical::(data, Error::NotCanonical)?; - let mut flags = Self { - include_primary_block: false, - include_target_header: false, - include_security_header: false, - unrecognised: None, - }; +impl From for ScopeFlags { + fn from(value: u64) -> Self { + let mut flags = Self::NONE; let mut unrecognised = value; if (value & (1 << 0)) != 0 { @@ -108,27 +146,40 @@ impl FromCbor for ScopeFlags { unrecognised &= !(1 << 2); } - if unrecognised != 0 { - flags.unrecognised = Some(unrecognised); - } - Ok((flags, true, len)) + flags.unrecognised = unrecognised; + flags } } -impl ToCbor for ScopeFlags { - type Result = (); - - fn to_cbor(&self, encoder: &mut Encoder) -> Self::Result { - let mut flags = self.unrecognised.unwrap_or(0); - if self.include_primary_block { +impl From<&ScopeFlags> for u64 { + fn from(value: &ScopeFlags) -> Self { + let mut flags = value.unrecognised; + if value.include_primary_block { flags |= 1 << 0; } - if self.include_target_header { + if value.include_target_header { flags |= 1 << 1; } - if self.include_security_header { + if value.include_security_header { flags |= 1 << 2; } - encoder.emit(&flags) + flags + } +} + +impl FromCbor for ScopeFlags { + type Error = Error; + + fn from_cbor(data: &[u8]) -> Result<(Self, bool, usize), Self::Error> { + let (value, len) = crate::error::parse_canonical::(data, Error::NotCanonical)?; + Ok((Self::from(value), true, len)) + } +} + +impl ToCbor for ScopeFlags { + type Result = (); + + fn to_cbor(&self, encoder: &mut Encoder) -> Self::Result { + encoder.emit(&u64::from(self)); } } diff --git a/bpv7/src/builder.rs b/bpv7/src/builder.rs index 5e1dc4373..6043439ea 100644 --- a/bpv7/src/builder.rs +++ b/bpv7/src/builder.rs @@ -64,9 +64,10 @@ impl<'a> Builder<'a> { } } - /// Sets the [`bundle::Flags`] for this [`Builder`]. + /// Sets the [`bundle::Flags`] for this [`Builder`], canonicalized so an + /// alias bit in `unrecognised` counts as the flag it encodes. pub fn with_flags(mut self, flags: bundle::Flags) -> Self { - self.bundle_flags = flags; + self.bundle_flags = flags.canonicalize(); // The fragment flag is owned by the fragmentation logic, not the // caller: flag it in debug builds to catch API misuse, but always @@ -118,7 +119,9 @@ impl<'a> Builder<'a> { .build(data) } - /// Adds the HopCount block to this [`Builder`]. + /// Adds the HopCount block to this [`Builder`], requesting + /// `report_on_failure`; [`build`](Self::build) clears that flag on a + /// bundle that forbids it. pub fn with_hop_count(self, hop_info: &hop_info::HopInfo) -> Self { self.add_extension_block(block::Type::HopCount) .expect("Failed to add HopCount block") @@ -131,10 +134,14 @@ impl<'a> Builder<'a> { } /// Builds the bundle with the given timestamp, returning the parsed - /// [`Bundle`] view (primary block + blocks map) alongside + /// [`Bundle`](bundle::Bundle) view (primary block + blocks map) alongside /// the encoded wire bytes. + /// + /// An administrative record or null-source bundle is built with + /// `report_on_failure` clear on every block (RFC 9171 §4.2.3-4/-5), + /// whatever the block templates asked for. pub fn build( - self, + mut self, timestamp: creation_timestamp::CreationTimestamp, ) -> Result<(bundle::Bundle, Box<[u8]>), Error> { let primary = primary_block::PrimaryBlock { @@ -150,6 +157,14 @@ impl<'a> Builder<'a> { lifetime: self.lifetime, }; + // Normalised rather than refused, like the fragment flag: the parser + // rejects the combination, so the builder never emits it. + if primary.forbids_report_on_failure() { + for template in self.extensions.iter_mut().chain([&mut self.payload]) { + template.block.flags.report_on_failure = false; + } + } + let mut blocks = HashMap::new(); let data = hardy_cbor::encode::try_emit_array(None, |a| { // Emit primary block — capture the actual extent in the @@ -201,9 +216,12 @@ impl<'a> BlockBuilder<'a> { } } - /// Sets the [`block::Flags`] for this [`BlockBuilder`]. + /// Sets the [`block::Flags`] for this [`BlockBuilder`], canonicalized so + /// an alias bit in `unrecognised` counts as the flag it encodes. + /// [`Builder::build`] clears `report_on_failure` on a bundle that + /// forbids it. pub fn with_flags(mut self, flags: block::Flags) -> Self { - self.template.block.flags = flags; + self.template.block.flags = flags.canonicalize(); self } @@ -245,11 +263,12 @@ impl<'a> BlockTemplate<'a> { block: block::Block { // Canonicalized here as the invariant every door funnels // through: a stored template never holds an `Unrecognised` - // alias of a known code, so type-keyed policy and - // replace-by-type matches over templates are sound even if - // a future door forgets its own canonicalize. + // alias of a known code or flag, so type-keyed policy, + // replace-by-type matches and flag-reading normalisation over + // templates are sound even if a future door forgets its own + // canonicalize. block_type: block_type.canonicalize(), - flags, + flags: flags.canonicalize(), crc_type, ..Default::default() }, @@ -399,3 +418,31 @@ fn test_template() { .build(creation_timestamp::CreationTimestamp::now()) .unwrap(); } + +// The constructor is the backstop every door funnels through: a template +// never holds an alias of a known code or flag, even from a door that forgot +// its own canonicalize. Through the public API every door canonicalizes +// first, so only a direct construction reaches it with an alias. +#[test] +fn block_template_canonicalizes_its_type_and_flags() { + let template = BlockTemplate::new( + block::Type::Unrecognised(10), + block::Flags { + unrecognised: (1 << 1) | (1 << 8), + ..Default::default() + }, + crc::CrcType::None, + None, + ); + // Matched structurally: equality counts an alias as its canonical form. + assert!(matches!(template.block.block_type, block::Type::HopCount)); + assert!(template.block.flags.is_canonical()); + assert_eq!( + template.block.flags, + block::Flags { + report_on_failure: true, + unrecognised: 1 << 8, + ..Default::default() + } + ); +} diff --git a/bpv7/src/bundle.rs b/bpv7/src/bundle.rs index 0384877ce..74580b506 100644 --- a/bpv7/src/bundle.rs +++ b/bpv7/src/bundle.rs @@ -6,22 +6,30 @@ This module defines the core bundle data model: the [`Bundle`] structure [`crate::checks`] and [`crate::rewrite`]. */ -use super::*; -use crate::primary_block::PrimaryBlock; use alloc::collections::{BTreeMap, BTreeSet}; +use core::hash::{Hash, Hasher}; + use base64::prelude::*; -use bpsec::{bcb, bib}; use hardy_cbor::decode::{self, FromCbor}; +use super::*; +#[cfg(feature = "serde")] +use crate::block::is_zero; +use crate::{ + bpsec::{bcb, bib}, + primary_block::PrimaryBlock, +}; + /// A parsed BPv7 bundle: the primary block plus the extension and payload /// blocks keyed by block number. This is the crate's structural bundle /// representation, produced by [`parse`](crate::parse::parse) and emitted /// by [`Builder`](crate::builder::Builder) / [`Editor`](crate::editor::Editor). /// -/// The derived `==` is structural and offset-sensitive — block extents are -/// buffer-relative, so re-encodings of the same bundle compare unequal. For -/// data-aware, offset-insensitive equivalence use -/// [`semantic_eq`](Self::semantic_eq). +/// The derived `==` is offset-sensitive — block extents are +/// buffer-relative, so re-encodings of the same bundle compare unequal — +/// and compares flags and block types by what they encode, so an alias +/// equals its canonical form. For data-aware, offset-insensitive +/// equivalence use [`semantic_eq`](Self::semantic_eq). #[derive(Debug, Clone, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct Bundle { @@ -496,78 +504,158 @@ impl core::fmt::Display for Id { /// These flags, defined in RFC 9171 Section 4.2.3, control how a node should /// handle the bundle, such as whether it can be fragmented or if status reports /// are requested. -#[derive(Default, Debug, Clone, PartialEq, Eq)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +/// +/// A bit carried in [`unrecognised`](Self::unrecognised) that names a flag +/// is an alias of that flag: it encodes as the flag's bit, so the next +/// parser reads the flag set. Equality and hashing compare what the value +/// encodes, so an alias equals its named flag; code that reads the named +/// fields must [`canonicalize`](Self::canonicalize) first. Parsed +/// values are canonical, serde canonicalizes in both directions, and the +/// methods that take bundle flags canonicalize them: +/// [`Builder::with_flags`](crate::builder::Builder::with_flags) and +/// [`Editor::with_bundle_flags`](crate::editor::Editor::with_bundle_flags). +/// Direct field writes are not canonicalized. +#[derive(Default, Debug, Clone)] +#[cfg_attr( + feature = "serde", + derive(serde::Serialize, serde::Deserialize), + serde(from = "FlagsRepr", into = "FlagsRepr") +)] pub struct Flags { /// If set, this bundle is a fragment of a larger bundle. - #[cfg_attr( - feature = "serde", - serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not") - )] pub is_fragment: bool, /// If set, the payload is an administrative record. - #[cfg_attr( - feature = "serde", - serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not") - )] pub is_admin_record: bool, /// If set, the bundle must not be fragmented. - #[cfg_attr( - feature = "serde", - serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not") - )] pub do_not_fragment: bool, /// If set, the destination application is requested to send an acknowledgement. - #[cfg_attr( - feature = "serde", - serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not") - )] pub app_ack_requested: bool, /// If set, status reports should include the time of the reported event. - #[cfg_attr( - feature = "serde", - serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not") - )] pub report_status_time: bool, /// If set, a status report should be generated upon bundle reception. - #[cfg_attr( - feature = "serde", - serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not") - )] pub receipt_report_requested: bool, /// If set, a status report should be generated upon bundle forwarding. - #[cfg_attr( - feature = "serde", - serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not") - )] pub forward_report_requested: bool, /// If set, a status report should be generated upon bundle delivery. - #[cfg_attr( - feature = "serde", - serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not") - )] pub delivery_report_requested: bool, /// If set, a status report should be generated upon bundle deletion. - #[cfg_attr( - feature = "serde", - serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not") - )] pub delete_report_requested: bool, - /// A bitmask of any unrecognized flags encountered during parsing. - #[cfg_attr( - feature = "serde", - serde(default, skip_serializing_if = "Option::is_none") - )] - pub unrecognised: Option, + /// A bitmask of the flag bits this implementation does not name; zero + /// when there are none. + /// + /// Encoding carries every bit set here; a parsed `Flags` never holds a + /// named bit in it. + pub unrecognised: u64, +} + +impl PartialEq for Flags { + fn eq(&self, other: &Self) -> bool { + u64::from(self) == u64::from(other) + } +} + +impl Eq for Flags {} + +impl Hash for Flags { + fn hash(&self, state: &mut H) { + u64::from(self).hash(state); + } +} + +// The serde form: the same fields as `Flags`, so the stored shape is the +// plain field list; `Flags` converts through it, canonicalizing both ways. +#[cfg(feature = "serde")] +#[derive(serde::Serialize, serde::Deserialize)] +struct FlagsRepr { + #[serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not")] + is_fragment: bool, + #[serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not")] + is_admin_record: bool, + #[serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not")] + do_not_fragment: bool, + #[serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not")] + app_ack_requested: bool, + #[serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not")] + report_status_time: bool, + #[serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not")] + receipt_report_requested: bool, + #[serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not")] + forward_report_requested: bool, + #[serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not")] + delivery_report_requested: bool, + #[serde(default, skip_serializing_if = "<&bool as core::ops::Not>::not")] + delete_report_requested: bool, + #[serde(default, skip_serializing_if = "is_zero")] + unrecognised: u64, +} + +#[cfg(feature = "serde")] +impl From for Flags { + fn from(repr: FlagsRepr) -> Self { + Self { + is_fragment: repr.is_fragment, + is_admin_record: repr.is_admin_record, + do_not_fragment: repr.do_not_fragment, + app_ack_requested: repr.app_ack_requested, + report_status_time: repr.report_status_time, + receipt_report_requested: repr.receipt_report_requested, + forward_report_requested: repr.forward_report_requested, + delivery_report_requested: repr.delivery_report_requested, + delete_report_requested: repr.delete_report_requested, + unrecognised: repr.unrecognised, + } + .canonicalize() + } +} + +#[cfg(feature = "serde")] +impl From for FlagsRepr { + fn from(flags: Flags) -> Self { + let flags = flags.canonicalize(); + Self { + is_fragment: flags.is_fragment, + is_admin_record: flags.is_admin_record, + do_not_fragment: flags.do_not_fragment, + app_ack_requested: flags.app_ack_requested, + report_status_time: flags.report_status_time, + receipt_report_requested: flags.receipt_report_requested, + forward_report_requested: flags.forward_report_requested, + delivery_report_requested: flags.delivery_report_requested, + delete_report_requested: flags.delete_report_requested, + unrecognised: flags.unrecognised, + } + } +} + +impl Flags { + /// Folds every bit of [`unrecognised`](Self::unrecognised) that names a + /// flag into its named field; genuinely unrecognised bits are kept. + /// + /// A hand-built `unrecognised` encodes bit for bit, so `1 << 1` + /// *is* `is_admin_record` to the next parser. Policy that reads the + /// named fields must canonicalize first, or it misreads the flags the + /// bytes carry. + #[must_use] + pub fn canonicalize(self) -> Self { + Self::from(u64::from(&self)) + } + + /// Whether no bit of [`unrecognised`](Self::unrecognised) names a flag — + /// the form [`canonicalize`](Self::canonicalize) returns. Equality + /// cannot tell an alias from its canonical form; this can. + #[must_use] + pub fn is_canonical(&self) -> bool { + Self::from(self.unrecognised).unrecognised == self.unrecognised + } } impl From for Flags { @@ -612,16 +700,14 @@ impl From for Flags { unrecognised &= !(1 << 18); } - if unrecognised != 0 { - flags.unrecognised = Some(unrecognised); - } + flags.unrecognised = unrecognised; flags } } impl From<&Flags> for u64 { fn from(value: &Flags) -> Self { - let mut flags = value.unrecognised.unwrap_or(0); + let mut flags = value.unrecognised; if value.is_fragment { flags |= 1 << 0; } diff --git a/bpv7/src/editor.rs b/bpv7/src/editor.rs index b5288878b..96fac5d52 100644 --- a/bpv7/src/editor.rs +++ b/bpv7/src/editor.rs @@ -38,6 +38,12 @@ pub enum Error { #[error("Security blocks (BIB/BCB) must be managed via Signer/Encryptor, not the Editor")] SecurityBlock, + /// A block carries `report_on_failure` on a bundle whose final primary + /// forbids it (RFC 9171 §4.2.3-4/-5); refused when the bundle is rebuilt, + /// naming the lowest such block. + #[error("Block {0} requests report_on_failure, which the bundle forbids")] + ReportOnFailureForbidden(u64), + #[error(transparent)] Builder(#[from] builder::Error), } @@ -386,14 +392,15 @@ impl<'a> Editor<'a> { Ok(self.primary.as_mut().unwrap()) } - /// Sets the bundle flags for this [`Editor`]. + /// Sets the bundle flags for this [`Editor`], canonicalized so an alias + /// bit in `unrecognised` counts as the flag it encodes. /// /// On error, returns the editor along with the error so it can be reused for recovery. #[allow(clippy::result_large_err)] pub fn with_bundle_flags(mut self, flags: bundle::Flags) -> Result { match self.primary_block() { Ok(pb) => { - pb.flags = flags; + pb.flags = flags.canonicalize(); Ok(self) } Err(e) => Err((self, e)), @@ -1034,7 +1041,15 @@ impl<'a> Editor<'a> { /// - The public API prevents adding/updating security blocks /// - Cascade deletes preserve or remove security block references /// - Signer/Encryptor set bib/bcb overrides explicitly + /// + /// # Errors + /// + /// [`ReportOnFailureForbidden`](Error::ReportOnFailureForbidden) when the + /// final primary forbids `report_on_failure` (RFC 9171 §4.2.3-4/-5) and + /// a block carries it, whether an edit set the flag or a kept block + /// carried it under the primary the edits replaced. pub fn rebuild_bundle(mut self) -> Result<(bundle::Bundle, Vec), Error> { + self.check_report_on_failure()?; let mut blocks_out: HashMap = HashMap::new(); let primary_block = self.blocks.remove(&0).expect("No primary block!"); @@ -1135,7 +1150,13 @@ impl<'a> Editor<'a> { /// `Chunk::Unchanged` references ranges in the original `source_data`, /// `Chunk::New` contains freshly encoded bytes. Use `Chunk::flatten()` /// to concatenate into contiguous bytes. + /// + /// # Errors + /// + /// [`ReportOnFailureForbidden`](Error::ReportOnFailureForbidden), as + /// [`rebuild_bundle`](Self::rebuild_bundle) refuses it. pub fn rebuild(mut self) -> Result, Error> { + self.check_report_on_failure()?; let primary_block = self.blocks.remove(&0).expect("No primary block!"); // Build primary chunk @@ -1180,6 +1201,40 @@ impl<'a> Editor<'a> { )) } + // RFC 9171 §4.2.3-4/-5 against the final primary: checked when the + // bundle is rebuilt, not at the setters, since the primary can change + // after a block flag is set. Block 0's flags are nominal and skipped. + // The lowest flagged block is named, so the error does not depend on + // the map's iteration order. + fn check_report_on_failure(&self) -> Result<(), Error> { + let primary = self.primary.as_ref().unwrap_or(&self.original.primary); + if !primary.forbids_report_on_failure() { + return Ok(()); + } + let flagged = self + .blocks + .iter() + .filter(|&(&block_number, template)| { + block_number != 0 + && match template { + BlockTemplate::Update(template) | BlockTemplate::Insert(template) => { + template.block.flags.report_on_failure + } + BlockTemplate::Keep(_) => self + .original + .blocks + .get(&block_number) + .is_some_and(|block| block.flags.report_on_failure), + } + }) + .map(|(&block_number, _)| block_number) + .min(); + match flagged { + Some(block_number) => Err(Error::ReportOnFailureForbidden(block_number)), + None => Ok(()), + } + } + fn build_chunk( &self, block_number: u64, @@ -1232,9 +1287,10 @@ impl<'a> BlockBuilder<'a> { } } - /// Set the `Flags` for this block. + /// Set the `Flags` for this block, canonicalized so an alias bit in + /// `unrecognised` counts as the flag it encodes. pub fn with_flags(mut self, flags: block::Flags) -> Self { - self.template.block.flags = flags; + self.template.block.flags = flags.canonicalize(); self } diff --git a/bpv7/src/extension_editor.rs b/bpv7/src/extension_editor.rs index 7cd5a91f8..0764e88c8 100644 --- a/bpv7/src/extension_editor.rs +++ b/bpv7/src/extension_editor.rs @@ -16,19 +16,38 @@ //! `replace`/`remove`: a fresh insert is by definition an uncovered //! extension block, so every gate above still holds. //! +//! An edit that would make a malformed bundle is refused at call time +//! instead of failing at [`finish`](ExtensionEditor::finish) or at the +//! receiver's decoders. What the structural parser rejects — a +//! `report_on_failure` flag the bundle forbids, an unrecognised CRC type — +//! is refused with the parser's own error. Previous Node, Bundle Age, or +//! Hop Count data that does not decode as its type is refused as +//! [`UndecodableBody`](Error::UndecodableBody): the parser never decodes +//! those bodies, but a receiving BPA does, with the same decoders. +//! +//! Receiver policy over a well-formed bundle is the caller's to decide, not +//! refused here: RFC 9171 §4.4.2's Bundle Age requirement on a bundle +//! without a clock, hop limits, and how a receiver treats blocks it does +//! not support. +//! //! Edits accumulate in memory; nothing is materialised until //! [`finish`](ExtensionEditor::finish). use alloc::{borrow::Cow, boxed::Box, vec::Vec}; +use hardy_cbor::decode::parse_exact; use thiserror::Error; +// Aliased `Error`s: this module's own `Error` is the refusal enum below. use crate::{ + Error as Bpv7Error, block::{BibCoverage, Flags, Type}, bundle::Bundle, - crc::CrcType, - // Aliased: this module's own `Error` is the refusal enum below. + bundle_age::BundleAge, + crc::{CrcType, Error as CrcError}, editor::{Chunk, Editor, Error as EditorError}, + eid::Eid, + hop_info::HopInfo, }; /// Errors from scoped editing — each refused operation names its reason, @@ -54,6 +73,25 @@ pub enum Error { #[error("Block {0} is under BPSec coverage and cannot be edited")] Covered(u64), + /// The edit would produce a bundle the structural parser rejects, + /// refused at call time with the parser's own error: `InvalidFlags` for + /// a `report_on_failure` flag the bundle forbids (RFC 9171 + /// §4.2.3-4/-5) and `InvalidCrc` for an unrecognised CRC type. + #[error(transparent)] + Invalid(#[from] Bpv7Error), + + /// Previous Node, Bundle Age, or Hop Count data that does not decode as + /// its type. The structural parser never decodes these bodies; a + /// receiving BPA does, with the same decoders. + #[error("{block_type:?} block data does not decode")] + UndecodableBody { + /// The well-known type the data was checked as. + block_type: Type, + /// The type's decode error, boxed to keep every `Result` small. + #[source] + source: Box, + }, + /// A structural editing failure reported by the underlying editor /// (illegal duplicate of a singleton type, block numbers exhausted, …). #[error(transparent)] @@ -72,6 +110,9 @@ pub struct ExtensionEditor<'a> { // operation. `None` only transiently inside an operation. editor: Option>, edited: bool, + // The primary block is out of scope, so its verdict is fixed at + // construction. + report_on_failure_forbidden: bool, } impl<'a> ExtensionEditor<'a> { @@ -80,6 +121,7 @@ impl<'a> ExtensionEditor<'a> { Self { editor: Some(Editor::new(original, source_data)), edited: false, + report_on_failure_forbidden: original.primary.forbids_report_on_failure(), } } @@ -88,6 +130,19 @@ impl<'a> ExtensionEditor<'a> { /// Inserting a second instance of a singleton type (Previous Node, /// Bundle Age, Hop Count) is refused by the underlying editor; multiple /// instances of an [`Unrecognised`](Type::Unrecognised) type are legal. + /// A `report_on_failure` flag the bundle forbids, an unrecognised CRC + /// type, and a well-known type's undecodable data are refused. + /// + /// # Errors + /// + /// - [`ReservedType`](Error::ReservedType) for the primary, payload, + /// BIB, and BCB types. + /// - [`Invalid`](Error::Invalid) for a `report_on_failure` flag the + /// bundle forbids or an unrecognised CRC type. + /// - [`UndecodableBody`](Error::UndecodableBody) for well-known data + /// that does not decode as its type. + /// - [`Editor`](Error::Editor) for a second instance of a singleton type + /// or exhausted block numbers. pub fn insert( &mut self, block_type: Type, @@ -96,14 +151,23 @@ impl<'a> ExtensionEditor<'a> { data: Box<[u8]>, ) -> Result { // Canonicalized so an `Unrecognised` alias of a reserved code is - // refused here as the reserved type it encodes, not further down. + // refused here as the reserved type it encodes, not further down, and + // an alias of the forbidden flag as the flag it encodes. let block_type = block_type.canonicalize(); + let flags = flags.canonicalize(); if matches!( block_type, Type::Primary | Type::Payload | Type::BlockIntegrity | Type::BlockSecurity ) { return Err(Error::ReservedType(block_type)); } + if flags.report_on_failure && self.report_on_failure_forbidden { + return Err(Bpv7Error::InvalidFlags.into()); + } + if let CrcType::Unrecognised(code) = crc_type { + return Err(Bpv7Error::InvalidCrc(CrcError::InvalidType(code)).into()); + } + check_body(block_type, &data)?; let editor = self .editor @@ -130,9 +194,19 @@ impl<'a> ExtensionEditor<'a> { } /// Replaces an extension block's block-specific data, keeping its flags - /// and CRC type. + /// and CRC type. A well-known type's undecodable data is refused. + /// + /// # Errors + /// + /// - [`ReservedBlock`](Error::ReservedBlock), + /// [`NoSuchBlock`](Error::NoSuchBlock), + /// [`ReservedType`](Error::ReservedType), or + /// [`Covered`](Error::Covered) for a target outside the scope. + /// - [`UndecodableBody`](Error::UndecodableBody) for well-known data + /// that does not decode as its type. pub fn replace(&mut self, block_number: u64, data: Box<[u8]>) -> Result<()> { - self.check_target(block_number)?; + let block_type = self.check_target(block_number)?; + check_body(block_type, &data)?; let editor = self .editor @@ -152,6 +226,13 @@ impl<'a> ExtensionEditor<'a> { } /// Removes an extension block. + /// + /// # Errors + /// + /// [`ReservedBlock`](Error::ReservedBlock), + /// [`NoSuchBlock`](Error::NoSuchBlock), + /// [`ReservedType`](Error::ReservedType), or [`Covered`](Error::Covered) + /// for a target outside the scope. pub fn remove(&mut self, block_number: u64) -> Result<()> { self.check_target(block_number)?; @@ -174,8 +255,8 @@ impl<'a> ExtensionEditor<'a> { // The scoped refusals for an edit target, checked against the editor's // current view — so a block inserted by this editor is a valid target, - // and a block already removed is not. - fn check_target(&self, block_number: u64) -> Result<()> { + // and a block already removed is not. Returns the target's type. + fn check_target(&self, block_number: u64) -> Result { if block_number <= 1 { return Err(Error::ReservedBlock(block_number)); } @@ -194,7 +275,7 @@ impl<'a> ExtensionEditor<'a> { if !matches!(block.bib, BibCoverage::None) || block.bcb.is_some() { return Err(Error::Covered(block_number)); } - Ok(()) + Ok(block.block_type) } /// Whether any operation has been applied since construction. @@ -216,3 +297,20 @@ impl<'a> ExtensionEditor<'a> { .map_err(Error::Editor) } } + +// A well-known extension block's data must decode as its type, exactly as +// the receive path decodes it: the structural parser never looks inside +// these bodies, so a malformed one would otherwise ship. Other types are +// opaque here. +fn check_body(block_type: Type, data: &[u8]) -> Result<()> { + let decoded = match block_type { + Type::PreviousNode => parse_exact::(data).map(drop).map_err(Bpv7Error::from), + Type::BundleAge => parse_exact::(data).map(drop), + Type::HopCount => parse_exact::(data).map(drop), + _ => Ok(()), + }; + decoded.map_err(|source| Error::UndecodableBody { + block_type, + source: Box::new(source), + }) +} diff --git a/bpv7/src/parse.rs b/bpv7/src/parse.rs index aab1ea718..ed16e50dd 100644 --- a/bpv7/src/parse.rs +++ b/bpv7/src/parse.rs @@ -905,9 +905,7 @@ impl BundleParser { // RFC 9171 §4.2.3-4 / §4.2.3-5: an admin-record or null-source // bundle MUST NOT have the `report_on_failure` flag set on any // extension block. - if (bundle.primary.flags.is_admin_record || bundle.primary.id.source.is_null()) - && header.flags.report_on_failure - { + if bundle.primary.forbids_report_on_failure() && header.flags.report_on_failure { return Err(Error::InvalidFlags); } diff --git a/bpv7/src/primary_block.rs b/bpv7/src/primary_block.rs index aa5ac04ad..b2461feba 100644 --- a/bpv7/src/primary_block.rs +++ b/bpv7/src/primary_block.rs @@ -219,4 +219,20 @@ impl PrimaryBlock { ..Default::default() } } + + /// Whether this bundle forbids the `report_on_failure` block flag. + /// + /// RFC 9171 §4.2.3-4/-5 require the flag clear on every block of an + /// administrative-record or null-source bundle, since the receiver has + /// nowhere meaningful to report to. This is the crate's one statement of + /// that block-flag rule: the parser rejects the combination, + /// [`Builder`](crate::builder::Builder) clears the flag, + /// [`ExtensionEditor`](crate::extension_editor::ExtensionEditor) refuses + /// it at call time, and the owner [`Editor`](crate::editor::Editor) + /// refuses it when it rebuilds — there, not at its setters, since the + /// primary can change after a block flag is set. + #[must_use] + pub fn forbids_report_on_failure(&self) -> bool { + self.flags.is_admin_record || self.id.source.is_null() + } } diff --git a/bpv7/tests/builder.rs b/bpv7/tests/builder.rs new file mode 100644 index 000000000..3feaf61c7 --- /dev/null +++ b/bpv7/tests/builder.rs @@ -0,0 +1,161 @@ +//! Integration tests for `hardy_bpv7::builder` — the flags a built bundle +//! carries on the wire, read back through the parser. + +use core::num::NonZeroU8; + +use bytes::Bytes; +use hardy_bpv7::{block, builder, bundle, creation_timestamp, hop_info, parse}; + +// A bundle that forbids `report_on_failure` (RFC 9171 §4.2.3-4/-5) is built +// with the flag clear on every block — the Hop Count block's default and +// caller-set flags on an extension block and on the payload alike — so it +// parses; an ordinary bundle keeps all three. +#[test] +fn builder_clears_report_on_failure_on_a_forbidding_bundle() { + let hop_info = hop_info::HopInfo { + limit: NonZeroU8::new(30).unwrap(), + count: 0, + }; + let reporting = || block::Flags { + report_on_failure: true, + ..Default::default() + }; + for (label, source, flags, reports) in [ + ( + "null source", + "dtn:none", + bundle::Flags { + do_not_fragment: true, + ..Default::default() + }, + false, + ), + ( + "admin record", + "ipn:1.0", + bundle::Flags { + is_admin_record: true, + ..Default::default() + }, + false, + ), + ("ordinary", "ipn:1.0", bundle::Flags::default(), true), + ] { + let (_, data) = builder::Builder::new(source.parse().unwrap(), "ipn:2.0".parse().unwrap()) + .with_flags(flags) + .with_hop_count(&hop_info) + .add_extension_block(block::Type::Unrecognised(200)) + .unwrap() + .with_flags(reporting()) + .build(b"ext-data".as_slice().into()) + .add_extension_block(block::Type::Payload) + .unwrap() + .with_flags(reporting()) + .build(b"Hello".as_slice().into()) + .build(creation_timestamp::CreationTimestamp::now()) + .unwrap(); + let parsed = parse::parse(Bytes::from(data)) + .unwrap_or_else(|e| panic!("{label} bundle must parse: {e:?}")) + .bundle; + for block_type in [ + block::Type::HopCount, + block::Type::Unrecognised(200), + block::Type::Payload, + ] { + let block = parsed + .blocks + .values() + .find(|b| b.block_type == block_type) + .expect("the block is present"); + assert_eq!( + block.flags.report_on_failure, reports, + "{label}: {block_type:?} reports on failure exactly when the bundle allows it" + ); + } + } +} + +// The builder canonicalizes the flags it is handed, at both flag levels: a +// named bit carried in `unrecognised` is the flag it encodes — the +// admin-record alias makes an administrative record, whose normalisation +// then clears the Hop Count's `report_on_failure`, and `report_on_failure`'s +// alias reports on an ordinary bundle's block — while genuinely unrecognised +// bits pass through. The returned `Bundle` holds what the bytes hold. +#[test] +fn builder_canonicalizes_unrecognised_aliases_of_named_flags() { + let (built, data) = + builder::Builder::new("ipn:1.0".parse().unwrap(), "ipn:2.0".parse().unwrap()) + .with_flags(bundle::Flags { + unrecognised: (1 << 1) | (1 << 24), + ..Default::default() + }) + .with_hop_count(&hop_info::HopInfo { + limit: NonZeroU8::new(30).unwrap(), + count: 0, + }) + .with_payload(b"Hello".as_slice().into()) + .build(creation_timestamp::CreationTimestamp::now()) + .unwrap(); + let parsed = parse::parse(Bytes::from(data)) + .expect("the administrative record's blocks request no report") + .bundle; + assert!(parsed.primary.flags.is_admin_record); + assert_eq!(parsed.primary.flags.unrecognised, 1 << 24); + assert!(built.primary.flags.is_canonical()); + assert_eq!(built.primary.flags, parsed.primary.flags); + let hop_count = parsed + .blocks + .values() + .find(|b| b.block_type == block::Type::HopCount) + .expect("the Hop Count block is present"); + assert!( + !hop_count.flags.report_on_failure, + "an administrative record clears the Hop Count default" + ); + + for (label, bundle_flags, reports) in [ + ("ordinary", bundle::Flags::default(), true), + ( + "admin record", + bundle::Flags { + is_admin_record: true, + ..Default::default() + }, + false, + ), + ] { + let (built, data) = + builder::Builder::new("ipn:1.0".parse().unwrap(), "ipn:2.0".parse().unwrap()) + .with_flags(bundle_flags) + .add_extension_block(block::Type::Unrecognised(200)) + .unwrap() + .with_flags(block::Flags { + unrecognised: (1 << 1) | (1 << 8), + ..Default::default() + }) + .build(b"ext-data".as_slice().into()) + .with_payload(b"Hello".as_slice().into()) + .build(creation_timestamp::CreationTimestamp::now()) + .unwrap(); + let parsed = parse::parse(Bytes::from(data)) + .unwrap_or_else(|e| panic!("{label}: the bundle must parse: {e:?}")) + .bundle; + let block = parsed + .blocks + .values() + .find(|b| b.block_type == block::Type::Unrecognised(200)) + .unwrap_or_else(|| panic!("{label}: the extension block is present")); + assert_eq!( + block.flags.report_on_failure, reports, + "{label}: the alias reports exactly when the bundle allows it" + ); + assert_eq!(block.flags.unrecognised, 1 << 8, "{label}"); + let built_block = built + .blocks + .values() + .find(|b| b.block_type == block::Type::Unrecognised(200)) + .unwrap_or_else(|| panic!("{label}: the built view has the block")); + assert!(built_block.flags.is_canonical(), "{label}"); + assert_eq!(built_block.flags, block.flags, "{label}"); + } +} diff --git a/bpv7/tests/editor.rs b/bpv7/tests/editor.rs index 942615a00..2aa953288 100644 --- a/bpv7/tests/editor.rs +++ b/bpv7/tests/editor.rs @@ -9,13 +9,16 @@ use bytes::Bytes; use hardy_bpv7::{ Bundle, block, bpsec::{encryptor, key, rfc9173::ScopeFlags, signer}, - builder, checks, crc, creation_timestamp, + builder, bundle, checks, crc, creation_timestamp, editor::{Chunk, Editor, Error}, eid, extension_editor::{self, ExtensionEditor}, hop_info, parse, }; -use hardy_cbor::encode::emit; +// Aliased: the parser's error, beside the editor's `Error` imported above. +use hardy_bpv7::Error as Bpv7Error; +// Aliased: the CBOR codec's error, beside the two above. +use hardy_cbor::{decode::Error as CborError, encode::emit}; mod common; use self::common::rand_k; @@ -53,7 +56,9 @@ fn ok(result: Result) -> T { // Edit a bundle, rebuild, re-parse, and return the parsed Bundle. fn reparse(data: &[u8]) -> Bundle { - parse::parse(Bytes::copy_from_slice(data)).unwrap().bundle + parse::parse(Bytes::copy_from_slice(data)) + .unwrap_or_else(|e| panic!("the rebuilt bundle must re-parse: {e:?}")) + .bundle } #[test] @@ -245,6 +250,12 @@ fn assert_rebuild_matches_parse(bundle: &Bundle, data: &[u8]) { ), "CRC type mismatch" ); + // Equality compares the encoding, so canonical form is checked apart: + // the rebuilt view holds what the bytes hold, not an alias of it. + assert!( + bundle.primary.flags.is_canonical(), + "primary flags not canonical" + ); assert_eq!(bundle.primary.flags, reparsed.primary.flags); // Same set of block numbers @@ -257,6 +268,10 @@ fn assert_rebuild_matches_parse(bundle: &Bundle, data: &[u8]) { // Block fields match and ranges index validly into the data for (block_number, block) in &bundle.blocks { let reparsed_block = reparsed.blocks.get(block_number).unwrap(); + assert!( + block.block_type.is_canonical() && block.flags.is_canonical(), + "Block {block_number} type or flags not canonical" + ); assert_eq!( block.block_type, reparsed_block.block_type, "Block {block_number} type mismatch" @@ -327,6 +342,46 @@ fn rebuild_bundle_change_destination() { assert_rebuild_matches_parse(&new_bundle, &new_data); } +// The owner editor canonicalizes the flags it is handed, so the `Bundle` +// `rebuild_bundle()` returns holds what the bytes hold: the admin-record +// bit carried in `unrecognised` makes an administrative record while a +// genuinely unrecognised bit passes through, and `report_on_failure`'s bit +// makes an inserted block report. +#[test] +fn rebuild_bundle_canonicalizes_flag_aliases() { + let (bundle, data) = make_bundle(); + let aliased = bundle::Flags { + unrecognised: (1 << 1) | (1 << 24), + ..Default::default() + }; + let (new_bundle, new_data) = ok(Editor::new(&bundle, &data).with_bundle_flags(aliased)) + .rebuild_bundle() + .map(|(b, c)| (b, Chunk::flatten(c, &data))) + .unwrap(); + assert!(new_bundle.primary.flags.is_admin_record); + assert_eq!(new_bundle.primary.flags.unrecognised, 1 << 24); + assert_rebuild_matches_parse(&new_bundle, &new_data); + + let editor = ok(Editor::new(&bundle, &data).insert_block(block::Type::Unrecognised(200))); + let inserted = editor.block_number(); + let (new_bundle, new_data) = editor + .with_flags(block::Flags { + unrecognised: 1 << 1, + ..Default::default() + }) + .with_data((&[0x01, 0x02][..]).into()) + .rebuild() + .rebuild_bundle() + .map(|(b, c)| (b, Chunk::flatten(c, &data))) + .unwrap(); + assert!(new_bundle.blocks[&inserted].flags.report_on_failure); + assert!(new_bundle.blocks[&inserted].flags.is_canonical()); + assert_eq!( + new_bundle.blocks[&inserted].flags, + reparse(&new_data).blocks[&inserted].flags + ); +} + #[test] fn rebuild_bundle_multiple_primary_changes() { let (bundle, data) = make_bundle(); @@ -536,27 +591,133 @@ fn remove_block_rejects_security_block() { ); } +// RFC 9171 §4.2.3-4/-5 against the final primary: the owner editor refuses +// a forbidden `report_on_failure` when it rebuilds, whichever order the +// flag and the forbidding primary were set in, and whether an edit set the +// flag or a kept block carried it under the primary the edits replaced. +#[test] +fn owner_editor_refuses_the_forbidden_flag_at_rebuild() { + fn with_reporting_block(editor: Editor<'_>) -> Editor<'_> { + ok(editor.push_block(block::Type::Unrecognised(200))) + .with_flags(block::Flags { + report_on_failure: true, + ..Default::default() + }) + .with_data(b"ext-data".as_slice().into()) + .rebuild() + } + fn refused(result: Result<(Bundle, Vec), Error>, block_number: u64) { + assert!( + matches!(result, Err(Error::ReportOnFailureForbidden(n)) if n == block_number), + "refused, naming block {block_number}" + ); + } + // The pushed block takes the first free number: 2 on a bundle of a + // primary and a payload. + let pushed = 2; + let admin = bundle::Flags { + is_admin_record: true, + ..Default::default() + }; + + // The flag set on a bundle that already forbids it. + let (bundle, data) = make_bundle_from( + "dtn:none", + bundle::Flags { + do_not_fragment: true, + ..Default::default() + }, + ); + refused( + with_reporting_block(Editor::new(&bundle, &data)).rebuild_bundle(), + pushed, + ); + + // The flag set first, then the primary made to forbid it. + let (bundle, data) = make_bundle(); + let editor = with_reporting_block(Editor::new(&bundle, &data)); + refused( + ok(editor.with_bundle_flags(admin.clone())).rebuild_bundle(), + pushed, + ); + let editor = with_reporting_block(Editor::new(&bundle, &data)); + refused( + ok(editor.with_source(eid::Eid::Null)).rebuild_bundle(), + pushed, + ); + + // A kept block: the Hop Count reports on an ordinary bundle, then the + // bundle becomes an administrative record. Both rebuild paths refuse. + let (bundle, data) = make_bundle_with_hop_count(); + let hop_count = *bundle + .blocks + .iter() + .find(|(_, b)| b.block_type == block::Type::HopCount) + .expect("the bundle carries a Hop Count block") + .0; + refused( + ok(Editor::new(&bundle, &data).with_bundle_flags(admin.clone())).rebuild_bundle(), + hop_count, + ); + assert!(matches!( + ok(Editor::new(&bundle, &data).with_bundle_flags(admin.clone())).rebuild(), + Err(Error::ReportOnFailureForbidden(n)) if n == hop_count + )); + + // Three flagged blocks, the kept Hop Count and two pushed above it: the + // refusal names the lowest, whatever the iteration order. Each fresh + // editor iterates its blocks in its own hash order, so the loop covers + // many orders on both rebuild paths. + assert_eq!(hop_count, 2, "precondition: the Hop Count holds block 2"); + for _ in 0..16 { + let editor = with_reporting_block(with_reporting_block(Editor::new(&bundle, &data))); + refused( + ok(editor.with_bundle_flags(admin.clone())).rebuild_bundle(), + hop_count, + ); + let editor = with_reporting_block(with_reporting_block(Editor::new(&bundle, &data))); + assert!(matches!( + ok(editor.with_bundle_flags(admin.clone())).rebuild(), + Err(Error::ReportOnFailureForbidden(n)) if n == hop_count + )); + } + + // Control: an ordinary bundle keeps the flag. + let (bundle, data) = make_bundle(); + let (rebuilt, chunks) = with_reporting_block(Editor::new(&bundle, &data)) + .rebuild_bundle() + .expect("an ordinary bundle permits the flag"); + assert_rebuild_matches_parse(&rebuilt, &Chunk::flatten(chunks, &data)); +} + // `Type::canonicalize` folds an `Unrecognised` alias of a known code back // to the named variant it encodes as, and leaves everything else alone. +// Matched structurally: equality already counts an alias as its variant. #[test] fn canonicalize_folds_reserved_aliases() { - assert_eq!( + assert!(matches!( block::Type::Unrecognised(0).canonicalize(), block::Type::Primary - ); - assert_eq!( + )); + assert!(matches!( block::Type::Unrecognised(11).canonicalize(), block::Type::BlockIntegrity - ); - assert_eq!( + )); + assert!(matches!( block::Type::Unrecognised(12).canonicalize(), block::Type::BlockSecurity - ); - assert_eq!( + )); + assert!(matches!( block::Type::Unrecognised(192).canonicalize(), block::Type::Unrecognised(192) - ); - assert_eq!(block::Type::Payload.canonicalize(), block::Type::Payload); + )); + assert!(matches!( + block::Type::Payload.canonicalize(), + block::Type::Payload + )); + assert!(!block::Type::Unrecognised(11).is_canonical()); + assert!(block::Type::Unrecognised(192).is_canonical()); + assert!(block::Type::BlockIntegrity.is_canonical()); } // Reserved wire codes must be refused whatever `Type` variant carries them: @@ -746,7 +907,7 @@ fn extension_editor_maps_singleton_duplicates_through() { block::Type::HopCount, block::Flags::default(), crc::CrcType::None, - b"x".as_slice().into(), + hop_count_body(), ), Err(extension_editor::Error::Editor(Error::IllegalDuplicate( block::Type::HopCount @@ -1047,6 +1208,290 @@ fn extension_editor_materialises_inserts_and_skips_untouched() { assert_eq!(new_bundle.blocks.len(), reparsed.blocks.len()); } +// === ExtensionEditor: the parser's accept-set at call time ============= + +// A well-formed Hop Count body. +fn hop_count_body() -> Box<[u8]> { + emit(&hop_info::HopInfo { + limit: NonZeroU8::new(30).unwrap(), + count: 0, + }) + .0 + .into() +} + +// A parsed bundle built with the given source and bundle flags. +fn make_bundle_from(source: &str, flags: bundle::Flags) -> (Bundle, Box<[u8]>) { + let (_, data) = builder::Builder::new(source.parse().unwrap(), "ipn:2.0".parse().unwrap()) + .with_flags(flags) + .with_payload(b"Hello".as_slice().into()) + .build(creation_timestamp::CreationTimestamp::now()) + .unwrap(); + let bundle = reparse(&data); + (bundle, data) +} + +// Insert an Unrecognised(200) block with `report_on_failure` set. +fn insert_reporting_block(editor: &mut ExtensionEditor) -> extension_editor::Result { + editor.insert( + block::Type::Unrecognised(200), + block::Flags { + report_on_failure: true, + ..Default::default() + }, + crc::CrcType::None, + b"ext-data".as_slice().into(), + ) +} + +#[test] +fn extension_editor_refuses_report_on_failure_the_bundle_forbids() { + // RFC 9171 §4.2.3-4/-5: a null-source bundle (which must also be + // unfragmentable) and an administrative record both forbid the flag. + let null_source = make_bundle_from( + "dtn:none", + bundle::Flags { + do_not_fragment: true, + ..Default::default() + }, + ); + let admin_record = make_bundle_from( + "ipn:1.0", + bundle::Flags { + is_admin_record: true, + ..Default::default() + }, + ); + for (bundle, data) in [&null_source, &admin_record] { + assert!( + bundle.primary.forbids_report_on_failure(), + "precondition: the fixture forbids report_on_failure" + ); + let mut editor = ExtensionEditor::new(bundle, data); + assert!(matches!( + insert_reporting_block(&mut editor), + Err(extension_editor::Error::Invalid(Bpv7Error::InvalidFlags)) + )); + assert!(!editor.is_modified(), "a refusal is not an edit"); + } + + // Control: an ordinary bundle takes the same insert, and the rewrite + // re-parses. + let (bundle, data) = make_bundle(); + assert!(!bundle.primary.forbids_report_on_failure()); + let mut editor = ExtensionEditor::new(&bundle, &data); + let inserted = insert_reporting_block(&mut editor).expect("the insert is accepted"); + let (_, chunks) = editor.finish().unwrap().expect("an edit materialises"); + let rewritten = reparse(&Chunk::flatten(chunks, &data)); + assert!(rewritten.blocks[&inserted].flags.report_on_failure); +} + +// A named bit carried in `unrecognised` is the flag it encodes: the editor +// refuses `report_on_failure`'s alias on an administrative record as the +// forbidden flag, and on an ordinary bundle the alias reports, with a +// genuinely unrecognised bit passing through. +#[test] +fn extension_editor_canonicalizes_unrecognised_aliases_of_named_flags() { + fn aliased() -> block::Flags { + block::Flags { + unrecognised: (1 << 1) | (1 << 8), + ..Default::default() + } + } + + let (bundle, data) = make_bundle_from( + "ipn:1.0", + bundle::Flags { + is_admin_record: true, + ..Default::default() + }, + ); + let mut editor = ExtensionEditor::new(&bundle, &data); + assert!(matches!( + editor.insert( + block::Type::Unrecognised(200), + aliased(), + crc::CrcType::None, + b"ext-data".as_slice().into(), + ), + Err(extension_editor::Error::Invalid(Bpv7Error::InvalidFlags)) + )); + assert!(!editor.is_modified(), "a refusal is not an edit"); + + let (bundle, data) = make_bundle_from("ipn:1.0", bundle::Flags::default()); + let mut editor = ExtensionEditor::new(&bundle, &data); + let inserted = editor + .insert( + block::Type::Unrecognised(200), + aliased(), + crc::CrcType::None, + b"ext-data".as_slice().into(), + ) + .expect("an ordinary bundle permits the flag"); + let (_, chunks) = editor.finish().unwrap().expect("an edit materialises"); + let rewritten = reparse(&Chunk::flatten(chunks, &data)); + let flags = &rewritten.blocks[&inserted].flags; + assert!(flags.report_on_failure); + assert_eq!(flags.unrecognised, 1 << 8); +} + +#[test] +fn extension_editor_refuses_an_unrecognised_crc_type() { + let (bundle, data) = make_bundle(); + let mut editor = ExtensionEditor::new(&bundle, &data); + assert!(matches!( + editor.insert( + block::Type::Unrecognised(200), + block::Flags::default(), + crc::CrcType::Unrecognised(5), + b"ext-data".as_slice().into(), + ), + Err(extension_editor::Error::Invalid(Bpv7Error::InvalidCrc( + crc::Error::InvalidType(5) + ))) + )); + assert!(!editor.is_modified()); + + // Control: each recognised CRC type is accepted and the inserted block + // carries it — the re-parse checks the CRC value too. + for crc_type in [crc::CrcType::CRC16_X25, crc::CrcType::CRC32_CASTAGNOLI] { + let mut editor = ExtensionEditor::new(&bundle, &data); + let inserted = editor + .insert( + block::Type::Unrecognised(200), + block::Flags::default(), + crc_type, + b"ext-data".as_slice().into(), + ) + .expect("a recognised CRC type is accepted"); + let (_, chunks) = editor.finish().unwrap().expect("an edit materialises"); + let rewritten = reparse(&Chunk::flatten(chunks, &data)); + assert_eq!(rewritten.blocks[&inserted].crc_type, crc_type); + } +} + +// Insert `body` as `block_type` into a fresh bundle, expecting a refusal; +// returns the parser error the refusal carries. +fn refused_insert(block_type: block::Type, body: &[u8]) -> Bpv7Error { + let (bundle, data) = make_bundle(); + let mut editor = ExtensionEditor::new(&bundle, &data); + let result = editor.insert( + block_type, + block::Flags::default(), + crc::CrcType::None, + body.into(), + ); + assert!(!editor.is_modified(), "a refusal is not an edit"); + match result { + Err(extension_editor::Error::UndecodableBody { + block_type: checked, + source, + }) => { + assert_eq!(checked, block_type, "the refusal names the type checked"); + *source + } + other => panic!("{block_type:?} must be refused as UndecodableBody, got {other:?}"), + } +} + +#[test] +fn extension_editor_refuses_undecodable_well_known_insert_bodies() { + // Each type's body decoder rejects a CBOR text string ("bad") with its + // own error, and the refusal carries exactly that error. + let bad = b"\x63bad".as_slice(); + assert!(matches!( + refused_insert(block::Type::PreviousNode, bad), + Bpv7Error::InvalidEid(eid::Error::InvalidCBOR(CborError::IncorrectType(..))) + )); + assert!(matches!( + refused_insert(block::Type::BundleAge, bad), + Bpv7Error::InvalidCBOR(CborError::IncorrectType(..)) + )); + assert!(matches!( + refused_insert(block::Type::HopCount, bad), + Bpv7Error::InvalidCBOR(CborError::IncorrectType(..)) + )); + // The `Unrecognised` alias of Hop Count's code is validated as a Hop + // Count: a bare integer is not the Hop Count array. + assert!(matches!( + refused_insert(block::Type::Unrecognised(10), &[0x00]), + Bpv7Error::InvalidCBOR(CborError::IncorrectType(..)) + )); + + // Control: a valid body of each type is accepted and re-parses. + let valid_bodies: [(block::Type, Box<[u8]>); 3] = [ + ( + block::Type::PreviousNode, + emit(&"ipn:3.0".parse::().unwrap()).0.into(), + ), + (block::Type::BundleAge, emit(&0u64).0.into()), + (block::Type::HopCount, hop_count_body()), + ]; + for (block_type, valid) in valid_bodies { + let (bundle, data) = make_bundle(); + let mut editor = ExtensionEditor::new(&bundle, &data); + let inserted = editor + .insert( + block_type, + block::Flags::default(), + crc::CrcType::None, + valid.clone(), + ) + .expect("a valid body is accepted"); + let (_, chunks) = editor.finish().unwrap().expect("an edit materialises"); + let rewritten_data = Chunk::flatten(chunks, &data); + let block = &reparse(&rewritten_data).blocks[&inserted]; + assert_eq!(block.block_type, block_type); + assert_eq!(block.payload(&rewritten_data), Some(&*valid)); + } +} + +#[test] +fn extension_editor_refuses_an_undecodable_well_known_replacement() { + let (bundle, data) = make_bundle_with_hop_count(); + let hop = bundle + .blocks + .iter() + .find(|(_, b)| matches!(b.block_type, block::Type::HopCount)) + .map(|(n, _)| *n) + .expect("the hop count is present"); + let mut editor = ExtensionEditor::new(&bundle, &data); + // Well-formed CBOR, but a hop limit of 0 is outside RFC 9171 §4.4.3's + // 1..=255: the semantic check refuses it, not just the shape check. + let Err(extension_editor::Error::UndecodableBody { + block_type: block::Type::HopCount, + source, + }) = editor.replace(hop, [0x82, 0x00, 0x00].as_slice().into()) + else { + panic!("an out-of-range hop limit must be refused as UndecodableBody"); + }; + assert!(matches!(*source, Bpv7Error::InvalidHopLimit(0))); + assert!(!editor.is_modified()); + + // Control: a valid body is accepted, and the replace changes the data + // alone — the block keeps its flags and CRC type. + let replacement = hop_info::HopInfo { + limit: NonZeroU8::new(30).unwrap(), + count: 1, + }; + editor + .replace(hop, emit(&replacement).0.into()) + .expect("a valid body is accepted"); + let (_, chunks) = editor.finish().unwrap().expect("an edit materialises"); + let rewritten_data = Chunk::flatten(chunks, &data); + let rewritten = reparse(&rewritten_data); + let original = &bundle.blocks[&hop]; + let replaced = &rewritten.blocks[&hop]; + assert_eq!(replaced.flags, original.flags); + assert_eq!(replaced.crc_type, original.crc_type); + assert_eq!( + replaced + .extract::(&rewritten_data) + .unwrap(), + Some(replacement) + ); +} + // === insert_block replace-by-type: BIB/BCB coverage parity ============= // A bundle whose HopCount block is BIB-signed: (bundle, data, hop block diff --git a/bpv7/tests/flags.rs b/bpv7/tests/flags.rs new file mode 100644 index 000000000..04d5b2ad1 --- /dev/null +++ b/bpv7/tests/flags.rs @@ -0,0 +1,323 @@ +//! Integration tests for the RFC 9171 codes bpv7 names — `block::Flags` +//! and `bundle::Flags` against the bit assignments of §4.2.4 and §4.2.3, +//! and `block::Type` against §4.2.1's type codes — and for their equality, +//! hashing, and serde, which all go by the encoding. The BPSec scope flags' +//! walk lives with their codec in `tests/rfc9173.rs`. + +use core::cmp::Ordering; +use std::{ + collections::{BTreeMap, BTreeSet}, + hash::{DefaultHasher, Hash, Hasher}, +}; + +use hardy_bpv7::{block, bundle}; + +fn hash_of(value: &T) -> u64 { + let mut hasher = DefaultHasher::new(); + value.hash(&mut hasher); + hasher.finish() +} + +// The block processing control flags bpv7 names are exactly RFC 9171 +// §4.2.4's, each decoding to the field the RFC names it: every other bit +// round-trips as unrecognised, and a named bit carried in `unrecognised` +// encodes as its bit and canonicalizes to its field. +#[test] +fn block_flags_name_exactly_the_rfc_bits() { + let named = [ + ( + 0, + block::Flags { + must_replicate: true, + ..Default::default() + }, + ), + ( + 1, + block::Flags { + report_on_failure: true, + ..Default::default() + }, + ), + ( + 2, + block::Flags { + delete_bundle_on_failure: true, + ..Default::default() + }, + ), + ( + 4, + block::Flags { + delete_block_on_failure: true, + ..Default::default() + }, + ), + ]; + for bit in 0..64 { + let value = 1u64 << bit; + let decoded = block::Flags::from(value); + assert!( + decoded.is_canonical(), + "bit {bit}: a decoded value is canonical" + ); + let named_flags = named + .iter() + .find(|(named_bit, _)| *named_bit == bit) + .map(|(_, flags)| flags.clone()); + let expected = named_flags.clone().unwrap_or(block::Flags { + unrecognised: value, + ..Default::default() + }); + // Both canonical, so equal encodings mean equal fields. + assert_eq!( + decoded, expected, + "bit {bit}: decodes to the field RFC 9171 names, or as unrecognised" + ); + assert_eq!(u64::from(&decoded), value, "bit {bit} round-trips"); + let alias = block::Flags { + unrecognised: value, + ..Default::default() + }; + assert_eq!( + u64::from(&alias), + value, + "bit {bit}: an alias encodes as its bit" + ); + assert_eq!( + alias.is_canonical(), + named_flags.is_none(), + "bit {bit}: an alias of a named bit is not canonical" + ); + let folded = alias.clone().canonicalize(); + assert!(folded.is_canonical(), "bit {bit}: the alias folds"); + assert_eq!( + folded, decoded, + "bit {bit}: the alias folds to the decoded value" + ); + assert_eq!(alias, decoded, "bit {bit}: an alias equals what it encodes"); + assert_eq!( + hash_of(&alias), + hash_of(&decoded), + "bit {bit}: and hashes alike" + ); + } +} + +// The bundle processing control flags bpv7 names are exactly RFC 9171 +// §4.2.3's, with the same field, round-trip and alias rules. +#[test] +fn bundle_flags_name_exactly_the_rfc_bits() { + let named = [ + ( + 0, + bundle::Flags { + is_fragment: true, + ..Default::default() + }, + ), + ( + 1, + bundle::Flags { + is_admin_record: true, + ..Default::default() + }, + ), + ( + 2, + bundle::Flags { + do_not_fragment: true, + ..Default::default() + }, + ), + ( + 5, + bundle::Flags { + app_ack_requested: true, + ..Default::default() + }, + ), + ( + 6, + bundle::Flags { + report_status_time: true, + ..Default::default() + }, + ), + ( + 14, + bundle::Flags { + receipt_report_requested: true, + ..Default::default() + }, + ), + ( + 16, + bundle::Flags { + forward_report_requested: true, + ..Default::default() + }, + ), + ( + 17, + bundle::Flags { + delivery_report_requested: true, + ..Default::default() + }, + ), + ( + 18, + bundle::Flags { + delete_report_requested: true, + ..Default::default() + }, + ), + ]; + for bit in 0..64 { + let value = 1u64 << bit; + let decoded = bundle::Flags::from(value); + assert!( + decoded.is_canonical(), + "bit {bit}: a decoded value is canonical" + ); + let named_flags = named + .iter() + .find(|(named_bit, _)| *named_bit == bit) + .map(|(_, flags)| flags.clone()); + let expected = named_flags.clone().unwrap_or(bundle::Flags { + unrecognised: value, + ..Default::default() + }); + // Both canonical, so equal encodings mean equal fields. + assert_eq!( + decoded, expected, + "bit {bit}: decodes to the field RFC 9171 names, or as unrecognised" + ); + assert_eq!(u64::from(&decoded), value, "bit {bit} round-trips"); + let alias = bundle::Flags { + unrecognised: value, + ..Default::default() + }; + assert_eq!( + u64::from(&alias), + value, + "bit {bit}: an alias encodes as its bit" + ); + assert_eq!( + alias.is_canonical(), + named_flags.is_none(), + "bit {bit}: an alias of a named bit is not canonical" + ); + let folded = alias.clone().canonicalize(); + assert!(folded.is_canonical(), "bit {bit}: the alias folds"); + assert_eq!( + folded, decoded, + "bit {bit}: the alias folds to the decoded value" + ); + assert_eq!(alias, decoded, "bit {bit}: an alias equals what it encodes"); + assert_eq!( + hash_of(&alias), + hash_of(&decoded), + "bit {bit}: and hashes alike" + ); + } +} + +// Block types compare, order, and hash by their RFC 9171 §4.2.1 code: an +// `Unrecognised` alias of a known code equals the variant it encodes as, +// and the order is the code order. +#[test] +fn block_types_compare_by_code() { + for (alias, named) in [ + (block::Type::Unrecognised(1), block::Type::Payload), + (block::Type::Unrecognised(10), block::Type::HopCount), + (block::Type::Unrecognised(11), block::Type::BlockIntegrity), + ] { + assert_eq!(alias, named); + assert_eq!(alias.cmp(&named), Ordering::Equal); + assert_eq!(hash_of(&alias), hash_of(&named)); + assert!(!alias.is_canonical()); + assert!(named.is_canonical()); + } + assert!(block::Type::Unrecognised(2) < block::Type::PreviousNode); + assert!(block::Type::HopCount < block::Type::BlockIntegrity); + assert!(block::Type::BlockSecurity < block::Type::Unrecognised(13)); + + // In ordered containers an alias and its named variant are one key, and + // iteration runs in code order. + assert_eq!( + BTreeSet::from([block::Type::Unrecognised(1), block::Type::Payload]).len(), + 1 + ); + let order: Vec<_> = BTreeMap::from([ + (block::Type::BlockIntegrity, ()), + (block::Type::Unrecognised(2), ()), + (block::Type::HopCount, ()), + (block::Type::Payload, ()), + ]) + .into_keys() + .collect(); + assert_eq!( + order, + [ + block::Type::Payload, + block::Type::Unrecognised(2), + block::Type::HopCount, + block::Type::BlockIntegrity + ] + ); +} + +// Serde canonicalizes in both directions: an alias in a stored record loads +// as the flag it encodes, and a non-canonical value stores canonically, in +// the plain field-list shape. +#[test] +fn flags_serde_canonicalizes_both_ways() { + let loaded: block::Flags = serde_json::from_str(r#"{"unrecognised":2}"#).unwrap(); + assert!(loaded.is_canonical()); + assert!(loaded.report_on_failure); + let stored = serde_json::to_string(&block::Flags { + unrecognised: (1 << 1) | (1 << 8), + ..Default::default() + }) + .unwrap(); + assert_eq!(stored, r#"{"report_on_failure":true,"unrecognised":256}"#); + + let loaded: bundle::Flags = serde_json::from_str(r#"{"unrecognised":2}"#).unwrap(); + assert!(loaded.is_canonical()); + assert!(loaded.is_admin_record); + let stored = serde_json::to_string(&bundle::Flags { + unrecognised: (1 << 1) | (1 << 24), + ..Default::default() + }) + .unwrap(); + assert_eq!( + stored, + r#"{"is_admin_record":true,"unrecognised":16777216}"# + ); + + // `unrecognised` is a plain integer: an explicit null is a data error. + for null in [ + serde_json::from_str::(r#"{"unrecognised":null}"#).map(drop), + serde_json::from_str::(r#"{"unrecognised":null}"#).map(drop), + ] { + let Err(e) = null else { + panic!("an explicit null must not deserialize"); + }; + assert!(matches!(e.classify(), serde_json::error::Category::Data)); + } +} + +#[test] +fn block_type_serde_canonicalizes_both_ways() { + let loaded: block::Type = serde_json::from_str(r#"{"Unrecognised":11}"#).unwrap(); + assert!(matches!(loaded, block::Type::BlockIntegrity)); + assert_eq!( + serde_json::to_string(&block::Type::Unrecognised(10)).unwrap(), + r#""HopCount""# + ); + assert_eq!( + serde_json::to_string(&block::Type::Unrecognised(200)).unwrap(), + r#"{"Unrecognised":200}"# + ); +} diff --git a/bpv7/tests/parse.rs b/bpv7/tests/parse.rs index f35ca1eac..20c8180aa 100644 --- a/bpv7/tests/parse.rs +++ b/bpv7/tests/parse.rs @@ -6,12 +6,13 @@ use core::{iter::repeat_n, num::NonZeroU8}; use bytes::Bytes; -use hardy_bpv7::{Error, block, builder, crc, creation_timestamp, hop_info, parse}; +use hardy_bpv7::{Error, block, builder, bundle, crc, creation_timestamp, hop_info, parse}; // Aliased: collides with the bpv7 `Error` imported above. use hardy_cbor::decode::Error as CborError; use hex_literal::hex; mod common; +use self::common::{insert_after_primary, make_block}; // Build a minimal valid bundle and return its serialised bytes. fn build_minimal_bundle() -> Box<[u8]> { @@ -79,6 +80,68 @@ fn invalid_flags() { )); } +// RFC 9171 §4.2.3-4/-5 at the parser: a block requesting a report on +// failure is rejected in a null-source bundle and in an administrative +// record, and accepted in an ordinary bundle. The block is spliced into the +// wire bytes, since the builder never emits the forbidden combination; the +// same splice without the flag parses, so a rejection is the flag's alone. +#[test] +fn parser_rejects_report_on_failure_where_the_bundle_forbids_it() { + for (label, source, flags, rejected) in [ + ( + "null source", + "dtn:none", + bundle::Flags { + do_not_fragment: true, + ..Default::default() + }, + true, + ), + ( + "admin record", + "ipn:1.0", + bundle::Flags { + is_admin_record: true, + ..Default::default() + }, + true, + ), + ("ordinary", "ipn:1.0", bundle::Flags::default(), false), + ] { + let (_, data) = builder::Builder::new(source.parse().unwrap(), "ipn:2.0".parse().unwrap()) + .with_flags(flags) + .with_payload(b"Hello".as_slice().into()) + .build(creation_timestamp::CreationTimestamp::now()) + .unwrap(); + + let plain = insert_after_primary(&data, &[&make_block(200, 2, 0, b"ext-data")]); + parse::parse(Bytes::from(plain)) + .unwrap_or_else(|e| panic!("{label}: the splice without the flag must parse: {e:?}")); + + let reporting = insert_after_primary(&data, &[&make_block(200, 2, 1 << 1, b"ext-data")]); + match (rejected, parse::parse(Bytes::from(reporting))) { + (true, Err(Error::InvalidFlags)) => {} + (false, Ok(parsed)) => { + let block = parsed + .bundle + .blocks + .values() + .find(|b| b.block_type == block::Type::Unrecognised(200)) + .unwrap_or_else(|| panic!("{label}: the spliced block is present")); + assert!( + block.flags.report_on_failure, + "{label}: the flag arrives set" + ); + } + (_, result) => panic!( + "{label}: expected {}, got {:?}", + if rejected { "InvalidFlags" } else { "a parse" }, + result.map(drop) + ), + } + } +} + // NOTE: LLR 1.1.33 (Bundle Age required when Creation Time is zero) is enforced // by the BPA rfc9171-filter, not the parser. The parser accepts such bundles for // compatibility with RFC 9173 test vectors. diff --git a/bpv7/tests/rfc9173.rs b/bpv7/tests/rfc9173.rs index d4acc371a..97ec22308 100644 --- a/bpv7/tests/rfc9173.rs +++ b/bpv7/tests/rfc9173.rs @@ -2,7 +2,7 @@ use core::time::Duration; use hardy_bpv7::{ Bundle, block::{Block, Type}, - bpsec::{self, edit::BPSecEditor, encryptor, key, rfc9173::ScopeFlags, signer}, + bpsec::{self, bcb, bib, edit::BPSecEditor, encryptor, key, rfc9173::ScopeFlags, signer}, builder::Builder, checks, creation_timestamp::CreationTimestamp, @@ -1317,3 +1317,256 @@ fn test_sign_removes_crc_from_target_block() { payload_cbor[0] ); } + +// The scope flags name exactly RFC 9173 §3.3.3's three bits (shared by +// §4.3.4's AAD scope), each decoding to the field the RFC names it: every +// other bit round-trips as unrecognised, and a named bit carried in +// `unrecognised` encodes as its bit and canonicalizes to its field. +#[test] +fn scope_flags_name_exactly_the_rfc_bits() { + let named = [ + ( + 0, + ScopeFlags { + include_primary_block: true, + ..ScopeFlags::NONE + }, + ), + ( + 1, + ScopeFlags { + include_target_header: true, + ..ScopeFlags::NONE + }, + ), + ( + 2, + ScopeFlags { + include_security_header: true, + ..ScopeFlags::NONE + }, + ), + ]; + for bit in 0..64 { + let value = 1u64 << bit; + let decoded = ScopeFlags::from(value); + assert!( + decoded.is_canonical(), + "bit {bit}: a decoded value is canonical" + ); + let named_flags = named + .iter() + .find(|(named_bit, _)| *named_bit == bit) + .map(|(_, flags)| flags.clone()); + let expected = named_flags.clone().unwrap_or(ScopeFlags { + unrecognised: value, + ..ScopeFlags::NONE + }); + // Both canonical, so equal encodings mean equal fields. + assert_eq!( + decoded, expected, + "bit {bit}: decodes to the field RFC 9173 names, or as unrecognised" + ); + assert_eq!(u64::from(&decoded), value, "bit {bit} round-trips"); + let alias = ScopeFlags { + unrecognised: value, + ..ScopeFlags::NONE + }; + assert_eq!( + u64::from(&alias), + value, + "bit {bit}: an alias encodes as its bit" + ); + assert_eq!( + alias.is_canonical(), + named_flags.is_none(), + "bit {bit}: an alias of a named bit is not canonical" + ); + let folded = alias.clone().canonicalize(); + assert!(folded.is_canonical(), "bit {bit}: the alias folds"); + assert_eq!( + folded, decoded, + "bit {bit}: the alias folds to the decoded value" + ); + assert_eq!(alias, decoded, "bit {bit}: an alias equals what it encodes"); + } +} + +// The primary-block scope bit carried in `unrecognised` on an otherwise +// empty scope: the scope these tests sign and encrypt under. It folds to a +// primary-only scope, which is not the default, so the operation emits it +// as its scope parameter. +fn primary_scope_alias() -> ScopeFlags { + ScopeFlags { + unrecognised: 1 << 0, + ..ScopeFlags::NONE + } +} + +// The alias folded: the scope the emitted parameter must carry. +fn primary_scope() -> ScopeFlags { + ScopeFlags { + include_primary_block: true, + ..ScopeFlags::NONE + } +} + +// Signing canonicalizes the scope: the parameter it emits carries the +// folded, primary-only scope, and the IPPT the source computed agrees with +// it, so the BIB verifies. +#[test] +fn signing_canonicalizes_an_alias_scope_bit() { + let (_bundle, bundle_bytes) = + Builder::new("ipn:1.2".parse().unwrap(), "ipn:2.1".parse().unwrap()) + .with_payload(b"aliased scope".as_slice().into()) + .build(CreationTimestamp::now()) + .unwrap(); + let sign_key: key::Key = serde_json::from_value(serde_json::json!({ + "kid": "ipn:2.1", + "kty": "oct", + "alg": "HS256", + "key_ops": ["sign", "verify"], + "k": rand_k(18) + })) + .unwrap(); + let keys = key::KeySet::new(vec![sign_key.clone()]); + + let raw = raw_of(&bundle_bytes); + let signed_bytes = signer::Signer::new(&raw, &bundle_bytes) + .sign_block( + 1, + signer::Context::HMAC_SHA2(primary_scope_alias()), + "ipn:2.1".parse().unwrap(), + &sign_key, + ) + .map_err(|(_, e)| e) + .expect("signing accepts the alias scope") + .rebuild() + .expect("the signed bundle rebuilds"); + + let (signed_bytes, parsed, bcb_ops, bib_ops) = + validate_with_keys(&signed_bytes, &keys).expect("the signed bundle verifies at parse"); + // `bib` and `bcb` each name an `Operation`, so both stay module-qualified. + let bib::Operation::HMAC_SHA2(op) = &bib_ops + .values() + .next() + .expect("the bundle carries one BIB") + .operations()[&1] + else { + panic!("the BIB is BIB-HMAC-SHA2"); + }; + assert_eq!(op.parameters.flags, primary_scope()); + assert!( + verify_block(1, &parsed.blocks, &signed_bytes, &bcb_ops, &bib_ops, &keys) + .expect("the BIB verifies under the scope the parameter states"), + "a BIB covers the payload" + ); +} + +// Encryption canonicalizes the scope: the parameter it emits carries the +// folded, primary-only scope, and the AAD the source computed agrees with +// it, so the payload decrypts. +#[test] +fn encryption_canonicalizes_an_alias_scope_bit() { + let plaintext = b"aliased scope"; + let (_bundle, bundle_bytes) = + Builder::new("ipn:1.2".parse().unwrap(), "ipn:2.1".parse().unwrap()) + .with_payload(plaintext.as_slice().into()) + .build(CreationTimestamp::now()) + .unwrap(); + let enc_key: key::Key = serde_json::from_value(serde_json::json!({ + "kid": "ipn:2.1", + "kty": "oct", + "alg": "A128KW", + "enc": "A128GCM", + "key_ops": ["encrypt", "decrypt", "wrapKey", "unwrapKey"], + "k": rand_k(16) + })) + .unwrap(); + let keys = key::KeySet::new(vec![enc_key.clone()]); + + let raw = raw_of(&bundle_bytes); + let encrypted_bytes = encryptor::Encryptor::new(&raw, &bundle_bytes) + .encrypt_block( + 1, + encryptor::Context::AES_GCM(primary_scope_alias()), + "ipn:2.1".parse().unwrap(), + &enc_key, + ) + .map_err(|(_, e)| e) + .expect("encryption accepts the alias scope") + .rebuild() + .expect("the encrypted bundle rebuilds"); + + let (encrypted_bytes, parsed, bcb_ops, _bib_ops) = + validate_with_keys(&encrypted_bytes, &keys).expect("the encrypted bundle parses"); + let bcb::Operation::AES_GCM(op) = &bcb_ops + .values() + .next() + .expect("the bundle carries one BCB") + .operations()[&1] + else { + panic!("the BCB is BCB-AES-GCM"); + }; + assert_eq!(op.parameters.flags, primary_scope()); + let decrypted = block_data(1, &parsed.blocks, &encrypted_bytes, &bcb_ops, &keys) + .expect("the payload decrypts under the scope the parameter states"); + assert_eq!(decrypted.as_ref(), plaintext); +} + +// Scope equality compares the encoding, so an aliased scope and its +// canonical form are one security context: signing two blocks under them +// yields one BIB carrying both targets, not two BIBs with identical +// parameters. +#[test] +fn aliased_and_canonical_scopes_share_one_bib() { + let (_bundle, bundle_bytes) = + Builder::new("ipn:1.2".parse().unwrap(), "ipn:2.1".parse().unwrap()) + .add_extension_block(Type::Unrecognised(200)) + .unwrap() + .build(b"ext-data".as_slice().into()) + .with_payload(b"aliased scope".as_slice().into()) + .build(CreationTimestamp::now()) + .unwrap(); + let sign_key: key::Key = serde_json::from_value(serde_json::json!({ + "kid": "ipn:2.1", + "kty": "oct", + "alg": "HS256", + "key_ops": ["sign", "verify"], + "k": rand_k(18) + })) + .unwrap(); + let keys = key::KeySet::new(vec![sign_key.clone()]); + + let raw = raw_of(&bundle_bytes); + let ext = *raw + .blocks + .iter() + .find(|(_, b)| b.block_type == Type::Unrecognised(200)) + .expect("the extension block is present") + .0; + let signed_bytes = signer::Signer::new(&raw, &bundle_bytes) + .sign_block( + 1, + signer::Context::HMAC_SHA2(primary_scope_alias()), + "ipn:2.1".parse().unwrap(), + &sign_key, + ) + .map_err(|(_, e)| e) + .expect("signing accepts the alias scope") + .sign_block( + ext, + signer::Context::HMAC_SHA2(primary_scope()), + "ipn:2.1".parse().unwrap(), + &sign_key, + ) + .map_err(|(_, e)| e) + .expect("signing accepts the canonical scope") + .rebuild() + .expect("the signed bundle rebuilds"); + + let (_, _, _, bib_ops) = + validate_with_keys(&signed_bytes, &keys).expect("the signed bundle verifies at parse"); + assert_eq!(bib_ops.len(), 1, "one BIB for the one scope"); + assert_eq!(bib_ops.values().next().unwrap().operations().len(), 2); +} diff --git a/bpv7/tools/CHANGELOG.md b/bpv7/tools/CHANGELOG.md index f39a50d6d..afe4fc4f7 100644 --- a/bpv7/tools/CHANGELOG.md +++ b/bpv7/tools/CHANGELOG.md @@ -6,6 +6,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ## [Unreleased] +### Fixed +- `inspect` lists a non-fragment bundle's primary flags: every flag after "Is a fragment", and the report-to note, sat inside the fragment check, so only a fragment's flags were printed. The note that status time is requested without reports now appears only when the bundle requests no status report at all, not whenever one of the four is unrequested. + ### Removed - `add-block` no longer accepts `block-integrity` (`bib`) or `block-security` (`bcb`) as `--type` values, and its help no longer advertises them: with every editor door refusing the reserved wire codes, no `add-block` invocation can craft a BIB/BCB — a numeric `--type 11`/`12` now surfaces the editor's typed `SecurityBlock` refusal. Properly-formed security blocks come from the signing and encryption commands. diff --git a/bpv7/tools/src/cmd/inspect.rs b/bpv7/tools/src/cmd/inspect.rs index 0f75f558d..677d220df 100644 --- a/bpv7/tools/src/cmd/inspect.rs +++ b/bpv7/tools/src/cmd/inspect.rs @@ -1,8 +1,13 @@ -use super::*; use core::time::Duration; -use hardy_bpv7::{block, bpsec, bundle, bundle_age, crc, eid, hop_info}; -use hardy_cbor::decode::{parse_exact, parse_value}; use std::collections::HashMap; + +use hardy_bpv7::{ + block, bpsec, bundle, bundle_age, crc, eid, hop_info, primary_block::PrimaryBlock, +}; +use hardy_cbor::decode::{parse_exact, parse_value}; + +use super::*; + #[derive(Parser, Debug)] #[command( about = "Inspect and display bundle information", @@ -251,70 +256,7 @@ fn dump_markdown( dump_crc(primary.crc_type, &output)?; - if primary.flags == bundle::Flags::default() { - output.append_str("Bundle Flags: None\n\n")?; - } else { - output.append_str("Bundle Flags:\n\n")?; - - if primary.flags.is_fragment { - output.append_str("* Is a fragment\n")?; - - if primary.flags.is_admin_record { - output.append_str("* ADU is an Administrative Record\n")?; - } - - if primary.flags.do_not_fragment { - output.append_str("* Do not fragment\n")?; - } - - if primary.flags.app_ack_requested { - output.append_str("* Application acknowledgement requested\n")?; - } - - if primary.flags.report_status_time { - output.append_str("* Include status time with reports\n")?; - - if !primary.flags.receipt_report_requested - || !primary.flags.forward_report_requested - || !primary.flags.delivery_report_requested - || !primary.flags.delete_report_requested - { - notes.push("Bundle flags request status time to be included with status reports, but no reports are requested."); - } - } - - if primary.flags.receipt_report_requested { - output.append_str("* Reception report requested\n")?; - } - - if primary.flags.forward_report_requested { - output.append_str("* Forwarding report requested\n")?; - } - - if primary.flags.delivery_report_requested { - output.append_str("* Delivery report requested\n")?; - } - - if primary.flags.delete_report_requested { - output.append_str("* Deletion report requested\n")?; - } - - if let Some(u) = primary.flags.unrecognised { - output.append_str(format!("* Unrecognised: {u:#x}\n",))?; - } - - output.append_str("\n")?; - - if (primary.flags.receipt_report_requested - || primary.flags.forward_report_requested - || primary.flags.delivery_report_requested - || primary.flags.delete_report_requested) - && primary.report_to.is_null() - { - notes.push("Null endpoint EID specified for 'Report To', but status reports are requested."); - } - } - } + output.append_str(bundle_flags_markdown(primary, &mut notes))?; output.append_str(format!("Report-To: {}\n\n", primary.report_to))?; @@ -351,6 +293,66 @@ fn dump_markdown( Ok(()) } +// The primary block's flags section, pushing the notes its flags raise. +fn bundle_flags_markdown(primary: &PrimaryBlock, notes: &mut Vec<&'static str>) -> String { + let flags = &primary.flags; + if *flags == bundle::Flags::default() { + return "Bundle Flags: None\n\n".to_string(); + } + + let any_report = flags.receipt_report_requested + || flags.forward_report_requested + || flags.delivery_report_requested + || flags.delete_report_requested; + let mut md = String::from("Bundle Flags:\n\n"); + for (set, line) in [ + (flags.is_fragment, "* Is a fragment\n"), + (flags.is_admin_record, "* ADU is an Administrative Record\n"), + (flags.do_not_fragment, "* Do not fragment\n"), + ( + flags.app_ack_requested, + "* Application acknowledgement requested\n", + ), + ( + flags.report_status_time, + "* Include status time with reports\n", + ), + ( + flags.receipt_report_requested, + "* Reception report requested\n", + ), + ( + flags.forward_report_requested, + "* Forwarding report requested\n", + ), + ( + flags.delivery_report_requested, + "* Delivery report requested\n", + ), + ( + flags.delete_report_requested, + "* Deletion report requested\n", + ), + ] { + if set { + md.push_str(line); + } + } + if flags.unrecognised != 0 { + md.push_str(&format!("* Unrecognised: {:#x}\n", flags.unrecognised)); + } + md.push('\n'); + + if flags.report_status_time && !any_report { + notes.push("Bundle flags request status time to be included with status reports, but no reports are requested."); + } + if any_report && primary.report_to.is_null() { + notes + .push("Null endpoint EID specified for 'Report To', but status reports are requested."); + } + md +} + fn dump_crc(crc: crc::CrcType, output: &io::Output) -> anyhow::Result<()> { output.append_str("CRC: ")?; match crc { @@ -406,8 +408,8 @@ fn dump_block( output.append_str("* Delete bundle on failure\n")?; } - if let Some(u) = block.flags.unrecognised { - output.append_str(format!("* Unrecognised: {u:#x}\n"))?; + if block.flags.unrecognised != 0 { + output.append_str(format!("* Unrecognised: {:#x}\n", block.flags.unrecognised))?; } output.append_str("\n")?; @@ -587,8 +589,11 @@ fn dump_bcb(data: &[u8], output: &io::Output) -> anyhow::Result<()> { output.append_str("* Include security header\n")?; } - if let Some(u) = op.parameters.flags.unrecognised { - output.append_str(format!("* Unrecognised: {u:#x}\n"))?; + if op.parameters.flags.unrecognised != 0 { + output.append_str(format!( + "* Unrecognised: {:#x}\n", + op.parameters.flags.unrecognised + ))?; } output.append_str("\n")?; @@ -658,8 +663,11 @@ fn dump_bib(data: &[u8], output: &io::Output) -> anyhow::Result<()> { output.append_str("* Include security header\n")?; } - if let Some(u) = op.parameters.flags.unrecognised { - output.append_str(format!("* Unrecognised: {u:#x}\n"))?; + if op.parameters.flags.unrecognised != 0 { + output.append_str(format!( + "* Unrecognised: {:#x}\n", + op.parameters.flags.unrecognised + ))?; } output.append_str("\n")?; @@ -697,3 +705,68 @@ fn dump_bytes(data: &[u8]) -> String { .collect::>() .join("") } + +#[cfg(test)] +mod tests { + use hardy_bpv7::{builder::Builder, creation_timestamp::CreationTimestamp}; + + use super::*; + + fn primary(flags: bundle::Flags) -> PrimaryBlock { + let (bundle, _) = Builder::new("ipn:1.0".parse().unwrap(), "ipn:2.0".parse().unwrap()) + .with_flags(flags) + .with_report_to("ipn:3.0".parse().unwrap()) + .with_payload(b"inspect".as_slice().into()) + .build(CreationTimestamp::now()) + .expect("build the bundle"); + bundle.primary + } + + // A bundle that is not a fragment lists every flag it sets. + #[test] + fn a_non_fragment_bundle_lists_its_flags() { + let mut notes = Vec::new(); + let md = bundle_flags_markdown( + &primary(bundle::Flags { + do_not_fragment: true, + delivery_report_requested: true, + ..Default::default() + }), + &mut notes, + ); + assert_eq!( + md, + "Bundle Flags:\n\n* Do not fragment\n* Delivery report requested\n\n" + ); + assert!(notes.is_empty()); + } + + // The status-time note fires only when no report is requested at all. + #[test] + fn the_status_time_note_needs_every_report_unrequested() { + let mut notes = Vec::new(); + bundle_flags_markdown( + &primary(bundle::Flags { + report_status_time: true, + delivery_report_requested: true, + ..Default::default() + }), + &mut notes, + ); + assert!(notes.is_empty(), "one requested report suppresses the note"); + + bundle_flags_markdown( + &primary(bundle::Flags { + report_status_time: true, + ..Default::default() + }), + &mut notes, + ); + assert_eq!( + notes, + [ + "Bundle flags request status time to be included with status reports, but no reports are requested." + ] + ); + } +}