fix(otp): stop reporting a key that answered as a key we couldn't reach

An OTP command against a Token2 key supplied without the on-device OTP
function told the user "the OTP applet is not reachable over HID or CCID —
HID may be disabled on the key". That advice could not help: the debug
trace in the report shows a complete HID exchange, with the key answering
status word 0105. HID was working; the key was declining.

The cause was a discarded error. detect() probes the applet over HID, falls
back to CCID, and when both fail reported NoUsableInterface unconditionally
— an `Err(_)` arm that threw away whether the probe had been answered at
all. A device-level refusal and a dead interface became the same message.

Keep the probe error and classify it. An applet status word proves the
interface carried a full request and response, so it now reports the key
declining and quotes the status word; only a probe that got no answer
(silence, framing damage, a failed open) still blames the interface. The
decision moved into a pure function so it is testable — the original bug
lived in an inline match arm no test could reach.

Also refuse earlier where we already know better. The USB product id
encodes which function set a key shipped with, so for a configuration with
no OTP function the command is now refused before any APDU, naming the
reason. Unrecognised ids are still probed, so this cannot lock out a model
the table has not learned yet.

Adds OtpError::status_word() as the inverse of check(); the typed error had
been discarding the value the transport needs to tell these cases apart.

re #95

Author: framefilter

CHANGELOG.md

