From a731e7b26b00c1429f176014df125e2544a650d9 Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Sun, 28 Jun 2026 22:28:09 -0700 Subject: [PATCH] 1.2.0: single MKB framing walker + AACS resolve hardening - mkb_records() as the one record-framing iterator; rebuild walk_mkb, find_record_body, mkb_find_subdiff_records, mkb_content_len, mkb_version, mkb_type_raw, mkb_find_mk_dv on it (D4). - resolve_vid_only / read_aacs_version default to UHD (+warn) on a missing content cert instead of BD; route MKB through bounded read_mkb_content. - AacsVersion major()/from_major() + AACS_MAJOR_BD/UHD as the stride source; table + stride-discriminating regression tests. - read_encrypted_units probes 8 evenly-spaced points per extent (off-midpoint scrambled content now sampled); decrypt source-zero mask uses PKT. --- src/aacs/decrypt.rs | 2 +- src/aacs/keys.rs | 157 +++++++++++++++---------------------------- src/aacs/variants.rs | 60 +++++++++++++---- src/disc/encrypt.rs | 6 +- src/disc/mod.rs | 25 +++++-- src/keysource.rs | 48 +++++++++++++ 6 files changed, 175 insertions(+), 123 deletions(-) diff --git a/src/aacs/decrypt.rs b/src/aacs/decrypt.rs index c7f7759..1d287c1 100644 --- a/src/aacs/decrypt.rs +++ b/src/aacs/decrypt.rs @@ -283,7 +283,7 @@ pub fn decrypt_unit(unit: &mut [u8], unit_key: &[u8; 16]) -> bool { // non-zero (real ciphertext that won't decrypt) → still rejected. const PKT: usize = BD_SOURCE_PACKET_BYTES; // 192 let npkt = ALIGNED_UNIT_LEN / PKT; - let mut pad = [false; ALIGNED_UNIT_LEN / 192]; + let mut pad = [false; ALIGNED_UNIT_LEN / PKT]; for (p, slot) in pad.iter_mut().enumerate().take(npkt) { let off = p * PKT; *slot = unit[off..off + PKT].iter().all(|&b| b == 0); diff --git a/src/aacs/keys.rs b/src/aacs/keys.rs index 228363e..74876e3 100644 --- a/src/aacs/keys.rs +++ b/src/aacs/keys.rs @@ -460,61 +460,38 @@ pub mod probe { /// Find Verify Media Key Record (type 0x81 for AACS 1.0, 0x86 for AACS 2.0/2.1) in MKB. fn mkb_find_mk_dv(mkb: &[u8]) -> Option<[u8; 16]> { - let mut pos = 0; - let mut verify_rec_seen: Vec<(u8, usize, usize)> = Vec::new(); - while pos + 4 <= mkb.len() { - let rec_type = mkb[pos]; - let rec_len = u32::from_be_bytes([0, mkb[pos + 1], mkb[pos + 2], mkb[pos + 3]]) as usize; - if rec_len < 4 || pos + rec_len > mkb.len() { - break; - } - - if rec_type == 0x81 || rec_type == 0x86 { - verify_rec_seen.push((rec_type, pos, rec_len)); - } - - if (rec_type == 0x81 || rec_type == 0x86) && rec_len >= 20 { - // mk_dv is at offset 4 of the record (after the 4-byte header) + // Verify-Media-Key record (0x81 for AACS 1.0, 0x86 for AACS 2.x): mk_dv is + // the 16 bytes at record offset 4 (body offset 0). Needs rec_len >= 20. + let found = crate::aacs::variants::mkb_records(mkb) + .find(|&(_, rt, len)| (rt == 0x81 || rt == 0x86) && len >= 20); + match found { + Some((o, rec_type, rec_len)) => { let mut dv = [0u8; 16]; - dv.copy_from_slice(&mkb[pos + 4..pos + 20]); + dv.copy_from_slice(&mkb[o + 4..o + 20]); tracing::debug!( target: "freemkv::disc", phase = "mkb_mk_dv_found", rec_type, - pos, + pos = o, rec_len, "mk_dv extracted from MKB" ); - return Some(dv); + Some(dv) + } + None => { + tracing::warn!( + target: "freemkv::disc", + phase = "mkb_mk_dv_not_found", + "no 0x81/0x86 record with rec_len>=20 found" + ); + None } - pos += rec_len; } - tracing::warn!( - target: "freemkv::disc", - phase = "mkb_mk_dv_not_found", - verify_rec_seen = ?verify_rec_seen, - scanned_bytes = pos, - "no 0x81/0x86 record with rec_len>=20 found" - ); - None } /// Find Subset-Difference records (type 0x04) in MKB. fn mkb_find_subdiff_records(mkb: &[u8]) -> Option> { - let mut pos = 0; - while pos + 4 <= mkb.len() { - let rec_type = mkb[pos]; - let rec_len = u32::from_be_bytes([0, mkb[pos + 1], mkb[pos + 2], mkb[pos + 3]]) as usize; - if rec_len < 4 || pos + rec_len > mkb.len() { - break; - } - - if rec_type == 0x04 && rec_len > 4 { - return Some(mkb[pos + 4..pos + rec_len].to_vec()); - } - pos += rec_len; - } - None + find_record_body(mkb, 0x04) } /// Find the Media Key Data Record (cvalues table) in an MKB. @@ -545,19 +522,9 @@ fn mkb_find_cvalues(mkb: &[u8]) -> Option> { /// record matching `rec_type`. Returns `None` if no such record exists or /// the record is empty. fn find_record_body(mkb: &[u8], rec_type_wanted: u8) -> Option> { - let mut pos = 0; - while pos + 4 <= mkb.len() { - let rec_type = mkb[pos]; - let rec_len = u32::from_be_bytes([0, mkb[pos + 1], mkb[pos + 2], mkb[pos + 3]]) as usize; - if rec_len < 4 || pos + rec_len > mkb.len() { - break; - } - if rec_type == rec_type_wanted && rec_len > 4 { - return Some(mkb[pos + 4..pos + rec_len].to_vec()); - } - pos += rec_len; - } - None + crate::aacs::variants::mkb_records(mkb) + .find(|&(_, rt, len)| rt == rec_type_wanted && len > 4) + .map(|(o, _, len)| mkb[o + 4..o + len].to_vec()) } /// Real content length of an MKB: the byte offset where the record stream @@ -568,17 +535,13 @@ fn find_record_body(mkb: &[u8], rec_type_wanted: u8) -> Option> { /// actual size so callers can trim off megabytes of zeros before sending or /// archiving. Returns `mkb.len()` only if the whole buffer parsed as records. pub fn mkb_content_len(mkb: &[u8]) -> usize { - let mut pos = 0; - while pos + 4 <= mkb.len() { - let rec_type = mkb[pos]; - let rec_len = u32::from_be_bytes([0, mkb[pos + 1], mkb[pos + 2], mkb[pos + 3]]) as usize; - // A zero type, a zero/short length, or an overrun = records done, padding begun. - if rec_type == 0x00 || rec_len < 4 || pos + rec_len > mkb.len() { - break; - } - pos += rec_len; - } - pos + // End of the last framed record = where the fixed-region zero padding begins. + // (The `00 000000` terminator / overrun stops the walk; real MKBs pad with + // zeros, so this matches the prior "stop at the first padding byte".) + crate::aacs::variants::mkb_records(mkb) + .last() + .map(|(o, _, len)| o + len) + .unwrap_or(0) } /// Trim an MKB's trailing fixed-region padding to its real content length — @@ -602,25 +565,11 @@ pub fn trim_mkb(mut mkb: Vec) -> Vec { /// body offset 0 (`pos + 4`), then the BE u32 version at body offset 4 /// (`pos + 8`). pub fn mkb_version(mkb: &[u8]) -> Option { - let mut pos = 0; - while pos + 4 <= mkb.len() { - let rec_type = mkb[pos]; - let rec_len = u32::from_be_bytes([0, mkb[pos + 1], mkb[pos + 2], mkb[pos + 3]]) as usize; - if rec_len < 4 || pos + rec_len > mkb.len() { - break; - } - - if rec_type == 0x10 && rec_len >= 12 { - return Some(u32::from_be_bytes([ - mkb[pos + 8], - mkb[pos + 9], - mkb[pos + 10], - mkb[pos + 11], - ])); - } - pos += rec_len; - } - None + // Type-and-Version record (0x10): version is the BE u32 at body offset 4 + // (record offset 8). Needs rec_len >= 12 (4 header + 4 type + 4 version). + crate::aacs::variants::mkb_records(mkb) + .find(|&(_, rt, len)| rt == 0x10 && len >= 12) + .map(|(o, _, _)| u32::from_be_bytes([mkb[o + 8], mkb[o + 9], mkb[o + 10], mkb[o + 11]])) } // ── MKB Type field (Type-and-Version record 0x10, bytes 4-7) ──────────────── @@ -687,24 +636,11 @@ impl MkbType { /// The raw 32-bit MKBType field from the Type-and-Version record (0x10), bytes /// 4-7. `None` if no 0x10 record is present. pub fn mkb_type_raw(mkb: &[u8]) -> Option { - let mut pos = 0; - while pos + 4 <= mkb.len() { - let rec_type = mkb[pos]; - let rec_len = u32::from_be_bytes([0, mkb[pos + 1], mkb[pos + 2], mkb[pos + 3]]) as usize; - if rec_len < 4 || pos + rec_len > mkb.len() { - break; - } - if rec_type == 0x10 && rec_len >= 8 { - return Some(u32::from_be_bytes([ - mkb[pos + 4], - mkb[pos + 5], - mkb[pos + 6], - mkb[pos + 7], - ])); - } - pos += rec_len; - } - None + // Type-and-Version record (0x10): the 32-bit MKBType is bytes 4-7 (body + // offset 0). Needs rec_len >= 8 (4 header + 4 type). + crate::aacs::variants::mkb_records(mkb) + .find(|&(_, rt, len)| rt == 0x10 && len >= 8) + .map(|(o, _, _)| u32::from_be_bytes([mkb[o + 4], mkb[o + 5], mkb[o + 6], mkb[o + 7]])) } /// Decode an MKB's Type field. `None` if no Type-and-Version record is present. @@ -1571,6 +1507,23 @@ mod tests { use super::super::types::DiscEntry; use super::*; + /// Audit #5: the `major` / `from_major` mapping is load-bearing for the + /// Unit_Key_RO stride, so pin it as a table. V10 ↔ BD; V20/V21 → UHD; any + /// non-BD major selects the V20/V21 64-byte stride (V10 is the only 48-byte). + #[test] + fn aacs_major_round_trips_and_strides_differ() { + assert_eq!(AacsVersion::V10.major(), AACS_MAJOR_BD); + assert_eq!(AacsVersion::V20.major(), AACS_MAJOR_UHD); + assert_eq!(AacsVersion::V21.major(), AACS_MAJOR_UHD); + assert_eq!(AacsVersion::from_major(AACS_MAJOR_BD), AacsVersion::V10); + assert_eq!(AacsVersion::from_major(AACS_MAJOR_UHD), AacsVersion::V20); + assert_eq!(AacsVersion::from_major(99), AacsVersion::V20); // any non-BD → V20 + assert_ne!( + AacsVersion::from_major(AACS_MAJOR_BD).unit_key_stride(), + AacsVersion::from_major(AACS_MAJOR_UHD).unit_key_stride() + ); + } + /// Finding #5 regression: parse_unit_key_ro must REJECT a Unit_Key_RO.inf /// whose declared `num_unit_keys` exceeds the keys actually present in the /// buffer, instead of silently returning a short list. A truncated list diff --git a/src/aacs/variants.rs b/src/aacs/variants.rs index dc34d27..36fafed 100644 --- a/src/aacs/variants.rs +++ b/src/aacs/variants.rs @@ -88,29 +88,44 @@ pub struct MkbRecord { /// INCLUDING the 4-byte header, followed by payload. The walker stops /// at the first `(type=0, len=0)` end marker or at end of buffer. pub fn walk_mkb(mkb: &[u8]) -> Vec { - let mut out = Vec::new(); - let mut pos = 0; - while pos + 4 <= mkb.len() { + mkb_records(mkb) + .map(|(offset, rec_type, rec_len)| MkbRecord { + offset, + rec_type, + rec_len, + body: mkb[offset + 4..offset + rec_len].to_vec(), + }) + .collect() +} + +/// THE single MKB record-framing walker: yields `(offset, rec_type, rec_len)` +/// for each record — a 4-byte header (type byte + big-endian 24-bit length) +/// 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 +/// 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 + '_ { + let mut pos = 0usize; + std::iter::from_fn(move || { + if pos + 4 > mkb.len() { + return None; + } let rec_type = mkb[pos]; let rec_len = ((mkb[pos + 1] as usize) << 16) | ((mkb[pos + 2] as usize) << 8) | (mkb[pos + 3] as usize); if rec_type == 0 && rec_len == 0 { - break; + return None; } if rec_len < 4 || pos + rec_len > mkb.len() { - break; + return None; } - let body = mkb[pos + 4..pos + rec_len].to_vec(); - out.push(MkbRecord { - offset: pos, - rec_type, - rec_len, - body, - }); + let here = pos; pos += rec_len; - } - out + Some((here, rec_type, rec_len)) + }) } /// True iff `records` contains at least one Media Key Variant record @@ -773,6 +788,23 @@ mod tests { assert_eq!(recs[1].body, vec![1, 2, 3, 4]); } + #[test] + fn mkb_records_matches_walk_mkb_framing() { + // The lazy `mkb_records` iterator and the owning `walk_mkb` must agree on + // (offset, type, len) for every record — they share the one framing + // walker, and every keys.rs MKB walk now relies on this equivalence. + let mut mkb = vec![0x10, 0x00, 0x00, 0x06, 0xAA, 0xBB]; + mkb.extend_from_slice(&[0x05, 0x00, 0x00, 0x08, 1, 2, 3, 4]); + mkb.extend_from_slice(&[0x00, 0x00, 0x00, 0x00, 0xFF]); // terminator + trailing + let owned: Vec<(usize, u8, usize)> = walk_mkb(&mkb) + .iter() + .map(|r| (r.offset, r.rec_type, r.rec_len)) + .collect(); + let lazy: Vec<(usize, u8, usize)> = mkb_records(&mkb).collect(); + assert_eq!(lazy, owned); + assert_eq!(lazy, vec![(0, 0x10, 6), (6, 0x05, 8)]); + } + #[test] fn walk_mkb_be24_high_byte_is_honored() { // A record longer than 255 bytes needs the high BE24 byte. Build a diff --git a/src/disc/encrypt.rs b/src/disc/encrypt.rs index ea9b336..44a6736 100644 --- a/src/disc/encrypt.rs +++ b/src/disc/encrypt.rs @@ -327,10 +327,14 @@ impl Disc { .as_deref() .and_then(aacs::parse_content_cert); let bus_encryption = cc.as_ref().map(|c| c.bus_encryption).unwrap_or(false); + // No-cert default = UHD (V20 stride), matching `read_aacs_version` so the + // scanned `AacsState.version` and the out-of-band fetch agree. A wrong + // stride on the main resolve path fails loudly (sample validation) rather + // than silently, so the conservative V20 default is safe here too. let version = cc .as_ref() .map(|c| c.version.major()) - .unwrap_or(aacs::AACS_MAJOR_BD); + .unwrap_or(aacs::AACS_MAJOR_UHD); // OEM bus-key gate (wrong-keys guard). A bus-encrypted disc (Content // Certificate bus-encryption bit set) still carries bus encryption on diff --git a/src/disc/mod.rs b/src/disc/mod.rs index ec65764..22fbc03 100644 --- a/src/disc/mod.rs +++ b/src/disc/mod.rs @@ -1614,17 +1614,32 @@ impl Disc { /// [`crate::aacs::AACS_MAJOR_UHD`]) from the content certificate. Drives the /// `Unit_Key_RO.inf` parse stride (48-byte V10 vs 64-byte V20/V21), so the /// out-of-band key-fetch path parses `enc_title_keys` at the right stride (a - /// server VUK then derives the correct unit keys). Defaults to BD (V10) when - /// no content certificate is present. + /// server VUK then derives the correct unit keys). + /// + /// When no content certificate is readable/parseable, defaults to **UHD + /// (V20, 64-byte stride)** — the conservative choice the pre-1.2.0 fetch path + /// hardcoded — and logs it: a wrong stride here folds a server VUK against + /// mis-strided title keys (silent wrong unit keys), so a missing cert must + /// not quietly pick the V10 stride for a UHD disc. fn read_aacs_version(reader: &mut dyn SectorSource, udf_fs: &udf::UdfFs) -> u8 { - udf_fs + match udf_fs .read_file(reader, crate::aacs::PATH_CONTENT_CERT) .or_else(|_| udf_fs.read_file(reader, crate::aacs::PATH_CONTENT_CERT_ALT)) .ok() .as_deref() .and_then(crate::aacs::parse_content_cert) - .map(|c| c.version.major()) - .unwrap_or(crate::aacs::AACS_MAJOR_BD) + { + Some(c) => c.version.major(), + None => { + tracing::warn!( + target: "freemkv::disc", + phase = "scan_aacs_version", + "no readable AACS content certificate; defaulting to the V20/UHD \ + Unit_Key_RO stride (a VUK-from-server path would otherwise mis-stride)" + ); + crate::aacs::AACS_MAJOR_UHD + } + } } /// Read the AACS MKB's real record stream — NOT its zero padding. diff --git a/src/keysource.rs b/src/keysource.rs index bcdf7d7..ed68539 100644 --- a/src/keysource.rs +++ b/src/keysource.rs @@ -716,4 +716,52 @@ mod tests { assert!(ts_sync_destroyed(s), "every sample is a scrambled unit"); } } + + /// Audit #5 — a DISCRIMINATING test for the version→stride fix. A 2-key + /// `Unit_Key_RO.inf` whose SECOND key sits at the V20 (64-byte) offset; a V10 + /// (48-byte) parse reads a DIFFERENT region. Confirms `DiscInputsCtx` parses + /// at the stride for `inputs.version` — a swapped `from_major` branch or a + /// hardcoded stride (the exact bug 1.2.0 fixes) would fail this, where the + /// prior single-key fixtures passed regardless of stride. + #[test] + fn disc_inputs_ctx_parses_unit_keys_at_the_version_stride() { + use crate::aacs::{AACS_MAJOR_BD, AACS_MAJOR_UHD}; + const UK_POS: usize = 64; + let mut inf = vec![0u8; 200]; + inf[0..4].copy_from_slice(&(UK_POS as u32).to_be_bytes()); // uk_pos + inf[UK_POS..UK_POS + 2].copy_from_slice(&2u16.to_be_bytes()); // num_uk = 2 + let key0_at = UK_POS + 48; // first key — same for both strides + let key1_v10_at = key0_at + 48; // second key if parsed at V10 stride + let key1_v20_at = key0_at + 64; // second key if parsed at V20 stride + inf[key0_at..key0_at + 16].fill(0xA0); + inf[key1_v10_at..key1_v10_at + 16].fill(0x10); + inf[key1_v20_at..key1_v20_at + 16].fill(0x20); + + let base = DiscInputs { + disc_hash: String::new(), + volume_id: [0u8; 16], + version: AACS_MAJOR_UHD, + mkb: Vec::new(), + unit_key_ro: inf, + samples: Vec::new(), + volume_label: None, + }; + let k20 = DiscInputsCtx::new(&base).enc_title_keys().unwrap().to_vec(); + let v10_inputs = DiscInputs { + version: AACS_MAJOR_BD, + ..base.clone() + }; + let k10 = DiscInputsCtx::new(&v10_inputs) + .enc_title_keys() + .unwrap() + .to_vec(); + + assert_eq!(k20.len(), 2); + assert_eq!(k10.len(), 2); + assert_eq!(k20[0], [0xA0; 16], "first key is at +48 for both strides"); + assert_eq!(k10[0], [0xA0; 16]); + assert_eq!(k20[1], [0x20; 16], "V20 reads the 2nd key at +64"); + assert_eq!(k10[1], [0x10; 16], "V10 reads the 2nd key at +48"); + assert_ne!(k20[1], k10[1], "the parse stride follows inputs.version"); + } }