diff --git a/Cargo.lock b/Cargo.lock index 04f97f266..32c837b90 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -396,6 +396,7 @@ dependencies = [ "bitwarden-crypto", "bitwarden-error", "bitwarden-state", + "bitwarden-uniffi-error", "bitwarden-uuid", "chrono", "getrandom 0.2.16", @@ -430,6 +431,7 @@ dependencies = [ "argon2", "base64", "bitwarden-error", + "bitwarden-uniffi-error", "cbc", "chacha20poly1305", "ciborium", @@ -694,6 +696,7 @@ dependencies = [ "bitwarden-send", "bitwarden-ssh", "bitwarden-state", + "bitwarden-uniffi-error", "bitwarden-vault", "chrono", "env_logger", @@ -707,6 +710,14 @@ dependencies = [ "uuid", ] +[[package]] +name = "bitwarden-uniffi-error" +version = "1.0.0" +dependencies = [ + "anyhow", + "uniffi", +] + [[package]] name = "bitwarden-uuid" version = "1.0.0" diff --git a/Cargo.toml b/Cargo.toml index 778a91e39..2bd238a6b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,6 +38,7 @@ bitwarden-ssh = { path = "crates/bitwarden-ssh", version = "=1.0.0" } bitwarden-state = { path = "crates/bitwarden-state", version = "=1.0.0" } bitwarden-test = { path = "crates/bitwarden-test", version = "=1.0.0" } bitwarden-threading = { path = "crates/bitwarden-threading", version = "=1.0.0" } +bitwarden-uniffi-error = { path = "crates/bitwarden-uniffi-error", version = "=1.0.0" } bitwarden-uuid = { path = "crates/bitwarden-uuid", version = "=1.0.0" } bitwarden-uuid-macro = { path = "crates/bitwarden-uuid-macro", version = "=1.0.0" } bitwarden-vault = { path = "crates/bitwarden-vault", version = "=1.0.0" } diff --git a/crates/bitwarden-core/Cargo.toml b/crates/bitwarden-core/Cargo.toml index 9f0915938..371eb79ae 100644 --- a/crates/bitwarden-core/Cargo.toml +++ b/crates/bitwarden-core/Cargo.toml @@ -36,6 +36,7 @@ bitwarden-api-identity = { workspace = true } bitwarden-crypto = { workspace = true } bitwarden-error = { workspace = true } bitwarden-state = { workspace = true } +bitwarden-uniffi-error = { workspace = true } bitwarden-uuid = { workspace = true } chrono = { workspace = true, features = ["std"] } # We don't use this directly (it's used by rand), but we need it here to enable WASM support diff --git a/crates/bitwarden-core/src/uniffi_support.rs b/crates/bitwarden-core/src/uniffi_support.rs index 059c7510e..9a4dd2566 100644 --- a/crates/bitwarden-core/src/uniffi_support.rs +++ b/crates/bitwarden-core/src/uniffi_support.rs @@ -3,6 +3,7 @@ use std::num::NonZeroU32; use bitwarden_crypto::CryptoError; +use bitwarden_uniffi_error::convert_result; use uuid::Uuid; use crate::key_management::SignedSecurityState; @@ -14,7 +15,7 @@ uniffi::custom_type!(DateTime, std::time::SystemTime, { remote }); uniffi::custom_type!(Uuid, String, { remote, - try_lift: |val| Uuid::parse_str(val.as_str()).map_err(|e| e.into()), + try_lift: |val| convert_result(Uuid::parse_str(val.as_str())), lower: |obj| obj.to_string(), }); @@ -29,9 +30,7 @@ struct UniffiConverterDummyRecord { uniffi::custom_type!(SignedSecurityState, String, { try_lift: |val| { - val.parse().map_err(|e| { - CryptoError::EncodingError(e).into() - }) + convert_result(val.parse().map_err(CryptoError::EncodingError)) }, lower: |obj| obj.into(), }); diff --git a/crates/bitwarden-crypto/Cargo.toml b/crates/bitwarden-crypto/Cargo.toml index db8633b29..688e144e6 100644 --- a/crates/bitwarden-crypto/Cargo.toml +++ b/crates/bitwarden-crypto/Cargo.toml @@ -29,6 +29,7 @@ argon2 = { version = ">=0.5.0, <0.6", features = [ ], default-features = false } base64 = ">=0.22.1, <0.23" bitwarden-error = { workspace = true } +bitwarden-uniffi-error = { workspace = true } cbc = { version = ">=0.1.2, <0.2", features = ["alloc", "zeroize"] } chacha20poly1305 = { version = "0.10.1" } ciborium = { version = ">=0.2.2, <0.3" } @@ -46,7 +47,7 @@ rayon = ">=1.8.1, <2.0" rsa = ">=0.9.2, <0.10" schemars = { workspace = true } serde = { workspace = true } -serde_bytes = { workspace = true } +serde_bytes = { workspace = true } serde_repr.workspace = true sha1 = ">=0.10.5, <0.11" sha2 = ">=0.10.6, <0.11" diff --git a/crates/bitwarden-crypto/src/uniffi_support.rs b/crates/bitwarden-crypto/src/uniffi_support.rs index 8d66a0429..fdceec622 100644 --- a/crates/bitwarden-crypto/src/uniffi_support.rs +++ b/crates/bitwarden-crypto/src/uniffi_support.rs @@ -1,34 +1,34 @@ use std::{num::NonZeroU32, str::FromStr}; +use bitwarden_uniffi_error::convert_result; + use crate::{CryptoError, EncString, SignedPublicKey, UnsignedSharedKey}; uniffi::custom_type!(NonZeroU32, u32, { remote, try_lift: |val| { - NonZeroU32::new(val).ok_or(CryptoError::ZeroNumber.into()) + convert_result(NonZeroU32::new(val).ok_or(CryptoError::ZeroNumber)) }, lower: |obj| obj.get(), }); uniffi::custom_type!(EncString, String, { try_lift: |val| { - EncString::from_str(&val).map_err(|e: CryptoError| e.into()) + convert_result(EncString::from_str(&val)) }, lower: |obj| obj.to_string(), }); uniffi::custom_type!(UnsignedSharedKey, String, { try_lift: |val| { - UnsignedSharedKey::from_str(&val).map_err(|e: CryptoError| e.into()) + convert_result(UnsignedSharedKey::from_str(&val)) }, lower: |obj| obj.to_string(), }); uniffi::custom_type!(SignedPublicKey, String, { try_lift: |val| { - val.parse().map_err(|e| { - CryptoError::EncodingError(e).into() - }) + convert_result(val.parse().map_err(CryptoError::EncodingError)) }, lower: |obj| obj.into(), }); diff --git a/crates/bitwarden-uniffi-error/Cargo.toml b/crates/bitwarden-uniffi-error/Cargo.toml new file mode 100644 index 000000000..a33ff25ac --- /dev/null +++ b/crates/bitwarden-uniffi-error/Cargo.toml @@ -0,0 +1,25 @@ +[package] +name = "bitwarden-uniffi-error" +description = """ +Internal crate for the bitwarden crate. Do not use. +""" + +version.workspace = true +authors.workspace = true +edition.workspace = true +rust-version.workspace = true +readme.workspace = true +homepage.workspace = true +repository.workspace = true +license-file.workspace = true +keywords.workspace = true + +[features] +uniffi = ["dep:uniffi"] + +[dependencies] +anyhow = { version = ">=1.0, <2.0" } +uniffi = { workspace = true, optional = true } + +[lints] +workspace = true diff --git a/crates/bitwarden-uniffi-error/README.md b/crates/bitwarden-uniffi-error/README.md new file mode 100644 index 000000000..7ea42630a --- /dev/null +++ b/crates/bitwarden-uniffi-error/README.md @@ -0,0 +1,4 @@ +# Bitwarden UniFFI error + +This crate provides some utilities to convert results inside `uniffi::custom_type!` calls, so that +they don't produce panics when there is a parsing error. diff --git a/crates/bitwarden-uniffi-error/src/lib.rs b/crates/bitwarden-uniffi-error/src/lib.rs new file mode 100644 index 000000000..88908b1cf --- /dev/null +++ b/crates/bitwarden-uniffi-error/src/lib.rs @@ -0,0 +1,38 @@ +#![doc = include_str!("../README.md")] + +use std::sync::OnceLock; + +#[allow(clippy::type_complexity)] +static ERROR_TO_UNIFFI_ERROR: OnceLock< + Box) -> anyhow::Error + Send + Sync + 'static>, +> = OnceLock::new(); + +pub use anyhow::Error; + +/// Configure an error converter to conver errors in calls to [`uniffi::custom_type!`] into the main +/// error of the application (`bitwarden_uniffi::error::BitwardenError). This is needed because if +/// the errors don't match, Uniffi will panic instead of returning an error. This needs to be called +/// by the `bitwarden_uniffi` crate before any other Uniffi code is run. +pub fn set_error_to_uniffi_error(f: F) +where + F: Fn(Box) -> anyhow::Error + Send + Sync + 'static, +{ + let _ = ERROR_TO_UNIFFI_ERROR.set(Box::new(f)); +} + +fn convert_error(error: E) -> anyhow::Error { + if let Some(f) = ERROR_TO_UNIFFI_ERROR.get() { + f(Box::new(error)) + } else { + anyhow::Error::new(error) + } +} + +/// Convert a `Result` into one that will not cause a panic when called inside +/// [`uniffi::custom_type!`]. It is required that all the results created inside a `custom_type!` +/// are converted using this function. +pub fn convert_result( + result: Result, +) -> Result { + result.map_err(|e| convert_error(e)) +} diff --git a/crates/bitwarden-uniffi/Cargo.toml b/crates/bitwarden-uniffi/Cargo.toml index 1716ccbcc..570404b05 100644 --- a/crates/bitwarden-uniffi/Cargo.toml +++ b/crates/bitwarden-uniffi/Cargo.toml @@ -29,6 +29,7 @@ bitwarden-generators = { workspace = true, features = ["uniffi"] } bitwarden-send = { workspace = true, features = ["uniffi"] } bitwarden-ssh = { workspace = true, features = ["uniffi"] } bitwarden-state = { workspace = true, features = ["uniffi"] } +bitwarden-uniffi-error = { workspace = true, features = ["uniffi"] } bitwarden-vault = { workspace = true, features = ["uniffi"] } chrono = { workspace = true, features = ["std"] } env_logger = "0.11.1" diff --git a/crates/bitwarden-uniffi/src/error.rs b/crates/bitwarden-uniffi/src/error.rs index 3e0ca8db3..cccd21e3b 100644 --- a/crates/bitwarden-uniffi/src/error.rs +++ b/crates/bitwarden-uniffi/src/error.rs @@ -9,6 +9,7 @@ use bitwarden_generators::{PassphraseError, PasswordError, UsernameError}; #[uniffi(flat_error)] pub enum BitwardenError { E(Error), + ConversionError(Box), } impl From for BitwardenError { @@ -21,6 +22,7 @@ impl Display for BitwardenError { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { Self::E(e) => Display::fmt(e, f), + Self::ConversionError(e) => Display::fmt(e, f), } } } @@ -29,6 +31,7 @@ impl std::error::Error for BitwardenError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match self { BitwardenError::E(e) => Some(e), + BitwardenError::ConversionError(e) => Some(e.as_ref()), } } } diff --git a/crates/bitwarden-uniffi/src/lib.rs b/crates/bitwarden-uniffi/src/lib.rs index e9cc34e9b..92e61c759 100644 --- a/crates/bitwarden-uniffi/src/lib.rs +++ b/crates/bitwarden-uniffi/src/lib.rs @@ -41,6 +41,7 @@ impl Client { #[uniffi::constructor] pub fn new(settings: Option) -> Self { init_logger(); + setup_error_converter(); #[cfg(target_os = "android")] android_support::init(); @@ -123,3 +124,11 @@ fn init_logger() { .with_max_level(log::LevelFilter::Info), ); } + +/// Setup the error converter to ensure conversion errors don't cause panics +/// Check [`bitwarden_uniffi_error`] for more details +fn setup_error_converter() { + bitwarden_uniffi_error::set_error_to_uniffi_error(|e| { + crate::error::BitwardenError::ConversionError(e).into() + }); +} diff --git a/crates/bitwarden-uniffi/src/vault/ciphers.rs b/crates/bitwarden-uniffi/src/vault/ciphers.rs index 595e84f8c..813691341 100644 --- a/crates/bitwarden-uniffi/src/vault/ciphers.rs +++ b/crates/bitwarden-uniffi/src/vault/ciphers.rs @@ -30,8 +30,13 @@ impl CiphersClient { /// Decrypt cipher list with failures /// Returns both successfully decrypted ciphers and any that failed to decrypt - pub fn decrypt_list_with_failures(&self, ciphers: Vec) -> DecryptCipherListResult { - self.0.decrypt_list_with_failures(ciphers) + // Note that this function still needs to return a Result, as the parameter conversion can still + // fail + pub fn decrypt_list_with_failures( + &self, + ciphers: Vec, + ) -> Result { + Ok(self.0.decrypt_list_with_failures(ciphers)) } pub fn decrypt_fido2_credentials(