From 726343f979ac78a353914afa7f482cd8c0126d19 Mon Sep 17 00:00:00 2001 From: Rene Leveille Date: Mon, 8 Jul 2024 15:24:40 -0400 Subject: [PATCH] fix!: return android app signature as the origin --- CHANGELOG.md | 7 ++- passkey-client/src/lib.rs | 98 ++++++++++++++++++++++++++++++--- passkey-client/src/tests/mod.rs | 24 ++++---- 3 files changed, 109 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49cb38d..15a69cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,11 @@ - Changed: The `Client` no longer hardcodes the UV value sent to the `Authenticator` ([#22](https://github.com/1Password/passkey-rs/pull/22)). - Changed: The `Client` no longer hardcodes the RK value sent to the `Authenticator` ([#27](https://github.com/1Password/passkey-rs/pull/27)). +- ⚠ BREAKING: Added the `Origin` enum which is now the origin parameter for the following methods ([#32](https://github.com/1Password/passkey-rs/pull/27)): + - `Client::register` takes an `impl Into` instead of a `&Url` + - `Client::authenticate` takes an `impl Into` instead of a `&Url` + - `RpIdValidator::assert_domain` takes an `&Origin` instead of a `&Url` +- ⚠ BREAKING: The collected client data will now have the android app signature as the origin when a request comes from an app directly. ## Passkey v0.2.0 ### passkey-types v0.2.0 @@ -63,4 +68,4 @@ Most of these changes are adding fields to structs which are breaking changes du ### public-suffix v0.1.1 -- Update the public suffix list \ No newline at end of file +- Update the public suffix list diff --git a/passkey-client/src/lib.rs b/passkey-client/src/lib.rs index 2b48f59..afcf17e 100644 --- a/passkey-client/src/lib.rs +++ b/passkey-client/src/lib.rs @@ -14,7 +14,7 @@ //! [version]: https://img.shields.io/crates/v/passkey-client?logo=rust&style=flat //! [documentation]: https://img.shields.io/docsrs/passkey-client/latest?logo=docs.rs&style=flat //! [Webauthn]: https://w3c.github.io/webauthn/ -use std::borrow::Cow; +use std::{borrow::Cow, fmt::Display}; use ciborium::{cbor, value::Value}; use coset::{iana::EnumI64, Algorithm}; @@ -95,6 +95,43 @@ fn decode_host(host: &str) -> Option> { } } +/// The origin of a WebAuthn request. +pub enum Origin<'a> { + /// A Url, meant for a request in the web browser. + Web(Cow<'a, Url>), + /// An android digital asset fingerprint. + /// Meant for a request coming from an android application. + #[cfg(feature = "android-asset-validation")] + Android(UnverifiedAssetLink<'a>), +} + +impl From for Origin<'_> { + fn from(value: Url) -> Self { + Origin::Web(Cow::Owned(value)) + } +} + +impl<'a> From<&'a Url> for Origin<'a> { + fn from(value: &'a Url) -> Self { + Origin::Web(Cow::Borrowed(value)) + } +} + +impl Display for Origin<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Origin::Web(url) => write!(f, "{}", url.as_str().trim_end_matches('/')), + Origin::Android(target_link) => { + write!( + f, + "android:apk-key-hash:{}", + encoding::base64url(target_link.sha256_cert_fingerprint()) + ) + } + } + } +} + /// A `Client` represents a Webauthn client. Users of this struct should supply a /// [`CredentialStore`], a [`UserValidationMethod`] and, optionally, an implementation of /// [`public_suffix::EffectiveTLDProvider`]. @@ -171,10 +208,12 @@ where /// Returns either a [`webauthn::CreatedPublicKeyCredential`] on success or some [`WebauthnError`] pub async fn register( &mut self, - origin: &Url, + origin: impl Into>, request: webauthn::CredentialCreationOptions, client_data_hash: Option>, ) -> Result { + let origin = origin.into(); + // 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(); @@ -193,12 +232,12 @@ where let rp_id = self .rp_id_verifier - .assert_domain(origin, request.rp.id.as_deref())?; + .assert_domain(&origin, request.rp.id.as_deref())?; let collected_client_data = webauthn::CollectedClientData { ty: webauthn::ClientDataType::Create, challenge: encoding::base64url(&request.challenge), - origin: origin.as_str().trim_end_matches('/').to_owned(), + origin: origin.to_string(), cross_origin: None, unknown_keys: Default::default(), }; @@ -301,10 +340,12 @@ where /// Returns either an [`webauthn::AuthenticatedPublicKeyCredential`] on success or some [`WebauthnError`]. pub async fn authenticate( &mut self, - origin: &Url, + origin: impl Into>, request: webauthn::CredentialRequestOptions, client_data_hash: Option>, ) -> Result { + let origin = origin.into(); + // extract inner value of request as there is nothing else of value directly in CredentialRequestOptions let request = request.public_key; @@ -317,12 +358,12 @@ where let rp_id = self .rp_id_verifier - .assert_domain(origin, request.rp_id.as_deref())?; + .assert_domain(&origin, request.rp_id.as_deref())?; let collected_client_data = webauthn::CollectedClientData { ty: webauthn::ClientDataType::Get, challenge: encoding::base64url(&request.challenge), - origin: origin.as_str().trim_end_matches('/').to_owned(), + origin: origin.to_string(), cross_origin: None, //Some(false), unknown_keys: Default::default(), }; @@ -450,6 +491,18 @@ where /// /// Returns the effective domain on success or some [`WebauthnError`] pub fn assert_domain<'a>( + &self, + origin: &'a Origin, + rp_id: Option<&'a str>, + ) -> Result<&'a str, WebauthnError> { + match origin { + Origin::Web(url) => self.assert_web_rp_id(url, rp_id), + #[cfg(feature = "android-asset-validation")] + Origin::Android(unverified) => self.assert_android_rp_id(unverified, rp_id), + } + } + + fn assert_web_rp_id<'a>( &self, origin: &'a Url, rp_id: Option<&'a str>, @@ -489,6 +542,37 @@ where Ok(effective_domain) } + + #[cfg(feature = "android-asset-validation")] + fn assert_android_rp_id<'a>( + &self, + target_link: &'a UnverifiedAssetLink, + rp_id: Option<&'a str>, + ) -> Result<&'a str, WebauthnError> { + let mut effective_rp_id = target_link.host(); + + if let Some(rp_id) = rp_id { + // subset from assert_web_rp_id + if !effective_rp_id.ends_with(rp_id) { + return Err(WebauthnError::OriginRpMissmatch); + } + effective_rp_id = rp_id; + } + + if decode_host(effective_rp_id) + .as_ref() + .and_then(|s| self.tld_provider.effective_tld_plus_one(s).ok()) + .is_none() + { + return Err(WebauthnError::InvalidRpId); + } + + // TODO: Find an ergonomic and caching friendly way to fetch the remote + // assetlinks and validate them here. + // https://github.com/1Password/passkey-rs/issues/13 + + Ok(effective_rp_id) + } } #[cfg(test)] diff --git a/passkey-client/src/tests/mod.rs b/passkey-client/src/tests/mod.rs index 8092b88..536357f 100644 --- a/passkey-client/src/tests/mod.rs +++ b/passkey-client/src/tests/mod.rs @@ -97,7 +97,7 @@ async fn create_and_authenticate() { public_key: good_credential_request_options(credential_id), }; client - .authenticate(&origin, auth_options, None) + .authenticate(origin, auth_options, None) .await .expect("failed to authenticate with freshly created credential"); } @@ -132,7 +132,7 @@ async fn create_and_authenticate_with_origin_subdomain() { public_key: good_credential_request_options(cred.raw_id), }; let res = client - .authenticate(&origin, auth_options, None) + .authenticate(origin, auth_options, None) .await .expect("failed to authenticate with freshly created credential"); let att_obj = ctap2::AuthenticatorData::from_slice(&res.response.authenticator_data) @@ -179,7 +179,7 @@ async fn create_and_authenticate_without_rp_id() { }, }; let res = client - .authenticate(&origin, auth_options, None) + .authenticate(origin, auth_options, None) .await .expect("failed to authenticate with freshly created credential"); let att_obj = ctap2::AuthenticatorData::from_slice(&res.response.authenticator_data) @@ -214,7 +214,7 @@ async fn create_and_authenticate_without_cred_params() { public_key: good_credential_request_options(credential_id), }; client - .authenticate(&origin, auth_options, None) + .authenticate(origin, auth_options, None) .await .expect("failed to authenticate with freshly created credential"); } @@ -223,26 +223,26 @@ async fn create_and_authenticate_without_cred_params() { fn validate_rp_id() -> Result<(), ParseError> { let client = RpIdVerifier::new(public_suffix::DEFAULT_PROVIDER); - let example = "https://example.com".parse()?; + let example = Url::parse("https://example.com")?.into(); let com_tld = client.assert_domain(&example, Some("com")); assert_eq!(com_tld, Err(WebauthnError::InvalidRpId)); - let example_dots = "https://example...com".parse()?; + let example_dots = Url::parse("https://example...com")?.into(); let bunch_of_dots = client.assert_domain(&example_dots, Some("...com")); assert_eq!(bunch_of_dots, Err(WebauthnError::InvalidRpId)); - let future = "https://www.future.1password.com".parse()?; + let future = Url::parse("https://www.future.1password.com")?.into(); let sub_domain_ignored = client.assert_domain(&future, Some("future.1password.com")); assert_eq!(sub_domain_ignored, Ok("future.1password.com")); let use_effective_domain = client.assert_domain(&future, None); assert_eq!(use_effective_domain, Ok("www.future.1password.com")); - let not_protected = "http://example.com".parse()?; + let not_protected = Url::parse("http://example.com")?.into(); let not_https = client.assert_domain(¬_protected, Some("example.com")); assert_eq!(not_https, Err(WebauthnError::UnprotectedOrigin)); - let localhost = "http://localhost:8080".parse()?; + let localhost = Url::parse("http://localhost:8080")?.into(); let should_still_match = client.assert_domain(&localhost, Some("example.com")); assert_eq!(should_still_match, Err(WebauthnError::OriginRpMissmatch)); @@ -286,7 +286,7 @@ fn validate_domain_with_private_list_provider() -> Result<(), ParseError> { // Notice that, in this test, this is a legitimate origin/rp_id combination // We assert that this produces an error to prove that we are indeed using our // BrokenTLDProvider which always returns Err() regardless of the TLD. - let origin = "https://www.future.1password.com".parse()?; + let origin = Url::parse("https://www.future.1password.com")?.into(); let rp_id = "future.1password.com"; let result = client.assert_domain(&origin, Some(rp_id)); assert_eq!(result, Err(WebauthnError::InvalidRpId)); @@ -351,7 +351,7 @@ async fn client_register_triggers_uv_when_uv_is_required() { // Act & Assert client - .register(&origin, options, None) + .register(origin, options, None) .await .expect("failed to register with options"); } @@ -378,7 +378,7 @@ async fn client_register_does_not_trigger_uv_when_uv_is_discouraged() { // Act & Assert client - .register(&origin, options, None) + .register(origin, options, None) .await .expect("failed to register with options"); }