From dfd0ba8d3d796021a5ac9fb46e2d86d048188add Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa?= Date: Tue, 11 Jun 2024 11:27:34 +0200 Subject: [PATCH 1/6] Fix save_credential failing when the selected cipher doesn't have a Passkey (#831) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 🎟ī¸ Tracking ## 📔 Objective The current call to `get_selected_credential` inside `save_credential` will try to fetch and decrypt the ciphers FIDO2 credentials, and error if they are not there. This can only happen when creating a new Passkey, so instead of calling `get_selected_credential` we just get the value from the lock. The other places where `get_selected_credential` is used is in `update_credential` and right at the end of the `assertion`, `register` and `authenticate` operations, so those should be safe. ## ⏰ 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 --------- Co-authored-by: Andreas Coroiu --- crates/bitwarden/src/platform/fido2/authenticator.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/bitwarden/src/platform/fido2/authenticator.rs b/crates/bitwarden/src/platform/fido2/authenticator.rs index c310cf071..7ea6ada4b 100644 --- a/crates/bitwarden/src/platform/fido2/authenticator.rs +++ b/crates/bitwarden/src/platform/fido2/authenticator.rs @@ -311,7 +311,14 @@ impl passkey::authenticator::CredentialStore for CredentialStoreImpl<'_> { let cred = try_from_credential_full(cred, user, rp)?; // Get the previously selected cipher and add the new credential to it - let mut selected: CipherView = this.authenticator.get_selected_credential()?.cipher; + let mut selected: CipherView = this + .authenticator + .selected_credential + .lock() + .expect("Mutex is not poisoned") + .clone() + .ok_or("No selected cipher available")?; + selected.set_new_fido2_credentials(enc, vec![cred])?; // Store the updated credential for later use From 47f4b1c9f74e96e7a69655476aeded2e799f1886 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa?= Date: Tue, 11 Jun 2024 12:20:59 +0200 Subject: [PATCH 2/6] Rename selected_credential to selected_cipher (#832) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 📔 Objective `selected_credential` is a misleading name as the selected value doesn't have to contain credentials, for example when creating a new passkey a cipher will usually not contain any credentials. Renaming it to `selected_cipher` should help clear the confusion a bit. I've left the `get_selected_credential` function with the same name, as in this case we are actually getting the credential inside the cipher. ## ⏰ 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 --- .../bitwarden/src/platform/fido2/authenticator.rs | 14 +++++++------- crates/bitwarden/src/platform/fido2/mod.rs | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/bitwarden/src/platform/fido2/authenticator.rs b/crates/bitwarden/src/platform/fido2/authenticator.rs index 7ea6ada4b..797ba87a1 100644 --- a/crates/bitwarden/src/platform/fido2/authenticator.rs +++ b/crates/bitwarden/src/platform/fido2/authenticator.rs @@ -27,7 +27,7 @@ pub struct Fido2Authenticator<'a> { pub(crate) user_interface: &'a dyn Fido2UserInterface, pub(crate) credential_store: &'a dyn Fido2CredentialStore, - pub(crate) selected_credential: Mutex>, + pub(crate) selected_cipher: Mutex>, pub(crate) requested_uv: Mutex>, } @@ -202,7 +202,7 @@ impl<'a> Fido2Authenticator<'a> { let enc = self.client.get_encryption_settings()?; let cipher = self - .selected_credential + .selected_cipher .lock() .expect("Mutex is not poisoned") .clone() @@ -275,7 +275,7 @@ impl passkey::authenticator::CredentialStore for CredentialStoreImpl<'_> { // Store the selected credential for later use this.authenticator - .selected_credential + .selected_cipher .lock() .expect("Mutex is not poisoned") .replace(picked.clone()); @@ -313,7 +313,7 @@ impl passkey::authenticator::CredentialStore for CredentialStoreImpl<'_> { // Get the previously selected cipher and add the new credential to it let mut selected: CipherView = this .authenticator - .selected_credential + .selected_cipher .lock() .expect("Mutex is not poisoned") .clone() @@ -323,7 +323,7 @@ impl passkey::authenticator::CredentialStore for CredentialStoreImpl<'_> { // Store the updated credential for later use this.authenticator - .selected_credential + .selected_cipher .lock() .expect("Mutex is not poisoned") .replace(selected.clone()); @@ -370,7 +370,7 @@ impl passkey::authenticator::CredentialStore for CredentialStoreImpl<'_> { // Store the updated credential for later use this.authenticator - .selected_credential + .selected_cipher .lock() .expect("Mutex is not poisoned") .replace(selected.clone()); @@ -437,7 +437,7 @@ impl passkey::authenticator::UserValidationMethod for UserValidationMethodImpl<' .map_err(|_| Ctap2Error::OperationDenied)?; self.authenticator - .selected_credential + .selected_cipher .lock() .expect("Mutex is not poisoned") .replace(cipher_view); diff --git a/crates/bitwarden/src/platform/fido2/mod.rs b/crates/bitwarden/src/platform/fido2/mod.rs index d4bc32a52..b523cf7e5 100644 --- a/crates/bitwarden/src/platform/fido2/mod.rs +++ b/crates/bitwarden/src/platform/fido2/mod.rs @@ -56,7 +56,7 @@ impl<'a> ClientFido2<'a> { client: self.client, user_interface, credential_store, - selected_credential: Mutex::new(None), + selected_cipher: Mutex::new(None), requested_uv: Mutex::new(None), }) } From 3fc7fd27b15ead4c2d41fea711194d1353e2eb49 Mon Sep 17 00:00:00 2001 From: Oscar Hinton Date: Tue, 11 Jun 2024 16:17:57 +0200 Subject: [PATCH 3/6] Flip the dependency between exporters and vault (#833) In preparation for #798 we need to flip the relationship between vault and exporters. Due to exporters in the future getting a `client_exporters` which means they need to be able to access the vault models to properly model it. --- Cargo.lock | 3 +- crates/bitwarden-exporters/Cargo.toml | 2 + crates/bitwarden-exporters/src/lib.rs | 2 +- .../src/models.rs} | 47 +++++++++---------- crates/bitwarden-vault/Cargo.toml | 1 - crates/bitwarden-vault/src/cipher/field.rs | 8 ++-- crates/bitwarden-vault/src/cipher/mod.rs | 2 +- .../bitwarden-vault/src/cipher/secure_note.rs | 2 +- crates/bitwarden-vault/src/lib.rs | 1 - 9 files changed, 34 insertions(+), 34 deletions(-) rename crates/{bitwarden-vault/src/exporters.rs => bitwarden-exporters/src/models.rs} (80%) diff --git a/Cargo.lock b/Cargo.lock index 5755f5cd5..480a1d3bd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -493,7 +493,9 @@ name = "bitwarden-exporters" version = "0.5.0" dependencies = [ "base64 0.22.1", + "bitwarden-core", "bitwarden-crypto", + "bitwarden-vault", "chrono", "csv", "serde", @@ -584,7 +586,6 @@ dependencies = [ "bitwarden-api-api", "bitwarden-core", "bitwarden-crypto", - "bitwarden-exporters", "chrono", "hmac", "rand", diff --git a/crates/bitwarden-exporters/Cargo.toml b/crates/bitwarden-exporters/Cargo.toml index b607879ba..ac302526c 100644 --- a/crates/bitwarden-exporters/Cargo.toml +++ b/crates/bitwarden-exporters/Cargo.toml @@ -16,7 +16,9 @@ keywords.workspace = true [dependencies] base64 = ">=0.21.2, <0.23" +bitwarden-core = { workspace = true } bitwarden-crypto = { workspace = true } +bitwarden-vault = { workspace = true } chrono = { version = ">=0.4.26, <0.5", features = [ "clock", "serde", diff --git a/crates/bitwarden-exporters/src/lib.rs b/crates/bitwarden-exporters/src/lib.rs index 762556388..e754a64d1 100644 --- a/crates/bitwarden-exporters/src/lib.rs +++ b/crates/bitwarden-exporters/src/lib.rs @@ -10,8 +10,8 @@ use crate::csv::export_csv; mod json; use json::export_json; mod encrypted_json; - use encrypted_json::export_encrypted_json; +mod models; pub enum Format { Csv, diff --git a/crates/bitwarden-vault/src/exporters.rs b/crates/bitwarden-exporters/src/models.rs similarity index 80% rename from crates/bitwarden-vault/src/exporters.rs rename to crates/bitwarden-exporters/src/models.rs index 3b228f231..1d56e7233 100644 --- a/crates/bitwarden-vault/src/exporters.rs +++ b/crates/bitwarden-exporters/src/models.rs @@ -1,8 +1,9 @@ use bitwarden_core::{require, MissingFieldError}; +use bitwarden_vault::{ + CipherType, CipherView, FieldView, FolderView, LoginUriView, SecureNoteType, +}; -use crate::{login::LoginUriView, CipherType, CipherView, FieldView, FolderView, SecureNoteType}; - -impl TryFrom for bitwarden_exporters::Folder { +impl TryFrom for crate::Folder { type Error = MissingFieldError; fn try_from(value: FolderView) -> Result { @@ -13,14 +14,14 @@ impl TryFrom for bitwarden_exporters::Folder { } } -impl TryFrom for bitwarden_exporters::Cipher { +impl TryFrom for crate::Cipher { type Error = MissingFieldError; fn try_from(value: CipherView) -> Result { let r = match value.r#type { CipherType::Login => { let l = require!(value.login); - bitwarden_exporters::CipherType::Login(Box::new(bitwarden_exporters::Login { + crate::CipherType::Login(Box::new(crate::Login { username: l.username, password: l.password, login_uris: l @@ -32,18 +33,16 @@ impl TryFrom for bitwarden_exporters::Cipher { totp: l.totp, })) } - CipherType::SecureNote => bitwarden_exporters::CipherType::SecureNote(Box::new( - bitwarden_exporters::SecureNote { - r#type: value - .secure_note - .map(|t| t.r#type) - .unwrap_or(SecureNoteType::Generic) - .into(), - }, - )), + CipherType::SecureNote => crate::CipherType::SecureNote(Box::new(crate::SecureNote { + r#type: value + .secure_note + .map(|t| t.r#type) + .unwrap_or(SecureNoteType::Generic) + .into(), + })), CipherType::Card => { let c = require!(value.card); - bitwarden_exporters::CipherType::Card(Box::new(bitwarden_exporters::Card { + crate::CipherType::Card(Box::new(crate::Card { cardholder_name: c.cardholder_name, exp_month: c.exp_month, exp_year: c.exp_year, @@ -54,7 +53,7 @@ impl TryFrom for bitwarden_exporters::Cipher { } CipherType::Identity => { let i = require!(value.identity); - bitwarden_exporters::CipherType::Identity(Box::new(bitwarden_exporters::Identity { + crate::CipherType::Identity(Box::new(crate::Identity { title: i.title, first_name: i.first_name, middle_name: i.middle_name, @@ -98,7 +97,7 @@ impl TryFrom for bitwarden_exporters::Cipher { } } -impl From for bitwarden_exporters::Field { +impl From for crate::Field { fn from(value: FieldView) -> Self { Self { name: value.name, @@ -109,7 +108,7 @@ impl From for bitwarden_exporters::Field { } } -impl From for bitwarden_exporters::LoginUri { +impl From for crate::LoginUri { fn from(value: LoginUriView) -> Self { Self { r#match: value.r#match.map(|v| v as u8), @@ -118,20 +117,20 @@ impl From for bitwarden_exporters::LoginUri { } } -impl From for bitwarden_exporters::SecureNoteType { +impl From for crate::SecureNoteType { fn from(value: SecureNoteType) -> Self { match value { - SecureNoteType::Generic => bitwarden_exporters::SecureNoteType::Generic, + SecureNoteType::Generic => crate::SecureNoteType::Generic, } } } #[cfg(test)] mod tests { + use bitwarden_vault::{CipherRepromptType, LoginView}; use chrono::{DateTime, Utc}; use super::*; - use crate::{CipherRepromptType, LoginView}; #[test] fn test_try_from_folder_view() { @@ -141,7 +140,7 @@ mod tests { revision_date: "2024-01-30T17:55:36.150Z".parse().unwrap(), }; - let f: bitwarden_exporters::Folder = view.try_into().unwrap(); + let f: crate::Folder = view.try_into().unwrap(); assert_eq!( f.id, @@ -187,7 +186,7 @@ mod tests { revision_date: "2024-01-30T17:55:36.150Z".parse().unwrap(), }; - let cipher: bitwarden_exporters::Cipher = cipher_view.try_into().unwrap(); + let cipher: crate::Cipher = cipher_view.try_into().unwrap(); assert_eq!( cipher.id, @@ -209,7 +208,7 @@ mod tests { ); assert_eq!(cipher.deleted_date, None); - if let bitwarden_exporters::CipherType::Login(l) = cipher.r#type { + if let crate::CipherType::Login(l) = cipher.r#type { assert_eq!(l.username, Some("test_username".to_string())); assert_eq!(l.password, Some("test_password".to_string())); assert!(l.login_uris.is_empty()); diff --git a/crates/bitwarden-vault/Cargo.toml b/crates/bitwarden-vault/Cargo.toml index 379b23997..f14f090d9 100644 --- a/crates/bitwarden-vault/Cargo.toml +++ b/crates/bitwarden-vault/Cargo.toml @@ -25,7 +25,6 @@ base64 = ">=0.21.2, <0.23" bitwarden-api-api = { workspace = true } bitwarden-core = { workspace = true } bitwarden-crypto = { workspace = true } -bitwarden-exporters = { workspace = true } chrono = { version = ">=0.4.26, <0.5", default-features = false } rand = ">=0.8.5, <0.9" hmac = ">=0.12.1, <0.13" diff --git a/crates/bitwarden-vault/src/cipher/field.rs b/crates/bitwarden-vault/src/cipher/field.rs index 2141006af..6c826d4ba 100644 --- a/crates/bitwarden-vault/src/cipher/field.rs +++ b/crates/bitwarden-vault/src/cipher/field.rs @@ -35,11 +35,11 @@ pub struct Field { #[serde(rename_all = "camelCase", deny_unknown_fields)] #[cfg_attr(feature = "uniffi", derive(uniffi::Record))] pub struct FieldView { - pub(crate) name: Option, - pub(crate) value: Option, - pub(crate) r#type: FieldType, + pub name: Option, + pub value: Option, + pub r#type: FieldType, - pub(crate) linked_id: Option, + pub linked_id: Option, } impl KeyEncryptable for FieldView { diff --git a/crates/bitwarden-vault/src/cipher/mod.rs b/crates/bitwarden-vault/src/cipher/mod.rs index 1b13ac743..67513d524 100644 --- a/crates/bitwarden-vault/src/cipher/mod.rs +++ b/crates/bitwarden-vault/src/cipher/mod.rs @@ -16,6 +16,6 @@ pub use cipher::{Cipher, CipherError, CipherListView, CipherRepromptType, Cipher pub use field::FieldView; pub use login::{ Fido2Credential, Fido2CredentialFullView, Fido2CredentialNewView, Fido2CredentialView, Login, - LoginView, + LoginUriView, LoginView, }; pub use secure_note::SecureNoteType; diff --git a/crates/bitwarden-vault/src/cipher/secure_note.rs b/crates/bitwarden-vault/src/cipher/secure_note.rs index 8ae39eb4d..2563160bb 100644 --- a/crates/bitwarden-vault/src/cipher/secure_note.rs +++ b/crates/bitwarden-vault/src/cipher/secure_note.rs @@ -25,7 +25,7 @@ pub struct SecureNote { #[serde(rename_all = "camelCase", deny_unknown_fields)] #[cfg_attr(feature = "uniffi", derive(uniffi::Record))] pub struct SecureNoteView { - pub(crate) r#type: SecureNoteType, + pub r#type: SecureNoteType, } impl KeyEncryptable for SecureNoteView { diff --git a/crates/bitwarden-vault/src/lib.rs b/crates/bitwarden-vault/src/lib.rs index 4928f2766..1e5b5c5aa 100644 --- a/crates/bitwarden-vault/src/lib.rs +++ b/crates/bitwarden-vault/src/lib.rs @@ -17,4 +17,3 @@ mod totp; pub use totp::{generate_totp, TotpError, TotpResponse}; mod error; pub use error::VaultParseError; -mod exporters; From 3f56e5850a39ad53a885ec8ac92fcc08d3635cf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa?= Date: Tue, 11 Jun 2024 16:21:13 +0200 Subject: [PATCH 4/6] Remove unnecessary into() (#834) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 📔 Objective Clippy was complaining in my local build that this into was unnecessary, and for some reason this wasn't caught by CI ## ⏰ 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 --- crates/bitwarden-vault/src/cipher/linked_id.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bitwarden-vault/src/cipher/linked_id.rs b/crates/bitwarden-vault/src/cipher/linked_id.rs index 7ee29dbc2..9e7cd91cc 100644 --- a/crates/bitwarden-vault/src/cipher/linked_id.rs +++ b/crates/bitwarden-vault/src/cipher/linked_id.rs @@ -112,7 +112,7 @@ impl TryFrom for LinkedIdType { 416 => Ok(LinkedIdType::Identity(IdentityLinkedIdType::FirstName)), 417 => Ok(LinkedIdType::Identity(IdentityLinkedIdType::LastName)), 418 => Ok(LinkedIdType::Identity(IdentityLinkedIdType::FullName)), - _ => Err(MissingFieldError("LinkedIdType").into()), + _ => Err(MissingFieldError("LinkedIdType")), } } } From 6c18348b83c5a685132e00b39595f2684bed210e Mon Sep 17 00:00:00 2001 From: Oscar Hinton Date: Thu, 13 Jun 2024 13:59:38 +0200 Subject: [PATCH 5/6] Deny unused async (#838) We currently use async on several methods that don't actually require it. This PR enables the clippy rule which will warn if any async is unnecessary. It also resolves all existing usages. --- Cargo.toml | 1 + crates/bitwarden-uniffi/src/auth/mod.rs | 2 - crates/bitwarden-uniffi/src/crypto.rs | 8 +-- crates/bitwarden-uniffi/src/tool/mod.rs | 24 ++------ crates/bitwarden-uniffi/src/tool/sends.rs | 46 +++++---------- .../bitwarden-uniffi/src/vault/attachments.rs | 46 +++++---------- crates/bitwarden-uniffi/src/vault/ciphers.rs | 19 ++----- .../bitwarden-uniffi/src/vault/collections.rs | 6 +- crates/bitwarden-uniffi/src/vault/folders.rs | 23 +------- .../src/vault/password_history.rs | 6 +- crates/bitwarden/src/auth/client_auth.rs | 4 +- crates/bitwarden/src/mobile/client_crypto.rs | 9 +-- .../bitwarden/src/mobile/tool/client_sends.rs | 18 +++--- .../src/mobile/vault/client_attachments.rs | 12 ++-- .../src/mobile/vault/client_ciphers.rs | 57 ++++++------------- .../src/mobile/vault/client_collection.rs | 8 +-- .../src/mobile/vault/client_folders.rs | 6 +- .../mobile/vault/client_password_history.rs | 7 +-- crates/bitwarden/src/tool/client_generator.rs | 8 +-- .../src/tool/exporters/client_exporter.rs | 4 +- crates/bw/src/main.rs | 34 +++++------ 21 files changed, 114 insertions(+), 234 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3f826b761..be4ab72c4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,6 +28,7 @@ bitwarden-generators = { path = "crates/bitwarden-generators", version = "=0.5.0 bitwarden-vault = { path = "crates/bitwarden-vault", version = "=0.5.0" } [workspace.lints.clippy] +unused_async = "deny" unwrap_used = "deny" # Compile all dependencies with some optimizations when building this crate on debug diff --git a/crates/bitwarden-uniffi/src/auth/mod.rs b/crates/bitwarden-uniffi/src/auth/mod.rs index 6c18bfc61..1065a01a9 100644 --- a/crates/bitwarden-uniffi/src/auth/mod.rs +++ b/crates/bitwarden-uniffi/src/auth/mod.rs @@ -26,7 +26,6 @@ impl ClientAuth { .await .auth() .password_strength(password, email, additional_inputs) - .await } /// Evaluate if the provided password satisfies the provided policy @@ -42,7 +41,6 @@ impl ClientAuth { .await .auth() .satisfies_policy(password, strength, &policy) - .await } /// Hash the user password diff --git a/crates/bitwarden-uniffi/src/crypto.rs b/crates/bitwarden-uniffi/src/crypto.rs index 0f877d52e..991ff820f 100644 --- a/crates/bitwarden-uniffi/src/crypto.rs +++ b/crates/bitwarden-uniffi/src/crypto.rs @@ -60,15 +60,14 @@ impl ClientCrypto { .write() .await .crypto() - .update_password(new_password) - .await?) + .update_password(new_password)?) } /// Generates a PIN protected user key from the provided PIN. The result can be stored and later /// used to initialize another client instance by using the PIN and the PIN key with /// `initialize_user_crypto`. pub async fn derive_pin_key(&self, pin: String) -> Result { - Ok(self.0 .0.write().await.crypto().derive_pin_key(pin).await?) + Ok(self.0 .0.write().await.crypto().derive_pin_key(pin)?) } /// Derives the pin protected user key from encrypted pin. Used when pin requires master @@ -80,8 +79,7 @@ impl ClientCrypto { .write() .await .crypto() - .derive_pin_user_key(encrypted_pin) - .await?) + .derive_pin_user_key(encrypted_pin)?) } pub async fn enroll_admin_password_reset( diff --git a/crates/bitwarden-uniffi/src/tool/mod.rs b/crates/bitwarden-uniffi/src/tool/mod.rs index 3ad4f7f93..78781b263 100644 --- a/crates/bitwarden-uniffi/src/tool/mod.rs +++ b/crates/bitwarden-uniffi/src/tool/mod.rs @@ -18,26 +18,12 @@ pub struct ClientGenerators(pub(crate) Arc); impl ClientGenerators { /// **API Draft:** Generate Password pub async fn password(&self, settings: PasswordGeneratorRequest) -> Result { - Ok(self - .0 - .0 - .read() - .await - .generator() - .password(settings) - .await?) + Ok(self.0 .0.read().await.generator().password(settings)?) } /// **API Draft:** Generate Passphrase pub async fn passphrase(&self, settings: PassphraseGeneratorRequest) -> Result { - Ok(self - .0 - .0 - .read() - .await - .generator() - .passphrase(settings) - .await?) + Ok(self.0 .0.read().await.generator().passphrase(settings)?) } /// **API Draft:** Generate Username @@ -71,8 +57,7 @@ impl ClientExporters { .read() .await .exporters() - .export_vault(folders, ciphers, format) - .await?) + .export_vault(folders, ciphers, format)?) } /// **API Draft:** Export organization vault @@ -88,7 +73,6 @@ impl ClientExporters { .read() .await .exporters() - .export_organization_vault(collections, ciphers, format) - .await?) + .export_organization_vault(collections, ciphers, format)?) } } diff --git a/crates/bitwarden-uniffi/src/tool/sends.rs b/crates/bitwarden-uniffi/src/tool/sends.rs index e6aef3e51..36f174d38 100644 --- a/crates/bitwarden-uniffi/src/tool/sends.rs +++ b/crates/bitwarden-uniffi/src/tool/sends.rs @@ -11,7 +11,7 @@ pub struct ClientSends(pub Arc); impl ClientSends { /// Encrypt send pub async fn encrypt(&self, send: SendView) -> Result { - Ok(self.0 .0.write().await.sends().encrypt(send).await?) + Ok(self.0 .0.write().await.sends().encrypt(send)?) } /// Encrypt a send file in memory @@ -22,8 +22,7 @@ impl ClientSends { .write() .await .sends() - .encrypt_buffer(send, &buffer) - .await?) + .encrypt_buffer(send, &buffer)?) } /// Encrypt a send file located in the file system @@ -33,28 +32,21 @@ impl ClientSends { decrypted_file_path: String, encrypted_file_path: String, ) -> Result<()> { - Ok(self - .0 - .0 - .write() - .await - .sends() - .encrypt_file( - send, - Path::new(&decrypted_file_path), - Path::new(&encrypted_file_path), - ) - .await?) + Ok(self.0 .0.write().await.sends().encrypt_file( + send, + Path::new(&decrypted_file_path), + Path::new(&encrypted_file_path), + )?) } /// Decrypt send pub async fn decrypt(&self, send: Send) -> Result { - Ok(self.0 .0.write().await.sends().decrypt(send).await?) + Ok(self.0 .0.write().await.sends().decrypt(send)?) } /// Decrypt send list pub async fn decrypt_list(&self, sends: Vec) -> Result> { - Ok(self.0 .0.write().await.sends().decrypt_list(sends).await?) + Ok(self.0 .0.write().await.sends().decrypt_list(sends)?) } /// Decrypt a send file in memory @@ -65,8 +57,7 @@ impl ClientSends { .write() .await .sends() - .decrypt_buffer(send, &buffer) - .await?) + .decrypt_buffer(send, &buffer)?) } /// Decrypt a send file located in the file system @@ -76,17 +67,10 @@ impl ClientSends { encrypted_file_path: String, decrypted_file_path: String, ) -> Result<()> { - Ok(self - .0 - .0 - .write() - .await - .sends() - .decrypt_file( - send, - Path::new(&encrypted_file_path), - Path::new(&decrypted_file_path), - ) - .await?) + Ok(self.0 .0.write().await.sends().decrypt_file( + send, + Path::new(&encrypted_file_path), + Path::new(&decrypted_file_path), + )?) } } diff --git a/crates/bitwarden-uniffi/src/vault/attachments.rs b/crates/bitwarden-uniffi/src/vault/attachments.rs index b844c0318..bdac7c97c 100644 --- a/crates/bitwarden-uniffi/src/vault/attachments.rs +++ b/crates/bitwarden-uniffi/src/vault/attachments.rs @@ -23,8 +23,7 @@ impl ClientAttachments { .await .vault() .attachments() - .encrypt_buffer(cipher, attachment, &buffer) - .await?) + .encrypt_buffer(cipher, attachment, &buffer)?) } /// Encrypt an attachment file located in the file system @@ -35,20 +34,12 @@ impl ClientAttachments { decrypted_file_path: String, encrypted_file_path: String, ) -> Result { - Ok(self - .0 - .0 - .write() - .await - .vault() - .attachments() - .encrypt_file( - cipher, - attachment, - Path::new(&decrypted_file_path), - Path::new(&encrypted_file_path), - ) - .await?) + Ok(self.0 .0.write().await.vault().attachments().encrypt_file( + cipher, + attachment, + Path::new(&decrypted_file_path), + Path::new(&encrypted_file_path), + )?) } /// Decrypt an attachment file in memory pub async fn decrypt_buffer( @@ -64,8 +55,7 @@ impl ClientAttachments { .await .vault() .attachments() - .decrypt_buffer(cipher, attachment, &buffer) - .await?) + .decrypt_buffer(cipher, attachment, &buffer)?) } /// Decrypt an attachment file located in the file system @@ -76,19 +66,11 @@ impl ClientAttachments { encrypted_file_path: String, decrypted_file_path: String, ) -> Result<()> { - Ok(self - .0 - .0 - .write() - .await - .vault() - .attachments() - .decrypt_file( - cipher, - attachment, - Path::new(&encrypted_file_path), - Path::new(&decrypted_file_path), - ) - .await?) + Ok(self.0 .0.write().await.vault().attachments().decrypt_file( + cipher, + attachment, + Path::new(&encrypted_file_path), + Path::new(&decrypted_file_path), + )?) } } diff --git a/crates/bitwarden-uniffi/src/vault/ciphers.rs b/crates/bitwarden-uniffi/src/vault/ciphers.rs index 329ca3c17..d637bc5fc 100644 --- a/crates/bitwarden-uniffi/src/vault/ciphers.rs +++ b/crates/bitwarden-uniffi/src/vault/ciphers.rs @@ -19,21 +19,12 @@ impl ClientCiphers { .await .vault() .ciphers() - .encrypt(cipher_view) - .await?) + .encrypt(cipher_view)?) } /// Decrypt cipher pub async fn decrypt(&self, cipher: Cipher) -> Result { - Ok(self - .0 - .0 - .write() - .await - .vault() - .ciphers() - .decrypt(cipher) - .await?) + Ok(self.0 .0.write().await.vault().ciphers().decrypt(cipher)?) } /// Decrypt cipher list @@ -45,8 +36,7 @@ impl ClientCiphers { .await .vault() .ciphers() - .decrypt_list(ciphers) - .await?) + .decrypt_list(ciphers)?) } /// Move a cipher to an organization, reencrypting the cipher key if necessary @@ -62,7 +52,6 @@ impl ClientCiphers { .await .vault() .ciphers() - .move_to_organization(cipher, organization_id) - .await?) + .move_to_organization(cipher, organization_id)?) } } diff --git a/crates/bitwarden-uniffi/src/vault/collections.rs b/crates/bitwarden-uniffi/src/vault/collections.rs index 36315aaea..f6cde84ab 100644 --- a/crates/bitwarden-uniffi/src/vault/collections.rs +++ b/crates/bitwarden-uniffi/src/vault/collections.rs @@ -18,8 +18,7 @@ impl ClientCollections { .await .vault() .collections() - .decrypt(collection) - .await?) + .decrypt(collection)?) } /// Decrypt collection list @@ -31,7 +30,6 @@ impl ClientCollections { .await .vault() .collections() - .decrypt_list(collections) - .await?) + .decrypt_list(collections)?) } } diff --git a/crates/bitwarden-uniffi/src/vault/folders.rs b/crates/bitwarden-uniffi/src/vault/folders.rs index 8847b9a45..5cceeca8b 100644 --- a/crates/bitwarden-uniffi/src/vault/folders.rs +++ b/crates/bitwarden-uniffi/src/vault/folders.rs @@ -11,28 +11,12 @@ pub struct ClientFolders(pub Arc); impl ClientFolders { /// Encrypt folder pub async fn encrypt(&self, folder: FolderView) -> Result { - Ok(self - .0 - .0 - .write() - .await - .vault() - .folders() - .encrypt(folder) - .await?) + Ok(self.0 .0.write().await.vault().folders().encrypt(folder)?) } /// Decrypt folder pub async fn decrypt(&self, folder: Folder) -> Result { - Ok(self - .0 - .0 - .write() - .await - .vault() - .folders() - .decrypt(folder) - .await?) + Ok(self.0 .0.write().await.vault().folders().decrypt(folder)?) } /// Decrypt folder list @@ -44,7 +28,6 @@ impl ClientFolders { .await .vault() .folders() - .decrypt_list(folders) - .await?) + .decrypt_list(folders)?) } } diff --git a/crates/bitwarden-uniffi/src/vault/password_history.rs b/crates/bitwarden-uniffi/src/vault/password_history.rs index 62c468d4b..863eddbac 100644 --- a/crates/bitwarden-uniffi/src/vault/password_history.rs +++ b/crates/bitwarden-uniffi/src/vault/password_history.rs @@ -18,8 +18,7 @@ impl ClientPasswordHistory { .await .vault() .password_history() - .encrypt(password_history) - .await?) + .encrypt(password_history)?) } /// Decrypt password history @@ -34,7 +33,6 @@ impl ClientPasswordHistory { .await .vault() .password_history() - .decrypt_list(list) - .await?) + .decrypt_list(list)?) } } diff --git a/crates/bitwarden/src/auth/client_auth.rs b/crates/bitwarden/src/auth/client_auth.rs index 0339c5351..14261efee 100644 --- a/crates/bitwarden/src/auth/client_auth.rs +++ b/crates/bitwarden/src/auth/client_auth.rs @@ -46,7 +46,7 @@ impl<'a> ClientAuth<'a> { #[cfg(feature = "internal")] impl<'a> ClientAuth<'a> { - pub async fn password_strength( + pub fn password_strength( &self, password: String, email: String, @@ -55,7 +55,7 @@ impl<'a> ClientAuth<'a> { password_strength(password, email, additional_inputs) } - pub async fn satisfies_policy( + pub fn satisfies_policy( &self, password: String, strength: u8, diff --git a/crates/bitwarden/src/mobile/client_crypto.rs b/crates/bitwarden/src/mobile/client_crypto.rs index 6ef65975d..30e88332a 100644 --- a/crates/bitwarden/src/mobile/client_crypto.rs +++ b/crates/bitwarden/src/mobile/client_crypto.rs @@ -33,20 +33,17 @@ impl<'a> ClientCrypto<'a> { } #[cfg(feature = "internal")] - pub async fn update_password( - &mut self, - new_password: String, - ) -> Result { + pub fn update_password(&mut self, new_password: String) -> Result { update_password(self.client, new_password) } #[cfg(feature = "internal")] - pub async fn derive_pin_key(&mut self, pin: String) -> Result { + pub fn derive_pin_key(&mut self, pin: String) -> Result { derive_pin_key(self.client, pin) } #[cfg(feature = "internal")] - pub async fn derive_pin_user_key(&mut self, encrypted_pin: EncString) -> Result { + pub fn derive_pin_user_key(&mut self, encrypted_pin: EncString) -> Result { derive_pin_user_key(self.client, encrypted_pin) } diff --git a/crates/bitwarden/src/mobile/tool/client_sends.rs b/crates/bitwarden/src/mobile/tool/client_sends.rs index 83652c8c4..9feeec030 100644 --- a/crates/bitwarden/src/mobile/tool/client_sends.rs +++ b/crates/bitwarden/src/mobile/tool/client_sends.rs @@ -14,7 +14,7 @@ pub struct ClientSends<'a> { } impl<'a> ClientSends<'a> { - pub async fn decrypt(&self, send: Send) -> Result { + pub fn decrypt(&self, send: Send) -> Result { let enc = self.client.get_encryption_settings()?; let key = enc.get_key(&None).ok_or(VaultLocked)?; @@ -23,7 +23,7 @@ impl<'a> ClientSends<'a> { Ok(send_view) } - pub async fn decrypt_list(&self, sends: Vec) -> Result> { + pub fn decrypt_list(&self, sends: Vec) -> Result> { let enc = self.client.get_encryption_settings()?; let key = enc.get_key(&None).ok_or(VaultLocked)?; @@ -32,19 +32,19 @@ impl<'a> ClientSends<'a> { Ok(send_views) } - pub async fn decrypt_file( + pub fn decrypt_file( &self, send: Send, encrypted_file_path: &Path, decrypted_file_path: &Path, ) -> Result<()> { let data = std::fs::read(encrypted_file_path)?; - let decrypted = self.decrypt_buffer(send, &data).await?; + let decrypted = self.decrypt_buffer(send, &data)?; std::fs::write(decrypted_file_path, decrypted)?; Ok(()) } - pub async fn decrypt_buffer(&self, send: Send, encrypted_buffer: &[u8]) -> Result> { + pub fn decrypt_buffer(&self, send: Send, encrypted_buffer: &[u8]) -> Result> { let enc = self.client.get_encryption_settings()?; let key = enc.get_key(&None).ok_or(VaultLocked)?; let key = Send::get_key(&send.key, key)?; @@ -53,7 +53,7 @@ impl<'a> ClientSends<'a> { Ok(buf.decrypt_with_key(&key)?) } - pub async fn encrypt(&self, send_view: SendView) -> Result { + pub fn encrypt(&self, send_view: SendView) -> Result { let enc = self.client.get_encryption_settings()?; let key = enc.get_key(&None).ok_or(VaultLocked)?; @@ -62,19 +62,19 @@ impl<'a> ClientSends<'a> { Ok(send) } - pub async fn encrypt_file( + pub fn encrypt_file( &self, send: Send, decrypted_file_path: &Path, encrypted_file_path: &Path, ) -> Result<()> { let data = std::fs::read(decrypted_file_path)?; - let encrypted = self.encrypt_buffer(send, &data).await?; + let encrypted = self.encrypt_buffer(send, &data)?; std::fs::write(encrypted_file_path, encrypted)?; Ok(()) } - pub async fn encrypt_buffer(&self, send: Send, buffer: &[u8]) -> Result> { + pub fn encrypt_buffer(&self, send: Send, buffer: &[u8]) -> Result> { let key = self .client .get_encryption_settings()? diff --git a/crates/bitwarden/src/mobile/vault/client_attachments.rs b/crates/bitwarden/src/mobile/vault/client_attachments.rs index ce7570fab..2f9edb82b 100644 --- a/crates/bitwarden/src/mobile/vault/client_attachments.rs +++ b/crates/bitwarden/src/mobile/vault/client_attachments.rs @@ -17,7 +17,7 @@ pub struct ClientAttachments<'a> { } impl<'a> ClientAttachments<'a> { - pub async fn encrypt_buffer( + pub fn encrypt_buffer( &self, cipher: Cipher, attachment: AttachmentView, @@ -33,7 +33,7 @@ impl<'a> ClientAttachments<'a> { } .encrypt_with_key(key)?) } - pub async fn encrypt_file( + pub fn encrypt_file( &self, cipher: Cipher, attachment: AttachmentView, @@ -44,12 +44,12 @@ impl<'a> ClientAttachments<'a> { let AttachmentEncryptResult { attachment, contents, - } = self.encrypt_buffer(cipher, attachment, &data).await?; + } = self.encrypt_buffer(cipher, attachment, &data)?; std::fs::write(encrypted_file_path, contents)?; Ok(attachment) } - pub async fn decrypt_buffer( + pub fn decrypt_buffer( &self, cipher: Cipher, attachment: Attachment, @@ -66,7 +66,7 @@ impl<'a> ClientAttachments<'a> { .decrypt_with_key(key) .map_err(Error::Crypto) } - pub async fn decrypt_file( + pub fn decrypt_file( &self, cipher: Cipher, attachment: Attachment, @@ -74,7 +74,7 @@ impl<'a> ClientAttachments<'a> { decrypted_file_path: &Path, ) -> Result<()> { let data = std::fs::read(encrypted_file_path)?; - let decrypted = self.decrypt_buffer(cipher, attachment, &data).await?; + let decrypted = self.decrypt_buffer(cipher, attachment, &data)?; std::fs::write(decrypted_file_path, decrypted)?; Ok(()) } diff --git a/crates/bitwarden/src/mobile/vault/client_ciphers.rs b/crates/bitwarden/src/mobile/vault/client_ciphers.rs index 63d11766d..e459f4e7c 100644 --- a/crates/bitwarden/src/mobile/vault/client_ciphers.rs +++ b/crates/bitwarden/src/mobile/vault/client_ciphers.rs @@ -10,7 +10,7 @@ pub struct ClientCiphers<'a> { } impl<'a> ClientCiphers<'a> { - pub async fn encrypt(&self, mut cipher_view: CipherView) -> Result { + pub fn encrypt(&self, mut cipher_view: CipherView) -> Result { let enc = self.client.get_encryption_settings()?; // TODO: Once this flag is removed, the key generation logic should @@ -26,7 +26,7 @@ impl<'a> ClientCiphers<'a> { Ok(cipher) } - pub async fn decrypt(&self, cipher: Cipher) -> Result { + pub fn decrypt(&self, cipher: Cipher) -> Result { let enc = self.client.get_encryption_settings()?; let key = cipher .locate_key(enc, &None) @@ -37,7 +37,7 @@ impl<'a> ClientCiphers<'a> { Ok(cipher_view) } - pub async fn decrypt_list(&self, ciphers: Vec) -> Result> { + pub fn decrypt_list(&self, ciphers: Vec) -> Result> { let enc = self.client.get_encryption_settings()?; let cipher_views: Result> = ciphers @@ -51,7 +51,7 @@ impl<'a> ClientCiphers<'a> { cipher_views } - pub async fn move_to_organization( + pub fn move_to_organization( &self, mut cipher_view: CipherView, organization_id: Uuid, @@ -114,7 +114,7 @@ mod tests { deleted_date: None, revision_date: "2024-05-31T09:35:55.12Z".parse().unwrap(), }]) - .await + .unwrap(); assert_eq!(dec[0].name, "Test item"); @@ -186,22 +186,13 @@ mod tests { let mut cipher = test_cipher(); cipher.attachments = Some(vec![test_attachment_legacy()]); - let view = client - .vault() - .ciphers() - .decrypt(cipher.clone()) - .await - .unwrap(); + let view = client.vault().ciphers().decrypt(cipher.clone()).unwrap(); // Move cipher to organization - let res = client - .vault() - .ciphers() - .move_to_organization( - view, - "1bc9ac1e-f5aa-45f2-94bf-b181009709b8".parse().unwrap(), - ) - .await; + let res = client.vault().ciphers().move_to_organization( + view, + "1bc9ac1e-f5aa-45f2-94bf-b181009709b8".parse().unwrap(), + ); assert!(res.is_err()); } @@ -214,20 +205,15 @@ mod tests { let attachment = test_attachment_legacy(); cipher.attachments = Some(vec![attachment.clone()]); - let view = client - .vault() - .ciphers() - .decrypt(cipher.clone()) - .await - .unwrap(); + let view = client.vault().ciphers().decrypt(cipher.clone()).unwrap(); assert!(cipher.key.is_none()); // Assert the cipher has a key, and the attachment is still readable - let new_cipher = client.vault().ciphers().encrypt(view).await.unwrap(); + let new_cipher = client.vault().ciphers().encrypt(view).unwrap(); assert!(new_cipher.key.is_some()); - let view = client.vault().ciphers().decrypt(new_cipher).await.unwrap(); + let view = client.vault().ciphers().decrypt(new_cipher).unwrap(); let attachments = view.clone().attachments.unwrap(); let attachment_view = attachments.first().unwrap().clone(); assert!(attachment_view.key.is_none()); @@ -245,7 +231,6 @@ mod tests { .vault() .attachments() .decrypt_buffer(cipher, attachment, buf.as_slice()) - .await .unwrap(); assert_eq!(content, b"Hello"); @@ -259,20 +244,15 @@ mod tests { let attachment = test_attachment_v2(); cipher.attachments = Some(vec![attachment.clone()]); - let view = client - .vault() - .ciphers() - .decrypt(cipher.clone()) - .await - .unwrap(); + let view = client.vault().ciphers().decrypt(cipher.clone()).unwrap(); assert!(cipher.key.is_none()); // Assert the cipher has a key, and the attachment is still readable - let new_cipher = client.vault().ciphers().encrypt(view).await.unwrap(); + let new_cipher = client.vault().ciphers().encrypt(view).unwrap(); assert!(new_cipher.key.is_some()); - let view = client.vault().ciphers().decrypt(new_cipher).await.unwrap(); + let view = client.vault().ciphers().decrypt(new_cipher).unwrap(); let attachments = view.clone().attachments.unwrap(); let attachment_view = attachments.first().unwrap().clone(); assert!(attachment_view.key.is_some()); @@ -296,7 +276,6 @@ mod tests { .vault() .attachments() .decrypt_buffer(cipher, attachment, buf.as_slice()) - .await .unwrap(); assert_eq!(content, b"Hello"); @@ -309,9 +288,8 @@ mod tests { view, "1bc9ac1e-f5aa-45f2-94bf-b181009709b8".parse().unwrap(), ) - .await .unwrap(); - let new_cipher = client.vault().ciphers().encrypt(new_view).await.unwrap(); + let new_cipher = client.vault().ciphers().encrypt(new_view).unwrap(); let attachment = new_cipher .clone() @@ -331,7 +309,6 @@ mod tests { .vault() .attachments() .decrypt_buffer(new_cipher, attachment, buf.as_slice()) - .await .unwrap(); assert_eq!(content, b"Hello"); diff --git a/crates/bitwarden/src/mobile/vault/client_collection.rs b/crates/bitwarden/src/mobile/vault/client_collection.rs index 26b660461..0727130df 100644 --- a/crates/bitwarden/src/mobile/vault/client_collection.rs +++ b/crates/bitwarden/src/mobile/vault/client_collection.rs @@ -8,7 +8,7 @@ pub struct ClientCollections<'a> { } impl<'a> ClientCollections<'a> { - pub async fn decrypt(&self, collection: Collection) -> Result { + pub fn decrypt(&self, collection: Collection) -> Result { let enc = self.client.get_encryption_settings()?; let key = collection .locate_key(enc, &None) @@ -19,7 +19,7 @@ impl<'a> ClientCollections<'a> { Ok(view) } - pub async fn decrypt_list(&self, collections: Vec) -> Result> { + pub fn decrypt_list(&self, collections: Vec) -> Result> { let enc = self.client.get_encryption_settings()?; let views: Result> = collections @@ -59,7 +59,7 @@ mod tests { external_id: None, hide_passwords: false, read_only: false, - }]).await.unwrap(); + }]).unwrap(); assert_eq!(dec[0].name, "Default collection"); } @@ -75,7 +75,7 @@ mod tests { external_id: None, hide_passwords: false, read_only: false, - }).await.unwrap(); + }).unwrap(); assert_eq!(dec.name, "Default collection"); } diff --git a/crates/bitwarden/src/mobile/vault/client_folders.rs b/crates/bitwarden/src/mobile/vault/client_folders.rs index cdbcdcd17..668125da2 100644 --- a/crates/bitwarden/src/mobile/vault/client_folders.rs +++ b/crates/bitwarden/src/mobile/vault/client_folders.rs @@ -8,7 +8,7 @@ pub struct ClientFolders<'a> { } impl<'a> ClientFolders<'a> { - pub async fn encrypt(&self, folder_view: FolderView) -> Result { + pub fn encrypt(&self, folder_view: FolderView) -> Result { let enc = self.client.get_encryption_settings()?; let key = enc.get_key(&None).ok_or(CryptoError::MissingKey)?; @@ -17,7 +17,7 @@ impl<'a> ClientFolders<'a> { Ok(folder) } - pub async fn decrypt(&self, folder: Folder) -> Result { + pub fn decrypt(&self, folder: Folder) -> Result { let enc = self.client.get_encryption_settings()?; let key = enc.get_key(&None).ok_or(CryptoError::MissingKey)?; @@ -26,7 +26,7 @@ impl<'a> ClientFolders<'a> { Ok(folder_view) } - pub async fn decrypt_list(&self, folders: Vec) -> Result> { + pub fn decrypt_list(&self, folders: Vec) -> Result> { let enc = self.client.get_encryption_settings()?; let key = enc.get_key(&None).ok_or(CryptoError::MissingKey)?; diff --git a/crates/bitwarden/src/mobile/vault/client_password_history.rs b/crates/bitwarden/src/mobile/vault/client_password_history.rs index b8fe0e0c5..db0f5fa53 100644 --- a/crates/bitwarden/src/mobile/vault/client_password_history.rs +++ b/crates/bitwarden/src/mobile/vault/client_password_history.rs @@ -8,7 +8,7 @@ pub struct ClientPasswordHistory<'a> { } impl<'a> ClientPasswordHistory<'a> { - pub async fn encrypt(&self, history_view: PasswordHistoryView) -> Result { + pub fn encrypt(&self, history_view: PasswordHistoryView) -> Result { let enc = self.client.get_encryption_settings()?; let key = enc.get_key(&None).ok_or(CryptoError::MissingKey)?; @@ -17,10 +17,7 @@ impl<'a> ClientPasswordHistory<'a> { Ok(history) } - pub async fn decrypt_list( - &self, - history: Vec, - ) -> Result> { + pub fn decrypt_list(&self, history: Vec) -> Result> { let enc = self.client.get_encryption_settings()?; let key = enc.get_key(&None).ok_or(CryptoError::MissingKey)?; diff --git a/crates/bitwarden/src/tool/client_generator.rs b/crates/bitwarden/src/tool/client_generator.rs index 16c786f9b..fe5319fc7 100644 --- a/crates/bitwarden/src/tool/client_generator.rs +++ b/crates/bitwarden/src/tool/client_generator.rs @@ -27,12 +27,12 @@ impl<'a> ClientGenerator<'a> { /// length: 20, /// ..Default::default() /// }; - /// let password = Client::new(None).generator().password(input).await.unwrap(); + /// let password = Client::new(None).generator().password(input).unwrap(); /// println!("{}", password); /// Ok(()) /// } /// ``` - pub async fn password(&self, input: PasswordGeneratorRequest) -> Result { + pub fn password(&self, input: PasswordGeneratorRequest) -> Result { Ok(password(input)?) } @@ -52,12 +52,12 @@ impl<'a> ClientGenerator<'a> { /// num_words: 4, /// ..Default::default() /// }; - /// let passphrase = Client::new(None).generator().passphrase(input).await.unwrap(); + /// let passphrase = Client::new(None).generator().passphrase(input).unwrap(); /// println!("{}", passphrase); /// Ok(()) /// } /// ``` - pub async fn passphrase(&self, input: PassphraseGeneratorRequest) -> Result { + pub fn passphrase(&self, input: PassphraseGeneratorRequest) -> Result { Ok(passphrase(input)?) } diff --git a/crates/bitwarden/src/tool/exporters/client_exporter.rs b/crates/bitwarden/src/tool/exporters/client_exporter.rs index 788ba272e..5257538cd 100644 --- a/crates/bitwarden/src/tool/exporters/client_exporter.rs +++ b/crates/bitwarden/src/tool/exporters/client_exporter.rs @@ -12,7 +12,7 @@ pub struct ClientExporters<'a> { impl<'a> ClientExporters<'a> { /// **Draft:** Export the vault as a CSV, JSON, or encrypted JSON file. - pub async fn export_vault( + pub fn export_vault( &self, folders: Vec, ciphers: Vec, @@ -21,7 +21,7 @@ impl<'a> ClientExporters<'a> { export_vault(self.client, folders, ciphers, format) } - pub async fn export_organization_vault( + pub fn export_organization_vault( &self, collections: Vec, ciphers: Vec, diff --git a/crates/bw/src/main.rs b/crates/bw/src/main.rs index 6674bda1e..6a1918126 100644 --- a/crates/bw/src/main.rs +++ b/crates/bw/src/main.rs @@ -217,30 +217,24 @@ async fn process_commands() -> Result<()> { Commands::Sync {} => todo!(), Commands::Generate { command } => match command { GeneratorCommands::Password(args) => { - let password = client - .generator() - .password(PasswordGeneratorRequest { - lowercase: args.lowercase, - uppercase: args.uppercase, - numbers: args.numbers, - special: args.special, - length: args.length, - ..Default::default() - }) - .await?; + let password = client.generator().password(PasswordGeneratorRequest { + lowercase: args.lowercase, + uppercase: args.uppercase, + numbers: args.numbers, + special: args.special, + length: args.length, + ..Default::default() + })?; println!("{}", password); } GeneratorCommands::Passphrase(args) => { - let passphrase = client - .generator() - .passphrase(PassphraseGeneratorRequest { - num_words: args.words, - word_separator: args.separator.to_string(), - capitalize: args.capitalize, - include_number: args.include_number, - }) - .await?; + let passphrase = client.generator().passphrase(PassphraseGeneratorRequest { + num_words: args.words, + word_separator: args.separator.to_string(), + capitalize: args.capitalize, + include_number: args.include_number, + })?; println!("{}", passphrase); } From e5a8dba1200af5f901cd686877ae47f927d47020 Mon Sep 17 00:00:00 2001 From: Oscar Hinton Date: Thu, 13 Jun 2024 15:26:50 +0200 Subject: [PATCH 6/6] Migrate fido logic to use explicit errors (#835) This PR establishes explicit error types for the fido logic. In order to extract the fido logic into a separate `bitwarden-fido` crate, we need to remove the dependencies on the `bitwarden` logic. Most of this is either the `Client` struct which is currently difficult, or the `Error` and `Result` types which this PR focuses on. --- crates/bitwarden-uniffi/src/platform/fido2.rs | 36 ++-- crates/bitwarden/src/client/client.rs | 4 +- crates/bitwarden/src/error.rs | 27 +-- .../src/platform/fido2/authenticator.rs | 161 +++++++++++++++--- crates/bitwarden/src/platform/fido2/client.rs | 35 +++- crates/bitwarden/src/platform/fido2/crypto.rs | 28 +-- crates/bitwarden/src/platform/fido2/mod.rs | 98 +++++++---- crates/bitwarden/src/platform/fido2/traits.rs | 2 - crates/bitwarden/src/platform/fido2/types.rs | 19 ++- 9 files changed, 310 insertions(+), 100 deletions(-) diff --git a/crates/bitwarden-uniffi/src/platform/fido2.rs b/crates/bitwarden-uniffi/src/platform/fido2.rs index 8a1c10e64..3fc67f1d4 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, @@ -62,9 +63,12 @@ impl ClientFido2Authenticator { let mut 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) } @@ -75,9 +79,12 @@ impl ClientFido2Authenticator { let mut 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) } @@ -91,9 +98,12 @@ impl ClientFido2Authenticator { let mut 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) } } @@ -115,9 +125,12 @@ impl ClientFido2Client { let mut 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) } @@ -133,9 +146,12 @@ impl ClientFido2Client { let mut 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 34678983d..0d2f30864 100644 --- a/crates/bitwarden/src/client/client.rs +++ b/crates/bitwarden/src/client/client.rs @@ -188,8 +188,8 @@ impl Client { } } - pub(crate) fn get_encryption_settings(&self) -> Result<&EncryptionSettings> { - self.encryption_settings.as_ref().ok_or(VaultLocked.into()) + pub(crate) fn get_encryption_settings(&self) -> Result<&EncryptionSettings, VaultLocked> { + self.encryption_settings.as_ref().ok_or(VaultLocked) } pub(crate) fn set_login_method(&mut 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 797ba87a1..825e4f6d8 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 mut 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 b523cf7e5..3f08b8810 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 mut 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 mut 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 {