Merge branch 'fix/key-service-outage-not-missing-key' into dev

src/disc/mod.rs

@@ -2720,10 +2720,28 @@ impl Disc {
                 // material (device / processing keys) but no Volume ID to derive
                 // the unit key, the captured `aacs_error` is `AacsVidUnavailable`
                 // — report THAT (the fix is recovering the VID, not adding keys),
-                // not the generic `NoDiscKey`. Any other (or absent) reason →
+                // not the generic `NoDiscKey`. Any reason NOT listed below (or an
-                // `NoDiscKey` naming the disc by hash, unchanged.
+                // absent one) → `NoDiscKey` naming the disc by hash, unchanged.
-                if matches!(self.aacs_error, Some(Error::AacsVidUnavailable)) {
+                //
-                    return Err(Error::AacsVidUnavailable);
+                // Same split, second axis: when a key SOURCE could not answer
+                // (unreachable / 5xx / bad token / rate-limited) resolution
+                // stamps that reason here, and it must surface as ITSELF.
+                // `NoDiscKey` asserts every source answered and none holds a key
+                // — the opposite of what happened — and reporting a seven-hour
+                // 502 outage that way sent operators looking for a VUK when the
+                // right action was to wait.
+                match self.aacs_error {
+                    Some(Error::AacsVidUnavailable) => return Err(Error::AacsVidUnavailable),
+                    Some(Error::KeyServiceUnavailable) => {
+                        return Err(Error::KeyServiceUnavailable);
+                    }
+                    Some(Error::KeyServiceUnauthorized) => {
+                        return Err(Error::KeyServiceUnauthorized);
+                    }
+                    Some(Error::KeyServiceRateLimited) => {
+                        return Err(Error::KeyServiceRateLimited);
+                    }
+                    _ => {}
                 }
                 return Err(Error::NoDiscKey {
                     disc_hash: self.aacs_disc_hash(),
@@ -4699,6 +4717,140 @@ mod tests {
         );
     }
 
+    /// THE defect: a key SOURCE that could not answer must not be reported as
+    /// "this disc has no key".
+    ///
+    /// Drives the real chain end to end — `KeySource` → `resolve_and_apply_traced`
+    /// → `Disc::aacs_error` → `ensure_decryptable` — twice over the SAME disc, and
+    /// asserts the two operator-visible verdicts differ:
+    ///
+    /// * source returns `Err(KeyServiceUnavailable)` (the HTTP-502 outage) → E7028
+    ///   "the key service could not answer; retry later",
+    /// * source returns `Ok(vec![])` (the service answered, no entry) → E7022
+    ///   "no key source has a decryption key for this disc".
+    ///
+    /// Before the fix both arms produced E7022, which is what sent an operator
+    /// hunting for a VUK through seven hours of 502s. The credential and
+    /// rate-limit verdicts are asserted alongside, since each is a different
+    /// operator action (fix the token / back off).
+    #[test]
+    fn key_source_failure_is_not_reported_as_a_missing_disc_key() {
+        use crate::keysource::{KeySource, ResolveCtx, resolve_and_apply_traced};
+
+        /// A source that fails the way a down / hostile key service fails.
+        struct FailingSource(fn() -> crate::error::Error);
+        impl KeySource for FailingSource {
+            fn get_unit_keys(
+                &self,
+                _ctx: &dyn ResolveCtx,
+            ) -> std::result::Result<Vec<crate::aacs::types::UnitKey>, crate::error::Error>
+            {
+                Err((self.0)())
+            }
+            fn label(&self) -> &'static str {
+                "online"
+            }
+        }
+        /// A source that ANSWERS and holds nothing — the genuine miss.
+        struct AnsweredNoEntry;
+        impl KeySource for AnsweredNoEntry {
+            fn get_unit_keys(
+                &self,
+                _ctx: &dyn ResolveCtx,
+            ) -> std::result::Result<Vec<crate::aacs::types::UnitKey>, crate::error::Error>
+            {
+                Ok(Vec::new())
+            }
+            fn label(&self) -> &'static str {
+                "online"
+            }
+        }
+
+        let inputs = crate::keysource::DiscInputs {
+            disc_hash: "0x422EB".into(),
+            volume_id: [0u8; 16],
+            version: crate::aacs::mkb::AACS_MAJOR_UHD,
+            mkb: Vec::new(),
+            unit_key_ro: Vec::new(),
+            samples: Vec::new(),
+            volume_label: None,
+        };
+        let encrypted_aacs_disc = || {
+            let mut d = make_test_disc(1000, "UHD");
+            d.encrypted = true;
+            d.aacs = Some(aacs_with(Vec::new())); // AACS state, no unit keys
+            d
+        };
+
+        // The genuine miss — the service answered, nothing for this disc.
+        let mut answered = encrypted_aacs_disc();
+        let sources: Vec<Box<dyn KeySource>> = vec![Box::new(AnsweredNoEntry)];
+        let (ok, trace) = resolve_and_apply_traced(&sources, &inputs, &mut answered);
+        assert!(!ok);
+        assert_eq!(
+            trace.keys[0].path,
+            vec![crate::aacs::trace::KeyNode::NoEntry],
+            "a source that ANSWERED and holds nothing is the one true `NoEntry`"
+        );
+        assert!(
+            answered.aacs_error.is_none(),
+            "a genuine miss stamps no failure reason on the disc"
+        );
+        let miss_code = answered
+            .ensure_decryptable(false)
+            .expect_err("no key, !raw must error")
+            .code();
+        assert_eq!(
+            miss_code,
+            crate::error::Error::NoDiscKey {
+                disc_hash: String::new()
+            }
+            .code(),
+            "a genuine miss keeps E7022 — that wording is correct for it"
+        );
+
+        // Each way the service can FAIL to answer, and the code it must produce.
+        type MakeError = fn() -> crate::error::Error;
+        let failures: &[(MakeError, u16)] = &[
+            (
+                || crate::error::Error::KeyServiceUnavailable,
+                crate::error::E_KEY_SERVICE_UNAVAILABLE,
+            ),
+            (
+                || crate::error::Error::KeyServiceUnauthorized,
+                crate::error::E_KEY_SERVICE_UNAUTHORIZED,
+            ),
+            (
+                || crate::error::Error::KeyServiceRateLimited,
+                crate::error::E_KEY_SERVICE_RATE_LIMITED,
+            ),
+        ];
+        for (make, want) in failures {
+            let mut down = encrypted_aacs_disc();
+            let sources: Vec<Box<dyn KeySource>> = vec![Box::new(FailingSource(*make))];
+            let (ok, trace) = resolve_and_apply_traced(&sources, &inputs, &mut down);
+            assert!(!ok);
+            assert!(
+                trace.keys[0].path.is_empty(),
+                "a source that could not ANSWER must not claim `no entry` in the trace"
+            );
+            assert_eq!(
+                down.aacs_error.as_ref().map(crate::error::Error::code),
+                Some(*want),
+                "the source's failure reason must reach the disc"
+            );
+            let code = down
+                .ensure_decryptable(false)
+                .expect_err("no key, !raw must error")
+                .code();
+            assert_eq!(code, *want, "the gate must surface the SOURCE's code");
+            assert_ne!(
+                code, miss_code,
+                "a source that could not answer must NOT render as \"this disc has no key\""
+            );
+        }
+    }
+
     /// Same AACS-no-key disc under `--raw` (raw=true) must PROCEED — the user
     /// asked for the encrypted image and needs no key.
     #[test]

src/error.rs

@@ -101,6 +101,25 @@ pub const E_FMTS_KEY_MISSING: u16 = 7026;
 /// failure reported as success. [`is_disc_level_no_key`] classifies this code, so
 /// a multi-title rip loop fails fast on it.
 pub const E_CSS_NO_DISC_KEY: u16 = 7027;
+/// A key SOURCE could not be reached, or failed on its own side — transport
+/// error, DNS failure, timeout, TLS failure, an HTTP 5xx, or a reply the client
+/// could not read. The source never got as far as answering the question, so
+/// nothing at all is known about whether a key for this disc exists.
+///
+/// Deliberately NOT [`E_NO_DISC_KEY`], which asserts the OPPOSITE — every source
+/// answered and none holds a key. A seven-hour run of HTTP 502s reported as
+/// `E_NO_DISC_KEY` told operators their disc was not in the key database and sent
+/// them hunting for a VUK that was never missing; the correct action was to wait.
+/// Transient: retry later.
+pub const E_KEY_SERVICE_UNAVAILABLE: u16 = 7028;
+/// A key source rejected the configured credentials (HTTP 401/403 from the online
+/// key service). NOT transient and NOT an absent key — the operator action is to
+/// fix the token, not to wait and not to look for a VUK.
+pub const E_KEY_SERVICE_UNAUTHORIZED: u16 = 7029;
+/// A key source rate-limited the request (HTTP 429 from the online key service).
+/// The operator action is to back off and retry more slowly; the disc's key may
+/// well exist.
+pub const E_KEY_SERVICE_RATE_LIMITED: u16 = 7030;
 
 // Keydb (8xxx)
 pub const E_KEYDB_CONNECT: u16 = 8000;
@@ -462,6 +481,21 @@ pub enum Error {
     /// gate once did) makes an undecryptable disc log one "title skipped" notice
     /// per title and exit successfully.
     CssNoDiscKey,
+    /// A key source could not be reached, or failed on its own side — transport
+    /// error, DNS failure, timeout, TLS failure, HTTP 5xx, or an unreadable /
+    /// unparseable reply. See [`E_KEY_SERVICE_UNAVAILABLE`]: the source never
+    /// answered the question, so this is emphatically NOT [`Error::NoDiscKey`]
+    /// (which asserts every source DID answer and none holds a key). Transient.
+    ///
+    /// Carries no detail by design: the key-service URL and the resolved address
+    /// are operator-confidential and must not reach a log or a bug report.
+    KeyServiceUnavailable,
+    /// A key source rejected the configured credentials (HTTP 401/403). See
+    /// [`E_KEY_SERVICE_UNAUTHORIZED`]. Not transient: fix the token.
+    KeyServiceUnauthorized,
+    /// A key source rate-limited the request (HTTP 429). See
+    /// [`E_KEY_SERVICE_RATE_LIMITED`]. Back off and retry more slowly.
+    KeyServiceRateLimited,
     /// The live-drive AACS cert-auth handshake (the OEM/AACS baseline route)
     /// could not run because NO host certificate was available from any key
     /// source. Host certs are keysource-served, never compiled in, so without
@@ -742,6 +776,9 @@ impl Error {
             Error::NoDiscKey { .. } => E_NO_DISC_KEY,
             Error::CssKeyMissing => E_CSS_KEY_MISSING,
             Error::CssNoDiscKey => E_CSS_NO_DISC_KEY,
+            Error::KeyServiceUnavailable => E_KEY_SERVICE_UNAVAILABLE,
+            Error::KeyServiceUnauthorized => E_KEY_SERVICE_UNAUTHORIZED,
+            Error::KeyServiceRateLimited => E_KEY_SERVICE_RATE_LIMITED,
             Error::AacsNoHostCert { .. } => E_AACS_NO_HOST_CERT,
             Error::AacsBusKeyUnavailable => E_AACS_BUS_KEY_UNAVAILABLE,
             Error::FmtsKeyMissing => E_FMTS_KEY_MISSING,
@@ -1139,10 +1176,26 @@ pub fn is_halt(e: &std::io::Error) -> bool {
 /// [`E_CSS_KEY_MISSING`]: an undecryptable CSS disc landed in
 /// [`is_skippable_title_stub`], so the rip loop skipped all N titles with an
 /// "empty stub" notice and exited successfully.
+/// The key-SOURCE failures ([`E_KEY_SERVICE_UNAVAILABLE`],
+/// [`E_KEY_SERVICE_UNAUTHORIZED`], [`E_KEY_SERVICE_RATE_LIMITED`]) are here for
+/// the same fail-fast reason and NOT because they mean "no key": a service that
+/// is down, refusing the token, or throttling is down for every title on the
+/// disc, so iterating N titles re-issues N doomed requests (and, on 429, digs the
+/// rate-limit hole deeper). They are separate CODES precisely so the front-end
+/// can say "retry later" / "fix the token" instead of `E_NO_DISC_KEY`'s "no key
+/// source has a key for this disc".
 pub fn is_disc_level_no_key(e: &std::io::Error) -> bool {
     matches!(
         error_code(e),
-        Some(E_NO_DISC_KEY | E_KEYDB_LOAD | E_AACS_NO_KEYS | E_CSS_NO_DISC_KEY)
+        Some(
+            E_NO_DISC_KEY
+                | E_KEYDB_LOAD
+                | E_AACS_NO_KEYS
+                | E_CSS_NO_DISC_KEY
+                | E_KEY_SERVICE_UNAVAILABLE
+                | E_KEY_SERVICE_UNAUTHORIZED
+                | E_KEY_SERVICE_RATE_LIMITED
+        )
     )
 }
 
@@ -1420,6 +1473,12 @@ mod tests {
             // Both CSS no-key verdicts: numeric-only Display, no English.
             (Error::CssKeyMissing, E_CSS_KEY_MISSING),
             (Error::CssNoDiscKey, E_CSS_NO_DISC_KEY),
+            // Key-SOURCE failures: bare numeric Display. They must carry NO
+            // detail — the service URL and its resolved address are
+            // operator-confidential and must never reach a pasted bug report.
+            (Error::KeyServiceUnavailable, E_KEY_SERVICE_UNAVAILABLE),
+            (Error::KeyServiceUnauthorized, E_KEY_SERVICE_UNAUTHORIZED),
+            (Error::KeyServiceRateLimited, E_KEY_SERVICE_RATE_LIMITED),
         ];
         for (e, want_code) in cases {
             let s = e.to_string();
@@ -1621,6 +1680,9 @@ mod tests {
             E_NO_DISC_KEY,
             E_CSS_KEY_MISSING,
             E_CSS_NO_DISC_KEY,
+            E_KEY_SERVICE_UNAVAILABLE,
+            E_KEY_SERVICE_UNAUTHORIZED,
+            E_KEY_SERVICE_RATE_LIMITED,
             E_AACS_NO_HOST_CERT,
             E_AACS_BUS_KEY_UNAVAILABLE,
             E_FMTS_KEY_MISSING,

src/keysource.rs

@@ -381,6 +381,14 @@ pub fn resolve_and_apply_traced(
 
     let mut trace = crate::aacs::trace::ResolutionTrace::new();
 
+    // The FIRST source failure seen, if any. A source that returns `Err` did not
+    // answer "no key for this disc" — it could not answer at all — and that
+    // reason is stamped onto `disc.aacs_error` below so the decrypt gate reports
+    // THAT instead of the generic `NoDiscKey`. First-wins (not last) so the
+    // ordered sources' most-preferred failure is the one the operator is told
+    // about, matching the first-valid-wins rule for successes.
+    let mut source_failure: Option<crate::error::Error> = None;
+
     // The ctx parses Unit_Key_RO.inf at the stride for `inputs.version` (the
     // disc's own AACS major), so the stride is the disc's single source of truth.
     let ctx = DiscInputsCtx::new(inputs);
@@ -413,46 +421,55 @@ pub fn resolve_and_apply_traced(
                     outcome: KeyOutcome::NoKey,
                 });
             }
-            // Empty (no key here) or a source failure — both are "no key from
+            // The source ANSWERED and holds nothing for this disc. This — and
-            // this source"; move on to the next.
+            // only this — is `NoEntry`: the claim "I looked, it is not there".
-            //
+            Ok(_) => {
-            // NOTE: the `Err` half is currently UNREACHABLE in production, and the
-            // conflation below is therefore latent rather than live. Every shipped
-            // `KeySource` swallows its own failures into `Ok(Vec::new())`:
-            // `KeydbSource::get_unit_keys` maps a load/parse error to an empty vec,
-            // `OnlineSource::get_unit_keys` is `Ok(self.query(ctx))` where `query`
-            // returns empty on transport error, HTTP status, oversize body and bad
-            // JSON alike, and `MultiSource` discards inner `Err`s. Only test doubles
-            // return `Err`.
-            //
-            // Consequence: an unreachable key server arrives here as "no entry",
-            // and an operator is told their disc is not in the database. autorip
-            // works around it by re-probing the service over HTTP
-            // (`probe_online_reachability` / `key_service_transient_status`), whose
-            // own comment names the incident — "the online keysource swallows every
-            // failure (transport error, 502, timeout)".
-            //
-            // Fixing it HERE would change nothing: the fix belongs at the source
-            // boundary in `freemkv-keysources`, so a failure is reported as a
-            // failure, with `Disc::aacs_error` as the channel the operator actually
-            // reads. `FetchOutcome::errored` in `drive_unit_keys` /
-            // `drive_fmts_indexes` never FIRES for the same reason — it is the
-            // right contract, honoured by no shipped source yet. It is NOT dead
-            // code: it is written at both `drive_*` sites and read by the
-            // cache-insert guard `if !keys.is_empty() || !outcome.errored`, the
-            // only thing that stops a transient source outage from being memoised
-            // permanently into the per-fingerprint key cache — pinned by
-            // `errored_empty_is_not_cached_and_retries_when_source_recovers`. Do
-            // not delete it while making a source report failures as `Err`; that
-            // is precisely when it starts to matter.
-            Ok(_) | Err(_) => {
                 trace.keys.push(KeyStep {
                     who,
                     path: vec![KeyNode::NoEntry],
                     outcome: KeyOutcome::NoKey,
                 });
             }
+            // The source could NOT answer — it was unreachable, it errored, or it
+            // refused. Nothing is known about whether a key exists, so the path
+            // is EMPTY: recording `NoEntry` here is exactly the conflation that
+            // made a seven-hour run of HTTP 502s render as
+            // `key: online > no entry > NO KEY` + `E7022 No key source has a
+            // decryption key for this disc`, and sent operators hunting for a VUK
+            // that was never missing.
+            //
+            // The reason itself rides out on `disc.aacs_error` (below), the
+            // channel `Disc::ensure_decryptable_keys` already reads for the
+            // E7017-vs-E7022 split — so the decrypt gate raises the SOURCE's code
+            // (`KeyServiceUnavailable` / `KeyServiceUnauthorized` /
+            // `KeyServiceRateLimited`) instead of the generic `NoDiscKey`.
+            //
+            // `KeyOutcome` deliberately gains no variant: it is matched
+            // exhaustively by every front-end's trace renderer (freemkv's
+            // `pipe::render_resolution_trace`, autorip's
+            // `keysource::render_resolution_trace`), and this fix must not turn
+            // into a breaking change across four repos to say something the error
+            // code already says precisely.
+            Err(e) => {
+                if source_failure.is_none() {
+                    source_failure = Some(e);
                 }
+                trace.keys.push(KeyStep {
+                    who,
+                    path: Vec::new(),
+                    outcome: KeyOutcome::NoKey,
+                });
+            }
+        }
+    }
+    // Nothing resolved. If a source FAILED rather than answered, stamp that
+    // reason onto the disc so the decrypt gate can report it — but never clobber
+    // a reason the scan already captured (e.g. `AacsVidUnavailable`), which is
+    // closer to the disc itself than a source outage is.
+    if let Some(e) = source_failure
+        && disc.aacs_error.is_none()
+    {
+        disc.aacs_error = Some(e);
     }
     (false, trace)
 }