@@ -4,6 +4,21 @@ All notable changes to keyroost are documented here. The format follows
 [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and the project aims to
 follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+### Fixed
+- **A key that answers is no longer reported as a key that can't be reached.**
+  Running an OTP command against a Token2 key supplied *without* the on-device
+  OTP function said "the OTP applet is not reachable over HID or CCID — HID may
+  be disabled on the key", which sent people to enable an interface that was
+  already working: the key had completed a full USB-HID exchange and simply
+  declined. keyroost now distinguishes the two — an applet that answers with a
+  status word means the interface works and the *key* refused, and the message
+  says so and quotes the status word. Where the USB product id already
+  identifies a configuration with no OTP function, the command is refused up
+  front, naming the real reason, without sending an APDU that could only fail.
+  Unrecognised product ids are still probed. ([#95])
+
 ## [0.7.7] - 2026-08-04
 
 ### Added
@@ -783,6 +798,7 @@ multi-vendor hardware-security-key manager, then took its neutral name. Highligh
 [#81]: https://github.com/framefilter/keyroost/issues/81
 [#82]: https://github.com/framefilter/keyroost/issues/82
 [#83]: https://github.com/framefilter/keyroost/issues/83
+[#95]: https://github.com/framefilter/keyroost/issues/95
 [Unreleased]: https://github.com/framefilter/keyroost/compare/v0.7.7...HEAD
 [0.7.7]: https://github.com/framefilter/keyroost/compare/v0.7.6...v0.7.7
 [0.7.6]: https://github.com/framefilter/keyroost/compare/v0.7.5...v0.7.6

crates/keyroost-token2otp/src/lib.rs

@@ -140,6 +140,29 @@ impl OtpError {
     pub fn is_empty_token(&self) -> bool {
         matches!(self, OtpError::EntryNotFound)
     }
+
+    /// The status word this error was mapped from — the inverse of
+    /// [`check`](OtpError::check).
+    ///
+    /// Every variant here came from one, but only `BadStatusCode` kept it, so
+    /// callers that needed the raw value had to discard the typed error. The
+    /// transport needs it to tell "the applet answered and declined" apart from
+    /// "the interface never worked": a key that returns a status word has a
+    /// working interface, and blaming the transport sends the user off to
+    /// enable something that is already on (issue #95).
+    ///
+    /// `EntryNotFound` reports the primary `6A80`; it is also produced by the
+    /// `6A83` alias, so this is a lossy inverse for that one variant.
+    #[must_use]
+    pub fn status_word(&self) -> u16 {
+        match self {
+            OtpError::EntryNotFound => sw::ENTRY_NOT_FOUND,
+            OtpError::NotEnoughSpace => sw::NOT_ENOUGH_SPACE,
+            OtpError::ButtonPressRequired => sw::BUTTON_TIMEOUT,
+            OtpError::HidNotSupported => sw::HID_NOT_SUPPORTED,
+            OtpError::BadStatusCode(sw) => *sw,
+        }
+    }
 }
 
 impl std::fmt::Display for OtpError {

crates/keyroost-transport/src/token2otp.rs

@@ -74,6 +74,22 @@ pub enum OtpTransportError {
     /// flag must never drive unbounded host work (audit KEY-009), so this is
     /// treated as a buggy or hostile token rather than looped on forever.
     EnumerationCapExceeded,
+    /// The connected key's USB product id identifies a Token2 product
+    /// configuration supplied *without* the on-device OTP function, so no OTP
+    /// command can succeed on it. Refused before any APDU is sent so the user
+    /// gets the real reason rather than a transport error (issue #95).
+    OtpFunctionNotFitted {
+        pid: u16,
+        model: Option<&'static str>,
+    },
+    /// The OTP applet answered over HID but declined the read-only detect
+    /// probe with a non-`9000` status word, and no CCID fallback was
+    /// available. Distinct from [`NoUsableInterface`](Self::NoUsableInterface)
+    /// on purpose: a status word proves the interface carried a full
+    /// request/response, so this is the key refusing rather than a connection
+    /// problem, and telling the user to enable HID would send them the wrong
+    /// way (issue #95).
+    HidProbeDeclined { sw: u16 },
 }
 
 impl std::fmt::Display for OtpTransportError {
@@ -102,6 +118,27 @@ impl std::fmt::Display for OtpTransportError {
                 "the device kept reporting more OTP entries than any supported \
                  token stores; aborting enumeration"
             ),
+            OtpTransportError::OtpFunctionNotFitted { pid, model } => write!(
+                f,
+                "this key does not have the on-device OTP function: USB product id \
+                 {:#06X}{} is a Token2 configuration supplied without it. Token2 keys \
+                 aren't upgradable after purchase — OTP is a separate product \
+                 configuration, not something that can be switched on later",
+                pid,
+                match model {
+                    Some(m) => format!(" ({})", m),
+                    None => String::new(),
+                }
+            ),
+            OtpTransportError::HidProbeDeclined { sw } => write!(
+                f,
+                "the key answered over USB-HID but declined the OTP applet with status \
+                 word {:#06X}. The interface is working — this is the key refusing, not \
+                 a connection problem — most often because it was supplied without the \
+                 on-device OTP function, which cannot be enabled after purchase. Run \
+                 `keyroostctl list` to see what this key reports",
+                sw
+            ),
         }
     }
 }
@@ -251,6 +288,19 @@ impl HidOtpTransport {
                     || d.product_id == t2::USB_PID)
         });
         let dev = found.ok_or(OtpTransportError::TokenNotDetected)?;
+        // The PID encodes which function set the key was supplied with. On a
+        // configuration that has no OTP function, every OTP command is doomed,
+        // and probing anyway produces a transport-shaped error that blames the
+        // interface for the key's own product configuration (issue #95). Refuse
+        // here, before a single APDU, so the message names the real reason.
+        // Fail-open: an unrecognised PID is still probed (`token2_pid_may_have_otp`
+        // returns true when the table has no entry).
+        if !keyroost_proto::token2_pid_may_have_otp(dev.product_id) {
+            return Err(OtpTransportError::OtpFunctionNotFitted {
+                pid: dev.product_id,
+                model: keyroost_proto::token2_pid_label(dev.product_id),
+            });
+        }
         Self::open_path(&dev.path)
     }
 
@@ -756,6 +806,34 @@ fn probe_hid_owned(
     }
 }
 
+/// Which error to report when the HID probe failed *and* the CCID fallback was
+/// unavailable, so the probe outcome is the only evidence there is.
+///
+/// The distinction this makes is the whole point. If the applet returned a
+/// status word, the HID interface carried a complete request and response —
+/// it demonstrably works, and the key simply declined. Reporting that as
+/// [`NoUsableInterface`](OtpTransportError::NoUsableInterface) tells the user
+/// to enable an interface that is already enabled, which is what issue #95
+/// reported: a key answering `0105` was diagnosed as "HID may be disabled".
+/// Only a probe that never got an answer (framing error, timeout, I/O failure)
+/// is evidence about the interface itself.
+///
+/// Pure and total so the classification is unit-testable without hardware —
+/// the original defect was a `_ =>` arm in an inline match that no test could
+/// reach.
+fn detect_failure_from(probe_err: OtpTransportError) -> OtpTransportError {
+    match probe_err {
+        // The applet spoke. Carry the status word so the message can say what
+        // the key actually answered.
+        OtpTransportError::Applet(e) => OtpTransportError::HidProbeDeclined {
+            sw: e.status_word(),
+        },
+        // Silence, framing damage, or the interface never opening: these are
+        // genuinely about the transport.
+        _ => OtpTransportError::NoUsableInterface,
+    }
+}
+
 /// Confirm the OTP applet answers over HID using the read-only
 /// `GET_ECDH_PUBKEY` command (supported by every model, changes nothing). A
 /// non-`9000` status word or any transport error means HID isn't usable for the
@@ -795,15 +873,18 @@ impl Token2OtpSession {
                         transport: Box::new(t),
                         is_pcsc: false,
                     }),
-                    Err(_) => {
-                        // HID present but applet unreachable (HID disabled on the
-                        // device, or the probe timed out) — try CCID instead.
+                    Err((probe_err, _)) => {
+                        // HID present but the applet didn't accept the probe
+                        // (HID disabled on the device, the probe timed out, or
+                        // the key declined) — try CCID instead.
                         match PcScOtpTransport::open_first_debug(debug) {
                             Ok(p) => Ok(Self {
                                 transport: Box::new(p),
                                 is_pcsc: true,
                             }),
-                            Err(_) => Err(OtpTransportError::NoUsableInterface),
+                            // No CCID either, so the probe outcome is all we
+                            // know — and it matters which kind it was.
+                            Err(_) => Err(detect_failure_from(probe_err)),
                         }
                     }
                 }
@@ -1417,3 +1498,102 @@ mod hidraw_bounded_read_tests {
         assert!(start.elapsed() < Duration::from_secs(5));
     }
 }
