diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b5d255..68548ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased - Update to `ctap-types` v0.6.0-rc.2. -- Set `algorithms`, `firmware_version` and `remaining_discoverable_credentials` in `get_info` and add `firmware_version` to `Config`. +- Set `algorithms`, `authenticator_config_commands`, `firmware_version`, `max_serialized_large_blob_array` and `remaining_discoverable_credentials` in `get_info` and add `firmware_version` to `Config`. - Implement these new extensions: - `credBlob` - `hmac-secret-mc` @@ -16,8 +16,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Implement the `config` command with these subcommands: - `toggleAlwaysUv` - `setMinPINLength` +- Add `ccid_transport` to `Config` and set `transports` in `get_info` accordingly. +- Indicate support for `FIDO_2_3` in `get_info`. - Load full credential from filesstem for getAssertion if an allow list is used with a discoverable credential. - Use UTF-8 code points instead of bytes when checking the minimum length for PINs. +- Accept `up = true` in makeCredential. +- Fix PIN verification in `large_blobs_set`. ## [v0.3.0](https://github.com/trussed-dev/fido-authenticator/releases/tag/v0.3.0) (2026-03-25) diff --git a/Cargo.toml b/Cargo.toml index 4979073..f508f7e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,7 +12,7 @@ description = "FIDO authenticator Trussed app" apdu-app = { version = "0.2", optional = true } cbor-smol = "0.5" cosey = "0.4" -ctap-types = { version = "=0.6.0-rc.2", features = ["get-info-full", "large-blobs", "third-party-payment"] } +ctap-types = { version = "=0.6.0-rc.3", features = ["get-info-full", "large-blobs", "third-party-payment"] } ctaphid-app = { version = "0.2", optional = true } delog = "0.1" heapless = "0.9" diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index 7c822a0..2c86f52 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -8,7 +8,7 @@ edition = "2021" cargo-fuzz = true [dependencies] -ctap-types = { version = "=0.6.0-rc.2", features = ["arbitrary"] } +ctap-types = { version = "=0.6.0-rc.3", features = ["arbitrary"] } libfuzzer-sys = "0.4" trussed = { version = "0.1", features = ["certificate-client", "crypto-client", "filesystem-client", "management-client", "aes256-cbc", "ed255", "p256", "sha256"] } trussed-staging = { version = "0.4.0", features = ["chunked", "hkdf", "virt", "fs-info"] } diff --git a/fuzz/fuzz_targets/ctap.rs b/fuzz/fuzz_targets/ctap.rs index c937193..2b9b045 100644 --- a/fuzz/fuzz_targets/ctap.rs +++ b/fuzz/fuzz_targets/ctap.rs @@ -18,6 +18,7 @@ fuzz_target!(|requests: Vec>| { max_resident_credential_count: None, large_blobs: None, nfc_transport: false, + ccid_transport: false, firmware_version: Some(0), }, ); diff --git a/src/ctap2.rs b/src/ctap2.rs index 018fd1d..f2c89b4 100644 --- a/src/ctap2.rs +++ b/src/ctap2.rs @@ -67,6 +67,7 @@ impl Authenticator for crate::Authenti // and MUST not be present in versions member." CTAP 2.2 was an // addendum; 2.2-level features (e.g. hmac-secret-mc) are still // discoverable via the extensions list. + versions.push(Version::Fido2_3).unwrap(); let mut extensions = Vec::new(); extensions.push(Extension::CredProtect).unwrap(); @@ -110,15 +111,18 @@ impl Authenticator for crate::Authenti if self.config.nfc_transport { transports.push(Transport::Nfc).unwrap(); } + if self.config.ccid_transport { + transports.push(Transport::SmartCard).unwrap(); + } transports.push(Transport::Usb).unwrap(); let mut attestation_formats = Vec::new(); + // CTAP 2.1 §6.4: "none" is implied and MUST NOT appear in + // `authenticatorGetInfo.attestationFormats`. We still honour it + // internally when requested via `attestationFormatsPreference`. attestation_formats .push(AttestationStatementFormat::Packed) .unwrap(); - attestation_formats - .push(AttestationStatementFormat::None) - .unwrap(); let (_, aaguid) = self.state.identity.attestation(&mut self.trussed); @@ -149,10 +153,21 @@ impl Authenticator for crate::Authenti response.remaining_discoverable_credentials = remaining_discoverable_credentials.map(|count| count as usize); response.max_cred_blob_length = Some(MAX_CRED_BLOB_LENGTH); + // CTAP 2.1 §6.4 0x0B: required when largeBlobs is supported. + if let Some(cfg) = self.config.large_blobs.as_ref() { + response.max_serialized_large_blob_array = Some(cfg.max_size()); + } response.min_pin_length = Some(self.state.persistent.min_pin_length()); response.force_pin_change = Some(self.state.persistent.force_pin_change()); response.max_rpids_for_set_min_pin_length = Some(MAX_MIN_PIN_LENGTH_RP_IDS); response.attestation_formats = Some(attestation_formats); + // CTAP 2.3 §6.4 0x1F: supported authenticatorConfig sub-command IDs. + // 0x02 toggleAlwaysUv (CTAP 2.1 §6.11.2) + // 0x03 setMinPINLength (CTAP 2.1 §6.11.3) + let mut cfg_cmds = Vec::new(); + cfg_cmds.push(0x02).unwrap(); + cfg_cmds.push(0x03).unwrap(); + response.authenticator_config_commands = Some(cfg_cmds); response } @@ -199,8 +214,9 @@ impl Authenticator for crate::Authenti // 1-4. if let Some(options) = parameters.options.as_ref() { - // up option is not valid for make_credential - if options.up.is_some() { + // CTAP 2.1 §6.1.2: MakeCredential allows `up` only with value + // true (UP is implicit and required); `up=false` is invalid. + if options.up == Some(false) { return Err(Error::InvalidOption); } } @@ -2412,15 +2428,10 @@ impl crate::Authenticator { let Some(pin_uv_auth_protocol) = request.pin_uv_auth_protocol else { return Err(Error::PinRequired); }; - if pin_uv_auth_protocol != 1 { - return Err(Error::PinAuthInvalid); - } let pin_protocol = self.parse_pin_protocol(pin_uv_auth_protocol)?; - // TODO: check pinUvAuthToken - let pin_auth: [u8; 16] = pin_uv_auth_param - .as_ref() - .try_into() - .map_err(|_| Error::PinAuthInvalid)?; + // verify_pin_token truncates per protocol (16 B for v1, 32 B + // for v2), so pass the full param. + let pin_auth: &[u8] = pin_uv_auth_param.as_ref(); let mut auth_data: Bytes<70> = Bytes::new(); // 32x 0xff @@ -2436,7 +2447,7 @@ impl crate::Authenticator { auth_data.extend_from_slice(&Sha256::digest(data)).unwrap(); let mut pin_protocol = self.pin_protocol(pin_protocol); - let pin_token = pin_protocol.verify_pin_token(&pin_auth, &auth_data)?; + let pin_token = pin_protocol.verify_pin_token(&auth_data, pin_auth)?; pin_token.require_permissions(Permissions::LARGE_BLOB_WRITE)?; } diff --git a/src/lib.rs b/src/lib.rs index 86605af..03f400b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -126,6 +126,10 @@ pub struct Config { pub large_blobs: Option, /// Whether the authenticator supports the NFC transport. pub nfc_transport: bool, + /// Whether the authenticator exposes FIDO over a CCID smart-card interface + /// (CTAP 2.3 §3 FIDO Interfaces). When `true`, GetInfo advertises the + /// `"smart-card"` transport alongside `"usb"` / `"nfc"`. + pub ccid_transport: bool, /// Firmware version reported by `authenticatorGetInfo` (CTAP 2.1 §6.4 0x0E). /// /// The runner is expected to plumb its own version constant in here. diff --git a/src/state.rs b/src/state.rs index 3223000..7ce6493 100644 --- a/src/state.rs +++ b/src/state.rs @@ -360,11 +360,20 @@ impl PersistentState { self.consecutive_pin_mismatches = 0; self.pin_hash = None; self.timestamp = 0; - // CTAP 2.1 §6.7 authenticatorReset: "Always Require User Verification" - // is explicitly listed as a feature that MUST be reset. Other §6.7 - // feature flags (min_pin_length / min_pin_length_rp_ids / - // force_pin_change) are left for a follow-up — see AUDIT.md. + // CTAP 2.1 §6.7 authenticatorReset MUST reset the following features: + // - "Always Require User Verification" (alwaysUv) + // - "Set Minimum PIN Length" — its three pieces of state: + // * `minPINLength` → back to default (the `Default` impl + // leaves the raw field at 0, which our `min_pin_length()` + // getter reads as DEFAULT_MIN_PIN_LENGTH) + // * `minPinLengthRPIDs` → empty + // * `forcePINChange` → false + // - Enterprise attestation (we don't support it, so no state to + // clear). self.always_uv = false; + self.min_pin_length = 0; + self.min_pin_length_rp_ids = heapless::Vec::new(); + self.force_pin_change = false; self.save(trussed) } @@ -638,6 +647,21 @@ impl RuntimeState { self.clear_credential_cache(); self.active_get_assertion = None; + // Clear any in-flight credMgmt enumeration cursors. Otherwise a + // `next_relying_party`/`next_credential` call straight after + // `authenticatorReset` succeeds with stale data instead of + // returning `NotAllowed` (caught by fido2-tests + // test_rpnext_without_rpbegin). + self.cached_rp = None; + self.cached_rk = None; + + // The per-power-cycle pinAuthFailedAttempts counter is runtime + // state and lives here. `authenticatorReset` removes the PIN + // entirely (caller resets the persistent retries counter via + // `PersistentState::reset`), so the per-power-cycle counter + // should drop with it. + self.consecutive_pin_mismatches = 0; + if let Some(pin_protocol) = self.pin_protocol.take() { pin_protocol.reset(trussed); } diff --git a/tests/basic.rs b/tests/basic.rs index eb8eee8..31ba50d 100644 --- a/tests/basic.rs +++ b/tests/basic.rs @@ -49,10 +49,7 @@ fn test_get_info() { &hex!("8BC5496807B14D5FB249607F5D527DA2") ); assert_eq!(reply.pin_protocols, Some(vec![2, 1])); - assert_eq!( - reply.attestation_formats, - Some(vec!["packed".to_owned(), "none".to_owned()]) - ); + assert_eq!(reply.attestation_formats, Some(vec!["packed".to_owned()])); }); } @@ -574,8 +571,7 @@ struct TestMakeCredential { impl TestMakeCredential { fn expected_error(&self) -> Option { if let Some(options) = self.options { - // TODO: this is the current implementation, but the spec allows Some(true) - if options.up.is_some() { + if options.up == Some(false) { return Some(0x2c); } if !matches!(self.pin_auth, PinAuth::PinToken(_)) && options.uv == Some(true) { @@ -2669,3 +2665,106 @@ fn test_make_credential_hmac_secret_mc_invalid_salt_length_rejected() { assert_eq!(result.err(), Some(Ctap2Error(0x03))); }) } + +// ---------------------------------------------------------------------------- +// Transports (CTAP 2.1 §6.4 0x09 / CTAP 2.3 §3 smart-card) +// ---------------------------------------------------------------------------- + +/// Default config (USB only) advertises only `"usb"`. +#[test] +fn test_transports_usb_only_by_default() { + virt::run_ctap2(|device| { + let reply = device.exec(GetInfo).unwrap(); + let transports = reply.transports.expect("transports list missing"); + assert_eq!(transports, vec!["usb".to_owned()]); + }) +} + +/// With `nfc_transport=true`, `"nfc"` and `"usb"` are advertised. +#[test] +fn test_transports_nfc_added() { + let options = Options { + nfc_transport: true, + ..Default::default() + }; + virt::run_ctap2_with_options(options, |device| { + let reply = device.exec(GetInfo).unwrap(); + let transports = reply.transports.expect("transports list missing"); + assert!(transports.contains(&"nfc".to_owned())); + assert!(transports.contains(&"usb".to_owned())); + }) +} + +/// CTAP 2.3 §3: with `ccid_transport=true`, `"smart-card"` is advertised +/// alongside the other transports. +#[test] +fn test_transports_smart_card_advertised_when_ccid_enabled() { + let options = Options { + ccid_transport: true, + ..Default::default() + }; + virt::run_ctap2_with_options(options, |device| { + let reply = device.exec(GetInfo).unwrap(); + let transports = reply.transports.expect("transports list missing"); + assert!( + transports.contains(&"smart-card".to_owned()), + "smart-card missing from transports: {:?}", + transports + ); + }) +} + +/// `"smart-card"` is NOT advertised by default (no CCID). +#[test] +fn test_transports_smart_card_absent_when_ccid_disabled() { + virt::run_ctap2(|device| { + let reply = device.exec(GetInfo).unwrap(); + let transports = reply.transports.expect("transports list missing"); + assert!(!transports.contains(&"smart-card".to_owned())); + }) +} + +/// NFC + CCID together: all three transports advertised. Verifies the +/// flags are independent. +#[test] +fn test_transports_nfc_and_smart_card_combined() { + let options = Options { + nfc_transport: true, + ccid_transport: true, + ..Default::default() + }; + virt::run_ctap2_with_options(options, |device| { + let reply = device.exec(GetInfo).unwrap(); + let transports = reply.transports.expect("transports list missing"); + assert!( + transports.contains(&"nfc".to_owned()), + "nfc missing: {:?}", + transports + ); + assert!( + transports.contains(&"smart-card".to_owned()), + "smart-card missing: {:?}", + transports + ); + assert!( + transports.contains(&"usb".to_owned()), + "usb missing: {:?}", + transports + ); + }) +} + +// ---------------------------------------------------------------------------- +// FIDO_2_3 version advertisement (CTAP 2.3 §6.4) +// ---------------------------------------------------------------------------- + +/// CTAP 2.3 §6.4: `FIDO_2_3` is advertised in the versions list; `FIDO_2_2` +/// MUST be absent. +#[test] +fn test_versions_include_fido_2_3_exclude_fido_2_2() { + virt::run_ctap2(|device| { + let reply = device.exec(GetInfo).unwrap(); + assert!(reply.versions.contains(&"FIDO_2_3".to_owned())); + assert!(!reply.versions.contains(&"FIDO_2_2".to_owned())); + }) +} diff --git a/tests/virt/mod.rs b/tests/virt/mod.rs index 586d7b6..71f54c5 100644 --- a/tests/virt/mod.rs +++ b/tests/virt/mod.rs @@ -73,7 +73,8 @@ where skip_up_timeout: None, max_resident_credential_count: options.max_resident_credential_count, large_blobs: None, - nfc_transport: false, + nfc_transport: options.nfc_transport, + ccid_transport: options.ccid_transport, firmware_version: Some(0), }, ); @@ -134,6 +135,8 @@ pub type InspectFsFn = Box; pub struct Options { pub files: Vec<(PathBuf, Vec)>, pub max_resident_credential_count: Option, + pub nfc_transport: bool, + pub ccid_transport: bool, pub inspect_ifs: Option, } diff --git a/tests/webauthn/mod.rs b/tests/webauthn/mod.rs index 635a12d..15fe884 100644 --- a/tests/webauthn/mod.rs +++ b/tests/webauthn/mod.rs @@ -977,6 +977,7 @@ pub struct GetInfoReply { pub aaguid: Value, pub options: Option>, pub pin_protocols: Option>, + pub transports: Option>, pub force_pin_change: Option, pub min_pin_length: Option, pub attestation_formats: Option>, @@ -992,6 +993,8 @@ impl From for GetInfoReply { aaguid: map.remove(&3).unwrap().deserialized().unwrap(), options: map.remove(&4).map(|value| value.deserialized().unwrap()), pin_protocols: map.remove(&6).map(|value| value.deserialized().unwrap()), + // 0x09: transports (CTAP 2.1) + transports: map.remove(&9).map(|value| value.deserialized().unwrap()), // 0x0C: forcePINChange (CTAP 2.1) force_pin_change: map.remove(&0x0C).map(|value| value.deserialized().unwrap()), // 0x0D: minPINLength (CTAP 2.1)