From e5d2e78c853fa36f1ba90c523aaa57a686309bab Mon Sep 17 00:00:00 2001 From: Matthew Jackson Date: Mon, 11 May 2026 11:29:48 -0700 Subject: [PATCH] disc/scan: surface AACS resolution error on Disc.aacs_error scan_with() collapsed every failure path from resolve_encryption() into None via .ok(), so callers couldn't tell the difference between "no KEYDB found", "KEYDB failed to parse", "disc hash not in KEYDB and fallback derivation failed", "AACS files unreadable on disc", and a handshake that rejected every host cert. autorip's UI was stuck printing "no decryption keys found (check KEYDB)" for all of them, which is a particularly bad message when the user has actually loaded a KEYDB and the real failure is something else. Changes: - New pub field Disc.aacs_error: Option. Populated by scan_with whenever encrypted && aacs.is_none(). Sentinel KeydbLoad path "" distinguishes the no-keydb case from a real load failure without adding a new Error variant (which would be a breaking change for downstream exhaustive matches). - tracing::warn in scan_with at scan_aacs_resolve_failed and scan_aacs_no_keydb, with error_code and keydb path for grepping. - tracing in do_handshake: keydb load failure, host-cert exhaustion (with cert count and last error code), VID read failure post-auth, and a debug-level success log. Lets us see whether handshake even got off the ground for a given disc. Test fixtures updated to set aacs_error: None. --- src/disc/encrypt.rs | 53 ++++++++++++++++++++++++-- src/disc/mod.rs | 51 ++++++++++++++++++++++--- tests/integration_progress_and_halt.rs | 1 + tests/pass_n_size_aware_skip.rs | 1 + 4 files changed, 96 insertions(+), 10 deletions(-) diff --git a/src/disc/encrypt.rs b/src/disc/encrypt.rs index d865849..effaa6f 100644 --- a/src/disc/encrypt.rs +++ b/src/disc/encrypt.rs @@ -23,29 +23,74 @@ impl Disc { use crate::aacs::{self, KeyDb}; let keydb_path = opts.resolve_keydb()?; - let keydb = KeyDb::load(&keydb_path).ok()?; + let keydb = match KeyDb::load(&keydb_path) { + Ok(db) => db, + Err(e) => { + tracing::warn!( + target: "freemkv::aacs", + phase = "handshake_keydb_load_failed", + io_error_kind = ?e.kind(), + keydb = %keydb_path.display(), + "KEYDB load failed; handshake skipped" + ); + return None; + } + }; + + let host_cert_count = keydb.host_certs.len(); + tracing::debug!( + target: "freemkv::aacs", + phase = "handshake_start", + host_cert_count, + keydb = %keydb_path.display(), + ); const MAX_CERT_ATTEMPTS: usize = 16; - for hc in keydb.host_certs.iter().take(MAX_CERT_ATTEMPTS) { + let mut last_err_code: Option = None; + for (idx, hc) in keydb.host_certs.iter().take(MAX_CERT_ATTEMPTS).enumerate() { match aacs::handshake::aacs_authenticate(session, &hc.private_key, &hc.certificate) { Ok(mut auth) => { let volume_id = match aacs::handshake::read_volume_id(session, &mut auth) { Ok(vid) => vid, - Err(_) => return None, // handshake succeeded but can't read VID + Err(e) => { + tracing::warn!( + target: "freemkv::aacs", + phase = "handshake_vid_read_failed", + cert_index = idx, + error_code = e.code(), + "auth ok but volume ID read failed" + ); + return None; + } }; let read_data_key = aacs::handshake::read_data_keys(session, &mut auth) .ok() .map(|(rdk, _)| rdk); + tracing::debug!( + target: "freemkv::aacs", + phase = "handshake_ok", + cert_index = idx, + has_read_data_key = read_data_key.is_some(), + ); return Some(HandshakeResult { volume_id, read_data_key, }); } - Err(_) => { + Err(e) => { + last_err_code = Some(e.code()); continue; } } } + tracing::warn!( + target: "freemkv::aacs", + phase = "handshake_all_certs_failed", + host_cert_count, + tried = host_cert_count.min(MAX_CERT_ATTEMPTS), + last_error_code = last_err_code, + "all host certs in KEYDB rejected by drive" + ); // All host certs failed — return None, not a fake success None } diff --git a/src/disc/mod.rs b/src/disc/mod.rs index 88d21ed..3678549 100644 --- a/src/disc/mod.rs +++ b/src/disc/mod.rs @@ -55,6 +55,11 @@ pub struct Disc { pub css: Option, /// Whether this disc requires decryption (AACS or CSS) pub encrypted: bool, + /// AACS resolution error when `encrypted` is true and `aacs` is None. + /// Lets callers distinguish "no KEYDB found", "KEYDB failed to parse", + /// "disc hash not in KEYDB", etc. None when AACS resolution wasn't + /// attempted (unencrypted disc) or succeeded. + pub aacs_error: Option, /// Content format (BD transport stream vs DVD program stream) pub content_format: ContentFormat, } @@ -1110,14 +1115,45 @@ impl Disc { let encrypted = udf_fs.find_dir("/AACS").is_some() || udf_fs.find_dir("/BDMV/AACS").is_some(); - let aacs = if encrypted { - if let Some(keydb_path) = opts.resolve_keydb() { - Self::resolve_encryption(&udf_fs, reader, &keydb_path, handshake.as_ref()).ok() - } else { - None + let (aacs, aacs_error) = if encrypted { + match opts.resolve_keydb() { + Some(keydb_path) => { + match Self::resolve_encryption(&udf_fs, reader, &keydb_path, handshake.as_ref()) + { + Ok(state) => (Some(state), None), + Err(e) => { + tracing::warn!( + target: "freemkv::disc", + phase = "scan_aacs_resolve_failed", + error_code = e.code(), + keydb = %keydb_path.display(), + handshake_ok = handshake.is_some(), + "AACS key resolution failed" + ); + (None, Some(e)) + } + } + } + None => { + tracing::warn!( + target: "freemkv::disc", + phase = "scan_aacs_no_keydb", + "encrypted disc but no KEYDB found in search paths" + ); + // Reuse KeydbLoad with sentinel path — adding a new Error + // variant would be a breaking change for downstream + // exhaustive matches. The path string makes the cause + // unambiguous to autorip's message switch. + ( + None, + Some(crate::error::Error::KeydbLoad { + path: String::from(""), + }), + ) + } } } else { - None + (None, None) }; // 3. Titles — BD (MPLS playlists) or DVD (IFO title sets) @@ -1174,6 +1210,7 @@ impl Disc { aacs, css, encrypted, + aacs_error, content_format, }) } @@ -3582,6 +3619,7 @@ mod tests { aacs: None, css: None, encrypted: false, + aacs_error: None, content_format: ContentFormat::BdTs, }; let gb = disc.capacity_gb(); @@ -3667,6 +3705,7 @@ mod tests { aacs: None, css: None, encrypted: false, + aacs_error: None, content_format: ContentFormat::BdTs, } } diff --git a/tests/integration_progress_and_halt.rs b/tests/integration_progress_and_halt.rs index 7870f3f..685b9d7 100644 --- a/tests/integration_progress_and_halt.rs +++ b/tests/integration_progress_and_halt.rs @@ -106,6 +106,7 @@ fn synthetic_disc(capacity_sectors: u32) -> Disc { aacs: None, css: None, encrypted: false, + aacs_error: None, content_format: ContentFormat::BdTs, } } diff --git a/tests/pass_n_size_aware_skip.rs b/tests/pass_n_size_aware_skip.rs index 9b445d9..72bb8c5 100644 --- a/tests/pass_n_size_aware_skip.rs +++ b/tests/pass_n_size_aware_skip.rs @@ -100,6 +100,7 @@ fn synthetic_disc(capacity_sectors: u32) -> Disc { aacs: None, css: None, encrypted: false, + aacs_error: None, content_format: ContentFormat::BdTs, } }