Skip to content

Commit

Permalink
fix!: return android app signature as the origin
Browse files Browse the repository at this point in the history
  • Loading branch information
Progdrasil committed Jul 8, 2024
1 parent adde2da commit 726343f
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 20 deletions.
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<Origin>` instead of a `&Url`
- `Client::authenticate` takes an `impl Into<Origin>` 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
Expand Down Expand Up @@ -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
- Update the public suffix list
98 changes: 91 additions & 7 deletions passkey-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -95,6 +95,43 @@ fn decode_host(host: &str) -> Option<Cow<str>> {
}
}

/// 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<Url> 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`].
Expand Down Expand Up @@ -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<Origin<'_>>,
request: webauthn::CredentialCreationOptions,
client_data_hash: Option<Vec<u8>>,
) -> Result<webauthn::CreatedPublicKeyCredential, WebauthnError> {
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();
Expand All @@ -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(),
};
Expand Down Expand Up @@ -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<Origin<'_>>,
request: webauthn::CredentialRequestOptions,
client_data_hash: Option<Vec<u8>>,
) -> Result<webauthn::AuthenticatedPublicKeyCredential, WebauthnError> {
let origin = origin.into();

// extract inner value of request as there is nothing else of value directly in CredentialRequestOptions
let request = request.public_key;

Expand All @@ -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(),
};
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -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)]
Expand Down
24 changes: 12 additions & 12 deletions passkey-client/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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");
}
Expand All @@ -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(&not_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));

Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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");
}
Expand All @@ -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");
}

0 comments on commit 726343f

Please sign in to comment.