fix(keysources): resolve own-repo audit findings (v1.6.0->dev)

online.rs: - validate_keyserver_url/resolve_and_guard now reject any non-https:// scheme as a Config fault, so a cleartext misconfig is caught ONCE at config time instead of failing every rip via query()'s runtime https check. - is_blocked_ip: add 198.18.0.0/15 (RFC 2544 benchmarking) and 192.0.0.0/24 (RFC 6890 IETF protocol assignments, incl. 192.0.0.170/171) to the SSRF guard. - DNS in-flight cap is now PER HOST, not process-global, so black-holed lookups to one dead keyserver can't starve resolution of a healthy different keyserver. - reject a non-empty, non-":port" tail after a bracketed IPv6 authority (e.g. https://[::1]extra/) instead of silently dropping it. keydb.rs / keydb_format.rs: - MAX_KEYDB_BYTES defined once in keydb_format and re-used by keydb.rs. - CacheEntry::is_settled falls back to inode identity + stamped_at age when the mtime is absent (mtime-less FS) or in the future (clock skew), so the ~62 MiB keydb no longer re-parses on every lookup. - save()'s entry-count guard counts DK/PK/HC/HC2 rows via the real parsers (is_parseable_entry_line), so a prefixed-but-malformed row can't pass entries>0. Tests added for each fix.
2026-10-07 18:13:11 +00:00 · 2026-09-15 10:30:50 -07:00
parent ae8e5ade3c
commit 531e3a40bf
5 changed files with 570 additions and 59 deletions
@@ -1276,3 +1276,7 @@ version = "1.7.1"
 [[patch.unused]]
 name = "freemkv-i18n"
 version = "1.7.1"
+
+[[patch.unused]]
+name = "freemkv-unlock"
+version = "1.7.1"
@@ -20,11 +20,11 @@ use libfreemkv::keysource::ResolveCtx;
 use libfreemkv::{Error, KeySource};

 use crate::keydb_format::KeyDb;
-
 // Upper bound on decompressed keydb size (decompression-bomb cap): a tiny
-// zip/gz could otherwise inflate to GiB and OOM the refresh thread. Mirrors
-// keydb_format::MAX_KEYDB_BYTES (the on-disk load cap); keep the two equal.
-const MAX_KEYDB_BYTES: u64 = 128 * 1024 * 1024;
+// zip/gz could otherwise inflate to GiB and OOM the refresh thread. The SAME
+// constant `KeyDb::load` uses as the on-disk load cap — defined once in
+// keydb_format so the two can never drift apart.
+use crate::keydb_format::MAX_KEYDB_BYTES;

 /// Result of a KEYDB save/update -- path written, entry count, and byte size.
 #[derive(Debug)]
@@ -96,10 +96,27 @@ impl CacheEntry {
    // Proof, cost, and the clock caveat are in docs/keydb.md#settle-proof;
    // don't weaken this without re-reading them.
    fn is_settled(&self, granularity: std::time::Duration) -> bool {
-        self.stamp
+        match self
+            .stamp
            .modified
            .and_then(|m| self.stamped_at.duration_since(m).ok())
-            .is_some_and(|age| age >= granularity)
+        {
+            // The normal case: a real mtime that was already `granularity` in
+            // the past when we stamped it, so any later write bumps the mtime
+            // past our stamp and is detected. Trust it.
+            Some(age) => age >= granularity,
+            // No mtime at all (an mtime-less filesystem) OR an mtime in the
+            // FUTURE relative to stamped_at (clock skew) — `duration_since`
+            // yielded None either way. The mtime is useless as a change
+            // discriminator, so fall back to the inode identity (dev+ino, which
+            // an atomic rename always changes) and treat the entry as settled
+            // only once THIS observation has itself aged past the granularity.
+            // Without this, such a file's cache entry was NEVER settled, forcing
+            // a full ~62 MiB re-parse on every get_unit_keys/host_certs call.
+            None => std::time::SystemTime::now()
+                .duration_since(self.stamped_at)
+                .is_ok_and(|age| age >= granularity),
+        }
    }
 }

