From bc80f5ab55b609335ec91723e96ac2644f254bb9 Mon Sep 17 00:00:00 2001 From: Michael Farrell Date: Thu, 13 Aug 2026 17:53:43 +1000 Subject: [PATCH 01/10] Don't use U2F on devices that don't support it (#367) --- src/authenticatorservice.rs | 16 ++++ src/ctap2/commands/get_assertion.rs | 47 +++++++--- src/ctap2/commands/get_info.rs | 13 ++- src/ctap2/commands/get_version.rs | 11 ++- src/ctap2/mod.rs | 135 ++++++++++++++++------------ src/lib.rs | 4 +- src/statemachine.rs | 65 ++++++++++++-- src/transport/device_selector.rs | 19 ++-- src/transport/freebsd/device.rs | 19 ++-- src/transport/hid.rs | 14 ++- src/transport/linux/device.rs | 19 ++-- src/transport/macos/device.rs | 19 ++-- src/transport/mock/device.rs | 19 ++-- src/transport/mod.rs | 48 ++++++---- src/transport/netbsd/device.rs | 19 ++-- src/transport/openbsd/device.rs | 19 ++-- src/transport/stub/device.rs | 6 +- src/transport/windows/device.rs | 21 ++--- 18 files changed, 334 insertions(+), 179 deletions(-) diff --git a/src/authenticatorservice.rs b/src/authenticatorservice.rs index 084d9e17..ba19e843 100644 --- a/src/authenticatorservice.rs +++ b/src/authenticatorservice.rs @@ -26,6 +26,14 @@ pub struct RegisterArgs { pub resident_key_req: ResidentKeyRequirement, pub extensions: AuthenticationExtensionsClientInputs, pub pin: Option, + + /// Register the credential using CTAP1/U2F only. + /// + /// The request [must be compatible with CTAP1 authenticators][0]. + /// + /// This will automatically skip any authenticator that doesn't support CTAP1. + /// + /// [0]: https://fidoalliance.org/specs/fido-v2.1-ps-20210615/fido-client-to-authenticator-protocol-v2.1-ps-errata-20220621.html#u2f-authenticatorMakeCredential-interoperability pub use_ctap1_fallback: bool, } @@ -39,6 +47,14 @@ pub struct SignArgs { pub user_presence_req: bool, pub extensions: AuthenticationExtensionsClientInputs, pub pin: Option, + + /// Authenticate using CTAP1/U2F only. + /// + /// The request [must be compatible with CTAP1 authenticators][0]. + /// + /// This will automatically skip any authenticator that doesn't support CTAP1. + /// + /// [0]: https://fidoalliance.org/specs/fido-v2.1-ps-20210615/fido-client-to-authenticator-protocol-v2.1-ps-errata-20220621.html#u2f-authenticatorGetAssertion-interoperability pub use_ctap1_fallback: bool, } diff --git a/src/ctap2/commands/get_assertion.rs b/src/ctap2/commands/get_assertion.rs index 7e40614e..f481b200 100644 --- a/src/ctap2/commands/get_assertion.rs +++ b/src/ctap2/commands/get_assertion.rs @@ -933,7 +933,7 @@ pub mod test { }; use crate::transport::device_selector::Device; use crate::transport::hid::HIDDevice; - use crate::transport::{FidoDevice, FidoDeviceIO, FidoProtocol}; + use crate::transport::{CtapVersionSupport, FidoDevice, FidoDeviceIO, FidoProtocol}; use crate::u2ftypes::U2FDeviceInfo; use rand::{thread_rng, RngCore}; use std::sync::mpsc::channel; @@ -1308,8 +1308,25 @@ pub mod test { ); } + fn fill_device_ctap1_init(device: &mut Device, cid: [u8; 4]) { + // init + let mut msg = vec![0xFF, 0xFF, 0xFF, 0xFF]; // broadcast + msg.extend([HIDCmd::Init.into(), 0x00, 8]); // cmd + bcnt + msg.extend([0x08, 0x07, 0x06, 0x05, 0x04, 0x03, 0x02, 0x01]); // nonce + device.add_write(&msg, 0); + + let mut msg = vec![0xFF, 0xFF, 0xFF, 0xFF]; // broadcast + msg.extend([0x06, 0x00, 17]); // cmd + bcnt + msg.extend([0x08, 0x07, 0x06, 0x05, 0x04, 0x03, 0x02, 0x01]); // nonce + msg.extend(&cid); + msg.push(2); // CTAPHID protocol version identifir + msg.extend([1, 0, 0]); // Device version numbber + msg.push(0x01); // CAPABILITY_WINK + device.add_read(&msg, 0); + } + fn fill_device_ctap1(device: &mut Device, cid: [u8; 4], flags: u8, answer_status: [u8; 2]) { - // ctap2 request + // ctap1 request let mut msg = cid.to_vec(); msg.extend([HIDCmd::Msg.into(), 0x00, 0x8A]); // cmd + bcnt msg.extend([0x00, 0x2]); // U2F_AUTHENTICATE @@ -1376,13 +1393,15 @@ pub mod test { Default::default(), ); let mut device = Device::new("commands/get_assertion").unwrap(); // not really used (all functions ignore it) - // channel id - device.downgrade_to_ctap1(); - assert_eq!(device.get_protocol(), FidoProtocol::CTAP1); + let mut cid = [0u8; 4]; thread_rng().fill_bytes(&mut cid); - - device.set_cid(cid); + fill_device_ctap1_init(&mut device, cid); + HIDDevice::pre_init(&mut device).expect("pre_init"); + assert!(device.supports_ctap1()); + assert!(!device.supports_ctap2()); + device.downgrade_to_ctap1().expect("failed to downgrade"); + assert_eq!(device.get_protocol(), FidoProtocol::CTAP1); // ctap1 request let (tx, _rx) = channel(); @@ -1471,13 +1490,15 @@ pub mod test { ); let mut device = Device::new("commands/get_assertion").unwrap(); // not really used (all functions ignore it) - // channel id - device.downgrade_to_ctap1(); - assert_eq!(device.get_protocol(), FidoProtocol::CTAP1); + let mut cid = [0u8; 4]; thread_rng().fill_bytes(&mut cid); - - device.set_cid(cid); + fill_device_ctap1_init(&mut device, cid); + HIDDevice::pre_init(&mut device).expect("pre_init"); + assert!(device.supports_ctap1()); + assert!(!device.supports_ctap2()); + device.downgrade_to_ctap1().expect("failed to downgrade"); + assert_eq!(device.get_protocol(), FidoProtocol::CTAP1); let (tx, _rx) = channel(); assert_matches!( @@ -1672,6 +1693,8 @@ pub mod test { version_build: 0x08, cap_flags: Capability::WINK | Capability::CBOR, }); + assert!(device.supports_ctap1()); + assert!(device.supports_ctap2()); device.set_authenticator_info(AuthenticatorInfo { versions: vec![AuthenticatorVersion::U2F_V2, AuthenticatorVersion::FIDO_2_0], extensions: vec!["uvm".to_string(), "hmac-secret".to_string()], diff --git a/src/ctap2/commands/get_info.rs b/src/ctap2/commands/get_info.rs index a0eef804..efbce9ac 100644 --- a/src/ctap2/commands/get_info.rs +++ b/src/ctap2/commands/get_info.rs @@ -601,7 +601,7 @@ pub mod tests { use crate::crypto::COSEAlgorithm; use crate::transport::device_selector::Device; use crate::transport::platform::device::IN_HID_RPT_SIZE; - use crate::transport::{hid::HIDDevice, FidoDevice, FidoProtocol}; + use crate::transport::{hid::HIDDevice, CtapVersionSupport, FidoDevice, FidoProtocol}; use rand::{thread_rng, RngCore}; use serde_cbor::de::from_slice; @@ -959,7 +959,6 @@ pub mod tests { #[test] fn test_get_info_ctap2_only() { let mut device = Device::new("commands/get_info").unwrap(); - assert_eq!(device.get_protocol(), FidoProtocol::CTAP2); let nonce = [0x08, 0x07, 0x06, 0x05, 0x04, 0x03, 0x02, 0x01]; // channel id @@ -1006,6 +1005,16 @@ pub mod tests { assert_eq!(device.get_cid(), &cid); + assert!(!device.supports_ctap1()); + assert!(device.supports_ctap2()); + assert_eq!(device.get_protocol(), FidoProtocol::CTAP2); + device + .downgrade_to_ctap1() + .expect_err("downgrading to CTAP1 should fail"); + assert_eq!(device.get_protocol(), FidoProtocol::CTAP2); + assert!(!device.supports_ctap1()); + assert!(device.supports_ctap2()); + let dev_info = device.get_device_info(); assert_eq!( dev_info.cap_flags, diff --git a/src/ctap2/commands/get_version.rs b/src/ctap2/commands/get_version.rs index 40019c8f..dda7b9a5 100644 --- a/src/ctap2/commands/get_version.rs +++ b/src/ctap2/commands/get_version.rs @@ -62,14 +62,12 @@ impl RequestCtap1 for GetVersion { pub mod tests { use crate::consts::{Capability, HIDCmd, CID_BROADCAST, SW_NO_ERROR}; use crate::transport::device_selector::Device; - use crate::transport::{hid::HIDDevice, FidoDevice, FidoProtocol}; + use crate::transport::{hid::HIDDevice, CtapVersionSupport, FidoDevice, FidoProtocol}; use rand::{thread_rng, RngCore}; #[test] fn test_get_version_ctap1_only() { let mut device = Device::new("commands/get_version").unwrap(); - device.downgrade_to_ctap1(); - assert_eq!(device.get_protocol(), FidoProtocol::CTAP1); let nonce = [0x08, 0x07, 0x06, 0x05, 0x04, 0x03, 0x02, 0x01]; // channel id @@ -91,7 +89,7 @@ pub mod tests { msg.extend_from_slice(&nonce); msg.extend_from_slice(&cid); // new channel id - // We are not setting CBOR, to signal that the device does not support CTAP1 + // We are not setting CBOR, to signal that the device does not support CTAP2 msg.extend([0x02, 0x04, 0x01, 0x08, 0x01]); // versions + flags (wink) device.add_read(&msg, 0); @@ -109,6 +107,11 @@ pub mod tests { device.add_read(&msg, 0); device.init().expect("Failed to init device"); + assert!(device.supports_ctap1()); + assert!(!device.supports_ctap2()); + + device.downgrade_to_ctap1().expect("failed to downgrade"); + assert_eq!(device.get_protocol(), FidoProtocol::CTAP1); assert_eq!(device.get_cid(), &cid); diff --git a/src/ctap2/mod.rs b/src/ctap2/mod.rs index d05cabeb..406446fa 100644 --- a/src/ctap2/mod.rs +++ b/src/ctap2/mod.rs @@ -413,6 +413,37 @@ fn determine_puap_if_needed Err(AuthenticatorError::CancelledByUser) } +/// Check that the registration request can be processed by a CTAP1 device. +/// +/// See [CTAP 2.1 §10.2][0]. +/// +/// Some additional checks are performed in `MakeCredentials::RequestCtap1`. +/// +/// [0]: https://fidoalliance.org/specs/fido-v2.1-ps-20210615/fido-client-to-authenticator-protocol-v2.1-ps-errata-20220621.html#u2f-authenticatorMakeCredential-interoperability +pub(crate) fn check_ctap1_register_compatibility(args: &RegisterArgs) -> crate::Result<()> { + if args.resident_key_req == ResidentKeyRequirement::Required { + return Err(AuthenticatorError::UnsupportedOption( + UnsupportedOption::ResidentKey, + )); + } + if args.user_verification_req == UserVerificationRequirement::Required { + return Err(AuthenticatorError::UnsupportedOption( + UnsupportedOption::UserVerification, + )); + } + if !args + .pub_cred_params + .iter() + .any(|x| x.alg == COSEAlgorithm::ES256) + { + return Err(AuthenticatorError::UnsupportedOption( + UnsupportedOption::PubCredParams, + )); + } + + Ok(()) +} + pub fn register( dev: &mut Dev, args: RegisterArgs, @@ -422,50 +453,33 @@ pub fn register( ) -> bool { let mut options = MakeCredentialsOptions::default(); - if dev.get_protocol() == FidoProtocol::CTAP2 { - let info = match dev.get_authenticator_info() { - Some(info) => info, - None => { - callback.call(Err(HIDError::DeviceNotInitialized.into())); - return false; - } - }; + match dev.get_protocol() { + FidoProtocol::CTAP2 => { + let info = match dev.get_authenticator_info() { + Some(info) => info, + None => { + callback.call(Err(HIDError::DeviceNotInitialized.into())); + return false; + } + }; - // Set options based on the arguments and the device info. - // The user verification option will be set in `determine_puap_if_needed`. - options.resident_key = match args.resident_key_req { - ResidentKeyRequirement::Required => Some(true), - ResidentKeyRequirement::Preferred => { - // Use a resident key if the authenticator supports it - Some(info.options.resident_key) - } - ResidentKeyRequirement::Discouraged => Some(false), - } - } else { - // Check that the request can be processed by a CTAP1 device. - // See CTAP 2.1 Section 10.2. Some additional checks are performed in - // MakeCredentials::RequestCtap1 - if args.resident_key_req == ResidentKeyRequirement::Required { - callback.call(Err(AuthenticatorError::UnsupportedOption( - UnsupportedOption::ResidentKey, - ))); - return false; - } - if args.user_verification_req == UserVerificationRequirement::Required { - callback.call(Err(AuthenticatorError::UnsupportedOption( - UnsupportedOption::UserVerification, - ))); - return false; + // Set options based on the arguments and the device info. + // The user verification option will be set in `determine_puap_if_needed`. + options.resident_key = match args.resident_key_req { + ResidentKeyRequirement::Required => Some(true), + ResidentKeyRequirement::Preferred => { + // Use a resident key if the authenticator supports it + Some(info.options.resident_key) + } + ResidentKeyRequirement::Discouraged => Some(false), + }; } - if !args - .pub_cred_params - .iter() - .any(|x| x.alg == COSEAlgorithm::ES256) - { - callback.call(Err(AuthenticatorError::UnsupportedOption( - UnsupportedOption::PubCredParams, - ))); - return false; + + FidoProtocol::CTAP1 => { + if let Err(e) = check_ctap1_register_compatibility(&args) { + callback.call(Err(e)); + return false; + } } } @@ -590,6 +604,28 @@ pub fn register( false } +/// Check that the signing request can be processed by a CTAP1 device. +/// +/// See [CTAP 2.1 §10.3][0]. +/// +/// Some additional checks are performed in `GetAssertion::RequestCtap1`. +/// +/// [0]: https://fidoalliance.org/specs/fido-v2.1-ps-20210615/fido-client-to-authenticator-protocol-v2.1-ps-errata-20220621.html#u2f-authenticatorGetAssertion-interoperability +pub(crate) fn check_ctap1_sign_compatibility(args: &SignArgs) -> crate::Result<()> { + if args.user_verification_req == UserVerificationRequirement::Required { + return Err(AuthenticatorError::UnsupportedOption( + UnsupportedOption::UserVerification, + )); + } + if args.allow_list.is_empty() { + return Err(AuthenticatorError::UnsupportedOption( + UnsupportedOption::EmptyAllowList, + )); + } + + Ok(()) +} + pub fn sign( dev: &mut Dev, args: SignArgs, @@ -598,19 +634,8 @@ pub fn sign( alive: &dyn Fn() -> bool, ) -> bool { if dev.get_protocol() == FidoProtocol::CTAP1 { - // Check that the request can be processed by a CTAP1 device. - // See CTAP 2.1 Section 10.3. Some additional checks are performed in - // GetAssertion::RequestCtap1 - if args.user_verification_req == UserVerificationRequirement::Required { - callback.call(Err(AuthenticatorError::UnsupportedOption( - UnsupportedOption::UserVerification, - ))); - return false; - } - if args.allow_list.is_empty() { - callback.call(Err(AuthenticatorError::UnsupportedOption( - UnsupportedOption::EmptyAllowList, - ))); + if let Err(e) = check_ctap1_sign_compatibility(&args) { + callback.call(Err(e)); return false; } } diff --git a/src/lib.rs b/src/lib.rs index 38a64e53..b7698879 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -52,7 +52,9 @@ pub use status_update::{ BioEnrollmentCmd, CredManagementCmd, InteractiveRequest, InteractiveUpdate, MessageDirection, StatusPinUv, StatusUpdate, }; -pub use transport::{BlinkResult, FidoDevice, FidoDeviceIO, FidoProtocol, VirtualFidoDevice}; +pub use transport::{ + BlinkResult, CtapVersionSupport, FidoDevice, FidoDeviceIO, FidoProtocol, VirtualFidoDevice, +}; // Keep this in sync with the constants in u2fhid-capi.h. bitflags! { diff --git a/src/statemachine.rs b/src/statemachine.rs index c8ac631d..1834194f 100644 --- a/src/statemachine.rs +++ b/src/statemachine.rs @@ -12,7 +12,7 @@ use crate::transport::device_selector::{ BlinkResult, Device, DeviceBuildParameters, DeviceCommand, DeviceSelectorEvent, }; use crate::transport::platform::transaction::Transaction; -use crate::transport::{hid::HIDDevice, FidoDevice, FidoProtocol}; +use crate::transport::{hid::HIDDevice, CtapVersionSupport, FidoDevice, FidoProtocol}; use crate::{InteractiveRequest, ManageResult}; use std::sync::mpsc::{channel, RecvTimeoutError, Sender}; use std::time::Duration; @@ -118,6 +118,21 @@ impl StateMachine { status: Sender, callback: StateCallback>, ) { + // Could the request be handled by a CTAP1 authenticator? + let ctap2_only = { + if let Err(e) = ctap2::check_ctap1_register_compatibility(&args) { + if args.use_ctap1_fallback { + // There's no way a CTAP1 authenticator could approve this request, so cancel early. + callback.call(Err(e)); + return; + } + + true + } else { + false + } + }; + // Abort any prior register/sign calls. self.cancel(); let cbc = callback.clone(); @@ -130,12 +145,25 @@ impl StateMachine { Some(dev) => dev, None => return, }; + + if ctap2_only && !dev.supports_ctap2() { + // CTAP1-only authenticator with a CTAP2-only request. + return; + } + + if args.use_ctap1_fallback && !dev.supports_ctap1() { + // CTAP2-only authenticator with a CTAP1-only request. + return; + } + if !Self::wait_for_device_selector(&mut dev, &selector, &status, alive) { return; }; - if args.use_ctap1_fallback { - dev.downgrade_to_ctap1(); + if args.use_ctap1_fallback && dev.downgrade_to_ctap1().is_err() { + // We shouldn't reach this, but not downgrading early lets us use CTAP 2.1 + // device selection on devices that support it. + return; } info!("Device {:?} continues with the register process", dev.id()); @@ -158,6 +186,20 @@ impl StateMachine { status: Sender, callback: StateCallback>, ) { + // Could the request be handled by a CTAP1 authenticator? + let ctap2_only = { + if let Err(e) = ctap2::check_ctap1_sign_compatibility(&args) { + if args.use_ctap1_fallback { + // There's no way a CTAP1 authenticator could approve this request, so cancel early. + callback.call(Err(e)); + return; + } + true + } else { + false + } + }; + // Abort any prior register/sign calls. self.cancel(); let cbc = callback.clone(); @@ -171,12 +213,25 @@ impl StateMachine { Some(dev) => dev, None => return, }; + + if ctap2_only && !dev.supports_ctap2() { + // CTAP1-only authenticator with a CTAP2-only request. + return; + } + + if args.use_ctap1_fallback && !dev.supports_ctap1() { + // CTAP2-only authenticator with a CTAP1-only request. + return; + } + if !Self::wait_for_device_selector(&mut dev, &selector, &status, alive) { return; }; - if args.use_ctap1_fallback { - dev.downgrade_to_ctap1(); + if args.use_ctap1_fallback && dev.downgrade_to_ctap1().is_err() { + // We shouldn't reach this, but not downgrading early lets us use CTAP 2.1 + // device selection on devices that support it. + return; } info!("Device {:?} continues with the signing process", dev.id()); diff --git a/src/transport/device_selector.rs b/src/transport/device_selector.rs index f5c0a7db..d970ad24 100644 --- a/src/transport/device_selector.rs +++ b/src/transport/device_selector.rs @@ -196,12 +196,12 @@ pub mod tests { use crate::{ consts::Capability, ctap2::commands::get_info::{AuthenticatorInfo, AuthenticatorOptions}, - transport::FidoDevice, + transport::{CtapVersionSupport, FidoDevice}, u2ftypes::U2FDeviceInfo, }; use std::sync::mpsc::TryRecvError; - pub(crate) fn gen_info(id: String) -> U2FDeviceInfo { + pub(crate) fn gen_info(id: String, cap_flags: Capability) -> U2FDeviceInfo { U2FDeviceInfo { vendor_name: String::from("ExampleVendor").into_bytes(), device_name: id.into_bytes(), @@ -209,19 +209,24 @@ pub mod tests { version_major: 3, version_minor: 2, version_build: 1, - cap_flags: Capability::WINK | Capability::CBOR | Capability::NMSG, + cap_flags, } } pub(crate) fn make_device_simple_u2f(dev: &mut Device) { - dev.set_device_info(gen_info(dev.id())); + dev.set_device_info(gen_info(dev.id(), Capability::WINK)); dev.set_cid([1, 2, 3, 4]); // Need to set something other than broadcast - dev.downgrade_to_ctap1(); + dev.downgrade_to_ctap1().expect("failed to downgrade"); dev.create_channel(); + assert!(dev.supports_ctap1()); + assert!(!dev.supports_ctap2()); } pub(crate) fn make_device_with_pin(dev: &mut Device) { - dev.set_device_info(gen_info(dev.id())); + dev.set_device_info(gen_info( + dev.id(), + Capability::WINK | Capability::CBOR | Capability::NMSG, + )); dev.set_cid([1, 2, 3, 4]); // Need to set something other than broadcast dev.create_channel(); let info = AuthenticatorInfo { @@ -232,6 +237,8 @@ pub mod tests { ..Default::default() }; dev.set_authenticator_info(info); + assert!(!dev.supports_ctap1()); + assert!(dev.supports_ctap2()); } fn send_i_am_token(dev: &Device, selector: &DeviceSelector) { diff --git a/src/transport/freebsd/device.rs b/src/transport/freebsd/device.rs index 8682aeec..6abb0b17 100644 --- a/src/transport/freebsd/device.rs +++ b/src/transport/freebsd/device.rs @@ -4,11 +4,11 @@ extern crate libc; -use crate::consts::{Capability, CID_BROADCAST, MAX_HID_RPT_SIZE}; +use crate::consts::{CID_BROADCAST, MAX_HID_RPT_SIZE}; use crate::ctap2::commands::get_info::AuthenticatorInfo; use crate::transport::hid::HIDDevice; use crate::transport::platform::uhid; -use crate::transport::{FidoDevice, FidoProtocol, HIDError, SharedSecret}; +use crate::transport::{CtapVersionSupport, FidoDevice, FidoProtocol, HIDError, SharedSecret}; use crate::u2ftypes::U2FDeviceInfo; use crate::util::from_unix_result; use crate::util::io_err; @@ -188,12 +188,6 @@ impl FidoDevice for Device { HIDDevice::pre_init(self) } - fn should_try_ctap2(&self) -> bool { - HIDDevice::get_device_info(self) - .cap_flags - .contains(Capability::CBOR) - } - fn initialized(&self) -> bool { // During successful init, the broadcast channel id gets repplaced by an actual one self.cid != CID_BROADCAST @@ -229,7 +223,12 @@ impl FidoDevice for Device { self.protocol } - fn downgrade_to_ctap1(&mut self) { - self.protocol = FidoProtocol::CTAP1; + fn downgrade_to_ctap1(&mut self) -> Result<(), HIDError> { + if self.supports_ctap1() { + self.protocol = FidoProtocol::CTAP1; + Ok(()) + } else { + Err(HIDError::UnexpectedVersion) + } } } diff --git a/src/transport/hid.rs b/src/transport/hid.rs index 20ab7c48..a0e6d176 100644 --- a/src/transport/hid.rs +++ b/src/transport/hid.rs @@ -1,9 +1,9 @@ use super::TestDevice; -use crate::consts::{HIDCmd, CID_BROADCAST}; +use crate::consts::{Capability, HIDCmd, CID_BROADCAST}; use crate::ctap2::commands::{CommandError, RequestCtap1, RequestCtap2, Retryable, StatusCode}; use crate::status_update::{send_status, MessageDirection}; use crate::transport::errors::{ApduErrorStatus, HIDError}; -use crate::transport::{FidoDevice, FidoDeviceIO, FidoProtocol}; +use crate::transport::{CtapVersionSupport, FidoDevice, FidoDeviceIO, FidoProtocol}; use crate::u2ftypes::{U2FDeviceInfo, U2FHIDCont, U2FHIDInit, U2FHIDInitResp}; use crate::util::io_err; use crate::StatusUpdate; @@ -159,6 +159,16 @@ pub trait HIDDevice: FidoDevice + Read + Write { } } +impl CtapVersionSupport for T { + fn supports_ctap1(&self) -> bool { + !self.get_device_info().cap_flags.contains(Capability::NMSG) + } + + fn supports_ctap2(&self) -> bool { + self.get_device_info().cap_flags.contains(Capability::CBOR) + } +} + #[cfg(not(test))] impl TestDevice for T {} diff --git a/src/transport/linux/device.rs b/src/transport/linux/device.rs index 736bddd6..b5ed2404 100644 --- a/src/transport/linux/device.rs +++ b/src/transport/linux/device.rs @@ -3,11 +3,11 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ extern crate libc; -use crate::consts::{Capability, CID_BROADCAST}; +use crate::consts::CID_BROADCAST; use crate::ctap2::commands::get_info::AuthenticatorInfo; use crate::transport::hid::HIDDevice; use crate::transport::platform::{hidraw, monitor}; -use crate::transport::{FidoDevice, FidoProtocol, HIDError, SharedSecret}; +use crate::transport::{CtapVersionSupport, FidoDevice, FidoProtocol, HIDError, SharedSecret}; use crate::u2ftypes::U2FDeviceInfo; use crate::util::{from_unix_result, io_err}; use std::fs::OpenOptions; @@ -171,12 +171,6 @@ impl FidoDevice for Device { HIDDevice::pre_init(self) } - fn should_try_ctap2(&self) -> bool { - HIDDevice::get_device_info(self) - .cap_flags - .contains(Capability::CBOR) - } - fn initialized(&self) -> bool { // During successful init, the broadcast channel id gets replaced by an actual one self.cid != CID_BROADCAST @@ -206,7 +200,12 @@ impl FidoDevice for Device { self.protocol } - fn downgrade_to_ctap1(&mut self) { - self.protocol = FidoProtocol::CTAP1; + fn downgrade_to_ctap1(&mut self) -> Result<(), HIDError> { + if self.supports_ctap1() { + self.protocol = FidoProtocol::CTAP1; + Ok(()) + } else { + Err(HIDError::UnexpectedVersion) + } } } diff --git a/src/transport/macos/device.rs b/src/transport/macos/device.rs index 9acce3aa..5f1901d1 100644 --- a/src/transport/macos/device.rs +++ b/src/transport/macos/device.rs @@ -4,11 +4,11 @@ extern crate log; -use crate::consts::{Capability, CID_BROADCAST, MAX_HID_RPT_SIZE}; +use crate::consts::{CID_BROADCAST, MAX_HID_RPT_SIZE}; use crate::ctap2::commands::get_info::AuthenticatorInfo; use crate::transport::hid::HIDDevice; use crate::transport::platform::iokit::*; -use crate::transport::{FidoDevice, FidoProtocol, HIDError, SharedSecret}; +use crate::transport::{CtapVersionSupport, FidoDevice, FidoProtocol, HIDError, SharedSecret}; use crate::u2ftypes::U2FDeviceInfo; use core_foundation::base::*; use core_foundation::string::*; @@ -188,12 +188,6 @@ impl FidoDevice for Device { HIDDevice::pre_init(self) } - fn should_try_ctap2(&self) -> bool { - HIDDevice::get_device_info(self) - .cap_flags - .contains(Capability::CBOR) - } - fn initialized(&self) -> bool { self.cid != CID_BROADCAST } @@ -220,7 +214,12 @@ impl FidoDevice for Device { self.protocol } - fn downgrade_to_ctap1(&mut self) { - self.protocol = FidoProtocol::CTAP1; + fn downgrade_to_ctap1(&mut self) -> Result<(), HIDError> { + if self.supports_ctap1() { + self.protocol = FidoProtocol::CTAP1; + Ok(()) + } else { + Err(HIDError::UnexpectedVersion) + } } } diff --git a/src/transport/mock/device.rs b/src/transport/mock/device.rs index 40be689a..8ba579f2 100644 --- a/src/transport/mock/device.rs +++ b/src/transport/mock/device.rs @@ -1,13 +1,13 @@ /* This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use crate::consts::{Capability, HIDCmd, CID_BROADCAST}; +use crate::consts::{HIDCmd, CID_BROADCAST}; use crate::crypto::SharedSecret; use crate::ctap2::commands::get_info::AuthenticatorInfo; use crate::ctap2::commands::{CtapResponse, RequestCtap1, RequestCtap2}; use crate::transport::device_selector::DeviceCommand; use crate::transport::TestDevice; -use crate::transport::{hid::HIDDevice, FidoDevice, FidoProtocol, HIDError}; +use crate::transport::{hid::HIDDevice, CtapVersionSupport, FidoDevice, FidoProtocol, HIDError}; use crate::u2ftypes::{U2FDeviceInfo, U2FHIDInitResp}; use std::any::Any; use std::collections::VecDeque; @@ -291,12 +291,6 @@ impl FidoDevice for Device { HIDDevice::pre_init(self) } - fn should_try_ctap2(&self) -> bool { - HIDDevice::get_device_info(self) - .cap_flags - .contains(Capability::CBOR) - } - fn initialized(&self) -> bool { self.get_cid() != &CID_BROADCAST } @@ -325,7 +319,12 @@ impl FidoDevice for Device { self.protocol } - fn downgrade_to_ctap1(&mut self) { - self.protocol = FidoProtocol::CTAP1; + fn downgrade_to_ctap1(&mut self) -> Result<(), HIDError> { + if self.supports_ctap1() { + self.protocol = FidoProtocol::CTAP1; + Ok(()) + } else { + Err(HIDError::UnexpectedVersion) + } } } diff --git a/src/transport/mod.rs b/src/transport/mod.rs index fc62776e..c155c6d5 100644 --- a/src/transport/mod.rs +++ b/src/transport/mod.rs @@ -81,6 +81,14 @@ pub enum FidoProtocol { CTAP2, } +pub trait CtapVersionSupport { + /// `true` if the device supports CTAP1/U2F commands, according to the init message. + fn supports_ctap1(&self) -> bool; + + /// `true` if the device supports CTAP2 commands, according to the init message. + fn supports_ctap2(&self) -> bool; +} + pub trait FidoDeviceIO { fn send_msg + RequestCtap2>( &mut self, @@ -143,7 +151,7 @@ pub trait TestDevice { ) -> Result; } -pub trait FidoDevice: FidoDeviceIO +pub trait FidoDevice: FidoDeviceIO + CtapVersionSupport where Self: Sized, Self: fmt::Debug, @@ -153,7 +161,12 @@ where // Check if the device is actually a token fn is_u2f(&mut self) -> bool; - fn should_try_ctap2(&self) -> bool; + + #[deprecated = "use CtapVersionSupport::supports_ctap2"] + fn should_try_ctap2(&self) -> bool { + self.supports_ctap2() + } + fn get_authenticator_info(&self) -> Option<&AuthenticatorInfo>; fn set_authenticator_info(&mut self, authenticator_info: AuthenticatorInfo); fn refresh_authenticator_info(&mut self) -> Option<&AuthenticatorInfo> { @@ -165,15 +178,19 @@ where self.get_authenticator_info() } - // `get_protocol()` indicates whether we're using CTAP1 or CTAP2. - // Prior to initializing the device, `get_protocol()` should return CTAP2 unless - // there's a reason to believe that the device does not support CTAP2 (e.g. if - // it's a HID device and it does not have the CBOR capability). + /// Indicates whether we're using CTAP1 (U2F) or CTAP2. + /// + /// Prior to initializing the device, this returns CTAP2. Initialization checks the CBOR + /// capability, and automatically downgrades to CTAP1 if that's not supported. fn get_protocol(&self) -> FidoProtocol; - // We do not provide a generic `set_protocol(..)` function as this would have complicated - // interactions with the AuthenticatorInfo state. - fn downgrade_to_ctap1(&mut self); + /// Downgrades the connection to CTAP1/U2F. + /// + /// Returns [`HIDError::UnexpectedVersion`] if the authenticator does not support CTAP1. + /// + /// We do not provide a generic `set_protocol(..)` function as this would have complicated + /// interactions with the [`AuthenticatorInfo`] state. + fn downgrade_to_ctap1(&mut self) -> Result<(), HIDError>; fn get_shared_secret(&self) -> Option<&SharedSecret>; fn set_shared_secret(&mut self, secret: SharedSecret); @@ -181,19 +198,20 @@ where fn init(&mut self) -> Result<(), HIDError> { self.pre_init()?; - if self.should_try_ctap2() { + if self.supports_ctap2() { let command = GetInfo::default(); if let Ok(info) = self.send_cbor(&command, None) { debug!("{:?}", info); if info.max_supported_version() == AuthenticatorVersion::U2F_V2 { - self.downgrade_to_ctap1(); + self.downgrade_to_ctap1()?; } self.set_authenticator_info(info); return Ok(()); } } - self.downgrade_to_ctap1(); + // If the device sets NMSG, this will fail. + self.downgrade_to_ctap1()?; // We want to return an error here if this device doesn't support CTAP1, // so we send a U2F_VERSION command. let command = GetVersion::default(); @@ -203,9 +221,9 @@ where fn block_and_blink(&mut self, keep_alive: &dyn Fn() -> bool) -> BlinkResult { let supports_select_cmd = self.get_protocol() == FidoProtocol::CTAP2 - && self.get_authenticator_info().is_some_and(|i| { - i.versions.contains(&AuthenticatorVersion::FIDO_2_1) - }); + && self + .get_authenticator_info() + .is_some_and(|i| i.versions.contains(&AuthenticatorVersion::FIDO_2_1)); let resp = if supports_select_cmd { let msg = Selection {}; self.send_cbor_cancellable(&msg, keep_alive, None) diff --git a/src/transport/netbsd/device.rs b/src/transport/netbsd/device.rs index 693f079b..e5c07a49 100644 --- a/src/transport/netbsd/device.rs +++ b/src/transport/netbsd/device.rs @@ -3,13 +3,13 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ extern crate libc; -use crate::consts::{Capability, CID_BROADCAST, MAX_HID_RPT_SIZE}; +use crate::consts::{CID_BROADCAST, MAX_HID_RPT_SIZE}; use crate::ctap2::commands::get_info::AuthenticatorInfo; use crate::transport::hid::HIDDevice; use crate::transport::platform::fd::Fd; use crate::transport::platform::monitor::WrappedOpenDevice; use crate::transport::platform::uhid; -use crate::transport::{FidoDevice, FidoProtocol, HIDError, SharedSecret}; +use crate::transport::{CtapVersionSupport, FidoDevice, FidoProtocol, HIDError, SharedSecret}; use crate::u2ftypes::U2FDeviceInfo; use crate::util::io_err; use std::ffi::OsString; @@ -190,12 +190,6 @@ impl FidoDevice for Device { HIDDevice::pre_init(self) } - fn should_try_ctap2(&self) -> bool { - HIDDevice::get_device_info(self) - .cap_flags - .contains(Capability::CBOR) - } - fn initialized(&self) -> bool { // During successful init, the broadcast channel id gets repplaced by an actual one self.cid != CID_BROADCAST @@ -242,7 +236,12 @@ impl FidoDevice for Device { self.protocol } - fn downgrade_to_ctap1(&mut self) { - self.protocol = FidoProtocol::CTAP1; + fn downgrade_to_ctap1(&mut self) -> Result<(), HIDError> { + if self.supports_ctap1() { + self.protocol = FidoProtocol::CTAP1; + Ok(()) + } else { + Err(HIDError::UnexpectedVersion) + } } } diff --git a/src/transport/openbsd/device.rs b/src/transport/openbsd/device.rs index ad27ba27..fb56657f 100644 --- a/src/transport/openbsd/device.rs +++ b/src/transport/openbsd/device.rs @@ -3,11 +3,11 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ extern crate libc; -use crate::consts::{Capability, CID_BROADCAST, MAX_HID_RPT_SIZE}; +use crate::consts::{CID_BROADCAST, MAX_HID_RPT_SIZE}; use crate::ctap2::commands::get_info::AuthenticatorInfo; use crate::transport::hid::HIDDevice; use crate::transport::platform::monitor::WrappedOpenDevice; -use crate::transport::{FidoDevice, FidoProtocol, HIDError, SharedSecret}; +use crate::transport::{CtapVersionSupport, FidoDevice, FidoProtocol, HIDError, SharedSecret}; use crate::u2ftypes::U2FDeviceInfo; use crate::util::{from_unix_result, io_err}; use std::ffi::{CString, OsString}; @@ -171,12 +171,6 @@ impl FidoDevice for Device { HIDDevice::pre_init(self) } - fn should_try_ctap2(&self) -> bool { - HIDDevice::get_device_info(self) - .cap_flags - .contains(Capability::CBOR) - } - fn initialized(&self) -> bool { // During successful init, the broadcast channel id gets repplaced by an actual one self.cid != CID_BROADCAST @@ -218,7 +212,12 @@ impl FidoDevice for Device { self.protocol } - fn downgrade_to_ctap1(&mut self) { - self.protocol = FidoProtocol::CTAP1; + fn downgrade_to_ctap1(&mut self) -> Result<(), HIDError> { + if self.supports_ctap1() { + self.protocol = FidoProtocol::CTAP1; + Ok(()) + } else { + Err(HIDError::UnexpectedVersion) + } } } diff --git a/src/transport/stub/device.rs b/src/transport/stub/device.rs index 29d8a3ab..6d8ae0b0 100644 --- a/src/transport/stub/device.rs +++ b/src/transport/stub/device.rs @@ -77,10 +77,6 @@ impl FidoDevice for Device { unimplemented!(); } - fn should_try_ctap2(&self) -> bool { - unimplemented!(); - } - fn initialized(&self) -> bool { unimplemented!(); } @@ -109,7 +105,7 @@ impl FidoDevice for Device { unimplemented!() } - fn downgrade_to_ctap1(&mut self) { + fn downgrade_to_ctap1(&mut self) -> Result<(), HIDError> { unimplemented!() } } diff --git a/src/transport/windows/device.rs b/src/transport/windows/device.rs index fe59569d..11620dbb 100644 --- a/src/transport/windows/device.rs +++ b/src/transport/windows/device.rs @@ -3,12 +3,10 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use super::winapi::DeviceCapabilities; -use crate::consts::{ - Capability, CID_BROADCAST, FIDO_USAGE_PAGE, FIDO_USAGE_U2FHID, MAX_HID_RPT_SIZE, -}; +use crate::consts::{CID_BROADCAST, FIDO_USAGE_PAGE, FIDO_USAGE_U2FHID, MAX_HID_RPT_SIZE}; use crate::ctap2::commands::get_info::AuthenticatorInfo; use crate::transport::hid::HIDDevice; -use crate::transport::{FidoDevice, FidoProtocol, HIDError, SharedSecret}; +use crate::transport::{CtapVersionSupport, FidoDevice, FidoProtocol, HIDError, SharedSecret}; use crate::u2ftypes::U2FDeviceInfo; use std::fs::{File, OpenOptions}; use std::hash::{Hash, Hasher}; @@ -129,12 +127,6 @@ impl FidoDevice for Device { HIDDevice::pre_init(self) } - fn should_try_ctap2(&self) -> bool { - HIDDevice::get_device_info(self) - .cap_flags - .contains(Capability::CBOR) - } - fn initialized(&self) -> bool { // During successful init, the broadcast channel id gets repplaced by an actual one self.cid != CID_BROADCAST @@ -167,7 +159,12 @@ impl FidoDevice for Device { self.protocol } - fn downgrade_to_ctap1(&mut self) { - self.protocol = FidoProtocol::CTAP1; + fn downgrade_to_ctap1(&mut self) -> Result<(), HIDError> { + if self.supports_ctap1() { + self.protocol = FidoProtocol::CTAP1; + Ok(()) + } else { + Err(HIDError::UnexpectedVersion) + } } } From 9a57ff1797cd63218755887536042b2f92b0f334 Mon Sep 17 00:00:00 2001 From: Michael Farrell Date: Thu, 20 Aug 2026 11:30:37 +1000 Subject: [PATCH 02/10] check the response when downgrades should fail --- src/ctap2/commands/get_info.rs | 9 ++++++--- src/transport/device_selector.rs | 10 +++++++++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/ctap2/commands/get_info.rs b/src/ctap2/commands/get_info.rs index efbce9ac..28a7d6fa 100644 --- a/src/ctap2/commands/get_info.rs +++ b/src/ctap2/commands/get_info.rs @@ -1008,9 +1008,12 @@ pub mod tests { assert!(!device.supports_ctap1()); assert!(device.supports_ctap2()); assert_eq!(device.get_protocol(), FidoProtocol::CTAP2); - device - .downgrade_to_ctap1() - .expect_err("downgrading to CTAP1 should fail"); + assert_matches!( + device + .downgrade_to_ctap1() + .expect_err("downgrading to CTAP1 should fail when NMSG"), + HIDError::UnexpectedVersion + ); assert_eq!(device.get_protocol(), FidoProtocol::CTAP2); assert!(!device.supports_ctap1()); assert!(device.supports_ctap2()); diff --git a/src/transport/device_selector.rs b/src/transport/device_selector.rs index d970ad24..4dd40d42 100644 --- a/src/transport/device_selector.rs +++ b/src/transport/device_selector.rs @@ -196,7 +196,8 @@ pub mod tests { use crate::{ consts::Capability, ctap2::commands::get_info::{AuthenticatorInfo, AuthenticatorOptions}, - transport::{CtapVersionSupport, FidoDevice}, + errors::HIDError, + transport::{CtapVersionSupport, FidoDevice, FidoProtocol}, u2ftypes::U2FDeviceInfo, }; use std::sync::mpsc::TryRecvError; @@ -220,6 +221,7 @@ pub mod tests { dev.create_channel(); assert!(dev.supports_ctap1()); assert!(!dev.supports_ctap2()); + assert_eq!(FidoProtocol::CTAP1, dev.get_protocol()); } pub(crate) fn make_device_with_pin(dev: &mut Device) { @@ -228,6 +230,11 @@ pub mod tests { Capability::WINK | Capability::CBOR | Capability::NMSG, )); dev.set_cid([1, 2, 3, 4]); // Need to set something other than broadcast + assert_matches!( + dev.downgrade_to_ctap1() + .expect_err("downgrading to CTAP1 should fail when NMSG"), + HIDError::UnexpectedVersion + ); dev.create_channel(); let info = AuthenticatorInfo { options: AuthenticatorOptions { @@ -239,6 +246,7 @@ pub mod tests { dev.set_authenticator_info(info); assert!(!dev.supports_ctap1()); assert!(dev.supports_ctap2()); + assert_eq!(FidoProtocol::CTAP2, dev.get_protocol()); } fn send_i_am_token(dev: &Device, selector: &DeviceSelector) { From 601dd5225636e5a14ece958c5c0f8f2af7ba7766 Mon Sep 17 00:00:00 2001 From: Michael Farrell Date: Thu, 20 Aug 2026 12:03:24 +1000 Subject: [PATCH 03/10] Gate `send_cbor_cancellable` / `send_ctap1_cancellable` on version support, fixup tests with that call path --- src/ctap2/commands/get_assertion.rs | 10 ++--- src/ctap2/commands/get_info.rs | 33 +++------------ src/ctap2/commands/get_version.rs | 53 +++--------------------- src/ctap2/commands/large_blobs.rs | 64 ++++++++++++----------------- src/ctap2/commands/reset.rs | 13 +++--- src/ctap2/commands/selection.rs | 17 ++++---- src/transport/hid.rs | 8 ++++ src/transport/mock/device.rs | 42 +++++++++++++++++-- 8 files changed, 107 insertions(+), 133 deletions(-) diff --git a/src/ctap2/commands/get_assertion.rs b/src/ctap2/commands/get_assertion.rs index f481b200..3e951777 100644 --- a/src/ctap2/commands/get_assertion.rs +++ b/src/ctap2/commands/get_assertion.rs @@ -966,11 +966,11 @@ pub mod test { }, Default::default(), ); - let mut device = Device::new("commands/get_assertion").unwrap(); - assert_eq!(device.get_protocol(), FidoProtocol::CTAP2); - let mut cid = [0u8; 4]; - thread_rng().fill_bytes(&mut cid); - device.set_cid(cid); + let mut device = Device::new_pre_inited( + "commands/get_info", + Capability::CBOR | Capability::NMSG | Capability::WINK, + ); + let cid = device.get_cid().clone(); let mut msg = cid.to_vec(); msg.extend(vec![HIDCmd::Cbor.into(), 0x00, 0x90]); diff --git a/src/ctap2/commands/get_info.rs b/src/ctap2/commands/get_info.rs index 28a7d6fa..1002b7d7 100644 --- a/src/ctap2/commands/get_info.rs +++ b/src/ctap2/commands/get_info.rs @@ -597,12 +597,11 @@ impl<'de> Deserialize<'de> for AuthenticatorInfo { #[cfg(test)] pub mod tests { use super::*; - use crate::consts::{Capability, HIDCmd, CID_BROADCAST}; + use crate::consts::{Capability, HIDCmd}; use crate::crypto::COSEAlgorithm; use crate::transport::device_selector::Device; use crate::transport::platform::device::IN_HID_RPT_SIZE; use crate::transport::{hid::HIDDevice, CtapVersionSupport, FidoDevice, FidoProtocol}; - use rand::{thread_rng, RngCore}; use serde_cbor::de::from_slice; // Raw data take from https://github.com/Yubico/python-fido2/blob/master/test/test_ctap2.py @@ -958,31 +957,11 @@ pub mod tests { #[test] fn test_get_info_ctap2_only() { - let mut device = Device::new("commands/get_info").unwrap(); - let nonce = [0x08, 0x07, 0x06, 0x05, 0x04, 0x03, 0x02, 0x01]; - - // channel id - let mut cid = [0u8; 4]; - thread_rng().fill_bytes(&mut cid); - - // init packet - let mut msg = CID_BROADCAST.to_vec(); - msg.extend(vec![HIDCmd::Init.into(), 0x00, 0x08]); // cmd + bcnt - msg.extend_from_slice(&nonce); - device.add_write(&msg, 0); - - // init_resp packet - let mut msg = CID_BROADCAST.to_vec(); - msg.extend(vec![ - 0x06, /* HIDCmd::Init without TYPE_INIT */ - 0x00, 0x11, - ]); // cmd + bcnt - msg.extend_from_slice(&nonce); - msg.extend_from_slice(&cid); // new channel id - - // We are setting NMSG, to signal that the device does not support CTAP1 - msg.extend(vec![0x02, 0x04, 0x01, 0x08, 0x01 | 0x04 | 0x08]); // versions + flags (wink+cbor+nmsg) - device.add_read(&msg, 0); + let mut device = Device::new_pre_inited( + "commands/get_info", + Capability::CBOR | Capability::NMSG | Capability::WINK, + ); + let cid = device.get_cid().clone(); // ctap2 request let mut msg = cid.to_vec(); diff --git a/src/ctap2/commands/get_version.rs b/src/ctap2/commands/get_version.rs index dda7b9a5..f975aee3 100644 --- a/src/ctap2/commands/get_version.rs +++ b/src/ctap2/commands/get_version.rs @@ -60,60 +60,19 @@ impl RequestCtap1 for GetVersion { #[cfg(test)] pub mod tests { - use crate::consts::{Capability, HIDCmd, CID_BROADCAST, SW_NO_ERROR}; + use crate::consts::Capability; use crate::transport::device_selector::Device; - use crate::transport::{hid::HIDDevice, CtapVersionSupport, FidoDevice, FidoProtocol}; - use rand::{thread_rng, RngCore}; + use crate::transport::{hid::HIDDevice, FidoDevice, FidoProtocol}; + use crate::CtapVersionSupport; #[test] fn test_get_version_ctap1_only() { - let mut device = Device::new("commands/get_version").unwrap(); - let nonce = [0x08, 0x07, 0x06, 0x05, 0x04, 0x03, 0x02, 0x01]; - - // channel id - let mut cid = [0u8; 4]; - thread_rng().fill_bytes(&mut cid); - - // init packet - let mut msg = CID_BROADCAST.to_vec(); - msg.extend([HIDCmd::Init.into(), 0x00, 0x08]); // cmd + bcnt - msg.extend_from_slice(&nonce); - device.add_write(&msg, 0); - - // init_resp packet - let mut msg = CID_BROADCAST.to_vec(); - msg.extend(vec![ - 0x06, /* HIDCmd::Init without !TYPE_INIT */ - 0x00, 0x11, - ]); // cmd + bcnt - msg.extend_from_slice(&nonce); - msg.extend_from_slice(&cid); // new channel id - - // We are not setting CBOR, to signal that the device does not support CTAP2 - msg.extend([0x02, 0x04, 0x01, 0x08, 0x01]); // versions + flags (wink) - device.add_read(&msg, 0); - - // ctap1 U2F_VERSION request - let mut msg = cid.to_vec(); - msg.extend([HIDCmd::Msg.into(), 0x0, 0x7]); // cmd + bcnt - msg.extend([0x0, 0x3, 0x0, 0x0, 0x0, 0x0, 0x0]); - device.add_write(&msg, 0); - - // fido response - let mut msg = cid.to_vec(); - msg.extend([HIDCmd::Msg.into(), 0x0, 0x08]); // cmd + bcnt - msg.extend([0x55, 0x32, 0x46, 0x5f, 0x56, 0x32]); // 'U2F_V2' - msg.extend(SW_NO_ERROR); - device.add_read(&msg, 0); - - device.init().expect("Failed to init device"); - assert!(device.supports_ctap1()); - assert!(!device.supports_ctap2()); + let mut device = Device::new_pre_inited("commands/get_version", Capability::WINK); device.downgrade_to_ctap1().expect("failed to downgrade"); assert_eq!(device.get_protocol(), FidoProtocol::CTAP1); - - assert_eq!(device.get_cid(), &cid); + assert!(device.supports_ctap1()); + assert!(!device.supports_ctap2()); let dev_info = device.get_device_info(); assert_eq!(dev_info.cap_flags, Capability::WINK); diff --git a/src/ctap2/commands/large_blobs.rs b/src/ctap2/commands/large_blobs.rs index a16e8813..43f8bd26 100644 --- a/src/ctap2/commands/large_blobs.rs +++ b/src/ctap2/commands/large_blobs.rs @@ -471,12 +471,11 @@ where #[cfg(test)] pub mod tests { use super::*; - use crate::consts::HIDCmd; + use crate::consts::{Capability, HIDCmd}; use crate::transport::device_selector::Device; use crate::transport::hid::HIDDevice; use crate::transport::platform::device::{IN_HID_RPT_SIZE, OUT_HID_RPT_SIZE}; - use crate::transport::{FidoDevice, FidoProtocol}; - use rand::{thread_rng, RngCore}; + use crate::transport::FidoDevice; fn add_bytes_to_read(cid: &[u8], bytes: &[u8], device: &mut Device) { let mut data = Vec::new(); @@ -522,13 +521,11 @@ pub mod tests { #[test] fn test_read_large_blob_array() { let keep_alive = || true; - let mut device = Device::new("commands/large_blobs").unwrap(); - assert_eq!(device.get_protocol(), FidoProtocol::CTAP2); - - // 'initialize' the device - let mut cid = [0u8; 4]; - thread_rng().fill_bytes(&mut cid); - device.set_cid(cid); + let mut device = Device::new_pre_inited( + "commands/large_blob", + Capability::CBOR | Capability::NMSG | Capability::WINK, + ); + let cid = device.get_cid().clone(); let cmd = [ 0xa2, // map(2) @@ -553,13 +550,11 @@ pub mod tests { #[test] fn test_read_large_blob_array_with_wrong_hash() { let keep_alive = || true; - let mut device = Device::new("commands/large_blobs").unwrap(); - assert_eq!(device.get_protocol(), FidoProtocol::CTAP2); - - // 'initialize' the device - let mut cid = [0u8; 4]; - thread_rng().fill_bytes(&mut cid); - device.set_cid(cid); + let mut device = Device::new_pre_inited( + "commands/large_blob", + Capability::CBOR | Capability::NMSG | Capability::WINK, + ); + let cid = device.get_cid().clone(); let cmd = [ 0xa2, // map(2) @@ -595,18 +590,16 @@ pub mod tests { #[test] fn test_read_large_blob_array_multi_read() { let keep_alive = || true; - let mut device = Device::new("commands/large_blobs").unwrap(); - assert_eq!(device.get_protocol(), FidoProtocol::CTAP2); + let mut device = Device::new_pre_inited( + "commands/large_blob", + Capability::CBOR | Capability::NMSG | Capability::WINK, + ); + let cid = device.get_cid().clone(); device.set_authenticator_info(crate::AuthenticatorInfo { max_msg_size: Some(164), // Note: This value minus 64 will be the fragment size ..Default::default() }); - // 'initialize' the device - let mut cid = [0u8; 4]; - thread_rng().fill_bytes(&mut cid); - device.set_cid(cid); - for ii in 0..5 { let mut cmd = vec![ 0xa2, // map(2) @@ -634,13 +627,11 @@ pub mod tests { #[test] fn test_add_large_blob_element() { let keep_alive = || true; - let mut device = Device::new("commands/large_blobs").unwrap(); - assert_eq!(device.get_protocol(), FidoProtocol::CTAP2); - - // First we read the whole existing array - let mut cid = [0u8; 4]; - thread_rng().fill_bytes(&mut cid); - device.set_cid(cid); + let mut device = Device::new_pre_inited( + "commands/large_blob", + Capability::CBOR | Capability::NMSG | Capability::WINK, + ); + let cid = device.get_cid().clone(); let cmd = [ 0xa2, // map(2) @@ -684,18 +675,17 @@ pub mod tests { #[test] fn test_add_large_blob_element_multi_write() { let keep_alive = || true; - let mut device = Device::new("commands/large_blobs").unwrap(); - assert_eq!(device.get_protocol(), FidoProtocol::CTAP2); + let mut device = Device::new_pre_inited( + "commands/large_blob", + Capability::CBOR | Capability::NMSG | Capability::WINK, + ); + let cid = device.get_cid().clone(); device.set_authenticator_info(crate::AuthenticatorInfo { max_msg_size: Some(164), // Note: This value minus 64 will be the fragment size ..Default::default() }); // First we read the whole existing array - let mut cid = [0u8; 4]; - thread_rng().fill_bytes(&mut cid); - device.set_cid(cid); - for ii in 0..5 { let mut cmd = vec![ 0xa2, // map(2) diff --git a/src/ctap2/commands/reset.rs b/src/ctap2/commands/reset.rs index 51a376ca..720c42e5 100644 --- a/src/ctap2/commands/reset.rs +++ b/src/ctap2/commands/reset.rs @@ -52,19 +52,18 @@ impl RequestCtap2 for Reset { #[cfg(test)] pub mod tests { use super::*; - use crate::consts::HIDCmd; + use crate::consts::{Capability, HIDCmd}; use crate::transport::device_selector::Device; use crate::transport::{hid::HIDDevice, FidoDevice, FidoDeviceIO, FidoProtocol}; - use rand::{thread_rng, RngCore}; use serde_cbor::{de::from_slice, Value}; fn issue_command_and_get_response(cmd: u8, add: &[u8]) -> Result<(), HIDError> { - let mut device = Device::new("commands/Reset").unwrap(); + let mut device = Device::new_pre_inited( + "commands/reset", + Capability::CBOR | Capability::NMSG | Capability::WINK, + ); + let cid = device.get_cid().clone(); assert_eq!(device.get_protocol(), FidoProtocol::CTAP2); - // ctap2 request - let mut cid = [0u8; 4]; - thread_rng().fill_bytes(&mut cid); - device.set_cid(cid); let mut msg = cid.to_vec(); msg.extend(vec![HIDCmd::Cbor.into(), 0x00, 0x1]); // cmd + bcnt diff --git a/src/ctap2/commands/selection.rs b/src/ctap2/commands/selection.rs index b1550361..53241629 100644 --- a/src/ctap2/commands/selection.rs +++ b/src/ctap2/commands/selection.rs @@ -52,20 +52,23 @@ impl RequestCtap2 for Selection { #[cfg(test)] pub mod tests { use super::*; - use crate::consts::HIDCmd; + use crate::consts::{Capability, HIDCmd}; use crate::transport::device_selector::Device; use crate::transport::{hid::HIDDevice, FidoDevice, FidoDeviceIO, FidoProtocol}; - use rand::{thread_rng, RngCore}; + use crate::CtapVersionSupport; use serde_cbor::{de::from_slice, Value}; fn issue_command_and_get_response(cmd: u8, add: &[u8]) -> Result<(), HIDError> { - let mut device = Device::new("commands/selection").unwrap(); + let mut device = Device::new_pre_inited( + "commands/get_info", + Capability::CBOR | Capability::NMSG | Capability::WINK, + ); + let cid = device.get_cid().clone(); + assert!(!device.supports_ctap1()); + assert!(device.supports_ctap2()); assert_eq!(device.get_protocol(), FidoProtocol::CTAP2); - // ctap2 request - let mut cid = [0u8; 4]; - thread_rng().fill_bytes(&mut cid); - device.set_cid(cid); + // ctap2 request let mut msg = cid.to_vec(); msg.extend(vec![HIDCmd::Cbor.into(), 0x00, 0x1]); // cmd + bcnt msg.extend(vec![0x0B]); // authenticatorSelection diff --git a/src/transport/hid.rs b/src/transport/hid.rs index a0e6d176..925de078 100644 --- a/src/transport/hid.rs +++ b/src/transport/hid.rs @@ -195,6 +195,10 @@ impl FidoDeviceIO for T { status: Option<&Sender>, ) -> Result { debug!("sending {:?} to {:?}", msg, self); + if !self.supports_ctap2() { + return Err(HIDError::UnexpectedVersion); + } + #[cfg(test)] { if self.skip_serialization() { @@ -240,6 +244,10 @@ impl FidoDeviceIO for T { status: Option<&Sender>, ) -> Result { debug!("sending {:?} to {:?}", msg, self); + if !self.supports_ctap1() { + return Err(HIDError::UnexpectedVersion); + } + #[cfg(test)] { if self.skip_serialization() { diff --git a/src/transport/mock/device.rs b/src/transport/mock/device.rs index 8ba579f2..2c2c2bc2 100644 --- a/src/transport/mock/device.rs +++ b/src/transport/mock/device.rs @@ -1,7 +1,7 @@ /* This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use crate::consts::{HIDCmd, CID_BROADCAST}; +use crate::consts::{Capability, HIDCmd, CID_BROADCAST}; use crate::crypto::SharedSecret; use crate::ctap2::commands::get_info::AuthenticatorInfo; use crate::ctap2::commands::{CtapResponse, RequestCtap1, RequestCtap2}; @@ -9,6 +9,7 @@ use crate::transport::device_selector::DeviceCommand; use crate::transport::TestDevice; use crate::transport::{hid::HIDDevice, CtapVersionSupport, FidoDevice, FidoProtocol, HIDError}; use crate::u2ftypes::{U2FDeviceInfo, U2FHIDInitResp}; +use rand::{thread_rng, RngCore as _}; use std::any::Any; use std::collections::VecDeque; use std::hash::{Hash, Hasher}; @@ -95,6 +96,41 @@ impl Device { shared_secret: None, }) } + + pub fn new_pre_inited( + parameters: ::BuildParameters, + capabilities: Capability, + ) -> Device { + let mut device = Device::new(parameters).unwrap(); + let mut cid = [0u8; 4]; + thread_rng().fill_bytes(&mut cid); + + // init + let mut msg = vec![0xFF, 0xFF, 0xFF, 0xFF]; // broadcast + msg.extend([HIDCmd::Init.into(), 0x00, 8]); // cmd + bcnt + msg.extend([0x08, 0x07, 0x06, 0x05, 0x04, 0x03, 0x02, 0x01]); // nonce + device.add_write(&msg, 0); + + let mut msg = vec![0xFF, 0xFF, 0xFF, 0xFF]; // broadcast + msg.extend([0x06, 0x00, 17]); // cmd + bcnt + msg.extend([0x08, 0x07, 0x06, 0x05, 0x04, 0x03, 0x02, 0x01]); // nonce + msg.extend(&cid); + msg.push(2); // CTAPHID protocol version identifir + msg.extend([1, 0, 0]); // Device version numbber + msg.push(capabilities.bits()); + device.add_read(&msg, 0); + + HIDDevice::pre_init(&mut device).expect("pre_init"); + assert_eq!( + capabilities.contains(Capability::NMSG), + !device.supports_ctap1() + ); + assert_eq!( + capabilities.contains(Capability::CBOR), + device.supports_ctap2() + ); + device + } } impl Write for Device { @@ -131,8 +167,8 @@ impl Read for Device { impl Drop for Device { fn drop(&mut self) { if !std::thread::panicking() { - assert!(self.reads.is_empty()); - assert!(self.writes.is_empty()); + assert!(self.reads.is_empty(), "unused reads: {:?}", self.reads); + assert!(self.writes.is_empty(), "unused writes: {:?}", self.writes); } } } From a7f1569ea62b38ccddcfbb50bb02ab6b640172e8 Mon Sep 17 00:00:00 2001 From: Michael Farrell Date: Thu, 20 Aug 2026 12:11:09 +1000 Subject: [PATCH 04/10] Make `Device::get_device_info` return an `Option`, rather than panicing --- fuzz/fuzz_targets/u2f_read.rs | 6 ++---- fuzz/fuzz_targets/u2f_read_write.rs | 6 ++---- src/ctap2/commands/get_info.rs | 2 +- src/ctap2/commands/get_version.rs | 2 +- src/transport/freebsd/device.rs | 6 ++---- src/transport/hid.rs | 8 +++++--- src/transport/linux/device.rs | 6 ++---- src/transport/macos/device.rs | 6 ++---- src/transport/mock/device.rs | 6 +++--- src/transport/mod.rs | 4 ++++ src/transport/netbsd/device.rs | 6 ++---- src/transport/openbsd/device.rs | 6 ++---- src/transport/stub/device.rs | 2 +- src/transport/windows/device.rs | 6 ++---- 14 files changed, 31 insertions(+), 41 deletions(-) diff --git a/fuzz/fuzz_targets/u2f_read.rs b/fuzz/fuzz_targets/u2f_read.rs index 8631549e..faee7d7a 100644 --- a/fuzz/fuzz_targets/u2f_read.rs +++ b/fuzz/fuzz_targets/u2f_read.rs @@ -70,10 +70,8 @@ impl<'a> U2FDevice for TestDevice<'a> { Err(io::Error::new(io::ErrorKind::Other, "Not implemented")) } - fn get_device_info(&self) -> U2FDeviceInfo { - // unwrap is okay, as dev_info must have already been set, else - // a programmer error - self.dev_info.clone().unwrap() + fn get_device_info(&self) -> Option { + self.dev_info.clone() } fn set_device_info(&mut self, dev_info: U2FDeviceInfo) { diff --git a/fuzz/fuzz_targets/u2f_read_write.rs b/fuzz/fuzz_targets/u2f_read_write.rs index c6277b04..52166268 100644 --- a/fuzz/fuzz_targets/u2f_read_write.rs +++ b/fuzz/fuzz_targets/u2f_read_write.rs @@ -71,10 +71,8 @@ impl U2FDevice for TestDevice { Err(io::Error::new(io::ErrorKind::Other, "Not implemented")) } - fn get_device_info(&self) -> U2FDeviceInfo { - // unwrap is okay, as dev_info must have already been set, else - // a programmer error - self.dev_info.clone().unwrap() + fn get_device_info(&self) -> Option { + self.dev_info.clone() } fn set_device_info(&mut self, dev_info: U2FDeviceInfo) { diff --git a/src/ctap2/commands/get_info.rs b/src/ctap2/commands/get_info.rs index 1002b7d7..652528b5 100644 --- a/src/ctap2/commands/get_info.rs +++ b/src/ctap2/commands/get_info.rs @@ -997,7 +997,7 @@ pub mod tests { assert!(!device.supports_ctap1()); assert!(device.supports_ctap2()); - let dev_info = device.get_device_info(); + let dev_info = device.get_device_info().expect("device info is set"); assert_eq!( dev_info.cap_flags, Capability::WINK | Capability::CBOR | Capability::NMSG diff --git a/src/ctap2/commands/get_version.rs b/src/ctap2/commands/get_version.rs index f975aee3..c5bc8209 100644 --- a/src/ctap2/commands/get_version.rs +++ b/src/ctap2/commands/get_version.rs @@ -74,7 +74,7 @@ pub mod tests { assert!(device.supports_ctap1()); assert!(!device.supports_ctap2()); - let dev_info = device.get_device_info(); + let dev_info = device.get_device_info().expect("device info is set"); assert_eq!(dev_info.cap_flags, Capability::WINK); let result = device.get_authenticator_info(); diff --git a/src/transport/freebsd/device.rs b/src/transport/freebsd/device.rs index 6abb0b17..107ac960 100644 --- a/src/transport/freebsd/device.rs +++ b/src/transport/freebsd/device.rs @@ -172,10 +172,8 @@ impl HIDDevice for Device { Err(io::Error::other("Not implemented")) } - fn get_device_info(&self) -> U2FDeviceInfo { - // unwrap is okay, as dev_info must have already been set, else - // a programmer error - self.dev_info.clone().unwrap() + fn get_device_info(&self) -> Option { + self.dev_info.clone() } fn set_device_info(&mut self, dev_info: U2FDeviceInfo) { diff --git a/src/transport/hid.rs b/src/transport/hid.rs index 925de078..b443978a 100644 --- a/src/transport/hid.rs +++ b/src/transport/hid.rs @@ -25,7 +25,7 @@ pub trait HIDDevice: FidoDevice + Read + Write { fn new(parameters: Self::BuildParameters) -> Result; fn id(&self) -> Self::Id; - fn get_device_info(&self) -> U2FDeviceInfo; + fn get_device_info(&self) -> Option; fn set_device_info(&mut self, dev_info: U2FDeviceInfo); // Channel ID management @@ -161,11 +161,13 @@ pub trait HIDDevice: FidoDevice + Read + Write { impl CtapVersionSupport for T { fn supports_ctap1(&self) -> bool { - !self.get_device_info().cap_flags.contains(Capability::NMSG) + self.get_device_info() + .is_some_and(|i| !i.cap_flags.contains(Capability::NMSG)) } fn supports_ctap2(&self) -> bool { - self.get_device_info().cap_flags.contains(Capability::CBOR) + self.get_device_info() + .is_some_and(|i| i.cap_flags.contains(Capability::CBOR)) } } diff --git a/src/transport/linux/device.rs b/src/transport/linux/device.rs index b5ed2404..2a24a30a 100644 --- a/src/transport/linux/device.rs +++ b/src/transport/linux/device.rs @@ -155,10 +155,8 @@ impl HIDDevice for Device { monitor::get_property_linux(&self.path, prop_name) } - fn get_device_info(&self) -> U2FDeviceInfo { - // unwrap is okay, as dev_info must have already been set, else - // a programmer error - self.dev_info.clone().unwrap() + fn get_device_info(&self) -> Option { + self.dev_info.clone() } fn set_device_info(&mut self, dev_info: U2FDeviceInfo) { diff --git a/src/transport/macos/device.rs b/src/transport/macos/device.rs index 5f1901d1..21049856 100644 --- a/src/transport/macos/device.rs +++ b/src/transport/macos/device.rs @@ -172,10 +172,8 @@ impl HIDDevice for Device { unsafe { self.get_property_macos(prop_name) } } - fn get_device_info(&self) -> U2FDeviceInfo { - // unwrap is okay, as dev_info must have already been set, else - // a programmer error - self.dev_info.clone().unwrap() + fn get_device_info(&self) -> Option { + self.dev_info.clone() } fn set_device_info(&mut self, dev_info: U2FDeviceInfo) { diff --git a/src/transport/mock/device.rs b/src/transport/mock/device.rs index 2c2c2bc2..9e5eb19a 100644 --- a/src/transport/mock/device.rs +++ b/src/transport/mock/device.rs @@ -233,8 +233,8 @@ impl HIDDevice for Device { Ok(format!("{prop_name} not implemented")) } - fn get_device_info(&self) -> U2FDeviceInfo { - self.dev_info.clone().unwrap() + fn get_device_info(&self) -> Option { + self.dev_info.clone() } fn set_device_info(&mut self, dev_info: U2FDeviceInfo) { @@ -335,7 +335,7 @@ impl FidoDevice for Device { self.sender.is_some() } - fn get_shared_secret(&self) -> std::option::Option<&SharedSecret> { + fn get_shared_secret(&self) -> Option<&SharedSecret> { self.shared_secret.as_ref() } diff --git a/src/transport/mod.rs b/src/transport/mod.rs index c155c6d5..97343989 100644 --- a/src/transport/mod.rs +++ b/src/transport/mod.rs @@ -83,9 +83,13 @@ pub enum FidoProtocol { pub trait CtapVersionSupport { /// `true` if the device supports CTAP1/U2F commands, according to the init message. + /// + /// Returns `false` if the device has not sent an init message. fn supports_ctap1(&self) -> bool; /// `true` if the device supports CTAP2 commands, according to the init message. + /// + /// Returns `false` if the device has not sent an init message. fn supports_ctap2(&self) -> bool; } diff --git a/src/transport/netbsd/device.rs b/src/transport/netbsd/device.rs index e5c07a49..5cb9c23e 100644 --- a/src/transport/netbsd/device.rs +++ b/src/transport/netbsd/device.rs @@ -174,10 +174,8 @@ impl HIDDevice for Device { Err(io::Error::other("Not implemented")) } - fn get_device_info(&self) -> U2FDeviceInfo { - // unwrap is okay, as dev_info must have already been set, else - // a programmer error - self.dev_info.clone().unwrap() + fn get_device_info(&self) -> Option { + self.dev_info.clone() } fn set_device_info(&mut self, dev_info: U2FDeviceInfo) { diff --git a/src/transport/openbsd/device.rs b/src/transport/openbsd/device.rs index fb56657f..fce494a6 100644 --- a/src/transport/openbsd/device.rs +++ b/src/transport/openbsd/device.rs @@ -155,10 +155,8 @@ impl HIDDevice for Device { Err(io::Error::other("Not implemented")) } - fn get_device_info(&self) -> U2FDeviceInfo { - // unwrap is okay, as dev_info must have already been set, else - // a programmer error - self.dev_info.clone().unwrap() + fn get_device_info(&self) -> Option { + self.dev_info.clone() } fn set_device_info(&mut self, dev_info: U2FDeviceInfo) { diff --git a/src/transport/stub/device.rs b/src/transport/stub/device.rs index 6d8ae0b0..f2dcc887 100644 --- a/src/transport/stub/device.rs +++ b/src/transport/stub/device.rs @@ -63,7 +63,7 @@ impl HIDDevice for Device { unimplemented!(); } - fn get_device_info(&self) -> U2FDeviceInfo { + fn get_device_info(&self) -> Option { unimplemented!(); } diff --git a/src/transport/windows/device.rs b/src/transport/windows/device.rs index 11620dbb..4ad61e79 100644 --- a/src/transport/windows/device.rs +++ b/src/transport/windows/device.rs @@ -111,10 +111,8 @@ impl HIDDevice for Device { Err(io::Error::other("Not implemented")) } - fn get_device_info(&self) -> U2FDeviceInfo { - // unwrap is okay, as dev_info must have already been set, else - // a programmer error - self.dev_info.clone().unwrap() + fn get_device_info(&self) -> Option { + self.dev_info.clone() } fn set_device_info(&mut self, dev_info: U2FDeviceInfo) { From 4b2ca03815375096729692e328d2168f3b9addec Mon Sep 17 00:00:00 2001 From: Michael Farrell Date: Thu, 20 Aug 2026 15:14:50 +1000 Subject: [PATCH 05/10] Fix incompatible authenticators blocking other authenticators from being used --- src/statemachine.rs | 7 +++++++ src/transport/device_selector.rs | 2 ++ 2 files changed, 9 insertions(+) diff --git a/src/statemachine.rs b/src/statemachine.rs index 1834194f..d3d951d8 100644 --- a/src/statemachine.rs +++ b/src/statemachine.rs @@ -148,21 +148,25 @@ impl StateMachine { if ctap2_only && !dev.supports_ctap2() { // CTAP1-only authenticator with a CTAP2-only request. + let _ = selector.send(DeviceSelectorEvent::NotAToken(dev.id())); return; } if args.use_ctap1_fallback && !dev.supports_ctap1() { // CTAP2-only authenticator with a CTAP1-only request. + let _ = selector.send(DeviceSelectorEvent::NotAToken(dev.id())); return; } if !Self::wait_for_device_selector(&mut dev, &selector, &status, alive) { + let _ = selector.send(DeviceSelectorEvent::NotAToken(dev.id())); return; }; if args.use_ctap1_fallback && dev.downgrade_to_ctap1().is_err() { // We shouldn't reach this, but not downgrading early lets us use CTAP 2.1 // device selection on devices that support it. + let _ = selector.send(DeviceSelectorEvent::NotAToken(dev.id())); return; } @@ -216,11 +220,13 @@ impl StateMachine { if ctap2_only && !dev.supports_ctap2() { // CTAP1-only authenticator with a CTAP2-only request. + let _ = selector.send(DeviceSelectorEvent::NotAToken(dev.id())); return; } if args.use_ctap1_fallback && !dev.supports_ctap1() { // CTAP2-only authenticator with a CTAP1-only request. + let _ = selector.send(DeviceSelectorEvent::NotAToken(dev.id())); return; } @@ -231,6 +237,7 @@ impl StateMachine { if args.use_ctap1_fallback && dev.downgrade_to_ctap1().is_err() { // We shouldn't reach this, but not downgrading early lets us use CTAP 2.1 // device selection on devices that support it. + let _ = selector.send(DeviceSelectorEvent::NotAToken(dev.id())); return; } diff --git a/src/transport/device_selector.rs b/src/transport/device_selector.rs index 4dd40d42..01b51c2d 100644 --- a/src/transport/device_selector.rs +++ b/src/transport/device_selector.rs @@ -32,6 +32,8 @@ pub enum DeviceSelectorEvent { Timeout, DevicesAdded(Vec), DeviceRemoved(DeviceID), + /// The device is not a CTAP authenticator, or it is not compatible with the request + /// (eg: CTAP2-only request with a CTAP1 authenticator). NotAToken(DeviceID), ImAToken((DeviceID, Sender)), SelectedToken(DeviceID), From dac9b4e62019bef6e99d8b4ae9b704c0329f3678 Mon Sep 17 00:00:00 2001 From: Michael Farrell Date: Wed, 30 Sep 2026 14:43:08 +1000 Subject: [PATCH 06/10] Documentation fixes: * `RequestCtap1` implementations don't seem to check the request for compatibility anymore, so don't mention that. * Link to `CtapVersionSupport` in `use_ctap1_fallback`. * Document when `Device` is a mock, to aid Rust LSPs. --- src/authenticatorservice.rs | 6 ++++-- src/ctap2/mod.rs | 4 ---- src/transport/mock/device.rs | 2 ++ 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/authenticatorservice.rs b/src/authenticatorservice.rs index ba19e843..3b6186c1 100644 --- a/src/authenticatorservice.rs +++ b/src/authenticatorservice.rs @@ -31,7 +31,8 @@ pub struct RegisterArgs { /// /// The request [must be compatible with CTAP1 authenticators][0]. /// - /// This will automatically skip any authenticator that doesn't support CTAP1. + /// When `true`, the library will automatically skip authenticators that don't + /// [support CTAP1][crate::CtapVersionSupport::supports_ctap1]. /// /// [0]: https://fidoalliance.org/specs/fido-v2.1-ps-20210615/fido-client-to-authenticator-protocol-v2.1-ps-errata-20220621.html#u2f-authenticatorMakeCredential-interoperability pub use_ctap1_fallback: bool, @@ -52,7 +53,8 @@ pub struct SignArgs { /// /// The request [must be compatible with CTAP1 authenticators][0]. /// - /// This will automatically skip any authenticator that doesn't support CTAP1. + /// When `true`, the library will automatically skip authenticators that don't + /// [support CTAP1][crate::CtapVersionSupport::supports_ctap1]. /// /// [0]: https://fidoalliance.org/specs/fido-v2.1-ps-20210615/fido-client-to-authenticator-protocol-v2.1-ps-errata-20220621.html#u2f-authenticatorGetAssertion-interoperability pub use_ctap1_fallback: bool, diff --git a/src/ctap2/mod.rs b/src/ctap2/mod.rs index 406446fa..277626eb 100644 --- a/src/ctap2/mod.rs +++ b/src/ctap2/mod.rs @@ -417,8 +417,6 @@ fn determine_puap_if_needed /// /// See [CTAP 2.1 §10.2][0]. /// -/// Some additional checks are performed in `MakeCredentials::RequestCtap1`. -/// /// [0]: https://fidoalliance.org/specs/fido-v2.1-ps-20210615/fido-client-to-authenticator-protocol-v2.1-ps-errata-20220621.html#u2f-authenticatorMakeCredential-interoperability pub(crate) fn check_ctap1_register_compatibility(args: &RegisterArgs) -> crate::Result<()> { if args.resident_key_req == ResidentKeyRequirement::Required { @@ -608,8 +606,6 @@ pub fn register( /// /// See [CTAP 2.1 §10.3][0]. /// -/// Some additional checks are performed in `GetAssertion::RequestCtap1`. -/// /// [0]: https://fidoalliance.org/specs/fido-v2.1-ps-20210615/fido-client-to-authenticator-protocol-v2.1-ps-errata-20220621.html#u2f-authenticatorGetAssertion-interoperability pub(crate) fn check_ctap1_sign_compatibility(args: &SignArgs) -> crate::Result<()> { if args.user_verification_req == UserVerificationRequirement::Required { diff --git a/src/transport/mock/device.rs b/src/transport/mock/device.rs index 9e5eb19a..e890cbf1 100644 --- a/src/transport/mock/device.rs +++ b/src/transport/mock/device.rs @@ -19,6 +19,7 @@ use std::sync::mpsc::{channel, Receiver, Sender}; pub(crate) const IN_HID_RPT_SIZE: usize = 64; pub(crate) const OUT_HID_RPT_SIZE: usize = 64; +/// Mock authenticator device for unit tests. #[derive(Debug)] pub struct Device { pub id: String, @@ -97,6 +98,7 @@ impl Device { }) } + /// Create a new, pre-initialized mock authenticator. pub fn new_pre_inited( parameters: ::BuildParameters, capabilities: Capability, From 5137d68ebcdfd724da8866441cdf873986568748 Mon Sep 17 00:00:00 2001 From: Michael Farrell Date: Wed, 30 Sep 2026 15:46:01 +1000 Subject: [PATCH 07/10] fix clippy lints --- src/ctap2/commands/get_assertion.rs | 2 +- src/ctap2/commands/get_info.rs | 2 +- src/ctap2/commands/large_blobs.rs | 10 +++++----- src/ctap2/commands/reset.rs | 2 +- src/ctap2/commands/selection.rs | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/ctap2/commands/get_assertion.rs b/src/ctap2/commands/get_assertion.rs index 3e951777..0c8f9a9f 100644 --- a/src/ctap2/commands/get_assertion.rs +++ b/src/ctap2/commands/get_assertion.rs @@ -970,7 +970,7 @@ pub mod test { "commands/get_info", Capability::CBOR | Capability::NMSG | Capability::WINK, ); - let cid = device.get_cid().clone(); + let cid = *device.get_cid(); let mut msg = cid.to_vec(); msg.extend(vec![HIDCmd::Cbor.into(), 0x00, 0x90]); diff --git a/src/ctap2/commands/get_info.rs b/src/ctap2/commands/get_info.rs index 652528b5..e873069e 100644 --- a/src/ctap2/commands/get_info.rs +++ b/src/ctap2/commands/get_info.rs @@ -961,7 +961,7 @@ pub mod tests { "commands/get_info", Capability::CBOR | Capability::NMSG | Capability::WINK, ); - let cid = device.get_cid().clone(); + let cid = *device.get_cid(); // ctap2 request let mut msg = cid.to_vec(); diff --git a/src/ctap2/commands/large_blobs.rs b/src/ctap2/commands/large_blobs.rs index 43f8bd26..de264b0f 100644 --- a/src/ctap2/commands/large_blobs.rs +++ b/src/ctap2/commands/large_blobs.rs @@ -525,7 +525,7 @@ pub mod tests { "commands/large_blob", Capability::CBOR | Capability::NMSG | Capability::WINK, ); - let cid = device.get_cid().clone(); + let cid = *device.get_cid(); let cmd = [ 0xa2, // map(2) @@ -554,7 +554,7 @@ pub mod tests { "commands/large_blob", Capability::CBOR | Capability::NMSG | Capability::WINK, ); - let cid = device.get_cid().clone(); + let cid = *device.get_cid(); let cmd = [ 0xa2, // map(2) @@ -594,7 +594,7 @@ pub mod tests { "commands/large_blob", Capability::CBOR | Capability::NMSG | Capability::WINK, ); - let cid = device.get_cid().clone(); + let cid = *device.get_cid(); device.set_authenticator_info(crate::AuthenticatorInfo { max_msg_size: Some(164), // Note: This value minus 64 will be the fragment size ..Default::default() @@ -631,7 +631,7 @@ pub mod tests { "commands/large_blob", Capability::CBOR | Capability::NMSG | Capability::WINK, ); - let cid = device.get_cid().clone(); + let cid = *device.get_cid(); let cmd = [ 0xa2, // map(2) @@ -679,7 +679,7 @@ pub mod tests { "commands/large_blob", Capability::CBOR | Capability::NMSG | Capability::WINK, ); - let cid = device.get_cid().clone(); + let cid = *device.get_cid(); device.set_authenticator_info(crate::AuthenticatorInfo { max_msg_size: Some(164), // Note: This value minus 64 will be the fragment size ..Default::default() diff --git a/src/ctap2/commands/reset.rs b/src/ctap2/commands/reset.rs index 720c42e5..3cfc3762 100644 --- a/src/ctap2/commands/reset.rs +++ b/src/ctap2/commands/reset.rs @@ -62,7 +62,7 @@ pub mod tests { "commands/reset", Capability::CBOR | Capability::NMSG | Capability::WINK, ); - let cid = device.get_cid().clone(); + let cid = *device.get_cid(); assert_eq!(device.get_protocol(), FidoProtocol::CTAP2); let mut msg = cid.to_vec(); diff --git a/src/ctap2/commands/selection.rs b/src/ctap2/commands/selection.rs index 53241629..34b407f0 100644 --- a/src/ctap2/commands/selection.rs +++ b/src/ctap2/commands/selection.rs @@ -63,7 +63,7 @@ pub mod tests { "commands/get_info", Capability::CBOR | Capability::NMSG | Capability::WINK, ); - let cid = device.get_cid().clone(); + let cid = *device.get_cid(); assert!(!device.supports_ctap1()); assert!(device.supports_ctap2()); assert_eq!(device.get_protocol(), FidoProtocol::CTAP2); From 425823a166e37f9c01dc5f8af0520cd7fc0a4621 Mon Sep 17 00:00:00 2001 From: Michael Farrell Date: Thu, 1 Oct 2026 17:56:36 +1000 Subject: [PATCH 08/10] Gate availability of `StateMachine::reset`, `manage` and `set_pin` on supporting CTAP2, rather than it being the active version --- src/statemachine.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/statemachine.rs b/src/statemachine.rs index d3d951d8..c00c7eb3 100644 --- a/src/statemachine.rs +++ b/src/statemachine.rs @@ -12,7 +12,7 @@ use crate::transport::device_selector::{ BlinkResult, Device, DeviceBuildParameters, DeviceCommand, DeviceSelectorEvent, }; use crate::transport::platform::transaction::Transaction; -use crate::transport::{hid::HIDDevice, CtapVersionSupport, FidoDevice, FidoProtocol}; +use crate::transport::{hid::HIDDevice, CtapVersionSupport, FidoDevice}; use crate::{InteractiveRequest, ManageResult}; use std::sync::mpsc::{channel, RecvTimeoutError, Sender}; use std::time::Duration; @@ -282,7 +282,7 @@ impl StateMachine { None => return, }; - if dev.get_protocol() != FidoProtocol::CTAP2 { + if !dev.supports_ctap2() { info!("Device does not support CTAP2"); let _ = selector.send(DeviceSelectorEvent::NotAToken(dev.id())); return; @@ -320,7 +320,7 @@ impl StateMachine { None => return, }; - if dev.get_protocol() != FidoProtocol::CTAP2 { + if !dev.supports_ctap2() { info!("Device does not support CTAP2"); let _ = selector.send(DeviceSelectorEvent::NotAToken(dev.id())); return; @@ -371,8 +371,11 @@ impl StateMachine { None => return, }; - if dev.get_protocol() != FidoProtocol::CTAP2 { - info!("Device does not support CTAP2"); + if !dev.supports_ctap2() { + info!( + "Device {:?} cannot be managed because it does not support CTAP2", + dev.id() + ); let _ = selector.send(DeviceSelectorEvent::NotAToken(dev.id())); return; } From 3a7cc04c5eb5df23a136cd86da66e20fc9f333ea Mon Sep 17 00:00:00 2001 From: Michael Farrell Date: Thu, 1 Oct 2026 17:56:57 +1000 Subject: [PATCH 09/10] note that `NoDevicesFound` can mean a device is incompatible with a request --- src/status_update.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/status_update.rs b/src/status_update.rs index 822011a8..d993d5ed 100644 --- a/src/status_update.rs +++ b/src/status_update.rs @@ -124,7 +124,7 @@ pub enum StatusUpdate { /// After MakeCredential, supply the user with the large blob key and let /// them calculate the payload, to send back to us. LargeBlobData(Sender, Vec), - /// Inform user that no devices are plugged in + /// There are no connected devices which are compatible with the request NoDevicesFound, /// Logging of requests being sent to the device RequestLogging(MessageDirection, String), From 1da10a3804a7e810555e2c04261e989b9cfb2639 Mon Sep 17 00:00:00 2001 From: Michael Farrell Date: Thu, 1 Oct 2026 17:58:55 +1000 Subject: [PATCH 10/10] Block CTAP1-only authenticators from being used in more places without requiring a hardware call --- src/ctap2/mod.rs | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/ctap2/mod.rs b/src/ctap2/mod.rs index 277626eb..c8232e08 100644 --- a/src/ctap2/mod.rs +++ b/src/ctap2/mod.rs @@ -43,7 +43,7 @@ use crate::statecallback::StateCallback; use crate::status_update::{send_status, BioEnrollmentCmd, CredManagementCmd, InteractiveUpdate}; use crate::transport::device_selector::{Device, DeviceSelectorEvent}; use crate::transport::{errors::HIDError, hid::HIDDevice, FidoDevice, FidoDeviceIO, FidoProtocol}; -use crate::{ManageResult, ResetResult, StatusPinUv, StatusUpdate}; +use crate::{CtapVersionSupport, ManageResult, ResetResult, StatusPinUv, StatusUpdate}; use std::sync::mpsc::{channel, RecvError, Sender}; use std::thread; use std::time::Duration; @@ -806,6 +806,11 @@ pub(crate) fn reset_helper>( callback: StateCallback>, keep_alive: &dyn Fn() -> bool, ) { + if !dev.supports_ctap2() { + callback.call(Err(HIDError::UnsupportedCommand.into())); + return; + } + let reset = Reset {}; info!("Device {:?} continues with the reset process", dev.id()); @@ -841,6 +846,11 @@ pub fn set_or_change_pin_helper, Dev: FidoDevice>( callback: StateCallback>, alive: &dyn Fn() -> bool, ) { + if !dev.supports_ctap2() { + callback.call(Err(HIDError::UnsupportedCommand.into())); + return; + } + let mut shared_secret = match dev.establish_shared_secret(alive) { Ok(s) => s, Err(e) => { @@ -938,6 +948,11 @@ pub(crate) fn bio_enrollment( callback: StateCallback>, alive: &dyn Fn() -> bool, ) -> bool { + if !dev.supports_ctap2() { + callback.call(Err(HIDError::UnsupportedCommand.into())); + return false; + } + let authinfo = match dev.get_authenticator_info() { Some(i) => i, None => { @@ -1206,6 +1221,11 @@ pub fn credential_management( callback: StateCallback>, alive: &dyn Fn() -> bool, ) -> bool { + if !dev.supports_ctap2() { + callback.call(Err(HIDError::UnsupportedCommand.into())); + return false; + } + let mut skip_uv = false; let authinfo = match dev.get_authenticator_info() { Some(i) => i.clone(), @@ -1520,6 +1540,11 @@ pub(crate) fn configure_authenticator( callback: StateCallback>, alive: &dyn Fn() -> bool, ) -> bool { + if !dev.supports_ctap2() { + callback.call(Err(HIDError::UnsupportedCommand.into())); + return false; + } + let mut authcfg = AuthenticatorConfig::new(cfg_subcommand); let mut skip_uv = false; let authinfo = match dev.get_authenticator_info() {