diff --git a/crates/bitwarden-uniffi/src/platform/fido2.rs b/crates/bitwarden-uniffi/src/platform/fido2.rs index 8c20e905f..09162b7d0 100644 --- a/crates/bitwarden-uniffi/src/platform/fido2.rs +++ b/crates/bitwarden-uniffi/src/platform/fido2.rs @@ -1,6 +1,7 @@ use std::sync::Arc; use bitwarden::{ + error::Error, platform::fido2::{ CheckUserOptions, ClientData, Fido2CallbackError as BitFido2CallbackError, GetAssertionRequest, GetAssertionResult, MakeCredentialRequest, MakeCredentialResult, @@ -60,9 +61,12 @@ impl ClientFido2Authenticator { 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 mut auth = fido2.create_authenticator(&ui, &cs); - let result = auth.make_credential(request).await?; + let result = auth + .make_credential(request) + .await + .map_err(Error::MakeCredential)?; Ok(result) } @@ -71,9 +75,12 @@ impl ClientFido2Authenticator { 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 mut auth = fido2.create_authenticator(&ui, &cs); - let result = auth.get_assertion(request).await?; + let result = auth + .get_assertion(request) + .await + .map_err(Error::GetAssertion)?; Ok(result) } @@ -85,9 +92,12 @@ impl ClientFido2Authenticator { 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 mut auth = fido2.create_authenticator(&ui, &cs); - let result = auth.silently_discover_credentials(rp_id).await?; + let result = auth + .silently_discover_credentials(rp_id) + .await + .map_err(Error::SilentlyDiscoverCredentials)?; Ok(result) } } @@ -107,9 +117,12 @@ impl ClientFido2Client { let fido2 = platform.fido2(); let ui = UniffiTraitBridge(self.0 .1.as_ref()); let cs = UniffiTraitBridge(self.0 .2.as_ref()); - let mut client = fido2.create_client(&ui, &cs)?; + let mut client = fido2.create_client(&ui, &cs); - let result = client.register(origin, request, client_data).await?; + let result = client + .register(origin, request, client_data) + .await + .map_err(Error::Fido2Client)?; Ok(result) } @@ -123,9 +136,12 @@ impl ClientFido2Client { let fido2 = platform.fido2(); let ui = UniffiTraitBridge(self.0 .1.as_ref()); let cs = UniffiTraitBridge(self.0 .2.as_ref()); - let mut client = fido2.create_client(&ui, &cs)?; + let mut client = fido2.create_client(&ui, &cs); - let result = client.authenticate(origin, request, client_data).await?; + let result = client + .authenticate(origin, request, client_data) + .await + .map_err(Error::Fido2Client)?; Ok(result) } } diff --git a/crates/bitwarden/src/client/client.rs b/crates/bitwarden/src/client/client.rs index 790cc54f1..31e32f034 100644 --- a/crates/bitwarden/src/client/client.rs +++ b/crates/bitwarden/src/client/client.rs @@ -211,12 +211,12 @@ impl Client { } } - pub(crate) fn get_encryption_settings(&self) -> Result> { + pub(crate) fn get_encryption_settings(&self) -> Result, VaultLocked> { self.encryption_settings .read() .expect("RwLock is not poisoned") .clone() - .ok_or(VaultLocked.into()) + .ok_or(VaultLocked) } pub(crate) fn set_login_method(&self, login_method: LoginMethod) { diff --git a/crates/bitwarden/src/error.rs b/crates/bitwarden/src/error.rs index 9bf7814cf..a7f7e3c3d 100644 --- a/crates/bitwarden/src/error.rs +++ b/crates/bitwarden/src/error.rs @@ -8,8 +8,6 @@ use bitwarden_api_identity::apis::Error as IdentityError; use bitwarden_exporters::ExportError; #[cfg(feature = "internal")] use bitwarden_generators::{PassphraseError, PasswordError, UsernameError}; -#[cfg(feature = "uniffi")] -use passkey::client::WebauthnError; use reqwest::StatusCode; use thiserror::Error; @@ -81,15 +79,25 @@ pub enum Error { #[error(transparent)] ExportError(#[from] ExportError), - #[cfg(feature = "uniffi")] - #[error("Webauthn error: {0:?}")] - WebauthnError(WebauthnError), + // Fido + #[cfg(all(feature = "uniffi", feature = "internal"))] + #[error(transparent)] + MakeCredential(#[from] crate::platform::fido2::MakeCredentialError), + #[cfg(all(feature = "uniffi", feature = "internal"))] + #[error(transparent)] + GetAssertion(#[from] crate::platform::fido2::GetAssertionError), + #[cfg(all(feature = "uniffi", feature = "internal"))] + #[error(transparent)] + SilentlyDiscoverCredentials(#[from] crate::platform::fido2::SilentlyDiscoverCredentialsError), + #[cfg(all(feature = "uniffi", feature = "internal"))] + #[error(transparent)] + Fido2Client(#[from] crate::platform::fido2::Fido2ClientError), #[cfg(feature = "uniffi")] #[error("Uniffi callback error: {0}")] UniffiCallbackError(#[from] uniffi::UnexpectedUniFFICallbackError), - #[cfg(feature = "uniffi")] + #[cfg(all(feature = "uniffi", feature = "internal"))] #[error("Fido2 Callback error: {0:?}")] Fido2CallbackError(#[from] crate::platform::fido2::Fido2CallbackError), @@ -97,13 +105,6 @@ pub enum Error { Internal(Cow<'static, str>), } -#[cfg(feature = "uniffi")] -impl From for Error { - fn from(e: WebauthnError) -> Self { - Self::WebauthnError(e) - } -} - impl From for Error { fn from(s: String) -> Self { Self::Internal(s.into()) diff --git a/crates/bitwarden/src/platform/fido2/authenticator.rs b/crates/bitwarden/src/platform/fido2/authenticator.rs index 06d64f3b3..d0a7b83e4 100644 --- a/crates/bitwarden/src/platform/fido2/authenticator.rs +++ b/crates/bitwarden/src/platform/fido2/authenticator.rs @@ -1,8 +1,8 @@ use std::sync::Mutex; use bitwarden_core::VaultLocked; -use bitwarden_crypto::KeyEncryptable; -use bitwarden_vault::{CipherView, Fido2CredentialView}; +use bitwarden_crypto::{CryptoError, KeyEncryptable}; +use bitwarden_vault::{CipherError, CipherView, Fido2CredentialView}; use log::error; use passkey::{ authenticator::{Authenticator, DiscoverabilitySupport, StoreInfo, UIHint, UserCheck}, @@ -11,17 +11,71 @@ use passkey::{ Passkey, }, }; +use thiserror::Error; use super::{ try_from_credential_new_view, types::*, CheckUserOptions, CheckUserResult, CipherViewContainer, - Fido2CredentialStore, Fido2UserInterface, SelectedCredential, AAGUID, + Fido2CredentialStore, Fido2UserInterface, SelectedCredential, UnknownEnum, AAGUID, }; use crate::{ - error::Result, - platform::fido2::{fill_with_credential, string_to_guid_bytes, try_from_credential_full}, + platform::fido2::{ + fill_with_credential, string_to_guid_bytes, try_from_credential_full, Fido2CallbackError, + FillCredentialError, InvalidGuid, + }, Client, }; +#[derive(Debug, Error)] +pub enum GetSelectedCredentialError { + #[error("No selected credential available")] + NoSelectedCredential, + #[error("No fido2 credentials found")] + NoCredentialFound, + + #[error(transparent)] + VaultLocked(#[from] VaultLocked), + #[error(transparent)] + CipherError(#[from] CipherError), +} + +#[derive(Debug, Error)] +pub enum MakeCredentialError { + #[error(transparent)] + PublicKeyCredentialParametersError(#[from] PublicKeyCredentialParametersError), + #[error(transparent)] + UnknownEnum(#[from] UnknownEnum), + #[error(transparent)] + Serde(#[from] serde_json::Error), + #[error("Missing attested_credential_data")] + MissingAttestedCredentialData, + #[error("make_credential error: {0}")] + Other(String), +} + +#[derive(Debug, Error)] +pub enum GetAssertionError { + #[error(transparent)] + UnknownEnum(#[from] UnknownEnum), + #[error(transparent)] + Serde(#[from] serde_json::Error), + #[error(transparent)] + GetSelectedCredentialError(#[from] GetSelectedCredentialError), + #[error("Missing attested_credential_data")] + MissingAttestedCredentialData, + #[error("missing user")] + MissingUser, + #[error("get_assertion error: {0}")] + Other(String), +} + +#[derive(Debug, Error)] +pub enum SilentlyDiscoverCredentialsError { + #[error(transparent)] + VaultLocked(#[from] VaultLocked), + #[error(transparent)] + Fido2CallbackError(#[from] Fido2CallbackError), +} + pub struct Fido2Authenticator<'a> { pub(crate) client: &'a Client, pub(crate) user_interface: &'a dyn Fido2UserInterface, @@ -35,7 +89,7 @@ impl<'a> Fido2Authenticator<'a> { pub async fn make_credential( &mut self, request: MakeCredentialRequest, - ) -> Result { + ) -> Result { // Insert the received UV to be able to return it later in check_user self.requested_uv .get_mut() @@ -81,14 +135,14 @@ impl<'a> Fido2Authenticator<'a> { let response = match response { Ok(x) => x, - Err(e) => return Err(format!("make_credential error: {e:?}").into()), + Err(e) => return Err(MakeCredentialError::Other(format!("{e:?}"))), }; let authenticator_data = response.auth_data.to_vec(); let attested_credential_data = response .auth_data .attested_credential_data - .ok_or("Missing attested_credential_data")?; + .ok_or(MakeCredentialError::MissingAttestedCredentialData)?; let credential_id = attested_credential_data.credential_id().to_vec(); Ok(MakeCredentialResult { @@ -101,7 +155,7 @@ impl<'a> Fido2Authenticator<'a> { pub async fn get_assertion( &mut self, request: GetAssertionRequest, - ) -> Result { + ) -> Result { // Insert the received UV to be able to return it later in check_user self.requested_uv .get_mut() @@ -138,14 +192,14 @@ impl<'a> Fido2Authenticator<'a> { let response = match response { Ok(x) => x, - Err(e) => return Err(format!("get_assertion error: {e:?}").into()), + Err(e) => return Err(GetAssertionError::Other(format!("{e:?}"))), }; let authenticator_data = response.auth_data.to_vec(); let credential_id = response .auth_data .attested_credential_data - .ok_or("Missing attested_credential_data")? + .ok_or(GetAssertionError::MissingAttestedCredentialData)? .credential_id() .to_vec(); @@ -153,7 +207,11 @@ impl<'a> Fido2Authenticator<'a> { credential_id, authenticator_data, signature: response.signature.into(), - user_handle: response.user.ok_or("Missing user")?.id.into(), + user_handle: response + .user + .ok_or(GetAssertionError::MissingUser)? + .id + .into(), selected_credential: self.get_selected_credential()?, }) } @@ -161,7 +219,7 @@ impl<'a> Fido2Authenticator<'a> { pub async fn silently_discover_credentials( &mut self, rp_id: String, - ) -> Result> { + ) -> Result, SilentlyDiscoverCredentialsError> { let enc = self.client.get_encryption_settings()?; let result = self.credential_store.find_credentials(None, rp_id).await?; @@ -198,7 +256,9 @@ impl<'a> Fido2Authenticator<'a> { } } - pub(super) fn get_selected_credential(&self) -> Result { + pub(super) fn get_selected_credential( + &self, + ) -> Result { let enc = self.client.get_encryption_settings()?; let cipher = self @@ -206,11 +266,14 @@ impl<'a> Fido2Authenticator<'a> { .lock() .expect("Mutex is not poisoned") .clone() - .ok_or("No selected credential available")?; + .ok_or(GetSelectedCredentialError::NoSelectedCredential)?; let creds = cipher.decrypt_fido2_credentials(&enc)?; - let credential = creds.first().ok_or("No Fido2 credentials found")?.clone(); + let credential = creds + .first() + .ok_or(GetSelectedCredentialError::NoCredentialFound)? + .clone(); Ok(SelectedCredential { cipher, credential }) } @@ -232,12 +295,24 @@ impl passkey::authenticator::CredentialStore for CredentialStoreImpl<'_> { ids: Option<&[passkey::types::webauthn::PublicKeyCredentialDescriptor]>, rp_id: &str, ) -> Result, StatusCode> { + #[derive(Debug, Error)] + enum InnerError { + #[error(transparent)] + VaultLocked(#[from] VaultLocked), + #[error(transparent)] + CipherError(#[from] CipherError), + #[error(transparent)] + CryptoError(#[from] CryptoError), + #[error(transparent)] + Fido2CallbackError(#[from] Fido2CallbackError), + } + // This is just a wrapper around the actual implementation to allow for ? error handling async fn inner( this: &CredentialStoreImpl<'_>, ids: Option<&[passkey::types::webauthn::PublicKeyCredentialDescriptor]>, rp_id: &str, - ) -> Result> { + ) -> Result, InnerError> { let ids: Option>> = ids.map(|ids| ids.iter().map(|id| id.id.clone().into()).collect()); @@ -262,10 +337,10 @@ impl passkey::authenticator::CredentialStore for CredentialStoreImpl<'_> { // When using the credential for authentication we have to ask the user to pick one. if this.create_credential { - creds + Ok(creds .into_iter() .map(|c| CipherViewContainer::new(c, &enc)) - .collect() + .collect::>()?) } else { let picked = this .authenticator @@ -299,13 +374,30 @@ impl passkey::authenticator::CredentialStore for CredentialStoreImpl<'_> { rp: passkey::types::ctap2::make_credential::PublicKeyCredentialRpEntity, _options: passkey::types::ctap2::get_assertion::Options, ) -> Result<(), StatusCode> { + #[derive(Debug, Error)] + enum InnerError { + #[error(transparent)] + VaultLocked(#[from] VaultLocked), + #[error(transparent)] + FillCredentialError(#[from] FillCredentialError), + #[error(transparent)] + CipherError(#[from] CipherError), + #[error(transparent)] + CryptoError(#[from] CryptoError), + #[error(transparent)] + Fido2CallbackError(#[from] Fido2CallbackError), + + #[error("No selected credential available")] + NoSelectedCredential, + } + // This is just a wrapper around the actual implementation to allow for ? error handling async fn inner( this: &mut CredentialStoreImpl<'_>, cred: Passkey, user: passkey::types::ctap2::make_credential::PublicKeyCredentialUserEntity, rp: passkey::types::ctap2::make_credential::PublicKeyCredentialRpEntity, - ) -> Result<()> { + ) -> Result<(), InnerError> { let enc = this.authenticator.client.get_encryption_settings()?; let cred = try_from_credential_full(cred, user, rp)?; @@ -317,7 +409,7 @@ impl passkey::authenticator::CredentialStore for CredentialStoreImpl<'_> { .lock() .expect("Mutex is not poisoned") .clone() - .ok_or("No selected cipher available")?; + .ok_or(InnerError::NoSelectedCredential)?; selected.set_new_fido2_credentials(&enc, vec![cred])?; @@ -349,8 +441,31 @@ impl passkey::authenticator::CredentialStore for CredentialStoreImpl<'_> { } async fn update_credential(&mut self, cred: Passkey) -> Result<(), StatusCode> { + #[derive(Debug, Error)] + enum InnerError { + #[error(transparent)] + VaultLocked(#[from] VaultLocked), + #[error(transparent)] + InvalidGuid(#[from] InvalidGuid), + #[error("Credential ID does not match selected credential")] + CredentialIdMismatch, + #[error(transparent)] + FillCredentialError(#[from] FillCredentialError), + #[error(transparent)] + CipherError(#[from] CipherError), + #[error(transparent)] + CryptoError(#[from] CryptoError), + #[error(transparent)] + Fido2CallbackError(#[from] Fido2CallbackError), + #[error(transparent)] + GetSelectedCredentialError(#[from] GetSelectedCredentialError), + } + // This is just a wrapper around the actual implementation to allow for ? error handling - async fn inner(this: &mut CredentialStoreImpl<'_>, cred: Passkey) -> Result<()> { + async fn inner( + this: &mut CredentialStoreImpl<'_>, + cred: Passkey, + ) -> Result<(), InnerError> { let enc = this.authenticator.client.get_encryption_settings()?; // Get the previously selected cipher and update the credential @@ -360,7 +475,7 @@ impl passkey::authenticator::CredentialStore for CredentialStoreImpl<'_> { let new_id: &Vec = &cred.credential_id; let selected_id = string_to_guid_bytes(&selected.credential.credential_id)?; if new_id != &selected_id { - return Err("Credential ID does not match selected credential".into()); + return Err(InnerError::CredentialIdMismatch); } let cred = fill_with_credential(&selected.credential, cred)?; diff --git a/crates/bitwarden/src/platform/fido2/client.rs b/crates/bitwarden/src/platform/fido2/client.rs index f2f5703cf..0000bc69c 100644 --- a/crates/bitwarden/src/platform/fido2/client.rs +++ b/crates/bitwarden/src/platform/fido2/client.rs @@ -1,6 +1,9 @@ +use passkey::client::WebauthnError; use reqwest::Url; +use thiserror::Error; use super::{ + authenticator::GetSelectedCredentialError, get_string_name_from_enum, types::{ AuthenticatorAssertionResponse, AuthenticatorAttestationResponse, ClientData, @@ -9,7 +12,29 @@ use super::{ Fido2Authenticator, PublicKeyCredentialAuthenticatorAssertionResponse, PublicKeyCredentialAuthenticatorAttestationResponse, }; -use crate::error::Result; + +#[derive(Debug, Error)] +#[error("Invalid origin: {0}")] +pub struct InvalidOriginError(String); + +#[derive(Debug, Error)] +pub enum Fido2ClientError { + #[error(transparent)] + InvalidOrigin(#[from] InvalidOriginError), + #[error(transparent)] + Serde(#[from] serde_json::Error), + #[error(transparent)] + GetSelectedCredentialError(#[from] GetSelectedCredentialError), + + #[error("Webauthn error: {0:?}")] + Webauthn(WebauthnError), +} + +impl From for Fido2ClientError { + fn from(e: WebauthnError) -> Self { + Self::Webauthn(e) + } +} pub struct Fido2Client<'a> { pub(crate) authenticator: Fido2Authenticator<'a>, @@ -21,8 +46,8 @@ impl<'a> Fido2Client<'a> { origin: String, request: String, client_data: ClientData, - ) -> Result { - let origin = Url::parse(&origin).map_err(|e| format!("Invalid origin: {}", e))?; + ) -> Result { + let origin = Url::parse(&origin).map_err(|e| InvalidOriginError(format!("{}", e)))?; let request: passkey::types::webauthn::CredentialCreationOptions = serde_json::from_str(&request)?; @@ -76,8 +101,8 @@ impl<'a> Fido2Client<'a> { origin: String, request: String, client_data: ClientData, - ) -> Result { - let origin = Url::parse(&origin).map_err(|e| format!("Invalid origin: {}", e))?; + ) -> Result { + let origin = Url::parse(&origin).map_err(|e| InvalidOriginError(format!("{}", e)))?; let request: passkey::types::webauthn::CredentialRequestOptions = serde_json::from_str(&request)?; diff --git a/crates/bitwarden/src/platform/fido2/crypto.rs b/crates/bitwarden/src/platform/fido2/crypto.rs index a7fb5cff1..bca8e2a92 100644 --- a/crates/bitwarden/src/platform/fido2/crypto.rs +++ b/crates/bitwarden/src/platform/fido2/crypto.rs @@ -1,24 +1,28 @@ -use coset::{ - iana::{self}, - CoseKey, -}; +use coset::{iana, CoseKey}; use p256::{pkcs8::EncodePrivateKey, SecretKey}; use passkey::authenticator::{private_key_from_cose_key, CoseKeyPair}; +use thiserror::Error; -use crate::error::{Error, Result}; +#[derive(Debug, Error)] +pub enum CoseKeyToPkcs8Error { + #[error("Failed to extract private key from cose_key")] + FailedToExtractPrivateKeyFromCoseKey, + #[error("Failed to convert P256 private key to PKC8")] + FailedToConvertP256PrivateKeyToPkcs8, +} -pub fn cose_key_to_pkcs8(cose_key: &CoseKey) -> Result> { +pub fn cose_key_to_pkcs8(cose_key: &CoseKey) -> Result, CoseKeyToPkcs8Error> { // cose_key. let secret_key = private_key_from_cose_key(cose_key).map_err(|error| { log::error!("Failed to extract private key from cose_key: {:?}", error); - Error::Internal("Failed to extract private key from cose_key".into()) + CoseKeyToPkcs8Error::FailedToExtractPrivateKeyFromCoseKey })?; let vec = secret_key .to_pkcs8_der() .map_err(|error| { log::error!("Failed to convert P256 private key to PKC8: {:?}", error); - Error::Internal("Failed to convert P256 private key to PKC8".into()) + CoseKeyToPkcs8Error::FailedToConvertP256PrivateKeyToPkcs8 })? .as_bytes() .to_vec(); @@ -26,10 +30,14 @@ pub fn cose_key_to_pkcs8(cose_key: &CoseKey) -> Result> { Ok(vec) } -pub fn pkcs8_to_cose_key(secret_key: &[u8]) -> Result { +#[derive(Debug, Error)] +#[error("Failed to extract private key from secret_key")] +pub struct PrivateKeyFromSecretKeyError; + +pub fn pkcs8_to_cose_key(secret_key: &[u8]) -> Result { let secret_key = SecretKey::from_slice(secret_key).map_err(|error| { log::error!("Failed to extract private key from secret_key: {:?}", error); - Error::Internal("Failed to extract private key from secret_key".into()) + PrivateKeyFromSecretKeyError })?; let cose_key_pair = CoseKeyPair::from_secret_key(&secret_key, iana::Algorithm::ES256); diff --git a/crates/bitwarden/src/platform/fido2/mod.rs b/crates/bitwarden/src/platform/fido2/mod.rs index 2eb80906e..da9d19a65 100644 --- a/crates/bitwarden/src/platform/fido2/mod.rs +++ b/crates/bitwarden/src/platform/fido2/mod.rs @@ -3,8 +3,9 @@ use std::sync::Mutex; use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine}; use bitwarden_crypto::KeyContainer; use bitwarden_vault::{ - CipherView, Fido2CredentialFullView, Fido2CredentialNewView, Fido2CredentialView, + CipherError, CipherView, Fido2CredentialFullView, Fido2CredentialNewView, Fido2CredentialView, }; +use crypto::{CoseKeyToPkcs8Error, PrivateKeyFromSecretKeyError}; use passkey::types::{ctap2::Aaguid, Passkey}; mod authenticator; @@ -13,9 +14,12 @@ mod crypto; mod traits; mod types; -pub use authenticator::Fido2Authenticator; -pub use client::Fido2Client; +pub use authenticator::{ + Fido2Authenticator, GetAssertionError, MakeCredentialError, SilentlyDiscoverCredentialsError, +}; +pub use client::{Fido2Client, Fido2ClientError}; pub use passkey::authenticator::UIHint; +use thiserror::Error; pub use traits::{ CheckUserOptions, CheckUserResult, Fido2CallbackError, Fido2CredentialStore, Fido2UserInterface, Verification, @@ -29,10 +33,7 @@ pub use types::{ }; use self::crypto::{cose_key_to_pkcs8, pkcs8_to_cose_key}; -use crate::{ - error::{Error, Result}, - Client, -}; +use crate::Client; // This is the AAGUID for the Bitwarden Passkey provider (d548826e-79b4-db40-a3d8-11116f7e8349) // It is used for the Relaying Parties to identify the authenticator during registration @@ -48,28 +49,26 @@ pub struct ClientFido2<'a> { impl<'a> ClientFido2<'a> { pub fn create_authenticator( &'a self, - user_interface: &'a dyn Fido2UserInterface, credential_store: &'a dyn Fido2CredentialStore, - ) -> Result> { - Ok(Fido2Authenticator { + ) -> Fido2Authenticator<'a> { + Fido2Authenticator { client: self.client, user_interface, credential_store, selected_cipher: Mutex::new(None), requested_uv: Mutex::new(None), - }) + } } pub fn create_client( &'a self, - user_interface: &'a dyn Fido2UserInterface, credential_store: &'a dyn Fido2CredentialStore, - ) -> Result> { - Ok(Fido2Client { - authenticator: self.create_authenticator(user_interface, credential_store)?, - }) + ) -> Fido2Client<'a> { + Fido2Client { + authenticator: self.create_authenticator(user_interface, credential_store), + } } } @@ -89,7 +88,7 @@ pub(crate) struct CipherViewContainer { } impl CipherViewContainer { - fn new(cipher: CipherView, enc: &dyn KeyContainer) -> Result { + fn new(cipher: CipherView, enc: &dyn KeyContainer) -> Result { let fido2_credentials = cipher.get_fido2_credentials(enc)?; Ok(Self { cipher, @@ -98,20 +97,35 @@ impl CipherViewContainer { } } +#[derive(Debug, Error)] +pub enum Fido2Error { + #[error(transparent)] + UnknownEnum(#[from] UnknownEnum), + + #[error(transparent)] + InvalidGuid(#[from] InvalidGuid), + + #[error(transparent)] + PrivateKeyFromSecretKeyError(#[from] PrivateKeyFromSecretKeyError), + + #[error("No Fido2 credentials found")] + NoFido2CredentialsFound, +} + impl TryFrom for Passkey { - type Error = crate::error::Error; + type Error = Fido2Error; fn try_from(value: CipherViewContainer) -> Result { let cred = value .fido2_credentials .first() - .ok_or(Error::Internal("No Fido2 credentials found".into()))?; + .ok_or(Fido2Error::NoFido2CredentialsFound)?; try_from_credential_full_view(cred.clone()) } } -fn try_from_credential_full_view(value: Fido2CredentialFullView) -> Result { +fn try_from_credential_full_view(value: Fido2CredentialFullView) -> Result { let counter: u32 = value.counter.parse().expect("Invalid counter"); let counter = (counter != 0).then_some(counter); @@ -126,10 +140,18 @@ fn try_from_credential_full_view(value: Fido2CredentialFullView) -> Result Result { +) -> Result { let cred_id: Vec = value.credential_id.into(); Ok(Fido2CredentialFullView { @@ -153,7 +175,7 @@ pub fn fill_with_credential( pub(crate) fn try_from_credential_new_view( user: &passkey::types::ctap2::make_credential::PublicKeyCredentialUserEntity, rp: &passkey::types::ctap2::make_credential::PublicKeyCredentialRpEntity, -) -> Result { +) -> Result { let cred_id: Vec = vec![0; 16]; Ok(Fido2CredentialNewView { @@ -177,7 +199,7 @@ pub(crate) fn try_from_credential_full( value: Passkey, user: passkey::types::ctap2::make_credential::PublicKeyCredentialUserEntity, rp: passkey::types::ctap2::make_credential::PublicKeyCredentialRpEntity, -) -> Result { +) -> Result { let cred_id: Vec = value.credential_id.into(); Ok(Fido2CredentialFullView { @@ -198,33 +220,47 @@ pub(crate) fn try_from_credential_full( }) } -pub fn guid_bytes_to_string(source: &[u8]) -> Result { +#[derive(Debug, Error)] +#[error("Input should be a 16 byte array")] +pub struct InvalidInputLength; + +pub fn guid_bytes_to_string(source: &[u8]) -> Result { if source.len() != 16 { - return Err(Error::Internal("Input should be a 16 byte array".into())); + return Err(InvalidInputLength); } Ok(uuid::Uuid::from_bytes(source.try_into().expect("Invalid length")).to_string()) } -pub fn string_to_guid_bytes(source: &str) -> Result> { +#[derive(Debug, Error)] +#[error("Invalid GUID")] +pub struct InvalidGuid; + +pub fn string_to_guid_bytes(source: &str) -> Result, InvalidGuid> { if source.starts_with("b64.") { - let bytes = URL_SAFE_NO_PAD.decode(source.trim_start_matches("b64."))?; + let bytes = URL_SAFE_NO_PAD + .decode(source.trim_start_matches("b64.")) + .map_err(|_| InvalidGuid)?; Ok(bytes) } else { let Ok(uuid) = uuid::Uuid::try_parse(source) else { - return Err(Error::Internal("Input should be a valid GUID".into())); + return Err(InvalidGuid); }; Ok(uuid.as_bytes().to_vec()) } } +#[derive(Debug, Error)] +#[error("Unknown enum value")] +pub struct UnknownEnum; + // Some utilities to convert back and forth between enums and strings -fn get_enum_from_string_name(s: &str) -> Result { +fn get_enum_from_string_name(s: &str) -> Result { let serialized = format!(r#""{}""#, s); - let deserialized: T = serde_json::from_str(&serialized)?; + let deserialized: T = serde_json::from_str(&serialized).map_err(|_| UnknownEnum)?; Ok(deserialized) } -fn get_string_name_from_enum(s: impl serde::Serialize) -> Result { +fn get_string_name_from_enum(s: impl serde::Serialize) -> Result { let serialized = serde_json::to_string(&s)?; let deserialized: String = serde_json::from_str(&serialized)?; Ok(deserialized) diff --git a/crates/bitwarden/src/platform/fido2/traits.rs b/crates/bitwarden/src/platform/fido2/traits.rs index f64bf100e..0769eeb2a 100644 --- a/crates/bitwarden/src/platform/fido2/traits.rs +++ b/crates/bitwarden/src/platform/fido2/traits.rs @@ -2,8 +2,6 @@ use bitwarden_vault::{Cipher, CipherView, Fido2CredentialNewView}; use passkey::authenticator::UIHint; use thiserror::Error; -use crate::error::Result; - #[derive(Debug, Error)] pub enum Fido2CallbackError { #[error("The operation requires user interaction")] diff --git a/crates/bitwarden/src/platform/fido2/types.rs b/crates/bitwarden/src/platform/fido2/types.rs index bee66dbe8..3e41768f6 100644 --- a/crates/bitwarden/src/platform/fido2/types.rs +++ b/crates/bitwarden/src/platform/fido2/types.rs @@ -1,7 +1,8 @@ use passkey::types::webauthn::UserVerificationRequirement; use serde::Serialize; +use thiserror::Error; -use super::{get_enum_from_string_name, SelectedCredential, Verification}; +use super::{get_enum_from_string_name, SelectedCredential, UnknownEnum, Verification}; #[cfg_attr(feature = "uniffi", derive(uniffi::Record))] pub struct PublicKeyCredentialRpEntity { @@ -22,16 +23,26 @@ pub struct PublicKeyCredentialParameters { pub alg: i64, } +#[derive(Debug, Error)] +pub enum PublicKeyCredentialParametersError { + #[error("Invalid algorithm")] + InvalidAlgorithm, + + #[error("Unknown type")] + UnknownEnum(#[from] UnknownEnum), +} + impl TryFrom for passkey::types::webauthn::PublicKeyCredentialParameters { - type Error = crate::error::Error; + type Error = PublicKeyCredentialParametersError; fn try_from(value: PublicKeyCredentialParameters) -> Result { use coset::iana::EnumI64; Ok(Self { ty: get_enum_from_string_name(&value.ty)?, - alg: coset::iana::Algorithm::from_i64(value.alg).ok_or("Invalid algorithm")?, + alg: coset::iana::Algorithm::from_i64(value.alg) + .ok_or(PublicKeyCredentialParametersError::InvalidAlgorithm)?, }) } } @@ -46,7 +57,7 @@ pub struct PublicKeyCredentialDescriptor { impl TryFrom for passkey::types::webauthn::PublicKeyCredentialDescriptor { - type Error = crate::error::Error; + type Error = UnknownEnum; fn try_from(value: PublicKeyCredentialDescriptor) -> Result { Ok(Self {