There were TWO top-level decrypt paths: decrypt_sectors_impl for CSS and
clear media, whose AACS arm was a bare `return Err` stub, and a wholly
separate decrypt_sectors_mapped for AACS. Each scheme therefore decided
its own answer to "there is no key for these bytes", and nothing held
them to the same one.
They drifted, in opposite directions, within a single release:
css::descramble_region descrambled with a key the sector's own crib had
just proven stale — garbage behind an intact clear header, reported Ok.
the mapped path returned early for any LBA outside every range, before
ever asking whether those bytes were ciphertext, so an unkeyable
encrypted unit passed through and extract counted it as good.
Both were fixed individually earlier today. This removes the shape that
allowed them.
decrypt_span is now the single orchestrator: it owns the loop, the
refusal, and the loss count, and each scheme supplies only what
genuinely differs. apply_aacs_map is a scheme step that reports what it
could not open; it no longer decides what that means. The public
wrappers (decrypt_sectors, _in_content, _mapped) are unchanged in
signature and all funnel through it.
Adding a scheme now means adding an arm here, which means answering the
refusal question. That is the point.
The new test asserts ONE verdict across all three schemes — AACS with no
map, AACS with an encrypted unit outside every range, and CSS whose
re-crack failed — plus that clear media is NOT a refusal. A per-scheme
test cannot hold this: each would keep passing while the two disagreed.
Flipping the AACS arm back to pass-through reds it.
Also removes the last of the tracing-capture scaffolding. Serialising
those captures crate-wide did not fix the 1-in-10 flake, and asserting
the predicates directly made the helper, both capture subscribers and
an unrelated dead OrderSink unused. Deleted rather than left behind.
drive() and drive_mut() had ZERO callers — not in libfreemkv, freemkv,
autorip, bdemu, keysources or kdb. Deleted rather than converted: dead
public API that panics is not an API worth preserving the shape of.
into_drive() had two callers and now returns Result. The empty-slot
state is reachable through ordinary public use — stage_drive_as_reader
moves the drive into the reader slot, and calling into_drive twice moves
it out — so the panic was not guarding a caller error. identify() was
converted for exactly this reason in this same release; the fix went to
one of four public sinks and the other three were left.
I deferred this on the assumption the blast radius was large. It was
three call sites. Checking beats assuming.
Also fixes a REAL FLAKE in the gate, which is worth more than the above.
resolve_vid_only_bus_key_gate_reports_true_has_volume_id... failed about
one full-suite run in ten while passing every time in isolation. It
installed a capturing tracing subscriber to read back the has_volume_id
field of a warn.
That cannot be made reliable: dispatcher::set_default is THREAD-LOCAL
while tracing's callsite-interest cache is GLOBAL. The original author
knew, and called rebuild_interest_cache() — necessary but not
sufficient. I first serialised every capture in the crate behind one
lock (harness::with_captured_tracing, which also removed the same
hand-rolled dance from three other sites). Still 1-in-10, because the
cache can be re-evaluated against the process-default dispatch rather
than the thread-local one.
So the predicate is now a named function, handshake_has_volume_id, and
the test asserts the VALUE. A boolean does not need a subscriber to
check. The gate's hard-error behaviour keeps its own test.
Measured: 14 consecutive full-suite runs, 2994 passed, 0 failed.
A flaky gate is worse than a missing one — every green after it means
less, and this one had been eroding trust in the whole suite.
Five parsers that take bytes straight off a disc are now swept with
generated input asserting one property: they return Ok or Err and never
panic. That is this crate's own hard rule, and the class seven rounds of
reading is worst at.
Written in-crate rather than with cargo-fuzz, which needs a nightly
toolchain this project does not use, and without proptest or arbitrary,
because one dev-dependency is a deliberate posture and the parsers take
plain byte slices. What is given up is coverage-guided mutation, which is
the real loss. What is gained is determinism: the same seed replays the
same cases anywhere, so a CI failure reproduces locally verbatim.
Three generators, and the second is the one that matters. Pure random
bytes die at the magic check and exercise the entry guards only; prefixing
valid magic is what reaches the parser body; mutating a mostly-zero record
is what reaches the offset and count arithmetic a hostile image would lie
about.
That claim is MEASURED, not asserted. A harness whose cases all bounce off
the entry guards is the fuzzing equivalent of a test that cannot fail, so
one test counts how many generated cases parse to completion: 15,606 of
60,000, about 26%. If a future change to a guard drops that to zero, the
test fails rather than continuing to report a meaningless pass. Two further
tests pin that the three generators produce different bytes and that a seed
replays identically.
1.2M cases across all five targets found nothing. On this evidence that is
a real negative rather than an empty one.
The first version of this file was itself broken in the way this audit
keeps finding: its two meta-tests set FREEMKV_HARNESS_CASES and raced,
because the test harness runs them in parallel and env mutation is unsound
there. The budget is a parameter now, and the environment is read once at
the call site.
Two crate-internal parsers widened from private to pub(crate) so the
harness can reach them. No public API change.
Matthew Jackson