From 91b906fa619e85523d626b2fe4d84f05be3a67bb Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Sat, 26 Sep 2026 11:46:35 -0700 Subject: [PATCH] Prepare 1.7.7 with concise comments and shared CI policy --- .github/workflows/comment-guard.yml | 2 +- CHANGELOG.md | 6 + Cargo.lock | 10 +- Cargo.toml | 2 +- README.md | 7 + docs/keydb-format.md | 159 ---------- docs/keydb.md | 432 -------------------------- docs/lib-scope.md | 49 --- docs/multi-source-failure-collapse.md | 13 - docs/online-agent-cache.md | 39 --- docs/online-guardfail.md | 41 --- docs/online-preflight-guard-tests.md | 20 -- docs/online-query-contract.md | 19 -- docs/online-query-guards.md | 57 ---- docs/online-ssrf-guard.md | 44 --- docs/paths.md | 13 - src/keydb.rs | 169 +++++----- src/keydb_format.rs | 55 ++-- src/lib.rs | 29 +- src/online.rs | 84 ++--- src/paths.rs | 5 +- tests/key_sources.rs | 6 +- 22 files changed, 155 insertions(+), 1106 deletions(-) delete mode 100644 docs/keydb-format.md delete mode 100644 docs/keydb.md delete mode 100644 docs/lib-scope.md delete mode 100644 docs/multi-source-failure-collapse.md delete mode 100644 docs/online-agent-cache.md delete mode 100644 docs/online-guardfail.md delete mode 100644 docs/online-preflight-guard-tests.md delete mode 100644 docs/online-query-contract.md delete mode 100644 docs/online-query-guards.md delete mode 100644 docs/online-ssrf-guard.md delete mode 100644 docs/paths.md diff --git a/.github/workflows/comment-guard.yml b/.github/workflows/comment-guard.yml index f962514..1440dfa 100644 --- a/.github/workflows/comment-guard.yml +++ b/.github/workflows/comment-guard.yml @@ -9,4 +9,4 @@ concurrency: jobs: comment-guard: if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork - uses: freemkv/.github/.github/workflows/guard-comment.yml@v1 + uses: freemkv/.github/.github/workflows/guard-comment.yml@v2 diff --git a/CHANGELOG.md b/CHANGELOG.md index 0286deb..c986138 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## [1.7.7] — 2026-09-26 + +### Maintenance + +- Replace comment-overflow documentation with concise source contracts and README instructions; enforce the shared comment policy in CI. + ## [1.7.6] — 2026-09-26 ### Changed diff --git a/Cargo.lock b/Cargo.lock index e0fe865..4cf184c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -268,7 +268,7 @@ dependencies = [ [[package]] name = "freemkv-keysources" -version = "1.7.6" +version = "1.7.7" dependencies = [ "base64", "flate2", @@ -281,7 +281,7 @@ dependencies = [ [[package]] name = "freemkv-unlock" -version = "1.7.6" +version = "1.7.7" dependencies = [ "aes", "base64", @@ -487,7 +487,7 @@ checksum = "3eaf3ede3fee6db1a4c2ee091bf8a8b4dccdc6d17f656fb07896ee72867612f2" [[package]] name = "libfreemkv" -version = "1.7.6" +version = "1.7.7" dependencies = [ "aes", "base64", @@ -1237,8 +1237,8 @@ dependencies = [ [[patch.unused]] name = "freemkv-engine" -version = "1.7.6" +version = "1.7.7" [[patch.unused]] name = "freemkv-i18n" -version = "1.7.6" +version = "1.7.7" diff --git a/Cargo.toml b/Cargo.toml index 62db9f4..e7aefff 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "freemkv-keysources" -version = "1.7.6" +version = "1.7.7" edition = "2024" rust-version = "1.88" license = "MIT" diff --git a/README.md b/README.md index 1ab7950..0a36da4 100644 --- a/README.md +++ b/README.md @@ -9,6 +9,13 @@ Pluggable AACS key sources (keydb, online key service) for and hands libfreemkv its terminal Unit Keys via `get_unit_keys`; the library does all derivation. +## Development + +Build API documentation with `cargo doc --no-deps --open` for source construction, +keydb paths and online-query contracts. Run `cargo test --tests` to validate +parsing, caching and request guards. Tests use synthetic keys and local fixtures; +never commit real key material or credentials. + ## License MIT — see [LICENSE](LICENSE). diff --git a/docs/keydb-format.md b/docs/keydb-format.md deleted file mode 100644 index e2b9cee..0000000 --- a/docs/keydb-format.md +++ /dev/null @@ -1,159 +0,0 @@ -# keydb_format.rs — design notes - -## Full parser API retained on purpose - -The parser is copied verbatim, so it carries the full KeyDb/DiscEntry API -even though this crate's consumer (`keydb.rs`) only exercises a subset -(`load`, `find_disc`, `iter_disc_entries`, and the public fields read by -`KeydbSource::unit_keys_from` / `KeydbSource::host_certs`). NOTE: an earlier -revision of this comment cited a `candidates_from` function — no such -function has ever existed in this crate; the real consumer is -`KeydbSource::unit_keys_from` (keydb.rs), which reads `unit_keys`, `vuk`, -`media_key` AND `vid` (`vid` is load-bearing for the MK+VID derivation the -kat_c / kat_d KATs pin — it is NOT unused). `DiscEntry::title` is likewise -read — by `to_keydb_cfg` and the redacting `Debug` impl — so it is not -unused either. The genuinely unused items — `empty` and `find_vuk` — are -part of the faithful copy and are retained rather than pruned; allow -dead_code so the byte-for-byte copy compiles clean without diverging from -the libfreemkv original. - -## ParseStats — what `KeyDb::parse` threw away - -The parser is fed a THIRD-PARTY file: a mirror can serve a truncated or -byte-shifted download that is still valid UTF-8, still contains enough -recognisable rows to pass `KeydbSource::save`'s "at least one entry" check, -and is then atomically committed as the new keydb — after which every disc -past the corruption point silently resolves nothing. Before this counter, -each malformed `| DK` / `| PK` / `| HC` / `| HC2` / `0x…` row was dropped by -an `if let Some(..) = .. { }` with no `else`, no counter and no log, so that -file was indistinguishable from a healthy one at every layer above. - -The counts are logged ONCE per parse (a per-line log would be 181k lines on -a real keydb, which is why the rate limit is "one summary", never "hide the -count"). - -## `KeyDb::load_counted` — same-descriptor stamp + parse - -Two callers need this seam, both in `crate::KeydbSource`: - -* The identity stamp and the bytes must come from the SAME descriptor. - Stat-the-path-then-load-the-path is two independent resolutions of one - name, so an atomic rename landing between them stored a stamp - describing one file next to the parsed contents of another — and the - cache then served that pairing until the stamp changed again. `fstat` - on the open handle cannot be pointed at a different inode. -* The rejection counts have to be RETAINED, not just logged, so a cache - hit can re-emit them. - -## `to_keydb_cfg` — trailing-comment placement - -The trailing `; ` (MKB version / volume size / UHD) is emitted -ONLY after a `U` (unit-keys) field — that is the one place the parser -splits the value on `;`. Gluing a comment onto an `M`/`I`/`V` value would -make `parse_hex16` reject the whole field, so a comment-bearing entry that -has no unit keys drops its comment (keys always survive; the metadata is a -derivable hint). Real per-disc rows that carry metadata also carry keys. - -## `parse_disc_entry` title handling - -Title = everything between `= ` and the first ` | ` field (or the trailing -`;` comment), kept VERBATIM (trimmed). This is a FAITHFUL copy of the keydb -title, so it must round-trip exactly: a previous version extracted a `(...)` -substring as a "display title", but that TRUNCATED real titles that -legitimately contain parentheses ("Lawrence of Arabia (Restored Version) – -Disc 2 …" → "Restored Version") and broke serialize→parse idempotence. -Display prettification, if wanted, belongs in the title-display layer, NOT -this codec. - -## Real-data test notes - -`to_keydb_cfg_is_idempotent_on_real_keydb`: parse the full keydb -> serialize -(S1) -> parse S1 -> serialize again (S2). S1 MUST equal S2 byte-for-byte. -This is the right invariant: a raw keydb.cfg has formatting variance -(whitespace, optional fields, comment style) that our CANONICAL serializer -normalizes, so `text == to_keydb_cfg` is NOT expected — but once normalized, -a re-load+re-serialize must be stable. Idempotence here proves `parse` is -lossless on its own output and `to_keydb_cfg` is deterministic. Also asserts -no rows are dropped. It used to carry no `#[ignore]` and `return` early when -`KEYDB_PATH` was unset — so CI ran it, asserted NOTHING about 181k-entry -behaviour, and reported green. A skip has to be visible in the test output -to mean anything, and an ignored test that is RUN without its fixture must -fail loudly rather than silently pass. - -`real_keydb_path`: the old pattern (`match keydb_path() { Some(p) => p, None -=> return }`) on a non-ignored test is the worst of both worlds: CI ran the -test, asserted nothing, and reported success. These tests are `#[ignore]`d, -so running one at all is an explicit request — and an explicit request with -no fixture must fail, not quietly pass. - -## `load_counted_reads_the_handle_not_the_path` test - -`load_counted` must read the HANDLE it was given, never re-resolve the -path. `KeydbSource::cached_db` stamps the open file with `fstat` and then -parses from that same descriptor precisely so the recorded identity and -the parsed bytes cannot describe two different files; the moment this -function re-opens the path, that guarantee is gone and an atomic rename -landing in between stores one file's stamp beside another file's keys — -a mismatch the cache then serves as if it were valid. - -The race itself cannot be scheduled deterministically in-process, so the -SEAM is what is pinned: with the path already replaced, the handle's -contents must still win. Catches the mutation that restores an internal -`File::open(path)`. - -## `host_cert_debug_is_redacted_including_the_sibling_wrapper` test - -The SIBLING type. `KeyDb`'s hand-written `Debug` prints `host_certs_len`, so -it protects the whole-db rendering and nothing else — `KeyDb::host_certs` is -a public field, and `{:?}` on ONE element of it walks a completely different -code path: `KeydbHostCert`'s derive, then `HostCert`'s impl. That path -carries the AACS host PRIVATE key, the only key material in the crate that -authenticates US rather than opening a disc. - -It is safe today only because `libfreemkv::HostCert` hand-writes a redacting -`Debug` (`aacs::types`, `private_key: ""` + `certificate_len`) — -an EXTERNAL guarantee this crate silently inherits through the derive, with -nothing on this side pinning it. This test pins it: it catches a libfreemkv -upgrade that swaps that impl for `#[derive(Debug)]`, which would leak the -host private key out of this crate's public API with no diff in this -repository at all. - -Rendered three ways because callers reach the cert three ways: the whole -`Vec` (`db.host_certs`), one element, and the inner `HostCert` a caller -gets back from `KeyDb::host_certs(mkb)`. - -## KEYDB-parser integration tests relocated from libfreemkv - -These exercise the parser (`KeyDb::load`) end-to-end against a real -keydb.cfg and feed its material into libfreemkv's AACS crypto -(`derive_vuk`, then `decrypt_unit` + `is_clean_ts`). They live here now -that the parser lives here. All are `KEYDB_PATH`-env-gated and no-op in CI -when the env is unset; they must still COMPILE. - -## `test_decrypt_real_unit` — no-decrypt outcome - -Reaching the fallback path is the EXPECTED outcome for the AACS 2.0 (BEE) -sample this test was written against: the unit is still bus-encrypted, so -no unit key alone can open it. That makes "no key worked" an unusable -assertion on its own — which is exactly why this test used to end in an -`eprintln!` and fall off the end, i.e. pass unconditionally. A no-op -`decrypt_unit`, an `is_clean` stuck at `false`, or a parser that yielded -zero keys would all have produced this same clean exit. - -What CAN be asserted regardless of the sample: the pipeline actually ran, -and the decrypt primitive actually transformed the bytes. - -## Hand-written redacting `Debug` - -Key material must never reach a log, a panic message, or a bug report. Both -`KeyDb` and `DiscEntry` carry raw AACS key bytes, and both are public API, so -a `#[derive(Debug)]` on either is a leak of the crown jewels: `{:?}` on a -loaded `KeyDb` printed every processing key plus every disc's Media Key, -VID, VUK and Unit Keys. libfreemkv's equivalent types (`aacs::types`: -`DeviceKey`, `HostCert`, `Vid`, `MediaKey`, `Vuk`, `ProcessingKey`, -`UnitKey`, its own `DiscEntry`) all hand-write a redacting `Debug` for -exactly this reason; this crate's copy of the parser dropped that -convention when it was relocated. These restore it, field-for-field in the -same style (`` for key bytes, a `_len` for key-bearing -collections). Pinned by `keydb_debug_is_redacted` / -`disc_entry_debug_is_redacted`. diff --git a/docs/keydb.md b/docs/keydb.md deleted file mode 100644 index f05a62a..0000000 --- a/docs/keydb.md +++ /dev/null @@ -1,432 +0,0 @@ -# `keydb.rs` design notes - -Long-form rationale moved out of `src/keydb.rs` doc comments to satisfy the -repo's comment-guard line caps. Each section is pointed to from the file by a -one-line `// See docs/keydb.md#` comment. - -## candidate-order - -Full derivation path this source walks, cheapest-first: - -1. per-disc **Unit Keys** (hash hit) → returned terminal, no derivation. -2. per-disc **VUK** (hash hit) → `uks_from_vuk` over the disc's encrypted - title keys. -3. a **Media Key**, then `derive_vuk` → `uks_from_vuk`. The MK comes from, in - order: the disc's stored MK (hash hit); the keydb's **Processing Key** - pool walked against THIS disc's MKB via `derive_media_key_from_pk`; or the - device-key pool via `derive_media_key_from_dk`. The PK and DK pools - resolve the Media Key WITHOUT a VID; the final `derive_vuk` still needs - one. The VID is the unlocker's physical VID (`ResolveCtx::vid`) when - present, else the keydb entry's OWN stored VID (the `I` field, `vid`) for - the non-physical / ISO path. With no VID from either source the MK path - cannot complete — return nothing. - -The cross-disc MK-pool brute (trying OTHER discs' stored media keys against -this disc) stays RETIRED: every MK path here is anchored to the matched -disc's own MKB or stored material. - -The library still OWNS the crypto; this source owns only which primitive to -call with which material. Returning an empty `Vec` is a genuine "no key for -this disc here". - -## file-identity - -`(len, modified)` ALONE IS NOT AN IDENTITY. mtime is stored at whole-second -resolution on HFS+, on many container overlays and on most network -filesystems, so a writer that replaces `keydb.cfg` within the same second -with a file of the same byte length leaves `(len, modified)` bit-identical — -and the cache then serves the superseded parse for the rest of the process's -life. For a key database that is not a stale page: it is serving retired or -since-corrected keys while reporting success. The writers that hit this are -real and named in this crate's own docs: the daily-refresh job, a second -freemkv process, an operator's editor, a sync tool. - -Two independent discriminators close it: - -1. `dev` + `ino`. Every writer that publishes ATOMICALLY (temp file, fsync, - rename — what `KeydbSource::save` does, and what any correct publisher - does) creates a NEW inode, so the stamp differs immediately and - unconditionally, whatever the clock or the mtime resolution did. Zero on - platforms without them (see `FileStamp::of`), which costs nothing: rule 2 - stands alone. -2. A freshness rule on the cache entry — `CacheEntry::is_settled` — which - covers the residual in-place rewrite that reuses the inode. - -What this still does NOT catch: a writer that FORGES the timestamp, i.e. -restores an mtime equal to the cached one (`touch -t`, `rsync --times`, a -tar/backup restore) into the SAME inode with the SAME length. Nothing short -of hashing the content can see that, and hashing 62 MiB per lookup is -precisely the cost the cache exists to remove (~141 ms × ≥3 loads per rip). -`KeydbSource::save` therefore keeps invalidating the cache directly, so the -one write path this crate owns never depends on any of the above. - -## settle-proof - -Whether a cache entry's stamp can be TRUSTED to change if the file changes. - -The proof, for a filesystem storing mtime at granularity `G`: suppose this -entry was stamped at `stamped_at` with `stamped_at - modified >= G`, and some -writer later rewrites the file in place at wall time `T > stamped_at`. The -recorded mtime `m'` of that write is within one granule of `T`, so -`m' >= T - G > stamped_at - G >= modified`. Hence `m' != modified` and the -stamp DOES change. The ambiguous window is therefore exactly the one where -the cached entry was stamped less than `G` after the file's own mtime — i.e. -we looked at the file while it was still "hot" — and in that window the -entry is refused and the file is re-read. Once the file has been quiet for -`G`, its entry settles and the cache is fully effective again. - -Cost: after every keydb write, lookups re-parse for up to `G` (2 s). The -published keydb is rewritten daily, so that is ~2 s of re-parsing per day -against a cache that saves ~420 ms per rip. - -`modified: None` (a filesystem that reports no mtime) never settles: the only -two discriminators left would be size and inode, so the entry is re-read -every time rather than being trusted on a weaker basis. - -CLOCK CAVEAT: `stamped_at` is the local clock and `modified` is the -filesystem's. On a network share whose server clock runs AHEAD of the -client's by more than `G`, entries never settle (safe: more re-parsing). A -server clock BEHIND the client's makes an entry settle early, which re-opens -the same-second window it closes — the inode rule (1) is what covers the -realistic writers there. - -## keydbsource-cache - -`KeydbSource` caches the parsed database behind the file's identity stamp. -The published keydb is ~62 MiB / ~181k entries and a single AACS-cert rip -parses it repeatedly — `host_certs()` for the drive handshake (freemkv's -`pipe.rs`/`engine.rs`, autorip's `keysource.rs` each build their own source), -then the trait `host_certs(mkb)` during resolution, then `get_unit_keys` — -and every one of those calls used to re-read all 62 MiB from disk and -re-parse it from scratch, because the struct held nothing but a `PathBuf`. - -A keydb replaced underneath this source is picked up on the next call, with -ONE stated exception: a writer that rewrites the file in place, into the -same inode, at the same byte length, having restored the previous mtime -exactly (`touch -t` / `rsync --times` / a backup restore). Everything else — -any atomic rename, any length change, any real clock advance — changes the -stamp. `FileStamp` and `CacheEntry::is_settled` carry the full argument and -the clock caveats; do not weaken either without re-reading them. (The -earlier wording here — "an externally replaced keydb is still picked up" — -was simply false for a same-second, same-length replacement, which is the -common shape of a daily-refresh job on a whole-second-mtime filesystem.) - -MEMORY RESIDENCY (accepted, and deliberately recorded rather than fixed -here): a hit keeps the entire parsed keydb — every disc's Media Key / VID / -VUK / Unit Keys, the processing- and device-key pools, and the host-cert -PRIVATE keys — resident for the process's life, where the pre-cache code -held one parse transiently. Nothing in this crate zeroizes on drop, so that -material was always exposed to a core dump or a swapped page; the cache -widens the WINDOW, it does not create the class. Closing it properly means -zeroize-on-drop across `KeyDb`, `DiscEntry` and libfreemkv's `aacs::types` -(which owns most of the buffers) — a cross-crate change, tracked separately, -NOT smuggled in behind a cache fix. - -## cached-db-one-open - -Errors are the same ones `KeyDb::load` produces, unchanged: a missing file -is `NotFound` (the documented benign case), an over-cap or non-UTF-8 file is -`InvalidData`. The stamp costs an `fstat`, not a 62 MiB read. - -ONE OPEN, ONE IDENTITY. The file is opened once and both the stamp (`fstat` -on that handle) and, on a miss, the bytes come from it. The previous shape — -`std::fs::metadata(path)` and then a separate `KeyDb::load(path)` — resolved -the path TWICE with no atomicity between them, so a rename landing in the -gap cached the new file's stamp beside the old file's contents (or the -reverse); the pairing was then served, looking perfectly valid, until the -stamp changed again. The stamp is read BEFORE the bytes on purpose: a write -racing the read then leaves the OLD stamp cached, which the next call -detects, whereas stamping afterwards would file torn content under a -fresh-looking identity and pin it. - -## cached-db-reemit - -Re-emit the parser's rejection summary on EVERY hit, not just on the parse -that produced it. `KeyDb::parse` is what logs it, and a hit skips `parse` -entirely — so a daemon that cached a corrupt keydb logged its rejected-row -counts exactly once, at whatever moment it first read the file, and then -served that same damaged database to every later rip in silence. Repetition -is the correct failure mode here: the line is emitted only when the file -really does have rejected or duplicate rows, and a warning an operator can -still see on the tenth rip is worth more than one they had to catch on the -first. - -## load-failure - -A MISSING keydb is the documented benign case: the app may simply not have -one, another source may hold the key, and "no keydb" is genuinely "no key -here". Everything else is NOT that: `InvalidData` is a keydb over the -128 MiB cap or one that is not valid UTF-8 — i.e. corrupt, truncated, or a -half-finished download — and a permission/IO failure means the file could -not be read at all. Reporting either as an empty result made a broken keydb -indistinguishable from "this disc has no key" (the same conflation as the -seven-hour 502), with no log to notice it by. Both are now logged AND -surfaced as errors so the resolver reports a source failure instead of -`E7022`. - -## save-mirror-parse - -Mirror `KeyDb::parse`'s disc-entry rule EXACTLY by CALLING it -(`is_disc_entry_line`), so `save()` never validates + persists content that -parses to zero usable entries (e.g. a stray "0xDEADBEEF" comment line) and -the two can no longer drift — they did drift once already, on the `0X` case -rule. - -## unit-keys-from - -CPS-unit numbering: a returned `UnitKey::idx` is the POSITIONAL index -libfreemkv's `resolve_and_apply` turns into the canonical CPS-unit number -`idx + 1`. For the terminal per-disc unit-key path we therefore map the -keydb's stored CPS number `num` to `idx = num - 1`, so the committed number -is byte-identical to the keydb's `num` (and to what the OLD -`Key::Unit(entry.unit_keys)` path committed). For the VUK / MK paths the -boil primitive already yields 0-based positional indices, matching -`parse_unit_key_ro`'s `(i + 1)` after the resolver's `+ 1`. - -## unit-keys-per-disc-hit - -Per-disc hit (most specific). `find_disc` normalizes the hash form. Without -a matched entry this keydb has no per-disc material (Unit Keys / VUK / Media -Key / stored VID) to anchor a derivation for the disc, so it resolves -nothing. (The PK and DK pools are global, but the cross-disc MK-pool brute — -trying OTHER discs' media keys against this disc — stays retired; a PK/DK -pool only ever resolves a disc reached through its own matched entry below.) - -## unit-keys-union - -UNION every source of terminal keys, then dedup — never first-hit. A stored -`unit_keys` list can be PARTIAL (the key-import tool only ever sampled the -CPS units reachable from a playlist, so an orphan unit's key may be -missing), while the per-disc VUK boils EVERY declared CPS unit. Taking the -stored list alone (the old return-at-first-path) would shadow the VUK and -silently drop the orphan unit's key. So gather both and keep a unique-by-key -list: the read path tries every key per unit, so an extra or stale key is -harmless — only a MISSING key hurts. - -## unit-keys-vuk-or-mk - -2. Per-disc VUK — one step, no VID needed; boils ALL declared units. -3. Else a Media Key path (stored MK / PK pool / DK pool) → VUK → all - declared units. The MK itself carries no VID, but the final `vuk_from_mk` - needs one: physical (unlocker) VID first, else the entry's stored VID - (`I` field), else cannot derive. Either branch yields the COMPLETE - declared set, so we take the first that resolves (VUK preferred — - cheapest). - -## write-atomic - -keydb.cfg is the single source of AACS truth, and save/update run -unattended (first-boot download + daily-refresh thread, with a container -restart on every release). A bare in-place `fs::write` truncates the file -before writing, so a SIGKILL (docker stop's grace window), OOM-kill, power -loss, or ENOSPC mid-write would leave the keydb half-written — the prior -good copy already gone. A truncated keydb doesn't error at write time; it -silently breaks key resolution on every later AACS rip. Writing to a temp -file then renaming (POSIX rename is atomic within a filesystem) means an -interrupted update leaves the previous keydb fully intact. - -The fsync MUST succeed before the rename: a `sync_all` failure (ENOSPC, -ESTALE on the bind-mounted volume) means the kernel never guaranteed the -bytes reached stable storage, so publishing them via rename would defeat -crash-safety. The temp name is unique per call (pid + monotonic counter) so -a concurrent update can't share a fixed temp path and rename a mangled file -over the keydb. - -## get-unit-keys-trait - -A MISSING keydb is not an error — it simply yields no keys (another source -may have them), the same as the library's own loader. A keydb that exists -but cannot be USED (over the size cap, not UTF-8, unreadable) is a source -failure and returns `Err`: it says nothing about whether this disc has a -key, and reporting it as an empty result is the same looks-like-success -failure as a 502 reported as `E7022`. - -The keydb carries no AACS 2.1 forensic index keys today, so it does not -override `get_fmts_indexes` — the default (empty) opts it out, and an FMTS -disc's forensic set comes from the online source. - -## host-certs-trait - -Expose the keydb's host certs through the trait — the OEM/AACS cert-auth -route collects them across every source via this method. Wires the disc's -MKB generation through for revocation filtering (the keydb parser's -`; Revoked in MKBv` annotation): a cert revoked at generation `R` is -withheld once the disc's generation reaches `R`. - -## test-pinned-mtime - -Force a file's mtime to `PINNED_MTIME`, so two different files can be made -bit-identical to a `(len, mtime)` stamp on purpose. The cache tests that -matter all depend on FORCING the indistinguishable case rather than hoping -the filesystem produces it: on APFS (nanosecond mtimes) a same-second -replacement is otherwise detected by mtime alone, and a test that relies on -that passes with the fix reverted — which is worth nothing. - -## test-committed - -The committed `(cps, key)` pairs libfreemkv's `resolve_and_apply` derives -from a source's Unit Keys: positional `idx` → canonical CPS number -`idx + 1`. The KATs compare against THIS to prove byte-identical parity with -the OLD `Key::Unit` / resolver-derived commit. - -## test-mock-ctx-stub - -`MockCtx` implements the full `ResolveCtx` trait so it can stand in for a -real disc, but `KeydbSource` itself never calls `title()`/`samples()` (those -matter to the online source, not the local keydb). Exercise them directly -so the mock's trait surface is proven correct too. - -## test-kat-a - -Here `enc_title_keys` is empty, so the VUK can't derive anything — only the -stored list contributes, and it commits byte-identically to the stored -`(cps, key)` pairs. - -## test-orphan-union - -Orphan-unit completeness (the real keydb bug): an entry stores only `uk1` -(the key-import tool sampled one reachable CPS unit) but ALSO carries the -VUK, which boils BOTH declared units. The old return-at-first-path handed -back just `[uk1]`, shadowing the VUK and silently dropping the orphan unit. -The union must return BOTH — the stored uk1 AND the VUK-derived second unit. - -## test-kat-c - -A hash hit with a Media Key and a physical VID (from the unlocker) derives -`MK → VUK → UK`. The PHYSICAL VID must be used in preference to the keydb's -stored VID — proven by giving the entry a DIFFERENT stored VID and showing -the result tracks the physical one. - -## test-kat-f - -Owner decision #1 (AACS): a keydb Processing Key must be walked against the -matched disc's own MKB to recover the Media Key, then driven down the full -chain `PK → MK → VUK → UK`. The disc entry carries NO stored MK/VUK/UK — the -only key material is a global `PK` row — and the result must be the real -Unit Keys, byte-identical to deriving from the recovered MK. - -The MKB + PK use a known-answer construction (a planted PK whose derived MK -satisfies the synthetic verify record); the constants are precomputed AES -vectors so this crate needs no AES primitive of its own. They mirror -libfreemkv's `boil::mk_from_pk_drives_full_chain_to_uks` KAT. - -## test-repeated-lookups - -A single AACS-cert rip drives this source at least three times — the -scan-options builder's inherent `host_certs()`, the trait `host_certs(mkb)` -during the handshake, then `get_unit_keys` — and each call used to re-read -and re-parse the WHOLE file, because `KeydbSource` held nothing but a -`PathBuf`. Measured on the real 62 MiB / 184,860-entry keydb: ~140 ms per -parse in `--release`, i.e. ~420 ms and ~186 MiB of file reads per rip, -against ~1 µs for a cache hit. - -Catches the mutation that drops the cache (every call re-parses: count 3) -and the mutation that makes the cache unconditional (see the invalidation -tests below). - -The mtime is pinned into the PAST first, because a cache entry is only -trusted once the file has been quiet for the mtime granularity -(`CacheEntry::is_settled`) — a keydb written microseconds ago is -deliberately re-read. Unix-only: pinning an mtime needs `touch -t` (no -`filetime` dependency for a test). - -## test-changed-keydb - -The cache must not go stale: a keydb replaced UNDERNEATH the source (the -daily-refresh thread writing through a different `KeydbSource`, or an -operator dropping in a new file) has a different size/mtime stamp and must -be re-read. Catches the mutation that caches on first load and never -re-stats — which would pin a rip to a keydb deleted hours earlier. - -## test-two-discriminators - -The defect both of the inode/settle tests below pin: `(len, mtime)` is not a -file identity. An external writer — the daily-refresh job, a second freemkv -process, an editor, a sync tool — that replaces `keydb.cfg` inside one mtime -granule with a file of the SAME length leaves that pair bit-identical, and -the source then serves the superseded parse for the rest of the process's -life: retired or since-corrected keys, reported as success. - -Each test disables the OTHER discriminator through `mtime_granularity`, so -neither can pass on the strength of the one it is not testing, and neither -depends on the host filesystem's real mtime resolution (on APFS the naive -scenario is not reproducible at all). - -## test-inode-rename - -A keydb replaced by ATOMIC RENAME — how every correct publisher, including -this crate's own `save()`, installs a new file — must be re-read even when -length and mtime are identical. `mtime_granularity: 0` makes every entry -settle, so ONLY dev+ino can distinguish the two files here. - -Catches the mutation that drops `dev`/`ino` from `FileStamp` (the original -`(len, modified)` stamp): the source then keeps answering with the previous -keydb's keys. - -## test-inplace-rewrite - -A keydb rewritten IN PLACE — same inode, same length, mtime forced back to -the same value — must still be re-read while the cached entry is too young -to trust. `mtime_granularity` is set to a decade so nothing ever settles, -which is the same condition a real same-second, whole-second-mtime -replacement creates and the only way to reproduce it deterministically on a -nanosecond-mtime filesystem. - -Catches the mutation that deletes the `is_settled` check from `cached_db` -(every stamp match trusted unconditionally, which is what the original code -did): dev, ino, len and mtime are ALL identical here, so without the -freshness rule the source serves the old keys forever. - -## test-corrupt-warns - -`ParseStats::log` runs inside `KeyDb::parse`, and a cache hit skips `parse` -— so once the cache landed, the rejected-row summary for a given file was -emitted exactly ONCE for the entire life of the process, while that same -damaged file went on being served to every subsequent rip. A long-running -daemon (autorip) logs it at startup, the operator misses it in the startup -noise or loses it to log rotation, and there is no second chance for as long -as the process runs. - -Catches the mutation that deletes the `emit_parse_stats` call from the -cache-HIT arm of `cached_db`: warnings would drop to 1 while the file is -still served three times. - -## test-healthy-silent - -Catches the mutation that makes `emit_parse_stats` unconditional (drops -`ParseStats::log`'s empty-counts early return). - -## test-fresh-not-trusted - -Catches the mutation that widens the window to something unbounded (a -granularity of hours would re-parse 62 MiB on every lookup all day) by -pairing with `repeated_lookups_parse_the_keydb_once`, which proves the entry -DOES settle once the mtime is in the past. - -## test-save-invalidates - -`save()` replaces the very file this source reads, so it drops the cached -parse ITSELF rather than trusting the (size, mtime) stamp: a same-size -rewrite that lands inside the filesystem's timestamp granularity is the one -change the stamp cannot see — and it is exactly the change this crate -performs (the daily-refresh thread re-saving a keydb through the source it -then reads from). - -The test forces that indistinguishable case rather than hoping for it: the -file is written, stamped to a FIXED mtime, cached, re-saved with different -keys of the SAME length, and stamped back to the same mtime. To the stamp -the two files are identical, so only the explicit invalidation can produce -the new key. Unix-only because pinning an mtime needs `touch -t` (no -`filetime` dependency for one test). - -## test-corrupt-is-error - -`KeyDb::load` fails distinguishably for a keydb that is over the 128 MiB cap -or not valid UTF-8 — a truncated download, a half-written file, a binary -blob dropped in by mistake. Both used to collapse into `Ok(Vec::new())` with -ZERO tracing, i.e. into the benign "no key for this disc" the resolver -reports as `E7022` — the same looks-like-success failure as the seven-hour -502. Only a MISSING file is genuinely benign. - -Catches the mutation that restores `Err(_) => Ok(Vec::new())`. - diff --git a/docs/lib-scope.md b/docs/lib-scope.md deleted file mode 100644 index 9fda397..0000000 --- a/docs/lib-scope.md +++ /dev/null @@ -1,49 +0,0 @@ -# freemkv-keysources: scope and design notes - -## Mechanism vs. policy - -Applications (autorip, the `freemkv` CLI) choose and order the key sources -from their own config — the local-vs-online policy is just which impls they -plug in — then resolve and hand the resulting key to `Disc::decrypt_with`. - -Each source resolves a disc's terminal **Unit Keys** in one shot via -`KeySource::get_unit_keys`, driving libfreemkv's boil-down crypto primitives -for whatever level of material it holds. Compose several with `MultiSource` -in the caller's chosen order. - -Reading the encrypted content-sample units a key server validates on, and -applying the resolved keys against a disc, is decryption *mechanism* — it -lives in the library (`libfreemkv::resolve_and_apply`, -`libfreemkv::read_encrypted_units`), not here. - -## `MultiSource` failure semantics - -`MultiSource::get_unit_keys` tries each inner source in order and returns the -first non-empty Unit Key set (and `get_fmts_indexes` does the same for the -forensic set). The caller supplies the list AND the order — local-first -`[Keydb, Online]`, online-first `[Online, Keydb]`, etc. — so the "which -sources, in what order" policy lives entirely with the application, not the -library. `MultiSource` is itself a `KeySource`, so it nests and composes. - -**A source that could not ANSWER is not a source that answered "no key".** -When no inner source produces keys, the composition returns `Err` if any -inner source failed, and `Ok(empty)` only when every source genuinely -answered and none held a key. This mirrors libfreemkv's own resolver -(`keysource::drive_unit_keys`, which tracks an `errored` flag, and -`source_failure`, which stamps the first failure onto the disc) — a -composition that swallowed the `Err` would hand the resolver a clean -`Ok(empty)`, and an outage would once again be reported to the operator as -`E7022 No key source has a decryption key for this disc`. - -## `first_non_empty` ordering rule - -A source failure never blocks the chain (a later source is still tried, and -a later success still wins outright, error or no error), but it is not -forgotten either: with no keys anywhere, the FIRST failure is returned. -First (not last) matches libfreemkv's `source_failure` rule, so the -most-preferred source's reason is the one the operator is told about, in the -same order the caller expressed a preference for successes. - -`get` selects which trait method to drive, so the base and forensic paths -share ONE implementation of this rule and cannot drift apart — the forensic -half carried the identical bug. diff --git a/docs/multi-source-failure-collapse.md b/docs/multi-source-failure-collapse.md deleted file mode 100644 index 4f3f2aa..0000000 --- a/docs/multi-source-failure-collapse.md +++ /dev/null @@ -1,13 +0,0 @@ -# `MultiSource`: a source that could not answer is not a source that said no - -`MultiSource` used to re-collapse the Ok/Err distinction `OnlineSource` was -engineered to draw: `if let Ok(uks) = s.get_unit_keys(ctx)` discarded every -`Err`, so a composition whose sources ALL failed returned `Ok(Vec::new())` — -which the resolver reports as `E7022 No key source has a decryption key for -this disc`. That is the seven-hour-502 incident reproduced one layer up. - -The regression test `multi_source_reports_a_source_failure_instead_of_a_clean_no_key` -is THE regression check, one layer up from `interpret_reply`: when no source -holds a key AND a source could not answer, the composition must report the -FAILURE, never a clean "no key". It catches the mutation that restores -`if let Ok(uks) = ..` (which drops the `Err` and returns `Ok(empty)`). diff --git a/docs/online-agent-cache.md b/docs/online-agent-cache.md deleted file mode 100644 index 3a13ec7..0000000 --- a/docs/online-agent-cache.md +++ /dev/null @@ -1,39 +0,0 @@ -# `src/online.rs` — `OnlineSource::agent` caching - -## Why cache at all - -`query` used to build a FRESH `ureq::Agent` every call, and an FMTS disc -makes two calls per rip (`get_unit_keys` + `get_fmts_indexes`), so it paid -two TLS handshakes to the same host for no reason — an agent owns the -connection pool, and a discarded agent discards the pooled, already -negotiated TLS connection with it. - -## Security invariant - -The agent is reused ONLY when the freshly resolved + guarded address SET is -identical to the one the cached agent is pinned to. Every query still -re-resolves and re-runs the SSRF guard — that is the whole anti-rebinding -defence and is deliberately not cached; what is reused is the connection, -and only to addresses just re-validated this call. - -## Why the cache key is a sorted, deduped SET, not an ordered sequence - -`to_socket_addrs` hands back whatever order the resolver felt like, and a -round-robin keyserver reorders its A/AAAA records on essentially every -lookup — an ordered `==` therefore missed the cache every single time -against exactly the deployment that most needs it, silently degrading to -the per-query TLS handshake this field exists to avoid. - -Set equality is also the honest predicate: the security property being -asserted is "these are the same validated addresses", which says nothing -about their order. The agent itself stays pinned to the order it was BUILT -with; that is safe because an equal set means every address it holds was -re-resolved and re-guarded this call. - -## `a_reordered_but_identical_address_set_reuses_the_agent` - -Catches the mutation that drops the sort/dedup from `agent_for`'s key (the -reversed order would then build a second agent) and, via the second half of -the test, the mutation that "fixes" order-sensitivity by comparing only -lengths or the first element — which would wrongly reuse an agent pinned to -a DIFFERENT address. diff --git a/docs/online-guardfail.md b/docs/online-guardfail.md deleted file mode 100644 index 3316785..0000000 --- a/docs/online-guardfail.md +++ /dev/null @@ -1,41 +0,0 @@ -# `src/online.rs` — `GuardFail` classification details - -## Malformed-port authority - -A `host:port` authority whose port is not a u16 is a TYPO in the operator's -config — `https://example.com:notaport/keys`. This used to fall back to the -WHOLE authority as the hostname (port text included), which of course never -resolves, so the URL surfaced as `GuardFail::Unreachable` → -`Err(KeyServiceUnavailable)`: a standing misconfiguration reported as a -transient outage, the exact collapse `GuardFail`'s own doc says must not -happen, because the two halves demand OPPOSITE operator actions (fix the URL -vs. wait). - -Reject it as `Config`, identical to the bracketed-IPv6 branch a few lines -above, which already returns `Config` for an unparseable port. Silently -substituting `default_port` would be worse still — it would ship the bearer -token and key material to a port the operator never configured. - -## Guard-blocked address (`Config` arm, `resolve_and_guard`) - -Log THAT the URL was rejected, never WHY: the guard's message names the -resolved address, and an internal address must not reach a log an operator -may paste into a bug report. The static label is enough to separate -"misconfigured/blocked key-service URL" from "this disc has no key", which -is the only distinction the operator needs here. - -`error!`, not `warn!`, and `Err`, not `Ok(empty)`. This arm used to return -`Ok(Vec::new())`, which made the PERMANENT fault quieter than the transient -one directly above it: the service was never asked, yet the composition was -handed a clean empty and — with no other source holding the key — told the -operator `E7022 no key for this disc`. Same lie as the seven-hour 502, from -a config typo that never self-heals. A failure to ask is never evidence -about the disc, so it leaves as `Err`; `MultiSource::first_non_empty` still -lets a later source's real key win outright, so this cannot fail a rip that -the local keydb could have served. - -NOTE (reported upstream): libfreemkv has no `KeyServiceMisconfigured` code, -so the permanent fault borrows the transient E7028. E7028's contract — "the -source never got as far as answering the question" — is exactly true here; -only its "transient, retry later" hint is wrong, and the `error!` log -carries the correcting detail until a 70xx config code exists. diff --git a/docs/online-preflight-guard-tests.md b/docs/online-preflight-guard-tests.md deleted file mode 100644 index 8b0a8fc..0000000 --- a/docs/online-preflight-guard-tests.md +++ /dev/null @@ -1,20 +0,0 @@ -# `src/online.rs` tests — pre-flight guards in `query` - -Each of the three guards (cleartext scheme, over-cap MKB, under-sampled) -fires BEFORE any address is resolved and before anything is sent. None was -exercised before these tests: a reordering that let the cleartext POST -through would have shipped green, and that POST carries the bearer token -plus base64 key material. - -Their VERDICTS differ on purpose (see docs/online-query-guards.md): the -cleartext-scheme guard is an operator fault and yields `Err`, while the two -input-shaped guards yield `Ok(empty)` — this source has nothing for THIS -disc, permanently, and calling that an outage would send the operator -waiting for nothing. - -The discriminator in all three: the configured host is `.test`, which RFC -6761 guarantees never resolves. On the guarded path nothing resolves it, so -the test touches no network and returns `Ok(empty)`. Remove the guard and -control reaches `resolve_and_guard`, which fails as `Unreachable` -> -`Err(KeyServiceUnavailable)` — a different Result, so the mutation cannot -pass. diff --git a/docs/online-query-contract.md b/docs/online-query-contract.md deleted file mode 100644 index e8d28f1..0000000 --- a/docs/online-query-contract.md +++ /dev/null @@ -1,19 +0,0 @@ -# `src/online.rs` — `OnlineSource::query` return contract - -The return type draws THE distinction this source exists to draw: - -* `Ok(non-empty)` — the service answered with a key. -* `Ok(empty)` — the service ANSWERED and holds no key for this disc, or this - source had nothing to ask with (no service configured, a misconfigured - URL, an over-cap MKB, too few samples). A genuine miss; the resolver moves - to the next source and `E7022` is the right verdict. -* `Err(..)` — the service could not answer: unreachable, timed out, DNS - failed, returned 5xx, rejected the token (401/403), rate-limited (429), or - replied with something unreadable. NOTHING is known about whether a key - exists. Collapsing this into `Ok(empty)` is the bug this signature fixes: - a seven-hour run of HTTP 502s was reported to operators as - `E7022 No key source has a decryption key for this disc`, and they went - hunting for a VUK when the correct action was to wait. - -`&self`: one-shot is the resolver's contract (each source's -`get_unit_keys` is called once), so no per-call latch is needed. diff --git a/docs/online-query-guards.md b/docs/online-query-guards.md deleted file mode 100644 index 0ffc8df..0000000 --- a/docs/online-query-guards.md +++ /dev/null @@ -1,57 +0,0 @@ -# `src/online.rs` — `OnlineSource::query` pre-flight guards - -## Refusing cleartext (`http://`) - -The request body always carries base64 AACS key material -(inf/mkb/units/vid) and, when configured, a bearer token in a header — so an -`http://` POST hands both to any on-path observer, and the token is -replayable. - -There is no legitimate plaintext deployment to preserve here: the address -guard rejects loopback, RFC1918, ULA and link-local, so an `http://` URL can -only ever reach the PUBLIC internet — the single worst case for cleartext. A -self-hosted keyserver on a LAN is already impossible by that guard, whatever -its scheme. - -Still falls THROUGH to the next key source (a local keydb) — an `Err` here -does not block the chain, because `MultiSource::first_non_empty` returns any -later source's non-empty key set outright and only surfaces the failure -when nothing anywhere resolved. What it does stop is the silent case: a -cleartext URL is the same permanent operator fault as the address-guard -rejection (see `GuardFail`), so it gets the same verdict. Returning -`Ok(empty)` here made "we refused to ask" indistinguishable from "the -service answered and has no key". - -## The three-way skip/fault/miss line (MKB cap, sample count) - -Written out after the `Config`-was-quieter-than-`Unreachable` bug, in three -parts: - -* NO URL AT ALL is not a fault: the operator did not configure an online - source, so there is nothing to report. `Ok(empty)`, silently (the first - guard in `query`). -* A bad URL — wrong scheme, mistyped port, guard-blocked address — is an - OPERATOR FAULT. Every disc is affected, nothing was asked, and the fix is - to edit the config. That is `Err`. -* An over-cap MKB or too few content samples is a property of THIS DISC's - inputs, not a fault: the request cannot be *formed* for this disc, so this - source has nothing for it and never will, whatever the operator does and - however long they wait. Reporting that as `KeyServiceUnavailable` would - send the operator waiting out an outage that does not exist — the same - mislabelling in the opposite direction. These stay `Ok(empty)`, and stay - logged. - -Logging on the MKB-cap skip matters: a silent empty return there is -indistinguishable from "no key", so the real cause is surfaced (the cap is -64 MiB, far above any real trimmed MKB). - -## `cleartext_http_url_is_refused_before_anything_is_sent` - -Catches two mutations: sending the request anyway (the host is `.test` and -would surface as `Unreachable`, but the assertion would still hold, which is -why `too_few_samples_skips_the_request` guards the send-path ordering -separately) and — the one this test was rewritten for — returning -`Ok(Vec::new())`, which made a permanent cleartext misconfiguration -indistinguishable from a genuine miss. A later source's real key still wins -(`MultiSource::first_non_empty`), so refusing does not fail a rip the local -keydb can serve. diff --git a/docs/online-ssrf-guard.md b/docs/online-ssrf-guard.md deleted file mode 100644 index 396b23a..0000000 --- a/docs/online-ssrf-guard.md +++ /dev/null @@ -1,44 +0,0 @@ -# `src/online.rs` — SSRF guard - -## Threat model - -The keyserver URL is operator-supplied and the bearer token is confidential. -After `validate_keyserver_url` checked the host at config time, an attacker -who controls the keyserver's DNS can rebind it to 169.254.169.254 (cloud -metadata) or an RFC1918 host in the window between validation and the actual -POST, exfiltrating the key material and the Authorization token. - -## Defence - -Resolve the host once just before the POST, reject any blocked IP, and pin -the ureq connection to those validated addresses so a subsequent DNS flip -cannot redirect the request. Use `redirects(0)` so a public URL can't -30x-redirect to an internal host. - -## Resolver wiring pitfall (`PinnedResolver`) - -ureq 3 replaced v2's resolver closure with the `Resolver` trait. The agent -MUST be built through `Agent::with_parts` to take a custom one: -`Agent::new_with_config` silently keeps the default resolver, which would -send the request to live DNS and quietly reopen the rebinding window this -module exists to close. That failure is invisible — it has no symptom short -of an actual attack — so it is pinned by the test -`hardened_agent_connects_to_the_pinned_address_not_dns`. - -## Why that test is structured the way it is - -`resolve_and_guard` VALIDATES addresses; `hardened_agent` PINS them, so a -hostname cannot re-resolve to an internal address between validation and -connection. The validation half is well covered by other tests — but every -one of those still passes if the pinned resolver is never consulted, because -none of them makes a connection. Nothing else in the file proves the agent -honours the pin, and a mis-wired resolver fails OPEN: the connection quietly -falls back to live DNS and the rebinding window this module exists to close -is reopened, with no visible symptom. - -The discriminator: pin the agent to a loopback listener the test owns, then -ask it for a host that CANNOT resolve — `.test` is reserved by RFC 6761 and -never resolves in the public DNS. Only a consulted resolver can turn that -name into a connection; a fallback to live DNS fails instead. No network is -touched, and `query` (whose guard blocks loopback by design) is not -involved — this drives `hardened_agent` directly. diff --git a/docs/paths.md b/docs/paths.md deleted file mode 100644 index b7eabe1..0000000 --- a/docs/paths.md +++ /dev/null @@ -1,13 +0,0 @@ -# `keydb.cfg` path policy (`src/paths.rs`) - -Key-path policy belongs with the key sources (this crate), not the library: -libfreemkv is handed a path and reads it. The CLI/app asks here for the -list of locations to *search* (first existing wins) and for the single -*default* location to write to (e.g. `update-keys`/save). - -freemkv is a portable, standalone binary: the `keydb.cfg` lives *next to* -the executable — `/keydb.cfg` — and nowhere else. There -is no OS-specific config-dir lookup (`%APPDATA%`, `%USERPROFILE%\.config`, -`$XDG_CONFIG_HOME`, `$HOME/.config`). Drop the exe and its `keydb.cfg` in -the same folder and it works. Callers needing a custom location pass -`--keydb PATH`, which bypasses this module entirely. diff --git a/src/keydb.rs b/src/keydb.rs index 4fde142..6af4633 100644 --- a/src/keydb.rs +++ b/src/keydb.rs @@ -1,12 +1,10 @@ //! `keydb.cfg` key source (source #1). //! -//! Parses a local `keydb.cfg`, looks the disc up by hash, and derives the -//! disc's terminal **Unit Keys** by composing libfreemkv's raw -//! `aacs::derive` primitives — never re-implementing AES. The path mirrors -//! the OLD candidate order EXACTLY, cheapest-first: per-disc Unit Keys, then -//! VUK, then a Media Key (stored / PK pool / DK pool) via `derive_vuk`. See -//! docs/keydb.md#candidate-order for the full path and VID rules. The -//! library owns the crypto; this source owns only which primitive to call. +//! Parses a local `keydb.cfg`, looks the disc up by hash, and derives the disc's terminal +//! **Unit Keys** by composing libfreemkv's raw `aacs::derive` primitives — never +//! re-implementing AES. The path mirrors the OLD candidate order EXACTLY, cheapest-first: +//! per-disc Unit Keys, then VUK, then a Media Key (stored / PK pool / DK pool) via +//! `derive_vuk`. use std::io::{Read, Write}; use std::path::{Path, PathBuf}; @@ -34,14 +32,12 @@ pub struct UpdateResult { pub bytes: usize, } -// Widest observed granularity of a filesystem's stored mtime (HFS+, many -// container/network filesystems record whole seconds); 2 s leaves rounding -// room. See docs/keydb.md#settle-proof for CacheEntry::is_settled's argument. +// Widest observed granularity of a filesystem's stored mtime (HFS+, many container/network +// filesystems record whole seconds); 2 s leaves rounding room. const MTIME_GRANULARITY: std::time::Duration = std::time::Duration::from_secs(2); -// The identity of the keydb file a cache entry was parsed from. `(len, -// modified)` alone is not an identity (see docs/keydb.md#file-identity for -// why); `dev`+`ino` plus CacheEntry::is_settled close the gap. +// The identity of the keydb file a cache entry was parsed from. `(len, modified)` alone is not +// an identity; `dev`+`ino` plus CacheEntry::is_settled close the gap. #[derive(Debug, Clone, Copy, PartialEq, Eq)] struct FileStamp { /// Filesystem device id. `0` where unavailable. @@ -92,9 +88,9 @@ struct CacheEntry { } impl CacheEntry { - // Whether this entry's stamp can be TRUSTED to change if the file changes. - // Proof, cost, and the clock caveat are in docs/keydb.md#settle-proof; - // don't weaken this without re-reading them. + // Require a settled identity before reusing a cached parse: coarse mtimes can hide + // in-place edits within one timestamp tick. Missing/future mtimes use the age of + // the observation; inode identity detects atomic replacement. fn is_settled(&self, granularity: std::time::Duration) -> bool { match self .stamp @@ -117,13 +113,10 @@ impl CacheEntry { /// A [`KeySource`] backed by a local `keydb.cfg` file. /// -/// The parsed database is CACHED behind the file's identity stamp: a single -/// AACS-cert rip calls `host_certs()`, the trait `host_certs(mkb)`, and -/// `get_unit_keys`, and each used to re-read + re-parse the whole -/// ~62 MiB file. A replaced keydb is picked up on the next call, with one -/// narrow exception; see docs/keydb.md#keydbsource-cache for that exception -/// and the memory-residency tradeoff — do not weaken [`FileStamp`] or -/// [`CacheEntry::is_settled`] without re-reading it. +/// The parsed database is CACHED behind the file's identity stamp: a single AACS-cert rip calls +/// `host_certs()`, the trait `host_certs(mkb)`, and `get_unit_keys`, and each used to re-read + +/// re-parse the whole ~62 MiB file. A replaced keydb is picked up on the next call, with one +/// narrow exception. pub struct KeydbSource { path: PathBuf, // Mutex, not RwLock: the guarded section is a stamp compare + Arc clone, @@ -134,9 +127,8 @@ pub struct KeydbSource { // cache from a test, since timing is flaky and the parsed value is // identical either way. parses: AtomicUsize, - // The `G` of CacheEntry::is_settled, as a field so tests can isolate the - // two staleness discriminators one at a time (0 vs. a huge duration). See - // docs/keydb.md#test-two-discriminators. + // The `G` of CacheEntry::is_settled, as a field so tests can isolate the two staleness + // discriminators one at a time (0 vs. a huge duration). mtime_granularity: std::time::Duration, // Corruption summaries emitted (see emit_parse_stats). Same rationale as // `parses`: avoids pulling a `tracing` subscriber into dev-dependencies. @@ -179,9 +171,8 @@ impl KeydbSource { self } - // The parsed keydb, from cache when unchanged (errors mirror KeyDb::load). - // ONE OPEN, ONE IDENTITY: stamp (fstat) and bytes share one handle. See - // docs/keydb.md#cached-db-one-open for why a separate metadata()+load() pair was unsound. + // The parsed keydb, from cache when unchanged (errors mirror KeyDb::load). ONE OPEN, ONE + // IDENTITY: stamp (fstat) and bytes share one handle. fn cached_db(&self) -> std::io::Result> { let file = std::fs::File::open(&self.path)?; let stamp = FileStamp::of(&file.metadata()?); @@ -239,9 +230,9 @@ impl KeydbSource { self.parses.load(Ordering::Relaxed) } - // Turn a KeyDb::load failure into this source's verdict: MISSING is the - // documented benign case (Ok/None); everything else is logged and - // surfaced as an error, not silently reported as "no key". See docs/keydb.md#load-failure. + // Turn a KeyDb::load failure into this source's verdict: MISSING is the documented benign + // case (Ok/None); everything else is logged and surfaced as an error, not silently reported + // as "no key". fn load_failure(&self, e: &std::io::Error) -> Option { match e.kind() { std::io::ErrorKind::NotFound => None, @@ -291,8 +282,8 @@ impl KeydbSource { let entries = text .lines() // Mirror KeyDb::parse by running the same parsers, not a prefix check: a - // right-prefix row with bad hex / short cert parses to nothing, so a prefix - // count let junk past `entries > 0`. See docs/keydb.md#save-mirror-parse. + // right-prefix row with bad hex / short cert parses to nothing, so a prefix count + // let junk past `entries > 0`. .filter(|l| crate::keydb_format::is_parseable_entry_line(l)) .count(); @@ -349,9 +340,8 @@ impl KeydbSource { } } - // Terminal Unit Keys for this disc from a parsed keydb; empty Vec = none. - // CPS numbering (idx = num - 1): docs/keydb.md#unit-keys-from. Thin wrapper - // over [`resolve_from`] for callers that want only the keys (pure, no I/O). + // Return terminal Unit Keys from a parsed keydb without I/O. + // CPS unit numbers are one-based; the returned vector uses zero-based indexes. fn unit_keys_from(db: &KeyDb, ctx: &dyn ResolveCtx) -> Vec { Self::resolve_from(db, ctx).keys } @@ -360,17 +350,15 @@ impl KeydbSource { // keyless match — WHY nothing derived, so a matched-but-underivable disc is // never reported as a flat "no entry" (issue #46). Pure (no I/O). fn resolve_from(db: &KeyDb, ctx: &dyn ResolveCtx) -> KeydbResolution { - // Per-disc hit (most specific); find_disc normalizes the hash form. - // Without a match there is no per-disc anchor, so the global PK/DK - // pools are never consulted. See docs/keydb.md#unit-keys-per-disc-hit. + // Per-disc hit (most specific); find_disc normalizes the hash form. Without a match + // there is no per-disc anchor, so the global PK/DK pools are never consulted. let entries_loaded = db.disc_entries.len(); let Some(entry) = db.find_disc(ctx.disc_hash()) else { return KeydbResolution::miss(entries_loaded); }; - // UNION every source of terminal keys, then dedup — never first-hit, - // since a stored `unit_keys` list can be PARTIAL while the VUK boils - // every declared unit. See docs/keydb.md#unit-keys-union. + // UNION every source of terminal keys, then dedup — never first-hit, since a stored + // `unit_keys` list can be PARTIAL while the VUK boils every declared unit. let mut keys: Vec = Vec::new(); // 1. Terminal Unit Keys stored in the entry — directly usable, no @@ -406,8 +394,7 @@ impl KeydbSource { let mut miss_reason: Option = None; if !enc_title_keys.is_empty() { - // VUK path, else MK path (stored/PK/DK) → VUK. See - // docs/keydb.md#unit-keys-vuk-or-mk for the VID rules. + // VUK path, else MK path (stored/PK/DK) → VUK. let derived = if let Some(vuk) = entry.vuk { uks_from_vuk(&vuk, enc_title_keys) } else { @@ -545,9 +532,8 @@ fn extract_zip(data: &[u8]) -> Result { Err(Error::KeydbInvalid) } -// Write `text` to `path` crash-safely (temp file, fsync, atomic rename) so an -// interrupted update never leaves a half-written keydb. See -// docs/keydb.md#write-atomic for the fsync-before-rename argument. +// Write `text` to `path` crash-safely (temp file, fsync, atomic rename) so an interrupted +// update never leaves a half-written keydb. fn write_atomic(path: &Path, text: &str) -> Result<(), Error> { let werr = || Error::KeydbWrite { path: path.display().to_string(), @@ -607,9 +593,8 @@ fn write_atomic(path: &Path, text: &str) -> Result<(), Error> { } impl KeySource for KeydbSource { - // Resolve this disc's base per-CPS-unit Unit Keys from the keydb. A - // MISSING keydb yields no keys (Ok(empty)); an UNUSABLE one is a source - // failure (Err) — never conflated. See docs/keydb.md#get-unit-keys-trait. + // Resolve this disc's base per-CPS-unit Unit Keys from the keydb. A MISSING keydb yields no + // keys (Ok(empty)); an UNUSABLE one is a source failure (Err) — never conflated. fn get_unit_keys(&self, ctx: &dyn ResolveCtx) -> Result, Error> { match self.cached_db() { Ok(db) => Ok(Self::unit_keys_from(&db, ctx)), @@ -646,9 +631,8 @@ impl KeySource for KeydbSource { } } - // Expose the keydb's host certs through the trait, wiring the disc's MKB - // generation through for revocation filtering. See - // docs/keydb.md#host-certs-trait. + // Expose the keydb's host certs through the trait, wiring the disc's MKB generation through + // for revocation filtering. fn host_certs(&self, mkb: Option) -> Vec { match self.cached_db() { Ok(db) => db.host_certs(mkb), @@ -674,9 +658,8 @@ mod tests { #[cfg(unix)] const PINNED_MTIME: &str = "202601011200.00"; - // Force a file's mtime to PINNED_MTIME so two different files can be - // made bit-identical to a (len, mtime) stamp on purpose, rather than - // hoping the filesystem produces that case. See docs/keydb.md#test-pinned-mtime. + // Force a file's mtime to PINNED_MTIME so two different files can be made bit-identical to + // a (len, mtime) stamp on purpose, rather than hoping the filesystem produces that case. #[cfg(unix)] fn pin_mtime(p: &std::path::Path) { let ok = std::process::Command::new("touch") @@ -765,9 +748,8 @@ mod tests { } } - // The committed (cps, key) pairs resolve_and_apply derives from a - // source's Unit Keys (idx -> idx + 1). KATs compare against THIS. See - // docs/keydb.md#test-committed. + // The committed (cps, key) pairs resolve_and_apply derives from a source's Unit Keys (idx + // -> idx + 1). KATs compare against THIS. fn committed(uks: &[UnitKey]) -> Vec<(u32, [u8; 16])> { uks.iter() .map(|u| (u.idx.saturating_add(1), u.key)) @@ -854,9 +836,8 @@ mod tests { assert_eq!(shape.unit_keys_len, 1); } - // KAT (a): disc with terminal Unit Keys, no enc_title_keys. Stored - // terminal unit keys are returned with CPS numbering preserved. See - // docs/keydb.md#test-kat-a. + // KAT (a): disc with terminal Unit Keys, no enc_title_keys. Stored terminal unit keys are + // returned with CPS numbering preserved. #[test] fn kat_a_disc_with_unit_keys_is_terminal_and_preserves_cps_numbering() { let mut e = blank_entry(HASH); @@ -889,9 +870,8 @@ mod tests { assert_eq!(committed(&got), vec![(1u32, [0xA0u8; 16])]); } - // Orphan-unit completeness (the real keydb bug): a PARTIAL stored list - // plus a VUK that boils BOTH declared units must return BOTH, not shadow - // the VUK with the partial list. See docs/keydb.md#test-orphan-union. + // Orphan-unit completeness (the real keydb bug): a PARTIAL stored list plus a VUK that + // boils BOTH declared units must return BOTH, not shadow the VUK with the partial list. #[test] fn union_partial_stored_plus_vuk_yields_all_declared_units() { let vuk = [0x5Au8; 16]; @@ -945,9 +925,8 @@ mod tests { ); } - // KAT (c): MK + physical (unlock) VID derives MK -> VUK -> UK. The - // PHYSICAL VID must win over the keydb's stored VID. See - // docs/keydb.md#test-kat-c. + // KAT (c): MK + physical (unlock) VID derives MK -> VUK -> UK. The PHYSICAL VID must win + // over the keydb's stored VID. #[test] fn kat_c_disc_with_mk_uses_physical_vid_over_keydb_vid() { let mk = [0x77u8; 16]; @@ -1031,9 +1010,8 @@ mod tests { rec } - // KAT (f): disc with NO per-disc MK, resolved via the keydb PK pool - // (PK -> MK -> VUK -> UK) with a known-answer MKB/PK construction. See - // docs/keydb.md#test-kat-f. + // KAT (f): disc with NO per-disc MK, resolved via the keydb PK pool (PK -> MK -> VUK -> UK) + // with a known-answer MKB/PK construction. #[test] fn kat_f_disc_with_pk_pool_yields_uks() { // Planted PK and the MK it resolves to (see libfreemkv boil.rs KAT). @@ -1246,9 +1224,9 @@ mod tests { std::fs::remove_dir_all(&dir).ok(); } - // Caching: the 62 MiB / 181k-entry keydb is parsed ONCE. A single - // AACS-cert rip drives this source at least three times; the mtime is - // pinned into the PAST first, since an entry is only trusted once settled. See docs/keydb.md#test-repeated-lookups. + // Caching: the 62 MiB / 181k-entry keydb is parsed ONCE. A single AACS-cert rip drives this + // source at least three times; the mtime is pinned into the PAST first, since an entry is + // only trusted once settled. #[cfg(unix)] #[test] fn repeated_lookups_parse_the_keydb_once() { @@ -1277,9 +1255,8 @@ mod tests { ); } - // The cache must not go stale: a keydb replaced UNDERNEATH the source has - // a different size/mtime stamp and must be re-read. See - // docs/keydb.md#test-changed-keydb. + // The cache must not go stale: a keydb replaced UNDERNEATH the source has a different + // size/mtime stamp and must be re-read. #[test] fn a_changed_keydb_file_is_reparsed() { let dir = scratch("cache-stamp"); @@ -1306,9 +1283,9 @@ mod tests { assert_eq!(src.parse_count(), 2, "one parse per distinct file"); } - // Two stale-cache discriminators, tested ONE AT A TIME (each test - // disables the other via `mtime_granularity`). Discriminator 1, the - // inode: an ATOMIC RENAME must be re-read even with identical length and mtime. See docs/keydb.md#test-inode-rename. + // Two stale-cache discriminators, tested ONE AT A TIME (each test disables the other via + // `mtime_granularity`). Discriminator 1, the inode: an ATOMIC RENAME must be re-read even + // with identical length and mtime. #[cfg(unix)] #[test] fn a_same_length_same_mtime_rename_is_still_re_read() { @@ -1343,9 +1320,8 @@ mod tests { assert_eq!(src.parse_count(), 2, "the replacement must be re-parsed"); } - // Discriminator 2, the settle window: a keydb rewritten IN PLACE (same - // inode/length, mtime forced back) must still be re-read while too young - // to trust. See docs/keydb.md#test-inplace-rewrite. + // Discriminator 2, the settle window: a keydb rewritten IN PLACE (same inode/length, mtime + // forced back) must still be re-read while too young to trust. #[cfg(unix)] #[test] fn an_in_place_rewrite_under_an_unsettled_stamp_is_still_re_read() { @@ -1389,9 +1365,8 @@ mod tests { ); } - // A corrupt-but-parseable keydb must keep saying so on EVERY hit, not - // just the parse that produced the summary (a cache hit skips `parse`). - // See docs/keydb.md#test-corrupt-warns. + // A corrupt-but-parseable keydb must keep saying so on EVERY hit, not just the parse that + // produced the summary (a cache hit skips `parse`). #[cfg(unix)] #[test] fn a_corrupt_keydb_warns_on_every_lookup_not_only_on_the_parse() { @@ -1429,9 +1404,8 @@ mod tests { ); } - // A HEALTHY keydb must stay silent — the re-emission above must not turn - // into a per-lookup log line for every operator with an intact file. See - // docs/keydb.md#test-healthy-silent. + // A HEALTHY keydb must stay silent — the re-emission above must not turn into a per-lookup + // log line for every operator with an intact file. #[cfg(unix)] #[test] fn a_healthy_keydb_warns_on_no_lookup() { @@ -1455,9 +1429,8 @@ mod tests { ); } - // The cost side of the settle rule, asserted as a decision: a keydb - // written just now is re-read on every lookup until quiet for the - // granularity. See docs/keydb.md#test-fresh-not-trusted. + // The cost side of the settle rule, asserted as a decision: a keydb written just now is + // re-read on every lookup until quiet for the granularity. #[test] fn a_freshly_written_keydb_is_not_trusted_from_cache() { let dir = scratch("cache-fresh"); @@ -1486,9 +1459,8 @@ mod tests { ); } - // `save()` replaces the very file this source reads, so it drops the - // cached parse ITSELF rather than trusting the (size, mtime) stamp — the - // one change the stamp cannot see. See docs/keydb.md#test-save-invalidates. + // `save()` replaces the very file this source reads, so it drops the cached parse ITSELF + // rather than trusting the (size, mtime) stamp — the one change the stamp cannot see. #[cfg(unix)] #[test] fn save_invalidates_the_cache_even_when_the_file_stamp_is_unchanged() { @@ -1522,9 +1494,8 @@ mod tests { ); } - // A corrupt keydb is a source FAILURE, not "this disc has no key": both - // used to collapse into Ok(Vec::new()) with ZERO tracing. Only a MISSING - // file is genuinely benign. See docs/keydb.md#test-corrupt-is-error. + // A corrupt keydb is a source FAILURE, not "this disc has no key": both used to collapse + // into Ok(Vec::new()) with ZERO tracing. Only a MISSING file is genuinely benign. #[test] fn a_corrupt_keydb_is_an_error_not_an_empty_answer() { let dir = scratch("corrupt"); diff --git a/src/keydb_format.rs b/src/keydb_format.rs index f8eb44f..fd1f864 100644 --- a/src/keydb_format.rs +++ b/src/keydb_format.rs @@ -5,8 +5,6 @@ //! logic is identical; the only deviation is [`KeyDb::load`], which returns a //! standalone [`std::io::Result`] here instead of `libfreemkv::error::Result` //! (so the format crate carries no dependency on libfreemkv's error type). -// -// See docs/keydb-format.md — why the full parser API (incl. dead_code items) is retained. #![allow(dead_code)] use std::collections::HashMap; @@ -49,9 +47,7 @@ pub struct KeyDb { pub disc_entries: HashMap, DiscEntry>, } -// Key material must never leak via `{:?}`; both types below carry raw AACS -// key bytes. See docs/keydb-format.md for why these hand-write a redacting -// `Debug` instead of deriving it. Pinned by `keydb_debug_is_redacted` / `disc_entry_debug_is_redacted`. +// Key material must never leak via `{:?}`; both types below carry raw AACS key bytes. impl std::fmt::Debug for KeyDb { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { @@ -173,9 +169,7 @@ pub(crate) fn parse_hex20(s: &str) -> Option<[u8; 20]> { libfreemkv::hex::parse_hex_fixed::<20>(s) } -// What `KeyDb::parse` threw away, by reason. See docs/keydb-format.md — why -// this counter exists (silent corruption on a third-party keydb) and why -// it's logged once per parse rather than per-line. +// What `KeyDb::parse` threw away, by reason. #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] pub(crate) struct ParseStats { /// `| DK` rows that parsed as neither a positioned nor an orphan device key. @@ -233,13 +227,12 @@ pub(crate) fn is_disc_entry_line(line: &str) -> bool { (line.starts_with("0x") || line.starts_with("0X")) && line.contains(" = ") } -/// True when `line` is a row `KeyDb::parse` would actually ACCEPT into the db — -/// a disc row OR a DK/PK/HC/HC2 row that its real parser accepts, not merely one -/// carrying the right `| XX` prefix. `KeydbSource::save`'s "won't persist -/// unparseable content" guard counts entries with THIS, so a syntactically -/// prefixed but malformed DK/PK/HC row (right prefix, bad hex/short cert) can no -/// longer inflate the entry count past the `entries > 0` check. Mirrors the -/// dispatch in [`KeyDb::parse_counted`] exactly. See docs/keydb.md#save-mirror-parse. +/// True when `line` is a row `KeyDb::parse` would actually ACCEPT into the db — a disc row OR a +/// DK/PK/HC/HC2 row that its real parser accepts, not merely one carrying the right `| XX` +/// prefix. `KeydbSource::save`'s "won't persist unparseable content" guard counts entries with +/// THIS, so a syntactically prefixed but malformed DK/PK/HC row (right prefix, bad hex/short +/// cert) can no longer inflate the entry count past the `entries > 0` check. Mirrors the +/// dispatch in [`KeyDb::parse_counted`] exactly. pub(crate) fn is_parseable_entry_line(line: &str) -> bool { let line = line.trim(); if line.is_empty() || line.starts_with(';') || line.starts_with('#') { @@ -412,9 +405,7 @@ impl KeyDb { Ok(db) } - // `Self::load` from an ALREADY-OPEN file, without the log. See - // docs/keydb-format.md — why the same descriptor must supply both the - // identity stamp and the bytes, and why rejection counts are retained here. + // `Self::load` from an ALREADY-OPEN file, without the log. pub(crate) fn load_counted( f: std::fs::File, path: &std::path::Path, @@ -529,9 +520,6 @@ impl KeyDb { /// line per disc entry (sorted by hash). `parse(to_keydb_cfg(kd))` reproduces /// every field (see `round_trips_through_parse`). Used by the key-import tool /// to export a complete keydb.cfg (keys + host certs + VIDs). - /// - /// See docs/keydb-format.md for why the trailing `; ` is only - /// emitted after a `U` (unit-keys) field. pub fn to_keydb_cfg(&self) -> String { fn hx(b: &[u8]) -> String { use std::fmt::Write; @@ -784,7 +772,6 @@ impl KeyDb { let is_uhd = comment.contains("(UHD)"); // Title kept VERBATIM (trimmed), a faithful copy for exact round-trip. - // See docs/keydb-format.md — why display prettification does NOT belong here. let before_fields = rest.split(" | ").next().unwrap_or(""); // A title-only entry (no key fields) carries its `;` comment on the same // chunk — strip it so the comment doesn't leak into the title. @@ -968,9 +955,8 @@ mod tests { assert_eq!(b.host_certs[0].revoked_at_mkb, Some(72)); } - // REAL-DATA IDEMPOTENCE: parse → serialize (S1) → parse → serialize (S2); - // S1 must equal S2 byte-for-byte. REQUIRES a real keydb (KEYDB_PATH env); - // `#[ignore]` by default. See docs/keydb-format.md. + // REAL-DATA IDEMPOTENCE: parse → serialize (S1) → parse → serialize (S2); S1 must equal S2 + // byte-for-byte. REQUIRES a real keydb (KEYDB_PATH env); `#[ignore]` by default. #[test] #[ignore = "needs a real keydb.cfg: KEYDB_PATH= cargo test -- --ignored"] fn to_keydb_cfg_is_idempotent_on_real_keydb() { @@ -1028,8 +1014,6 @@ mod tests { } // The real keydb for the `#[ignore]`d real-scale tests, or a LOUD failure. - // See docs/keydb-format.md — why silent `return` on a missing fixture is - // wrong here. fn real_keydb_path() -> std::path::PathBuf { keydb_path().expect( "KEYDB_PATH must point at a real keydb.cfg; run: \ @@ -1234,9 +1218,8 @@ mod tests { ); } - // `load_counted` must read the HANDLE it was given, never re-resolve the - // path (identity stamp + bytes must match). See docs/keydb-format.md. - // With the path already replaced, the handle's contents must still win. + // `load_counted` must read the HANDLE it was given, never re-resolve the path (identity + // stamp + bytes must match). #[cfg(unix)] #[test] fn load_counted_reads_the_handle_not_the_path() { @@ -1367,9 +1350,9 @@ mod tests { ); } - // The SIBLING type: `KeyDb`'s hand-written `Debug` protects the whole-db - // rendering only; `{:?}` on one `KeydbHostCert` element walks a different - // path carrying the AACS host PRIVATE key. See docs/keydb-format.md. + // The SIBLING type: `KeyDb`'s hand-written `Debug` protects the whole-db rendering only; + // `{:?}` on one `KeydbHostCert` element walks a different path carrying the AACS host + // PRIVATE key. #[test] fn host_cert_debug_is_redacted_including_the_sibling_wrapper() { let hc = KeydbHostCert { @@ -2336,8 +2319,8 @@ mod tests { ); } - // KEYDB-parser integration tests relocated from libfreemkv; exercise the - // parser end-to-end against a real keydb.cfg. See docs/keydb-format.md. + // KEYDB-parser integration tests relocated from libfreemkv; exercise the parser end-to-end + // against a real keydb.cfg. /// REQUIRES a real keydb; `#[ignore]` by default. Run with: /// `KEYDB_PATH=/path/to/keydb.cfg cargo test --release -- --ignored`. @@ -2437,8 +2420,6 @@ mod tests { } // Reaching here is the EXPECTED outcome for this AACS 2.0 (BEE) sample. - // See docs/keydb-format.md for why "no key worked" alone would be an - // unusable assertion, and what is asserted instead. assert!(attempts > 0, "no unit key was tried at all"); assert!( any_key_changed_the_unit, diff --git a/src/lib.rs b/src/lib.rs index 6417ef5..71667d5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,11 +1,10 @@ //! Pluggable AACS key sources for libfreemkv. //! -//! libfreemkv owns the AACS crypto; this crate provides [`KeySource`] impls -//! that look a disc up and drive the boil-down primitives to terminal Unit -//! Keys: [`KeydbSource`] (local `keydb.cfg`) and [`OnlineSource`] (remote key -//! service). Applications choose and order sources, then resolve and hand -//! the key to `Disc::decrypt_with`; compose several with [`MultiSource`]. -//! See docs/lib-scope.md for the mechanism-vs-policy split with libfreemkv. +//! libfreemkv owns the AACS crypto; this crate provides [`KeySource`] impls that look a disc up +//! and drive the boil-down primitives to terminal Unit Keys: [`KeydbSource`] (local +//! `keydb.cfg`) and [`OnlineSource`] (remote key service). Applications choose and order +//! sources, then resolve and hand the key to `Disc::decrypt_with`; compose several with +//! [`MultiSource`]. mod keydb; /// The `keydb.cfg` parser (`KeyDb`, `DiscEntry`, …). Public: parsing the keydb @@ -42,13 +41,11 @@ pub(crate) fn uks_from_vuk(vuk: &[u8; 16], enc_title_keys: &[[u8; 16]]) -> Vec>, } @@ -60,9 +57,9 @@ impl MultiSource { } } -// Drive `sources` in order, returning the first non-empty result and -// preserving Ok/Err when nothing resolves. `get` selects the trait method so -// the base and forensic paths share one implementation. See docs/lib-scope.md. +// Drive `sources` in order, returning the first non-empty result and preserving Ok/Err when +// nothing resolves. `get` selects the trait method so the base and forensic paths share one +// implementation. fn first_non_empty( sources: &[Box], get: impl Fn(&dyn KeySource, &dyn ResolveCtx) -> Result, libfreemkv::Error>, diff --git a/src/online.rs b/src/online.rs index 3d883d2..f4b2994 100644 --- a/src/online.rs +++ b/src/online.rs @@ -35,9 +35,7 @@ pub use libfreemkv::keysource::MIN_SAMPLE_UNITS; /// the client to OOM with an unbounded body. const MAX_RESPONSE_BYTES: usize = 1024 * 1024; -// ── SSRF guard — see docs/online-ssrf-guard.md ─────────────────────────────── -// is_blocked_ip: true when `ip` must never be an outbound key-service POST -// target (loopback, link-local incl. cloud metadata, RFC1918, IPv4-mapped). +// ── SSRF guard. fn is_blocked_ip(ip: &IpAddr) -> bool { match ip { IpAddr::V4(v4) => { @@ -154,9 +152,8 @@ fn resolve_and_guard(url: &str) -> Result, (GuardFail, String)> } else if let Some((h, p)) = authority.rsplit_once(':') { match p.parse::() { Ok(p) => (h.to_string(), p), - // A malformed port (e.g. `:notaport`) is an operator config typo, - // not a service outage — reject as `Config`, same as the - // bracketed-IPv6 branch above. See docs/online-guardfail.md. + // A malformed port (e.g. `:notaport`) is an operator config typo, not a service + // outage — reject as `Config`, same as the bracketed-IPv6 branch above. Err(_) => return Err((GuardFail::Config, "invalid port".into())), } } else { @@ -254,9 +251,8 @@ pub fn validate_keyserver_url(url: &str) -> Result<(), String> { // first 16 — each already validated by `resolve_and_guard`. const MAX_PINNED_ADDRS: usize = 16; -// The pinned-address resolver behind `hardened_agent`. Must be wired via -// `Agent::with_parts` — `new_with_config` silently keeps live DNS and -// reopens the rebind window. See docs/online-ssrf-guard.md. +// The pinned-address resolver behind `hardened_agent`. Must be wired via `Agent::with_parts` — +// `new_with_config` silently keeps live DNS and reopens the rebind window. #[derive(Debug)] struct PinnedResolver(Vec); @@ -299,13 +295,10 @@ fn hardened_agent(pinned: Vec) -> ureq::Agent { pub struct OnlineSource { base_url: String, secret: String, - /// The last agent built, with the address set (sorted, deduped — an - /// order-insensitive SET KEY) it was pinned to. Reused only when a fresh - /// resolve + SSRF-guard of the host yields the identical address set, so - /// the anti-rebinding guarantee is untouched: only the pooled TLS - /// connection is reused, never a stale, un-reguarded address. See - /// docs/online-agent-cache.md for why this exists and why the key is a - /// set rather than an ordered sequence. + /// The last agent built, with the address set (sorted, deduped — an order-insensitive SET + /// KEY) it was pinned to. Reused only when a fresh resolve + SSRF-guard of the host yields + /// the identical address set, so the anti-rebinding guarantee is untouched: only the pooled + /// TLS connection is reused, never a stale, un-reguarded address. agent: Mutex, Arc)>>, } @@ -340,9 +333,8 @@ impl OnlineSource { agent } - // The server-resolved Unit Keys for this disc: one round-trip, returning - // a terminal `UK` or a `VUK` derived locally. `Ok`/`Err` draw the - // miss-vs-outage distinction — see docs/online-query-contract.md. + // The server-resolved Unit Keys for this disc: one round-trip, returning a terminal `UK` or + // a `VUK` derived locally. `Ok`/`Err` draw the miss-vs-outage distinction. fn query(&self, ctx: &dyn ResolveCtx) -> Result, Error> { // Reset the per-thread decode-reachability slot so it reflects ONLY this // query afterwards: `Some(..)` once a POST answers/transport-fails, `None` @@ -352,9 +344,8 @@ impl OnlineSource { if self.base_url.is_empty() { return Ok(Vec::new()); } - // Refuse to transmit over plaintext: the body carries base64 key - // material and, when configured, a replayable bearer token. `Err`, - // not `Ok(empty)` — see docs/online-query-guards.md. + // Refuse to transmit over plaintext: the body carries base64 key material and, when + // configured, a replayable bearer token. `Err`, not `Ok(empty)` if !self.base_url.starts_with("https://") { tracing::error!( target: "freemkv::keysource", @@ -365,9 +356,7 @@ impl OnlineSource { return Err(Error::KeyServiceUnavailable); } let mkb = ctx.mkb().unwrap_or(&[]); - // No-URL / bad-URL / over-cap-or-under-sampled draw three different - // verdicts — see docs/online-query-guards.md. Logged: a silent empty - // here reads as "no key", so the real cause (64 MiB cap) is surfaced. + // No-URL / bad-URL / over-cap-or-under-sampled draw three different verdicts. if mkb.len() > MAX_MKB_BYTES { tracing::warn!( target: "freemkv::keysource", @@ -422,8 +411,7 @@ impl OnlineSource { // time can't redirect the request to an internal/metadata host. let pinned = match resolve_and_guard(&self.base_url) { Ok(addrs) => addrs, - // Did-not-RESOLVE is the service unreachable, not a bad URL — see - // docs/online-guardfail.md. + // Did-not-RESOLVE is the service unreachable, not a bad URL. Err((GuardFail::Unreachable, _)) => { // The host did not resolve — the service never answered, so this // is a transport-class outcome for the decode-reachability slot @@ -438,9 +426,8 @@ impl OnlineSource { return Err(Error::KeyServiceUnavailable); } Err((GuardFail::Config, _)) => { - // Log THAT the URL was rejected, never WHY (the message names - // the address). `error!` + `Err`, not `Ok(empty)` — see - // docs/online-guardfail.md for the full rationale. + // Log THAT the URL was rejected, never WHY (the message names the address). + // `error!` + `Err`, not `Ok(empty)` tracing::error!( target: "freemkv::keysource", phase = "keyserver_post", @@ -683,9 +670,8 @@ fn interpret_reply( } impl KeySource for OnlineSource { - // Base per-CPS-unit Unit Keys via `query`: `Ok(empty)` means the service - // answered with no key, `Err` means it could not answer — see - // docs/online-query-contract.md. + // Base per-CPS-unit Unit Keys via `query`: `Ok(empty)` means the service answered with no + // key, `Err` means it could not answer. fn get_unit_keys(&self, ctx: &dyn ResolveCtx) -> Result, Error> { self.query(ctx) } @@ -950,9 +936,8 @@ mod tests { assert_eq!(addrs[0].port(), 8080); } - // The pin is actually consulted (a mis-wired resolver fails OPEN to - // live DNS with no symptom — see docs/online-ssrf-guard.md): pin to a - // loopback listener, then ask for a `.test` host that CANNOT resolve. + // The pin is actually consulted: pin to a loopback listener, then ask for a `.test` host + // that CANNOT resolve. #[test] fn hardened_agent_connects_to_the_pinned_address_not_dns() { use std::io::Write as _; @@ -1438,9 +1423,7 @@ mod tests { } } - // ── The pre-flight guards in `query` (nothing leaves the process) ────── - // See docs/online-preflight-guard-tests.md — why each guard's verdict - // differs and why `.test` is the discriminator host used below. + // ── The pre-flight guards in `query` (nothing leaves the process) ──────. /// A `ResolveCtx` whose MKB size and sample COUNT are dialled per guard. struct GuardCtx { @@ -1468,9 +1451,8 @@ mod tests { } } - // An `http://` key-service URL must never be POSTed to; the source - // refuses with `Err`, not an empty that reads as "no key" — see - // docs/online-query-guards.md. + // An `http://` key-service URL must never be POSTed to; the source refuses with `Err`, not + // an empty that reads as "no key". #[test] fn cleartext_http_url_is_refused_before_anything_is_sent() { let src = OnlineSource::new("http://keyserver.test/keys", "s3cr3t"); @@ -1562,8 +1544,7 @@ mod tests { // ── A bad port is CONFIG, not an outage ──────────────────────────────── - // A typo'd port must be `Config`, not `Unreachable` — see - // docs/online-guardfail.md for why collapsing the two is the bug. + // A typo'd port must be `Config`, not `Unreachable` #[test] fn unparseable_port_is_a_config_fault_not_an_outage() { for url in [ @@ -1582,9 +1563,8 @@ mod tests { assert_eq!(addrs[0].port(), 8443); } - // The caller-visible half: a mistyped port must get `Err`, exactly like - // an outage (only the log text differs) — catches the `Ok(Vec::new())` - // regression from `GuardFail::Config` (docs/online-guardfail.md). + // The caller-visible half: a mistyped port must get `Err`, exactly like an outage (only the + // log text differs) — catches the `Ok(Vec::new())` regression from `GuardFail::Config`. #[test] fn query_with_a_mistyped_port_reports_a_failure_not_a_miss() { let src = OnlineSource::new("https://example.com:notaport/keys", "s3cr3t"); @@ -1721,9 +1701,8 @@ mod tests { assert!(src.get_unit_keys(&WhitespaceTitleCtx).is_err()); } - // ── One agent per address set, not one per query ────────────────────── - // A round-robin keyserver reorders the SAME addresses; still the same - // set, so it must reuse the agent — see docs/online-agent-cache.md. + // ── One agent per address set, not one per query ────────────────────── A round-robin + // keyserver reorders the SAME addresses; still the same set, so it must reuse the agent. #[test] fn a_reordered_but_identical_address_set_reuses_the_agent() { let src = OnlineSource::new("https://keyserver.test/keys", ""); @@ -1753,9 +1732,8 @@ mod tests { ); } - // An FMTS disc calls `query` twice per rip; the agent is reused only - // while the freshly guarded address set is IDENTICAL — see - // docs/online-agent-cache.md. + // An FMTS disc calls `query` twice per rip; the agent is reused only while the freshly + // guarded address set is IDENTICAL. #[test] fn the_agent_is_reused_per_address_set_only() { let src = OnlineSource::new("https://keyserver.test/keys", ""); diff --git a/src/paths.rs b/src/paths.rs index f4339c9..82ce263 100644 --- a/src/paths.rs +++ b/src/paths.rs @@ -4,9 +4,8 @@ //! just reads a handed-in path. This module gives the *search* list (first //! existing wins) and the single *default* write location. //! -//! freemkv is portable: `keydb.cfg` lives next to the exe — -//! `/keydb.cfg` — with no OS config-dir fallback. -//! `--keydb PATH` bypasses this module. See docs/paths.md for rationale. +//! freemkv is portable: `keydb.cfg` lives next to the exe — `/keydb.cfg` — +//! with no OS config-dir fallback. `--keydb PATH` bypasses this module. use std::path::PathBuf; diff --git a/tests/key_sources.rs b/tests/key_sources.rs index 5b9b29b..107dc84 100644 --- a/tests/key_sources.rs +++ b/tests/key_sources.rs @@ -368,9 +368,7 @@ fn multi_source_real_keydb_resolves_through_chain() { ); } -// ── MultiSource: a source that could not ANSWER is not a source that said no ─ -// See docs/multi-source-failure-collapse.md — the Ok/Err collapse incident -// this section's tests exist to catch. +// ── MultiSource: a source that could not ANSWER is not a source that said no ─. /// A source that always fails, with a chosen error — the key-service outage. struct FailingSource { @@ -398,8 +396,6 @@ fn unauthorized() -> libfreemkv::Error { libfreemkv::Error::KeyServiceUnauthorized } -// See docs/multi-source-failure-collapse.md — must report FAILURE, not a -// clean "no key", when no source holds a key and one could not answer. #[test] fn multi_source_reports_a_source_failure_instead_of_a_clean_no_key() { let multi = MultiSource::new(vec![