diff --git a/Cargo.lock b/Cargo.lock index cbea079..0e45126 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -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" diff --git a/src/keydb.rs b/src/keydb.rs index 3327108..105c56a 100644 --- a/src/keydb.rs +++ b/src/keydb.rs @@ -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, 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] diff --git a/src/keydb_format.rs b/src/keydb_format.rs index a048251..e3bbe69 100644 --- a/src/keydb_format.rs +++ b/src/keydb_format.rs @@ -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| { + let mut m: Vec = 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::() + }; + 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. diff --git a/src/lib.rs b/src/lib.rs index ff9bcd4..5906ec1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -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, + } + impl KeySource for CertOnly { + fn get_unit_keys(&self, _ctx: &dyn ResolveCtx) -> Result, libfreemkv::Error> { + Ok(Vec::new()) + } + fn host_certs(&self, _mkb: Option) -> Vec { + 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) -> Box { + Box::new(CertOnly { certs }) as Box + } + + /// `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 = 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, libfreemkv::Error> { + Ok(Vec::new()) + } + fn host_certs(&self, mkb: Option) -> Vec { + 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]); + 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" + ); + } +} diff --git a/src/online.rs b/src/online.rs index 3222ed8..fcbdd12 100644 --- a/src/online.rs +++ b/src/online.rs @@ -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, (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, (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::() - .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::() + .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,20 +167,27 @@ fn resolve_and_guard(url: &str) -> Result, (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 = { - 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); - return Err(( - GuardFail::Unreachable, - "too many concurrent DNS resolutions in flight".into(), - )); + // 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> = + 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 for this host".into(), + )); + } + *n += 1; } let host = host.clone(); let (tx, rx) = mpsc::channel(); @@ -176,9 +197,15 @@ fn resolve_and_guard(url: &str) -> Result, (GuardFail, String)> .map(|it| it.collect::>()); // 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.