@@ -278,16 +295,12 @@ impl KeydbSource {

        let entries = text
            .lines()
-            .filter(|l| {
-                let t = l.trim();
-                // Mirror KeyDb::parse's disc-entry rule EXACTLY by CALLING it,
-                // so save() never persists content that parses to zero usable
-                // entries. See docs/keydb.md#save-mirror-parse.
-                crate::keydb_format::is_disc_entry_line(t)
-                    || t.starts_with("| DK")
-                    || t.starts_with("| PK")
-                    || t.starts_with("| HC")
-            })
+            // Mirror KeyDb::parse's ACCEPTANCE rule EXACTLY by running the same
+            // parsers (not just a `| DK`/`| PK`/`| HC` prefix check): a row with
+            // the right prefix but malformed hex / a short cert parses to
+            // nothing, so counting it by prefix let unparseable content slip
+            // past the `entries > 0` guard. See docs/keydb.md#save-mirror-parse.
+            .filter(|l| crate::keydb_format::is_parseable_entry_line(l))
            .count();

        if entries == 0 {
@@ -995,6 +1008,54 @@ mod tests {
        assert_eq!(KeydbSource::new("/nonexistent/keydb.cfg").label(), "keydb");
    }

+    // A cache entry whose stamp has NO mtime (mtime-less filesystem) or a mtime
+    // in the FUTURE (clock skew) must not be stuck un-settled forever — that
+    // forced a full ~62 MiB re-parse on every lookup. It now settles via the
+    // inode-identity fallback once the observation has aged past the granularity.
+    #[test]
+    fn is_settled_falls_back_when_mtime_is_absent_or_in_the_future() {
+        use std::time::{Duration, SystemTime};
+        let entry = |modified: Option<SystemTime>, stamped_at: SystemTime| CacheEntry {
+            stamp: FileStamp {
+                dev: 1,
+                ino: 2,
+                len: 3,
+                modified,
+            },
+            stamped_at,
+            db: Arc::new(KeyDb::empty()),
+            stats: crate::keydb_format::ParseStats::default(),
+        };
+        let gran = Duration::from_secs(2);
+        let now = SystemTime::now();
+
+        // None mtime, stamped well in the past → settled via the fallback.
+        assert!(
+            entry(None, now - Duration::from_secs(3600)).is_settled(gran),
+            "a mtime-less entry must settle once it has aged past the granularity"
+        );
+        // None mtime, just stamped → still inside the settle window, NOT settled.
+        assert!(
+            !entry(None, now).is_settled(gran),
+            "a freshly stamped mtime-less entry is too young to trust yet"
+        );
+        // A FUTURE mtime (clock skew) with an old stamp → still settles via the
+        // fallback rather than being wedged un-settled forever.
+        assert!(
+            entry(
+                Some(now + Duration::from_secs(3600)),
+                now - Duration::from_secs(3600)
+            )
+            .is_settled(gran),
+            "a future mtime must not wedge the entry un-settled forever"
+        );
+        // Sanity: a real past mtime still settles the normal way.
+        assert!(
+            entry(Some(now - Duration::from_secs(3600)), now).is_settled(gran),
+            "a normal past mtime still settles"
+        );
+    }
+
    /// No keydb → no host credentials, not an error (inherent and trait forms).
    #[test]
    fn host_certs_empty_when_keydb_missing() {
@@ -1431,6 +1492,42 @@ mod tests {
        assert!(matches!(src.save(garbage), Err(Error::KeydbInvalid)));
    }

+    /// DK/PK/HC rows with the RIGHT prefix but MALFORMED content (bad hex, a
+    /// short cert) parse to nothing, so `save` must count zero real entries and
+    /// reject — a prefix-only count let unparseable content persist past the
+    /// `entries > 0` guard.
+    #[test]
+    fn save_rejects_prefixed_but_unparseable_dk_pk_hc_rows() {
+        let dir = scratch("save-prefixed-junk");
+        let src = KeydbSource::new(dir.join("k.cfg"));
+        // Each line has a valid keydb PREFIX but content no parser accepts:
+        //  - PK: not 16 bytes of hex
+        //  - DK: DEVICE_KEY not valid hex
+        //  - HC: cert far shorter than the 92-byte minimum
+        let junk = b"| PK | 0xnothex\n\
+                     | DK | DEVICE_KEY 0xZZ | DEVICE_NODE 0x0800 | KEY_UV 0x00000400 | KEY_U_MASK_SHIFT 0x17\n\
+                     | HC | HOST_PRIV_KEY 0x00 | HOST_CERT 0x0011\n";
+        assert!(
+            matches!(src.save(junk), Err(Error::KeydbInvalid)),
+            "prefixed-but-unparseable rows must not count as entries"
+        );
+
+        // Contrast: the SAME shapes, but well-formed, ARE counted and saved.
+        let good = format!(
+            "| PK | 0x{}\n| HC | HOST_PRIV_KEY 0x{} | HOST_CERT 0x{}\n",
+            "11".repeat(16),
+            "00".repeat(20),
+            "00".repeat(92),
+        );
+        let ok = src
+            .save(good.as_bytes())
+            .expect("well-formed rows must save");
+        assert_eq!(
+            ok.entries, 2,
+            "one PK + one HC row are the two real entries"
+        );
+    }
+
    // A keydb.cfg over the size cap or not valid UTF-8 must surface as a
    // SOURCE FAILURE (KeydbInvalid), never a silent empty result.
    #[test]
@@ -17,10 +17,12 @@ use libfreemkv::aacs::types::{DeviceKey, HostCert};
 /// A keydb per-disc unit key: the CPS-unit number paired with its 16-byte key.
 pub type NumberedUnitKey = (u32, [u8; 16]);

-// Upper bound on the on-disk keydb.cfg size accepted by `KeyDb::load` (the
-// public UHD keydb is ~62 MiB; 128 MiB bounds a hostile/corrupt file).
-// Mirror of keydb.rs's MAX_KEYDB_BYTES decompression cap; keep them equal.
-const MAX_KEYDB_BYTES: u64 = 128 * 1024 * 1024;
+/// Upper bound on the keydb.cfg byte size (the public UHD keydb is ~62 MiB;
+/// 128 MiB bounds a hostile/corrupt file). The SINGLE definition for the crate:
+/// [`KeyDb::load`] uses it as the on-disk load cap and [`crate::KeydbSource`]
+/// re-uses it as the decompression-bomb cap on the save/update path, so the two
+/// can never drift out of lockstep.
+pub(crate) const MAX_KEYDB_BYTES: u64 = 128 * 1024 * 1024;

 /// Upper bound on parsed disc entries. The real public keydb carries
 /// ~170k+ entries, so the cap sits well above that while still bounding
@@ -240,6 +242,35 @@ pub(crate) fn is_disc_entry_line(line: &str) -> bool {
    (line.starts_with("0x") || line.starts_with("0X")) && line.contains(" = ")
 }

+/// True when `line` is a row `KeyDb::parse` would actually ACCEPT into the db —
+/// a disc row OR a DK/PK/HC/HC2 row that its real parser accepts, not merely one
+/// carrying the right `| XX` prefix. `KeydbSource::save`'s "won't persist
+/// unparseable content" guard counts entries with THIS, so a syntactically
+/// prefixed but malformed DK/PK/HC row (right prefix, bad hex/short cert) can no
+/// longer inflate the entry count past the `entries > 0` check. Mirrors the
+/// dispatch in [`KeyDb::parse_counted`] exactly. See docs/keydb.md#save-mirror-parse.
+pub(crate) fn is_parseable_entry_line(line: &str) -> bool {
+    let line = line.trim();
+    if line.is_empty() || line.starts_with(';') || line.starts_with('#') {
+        return false;
+    }
+    // Order matches `parse_counted`: HC2 before HC (both share the `| HC`
+    // prefix); an orphan DK (no position fields) still counts as a DK.
+    if line.starts_with("| DK") {
+        return KeyDb::parse_device_key(line).is_some() || KeyDb::parse_orphan_dk(line).is_some();
+    }
+    if line.starts_with("| PK") {
+        return KeyDb::parse_processing_key(line).is_some();
+    }
+    if line.starts_with("| HC2") {
+        return KeyDb::parse_host_cert_v2(line).is_some();
+    }
+    if line.starts_with("| HC") {
+        return KeyDb::parse_host_cert(line).is_some();
+    }
+    is_disc_entry_line(line) && KeyDb::parse_disc_entry(line).is_some()
+}
+
 impl KeyDb {
    /// Construct an empty KeyDb. Used by unit tests; production code
    /// reaches a populated KeyDb via [`KeyDb::load`] or [`KeyDb::parse`].
@@ -2008,6 +2039,202 @@ mod tests {
        assert_eq!(eb2.volume_size, None);
    }

+    // ── HC line: full parse into (priv[20], cert[92], revoked) ──────────────
+
+    /// A real-world-shaped `| HC |` line — a 20-byte private key, a 92-byte
+    /// cert that begins with the AACS host-cert header `02 01 00 5c ff ff 80 00
+    /// 02 10 …`, and a `; Revoked in MKBv72` note — must parse to exactly those
+    /// three: 20-byte key, 92-byte cert, and `revoked_at_mkb = Some(72)`.
+    #[test]
+    fn hc_line_parses_into_key_cert_and_revocation_generation() {
+        // Cert = 4-byte header + 6-byte host id + filler to the 92-byte length.
+        // Synthetic bytes only; no real key material.
+        let mut cert = vec![0x02u8, 0x01, 0x00, 0x5c, 0xff, 0xff, 0x80, 0x00, 0x02, 0x10];
+        cert.resize(92, 0xcc);
+        let cert_hex: String = cert.iter().map(|b| format!("{b:02x}")).collect();
+        let line = format!(
+            "| HC | HOST_PRIV_KEY 0x{} | HOST_CERT 0x{cert_hex} ; Revoked in MKBv72",
+            "88".repeat(20),
+        );
+        let hc = KeyDb::parse_host_cert(&line).expect("a well-formed HC line must parse");
+        assert_eq!(hc.cert.private_key, [0x88u8; 20], "20-byte private key");
+        assert_eq!(hc.cert.certificate.len(), 92, "AACS 1.0 cert is 92 bytes");
+        assert_eq!(
+            &hc.cert.certificate[..4],
+            &[0x02, 0x01, 0x00, 0x5c],
+            "header preserved"
+        );
+        assert_eq!(
+            hc.revoked_at_mkb,
+            Some(72),
+            "the ; Revoked in MKBvN note captures N"
+        );
+        // A 1.0 HC line carries no AACS 2.0 credentials.
+        assert!(hc.cert.private_key_v2.is_none());
+        assert!(hc.cert.certificate_v2.is_none());
+    }
+
+    /// A `| HC |` line with NO `; Revoked …` note is live: `revoked_at_mkb`
+    /// is `None`, so `host_certs` returns it at every generation.
+    #[test]
+    fn hc_line_without_note_is_live() {
+        let line = format!(
+            "| HC | HOST_PRIV_KEY 0x{} | HOST_CERT 0x{}",
+            "00".repeat(20),
+            "ab".repeat(92),
+        );
+        let hc = KeyDb::parse_host_cert(&line).unwrap();
+        assert_eq!(hc.revoked_at_mkb, None, "no note ⇒ never revoked");
+    }
+
+    // ── host_certs(mkb): the exact per-generation revocation filter ─────────
+
+    /// The hardware-critical filter: several certs revoked at DIFFERENT
+    /// generations plus one live cert. Each cert carries a distinct marker byte
+    /// 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]
+    fn host_certs_filters_several_certs_across_mkb_generations() {
+        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 = format!(
+            "{}{}{}{}{}",
+            hc(0x53, " ; Revoked in MKBv53"),
+            hc(0x58, " ; Revoked in MKBv58"),
+            hc(0x70, " ; Revoked in MKBv70"),
+            hc(0x72, " ; Revoked in MKBv72"),
+            hc(0x11, ""), // live: no revocation note
+        );
+        let db = KeyDb::parse(&cfg);
+        assert_eq!(db.host_certs.len(), 5);
+
+        // The first cert byte of each cert host_certs(mkb) hands back, sorted.
+        let markers = |mkb: Option<u32>| {
+            let mut m: Vec<u8> = db
+                .host_certs(mkb)
+                .iter()
+                .map(|c| c.certificate[0])
+                .collect();
+            m.sort_unstable();
+            m
+        };
+
+        // gen 77: every revocation has fired; only the live cert survives.
+        assert_eq!(markers(Some(77)), vec![0x11]);
+        // gen 50: below every revocation ⇒ all five are usable.
+        assert_eq!(markers(Some(50)), vec![0x11, 0x53, 0x58, 0x70, 0x72]);
+        // gen 58: revoked-at-53 and revoked-at-58 are out at the boundary
+        // (disc_gen < revoked is false); 70, 72 and the live cert remain.
+        assert_eq!(markers(Some(58)), vec![0x11, 0x70, 0x72]);
+        // gen 70: 70 itself is out at its own generation; only 72 + live remain.
+        assert_eq!(markers(Some(70)), vec![0x11, 0x72]);
+        // unknown disc MKB ⇒ cannot filter ⇒ every cert is returned.
+        assert_eq!(markers(None), vec![0x11, 0x53, 0x58, 0x70, 0x72]);
+    }
+
+    // ── Cert selection by the embedded 6-byte Host ID ──────────────────────
+
+    /// The AACS host ID lives INSIDE the 92-byte cert (offset 4, 6 bytes): a
+    /// cert beginning `02 01 00 5c ff ff 80 00 02 10 …` carries host id
+    /// `ff ff 80 00 02 10`. The parser keeps the cert bytes verbatim, so a
+    /// caller can select the cert for a specific host by that embedded id.
+    #[test]
+    fn host_cert_selected_by_embedded_6_byte_host_id() {
+        let cert_hex = |host_id: [u8; 6]| {
+            let mut bytes = vec![0x02u8, 0x01, 0x00, 0x5c]; // 4-byte header
+            bytes.extend_from_slice(&host_id); // host id at offset 4
+            bytes.resize(92, 0xcc); // pad to the 92-byte AACS 1.0 cert length
+            bytes.iter().map(|b| format!("{b:02x}")).collect::<String>()
+        };
+        let id_a = [0xff, 0xff, 0x80, 0x00, 0x02, 0x10];
+        let id_b = [0xff, 0xff, 0x80, 0x00, 0x02, 0x11];
+        let cfg = format!(
+            "| HC | HOST_PRIV_KEY 0x{p} | HOST_CERT 0x{a}\n\
+             | HC | HOST_PRIV_KEY 0x{p} | HOST_CERT 0x{b}\n",
+            p = "00".repeat(20),
+            a = cert_hex(id_a),
+            b = cert_hex(id_b),
+        );
+        let db = KeyDb::parse(&cfg);
+        assert_eq!(db.host_certs.len(), 2);
+
+        let find = |want: [u8; 6]| {
+            db.host_certs
+                .iter()
+                .find(|hc| hc.cert.certificate.get(4..10) == Some(&want[..]))
+        };
+        let hit = find(id_b).expect("the cert carrying host id …0211 must be found");
+        assert_eq!(&hit.cert.certificate[4..10], &id_b);
+        assert_eq!(hit.cert.certificate.len(), 92);
+        // A host id no cert carries selects nothing — and the byte-range slice
+        // must not panic when it does not match.
+        assert!(find([0x00, 0x00, 0x00, 0x00, 0x00, 0x00]).is_none());
+    }
+
+    /// A 1.0-only keydb (only `| HC |` rows, no `| HC2 |`) must leave every
+    /// cert's AACS 2.0 fields empty — no phantom v2 credentials appear.
+    #[test]
+    fn keydb_with_only_hc_rows_yields_no_v2_certs() {
+        let cfg = format!(
+            "| HC | HOST_PRIV_KEY 0x{} | HOST_CERT 0x{}\n\
+             | HC | HOST_PRIV_KEY 0x{} | HOST_CERT 0x{}\n",
+            "00".repeat(20),
+            "ab".repeat(92),
+            "01".repeat(20),
+            "cd".repeat(92),
+        );
+        let db = KeyDb::parse(&cfg);
+        assert_eq!(db.host_certs.len(), 2);
+        assert!(
+            db.host_certs
+                .iter()
+                .all(|hc| hc.cert.private_key_v2.is_none()),
+            "no HC2 rows ⇒ no v2 private keys"
+        );
+        assert!(
+            db.host_certs
+                .iter()
+                .all(|hc| hc.cert.certificate_v2.is_none()),
+            "no HC2 rows ⇒ no v2 certs"
+        );
+    }
+
+    /// The HC2 (AACS 2.0, type 0x11) slot: a full HC + HC2 pair parses into the
+    /// 1.0 fields AND the v2 fields, with the v2 private key exactly 32 bytes
+    /// and the v2 cert 132 bytes. Complements the field-ordering HC2 tests by
+    /// asserting the concrete v2 byte lengths a valid slot must yield.
+    #[test]
+    fn hc2_slot_parses_into_v2_key_and_cert_with_correct_lengths() {
+        let cfg = format!(
+            "| HC | HOST_PRIV_KEY 0x{} | HOST_CERT 0x{}\n\
+             | HC2 | HOST_PRIV_KEY 0x{} | HOST_CERT 0x{}\n",
+            "00".repeat(20),
+            "00".repeat(92),
+            "5b".repeat(32),
+            "6c".repeat(132),
+        );
+        let db = KeyDb::parse(&cfg);
+        assert_eq!(db.host_certs.len(), 1, "HC2 augments the preceding HC");
+        let c = &db.host_certs[0].cert;
+        assert_eq!(c.certificate.len(), 92, "1.0 cert retained");
+        assert_eq!(
+            c.private_key_v2,
+            Some([0x5bu8; 32]),
+            "32-byte v2 private key"
+        );
+        assert_eq!(
+            c.certificate_v2.as_ref().map(|v| v.len()),
+            Some(132),
+            "132-byte v2 cert"
+        );
+    }
+
    // KEYDB-parser integration tests relocated from libfreemkv; exercise the
    // parser end-to-end against a real keydb.cfg. See docs/keydb-format.md.

@@ -118,3 +118,96 @@ impl KeySource for MultiSource {
        "multi"
    }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use libfreemkv::aacs::types::HostCert;
+
+    // A minimal KeySource that only supplies host certs — the sole MultiSource
+    // behaviour under test. It never resolves a unit key.
+    struct CertOnly {
+        certs: Vec<HostCert>,
+    }
+    impl KeySource for CertOnly {
+        fn get_unit_keys(&self, _ctx: &dyn ResolveCtx) -> Result<Vec<UnitKey>, libfreemkv::Error> {
+            Ok(Vec::new())
+        }
+        fn host_certs(&self, _mkb: Option<u32>) -> Vec<HostCert> {
+            self.certs.clone()
+        }
+    }
+
+    // A host cert tagged with a distinct marker byte so a returned set can be
+    // identified exactly. Synthetic bytes only; no real key material.
+    fn cert(marker: u8) -> HostCert {
+        HostCert {
+            private_key: [marker; 20],
+            certificate: vec![marker; 92],
+            private_key_v2: None,
+            certificate_v2: None,
+        }
+    }
+
+    fn boxed(certs: Vec<HostCert>) -> Box<dyn KeySource> {
+        Box::new(CertOnly { certs }) as Box<dyn KeySource>
+    }
+
+    /// `MultiSource::host_certs` must UNION every inner source's certs. Without
+    /// this a composed source hides an inner source's cert from the OEM
+    /// cert-auth route — the gap the union closes.
+    #[test]
+    fn multi_source_host_certs_unions_every_inner_source() {
+        let multi = MultiSource::new(vec![
+            boxed(vec![cert(0x01)]),
+            boxed(vec![cert(0x02), cert(0x03)]),
+        ]);
+        let mut markers: Vec<u8> = multi
+            .host_certs(Some(50))
+            .iter()
+            .map(|c| c.certificate[0])
+            .collect();
+        markers.sort_unstable();
+        assert_eq!(
+            markers,
+            vec![0x01, 0x02, 0x03],
+            "certs from every inner source must be unioned"
+        );
+    }
+
+    /// With no inner source holding a cert, the union is empty (not an error).
+    #[test]
+    fn multi_source_host_certs_empty_when_no_source_has_one() {
+        let multi = MultiSource::new(vec![boxed(vec![]), boxed(vec![])]);
+        assert!(multi.host_certs(None).is_empty());
+    }
+
+    /// The `mkb` generation must reach each inner source so its OWN revocation
+    /// filter can act; a source that drops everything at a given generation is
+    /// honoured by the union.
+    #[test]
+    fn multi_source_host_certs_passes_mkb_through_to_inner_sources() {
+        struct GenAware;
+        impl KeySource for GenAware {
+            fn get_unit_keys(
+                &self,
+                _c: &dyn ResolveCtx,
+            ) -> Result<Vec<UnitKey>, libfreemkv::Error> {
+                Ok(Vec::new())
+            }
+            fn host_certs(&self, mkb: Option<u32>) -> Vec<HostCert> {
+                match mkb {
+                    // Modelled on a cert revoked in MKBv72.
+                    Some(g) if g >= 72 => Vec::new(),
+                    _ => vec![cert(0xaa)],
+                }
+            }
+        }
+        let multi = MultiSource::new(vec![Box::new(GenAware) as Box<dyn KeySource>]);
+        assert_eq!(multi.host_certs(Some(50)).len(), 1, "usable below gen 72");
+        assert!(
+            multi.host_certs(Some(72)).is_empty(),
+            "the gen must reach the inner source's filter"
+        );
+    }
+}
@@ -52,6 +52,13 @@ fn is_blocked_ip(ip: &IpAddr) -> bool {
                || (v4.octets()[0] == 100 && (v4.octets()[1] & 0xc0) == 0x40)
                // "This network" 0.0.0.0/8.
                || v4.octets()[0] == 0
+                // Benchmarking 198.18.0.0/15 (RFC 2544) — 198.18.x and 198.19.x
+                // (the /15 second octet is 18 with the low bit free, i.e. 18|19).
+                || (v4.octets()[0] == 198 && (v4.octets()[1] & 0xfe) == 18)
+                // IETF protocol assignments 192.0.0.0/24 (RFC 6890), which
+                // includes 192.0.0.170/171 (NAT64/DNS64 discovery). Distinct
+                // from 192.0.2.0/24 TEST-NET-1, already caught by is_documentation.
+                || (v4.octets()[0] == 192 && v4.octets()[1] == 0 && v4.octets()[2] == 0)
                // Class E reserved 240.0.0.0/4.
                || v4.octets()[0] >= 240
        }
@@ -104,17 +111,19 @@ enum GuardFail {
 // returns pinned socket addrs or a rejection reason + message. SECURITY: the
 // message names the address — log only at config time, never in `query`.
 fn resolve_and_guard(url: &str) -> Result<Vec<SocketAddr>, (GuardFail, String)> {
-    let rest = if let Some(r) = url.strip_prefix("https://") {
-        (r, 443u16)
-    } else if let Some(r) = url.strip_prefix("http://") {
-        (r, 80u16)
-    } else {
+    // ONLY https is accepted. The POST body carries base64 key material and a
+    // replayable bearer token, so cleartext `http://` is refused — and refused
+    // HERE, in the shared guard, so `validate_keyserver_url` catches it ONCE at
+    // config time rather than every rip tripping `query`'s runtime https check
+    // and reporting a transient error for a permanent misconfiguration. Any
+    // non-https scheme is a standing operator fault (`Config`), never an outage.
+    let Some(authority) = url.strip_prefix("https://") else {
        return Err((
            GuardFail::Config,
-            "URL must start with http:// or https://".into(),
+            "URL scheme must be https:// (cleartext http:// is refused)".into(),
        ));
    };
-    let (authority, default_port) = rest;
+    let default_port = 443u16;
    let authority = authority.split(['/', '?', '#']).next().unwrap_or(authority);
    let authority = authority.rsplit('@').next().unwrap_or(authority);
    if authority.is_empty() {
@@ -123,14 +132,19 @@ fn resolve_and_guard(url: &str) -> Result<Vec<SocketAddr>, (GuardFail, String)>
    let (host, port): (String, u16) = if let Some(stripped) = authority.strip_prefix('[') {
        match stripped.split_once(']') {
            Some((h, after)) => {
-                let p = after
-                    .strip_prefix(':')
-                    .map(|s| {
-                        s.parse::<u16>()
-                            .map_err(|_| (GuardFail::Config, "invalid port".to_string()))
-                    })
-                    .transpose()?
-                    .unwrap_or(default_port);
+                // The only thing allowed after `]` is an optional `:port`. A
+                // non-empty tail that is NOT `:port` (e.g. `[::1]extra`) is
+                // garbage — reject it as a `Config` fault instead of silently
+                // dropping it and connecting on the default port.
+                let p = if after.is_empty() {
+                    default_port
+                } else if let Some(port_str) = after.strip_prefix(':') {
+                    port_str
+                        .parse::<u16>()
+                        .map_err(|_| (GuardFail::Config, "invalid port".to_string()))?
+                } else {
+                    return Err((GuardFail::Config, "malformed IPv6 authority".into()));
+                };
                (h.to_string(), p)
            }
            None => return Err((GuardFail::Config, "malformed IPv6 host".into())),
@@ -153,21 +167,28 @@ fn resolve_and_guard(url: &str) -> Result<Vec<SocketAddr>, (GuardFail, String)>
    // resolver timeout and freeze the calling rip thread, so run it on a
    // spawned thread with a bounded deadline (mirrors autorip/libfreemkv).
    let addrs: Vec<SocketAddr> = {
-        use std::sync::atomic::{AtomicUsize, Ordering};
        use std::sync::mpsc;
        const DNS_TIMEOUT: Duration = Duration::from_secs(10);
        // Each resolver thread can hang for the OS timeout and is never joined,
        // so a black-holed keyserver leaks one thread+stack per attempt. Cap the
-        // outstanding ones; over the cap, report the host unreachable instead.
-        const MAX_DNS_THREADS: usize = 4;
-        static DNS_THREADS: AtomicUsize = AtomicUsize::new(0);
-        if DNS_THREADS.fetch_add(1, Ordering::SeqCst) >= MAX_DNS_THREADS {
-            DNS_THREADS.fetch_sub(1, Ordering::SeqCst);
+        // outstanding ones — but PER HOST, not process-globally: a global cap
+        // lets 4 hung lookups to one dead keyserver starve resolution of a
+        // healthy DIFFERENT keyserver. Over the per-host cap, report only THAT
+        // host unreachable. `BTreeMap::new` is const, so no lazy init is needed.
+        const MAX_DNS_THREADS_PER_HOST: usize = 4;
+        static DNS_INFLIGHT: Mutex<std::collections::BTreeMap<String, usize>> =
+            Mutex::new(std::collections::BTreeMap::new());
+        {
+            let mut inflight = DNS_INFLIGHT.lock().unwrap_or_else(|e| e.into_inner());
+            let n = inflight.entry(host.clone()).or_insert(0);
+            if *n >= MAX_DNS_THREADS_PER_HOST {
                return Err((
                    GuardFail::Unreachable,
-                "too many concurrent DNS resolutions in flight".into(),
+                    "too many concurrent DNS resolutions in flight for this host".into(),
                ));
            }
+            *n += 1;
+        }
        let host = host.clone();
        let (tx, rx) = mpsc::channel();
        std::thread::spawn(move || {
@@ -176,9 +197,15 @@ fn resolve_and_guard(url: &str) -> Result<Vec<SocketAddr>, (GuardFail, String)>
                .map(|it| it.collect::<Vec<SocketAddr>>());
            // Receiver may be gone after the timeout — ignore the send error.
            let _ = tx.send(res);
-            // Decrement only when the (possibly long-hung) lookup actually
-            // returns, so the cap reflects threads truly in flight.
-            DNS_THREADS.fetch_sub(1, Ordering::SeqCst);
+            // Release this host's slot only when the (possibly long-hung) lookup
+            // actually returns, so the cap reflects lookups truly in flight.
+            let mut inflight = DNS_INFLIGHT.lock().unwrap_or_else(|e| e.into_inner());
+            if let Some(n) = inflight.get_mut(&host) {
+                *n -= 1;
+                if *n == 0 {
+                    inflight.remove(&host);
+                }
+            }
        });
        match rx.recv_timeout(DNS_TIMEOUT) {
            Ok(Ok(addrs)) => addrs,
@@ -740,6 +767,28 @@ mod tests {
        ))));
    }

+    // Benchmarking 198.18.0.0/15 (RFC 2544) and IETF protocol assignments
+    // 192.0.0.0/24 (RFC 6890, incl. the 192.0.0.170/171 NAT64/DNS64 anycast)
+    // are non-public and must be blocked outbound.
+    #[test]
+    fn ssrf_guard_blocks_benchmarking_and_protocol_assignment_ranges() {
+        // 198.18.0.0/15 spans 198.18.x AND 198.19.x — both octets blocked.
+        assert!(is_blocked_ip(&IpAddr::V4(Ipv4Addr::new(198, 18, 0, 1))));
+        assert!(is_blocked_ip(&IpAddr::V4(Ipv4Addr::new(198, 18, 255, 255))));
+        assert!(is_blocked_ip(&IpAddr::V4(Ipv4Addr::new(198, 19, 0, 1))));
+        assert!(is_blocked_ip(&IpAddr::V4(Ipv4Addr::new(198, 19, 200, 5))));
+        // 198.17.x and 198.20.x are OUTSIDE the /15 — must stay allowed.
+        assert!(!is_blocked_ip(&IpAddr::V4(Ipv4Addr::new(198, 17, 0, 1))));
+        assert!(!is_blocked_ip(&IpAddr::V4(Ipv4Addr::new(198, 20, 0, 1))));
+        // 192.0.0.0/24, including 192.0.0.170 / 192.0.0.171.
+        assert!(is_blocked_ip(&IpAddr::V4(Ipv4Addr::new(192, 0, 0, 0))));
+        assert!(is_blocked_ip(&IpAddr::V4(Ipv4Addr::new(192, 0, 0, 170))));
+        assert!(is_blocked_ip(&IpAddr::V4(Ipv4Addr::new(192, 0, 0, 171))));
+        assert!(is_blocked_ip(&IpAddr::V4(Ipv4Addr::new(192, 0, 0, 255))));
+        // The adjacent 192.0.1.0 is a different block — not covered here.
+        assert!(!is_blocked_ip(&IpAddr::V4(Ipv4Addr::new(192, 0, 1, 1))));
+    }
+
    // 6to4 (2002::/16) and Teredo (2001:0000::/32) tunnel an IPv4 inside an
    // IPv6 address; the guard must decode and re-check that embedded IPv4 or an
    // internal target slips through the tunnel.
@@ -794,12 +843,14 @@ mod tests {

    #[test]
    fn resolve_and_guard_rejects_internal_literals() {
-        // Numeric literals resolve without DNS — must still be rejected.
-        assert!(resolve_and_guard("http://127.0.0.1/keys").is_err());
-        assert!(resolve_and_guard("http://169.254.169.254/latest/meta-data/").is_err());
-        assert!(resolve_and_guard(&format!("http://{}.{}.{}.{}:8080/keys", 10, 0, 0, 5)).is_err());
+        // Numeric literals resolve without DNS — must still be rejected. All
+        // https:// so the rejection is the SSRF address guard, not the scheme
+        // check (cleartext http:// is covered by its own test below).
+        assert!(resolve_and_guard("https://127.0.0.1/keys").is_err());
+        assert!(resolve_and_guard("https://169.254.169.254/latest/meta-data/").is_err());
+        assert!(resolve_and_guard(&format!("https://{}.{}.{}.{}:8080/keys", 10, 0, 0, 5)).is_err());
        assert!(resolve_and_guard(&format!("https://{}.{}.{}.{}/keys", 192, 168, 0, 1)).is_err());
-        assert!(resolve_and_guard("http://[::1]:9000/keys").is_err());
+        assert!(resolve_and_guard("https://[::1]:9000/keys").is_err());
    }

    #[test]
@@ -819,7 +870,7 @@ mod tests {
        // Scheme immediately followed by a path — empty authority.
        assert!(resolve_and_guard("https:///keys").is_err());
        // Bracketed IPv6 host missing its closing `]`.
-        assert!(resolve_and_guard("http://[::1/keys").is_err());
+        assert!(resolve_and_guard("https://[::1/keys").is_err());
        // `host:port` split with an empty host before the colon.
        assert!(resolve_and_guard("https://:8080/keys").is_err());
    }
@@ -857,7 +908,7 @@ mod tests {
        assert_eq!(addrs[0].port(), 443);

        let addrs =
-            resolve_and_guard("http://1.1.1.1:8080/keys").expect("public IP with port accepted");
+            resolve_and_guard("https://1.1.1.1:8080/keys").expect("public IP with port accepted");
        assert!(!addrs.is_empty());
        assert_eq!(addrs[0].port(), 8080);
    }
@@ -939,16 +990,55 @@ mod tests {
    #[test]
    fn validate_keyserver_url_rejects_internal_and_bad_scheme() {
        // Mirrors resolve_and_guard: the public wrapper rejects the same hosts.
-        assert!(validate_keyserver_url("http://127.0.0.1/keys").is_err());
-        assert!(validate_keyserver_url("http://169.254.169.254/latest/meta-data/").is_err());
-        assert!(validate_keyserver_url(&format!("http://{}.{}.{}.{}/k", 10, 0, 0, 5)).is_err());
-        assert!(validate_keyserver_url("http://[::1]:9000/keys").is_err());
+        assert!(validate_keyserver_url("https://127.0.0.1/keys").is_err());
+        assert!(validate_keyserver_url("https://169.254.169.254/latest/meta-data/").is_err());
+        assert!(validate_keyserver_url(&format!("https://{}.{}.{}.{}/k", 10, 0, 0, 5)).is_err());
+        assert!(validate_keyserver_url("https://[::1]:9000/keys").is_err());
        assert!(validate_keyserver_url("ftp://example.com/keys").is_err());
        assert!(validate_keyserver_url("").is_err());
        // A public literal IP passes (no DNS needed, deterministic).
        assert!(validate_keyserver_url("https://8.8.8.8/keys").is_ok());
    }

+    // Cleartext http:// must be refused at CONFIG time (validate_keyserver_url),
+    // not per-rip: the body carries base64 key material and a replayable bearer
+    // token, and `query` hard-refuses non-https at runtime. Catching it once
+    // here turns a permanent misconfig into a config error instead of a
+    // transient failure on every rip. A public host proves it's the SCHEME
+    // being rejected, not the address guard.
+    #[test]
+    fn non_https_scheme_is_rejected_at_config_time() {
+        // A perfectly reachable public host — only the http:// scheme is wrong.
+        assert!(validate_keyserver_url("http://8.8.8.8/keys").is_err());
+        assert!(resolve_and_guard("http://8.8.8.8/keys").is_err());
+        // The rejection is a standing Config fault, never a transient outage.
+        let (kind, _msg) = resolve_and_guard("http://8.8.8.8/keys")
+            .expect_err("cleartext http:// must be refused");
+        assert_eq!(kind, GuardFail::Config);
+        // https:// for the same public host is accepted.
+        assert!(validate_keyserver_url("https://8.8.8.8/keys").is_ok());
+    }
+
+    // A non-empty, non-`:port` tail after a bracketed IPv6 authority (e.g.
+    // `[::1]extra`) must be rejected as a Config fault, not silently dropped
+    // with a fall back to the default port.
+    #[test]
+    fn resolve_and_guard_rejects_garbage_after_bracketed_ipv6() {
+        // `[::1]junk` — junk after the closing bracket.
+        let (kind, _) = resolve_and_guard("https://[::1]junk/keys")
+            .expect_err("garbage after ] must be rejected");
+        assert_eq!(kind, GuardFail::Config);
+        // A public v6 literal with a trailing garbage tail is likewise rejected
+        // (so the garbage can't slip a request out on the default port).
+        assert!(resolve_and_guard("https://[2606:4700:4700::1111]extra/keys").is_err());
+        // Sanity: the same public v6 literal WITHOUT the tail is accepted.
+        assert!(resolve_and_guard("https://[2606:4700:4700::1111]/keys").is_ok());
+        // And an explicit :port after ] still parses.
+        let addrs = resolve_and_guard("https://[2606:4700:4700::1111]:8443/keys")
+            .expect("bracketed v6 with :port must parse");
+        assert_eq!(addrs[0].port(), 8443);
+    }
+
    // ── the reply → verdict mapping (THE defect) ──────────────────────────
    // Driven through `interpret_reply` directly: a stub HTTP server is NOT
    // usable here since `resolve_and_guard` blocks loopback by design.