Two defect classes dominate this change, and both are the same shape: something
that should have been loud was silent.
KEY MATERIAL IN A PUBLIC `Debug`. `KeyDb` and `DiscEntry` both carried a derived
`Debug` over raw key bytes — every processing key, and every disc's Media Key,
VID, VUK and Unit Keys. Both are public API, so a single `{:?}` in a caller's
log line, a `tracing` field, or an `assert_eq!` panic message published the
whole keydb: on the real file that is 184,860 discs' key material plus the
processing-key pool. libfreemkv's equivalent types (`aacs::types`) all
hand-write a redacting `Debug` for exactly this reason; this crate's copy of the
parser dropped the convention when it was relocated. Both impls are restored,
field-for-field in the same style, and pinned by tests that fail against the
derive.
A SOURCE THAT COULD NOT ANSWER IS NOT A SOURCE THAT SAID "NO KEY". `OnlineSource`
was engineered at length to separate `Err` (the service could not answer) from
`Ok(empty)` (it answered and holds nothing) after a seven-hour run of HTTP 502s
was reported to operators as E7022 "no key for this disc". Three places
re-collapsed that distinction:
* `MultiSource` discarded every inner `Err` (`if let Ok(uks) = ..`), so a
composition whose sources had all failed returned a clean `Ok(empty)` — the
incident reproduced one layer up, in both the base and the forensic path. It
now returns the FIRST source failure when nothing resolved, matching
libfreemkv's own resolver, while a later success still wins outright.
* `KeydbSource` turned every `KeyDb::load` failure into `Ok(empty)` with no
log at all. A keydb over the 128 MiB cap or one that is not valid UTF-8 is a
corrupt, truncated or half-written file, not a disc without a key; those are
now logged and returned as errors. A MISSING keydb stays the documented
benign empty.
* A mistyped port in the key-service URL (`https://host:notaport/`) reused the
whole authority as the hostname, so it failed DNS and surfaced as
`Unreachable` — a standing misconfiguration reported as a transient outage,
the one collapse `GuardFail`'s doc says must never happen because the two
halves demand opposite operator actions. It is now a `Config` rejection, the
same verdict the bracketed-IPv6 branch already gave.
The keydb parser was equally silent: every malformed `| DK` / `| PK` / `| HC` /
`| HC2` / `0x…` row was dropped with no counter and no log, and so was a row
lost to the entry cap or overwritten by a duplicate hash. A byte-shifted mirror
download therefore passed `save()`'s validation, was atomically committed as the
new keydb, and resolved nothing for every disc past the damage — indistinguishably
from a healthy file. Rejections are now counted by reason and summarised in one
log line per parse. The real published keydb parses with zero rejections and zero
duplicates, so the summary stays quiet on healthy input.
Also fixed, with tests:
* The 62 MiB / 184,860-entry keydb was re-read and re-parsed on EVERY call —
~140 ms each in release, three or more times per AACS-cert rip. `KeydbSource`
now caches the parse behind the file's (size, mtime) stamp (~1 µs on a hit),
re-reads a file replaced underneath it, and drops its own cache on `save()`
so a same-size rewrite inside the timestamp resolution cannot be missed.
Peak RSS is unchanged; the parse simply is not repeated.
* A disc row prefixed `0X` was dropped silently, though `parse_hex` in the same
file deliberately accepts both cases. `save()`'s entry count now CALLS the
parser's own predicate instead of restating it, so the two cannot drift again.
* Each disc hash was stored twice — once as the map key, once in the entry —
~13 MB of duplicate text at real scale, now one shared allocation.
* `OnlineSource` built a fresh agent per query, paying a second TLS handshake
on every FMTS disc. The agent is reused only while the freshly re-resolved
and re-guarded address set is identical, so the anti-rebinding guarantee is
untouched.
Test coverage for defences that were never exercised: the cleartext-`http://`
refusal, the over-cap MKB skip and the too-few-samples skip (a reordering that
let a plaintext POST through — carrying the bearer token and base64 key material
— would have shipped green), and the 1 MiB reply cap. The real-scale tests used
to `return` early when `KEYDB_PATH` was unset, so CI ran them, asserted nothing
about 181k-entry behaviour and reported success; they are `#[ignore]`d with a
documented run command and now fail loudly if run without their fixture.
Version bump and changelog for the 1.6.3 sync. All eight crates ship the
same version, so the crates with no functional change this cycle say so
rather than carrying an empty section.
The three crates move together because they must: freemkv resolves its
siblings from their git TAGS in its own CI, where keysources still wanted
ureq 2, so cargo saw no conflict and the earlier split bump went green on
a dependency graph the release never uses. The qa and hash-matrix
workflows resolve those siblings from their dev PATHS, cargo unifies to a
single ureq, and the mismatch stops compiling. Verified here the way
those workflows see it: freemkv built against the sibling paths now
resolves one ureq 3.4.0 and no ureq 2 anywhere.
Notable API changes:
AgentBuilder::new()..build() -> Config::builder()..build() + Agent
.redirects(0) -> .max_redirects(0)
.timeout_read(d) -> .timeout_recv_response(Some(d))
.resolver(closure) -> the Resolver trait
.set(k, v) -> .header(k, v)
ureq::Response -> http::Response<ureq::Body>
Error::Status(code, _) -> Error::StatusCode(code)
Error::Transport(_) -> fans out; matched with a catch-all
Two hazards the new resolver API introduces, both handled:
Agent::new_with_config compiles fine and then silently ignores a custom
resolver, so the pinned addresses would be dropped and every request
would go back to live DNS -- reopening the DNS-rebinding window the
guards exist to close, with no symptom. Only Agent::with_parts takes a
resolver, and that is what every pinned agent here uses.
ResolvedSocketAddrs is a fixed 16-slot array whose push writes straight
into it, so a host publishing more than 16 addresses would panic the
calling thread. Truncated to the first 16.
The four reply stubs used ureq::Response::new, which v3 removes; they are
rebuilt with http::Response::builder over a ureq::Body, so they stay
server-free unit tests.
The Windows UI needs current winsafe, whose real minimum is 1.89 (its manifest
under-declares 1.87 while it uses NonNull::from_ref). Rather than stop at the
minimum, this goes to current stable and fixes what that costs.
The counter-intuitive result: 1.97 is CHEAPER than 1.89. libfreemkv had 54
clippy errors at 1.89 and 6 at 1.97, because clippy tightened the noisy
collapsible_if lint in between. Stopping at the minimum would have been the most
expensive choice available.
Roughly 47 lints across the eight repos, the large majority auto-fixed:
libfreemkv 6, freemkv-engine 14, bdemu 8, freemkv-keysources 7, autorip 6,
freemkv-unlock 3, freemkv-i18n 3. The hand-fixed ones are a descending sort to
sort_by_key(Reverse), four manual checked-division sites, a loop counter replaced
by enumerate, and a loop whose first let-else became a while-let.
Worth recording for whoever bumps next: clippy is MSRV-AWARE. Those 54 lints only
appear once the crate DECLARES 1.89 or later, because let-chains become
available. A bare `cargo +1.89 clippy` against a manifest still pinned at 1.87
reports clean and is meaningless — gate with the real precommit script, which is
also the only thing that covers build scripts.
The pin still sits below the Mac default, so it keeps doing its job: catching
lint drift locally before CI sees it.
The Windows UI needs winsafe, whose current release requires rustc 1.87. The
alternative was pinning winsafe back to an older release, which would bake a
stale API surface into a brand-new UI permanently to dodge one minor version.
The pin's purpose is to sit BELOW the Mac default so clippy drift is caught
locally before CI, not to stay on 1.86 specifically, so 1.87 preserves the
discipline exactly.
Verified before moving anything, not after: `cargo +1.87 clippy -- -D warnings`
and `cargo +1.87 fmt --check` are clean across all eight repos, and the full
precommit gate (fmt + clippy + tests) passes on libfreemkv, autorip, bdemu,
freemkv-engine and freemkv-keysources. Zero new lints, zero formatting drift.
Bump to 1.6.0 (workspace sync, no source change). Fix stale get_uk ->
get_unit_keys references in the crate description + comments, correct the
libfreemkv version note, and drop a phantom Mapfile mention from the tests.
keysources deps libfreemkv, now git-only, so keysources follows it off
crates.io. A committed [patch.crates-io] redirects the bare libfreemkv version
req to the git tag (local dev overrides to a path via the gitignored config).
Move the keydb save/validation/atomic-write path out of libfreemkv onto
KeydbSource. New KeydbSource::save(bytes) validates + decompresses (zip /
gz / plain, decompressed-size capped) and crash-safely writes to the
source's OWN path (sibling-temp + fsync + rename + parent-dir fsync) —
not a hardcoded default, so the caller chooses the destination.
KeydbSource::update(fetch, url) calls an INJECTED fetch closure then
save, keeping this crate transport-agnostic on the update path (the app
supplies its own TLS / SSRF-guarded transport). UpdateResult moves here
and is re-exported. Add flate2 + zip (already in the resolved graph via
libfreemkv) for decompression; no new HTTP stack.
unit_keys_from now resolves the Media Key in order: stored per-disc MK
-> keydb Processing Key pool (mk_from_pk vs this disc's own MKB) ->
device-key pool (mk_from_dk), then MK+VID -> VUK -> UK. MK/VUK entries
still honored directly; cross-disc MK-pool brute stays retired. Fixes the
factually-wrong justifying comment + adds PK-pool KATs.
Matthew Jackson