From 547d0f2d1567add8a94f27b7092453df5b34c7b2 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Tue, 18 Jun 2024 11:12:26 +0200 Subject: [PATCH] Add fido2 credential autofill view (#849) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 🎟ī¸ Tracking ## 📔 Objective This PR adds a function on the Fido2 authenticator which returns all available credentials as a View tailored specifically for integrating with OS-level autofill APIs ## 📸 Screenshots ## ⏰ Reminders before review - Contributor guidelines followed - All formatters and local linters executed and passed - Written new unit and / or integration tests where applicable - Protected functional changes with optionality (feature flags) - Used internationalization (i18n) for all UI strings - CI builds passed - Communicated to DevOps any deployment requirements - Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team ## đŸĻŽ Reviewer guidelines - 👍 (`:+1:`) or similar for great changes - 📝 (`:memo:`) or ℹī¸ (`:information_source:`) for notes or general info - ❓ (`:question:`) for questions - 🤔 (`:thinking:`) or 💭 (`:thought_balloon:`) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion - 🎨 (`:art:`) for suggestions / improvements - ❌ (`:x:`) or ⚠ī¸ (`:warning:`) for more significant problems or concerns needing attention - 🌱 (`:seedling:`) or â™ģī¸ (`:recycle:`) for future improvements or indications of technical debt - ⛏ (`:pick:`) for minor or nitpick changes --- Cargo.lock | 21 +++-- crates/bitwarden-fido/Cargo.toml | 2 + crates/bitwarden-fido/src/authenticator.rs | 58 +++++++++++-- crates/bitwarden-fido/src/lib.rs | 9 +- crates/bitwarden-fido/src/traits.rs | 2 + crates/bitwarden-fido/src/types.rs | 85 ++++++++++++++++++- crates/bitwarden-fido/src/uniffi_support.rs | 3 + crates/bitwarden-uniffi/src/platform/fido2.rs | 42 ++++++++- crates/bitwarden/src/error.rs | 8 ++ crates/bitwarden/src/platform/client_fido.rs | 26 +++++- crates/bitwarden/src/platform/mod.rs | 2 + 11 files changed, 237 insertions(+), 21 deletions(-) create mode 100644 crates/bitwarden-fido/src/uniffi_support.rs diff --git a/Cargo.lock b/Cargo.lock index f8d175edb..e7fca6a43 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -358,7 +358,7 @@ name = "bitwarden" version = "0.5.0" dependencies = [ "async-trait", - "base64 0.21.7", + "base64 0.22.1", "bitwarden-api-api", "bitwarden-api-identity", "bitwarden-core", @@ -497,16 +497,18 @@ name = "bitwarden-fido" version = "0.5.0" dependencies = [ "async-trait", - "base64 0.21.7", + "base64 0.22.1", "bitwarden-core", "bitwarden-crypto", "bitwarden-vault", "chrono", "coset", + "itertools 0.13.0", "log", "p256", "passkey", "reqwest", + "schemars", "serde", "serde_json", "thiserror", @@ -570,7 +572,7 @@ dependencies = [ name = "bitwarden-send" version = "0.5.0" dependencies = [ - "base64 0.21.7", + "base64 0.22.1", "bitwarden-api-api", "bitwarden-core", "bitwarden-crypto", @@ -610,7 +612,7 @@ dependencies = [ name = "bitwarden-vault" version = "0.5.0" dependencies = [ - "base64 0.21.7", + "base64 0.22.1", "bitwarden-api-api", "bitwarden-core", "bitwarden-crypto", @@ -2149,6 +2151,15 @@ dependencies = [ "either", ] +[[package]] +name = "itertools" +version = "0.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "413ee7dfc52ee1a4949ceeb7dbc8a33f2d6c088194d9f922fb8318faf1f01186" +dependencies = [ + "either", +] + [[package]] name = "itoa" version = "1.0.11" @@ -2206,7 +2217,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0c2a198fb6b0eada2a8df47933734e6d35d350665a33a3593d7164fa52c75c19" dependencies = [ "cfg-if", - "windows-targets 0.48.5", + "windows-targets 0.52.5", ] [[package]] diff --git a/crates/bitwarden-fido/Cargo.toml b/crates/bitwarden-fido/Cargo.toml index d34177fd2..2d9dc9eaa 100644 --- a/crates/bitwarden-fido/Cargo.toml +++ b/crates/bitwarden-fido/Cargo.toml @@ -27,10 +27,12 @@ chrono = { version = ">=0.4.26, <0.5", features = [ "serde", ], default-features = false } coset = { version = "0.3.7" } +itertools = "0.13.0" log = ">=0.4.18, <0.5" p256 = { version = ">=0.13.2, <0.14" } passkey = { git = "https://github.com/bitwarden/passkey-rs", rev = "c48c2ddfd6b884b2d754432576c66cb2b1985a3a" } reqwest = { version = ">=0.12, <0.13", default-features = false } +schemars = { version = "0.8.21", features = ["uuid1", "chrono"] } serde = { version = ">=1.0, <2.0", features = ["derive"] } serde_json = ">=1.0.96, <2.0" thiserror = ">=1.0.40, <2.0" diff --git a/crates/bitwarden-fido/src/authenticator.rs b/crates/bitwarden-fido/src/authenticator.rs index edc5033a7..41f5d5b45 100644 --- a/crates/bitwarden-fido/src/authenticator.rs +++ b/crates/bitwarden-fido/src/authenticator.rs @@ -2,7 +2,8 @@ use std::sync::{Arc, Mutex}; use bitwarden_core::VaultLocked; use bitwarden_crypto::{CryptoError, KeyContainer, KeyEncryptable}; -use bitwarden_vault::{CipherError, CipherView, Fido2CredentialView}; +use bitwarden_vault::{CipherError, CipherView}; +use itertools::Itertools; use log::error; use passkey::{ authenticator::{Authenticator, DiscoverabilitySupport, StoreInfo, UIHint, UserCheck}, @@ -67,10 +68,30 @@ pub enum GetAssertionError { #[derive(Debug, Error)] pub enum SilentlyDiscoverCredentialsError { + #[error(transparent)] + CipherError(#[from] CipherError), #[error(transparent)] VaultLocked(#[from] VaultLocked), #[error(transparent)] + InvalidGuid(#[from] InvalidGuid), + #[error(transparent)] Fido2CallbackError(#[from] Fido2CallbackError), + #[error(transparent)] + FromCipherViewError(#[from] Fido2CredentialAutofillViewError), +} + +#[derive(Debug, Error)] +pub enum CredentialsForAutofillError { + #[error(transparent)] + CipherError(#[from] CipherError), + #[error(transparent)] + VaultLocked(#[from] VaultLocked), + #[error(transparent)] + InvalidGuid(#[from] InvalidGuid), + #[error(transparent)] + Fido2CallbackError(#[from] Fido2CallbackError), + #[error(transparent)] + FromCipherViewError(#[from] Fido2CredentialAutofillViewError), } /// Temporary trait for solving a circular dependency. When moving `Client` to `bitwarden-core` @@ -236,15 +257,40 @@ impl<'a> Fido2Authenticator<'a> { pub async fn silently_discover_credentials( &mut self, rp_id: String, - ) -> Result, SilentlyDiscoverCredentialsError> { + ) -> Result, SilentlyDiscoverCredentialsError> { let enc = self.client.get_encryption_settings()?; let result = self.credential_store.find_credentials(None, rp_id).await?; - Ok(result + result .into_iter() - .flat_map(|c| c.decrypt_fido2_credentials(&*enc)) - .flatten() - .collect()) + .map( + |cipher| -> Result, SilentlyDiscoverCredentialsError> { + Ok(Fido2CredentialAutofillView::from_cipher_view(&cipher, &*enc)?) + }, + ) + .flatten_ok() + .collect() + } + + /// Returns all Fido2 credentials that can be used for autofill, in a view + /// tailored for integration with OS autofill systems. + pub async fn credentials_for_autofill( + &mut self, + ) -> Result, CredentialsForAutofillError> { + let enc = self.client.get_encryption_settings()?; + let all_credentials = self.credential_store.all_credentials().await?; + + all_credentials + .into_iter() + .map( + |cipher| -> Result, CredentialsForAutofillError> { + Ok(Fido2CredentialAutofillView::from_cipher_view( + &cipher, &*enc, + )?) + }, + ) + .flatten_ok() + .collect() } pub(super) fn get_authenticator( diff --git a/crates/bitwarden-fido/src/lib.rs b/crates/bitwarden-fido/src/lib.rs index f3532a3c8..b572eef36 100644 --- a/crates/bitwarden-fido/src/lib.rs +++ b/crates/bitwarden-fido/src/lib.rs @@ -8,6 +8,8 @@ use passkey::types::{ctap2::Aaguid, Passkey}; #[cfg(feature = "uniffi")] uniffi::setup_scaffolding!(); +#[cfg(feature = "uniffi")] +mod uniffi_support; mod authenticator; mod client; @@ -15,8 +17,8 @@ mod crypto; mod traits; mod types; pub use authenticator::{ - Fido2Authenticator, FidoEncryptionSettingStore, GetAssertionError, MakeCredentialError, - SilentlyDiscoverCredentialsError, + CredentialsForAutofillError, Fido2Authenticator, FidoEncryptionSettingStore, GetAssertionError, + MakeCredentialError, SilentlyDiscoverCredentialsError, }; pub use client::{Fido2Client, Fido2ClientError}; pub use passkey::authenticator::UIHint; @@ -27,7 +29,8 @@ pub use traits::{ }; pub use types::{ AuthenticatorAssertionResponse, AuthenticatorAttestationResponse, ClientData, - GetAssertionRequest, GetAssertionResult, MakeCredentialRequest, MakeCredentialResult, Options, + Fido2CredentialAutofillView, Fido2CredentialAutofillViewError, GetAssertionRequest, + GetAssertionResult, MakeCredentialRequest, MakeCredentialResult, Options, PublicKeyCredentialAuthenticatorAssertionResponse, PublicKeyCredentialAuthenticatorAttestationResponse, PublicKeyCredentialRpEntity, PublicKeyCredentialUserEntity, diff --git a/crates/bitwarden-fido/src/traits.rs b/crates/bitwarden-fido/src/traits.rs index 0769eeb2a..1e00a318a 100644 --- a/crates/bitwarden-fido/src/traits.rs +++ b/crates/bitwarden-fido/src/traits.rs @@ -41,6 +41,8 @@ pub trait Fido2CredentialStore: Send + Sync { rip_id: String, ) -> Result, Fido2CallbackError>; + async fn all_credentials(&self) -> Result, Fido2CallbackError>; + async fn save_credential(&self, cred: Cipher) -> Result<(), Fido2CallbackError>; } diff --git a/crates/bitwarden-fido/src/types.rs b/crates/bitwarden-fido/src/types.rs index 16edde6ea..d6eaa4752 100644 --- a/crates/bitwarden-fido/src/types.rs +++ b/crates/bitwarden-fido/src/types.rs @@ -1,8 +1,89 @@ +use bitwarden_crypto::KeyContainer; +use bitwarden_vault::{CipherError, CipherView}; use passkey::types::webauthn::UserVerificationRequirement; -use serde::Serialize; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; use thiserror::Error; -use super::{get_enum_from_string_name, SelectedCredential, UnknownEnum, Verification}; +use super::{ + get_enum_from_string_name, string_to_guid_bytes, InvalidGuid, SelectedCredential, UnknownEnum, + Verification, +}; + +#[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +#[cfg_attr(feature = "uniffi", derive(uniffi::Record))] +pub struct Fido2CredentialAutofillView { + pub credential_id: Vec, + pub cipher_id: uuid::Uuid, + pub rp_id: String, + pub user_name_for_ui: Option, + pub user_handle: Vec, +} + +trait NoneWhitespace { + /// Convert only whitespace to None + fn none_whitespace(&self) -> Option; +} + +impl NoneWhitespace for String { + fn none_whitespace(&self) -> Option { + match self.trim() { + "" => None, + s => Some(s.to_owned()), + } + } +} + +impl NoneWhitespace for Option { + fn none_whitespace(&self) -> Option { + self.as_ref().and_then(|s| s.none_whitespace()) + } +} + +#[derive(Debug, Error)] +pub enum Fido2CredentialAutofillViewError { + #[error( + "Autofill credentials can only be created from existing ciphers that have a cipher id" + )] + MissingCipherId, + + #[error(transparent)] + InvalidGuid(#[from] InvalidGuid), + + #[error(transparent)] + CipherError(#[from] CipherError), +} + +impl Fido2CredentialAutofillView { + pub fn from_cipher_view( + cipher: &CipherView, + enc: &dyn KeyContainer, + ) -> Result, Fido2CredentialAutofillViewError> { + let credentials = cipher.decrypt_fido2_credentials(enc)?; + + credentials + .into_iter() + .filter_map(|c| -> Option> { + c.user_handle.map(|user_handle| { + Ok(Fido2CredentialAutofillView { + credential_id: string_to_guid_bytes(&c.credential_id)?, + cipher_id: cipher + .id + .ok_or(Fido2CredentialAutofillViewError::MissingCipherId)?, + rp_id: c.rp_id.clone(), + user_handle, + user_name_for_ui: c + .user_name + .none_whitespace() + .or(c.user_display_name.none_whitespace()) + .or(cipher.name.none_whitespace()), + }) + }) + }) + .collect() + } +} #[cfg_attr(feature = "uniffi", derive(uniffi::Record))] pub struct PublicKeyCredentialRpEntity { diff --git a/crates/bitwarden-fido/src/uniffi_support.rs b/crates/bitwarden-fido/src/uniffi_support.rs new file mode 100644 index 000000000..5bf94d09f --- /dev/null +++ b/crates/bitwarden-fido/src/uniffi_support.rs @@ -0,0 +1,3 @@ +use uuid::Uuid; + +uniffi::ffi_converter_forward!(Uuid, bitwarden_core::UniFfiTag, crate::UniFfiTag); diff --git a/crates/bitwarden-uniffi/src/platform/fido2.rs b/crates/bitwarden-uniffi/src/platform/fido2.rs index 09162b7d0..7cc45a2cb 100644 --- a/crates/bitwarden-uniffi/src/platform/fido2.rs +++ b/crates/bitwarden-uniffi/src/platform/fido2.rs @@ -4,12 +4,13 @@ use bitwarden::{ error::Error, platform::fido2::{ CheckUserOptions, ClientData, Fido2CallbackError as BitFido2CallbackError, - GetAssertionRequest, GetAssertionResult, MakeCredentialRequest, MakeCredentialResult, + Fido2CredentialAutofillView, GetAssertionRequest, GetAssertionResult, + MakeCredentialRequest, MakeCredentialResult, PublicKeyCredentialAuthenticatorAssertionResponse, PublicKeyCredentialAuthenticatorAttestationResponse, PublicKeyCredentialRpEntity, PublicKeyCredentialUserEntity, }, - vault::{Cipher, CipherView, Fido2CredentialNewView, Fido2CredentialView}, + vault::{Cipher, CipherView, Fido2CredentialNewView}, }; use crate::{error::Result, Client}; @@ -42,6 +43,21 @@ impl ClientFido2 { credential_store, ))) } + + pub fn decrypt_fido2_autofill_credentials( + self: Arc, + cipher_view: CipherView, + ) -> Result> { + let result = self + .0 + .0 + .platform() + .fido2() + .decrypt_fido2_autofill_credentials(cipher_view) + .map_err(Error::DecryptFido2AutofillCredentialsError)?; + + Ok(result) + } } #[derive(uniffi::Object)] @@ -87,7 +103,7 @@ impl ClientFido2Authenticator { pub async fn silently_discover_credentials( &self, rp_id: String, - ) -> Result> { + ) -> Result> { let platform = self.0 .0.platform(); let fido2 = platform.fido2(); let ui = UniffiTraitBridge(self.1.as_ref()); @@ -100,6 +116,20 @@ impl ClientFido2Authenticator { .map_err(Error::SilentlyDiscoverCredentials)?; Ok(result) } + + pub async fn credentials_for_autofill(&self) -> Result> { + let platform = self.0 .0.platform(); + let fido2 = platform.fido2(); + let ui = UniffiTraitBridge(self.1.as_ref()); + let cs = UniffiTraitBridge(self.2.as_ref()); + let mut auth = fido2.create_authenticator(&ui, &cs); + + let result = auth + .credentials_for_autofill() + .await + .map_err(Error::CredentialsForAutofillError)?; + Ok(result) + } } #[derive(uniffi::Object)] @@ -216,6 +246,8 @@ pub trait Fido2CredentialStore: Send + Sync { rip_id: String, ) -> Result, Fido2CallbackError>; + async fn all_credentials(&self) -> Result, Fido2CallbackError>; + async fn save_credential(&self, cred: Cipher) -> Result<(), Fido2CallbackError>; } @@ -240,6 +272,10 @@ impl bitwarden::platform::fido2::Fido2CredentialStore .map_err(Into::into) } + async fn all_credentials(&self) -> Result, BitFido2CallbackError> { + self.0.all_credentials().await.map_err(Into::into) + } + async fn save_credential(&self, cred: Cipher) -> Result<(), BitFido2CallbackError> { self.0.save_credential(cred).await.map_err(Into::into) } diff --git a/crates/bitwarden/src/error.rs b/crates/bitwarden/src/error.rs index 76005885f..46f34404f 100644 --- a/crates/bitwarden/src/error.rs +++ b/crates/bitwarden/src/error.rs @@ -96,6 +96,14 @@ pub enum Error { SilentlyDiscoverCredentials(#[from] bitwarden_fido::SilentlyDiscoverCredentialsError), #[cfg(all(feature = "uniffi", feature = "internal"))] #[error(transparent)] + CredentialsForAutofillError(#[from] bitwarden_fido::CredentialsForAutofillError), + #[cfg(all(feature = "uniffi", feature = "internal"))] + #[error(transparent)] + DecryptFido2AutofillCredentialsError( + #[from] crate::platform::fido2::DecryptFido2AutofillCredentialsError, + ), + #[cfg(all(feature = "uniffi", feature = "internal"))] + #[error(transparent)] Fido2Client(#[from] bitwarden_fido::Fido2ClientError), #[cfg(all(feature = "uniffi", feature = "internal"))] #[error("Fido2 Callback error: {0:?}")] diff --git a/crates/bitwarden/src/platform/client_fido.rs b/crates/bitwarden/src/platform/client_fido.rs index 63808e21b..79164b48f 100644 --- a/crates/bitwarden/src/platform/client_fido.rs +++ b/crates/bitwarden/src/platform/client_fido.rs @@ -1,9 +1,11 @@ use std::sync::Arc; use bitwarden_fido::{ - Fido2Authenticator, Fido2Client, Fido2CredentialStore, Fido2UserInterface, - FidoEncryptionSettingStore, + Fido2Authenticator, Fido2Client, Fido2CredentialAutofillView, Fido2CredentialStore, + Fido2UserInterface, FidoEncryptionSettingStore, }; +use bitwarden_vault::CipherView; +use thiserror::Error; use crate::Client; @@ -12,6 +14,14 @@ pub struct ClientFido2<'a> { pub(crate) client: &'a Client, } +#[derive(Debug, Error)] +pub enum DecryptFido2AutofillCredentialsError { + #[error(transparent)] + VaultLocked(#[from] bitwarden_core::VaultLocked), + #[error(transparent)] + Fido2CredentialAutofillViewError(#[from] bitwarden_fido::Fido2CredentialAutofillViewError), +} + impl FidoEncryptionSettingStore for Client { fn get_encryption_settings( &self, @@ -38,4 +48,16 @@ impl<'a> ClientFido2<'a> { authenticator: self.create_authenticator(user_interface, credential_store), } } + + pub fn decrypt_fido2_autofill_credentials( + &'a self, + cipher_view: CipherView, + ) -> Result, DecryptFido2AutofillCredentialsError> { + let enc = self.client.get_encryption_settings()?; + + Ok(Fido2CredentialAutofillView::from_cipher_view( + &cipher_view, + &*enc, + )?) + } } diff --git a/crates/bitwarden/src/platform/mod.rs b/crates/bitwarden/src/platform/mod.rs index 0fc051ac3..82d341e6e 100644 --- a/crates/bitwarden/src/platform/mod.rs +++ b/crates/bitwarden/src/platform/mod.rs @@ -13,4 +13,6 @@ pub use secret_verification_request::SecretVerificationRequest; #[cfg(feature = "uniffi")] pub mod fido2 { pub use bitwarden_fido::*; + + pub use super::client_fido::DecryptFido2AutofillCredentialsError; }