From a94f78d09051f53ac63f3896bd32b0b42d2e16eb Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Thu, 9 Jul 2026 14:14:20 -0700 Subject: [PATCH] audit: cap the sparse-PTS reorder buffer, FMTS key state, zero KCD MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-1 findings from the 10-phase release audit: - SparsePtsReorder buffered its current GOP with no bound, draining only on a keyframe — an open-GOP or crafted program stream that never signals one could hold the whole title in RAM. Force-complete the GOP at MAX_GOP_FRAMES, matching the MPEG-2 parser's backstop. - inject_unit_keys labelled a 2.1 FMTS disc as AACS 1.0 / bus-encryption off; FMTS is UHD-family, so synthesize the UHD version + bus encryption. - The compiled Key Correction Data was a non-zero 16-byte constant fed into the Media Key derivation. Per the no-compiled-keys rule it is now all-zero; the chain still cannot complete on a real disc (documented), so this is behaviour-neutral — all variant tests pass unchanged. - Fix stale doc references (broken `super::variants` intra-doc links, and `aacs::keys` comments) left by the module rename. --- src/aacs/crypto.rs | 2 +- src/aacs/derive.rs | 4 ++-- src/aacs/mkb.rs | 2 +- src/aacs/provider.rs | 2 +- src/aacs/variant.rs | 20 +++++++++----------- src/disc/mod.rs | 12 +++++++----- src/mux/codec/reorder.rs | 32 ++++++++++++++++++++++++++++++-- 7 files changed, 51 insertions(+), 23 deletions(-) diff --git a/src/aacs/crypto.rs b/src/aacs/crypto.rs index 2017bde..7ebd407 100644 --- a/src/aacs/crypto.rs +++ b/src/aacs/crypto.rs @@ -89,7 +89,7 @@ pub(crate) const AESG3_SEED: [u8; 16] = [ /// left=`D(k,s0)⊕s0` inc 0, pk=`D(k,s0+1)⊕(s0+1)` inc 1, right=`D(k,s0+2)⊕(s0+2)` inc 2). /// seed[15] += inc, then AES-DEC(key, seed) XOR seed. /// -/// Shared with [`super::variants`] (its variant chain runs the same SD +/// Shared with [`super::variant`] (its variant chain runs the same SD /// tree); a single definition keeps the two walks byte-identical. pub(crate) fn aesg3(key: &[u8; 16], inc: u8) -> [u8; 16] { let mut seed = AESG3_SEED; diff --git a/src/aacs/derive.rs b/src/aacs/derive.rs index 7de4b1f..8c98573 100644 --- a/src/aacs/derive.rs +++ b/src/aacs/derive.rs @@ -105,7 +105,7 @@ pub(crate) fn validate_processing_key( None } -/// Compute v_mask from a UV value. [C] §3.2.3. Shared with [`super::variants`]. +/// Compute v_mask from a UV value. [C] §3.2.3. Shared with [`super::variant`]. pub(super) fn calc_v_mask(uv: u32) -> u32 { let mut v_mask: u32 = 0xFFFF_FFFF; while (uv & !v_mask) == 0 && v_mask != 0 { @@ -115,7 +115,7 @@ pub(super) fn calc_v_mask(uv: u32) -> u32 { } /// Derive processing key from device key using subset-difference tree traversal. -/// [C] §3.2.4 (device-tree descent, MSB-branch, terminal PK). Shared with [`super::variants`]. +/// [C] §3.2.4 (device-tree descent, MSB-branch, terminal PK). Shared with [`super::variant`]. pub(super) fn calc_pk_from_dk( dk: &[u8; 16], uv: u32, diff --git a/src/aacs/mkb.rs b/src/aacs/mkb.rs index ac94589..19f7af2 100644 --- a/src/aacs/mkb.rs +++ b/src/aacs/mkb.rs @@ -68,7 +68,7 @@ pub fn walk_mkb(mkb: &[u8]) -> Vec { /// then the body — stopping at the `00 000000` end marker or a /// malformed/out-of-bounds length. Lazy (no body clone), so a find-one-record /// caller never materialises the multi-MB cvalue table. [`walk_mkb`] and every -/// MKB record walk in `aacs::keys` are built on this, so the framing rules — and +/// MKB record walk in `aacs::resolve`/`aacs::derive` are built on this, so the framing rules — and /// any future fix to them — live in exactly one place (they had drifted across /// six hand-rolled copies). pub(crate) fn mkb_records(mkb: &[u8]) -> impl Iterator + '_ { diff --git a/src/aacs/provider.rs b/src/aacs/provider.rs index b7715bc..5b5a1c3 100644 --- a/src/aacs/provider.rs +++ b/src/aacs/provider.rs @@ -42,7 +42,7 @@ use super::types::{DeviceKey, DiscEntry, HostCert}; /// Source of AACS key material. /// /// Implementors return raw material only — the resolver in -/// `aacs::keys` owns all the crypto (DK→PK walking, PK validation, +/// `aacs::resolve` and `aacs::derive` own the crypto (DK→PK walking, PK validation, /// MK→VUK→TK derivation). See module docs for method semantics. pub trait KeyProvider: Send + Sync { /// Device keys (top-of-tree, walked by the resolver). diff --git a/src/aacs/variant.rs b/src/aacs/variant.rs index c499315..7f4dde8 100644 --- a/src/aacs/variant.rs +++ b/src/aacs/variant.rs @@ -62,19 +62,17 @@ use super::types::DeviceKey; // ── Public constants ────────────────────────────────────────────────────── -/// AACS 2.1 Key Correction Data. +/// AACS 2.1 Key Correction Data — a zero placeholder, NOT real key material. /// /// **KCD is PER-LICENSEE** (per player manufacturer) — there is no single -/// universal value, so this one constant cannot be correct across discs. We do -/// NOT have the real per-manufacturer KCDs coded, and won't: libfreemkv compiles -/// in no AACS key material (keydb.cfg is the single source of truth). The bytes -/// below only let the chain's SHAPE exercise against synthetic fixtures; on a -/// real variant disc they yield a wrong Media Key that the final -/// Verify-Media-Key gate rejects. So the variant chain cannot complete on a real -/// disc today — a key-acquisition gap, not a code gap. -const KEY_CORRECTION_DATA: [u8; 16] = [ - 0x3b, 0x62, 0x8a, 0x78, 0x29, 0x00, 0xca, 0x2f, 0xdb, 0xe7, 0x7a, 0x49, 0xfe, 0x22, 0xd6, 0x6e, -]; +/// universal value. libfreemkv compiles in no AACS key material (keydb.cfg is +/// the single source of truth), so this stays all-zero: the chain's SHAPE still +/// runs, but on a real variant disc the derivation yields a wrong Media Key that +/// the final Verify-Media-Key gate rejects. The variant chain therefore cannot +/// complete on a real disc today — a key-acquisition gap, not a code gap. If a +/// real per-licensee KCD is ever available it must come from keydb.cfg, never a +/// compiled constant. +const KEY_CORRECTION_DATA: [u8; 16] = [0u8; 16]; // ── MKB record walking ──────────────────────────────────────────────────── diff --git a/src/disc/mod.rs b/src/disc/mod.rs index 75df219..feaa1b0 100644 --- a/src/disc/mod.rs +++ b/src/disc/mod.rs @@ -1490,9 +1490,9 @@ impl Disc { let (capacity, mut buffered, udf_fs) = Self::read_udf(session)?; let meta_title = Self::read_meta_title(&mut buffered, &udf_fs); - // Authoritative up front — same MKB-driven detector as the full scan - // (no titles needed: BD/UHD/FMTS come from the MKB generation). This - // no longer defaults to BluRay and defers UHD/FMTS to the full scan. + // Authoritative here — the same MKB-driven detector the full scan uses + // (no titles needed: BD/UHD/FMTS come from the MKB generation). It no + // longer defaults to BluRay or defers UHD/FMTS to the full scan. let format = Self::detect_disc_format(&mut buffered, &udf_fs, &[]); let encrypted = udf_fs.find_dir("/AACS").is_some() || udf_fs.find_dir("/BDMV/AACS").is_some(); @@ -2660,13 +2660,15 @@ impl Disc { aacs.unit_keys = keys; aacs.key_source = KeyOrigin::ExternalUk; } else if self.encrypted && self.css.is_none() { + // FMTS is AACS 2.1, a UHD-family (bus-encrypted) format — not BD. + let uhd_family = matches!(self.format, DiscFormat::Uhd | DiscFormat::Fmts); self.aacs = Some(AacsState { - version: if self.format == DiscFormat::Uhd { + version: if uhd_family { crate::aacs::mkb::AACS_MAJOR_UHD } else { crate::aacs::mkb::AACS_MAJOR_BD }, - bus_encryption: self.format == DiscFormat::Uhd, + bus_encryption: uhd_family, mkb_version: None, disc_hash: String::new(), key_source: KeyOrigin::ExternalUk, diff --git a/src/mux/codec/reorder.rs b/src/mux/codec/reorder.rs index afcdd55..305a8c9 100644 --- a/src/mux/codec/reorder.rs +++ b/src/mux/codec/reorder.rs @@ -36,6 +36,14 @@ use super::coding::CodingType; /// the timeline across GOPs. const FALLBACK_FRAME_DUR_NS: i64 = 1_001_000_000 / 24; +/// Force-complete the current GOP once it reaches this many buffered pictures +/// even without a keyframe. A GOP is normally a few dozen frames; a stream that +/// never signals a keyframe (open-GOP recovery-point coding, or crafted/corrupt +/// disc bytes) would otherwise buffer every access unit — the whole title — in +/// RAM. Mirrors the MPEG-2 parser's `MAX_PENDING_FRAMES` backstop so no +/// reassembly buffer grows unbounded on disc-controlled input. +const MAX_GOP_FRAMES: usize = 600; + /// One buffered coded picture awaiting its GOP's completion. struct Pending { /// Explicit PES PTS (ns) for this AU, or `None` when the source omitted it. @@ -92,9 +100,11 @@ impl SparsePtsReorder { .map(|c| c.coding_type()) .unwrap_or(CodingType::P); // A keyframe opens a new GOP: the picture already accumulated in `cur` is - // a complete GOP. Complete it (this frame belongs to the NEW GOP). + // a complete GOP. Complete it (this frame belongs to the NEW GOP). Also + // force-complete a pathologically long run that never signalled a + // keyframe, so a crafted/corrupt stream cannot buffer without bound. let mut out = Vec::new(); - if frame.keyframe && !self.cur.is_empty() { + if (frame.keyframe || self.cur.len() >= MAX_GOP_FRAMES) && !self.cur.is_empty() { out = self.complete_current_gop(); } self.cur.push(Pending { @@ -296,6 +306,24 @@ mod tests { ); } + #[test] + fn force_flushes_a_gop_that_never_signals_a_keyframe() { + use CodingType::*; + // A stream that never flags a keyframe (open-GOP recovery points, or a + // crafted/corrupt disc) must not buffer the whole title: the cap + // force-completes GOPs so frames are emitted well before flush(). + let mut r = SparsePtsReorder::new(); + let mut emitted = 0usize; + for i in 0..(MAX_GOP_FRAMES * 3) { + let pts = (i == 0).then_some(0); + emitted += r.push(pts, frame(P, false)).len(); + } + assert!( + emitted >= MAX_GOP_FRAMES, + "cap force-flushed GOPs before EOF (emitted {emitted})" + ); + } + #[test] fn no_pts_collisions_within_a_gop() { use CodingType::*;