mirror of
https://github.com/freemkv/libfreemkv.git
synced 2026-10-07 18:13:10 +00:00
aacs/disc/drive: fix bus-key gate credit, shift guard, tray-unlock log
disc/encrypt: on the disc-keyed cert route, drive_unlocked was derived from is_drive_unlocker(matched), which is provably always false - run_bus only ever dispatches the disc unlockers (AACS/DVD), never a firmware/drive unlocker. Hardcode false with an honest comment; the genuine drive-unlock case (drive_unlocked, no read_data_key) is the OEM-VID short-circuit and the bus-key gate already credits it. Gate the now test-only is_drive_unlocker behind cfg(test) and add a regression test through resolve_vid_only. aacs/derive: resolve_dk_node looped 0..u_mask_shift shifting 1u32 << b, which panics/wraps when the disc/keydb-controlled u_mask_shift is >= 32. Cap the search at the 32 real bit positions. drive: unlock_tray dropped its SCSI result with let _; warn! on failure so a stuck medium-removal PREVENT lock is visible. Fix the stale probe_disc doc (now a no-op; calibration moved into the unlocker).
This commit is contained in:
+17
-1
@@ -306,7 +306,11 @@ pub(crate) fn resolve_dk_node(
|
||||
uv: u32,
|
||||
u_mask_shift: u8,
|
||||
) -> Option<DeviceKey> {
|
||||
for b in 0..u_mask_shift {
|
||||
// `u_mask_shift` is a disc/keydb-controlled u8: bit positions 32..=255 do not
|
||||
// exist in a u32, and `1u32 << b` with b >= 32 panics (debug) / wraps (release).
|
||||
// Cap the search at the 32 real bit positions — same >= 32 guard the mask shifts
|
||||
// above use — so a bad disc can never trip the shift.
|
||||
for b in 0..u_mask_shift.min(32) {
|
||||
let dk = DeviceKey {
|
||||
key: *key,
|
||||
node: ((uv ^ (1u32 << b)) & 0xFFFF) as u16,
|
||||
@@ -910,6 +914,18 @@ mod position_recovery_tests {
|
||||
);
|
||||
}
|
||||
|
||||
// `u_mask_shift` is a disc/keydb-controlled u8; a value >= 32 would drive
|
||||
// `1u32 << b` past a u32's width and panic (debug) / wrap (release). The `.min(32)`
|
||||
// guard must bound the search so a hostile disc can never trip the shift.
|
||||
#[test]
|
||||
fn resolve_dk_node_does_not_shift_past_u32_on_a_huge_u_mask_shift() {
|
||||
// Empty MKB → the inner derive returns None every iteration, so the loop runs
|
||||
// its full range; with u_mask_shift = 200 the unguarded loop panics at b = 32.
|
||||
let dk = resolve_dk_node(&[], &[0u8; 16], 0, 200)
|
||||
.expect("must fall back to the node itself, not panic on the shift");
|
||||
assert_eq!(dk.u_mask_shift, 200);
|
||||
}
|
||||
|
||||
// probe::mkb_mk_dv feeds km_verifies for reproduction harnesses; a fixed
|
||||
// block would "verify" against a record no disc carries, and always-None
|
||||
// would make every verification report "unverifiable".
|
||||
|
||||
+49
-8
@@ -103,7 +103,10 @@ impl AacsCertUnlocker<'_> {
|
||||
// `session` across `scsi_mut()` + `&drive_id`, so clone the cheap identity.
|
||||
let drive_id = session.drive_id.clone();
|
||||
let fu_certs = crate::unlock_bridge::map_host_certs(&host_certs);
|
||||
let (matched, unlock_res) = crate::unlock_bridge::run_bus(
|
||||
// `matched` (the unlocker name) is intentionally unused: on this disc-keyed
|
||||
// cert route it is only ever "AACS"/"DVD"/"", never a drive unlock — see the
|
||||
// `drive_unlocked: false` note below.
|
||||
let (_matched, unlock_res) = crate::unlock_bridge::run_bus(
|
||||
session.scsi_mut(),
|
||||
&drive_id,
|
||||
freemkv_unlock::DiscKind::Aacs,
|
||||
@@ -127,13 +130,15 @@ impl AacsCertUnlocker<'_> {
|
||||
// AACS-specific "why the read_data_key read failed" diagnostic does
|
||||
// not cross the seam. The bus-key gate keys off presence, not cause.
|
||||
read_data_key_err: None,
|
||||
// Derive from WHICH unlocker claimed the drive rather than hardcoding
|
||||
// false: the disc-keyed cert route normally removes bus encryption via
|
||||
// the read_data_key (AKE), so the gate credits it through
|
||||
// `read_data_key.is_some()`. But if a firmware/drive unlocker in the
|
||||
// dispatch set is what matched, the drive is already unlocked and the
|
||||
// gate must credit that instead — the field now tracks reality.
|
||||
drive_unlocked: crate::unlock_bridge::is_drive_unlocker(matched),
|
||||
// Always false on the cert route: `run_bus` dispatches ONLY the disc-
|
||||
// keyed unlockers (`disc_unlockers` = AACS, DVD), so `matched` is only
|
||||
// ever "AACS"/"DVD"/"" — never a firmware/drive unlocker (freemkv, LD,
|
||||
// Renesas). The disc-keyed cert route removes bus encryption via the
|
||||
// read_data_key (AKE), which the gate credits through
|
||||
// `read_data_key.is_some()`. A genuine DRIVE unlock cannot surface here;
|
||||
// it arrives via the OEM-VID short-circuit in `do_handshake_cert`, which
|
||||
// sets `drive_unlocked: true` for exactly that (rdk-less) case.
|
||||
drive_unlocked: false,
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -834,6 +839,42 @@ mod tests {
|
||||
assert_eq!(st.read_data_key, Some([0x22u8; 16]));
|
||||
}
|
||||
|
||||
/// Finding: `is_drive_unlocker(matched)` on the cert route was provably always
|
||||
/// false — `run_bus` dispatches ONLY the disc-keyed AACS/DVD unlockers, never a
|
||||
/// firmware/drive unlocker — so a cert handshake's `drive_unlocked` is always
|
||||
/// false and bus removal there is credited solely via `read_data_key`. The
|
||||
/// GENUINE drive-unlock case (drive_unlocked:true, NO read_data_key) is the
|
||||
/// OEM/VID-only path, and the bus-key gate MUST credit it rather than
|
||||
/// hard-error `AacsBusKeyUnavailable`.
|
||||
#[test]
|
||||
fn drive_unlock_without_read_data_key_removes_bus_encryption() {
|
||||
// The cert route can never observe a firmware/drive unlocker: the only names
|
||||
// `run_bus` can return ("AACS"/"DVD"/"") classify as NOT a drive unlock, so
|
||||
// the replaced `is_drive_unlocker(matched)` was always false.
|
||||
for name in ["AACS", "DVD", ""] {
|
||||
assert!(
|
||||
!crate::unlock_bridge::is_drive_unlocker(name),
|
||||
"the disc-keyed cert route can never credit a drive unlock ({name:?})"
|
||||
);
|
||||
}
|
||||
// The real case: a bus-encrypted disc unlocked AT THE DRIVE (drive_unlocked)
|
||||
// with no cert read_data_key must resolve OK, not AacsBusKeyUnavailable.
|
||||
let (mut disc, udf) = disc_with_cert(0x01, true);
|
||||
let hs = HandshakeResult {
|
||||
volume_id: [0x11u8; 16],
|
||||
read_data_key: None,
|
||||
read_data_key_err: None,
|
||||
drive_unlocked: true,
|
||||
};
|
||||
let st = Disc::resolve_vid_only(&udf, &mut disc, Some(&hs))
|
||||
.expect("a drive-unlocked disc removes bus encryption even without a read_data_key");
|
||||
assert!(st.bus_encryption);
|
||||
assert_eq!(
|
||||
st.read_data_key, None,
|
||||
"the drive-unlock path carries no read_data_key"
|
||||
);
|
||||
}
|
||||
|
||||
/// ISO scan (handshake None) of a bus_encryption disc → Ok. Bus encryption
|
||||
/// was already removed at read time; the gate must NOT fire without a
|
||||
/// handshake (no UHD-ISO-mux regression).
|
||||
|
||||
+18
-5
@@ -541,9 +541,11 @@ impl Drive {
|
||||
r
|
||||
}
|
||||
|
||||
/// Probe disc surface so the drive firmware learns optimal read speeds
|
||||
/// per region. After this the host reads at max speed and the drive
|
||||
/// manages zones internally.
|
||||
/// No-op retained for API stability. Disc-speed calibration is now
|
||||
/// unlocker-specific and runs inside the unlocker's `unlock()` at `init()`
|
||||
/// (for every disc, including DVD), so there is nothing to do here — this
|
||||
/// only emits the begin/end trace span. Kept so existing callers and the
|
||||
/// `probe_disc_without_unlocker_is_ok_noop` test need not change.
|
||||
pub fn probe_disc(&mut self) -> Result<()> {
|
||||
let t0 = std::time::Instant::now();
|
||||
tracing::info!(target: "freemkv::drive", phase = "probe_disc", "begin");
|
||||
@@ -1026,10 +1028,21 @@ impl Drive {
|
||||
0x00,
|
||||
];
|
||||
let mut buf = [0u8; 0];
|
||||
let _ =
|
||||
// Best-effort (the tray-unlock is advisory), but a failure is worth a warn!
|
||||
// rather than a silent `let _`: a stuck PREVENT lock is a real symptom the
|
||||
// operator otherwise never sees until the tray won't open.
|
||||
if let Err(e) =
|
||||
self.scsi
|
||||
.as_mut()
|
||||
.execute(&allow, crate::scsi::DataDirection::None, &mut buf, 5_000);
|
||||
.execute(&allow, crate::scsi::DataDirection::None, &mut buf, 5_000)
|
||||
{
|
||||
tracing::warn!(
|
||||
target: "freemkv::drive",
|
||||
phase = "unlock_tray",
|
||||
error_code = e.code(),
|
||||
"Failed to clear the medium-removal PREVENT lock; the tray may stay locked until the drive is power-cycled."
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Eject the disc tray. Unlocks first, then ejects.
|
||||
|
||||
@@ -154,6 +154,13 @@ fn disc_unlockers(host_certs: Vec<fu::HostCert>) -> Vec<Box<dyn fu::Unlocker>> {
|
||||
// Whether `name` (a matched unlocker's `.name()`) is a firmware/drive unlocker,
|
||||
// i.e. one whose success means bus encryption is already removed at the drive.
|
||||
// Derived from the real unlocker set, never a hardcoded name list.
|
||||
//
|
||||
// Test-only: the disc-keyed cert route (`run_bus`) can only ever match the disc
|
||||
// unlockers (AACS/DVD), so no production path classifies a `matched` name through
|
||||
// here — the AACS cert handshake credits bus removal via its `read_data_key`, and a
|
||||
// genuine firmware drive-unlock is surfaced separately by the OEM-VID short-circuit.
|
||||
// Retained so the disjointness invariant stays under test.
|
||||
#[cfg(test)]
|
||||
pub(crate) fn is_drive_unlocker(name: &str) -> bool {
|
||||
firmware_unlockers().iter().any(|u| u.name() == name)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user