From 05281249d1695f63c02ec53b3abf27d4f68608dc Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Sun, 27 Sep 2026 17:15:22 -0700 Subject: [PATCH] keydb: hand out host certs best first, with no disc input Unannotated certs in keydb order, then revoked ones by latest revocation. The trait ignores its mkb hint; KeyDb::host_certs(mkb) is deprecated. --- CHANGELOG.md | 9 +++++++ src/keydb.rs | 59 +++++++++++++++++++++++++++++++++++--------- src/keydb_format.rs | 18 ++++++++++++++ src/lib.rs | 6 ++--- tests/key_sources.rs | 7 +++--- 5 files changed, 81 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ddae160..3bb6f74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,17 @@ - The online key service ignores `HTTP_PROXY`/`HTTPS_PROXY`/`ALL_PROXY`: with one set, every key lookup failed (the pinned resolver also answered the proxy's address, so the proxy request went to the key service). +### Changed + +- `KeydbSource` hands out host certs best first, with no disc input: certs with no `Revoked in MKBv` note in keydb order, then revoked ones by latest revocation. The inherent `host_certs()` and the `KeySource::host_certs` trait method both return this order; the trait ignores its `mkb` argument. + +### Deprecated + +- `KeyDb::host_certs(mkb)`: use `KeyDb::host_certs_ranked`. + ### Added +- `KeyDb::host_certs_ranked`. - `check_keyserver_url` with `KeyserverUrlRejection`/`KeyserverUrlFault`: a typed Permanent vs Temporary verdict, so callers need not match `validate_keyserver_url`'s text. - `test-hooks` feature exposing `set_last_decode_reachability`, to test decode-verdict handling without a network. diff --git a/src/keydb.rs b/src/keydb.rs index 6af4633..dc5bba7 100644 --- a/src/keydb.rs +++ b/src/keydb.rs @@ -324,13 +324,11 @@ impl KeydbSource { /// live-drive scan as `DriveCredentials` for the AACS handshake. Empty if /// the keydb is missing/unreadable or carries no host cert. /// - /// Inherent, no-MKB form: this is used by the **scan-options** builder, - /// which runs before the disc's MKB generation is known, so no revocation - /// filtering is applied (passes `None`). The [`KeySource::host_certs`] TRAIT - /// method wires the real MKB generation through for revocation filtering. + /// Best first ([`KeyDb::host_certs_ranked`]): this is what the scan-options + /// builder hands the live-drive handshake as `DriveCredentials`. pub fn host_certs(&self) -> Vec { match self.cached_db() { - Ok(db) => db.host_certs(None), + Ok(db) => db.host_certs_ranked(), // No error channel here (the scan-options builder wants a list), so // the failure can only be LOGGED — but it must not be invisible. Err(e) => { @@ -631,11 +629,10 @@ impl KeySource for KeydbSource { } } - // Expose the keydb's host certs through the trait, wiring the disc's MKB generation through - // for revocation filtering. - fn host_certs(&self, mkb: Option) -> Vec { + // The keydb's host certs, best first. `mkb` is ignored: cert selection takes no disc input. + fn host_certs(&self, _mkb: Option) -> Vec { match self.cached_db() { - Ok(db) => db.host_certs(mkb), + Ok(db) => db.host_certs_ranked(), // Vec-returning trait method: log the failure, return nothing. Err(e) => { let _ = self.load_failure(&e); @@ -1192,6 +1189,46 @@ mod tests { ); } + /// Best-first with no disc input: unannotated certs in file order, then revoked + /// ones by latest revocation. The trait ignores its MKB argument. + #[test] + fn host_certs_ranked_orders_unannotated_then_latest_revocation_first() { + let dir = scratch("ranked"); + let path = dir.join("keydb.cfg"); + let hc = |marker: u8, note: &str| { + format!( + "| HC | HOST_PRIV_KEY 0x{} | HOST_CERT 0x{}{}\n", + "00".repeat(20), + format!("{marker:02x}").repeat(92), + note + ) + }; + let cfg = [ + hc(0x68, " ; Revoked in MKBv68"), + hc(0xA1, ""), + hc(0x72, " ; Revoked in MKBv72"), + hc(0xB2, ""), + ] + .concat(); + std::fs::write(&path, cfg).unwrap(); + let src = KeydbSource::new(&path); + let markers = + |certs: Vec| -> Vec { certs.iter().map(|c| c.certificate[0]).collect() }; + let want = vec![0xA1, 0xB2, 0x72, 0x68]; + assert_eq!( + markers(src.host_certs()), + want, + "inherent (production credentials)" + ); + assert_eq!( + markers(KeySource::host_certs(&src, Some(99))), + want, + "trait ignores mkb" + ); + assert_eq!(markers(KeySource::host_certs(&src, None)), want); + std::fs::remove_dir_all(&dir).ok(); + } + /// No keydb → no host credentials, not an error (inherent and trait forms). #[test] fn host_certs_empty_when_keydb_missing() { @@ -1201,8 +1238,8 @@ mod tests { assert!(KeySource::host_certs(&src, Some(68)).is_empty()); } - /// The TRAIT `host_certs` surfaces a `| HC |` row and now wires the MKB - /// generation through. Placeholder all-zero material (never a real key). + /// The TRAIT `host_certs` surfaces a `| HC |` row for any `mkb` argument. + /// Placeholder all-zero material (never a real key). #[test] fn trait_host_certs_returns_keydb_hc_row() { let dir = std::env::temp_dir().join(format!("fmk_hc_{}", std::process::id())); diff --git a/src/keydb_format.rs b/src/keydb_format.rs index fd1f864..0224a11 100644 --- a/src/keydb_format.rs +++ b/src/keydb_format.rs @@ -477,6 +477,7 @@ impl KeyDb { /// is `None` the disc's generation is unknown and cannot be filtered, so /// every cert is returned; certs with no revocation annotation are always /// returned. + #[deprecated(note = "use host_certs_ranked; host cert selection takes no disc input")] pub fn host_certs(&self, mkb: Option) -> Vec { self.host_certs .iter() @@ -489,6 +490,19 @@ impl KeyDb { .collect() } + /// Every host cert, best first, with no disc input: certs with no `Revoked in + /// MKBv` note in file order, then revoked ones by latest revocation (a later + /// revocation is the better bet without knowing the disc's generation). + pub fn host_certs_ranked(&self) -> Vec { + let mut ranked: Vec<&KeydbHostCert> = self.host_certs.iter().collect(); + // Stable sort: ties (every unannotated cert, equal revocations) keep file order. + ranked.sort_by_key(|hc| { + let r = hc.revoked_at_mkb; + (r.is_some(), std::cmp::Reverse(r.unwrap_or(0))) + }); + ranked.into_iter().map(|hc| hc.cert.clone()).collect() + } + /// Standalone keydb accessor: the disc's Volume ID (the keydb `I` token), /// looked up by the same disc-hash form [`Self::find_disc`] accepts. Pure /// file lookup; no crypto/derivation. @@ -1913,6 +1927,7 @@ mod tests { // ── Host-cert revocation: parse + host_certs(mkb) filter ──────────────── #[test] + #[allow(deprecated)] fn host_cert_revoked_parses_and_filters_by_mkb() { let revoked_line = format!( "| HC | HOST_PRIV_KEY 0x{} | HOST_CERT 0x{} ; Revoked in MKBv72", @@ -1942,6 +1957,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn host_cert_without_revocation_included_for_all_mkb() { let line = format!( "| HC | HOST_PRIV_KEY 0x{} | HOST_CERT 0x{}", @@ -1958,6 +1974,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn hc2_revocation_propagates_when_hc_has_none() { // The HC line carries no annotation; the revocation lives on the HC2 // line. The combined cert must still be filtered by that generation @@ -2178,6 +2195,7 @@ mod tests { /// so the returned SET is identified exactly, not merely counted. A cert /// revoked at generation R is usable iff the disc's generation is `< R`. #[test] + #[allow(deprecated)] fn host_certs_filters_several_certs_across_mkb_generations() { let hc = |marker: u8, note: &str| { format!( diff --git a/src/lib.rs b/src/lib.rs index e4ec8f6..da15f00 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -136,9 +136,9 @@ impl KeySource for MultiSource { first_non_empty(&self.sources, |s, c| s.get_fmts_indexes(c), ctx) } - /// UNION every inner source's host certs (filtered at the given MKB - /// generation). Without this a composed source would hide an inner source's - /// cert from the OEM cert-auth route — the gap this fixes. + /// UNION every inner source's host certs, in source order (`mkb` is passed + /// through unchanged). Without this a composed source would hide an inner + /// source's cert from the OEM cert-auth route — the gap this fixes. fn host_certs(&self, mkb: Option) -> Vec { self.sources .iter() diff --git a/tests/key_sources.rs b/tests/key_sources.rs index 107dc84..b1d77b3 100644 --- a/tests/key_sources.rs +++ b/tests/key_sources.rs @@ -138,7 +138,7 @@ fn keydb_source_label_is_keydb() { assert_eq!(KeydbSource::new("/nonexistent/keydb.cfg").label(), "keydb"); } -// ── KeydbSource: MKB-aware host-cert serving from a file ──────────────────── +// ── KeydbSource: host-cert serving from a file ────────────────────────────── #[test] fn keydb_source_serves_host_cert_from_hc_row() { @@ -152,14 +152,13 @@ fn keydb_source_serves_host_cert_from_hc_row() { let path = s.write("keydb.cfg", &line); let src = KeydbSource::new(&path); - // Inherent (no-MKB, scan-options) form. + // Inherent (scan-options) form. assert_eq!( src.host_certs().len(), 1, "inherent host_certs sees the HC row" ); - // Trait form now wires the MKB generation through (no revocation annotation - // → always returned). + // Trait form: `mkb` is ignored; the ranked list is returned. let via_trait = KeySource::host_certs(&src, Some(70)); assert_eq!(via_trait.len(), 1, "trait host_certs sees the HC row"); assert_eq!(via_trait[0].certificate.len(), 92);