online, keydb: no env proxy; typed keyserver URL verdict; host certs best first

The key service never uses an environment proxy: it was resolved to the
service's pinned address, so every lookup failed. Keyserver URLs get a
typed verdict, and a test-hooks feature lets consumers' tests set the
last reachability. keydb hands out host certs best first, with no disc
input.

CHANGELOG.md

@@ -1,5 +1,25 @@
 # Changelog
 
+## [Unreleased]
+
+### Fixed
+
+- 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<N>` 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.
+
 ## [1.7.7] — 2026-09-26
 
 ### Maintenance

Cargo.toml

@@ -30,6 +30,10 @@ zip = { version = "8", default-features = false, features = ["deflate"] }
 # key material).
 tracing = "0.1"
 
+[features]
+# `set_last_decode_reachability`, for consumers' tests only (enable in dev-dependencies).
+test-hooks = []
+
 [profile.release]
 lto = "thin"
 codegen-units = 1

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<HostCert> {
         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<u32>) -> Vec<HostCert> {
+    // The keydb's host certs, best first. `mkb` is ignored: cert selection takes no disc input.
+    fn host_certs(&self, _mkb: Option<u32>) -> Vec<HostCert> {
         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<HostCert>| -> Vec<u8> { 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()));

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<u32>) -> Vec<HostCert> {
         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<N>` 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<HostCert> {
+        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!(

src/lib.rs

@@ -16,9 +16,11 @@ mod paths;
 
 pub use keydb::{KeydbSource, UpdateResult};
 pub use keydb_format::{DiscEntry, KeyDb};
+#[cfg(feature = "test-hooks")]
+pub use online::set_last_decode_reachability;
 pub use online::{
-    DecodeReachability, MIN_SAMPLE_UNITS, OnlineSource, take_last_decode_reachability,
-    validate_keyserver_url,
+    DecodeReachability, KeyserverUrlFault, KeyserverUrlRejection, MIN_SAMPLE_UNITS, OnlineSource,
+    check_keyserver_url, take_last_decode_reachability, validate_keyserver_url,
 };
 pub use paths::{default_keydb_path, existing_keydb_path, keydb_search_paths};
 
@@ -134,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<u32>) -> Vec<libfreemkv::aacs::types::HostCert> {
         self.sources
             .iter()

src/online.rs

@@ -243,7 +243,55 @@ fn resolve_and_guard(url: &str) -> Result<Vec<SocketAddr>, (GuardFail, String)>
 ///
 /// The *config-time* check; [`OnlineSource`] re-guards before each POST, closing the DNS-rebind window.
 pub fn validate_keyserver_url(url: &str) -> Result<(), String> {
-    resolve_and_guard(url).map(|_| ()).map_err(|(_, msg)| msg)
+    check_keyserver_url(url).map_err(|r| r.message)
+}
+
+/// Whether a rejected key-service URL can start working without a config change.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[non_exhaustive]
+pub enum KeyserverUrlFault {
+    /// Bad scheme, host or port, or a non-public address: retrying changes nothing.
+    Permanent,
+    /// The host did not resolve (DNS failure, timeout, lookup cap): may succeed later.
+    Temporary,
+}
+
+/// Why [`check_keyserver_url`] rejected a URL; `message` is [`validate_keyserver_url`]'s text.
+#[derive(Debug, Clone, PartialEq, Eq)]
+#[non_exhaustive]
+pub struct KeyserverUrlRejection {
+    pub fault: KeyserverUrlFault,
+    pub message: String,
+}
+
+impl KeyserverUrlRejection {
+    pub fn is_temporary(&self) -> bool {
+        self.fault == KeyserverUrlFault::Temporary
+    }
+}
+
+impl std::fmt::Display for KeyserverUrlRejection {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_str(&self.message)
+    }
+}
+
+impl std::error::Error for KeyserverUrlRejection {}
+
+impl From<(GuardFail, String)> for KeyserverUrlRejection {
+    fn from((fail, message): (GuardFail, String)) -> Self {
+        let fault = match fail {
+            GuardFail::Config => KeyserverUrlFault::Permanent,
+            GuardFail::Unreachable => KeyserverUrlFault::Temporary,
+        };
+        Self { fault, message }
+    }
+}
+
+/// [`validate_keyserver_url`] with a typed verdict: [`KeyserverUrlFault::Temporary`] only when
+/// the host lookup failed, so a caller can keep the source and retry at request time.
+pub fn check_keyserver_url(url: &str) -> Result<(), KeyserverUrlRejection> {
+    resolve_and_guard(url).map(|_| ()).map_err(Into::into)
 }
 
 // ureq's `ResolvedSocketAddrs` is a fixed 16-slot array; `push`ing a 17th
@@ -287,6 +335,9 @@ fn hardened_agent(pinned: Vec<SocketAddr>) -> ureq::Agent {
         // a body deadline a stalled reply hangs the POST forever. The JSON key
         // answer arrives in one read, so a TOTAL cap is the right shape here.
         .timeout_recv_body(Some(Duration::from_secs(TIMEOUT_SECS)))
+        // Never the env proxy: PinnedResolver would also answer the proxy's lookup with the key
+        // service's address, so with HTTP(S)_PROXY/ALL_PROXY set every key lookup failed.
+        .proxy(None)
         .build();
     // `with_parts`, never `new_with_config` — see [`PinnedResolver`].
     ureq::Agent::with_parts(config, DefaultConnector::new(), PinnedResolver(pinned))
@@ -495,6 +546,13 @@ pub fn take_last_decode_reachability() -> Option<DecodeReachability> {
     LAST_DECODE_REACHABILITY.with(std::cell::Cell::take)
 }
 
+/// Set this thread's decode-reachability slot, as a real decode POST would. Test hook (feature
+/// `test-hooks`): lets a caller test its [`take_last_decode_reachability`] handling offline.
+#[cfg(any(test, feature = "test-hooks"))]
+pub fn set_last_decode_reachability(outcome: Option<DecodeReachability>) {
+    LAST_DECODE_REACHABILITY.with(|c| c.set(outcome));
+}
+
 // Map a key-service HTTP status into the operator action it implies: 401/403
 // fix credentials, 429 back off, 5xx wait — none of them is "no key" (the
 // genuine miss is a 200 with an empty body), the original bug this fixes.
@@ -994,6 +1052,89 @@ mod tests {
         );
     }
 
+    // ureq 3 defaults to `Proxy::try_from_env()`; PinnedResolver would send the proxy connection to
+    // the key service's address, failing every lookup. Checked in a child so no test mutates env.
+    #[test]
+    fn the_key_service_agent_never_uses_an_environment_proxy() {
+        const CHILD: &str = "FMKV_KS_PROXY_CHILD";
+        const NAME: &str = "online::tests::the_key_service_agent_never_uses_an_environment_proxy";
+        if std::env::var_os(CHILD).is_some() {
+            assert!(
+                hardened_agent(Vec::new()).config().proxy().is_none(),
+                "the pinned agent picked up a proxy from the environment"
+            );
+            return;
+        }
+        let out = std::process::Command::new(std::env::current_exe().unwrap())
+            .args([NAME, "--exact", "--test-threads=1"])
+            .env(CHILD, "1")
+            .env("ALL_PROXY", "http://proxy.example:3128")
+            .env("HTTPS_PROXY", "http://proxy.example:3128")
+            .env("HTTP_PROXY", "http://proxy.example:3128")
+            .output()
+            .unwrap();
+        let stdout = String::from_utf8_lossy(&out.stdout);
+        assert!(
+            out.status.success() && stdout.contains("1 passed"),
+            "child run failed:\n{stdout}\n{}",
+            String::from_utf8_lossy(&out.stderr)
+        );
+    }
+
+    // The typed verdict autorip needs instead of matching message text: a standing fault
+    // (scheme, host, port, blocked address) is Permanent; only a failed lookup is Temporary.
+    #[test]
+    fn check_keyserver_url_types_permanent_rejections() {
+        for url in [
+            "http://8.8.8.8/keys",
+            "ftp://example.com/keys",
+            "https:///keys",
+            "https://8.8.8.8:notaport/keys",
+            "https://[::1/keys",
+            "https://127.0.0.1/keys",
+            "https://169.254.169.254/latest/meta-data",
+        ] {
+            let r = check_keyserver_url(url).expect_err(url);
+            assert_eq!(r.fault, KeyserverUrlFault::Permanent, "{url}");
+            assert!(!r.is_temporary(), "{url}");
+            assert_eq!(Err(r.message.clone()), validate_keyserver_url(url), "{url}");
+            assert_eq!(r.to_string(), r.message);
+        }
+        assert_eq!(check_keyserver_url("https://8.8.8.8/keys"), Ok(()));
+    }
+
+    #[test]
+    fn a_failed_lookup_is_a_temporary_rejection() {
+        // Offline: every GuardFail::Unreachable reason resolve_and_guard produces.
+        for msg in [
+            "DNS resolution timed out",
+            "could not resolve host: failed to lookup address information",
+            "host did not resolve to any address",
+            "too many concurrent DNS resolutions in flight for this host",
+        ] {
+            let r = KeyserverUrlRejection::from((GuardFail::Unreachable, msg.to_string()));
+            assert_eq!(r.fault, KeyserverUrlFault::Temporary, "{msg}");
+            assert!(r.is_temporary());
+            assert_eq!(r.message, msg);
+        }
+        let c = KeyserverUrlRejection::from((GuardFail::Config, "URL has no host".into()));
+        assert_eq!(c.fault, KeyserverUrlFault::Permanent);
+    }
+
+    // Callers test their verdict mapping with the hook instead of a real DNS lookup.
+    #[test]
+    fn the_decode_reachability_hook_plants_what_take_reads() {
+        set_last_decode_reachability(Some(DecodeReachability::Status(422)));
+        assert_eq!(
+            take_last_decode_reachability(),
+            Some(DecodeReachability::Status(422))
+        );
+        assert_eq!(take_last_decode_reachability(), None);
+        set_last_decode_reachability(Some(DecodeReachability::Transport));
+        set_last_decode_reachability(None);
+        assert_eq!(take_last_decode_reachability(), None);
+    }
+
     // ── bearer_header ──────────────────────────────────────────────────────
 
     #[test]

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);