diff --git a/CHANGELOG.md b/CHANGELOG.md index 31024ee..4a2aade 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ - Removed: `UserValidationMethod::check_user_verification` - Added: `UserValidationMethod::check_user`. This function now performs both user presence and user verification checks. The function now also returns which validations were performed, even if they were not requested. +- Added: Support for discoverable credentials + - ⚠ BREAKING: Added: `CredentialStore::get_info` which returns `StoreInfo` containing `DiscoverabilitySupport`. + - ⚠ BREAKING: Changed: `CredentialStore::save_credential` now also takes `Options`. + - Changed: `Authenticator::make_credentials` now returns an error if a discoverable credential was requested but not supported by the store. ### passkey-client @@ -35,6 +39,7 @@ handles client data and its hash. - `CollectedClientData` is now generic and supports additional strongly typed fields. ([#28](https://github.com/1Password/passkey-rs/pull/28)) - Changed: `CollectedClientData` has changed to `CollectedClientData` +- The `Client` now returns `CredProps::rk` depending on the authenticator's capabilities. ([#29](https://github.com/1Password/passkey-rs/pull/29)) - ⚠ BREAKING: Rename webauthn extension outputs to be consistent with inputs. ([#33](https://github.com/1Password/passkey-rs/pull/33)) - ⚠ BREAKING: Create new extension inputs for the CTAP authenticator inputs. ([#33](https://github.com/1Password/passkey-rs/pull/33)) diff --git a/passkey-authenticator/Cargo.toml b/passkey-authenticator/Cargo.toml index b78477f..2a09e61 100644 --- a/passkey-authenticator/Cargo.toml +++ b/passkey-authenticator/Cargo.toml @@ -31,6 +31,9 @@ tokio = { version = "1", features = ["sync"], optional = true } [dev-dependencies] mockall = { version = "0.11" } +passkey-types = { path = "../passkey-types", version = "0.2", features = [ + "testable", +] } tokio = { version = "1", features = ["sync", "macros", "rt"] } generic-array = { version = "0.14", default-features = false } signature = { version = "2", features = ["rand_core"] } diff --git a/passkey-authenticator/src/authenticator/get_assertion.rs b/passkey-authenticator/src/authenticator/get_assertion.rs index 3109e85..fefcf6a 100644 --- a/passkey-authenticator/src/authenticator/get_assertion.rs +++ b/passkey-authenticator/src/authenticator/get_assertion.rs @@ -63,6 +63,12 @@ where // Note that because this specification defines normative behaviors for them, all // authenticators MUST understand the "rk", "up", and "uv" options. + // 4. If the "rk" option is present then: + // 1. Return CTAP2_ERR_UNSUPPORTED_OPTION. + if input.options.rk { + return Err(Ctap2Error::UnsupportedOption.into()); + } + // 6. TODO, if the extensions parameter is present, process any extensions that this // authenticator supports. Authenticator extension outputs generated by the authenticator // extension processing are returned in the authenticator data. @@ -187,7 +193,7 @@ mod tests { options: Options { up: true, uv: true, - rk: true, + rk: false, }, } } diff --git a/passkey-authenticator/src/authenticator/get_info.rs b/passkey-authenticator/src/authenticator/get_info.rs index 1059a14..4229cb1 100644 --- a/passkey-authenticator/src/authenticator/get_info.rs +++ b/passkey-authenticator/src/authenticator/get_info.rs @@ -1,17 +1,20 @@ use passkey_types::ctap2::get_info::{Options, Response}; -use crate::{Authenticator, CredentialStore, UserValidationMethod}; +use crate::{ + credential_store::DiscoverabilitySupport, Authenticator, CredentialStore, UserValidationMethod, +}; impl Authenticator { /// Using this method, the host can request that the authenticator report a list of all /// supported protocol versions, supported extensions, AAGUID of the device, and its capabilities. - pub fn get_info(&self) -> Response { + pub async fn get_info(&self) -> Response { Response { versions: vec!["FIDO_2_0".into(), "U2F_V2".into()], extensions: None, aaguid: *self.aaguid(), options: Some(Options { - rk: true, + rk: self.store.get_info().await.discoverability + != DiscoverabilitySupport::OnlyNonDiscoverable, uv: self.user_validation.is_verification_enabled(), up: self.user_validation.is_presence_enabled(), ..Default::default() diff --git a/passkey-authenticator/src/authenticator/make_credential.rs b/passkey-authenticator/src/authenticator/make_credential.rs index 6ccf8a8..c45beae 100644 --- a/passkey-authenticator/src/authenticator/make_credential.rs +++ b/passkey-authenticator/src/authenticator/make_credential.rs @@ -55,7 +55,13 @@ where // return CTAP2_ERR_INVALID_OPTION. Ignore any options that are not understood. // Note that because this specification defines normative behaviors for them, all // authenticators MUST understand the "rk", "up", and "uv" options. - // NB: this is handled at the very begining of the method + // NOTE: Some of this step is handled at the very begining of the method + + // 4. If the "rk" option is present then: + // 1. If the rk option ID is not present in authenticatorGetInfo response, end the operation by returning CTAP2_ERR_UNSUPPORTED_OPTION. + if input.options.rk && !self.get_info().await.options.unwrap_or_default().rk { + return Err(Ctap2Error::UnsupportedOption.into()); + } // 4. TODO, if the extensions parameter is present, process any extensions that this // authenticator supports. Authenticator extension outputs generated by the authenticator @@ -141,7 +147,7 @@ where // 10 self.store_mut() - .save_credential(passkey, input.user.into(), input.rp) + .save_credential(passkey, input.user.into(), input.rp, input.options) .await?; Ok(response) @@ -154,8 +160,12 @@ mod tests { use coset::iana; use passkey_types::{ - ctap2::make_credential::{Options, PublicKeyCredentialRpEntity}, - ctap2::Aaguid, + ctap2::{ + make_credential::{ + Options, PublicKeyCredentialRpEntity, PublicKeyCredentialUserEntity, + }, + Aaguid, + }, rand::random_vec, webauthn, Bytes, }; @@ -163,7 +173,11 @@ mod tests { use tokio::sync::Mutex; use super::*; - use crate::{user_validation::MockUserValidationMethod, MemoryStore}; + use crate::{ + credential_store::{DiscoverabilitySupport, StoreInfo}, + user_validation::MockUserValidationMethod, + MemoryStore, + }; fn good_request() -> Request { Request { @@ -288,4 +302,60 @@ mod tests { let store = shared_store.lock().await; assert_eq!(store.as_ref().and_then(|c| c.counter).unwrap(), 0); } + + #[tokio::test] + async fn make_credential_returns_err_when_rk_is_requested_but_not_supported() { + struct StoreWithoutDiscoverableSupport; + #[async_trait::async_trait] + impl CredentialStore for StoreWithoutDiscoverableSupport { + type PasskeyItem = Passkey; + + async fn find_credentials( + &self, + _id: Option<&[webauthn::PublicKeyCredentialDescriptor]>, + _rp_id: &str, + ) -> Result, StatusCode> { + #![allow(clippy::unimplemented)] + unimplemented!("The test should not call find_credentials") + } + + async fn save_credential( + &mut self, + _cred: Passkey, + _user: PublicKeyCredentialUserEntity, + _rp: PublicKeyCredentialRpEntity, + _options: Options, + ) -> Result<(), StatusCode> { + #![allow(clippy::unimplemented)] + unimplemented!("The test should not call save_credential") + } + + async fn update_credential(&mut self, _cred: Passkey) -> Result<(), StatusCode> { + #![allow(clippy::unimplemented)] + unimplemented!("The test should not call update_credential") + } + + async fn get_info(&self) -> StoreInfo { + StoreInfo { + discoverability: DiscoverabilitySupport::OnlyNonDiscoverable, + } + } + } + + // Arrange + let store = StoreWithoutDiscoverableSupport; + let user_mock = MockUserValidationMethod::verified_user(1); + let request = good_request(); + let mut authenticator = Authenticator::new(Aaguid::new_empty(), store, user_mock); + authenticator.set_make_credentials_with_signature_counter(true); + + // Act + let err = authenticator + .make_credential(request) + .await + .expect_err("Succeeded with unsupported rk"); + + // Assert + assert_eq!(err, Ctap2Error::UnsupportedOption.into()); + } } diff --git a/passkey-authenticator/src/credential_store.rs b/passkey-authenticator/src/credential_store.rs index 03a9ab1..c68167d 100644 --- a/passkey-authenticator/src/credential_store.rs +++ b/passkey-authenticator/src/credential_store.rs @@ -3,13 +3,36 @@ use std::sync::Arc; use passkey_types::{ ctap2::{ - make_credential::PublicKeyCredentialRpEntity, - make_credential::PublicKeyCredentialUserEntity, Ctap2Error, StatusCode, + get_assertion::Options, + make_credential::{PublicKeyCredentialRpEntity, PublicKeyCredentialUserEntity}, + Ctap2Error, StatusCode, }, webauthn::PublicKeyCredentialDescriptor, Passkey, }; +/// A struct that defines the capabilities of a store. +pub struct StoreInfo { + /// How the store handles discoverability. + pub discoverability: DiscoverabilitySupport, +} + +/// Enum to define how the store handles discoverability. +/// Note that this is does not say anything about which storage mode will be used. +#[derive(PartialEq)] +pub enum DiscoverabilitySupport { + /// The store supports both discoverable and non-credentials. + Full, + + /// The store only supports non-discoverable credentials. + /// An error will be returned if a discoverable credential is requested. + OnlyNonDiscoverable, + + /// The store only supports discoverable credential. + /// No error will be returned if a non-discoverable credential is requested. + ForcedDiscoverable, +} + /// Use this on a type that enables storage and fetching of credentials #[async_trait::async_trait] pub trait CredentialStore { @@ -32,10 +55,14 @@ pub trait CredentialStore { cred: Passkey, user: PublicKeyCredentialUserEntity, rp: PublicKeyCredentialRpEntity, + options: Options, ) -> Result<(), StatusCode>; /// Update the credential in your store async fn update_credential(&mut self, cred: Passkey) -> Result<(), StatusCode>; + + /// Get information about the store + async fn get_info(&self) -> StoreInfo; } /// In-memory store for Passkeys @@ -70,6 +97,7 @@ impl CredentialStore for MemoryStore { cred: Passkey, _user: PublicKeyCredentialUserEntity, _rp: PublicKeyCredentialRpEntity, + _options: Options, ) -> Result<(), StatusCode> { self.insert(cred.credential_id.clone().into(), cred); Ok(()) @@ -79,6 +107,12 @@ impl CredentialStore for MemoryStore { self.insert(cred.credential_id.clone().into(), cred); Ok(()) } + + async fn get_info(&self) -> StoreInfo { + StoreInfo { + discoverability: DiscoverabilitySupport::ForcedDiscoverable, + } + } } #[async_trait::async_trait] @@ -107,6 +141,7 @@ impl CredentialStore for Option { cred: Passkey, _user: PublicKeyCredentialUserEntity, _rp: PublicKeyCredentialRpEntity, + _options: Options, ) -> Result<(), StatusCode> { self.replace(cred); Ok(()) @@ -116,6 +151,12 @@ impl CredentialStore for Option { self.replace(cred); Ok(()) } + + async fn get_info(&self) -> StoreInfo { + StoreInfo { + discoverability: DiscoverabilitySupport::ForcedDiscoverable, + } + } } #[cfg(any(feature = "tokio", test))] @@ -138,13 +179,21 @@ impl + Send + Sync> CredentialStore cred: Passkey, user: PublicKeyCredentialUserEntity, rp: PublicKeyCredentialRpEntity, + options: Options, ) -> Result<(), StatusCode> { - self.lock().await.save_credential(cred, user, rp).await + self.lock() + .await + .save_credential(cred, user, rp, options) + .await } async fn update_credential(&mut self, cred: Passkey) -> Result<(), StatusCode> { self.lock().await.update_credential(cred).await } + + async fn get_info(&self) -> StoreInfo { + self.lock().await.get_info().await + } } #[cfg(any(feature = "tokio", test))] @@ -167,13 +216,21 @@ impl + Send + Sync> CredentialStore cred: Passkey, user: PublicKeyCredentialUserEntity, rp: PublicKeyCredentialRpEntity, + options: Options, ) -> Result<(), StatusCode> { - self.write().await.save_credential(cred, user, rp).await + self.write() + .await + .save_credential(cred, user, rp, options) + .await } async fn update_credential(&mut self, cred: Passkey) -> Result<(), StatusCode> { self.write().await.update_credential(cred).await } + + async fn get_info(&self) -> StoreInfo { + self.read().await.get_info().await + } } #[cfg(any(feature = "tokio", test))] @@ -196,13 +253,21 @@ impl + Send + Sync> CredentialStore cred: Passkey, user: PublicKeyCredentialUserEntity, rp: PublicKeyCredentialRpEntity, + options: Options, ) -> Result<(), StatusCode> { - self.lock().await.save_credential(cred, user, rp).await + self.lock() + .await + .save_credential(cred, user, rp, options) + .await } async fn update_credential(&mut self, cred: Passkey) -> Result<(), StatusCode> { self.lock().await.update_credential(cred).await } + + async fn get_info(&self) -> StoreInfo { + self.lock().await.get_info().await + } } #[cfg(any(feature = "tokio", test))] @@ -225,11 +290,19 @@ impl + Send + Sync> CredentialStore cred: Passkey, user: PublicKeyCredentialUserEntity, rp: PublicKeyCredentialRpEntity, + options: Options, ) -> Result<(), StatusCode> { - self.write().await.save_credential(cred, user, rp).await + self.write() + .await + .save_credential(cred, user, rp, options) + .await } async fn update_credential(&mut self, cred: Passkey) -> Result<(), StatusCode> { self.write().await.update_credential(cred).await } + + async fn get_info(&self) -> StoreInfo { + self.read().await.get_info().await + } } diff --git a/passkey-authenticator/src/ctap2.rs b/passkey-authenticator/src/ctap2.rs index 1bfb317..0dcb324 100644 --- a/passkey-authenticator/src/ctap2.rs +++ b/passkey-authenticator/src/ctap2.rs @@ -52,7 +52,7 @@ where U: UserValidationMethod + Sync + Send, { async fn get_info(&self) -> get_info::Response { - self.get_info() + self.get_info().await } async fn make_credential( diff --git a/passkey-authenticator/src/lib.rs b/passkey-authenticator/src/lib.rs index e95e9ba..cc3405a 100644 --- a/passkey-authenticator/src/lib.rs +++ b/passkey-authenticator/src/lib.rs @@ -43,7 +43,7 @@ use passkey_types::{ctap2::Ctap2Error, Bytes}; pub use self::{ authenticator::Authenticator, - credential_store::{CredentialStore, MemoryStore}, + credential_store::{CredentialStore, DiscoverabilitySupport, MemoryStore, StoreInfo}, ctap2::Ctap2Api, u2f::U2fApi, user_validation::{UserCheck, UserValidationMethod}, diff --git a/passkey-authenticator/src/u2f.rs b/passkey-authenticator/src/u2f.rs index dac55ff..1ed0580 100644 --- a/passkey-authenticator/src/u2f.rs +++ b/passkey-authenticator/src/u2f.rs @@ -93,7 +93,16 @@ impl U2 let (passkey, user, rp) = Passkey::wrap_u2f_registration_request(&request, &response, handle, &private); - let result = self.store_mut().save_credential(passkey, user, rp).await; + // U2F registration does not use rk, uv, or up + let options = passkey_types::ctap2::get_assertion::Options { + rk: false, + uv: false, + up: false, + }; + let result = self + .store_mut() + .save_credential(passkey, user, rp, options) + .await; match result { Ok(_) => Ok(response), diff --git a/passkey-authenticator/src/user_validation.rs b/passkey-authenticator/src/user_validation.rs index 1ec969c..c289118 100644 --- a/passkey-authenticator/src/user_validation.rs +++ b/passkey-authenticator/src/user_validation.rs @@ -64,7 +64,9 @@ impl MockUserValidationMethod { user_mock.expect_is_presence_enabled().returning(|| true); user_mock .expect_is_verification_enabled() - .returning(|| Some(true)); + .returning(|| Some(true)) + .times(..); + user_mock.expect_is_presence_enabled().returning(|| true); user_mock .expect_check_user() .with( @@ -81,4 +83,23 @@ impl MockUserValidationMethod { .times(times); user_mock } + + /// Sets up the mock for returning true for the verification. + pub fn verified_user_with_credential(times: usize, credential: Passkey) -> Self { + let mut user_mock = MockUserValidationMethod::new(); + user_mock + .expect_is_verification_enabled() + .returning(|| Some(true)); + user_mock + .expect_check_user() + .withf(move |cred, up, uv| cred == &Some(&credential) && *up && *uv) + .returning(|_, _, _| { + Ok(UserCheck { + presence: true, + verification: true, + }) + }) + .times(times); + user_mock + } } diff --git a/passkey-client/src/extensions.rs b/passkey-client/src/extensions.rs index 0521ab8..e0ddfe4 100644 --- a/passkey-client/src/extensions.rs +++ b/passkey-client/src/extensions.rs @@ -8,7 +8,9 @@ //! [webauthn]: https://w3c.github.io/webauthn/#sctn-defined-extensions //! [credprops]: https://w3c.github.io/webauthn/#sctn-authenticator-credential-properties-extension -use passkey_authenticator::{CredentialStore, UserValidationMethod}; +use passkey_authenticator::{ + CredentialStore, DiscoverabilitySupport, StoreInfo, UserValidationMethod, +}; use passkey_types::{ ctap2::{get_assertion, make_credential}, webauthn::{ @@ -40,11 +42,19 @@ where pub(super) fn registration_extension_outputs( &self, request: Option<&AuthenticationExtensionsClientInputs>, + store_info: StoreInfo, rk: bool, ) -> AuthenticationExtensionsClientOutputs { - let cred_props = if let Some(true) = request.and_then(|ext| ext.cred_props) { + let cred_props_requested = request.and_then(|ext| ext.cred_props) == Some(true); + let cred_props = if cred_props_requested { + let discoverable = match store_info.discoverability { + DiscoverabilitySupport::Full => rk, + DiscoverabilitySupport::OnlyNonDiscoverable => false, + DiscoverabilitySupport::ForcedDiscoverable => true, + }; + Some(CredentialPropertiesOutput { - discoverable: Some(rk), + discoverable: Some(discoverable), authenticator_display_name: self.authenticator.display_name().cloned(), }) } else { diff --git a/passkey-client/src/lib.rs b/passkey-client/src/lib.rs index a146d59..7d281f1 100644 --- a/passkey-client/src/lib.rs +++ b/passkey-client/src/lib.rs @@ -225,7 +225,7 @@ where // extract inner value of request as there is nothing else of value directly in CredentialCreationOptions let request = request.public_key; - let auth_info = self.authenticator.get_info(); + let auth_info = self.authenticator.get_info().await; let pub_key_cred_params = if request.pub_key_cred_params.is_empty() { webauthn::PublicKeyCredentialParameters::default_algorithms() @@ -321,8 +321,9 @@ where .map_err(|e| WebauthnError::AuthenticatorError(e.into()))?, ); + let store_info = self.authenticator.store().get_info().await; let client_extension_results = - self.registration_extension_outputs(extension_request.as_ref(), rk); + self.registration_extension_outputs(extension_request.as_ref(), store_info, rk); let response = webauthn::CreatedPublicKeyCredential { id: encoding::base64url(credential_id.credential_id()), @@ -386,6 +387,8 @@ where .unwrap_or_else(|| sha256(client_data_json.as_bytes()).to_vec()); let ctap_extensions = self.auth_extension_ctap2_input(request.extensions.as_ref()); + let rk = false; + let uv = request.user_verification != UserVerificationRequirement::Discouraged; let ctap2_response = self .authenticator @@ -394,11 +397,7 @@ where client_data_hash: client_data_json_hash.into(), allow_list: request.allow_credentials, extensions: ctap_extensions, - options: ctap2::get_assertion::Options { - rk: true, - up: true, - uv: true, - }, + options: ctap2::get_assertion::Options { rk, up: true, uv }, pin_auth: None, pin_protocol: None, }) diff --git a/passkey-types/Cargo.toml b/passkey-types/Cargo.toml index dce682c..d7cb2ee 100644 --- a/passkey-types/Cargo.toml +++ b/passkey-types/Cargo.toml @@ -17,6 +17,7 @@ workspace = true [features] default = [] serialize_bytes_as_base64_string = [] +testable = [] [dependencies] bitflags = "2" diff --git a/passkey-types/src/passkey.rs b/passkey-types/src/passkey.rs index 321e69e..0fe5d5f 100644 --- a/passkey-types/src/passkey.rs +++ b/passkey-types/src/passkey.rs @@ -21,6 +21,7 @@ use coset::CoseKey; // TODO: Implement Zeroize on this if/when rolling our own CoseKey type // TODO: use `#[non_exhaustive]` here with a builder pattern for building new passkeys #[derive(Clone)] +#[cfg_attr(any(test, feature = "testable"), derive(PartialEq))] pub struct Passkey { /// The private key in COSE key format. /// @@ -36,8 +37,8 @@ pub struct Passkey { /// Credential IDs are generated by authenticators in two forms: /// 1. At least 16 bytes that include at least 100 bits of entropy, or /// 2. The [`Passkey`] item, without its `credential_id`, encrypted so only its managing - /// authenticator can decrypt it. This form allows the authenticator to be nearly stateless, by - /// having the Relying Party store any necessary state. + /// authenticator can decrypt it. This form allows the authenticator to be nearly stateless, by + /// having the Relying Party store any necessary state. /// /// Relying Parties do not need to distinguish these two `credential id` forms. /// diff --git a/public-suffix/src/lib.rs b/public-suffix/src/lib.rs index 07e1a2a..bf0b279 100644 --- a/public-suffix/src/lib.rs +++ b/public-suffix/src/lib.rs @@ -41,6 +41,7 @@ //! - "www.books.amazon.co.uk" //! - "books.amazon.co.uk" //! - "amazon.co.uk" +//! //! Specifically, the eTLD+1 is "amazon.co.uk", because the eTLD is "co.uk". //! //! ``` @@ -86,12 +87,12 @@ //! 0. Make sure you have golang installed. //! 1. Make the public-suffix crate the current working directory. //! 2. `wget https://publicsuffix.org/list/public_suffix_list.dat`, which will -//! overwrite the old version of this file. +//! overwrite the old version of this file. //! 3. Run `./gen.sh` to regenerate the list from the updated `public_suffix_list.dat`. -//! The first time you run this, you'll need network connectivity to `go get` the -//! dependencies. +//! The first time you run this, you'll need network connectivity to `go get` the +//! dependencies. //! 4. Commit the changed generated source code and the updated -//! `public_suffix_list.dat`. +//! `public_suffix_list.dat`. //! //! We intentionally do not try to download the latest version of the public suffix //! list during the build to keep the build deterministic and networking-free. @@ -167,6 +168,12 @@ impl EffectiveTLDProvider for ListProvider { } } +impl Default for ListProvider { + fn default() -> Self { + Self::new() + } +} + impl ListProvider { /// Create a new ListProvider. pub const fn new() -> Self { @@ -285,12 +292,6 @@ impl ListProvider { } } -impl Default for ListProvider { - fn default() -> Self { - Self::new() - } -} - fn after_or_all(dot: Option) -> RangeFrom { match dot { Some(dot) => (dot + 1)..,