+
+/// Issue #95: a key that answers must not be reported as a key that cannot be
+/// reached. The reporter's Bio3 Dual (FIDO + PGP) completed a full HID exchange
+/// and returned `0105`, and keyroost told them "HID may be disabled on the key"
+/// — advice that could not have helped, because HID was working.
+#[cfg(test)]
+mod declined_probe_is_not_an_unusable_interface {
+    use super::*;
+    use keyroost_token2otp::sw;
+
+    #[test]
+    fn an_applet_status_word_reports_the_key_declining() {
+        // Exactly the reporter's case.
+        let err = detect_failure_from(OtpTransportError::Applet(OtpError::BadStatusCode(0x0105)));
+        assert!(
+            matches!(err, OtpTransportError::HidProbeDeclined { sw: 0x0105 }),
+            "an answering applet must not be classified as an unusable interface, got {err:?}"
+        );
+        let msg = err.to_string();
+        assert!(
+            msg.contains("0x0105"),
+            "the status word belongs in the message: {msg}"
+        );
+        assert!(
+            !msg.contains("disabled"),
+            "must not tell the user to enable an interface that just carried a \
+             full request and response: {msg}"
+        );
+    }
+
+    #[test]
+    fn every_applet_error_keeps_its_status_word() {
+        // status_word() is the inverse of check(), so the transport can always
+        // recover what the key said regardless of which variant it mapped to.
+        for sw in [
+            sw::ENTRY_NOT_FOUND,
+            sw::NOT_ENOUGH_SPACE,
+            sw::BUTTON_TIMEOUT,
+            sw::HID_NOT_SUPPORTED,
+            0x0105,
+            0x6A80,
+        ] {
+            let e = OtpError::check(sw).expect_err("non-9000 must map to an error");
+            match detect_failure_from(OtpTransportError::Applet(e)) {
+                OtpTransportError::HidProbeDeclined { sw: got } => {
+                    assert_eq!(got, sw, "status word must survive the round trip");
+                }
+                other => panic!("expected HidProbeDeclined for {sw:#06X}, got {other:?}"),
+            }
+        }
+    }
+
+    #[test]
+    fn a_silent_or_broken_interface_still_blames_the_interface() {
+        // The other half of the distinction: no answer really is a transport
+        // problem, and must keep the original message.
+        for probe_err in [
+            OtpTransportError::TokenNotDetected,
+            OtpTransportError::EmptyResponse,
+            OtpTransportError::TransportUnavailable("no such device".into()),
+        ] {
+            assert!(
+                matches!(
+                    detect_failure_from(probe_err),
+                    OtpTransportError::NoUsableInterface
+                ),
+                "a probe that got no answer is evidence about the interface"
+            );
+        }
+    }
+
+    #[test]
+    fn the_reporters_product_id_is_refused_before_any_apdu() {
+        // 0x0204 is Bio3 Dual (FIDO + PGP) — supplied without OTP.
+        assert!(
+            !keyroost_proto::token2_pid_may_have_otp(0x0204),
+            "0x0204 has no OTP function and must not be probed"
+        );
+        let err = OtpTransportError::OtpFunctionNotFitted {
+            pid: 0x0204,
+            model: keyroost_proto::token2_pid_label(0x0204),
+        };
+        let msg = err.to_string();
+        assert!(msg.contains("0x0204"), "name the product id: {msg}");
+        assert!(
+            msg.contains("does not have the on-device OTP function"),
+            "state the real reason: {msg}"
+        );
+    }
+
+    #[test]
+    fn an_unknown_product_id_is_still_probed() {
+        // Fail-open: a PID we have never seen must not be refused offline.
+        assert!(
+            keyroost_proto::token2_pid_may_have_otp(0xFFFF),
+            "an unrecognised PID must fall through to a live probe"
+        );
+    }
+}