From 43942087cfd16feb3b506bfbf5e0f3cf3579a5a2 Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Thu, 11 Dec 2025 19:44:01 +0000 Subject: [PATCH 01/10] account registration v2 for key connector --- Cargo.lock | 9 +- .../set_key_connector_key_request_model.rs | 54 +++++--- crates/bitwarden-auth/Cargo.toml | 5 + crates/bitwarden-auth/src/registration.rs | 113 +++++++++++++++- crates/bitwarden-core/Cargo.toml | 2 +- .../src/key_management/crypto_client.rs | 81 ++++++++++- .../src/key_management/key_connector.rs | 128 ++++++++++++++++++ .../bitwarden-core/src/key_management/mod.rs | 7 +- .../src/keys/key_connector_key.rs | 37 +++++ crates/bitwarden-crypto/src/keys/mod.rs | 2 + 10 files changed, 412 insertions(+), 26 deletions(-) create mode 100644 crates/bitwarden-core/src/key_management/key_connector.rs create mode 100644 crates/bitwarden-crypto/src/keys/key_connector_key.rs diff --git a/Cargo.lock b/Cargo.lock index 6242826e0..9d31fb356 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -506,15 +506,20 @@ dependencies = [ name = "bitwarden-auth" version = "1.0.0" dependencies = [ + "bitwarden-api-api", "bitwarden-core", + "bitwarden-crypto", + "bitwarden-encoding", "bitwarden-error", "bitwarden-test", "chrono", "reqwest", "serde", + "serde_bytes", "serde_json", "thiserror 2.0.12", "tokio", + "tracing", "tsify", "wasm-bindgen", "wasm-bindgen-futures", @@ -626,7 +631,7 @@ dependencies = [ "rand 0.8.5", "rand_chacha 0.3.1", "rayon", - "rsa 0.9.8", + "rsa 0.9.9", "schemars 1.0.0", "serde", "serde_bytes", @@ -1022,7 +1027,7 @@ dependencies = [ "bitwarden-vault", "console_error_panic_hook", "rand 0.8.5", - "rsa", + "rsa 0.9.9", "serde", "sha1", "tracing", diff --git a/crates/bitwarden-api-api/src/models/set_key_connector_key_request_model.rs b/crates/bitwarden-api-api/src/models/set_key_connector_key_request_model.rs index 8c6053143..ba0a8c8c6 100644 --- a/crates/bitwarden-api-api/src/models/set_key_connector_key_request_model.rs +++ b/crates/bitwarden-api-api/src/models/set_key_connector_key_request_model.rs @@ -14,14 +14,22 @@ use crate::models; #[derive(Clone, Default, Debug, PartialEq, Serialize, Deserialize)] pub struct SetKeyConnectorKeyRequestModel { - #[serde(rename = "key", alias = "Key")] - pub key: String, - #[serde(rename = "keys", alias = "Keys")] - pub keys: Box, - #[serde(rename = "kdf", alias = "Kdf")] - pub kdf: models::KdfType, - #[serde(rename = "kdfIterations", alias = "KdfIterations")] - pub kdf_iterations: i32, + #[serde(rename = "key", alias = "Key", skip_serializing_if = "Option::is_none")] + pub key: Option, + #[serde( + rename = "keys", + alias = "Keys", + skip_serializing_if = "Option::is_none" + )] + pub keys: Option>, + #[serde(rename = "kdf", alias = "Kdf", skip_serializing_if = "Option::is_none")] + pub kdf: Option, + #[serde( + rename = "kdfIterations", + alias = "KdfIterations", + skip_serializing_if = "Option::is_none" + )] + pub kdf_iterations: Option, #[serde( rename = "kdfMemory", alias = "KdfMemory", @@ -34,25 +42,33 @@ pub struct SetKeyConnectorKeyRequestModel { skip_serializing_if = "Option::is_none" )] pub kdf_parallelism: Option, + #[serde( + rename = "keyConnectorKeyWrappedUserKey", + alias = "KeyConnectorKeyWrappedUserKey", + skip_serializing_if = "Option::is_none" + )] + pub key_connector_key_wrapped_user_key: Option, + #[serde( + rename = "accountKeys", + alias = "AccountKeys", + skip_serializing_if = "Option::is_none" + )] + pub account_keys: Option>, #[serde(rename = "orgIdentifier", alias = "OrgIdentifier")] pub org_identifier: String, } impl SetKeyConnectorKeyRequestModel { - pub fn new( - key: String, - keys: models::KeysRequestModel, - kdf: models::KdfType, - kdf_iterations: i32, - org_identifier: String, - ) -> SetKeyConnectorKeyRequestModel { + pub fn new(org_identifier: String) -> SetKeyConnectorKeyRequestModel { SetKeyConnectorKeyRequestModel { - key, - keys: Box::new(keys), - kdf, - kdf_iterations, + key: None, + keys: None, + kdf: None, + kdf_iterations: None, kdf_memory: None, kdf_parallelism: None, + key_connector_key_wrapped_user_key: None, + account_keys: None, org_identifier, } } diff --git a/crates/bitwarden-auth/Cargo.toml b/crates/bitwarden-auth/Cargo.toml index 416b18449..02637f685 100644 --- a/crates/bitwarden-auth/Cargo.toml +++ b/crates/bitwarden-auth/Cargo.toml @@ -24,12 +24,17 @@ wasm = [ # Note: dependencies must be alphabetized to pass the cargo sort check in the CI pipeline. [dependencies] +bitwarden-api-api = { workspace = true } bitwarden-core = { workspace = true, features = ["internal"] } +bitwarden-crypto = { workspace = true } +bitwarden-encoding = { workspace = true } bitwarden-error = { workspace = true } chrono = { workspace = true } reqwest = { workspace = true } serde = { workspace = true } +serde_bytes = { workspace = true } thiserror = { workspace = true } +tracing = { workspace = true } tsify = { workspace = true, optional = true } wasm-bindgen = { workspace = true, optional = true } wasm-bindgen-futures = { workspace = true, optional = true } diff --git a/crates/bitwarden-auth/src/registration.rs b/crates/bitwarden-auth/src/registration.rs index b1b7e9e76..30b92f6e0 100644 --- a/crates/bitwarden-auth/src/registration.rs +++ b/crates/bitwarden-auth/src/registration.rs @@ -5,7 +5,23 @@ //! authentication method such as SSO or master password, and a decryption method such as //! key-connector, TDE, or master password. -use bitwarden_core::Client; +use std::str::FromStr; + +use bitwarden_api_api::models::SetKeyConnectorKeyRequestModel; +use bitwarden_core::{ + Client, UserId, + key_management::{ + AccountCryptographyMakeKeysError, KeyConnectorApiError, + account_cryptographic_state::WrappedAccountCryptographicState, + key_connector_api_post_or_put_key_connector_key, + }, +}; +use bitwarden_crypto::EncString; +use bitwarden_encoding::B64; +use bitwarden_error::bitwarden_error; +use serde_bytes::ByteBuf; +use thiserror::Error; +use tracing::info; #[cfg(feature = "wasm")] use wasm_bindgen::prelude::*; @@ -33,4 +49,99 @@ impl RegistrationClient { let api_client = &client.get_api_configurations().await.api_client; // Do API request here. It will be authenticated using the client's tokens. } + + /// Initializes a new cryptographic state for a user and posts it to the server; enrolls the + /// user to key connector unlock. + pub async fn post_keys_for_key_connector_registration( + &self, + key_connector_url: String, + org_id: String, + user_id: String, + ) -> Result { + let client = &self.client.internal; + let api_client = &client.get_api_configurations().await.api_client; + let user_id = + UserId::from_str(user_id.as_str()).map_err(|_| UserRegistrationError::Serialization)?; + + // First call crypto API to get all keys + info!("Initializing account cryptography"); + let ( + cryptography_state, + wrapped_user_key, + user_key, + account_cryptographic_state_request, + key_connector_key, + ) = self + .client + .crypto() + .make_user_key_connector_registration(user_id) + .map_err(UserRegistrationError::AccountCryptographyMakeKeys)?; + + info!("Posting key connector key to key connector server"); + key_connector_api_post_or_put_key_connector_key( + &self.client, + key_connector_url.as_str(), + &key_connector_key, + ) + .await + .map_err(UserRegistrationError::KeyConnectorApi)?; + + info!("Posting user account cryptographic state to server"); + let request = SetKeyConnectorKeyRequestModel { + key_connector_key_wrapped_user_key: Some(wrapped_user_key.to_string()), + account_keys: Some(Box::new(account_cryptographic_state_request)), + ..SetKeyConnectorKeyRequestModel::new(org_id) + }; + api_client + .accounts_key_management_api() + .post_set_key_connector_key(Some(request)) + .await + .map_err(|e| UserRegistrationError::Api(e.into()))?; + + info!("User initialized!"); + // Note: This passing out of state and keys is temporary. Once SDK state management is more + // mature, the account cryptographic state and keys should be set directly here. + Ok(KeyConnectorRegistrationResult { + account_cryptographic_state: cryptography_state, + key_connector_key: key_connector_key.to_base64(), + key_connector_key_wrapped_user_key: wrapped_user_key, + user_key: user_key.to_encoded().to_vec().into(), + }) + } +} + +/// Result of Key Connector registration process. +#[cfg_attr( + feature = "wasm", + derive(tsify::Tsify), + tsify(into_wasm_abi, from_wasm_abi) +)] +#[derive(serde::Serialize, serde::Deserialize, Clone, Debug)] +pub struct KeyConnectorRegistrationResult { + /// The account cryptographic state of the user. + pub account_cryptographic_state: WrappedAccountCryptographicState, + /// The key connector key used for unlocking. + pub key_connector_key: B64, + /// The encrypted user key, wrapped with the key connector key. + pub key_connector_key_wrapped_user_key: EncString, + /// The decrypted user key. This can be used to get the consuming client to an unlocked state. + pub user_key: ByteBuf, +} + +/// Errors that can occur during user registration. +#[derive(Debug, Error)] +#[bitwarden_error(flat)] +pub enum UserRegistrationError { + /// Key Connector API call failed. + #[error(transparent)] + KeyConnectorApi(#[from] KeyConnectorApiError), + /// API call failed. + #[error(transparent)] + Api(#[from] bitwarden_core::ApiError), + /// Account cryptography initialization failed. + #[error(transparent)] + AccountCryptographyMakeKeys(#[from] AccountCryptographyMakeKeysError), + /// Serialization or deserialization error + #[error("Serialization error")] + Serialization, } diff --git a/crates/bitwarden-core/Cargo.toml b/crates/bitwarden-core/Cargo.toml index 80bff271e..4e6bcb0ea 100644 --- a/crates/bitwarden-core/Cargo.toml +++ b/crates/bitwarden-core/Cargo.toml @@ -51,6 +51,7 @@ 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 getrandom = { version = ">=0.2.9, <0.3", features = ["js"] } rand = ">=0.8.5, <0.9" +rand_chacha = ">=0.3.1, <0.4.0" reqwest = { workspace = true } schemars = { workspace = true } serde = { workspace = true } @@ -76,7 +77,6 @@ rustls = { version = "0.23.19", default-features = false } rustls-platform-verifier = "0.6.0" [dev-dependencies] -rand_chacha = "0.3.1" tokio = { workspace = true, features = ["rt"] } zeroize = { version = ">=1.7.0, <2.0", features = ["derive", "aarch64"] } diff --git a/crates/bitwarden-core/src/key_management/crypto_client.rs b/crates/bitwarden-core/src/key_management/crypto_client.rs index e1dfbf17d..94eafc523 100644 --- a/crates/bitwarden-core/src/key_management/crypto_client.rs +++ b/crates/bitwarden-core/src/key_management/crypto_client.rs @@ -1,9 +1,16 @@ +use std::sync::RwLock; + +use bitwarden_api_api::models::AccountKeysRequestModel; #[cfg(feature = "wasm")] use bitwarden_crypto::safe::PasswordProtectedKeyEnvelope; -use bitwarden_crypto::{CryptoError, Decryptable, Kdf}; +use bitwarden_crypto::{ + CryptoError, Decryptable, Kdf, KeyConnectorKey, KeyStore, SymmetricCryptoKey, +}; #[cfg(feature = "internal")] use bitwarden_crypto::{EncString, UnsignedSharedKey}; use bitwarden_encoding::B64; +use bitwarden_error::bitwarden_error; +use rand_chacha::{ChaCha8Rng, rand_core::SeedableRng}; #[cfg(feature = "wasm")] use wasm_bindgen::prelude::*; @@ -15,6 +22,9 @@ use super::crypto::{ #[cfg(feature = "internal")] use crate::key_management::{ SymmetricKeyId, + account_cryptographic_state::{ + AccountCryptographyInitializationError, WrappedAccountCryptographicState, + }, crypto::{ DerivePinKeyResponse, InitOrgCryptoRequest, InitUserCryptoRequest, UpdatePasswordResponse, derive_pin_key, derive_pin_user_key, enroll_admin_password_reset, get_user_encryption_key, @@ -22,7 +32,7 @@ use crate::key_management::{ }, }; use crate::{ - Client, + Client, UserId, client::encryption_settings::EncryptionSettingsError, error::StatefulCryptoError, key_management::crypto::{ @@ -187,6 +197,73 @@ impl CryptoClient { ) -> Result { derive_key_connector(request) } + + /// Creates a new V2 account cryptographic state for Key Connector registration. + /// This generates fresh cryptographic keys (private key, signing key, signed public key, + /// and security state) wrapped with a new user key. + /// + /// Returns the wrapped account cryptographic state that can be used for registration, + /// key connector key wrapped user key, key connector key and decrypted user key. + pub fn make_user_key_connector_registration( + &self, + user_id: UserId, + ) -> Result< + ( + WrappedAccountCryptographicState, + EncString, + SymmetricCryptoKey, + AccountKeysRequestModel, + KeyConnectorKey, + ), + AccountCryptographyMakeKeysError, + > { + let mut ctx = self.client.internal.get_key_store().context_mut(); + let (user_key, wrapped_state) = + WrappedAccountCryptographicState::make(&mut ctx, user_id) + .map_err(AccountCryptographyMakeKeysError::AccountCryptographyInitialization)?; + #[expect(deprecated)] + let user_key = ctx.dangerous_get_symmetric_key(user_key)?; + + // Key Connector unlock method + let mut rng = ChaCha8Rng::from_seed([0u8; 32]); + let key_connector_key = KeyConnectorKey::generate(&mut rng); + + let wrapped_user_key = key_connector_key + .encrypt_user_key(user_key) + .map_err(AccountCryptographyMakeKeysError::Crypto)?; + + let store = KeyStore::default(); + let mut ctx = store.context_mut(); + let user_key_id = ctx.add_local_symmetric_key(user_key.to_owned()); + let security_state = RwLock::new(None); + wrapped_state + .set_to_context(&security_state, user_key_id, &store, ctx) + .map_err(AccountCryptographyMakeKeysError::AccountCryptographyInitialization)?; + + let cryptography_state_request_model = wrapped_state + .to_request_model(&store) + .map_err(AccountCryptographyMakeKeysError::AccountCryptographyInitialization)?; + + Ok(( + wrapped_state, + wrapped_user_key, + user_key.to_owned(), + cryptography_state_request_model, + key_connector_key, + )) + } +} + +/// Errors that can occur during account cryptography key generation. +#[bitwarden_error(flat)] +#[derive(Debug, thiserror::Error)] +pub enum AccountCryptographyMakeKeysError { + /// Failed to initialize account cryptography + #[error("Failed to initialize account cryptography: {0}")] + AccountCryptographyInitialization(#[from] AccountCryptographyInitializationError), + /// Generic crypto error + #[error("Cryptography error: {0}")] + Crypto(#[from] CryptoError), } impl Client { diff --git a/crates/bitwarden-core/src/key_management/key_connector.rs b/crates/bitwarden-core/src/key_management/key_connector.rs new file mode 100644 index 000000000..eaa2c465f --- /dev/null +++ b/crates/bitwarden-core/src/key_management/key_connector.rs @@ -0,0 +1,128 @@ +use bitwarden_crypto::KeyConnectorKey; +use reqwest::Method; +use serde::{Deserialize, Serialize}; +use thiserror::Error; + +use crate::{ApiError, Client}; + +#[allow(missing_docs)] +#[derive(Debug, Error)] +pub enum KeyConnectorApiError { + #[error("Invalid Key Connector URL")] + InvalidKeyConnectorUrl, + #[error(transparent)] + Api(#[from] ApiError), +} + +/// Sends the key connector key to the Key Connector API. +/// If a key already exists, it will be updated; otherwise, a new key will be created for the user. +pub async fn key_connector_api_post_or_put_key_connector_key( + client: &Client, + key_connector_url: &str, + key_connector_key: &KeyConnectorKey, +) -> Result<(), KeyConnectorApiError> { + let request = KeyConnectorKeyRequestModel { + key: key_connector_key.to_base64().to_string(), + }; + + if get_key_connector_user_keys(client, key_connector_url) + .await + .is_ok() + { + put_key_connector_user_keys(client, key_connector_url, request).await?; + } else { + post_key_connector_user_keys(client, key_connector_url, request).await?; + } + + Ok(()) +} + +async fn get_key_connector_user_keys( + client: &Client, + key_connector_url: &str, +) -> Result { + let response = + request_key_connector_user_keys(client, Method::GET, key_connector_url, None::<()>).await?; + + let body = response.text().await.map_err(KeyConnectorApiError::from)?; + let response_model = serde_json::from_str::(&body)?; + Ok(response_model) +} + +async fn post_key_connector_user_keys( + client: &Client, + key_connector_url: &str, + request_model: KeyConnectorKeyRequestModel, +) -> Result<(), KeyConnectorApiError> { + request_key_connector_user_keys(client, Method::POST, key_connector_url, Some(request_model)) + .await?; + + Ok(()) +} + +async fn put_key_connector_user_keys( + client: &Client, + key_connector_url: &str, + request_model: KeyConnectorKeyRequestModel, +) -> Result<(), KeyConnectorApiError> { + request_key_connector_user_keys(client, Method::PUT, key_connector_url, Some(request_model)) + .await?; + + Ok(()) +} + +async fn request_key_connector_user_keys( + client: &Client, + method: Method, + key_connector_url: &str, + body: Option, +) -> Result { + let url = format!("{}/user-keys", key_connector_url); + + let config = client.internal.get_api_configurations().await; + let mut request = client + .internal + .get_http_client() + .request(method, url) + .header(reqwest::header::CONTENT_TYPE, "application/json") + .header(reqwest::header::ACCEPT, "application/json"); + + if let Some(ref user_agent) = config.api_config.user_agent { + request = request.header(reqwest::header::USER_AGENT, user_agent.clone()); + } + if let Some(ref access_token) = config.api_config.oauth_access_token { + request = request.bearer_auth(access_token.clone()); + } + if let Some(ref body) = body { + request = + request.body(serde_json::to_string(&body).expect("Serialize should be infallible")) + } + + let response = request.send().await.map_err(KeyConnectorApiError::from)?; + + Ok(response.error_for_status()?) +} + +impl From for KeyConnectorApiError { + fn from(e: reqwest::Error) -> Self { + KeyConnectorApiError::Api(ApiError::Reqwest(e)) + } +} + +impl From for KeyConnectorApiError { + fn from(e: serde_json::Error) -> Self { + KeyConnectorApiError::Api(ApiError::Serde(e)) + } +} + +#[derive(Clone, Default, Debug, PartialEq, Serialize, Deserialize)] +struct KeyConnectorKeyRequestModel { + #[serde(rename = "key", alias = "Key")] + pub key: String, +} + +#[derive(Clone, Default, Debug, PartialEq, Serialize, Deserialize)] +struct KeyConnectorKeyResponseModel { + #[serde(rename = "key", alias = "Key")] + pub key: String, +} diff --git a/crates/bitwarden-core/src/key_management/mod.rs b/crates/bitwarden-core/src/key_management/mod.rs index bd7c140a5..27b33507c 100644 --- a/crates/bitwarden-core/src/key_management/mod.rs +++ b/crates/bitwarden-core/src/key_management/mod.rs @@ -19,7 +19,7 @@ pub mod crypto; #[cfg(feature = "internal")] mod crypto_client; #[cfg(feature = "internal")] -pub use crypto_client::CryptoClient; +pub use crypto_client::{AccountCryptographyMakeKeysError, CryptoClient}; #[cfg(feature = "internal")] mod master_password; @@ -38,6 +38,11 @@ mod user_decryption; #[cfg(feature = "internal")] pub use user_decryption::UserDecryptionData; +#[cfg(feature = "internal")] +mod key_connector; +#[cfg(feature = "internal")] +pub use key_connector::{KeyConnectorApiError, key_connector_api_post_or_put_key_connector_key}; + use crate::OrganizationId; key_ids! { diff --git a/crates/bitwarden-crypto/src/keys/key_connector_key.rs b/crates/bitwarden-crypto/src/keys/key_connector_key.rs new file mode 100644 index 000000000..58ac47318 --- /dev/null +++ b/crates/bitwarden-crypto/src/keys/key_connector_key.rs @@ -0,0 +1,37 @@ +use std::pin::Pin; + +use bitwarden_encoding::B64; +use generic_array::GenericArray; +use rand::Rng; +use typenum::U32; + +use crate::{EncString, SymmetricCryptoKey, keys::utils::stretch_key}; + +/// Key connector key, used to protect the [UserKey]. +#[derive(Clone, Debug)] +pub struct KeyConnectorKey(pub(super) Pin>>); + +impl KeyConnectorKey { + /// Generate a new random for KeyConnector. + pub fn generate(mut rng: impl rand::RngCore) -> Self { + let mut key = Box::pin(GenericArray::::default()); + + rng.fill(key.as_mut_slice()); + KeyConnectorKey(key) + } + + #[allow(missing_docs)] + pub fn to_base64(&self) -> B64 { + B64::from(self.0.as_slice()) + } + + /// Wraps the user key with this key connector key. + pub fn encrypt_user_key( + &self, + user_key: &SymmetricCryptoKey, + ) -> crate::error::Result { + let stretched_master_key = stretch_key(&self.0)?; + let user_key_bytes = user_key.to_encoded(); + EncString::encrypt_aes256_hmac(user_key_bytes.as_ref(), &stretched_master_key) + } +} diff --git a/crates/bitwarden-crypto/src/keys/mod.rs b/crates/bitwarden-crypto/src/keys/mod.rs index 522b34537..4dfcb3570 100644 --- a/crates/bitwarden-crypto/src/keys/mod.rs +++ b/crates/bitwarden-crypto/src/keys/mod.rs @@ -34,4 +34,6 @@ pub use kdf::{ default_pbkdf2_iterations, }; pub(crate) use key_id::{KEY_ID_SIZE, KeyId}; +mod key_connector_key; pub(crate) mod utils; +pub use key_connector_key::KeyConnectorKey; From a7dc33ab0f58c303507d56b6dff33a2b494a84ef Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Fri, 12 Dec 2025 17:23:54 +0000 Subject: [PATCH 02/10] lint --- crates/bitwarden-crypto/src/keys/key_connector_key.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bitwarden-crypto/src/keys/key_connector_key.rs b/crates/bitwarden-crypto/src/keys/key_connector_key.rs index 58ac47318..d8c790f71 100644 --- a/crates/bitwarden-crypto/src/keys/key_connector_key.rs +++ b/crates/bitwarden-crypto/src/keys/key_connector_key.rs @@ -7,7 +7,7 @@ use typenum::U32; use crate::{EncString, SymmetricCryptoKey, keys::utils::stretch_key}; -/// Key connector key, used to protect the [UserKey]. +/// Key connector key, used to protect the user key. #[derive(Clone, Debug)] pub struct KeyConnectorKey(pub(super) Pin>>); From 27a3e16ae3dac5b665f21b1b8f7157a9ad2070a8 Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Thu, 18 Dec 2025 10:04:39 +0100 Subject: [PATCH 03/10] review suggestions --- Cargo.lock | 1 - crates/bitwarden-auth/Cargo.toml | 1 - crates/bitwarden-auth/src/registration.rs | 68 +++---- crates/bitwarden-core/Cargo.toml | 2 +- .../account_cryptographic_state.rs | 191 +++++------------- .../src/key_management/crypto_client.rs | 61 +++--- .../src/keys/key_connector_key.rs | 13 +- 7 files changed, 120 insertions(+), 217 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e43abaa3b..2c6830c13 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -515,7 +515,6 @@ dependencies = [ "chrono", "reqwest", "serde", - "serde_bytes", "serde_json", "thiserror 1.0.69", "tokio", diff --git a/crates/bitwarden-auth/Cargo.toml b/crates/bitwarden-auth/Cargo.toml index 0070aff28..4f3022b74 100644 --- a/crates/bitwarden-auth/Cargo.toml +++ b/crates/bitwarden-auth/Cargo.toml @@ -33,7 +33,6 @@ bitwarden-error = { workspace = true } chrono = { workspace = true } reqwest = { workspace = true } serde = { workspace = true } -serde_bytes = { workspace = true } thiserror = { workspace = true } tracing = { workspace = true } tsify = { workspace = true, optional = true } diff --git a/crates/bitwarden-auth/src/registration.rs b/crates/bitwarden-auth/src/registration.rs index 30b92f6e0..fc555e200 100644 --- a/crates/bitwarden-auth/src/registration.rs +++ b/crates/bitwarden-auth/src/registration.rs @@ -5,13 +5,11 @@ //! authentication method such as SSO or master password, and a decryption method such as //! key-connector, TDE, or master password. -use std::str::FromStr; - use bitwarden_api_api::models::SetKeyConnectorKeyRequestModel; use bitwarden_core::{ - Client, UserId, + Client, OrganizationId, UserId, key_management::{ - AccountCryptographyMakeKeysError, KeyConnectorApiError, + AccountCryptographyMakeKeysError, account_cryptographic_state::WrappedAccountCryptographicState, key_connector_api_post_or_put_key_connector_key, }, @@ -19,9 +17,8 @@ use bitwarden_core::{ use bitwarden_crypto::EncString; use bitwarden_encoding::B64; use bitwarden_error::bitwarden_error; -use serde_bytes::ByteBuf; use thiserror::Error; -use tracing::info; +use tracing::{error, info}; #[cfg(feature = "wasm")] use wasm_bindgen::prelude::*; @@ -55,23 +52,15 @@ impl RegistrationClient { pub async fn post_keys_for_key_connector_registration( &self, key_connector_url: String, - org_id: String, - user_id: String, + org_id: OrganizationId, + user_id: UserId, ) -> Result { let client = &self.client.internal; let api_client = &client.get_api_configurations().await.api_client; - let user_id = - UserId::from_str(user_id.as_str()).map_err(|_| UserRegistrationError::Serialization)?; // First call crypto API to get all keys info!("Initializing account cryptography"); - let ( - cryptography_state, - wrapped_user_key, - user_key, - account_cryptographic_state_request, - key_connector_key, - ) = self + let registration_crypto_result = self .client .crypto() .make_user_key_connector_registration(user_id) @@ -81,31 +70,42 @@ impl RegistrationClient { key_connector_api_post_or_put_key_connector_key( &self.client, key_connector_url.as_str(), - &key_connector_key, + ®istration_crypto_result.key_connector_key, ) .await - .map_err(UserRegistrationError::KeyConnectorApi)?; + .map_err(|e| { + error!("Failed to post key connector key to key connector server: {e:?}"); + UserRegistrationError::KeyConnectorApi + })?; info!("Posting user account cryptographic state to server"); let request = SetKeyConnectorKeyRequestModel { - key_connector_key_wrapped_user_key: Some(wrapped_user_key.to_string()), - account_keys: Some(Box::new(account_cryptographic_state_request)), - ..SetKeyConnectorKeyRequestModel::new(org_id) + key_connector_key_wrapped_user_key: Some( + registration_crypto_result + .key_connector_key_wrapped_user_key + .to_string(), + ), + account_keys: Some(Box::new(registration_crypto_result.account_keys_request)), + ..SetKeyConnectorKeyRequestModel::new(org_id.to_string()) }; api_client .accounts_key_management_api() .post_set_key_connector_key(Some(request)) .await - .map_err(|e| UserRegistrationError::Api(e.into()))?; + .map_err(|e| { + error!("Failed to post account cryptographic state to server: {e:?}"); + UserRegistrationError::Api + })?; info!("User initialized!"); // Note: This passing out of state and keys is temporary. Once SDK state management is more // mature, the account cryptographic state and keys should be set directly here. Ok(KeyConnectorRegistrationResult { - account_cryptographic_state: cryptography_state, - key_connector_key: key_connector_key.to_base64(), - key_connector_key_wrapped_user_key: wrapped_user_key, - user_key: user_key.to_encoded().to_vec().into(), + account_cryptographic_state: registration_crypto_result.account_cryptographic_state, + key_connector_key: registration_crypto_result.key_connector_key.to_base64(), + key_connector_key_wrapped_user_key: registration_crypto_result + .key_connector_key_wrapped_user_key, + user_key: registration_crypto_result.user_key.to_encoded().into(), }) } } @@ -116,6 +116,7 @@ impl RegistrationClient { derive(tsify::Tsify), tsify(into_wasm_abi, from_wasm_abi) )] +#[cfg_attr(feature = "uniffi", derive(uniffi::Record))] #[derive(serde::Serialize, serde::Deserialize, Clone, Debug)] pub struct KeyConnectorRegistrationResult { /// The account cryptographic state of the user. @@ -125,7 +126,7 @@ pub struct KeyConnectorRegistrationResult { /// The encrypted user key, wrapped with the key connector key. pub key_connector_key_wrapped_user_key: EncString, /// The decrypted user key. This can be used to get the consuming client to an unlocked state. - pub user_key: ByteBuf, + pub user_key: B64, } /// Errors that can occur during user registration. @@ -133,15 +134,12 @@ pub struct KeyConnectorRegistrationResult { #[bitwarden_error(flat)] pub enum UserRegistrationError { /// Key Connector API call failed. - #[error(transparent)] - KeyConnectorApi(#[from] KeyConnectorApiError), + #[error("Key Connector Api call failed")] + KeyConnectorApi, /// API call failed. - #[error(transparent)] - Api(#[from] bitwarden_core::ApiError), + #[error("Api call failed")] + Api, /// Account cryptography initialization failed. #[error(transparent)] AccountCryptographyMakeKeys(#[from] AccountCryptographyMakeKeysError), - /// Serialization or deserialization error - #[error("Serialization error")] - Serialization, } diff --git a/crates/bitwarden-core/Cargo.toml b/crates/bitwarden-core/Cargo.toml index b1fe673d5..c21f0f170 100644 --- a/crates/bitwarden-core/Cargo.toml +++ b/crates/bitwarden-core/Cargo.toml @@ -51,7 +51,6 @@ 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 getrandom = { version = ">=0.2.9, <0.3", features = ["js"] } rand = ">=0.8.5, <0.9" -rand_chacha = ">=0.3.1, <0.4.0" reqwest = { workspace = true } schemars = { workspace = true } serde = { workspace = true } @@ -77,6 +76,7 @@ rustls = { version = "0.23.19", default-features = false } rustls-platform-verifier = "0.6.0" [dev-dependencies] +rand_chacha = ">=0.3.1, <0.4.0" tokio = { workspace = true, features = ["rt"] } zeroize = { version = ">=1.7.0, <2.0", features = ["derive", "aarch64"] } diff --git a/crates/bitwarden-core/src/key_management/account_cryptographic_state.rs b/crates/bitwarden-core/src/key_management/account_cryptographic_state.rs index fd058d5aa..75455cca7 100644 --- a/crates/bitwarden-core/src/key_management/account_cryptographic_state.rs +++ b/crates/bitwarden-core/src/key_management/account_cryptographic_state.rs @@ -12,9 +12,8 @@ use std::sync::RwLock; use bitwarden_api_api::models::{AccountKeysRequestModel, SecurityStateModel}; use bitwarden_crypto::{ - AsymmetricPublicCryptoKey, CoseSerializable, CryptoError, EncString, KeyStore, KeyStoreContext, + CoseSerializable, CryptoError, EncString, KeyStore, KeyStoreContext, PublicKeyEncryptionAlgorithm, SignatureAlgorithm, SignedPublicKey, SymmetricKeyAlgorithm, - VerifyingKey, }; use bitwarden_encoding::B64; use bitwarden_error::bitwarden_error; @@ -100,44 +99,41 @@ impl WrappedAccountCryptographicState { /// user key required to unlock this state. pub fn to_request_model( &self, - store: &KeyStore, + user_key: &SymmetricKeyId, + ctx: &mut KeyStoreContext, ) -> Result { - let verifying_key = self.verifying_key(store)?; + let private_key = match self { + WrappedAccountCryptographicState::V1 { private_key } + | WrappedAccountCryptographicState::V2 { private_key, .. } => private_key.clone(), + }; + let private_key_tmp_id = ctx.unwrap_private_key(*user_key, &private_key)?; + let public_key = ctx.get_public_key(private_key_tmp_id)?; + + let signature_keypair = match self { + WrappedAccountCryptographicState::V1 { .. } => None, + WrappedAccountCryptographicState::V2 { signing_key, .. } => { + let signing_key_tmp_id = ctx.unwrap_signing_key(*user_key, signing_key)?; + let verifying_key = ctx.get_verifying_key(signing_key_tmp_id)?; + Some((signing_key.clone(), verifying_key)) + } + }; + Ok(AccountKeysRequestModel { // Note: This property is deprecated and should be removed after a transition period. - user_key_encrypted_account_private_key: match self { - WrappedAccountCryptographicState::V1 { private_key } - | WrappedAccountCryptographicState::V2 { private_key, .. } => { - Some(private_key.to_string()) - } - }, + user_key_encrypted_account_private_key: Some(private_key.to_string()), // Note: This property is deprecated and should be removed after a transition period. - account_public_key: match self.public_key(store)? { - Some(pk) => Some(B64::from(pk.to_der()?).to_string()), - None => None, - }, - signature_key_pair: match self { - WrappedAccountCryptographicState::V1 { .. } => None, - WrappedAccountCryptographicState::V2 { signing_key, .. } => Some(Box::new( - bitwarden_api_api::models::SignatureKeyPairRequestModel { + account_public_key: Some(B64::from(public_key.to_der()?).to_string()), + signature_key_pair: signature_keypair + .as_ref() + .map(|(signing_key, verifying_key)| { + Box::new(bitwarden_api_api::models::SignatureKeyPairRequestModel { wrapped_signing_key: Some(signing_key.to_string()), - verifying_key: Some( - B64::from( - verifying_key - .as_ref() - .map(|vk| vk.to_cose()) - .ok_or(AccountCryptographyInitializationError::CorruptData)?, - ) - .to_string(), - ), - signature_algorithm: verifying_key.as_ref().map(|vk| { - match vk.algorithm() { - SignatureAlgorithm::Ed25519 => "ed25519".to_string(), - } + verifying_key: Some(B64::from(verifying_key.to_cose()).to_string()), + signature_algorithm: Some(match verifying_key.algorithm() { + SignatureAlgorithm::Ed25519 => "ed25519".to_string(), }), - }, - )), - }, + }) + }), public_key_encryption_key_pair: Some(Box::new( bitwarden_api_api::models::PublicKeyEncryptionKeyPairRequestModel { wrapped_private_key: match self { @@ -146,29 +142,25 @@ impl WrappedAccountCryptographicState { Some(private_key.to_string()) } }, - public_key: match self.public_key(store) { - Ok(Some(pk)) => Some(B64::from(pk.to_der()?).to_string()), - _ => None, - }, + public_key: Some(B64::from(public_key.to_der()?).to_string()), signed_public_key: match self.signed_public_key() { Ok(Some(spk)) => Some(spk.clone().into()), _ => None, }, }, )), - security_state: match self { - WrappedAccountCryptographicState::V1 { .. } => None, - WrappedAccountCryptographicState::V2 { security_state, .. } => { - // ensure we have a verifying key reference and convert the verified state's - // version to i32 for the API model - let vk_ref = verifying_key - .as_ref() - .ok_or(AccountCryptographyInitializationError::CorruptData)?; + security_state: match (self, signature_keypair.as_ref()) { + (_, None) | (WrappedAccountCryptographicState::V1 { .. }, Some(_)) => None, + ( + WrappedAccountCryptographicState::V2 { security_state, .. }, + Some((_, verifying_key)), + ) => { + // Convert the verified state's version to i32 for the API model Some(Box::new(SecurityStateModel { security_state: Some(security_state.into()), security_version: security_state - .clone() - .verify_and_unwrap(vk_ref) + .to_owned() + .verify_and_unwrap(verifying_key) .map_err(|_| AccountCryptographyInitializationError::TamperedData)? .version() as i32, })) @@ -229,16 +221,11 @@ impl WrappedAccountCryptographicState { return Err(AccountCryptographyInitializationError::WrongUserKeyType); } - // Some users have unreadable V1 private keys. In this case, we set no keys to - // state. - if let Ok(private_key_id) = ctx.unwrap_private_key(user_key, private_key) { - ctx.persist_asymmetric_key(private_key_id, AsymmetricKeyId::UserPrivateKey)?; - } else { - tracing::warn!( - "V1 private key could not be unwrapped, skipping setting private key" - ); - } + let private_key_id = ctx + .unwrap_private_key(user_key, private_key) + .map_err(|_| AccountCryptographyInitializationError::WrongUserKey)?; + ctx.persist_asymmetric_key(private_key_id, AsymmetricKeyId::UserPrivateKey)?; ctx.persist_symmetric_key(user_key, SymmetricKeyId::User)?; } WrappedAccountCryptographicState::V2 { @@ -286,46 +273,6 @@ impl WrappedAccountCryptographicState { Ok(()) } - /// Retrieve the verifying key from the wrapped state, if present. This requires the user key to - /// be present in the store. - fn verifying_key( - &self, - store: &KeyStore, - ) -> Result, AccountCryptographyInitializationError> { - match self { - WrappedAccountCryptographicState::V1 { .. } => Ok(None), - WrappedAccountCryptographicState::V2 { signing_key, .. } => { - let mut ctx = store.context_mut(); - let signing_key = ctx - .unwrap_signing_key(SymmetricKeyId::User, signing_key) - .map_err(|_| AccountCryptographyInitializationError::WrongUserKey)?; - ctx.get_verifying_key(signing_key) - .map(Some) - .map_err(|e| e.into()) - } - } - } - - /// Retrieve the public key from the wrapped state, if present. This requires the user key to - /// be present in the store. - fn public_key( - &self, - store: &KeyStore, - ) -> Result, AccountCryptographyInitializationError> { - match self { - WrappedAccountCryptographicState::V1 { private_key } - | WrappedAccountCryptographicState::V2 { private_key, .. } => { - let mut ctx = store.context_mut(); - let private_key = ctx - .unwrap_private_key(SymmetricKeyId::User, private_key) - .map_err(|_| AccountCryptographyInitializationError::WrongUserKey)?; - ctx.get_public_key(private_key) - .map(Some) - .map_err(|e| e.into()) - } - } - } - /// Retrieve the signed public key from the wrapped state, if present. fn signed_public_key( &self, @@ -343,7 +290,7 @@ impl WrappedAccountCryptographicState { mod tests { use std::{str::FromStr, sync::RwLock}; - use bitwarden_crypto::{KeyStore, PrimitiveEncryptable}; + use bitwarden_crypto::KeyStore; use super::*; use crate::key_management::{AsymmetricKeyId, SigningKeyId, SymmetricKeyId}; @@ -472,9 +419,12 @@ mod tests { wrapped_account_cryptography_state .set_to_context(&RwLock::new(None), user_key, &temp_store, temp_ctx) .unwrap(); + + let mut ctx = temp_store.context_mut(); let model = wrapped_account_cryptography_state - .to_request_model(&temp_store) + .to_request_model(&SymmetricKeyId::User, &mut ctx) .expect("to_private_keys_request_model should succeed"); + drop(ctx); let ctx = temp_store.context(); @@ -517,47 +467,4 @@ mod tests { model.security_state.unwrap().security_version as u64 ); } - - #[test] - fn test_set_to_context_v1_corrupt_private_key() { - // Test that a V1 account with a corrupt private key (valid EncString but invalid key data) - // can still initialize, but skips setting the private key - let temp_store: KeyStore = KeyStore::default(); - let mut temp_ctx = temp_store.context_mut(); - - let user_key = temp_ctx.make_symmetric_key(SymmetricKeyAlgorithm::Aes256CbcHmac); - let corrupt_private_key = "not a private key" - .encrypt(&mut temp_ctx, user_key) - .unwrap(); - - // Construct the V1 wrapped state with corrupt private key - let wrapped = WrappedAccountCryptographicState::V1 { - private_key: corrupt_private_key, - }; - - #[expect(deprecated)] - let user_key_material = temp_ctx - .dangerous_get_symmetric_key(user_key) - .unwrap() - .to_owned(); - drop(temp_ctx); - drop(temp_store); - - // Now attempt to set this wrapped state into a fresh store - let store: KeyStore = KeyStore::default(); - let mut ctx = store.context_mut(); - let user_key = ctx.add_local_symmetric_key(user_key_material); - let security_state = RwLock::new(None); - - wrapped - .set_to_context(&security_state, user_key, &store, ctx) - .unwrap(); - - let ctx = store.context(); - - // The user symmetric key should be set - assert!(ctx.has_symmetric_key(SymmetricKeyId::User)); - // But the private key should NOT be set (due to corruption) - assert!(!ctx.has_asymmetric_key(AsymmetricKeyId::UserPrivateKey)); - } } diff --git a/crates/bitwarden-core/src/key_management/crypto_client.rs b/crates/bitwarden-core/src/key_management/crypto_client.rs index b2217f9f0..31b184ebd 100644 --- a/crates/bitwarden-core/src/key_management/crypto_client.rs +++ b/crates/bitwarden-core/src/key_management/crypto_client.rs @@ -1,16 +1,13 @@ -use std::sync::RwLock; - use bitwarden_api_api::models::AccountKeysRequestModel; #[cfg(feature = "wasm")] use bitwarden_crypto::safe::PasswordProtectedKeyEnvelope; use bitwarden_crypto::{ - CryptoError, Decryptable, Kdf, KeyConnectorKey, KeyStore, RotateableKeySet, SymmetricCryptoKey, + CryptoError, Decryptable, Kdf, KeyConnectorKey, RotateableKeySet, SymmetricCryptoKey, }; #[cfg(feature = "internal")] use bitwarden_crypto::{EncString, UnsignedSharedKey}; use bitwarden_encoding::B64; use bitwarden_error::bitwarden_error; -use rand_chacha::{ChaCha8Rng, rand_core::SeedableRng}; #[cfg(feature = "wasm")] use wasm_bindgen::prelude::*; @@ -213,53 +210,49 @@ impl CryptoClient { pub fn make_user_key_connector_registration( &self, user_id: UserId, - ) -> Result< - ( - WrappedAccountCryptographicState, - EncString, - SymmetricCryptoKey, - AccountKeysRequestModel, - KeyConnectorKey, - ), - AccountCryptographyMakeKeysError, - > { + ) -> Result { let mut ctx = self.client.internal.get_key_store().context_mut(); - let (user_key, wrapped_state) = + let (user_key_id, wrapped_state) = WrappedAccountCryptographicState::make(&mut ctx, user_id) .map_err(AccountCryptographyMakeKeysError::AccountCryptographyInitialization)?; #[expect(deprecated)] - let user_key = ctx.dangerous_get_symmetric_key(user_key)?; + let user_key = ctx.dangerous_get_symmetric_key(user_key_id)?.to_owned(); // Key Connector unlock method - let mut rng = ChaCha8Rng::from_seed([0u8; 32]); - let key_connector_key = KeyConnectorKey::generate(&mut rng); + let key_connector_key = KeyConnectorKey::make(); let wrapped_user_key = key_connector_key - .encrypt_user_key(user_key) + .encrypt_user_key(&user_key) .map_err(AccountCryptographyMakeKeysError::Crypto)?; - let store = KeyStore::default(); - let mut ctx = store.context_mut(); - let user_key_id = ctx.add_local_symmetric_key(user_key.to_owned()); - let security_state = RwLock::new(None); - wrapped_state - .set_to_context(&security_state, user_key_id, &store, ctx) - .map_err(AccountCryptographyMakeKeysError::AccountCryptographyInitialization)?; - let cryptography_state_request_model = wrapped_state - .to_request_model(&store) + .to_request_model(&user_key_id, &mut ctx) .map_err(AccountCryptographyMakeKeysError::AccountCryptographyInitialization)?; - Ok(( - wrapped_state, - wrapped_user_key, - user_key.to_owned(), - cryptography_state_request_model, + Ok(MakeKeyConnectorRegistrationResponse { + account_cryptographic_state: wrapped_state, + key_connector_key_wrapped_user_key: wrapped_user_key, + user_key, + account_keys_request: cryptography_state_request_model, key_connector_key, - )) + }) } } +/// The response from `make_user_key_connector_registration`. +pub struct MakeKeyConnectorRegistrationResponse { + /// The account cryptographic state + pub account_cryptographic_state: WrappedAccountCryptographicState, + /// Encrypted user's user key, wrapped with the key connector key + pub key_connector_key_wrapped_user_key: EncString, + /// The user's user key + pub user_key: SymmetricCryptoKey, + /// The request model for the account cryptographic state (also called Account Keys) + pub account_keys_request: AccountKeysRequestModel, + /// The key connector key used for unlocking + pub key_connector_key: KeyConnectorKey, +} + /// Errors that can occur during account cryptography key generation. #[bitwarden_error(flat)] #[derive(Debug, thiserror::Error)] diff --git a/crates/bitwarden-crypto/src/keys/key_connector_key.rs b/crates/bitwarden-crypto/src/keys/key_connector_key.rs index d8c790f71..ec5000e38 100644 --- a/crates/bitwarden-crypto/src/keys/key_connector_key.rs +++ b/crates/bitwarden-crypto/src/keys/key_connector_key.rs @@ -8,12 +8,13 @@ use typenum::U32; use crate::{EncString, SymmetricCryptoKey, keys::utils::stretch_key}; /// Key connector key, used to protect the user key. -#[derive(Clone, Debug)] +#[derive(Clone)] pub struct KeyConnectorKey(pub(super) Pin>>); impl KeyConnectorKey { - /// Generate a new random for KeyConnector. - pub fn generate(mut rng: impl rand::RngCore) -> Self { + /// Make a new random key for KeyConnector. + pub fn make() -> Self { + let mut rng = rand::thread_rng(); let mut key = Box::pin(GenericArray::::default()); rng.fill(key.as_mut_slice()); @@ -35,3 +36,9 @@ impl KeyConnectorKey { EncString::encrypt_aes256_hmac(user_key_bytes.as_ref(), &stretched_master_key) } } + +impl std::fmt::Debug for KeyConnectorKey { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("KeyConnectorKey").finish() + } +} From d1fa6e80513a1cb99e525f7e436a58ac4b844412 Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Thu, 18 Dec 2025 10:08:08 +0100 Subject: [PATCH 04/10] lock file update --- Cargo.lock | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 24f977de9..ccc0c4418 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -526,7 +526,10 @@ dependencies = [ name = "bitwarden-auth" version = "2.0.0" dependencies = [ + "bitwarden-api-api", "bitwarden-core", + "bitwarden-crypto", + "bitwarden-encoding", "bitwarden-error", "bitwarden-test", "chrono", @@ -535,6 +538,7 @@ dependencies = [ "serde_json", "thiserror 2.0.12", "tokio", + "tracing", "tsify", "uniffi", "wasm-bindgen", From 9f5c26ba936c29cdd2858b22bdc23b054c9aba8f Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Thu, 18 Dec 2025 13:34:32 +0100 Subject: [PATCH 05/10] key connector decrypt, api client, test coverage --- Cargo.lock | 2 + crates/bitwarden-auth/src/registration.rs | 103 +++--- crates/bitwarden-core/Cargo.toml | 2 + .../src/key_management/crypto_client.rs | 47 ++- .../src/key_management/key_connector.rs | 323 +++++++++++++++--- .../bitwarden-core/src/key_management/mod.rs | 4 +- .../src/keys/key_connector_key.rs | 158 ++++++++- 7 files changed, 542 insertions(+), 97 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ccc0c4418..ec9fe94b9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -604,6 +604,7 @@ dependencies = [ "bitwarden-uuid", "chrono", "getrandom 0.2.16", + "mockall", "rand 0.8.5", "rand_chacha 0.3.1", "reqwest", @@ -623,6 +624,7 @@ dependencies = [ "uuid", "wasm-bindgen", "wasm-bindgen-futures", + "wiremock", "zeroize", "zxcvbn", ] diff --git a/crates/bitwarden-auth/src/registration.rs b/crates/bitwarden-auth/src/registration.rs index fc555e200..97acfbcc3 100644 --- a/crates/bitwarden-auth/src/registration.rs +++ b/crates/bitwarden-auth/src/registration.rs @@ -9,9 +9,8 @@ use bitwarden_api_api::models::SetKeyConnectorKeyRequestModel; use bitwarden_core::{ Client, OrganizationId, UserId, key_management::{ - AccountCryptographyMakeKeysError, + AccountCryptographyMakeKeysError, KeyConnectorApiClient, account_cryptographic_state::WrappedAccountCryptographicState, - key_connector_api_post_or_put_key_connector_key, }, }; use bitwarden_crypto::EncString; @@ -57,57 +56,73 @@ impl RegistrationClient { ) -> Result { let client = &self.client.internal; let api_client = &client.get_api_configurations().await.api_client; + let key_connector_api_client = + KeyConnectorApiClient::new(client, key_connector_url.as_str()); - // First call crypto API to get all keys - info!("Initializing account cryptography"); - let registration_crypto_result = self - .client - .crypto() - .make_user_key_connector_registration(user_id) - .map_err(UserRegistrationError::AccountCryptographyMakeKeys)?; - - info!("Posting key connector key to key connector server"); - key_connector_api_post_or_put_key_connector_key( - &self.client, - key_connector_url.as_str(), - ®istration_crypto_result.key_connector_key, + internal_post_keys_for_key_connector_registration( + self, + api_client, + &key_connector_api_client, + org_id, + user_id, ) .await + } +} + +async fn internal_post_keys_for_key_connector_registration( + registration_client: &RegistrationClient, + api_client: &bitwarden_api_api::apis::ApiClient, + key_connector_api_client: &KeyConnectorApiClient, + org_id: OrganizationId, + user_id: UserId, +) -> Result { + // First call crypto API to get all keys + info!("Initializing account cryptography"); + let registration_crypto_result = registration_client + .client + .crypto() + .make_user_key_connector_registration(user_id) + .map_err(UserRegistrationError::AccountCryptographyMakeKeys)?; + + info!("Posting key connector key to key connector server"); + key_connector_api_client + .post_or_put_key_connector_key(®istration_crypto_result.key_connector_key) + .await .map_err(|e| { error!("Failed to post key connector key to key connector server: {e:?}"); UserRegistrationError::KeyConnectorApi })?; - info!("Posting user account cryptographic state to server"); - let request = SetKeyConnectorKeyRequestModel { - key_connector_key_wrapped_user_key: Some( - registration_crypto_result - .key_connector_key_wrapped_user_key - .to_string(), - ), - account_keys: Some(Box::new(registration_crypto_result.account_keys_request)), - ..SetKeyConnectorKeyRequestModel::new(org_id.to_string()) - }; - api_client - .accounts_key_management_api() - .post_set_key_connector_key(Some(request)) - .await - .map_err(|e| { - error!("Failed to post account cryptographic state to server: {e:?}"); - UserRegistrationError::Api - })?; + info!("Posting user account cryptographic state to server"); + let request = SetKeyConnectorKeyRequestModel { + key_connector_key_wrapped_user_key: Some( + registration_crypto_result + .key_connector_key_wrapped_user_key + .to_string(), + ), + account_keys: Some(Box::new(registration_crypto_result.account_keys_request)), + ..SetKeyConnectorKeyRequestModel::new(org_id.to_string()) + }; + api_client + .accounts_key_management_api() + .post_set_key_connector_key(Some(request)) + .await + .map_err(|e| { + error!("Failed to post account cryptographic state to server: {e:?}"); + UserRegistrationError::Api + })?; - info!("User initialized!"); - // Note: This passing out of state and keys is temporary. Once SDK state management is more - // mature, the account cryptographic state and keys should be set directly here. - Ok(KeyConnectorRegistrationResult { - account_cryptographic_state: registration_crypto_result.account_cryptographic_state, - key_connector_key: registration_crypto_result.key_connector_key.to_base64(), - key_connector_key_wrapped_user_key: registration_crypto_result - .key_connector_key_wrapped_user_key, - user_key: registration_crypto_result.user_key.to_encoded().into(), - }) - } + info!("User initialized!"); + // Note: This passing out of state and keys is temporary. Once SDK state management is more + // mature, the account cryptographic state and keys should be set directly here. + Ok(KeyConnectorRegistrationResult { + account_cryptographic_state: registration_crypto_result.account_cryptographic_state, + key_connector_key: registration_crypto_result.key_connector_key.to_base64(), + key_connector_key_wrapped_user_key: registration_crypto_result + .key_connector_key_wrapped_user_key, + user_key: registration_crypto_result.user_key.to_encoded().into(), + }) } /// Result of Key Connector registration process. diff --git a/crates/bitwarden-core/Cargo.toml b/crates/bitwarden-core/Cargo.toml index c21f0f170..b3ae95611 100644 --- a/crates/bitwarden-core/Cargo.toml +++ b/crates/bitwarden-core/Cargo.toml @@ -76,8 +76,10 @@ rustls = { version = "0.23.19", default-features = false } rustls-platform-verifier = "0.6.0" [dev-dependencies] +mockall = { version = ">=0.13.1, <0.15" } rand_chacha = ">=0.3.1, <0.4.0" tokio = { workspace = true, features = ["rt"] } +wiremock = { workspace = true } zeroize = { version = ">=1.7.0, <2.0", features = ["derive", "aarch64"] } [lints] diff --git a/crates/bitwarden-core/src/key_management/crypto_client.rs b/crates/bitwarden-core/src/key_management/crypto_client.rs index 31b184ebd..a3c2e79cd 100644 --- a/crates/bitwarden-core/src/key_management/crypto_client.rs +++ b/crates/bitwarden-core/src/key_management/crypto_client.rs @@ -279,7 +279,10 @@ mod tests { use bitwarden_crypto::{BitwardenLegacyKeyBytes, SymmetricCryptoKey}; use super::*; - use crate::client::test_accounts::test_bitwarden_com_account; + use crate::{ + client::test_accounts::test_bitwarden_com_account, + key_management::crypto::InitUserCryptoMethod, + }; #[tokio::test] async fn test_enroll_pin_envelope() { @@ -309,4 +312,46 @@ mod tests { let user_key_final = SymmetricCryptoKey::try_from(&secret).unwrap(); assert_eq!(user_key_initial, user_key_final); } + + #[tokio::test] + async fn test_make_user_key_connector_registration_success() { + let user_id = UserId::new_v4(); + let email = "test@bitwarden.com"; + let client = Client::init_test_account(test_bitwarden_com_account()).await; + + let make_keys_response = client + .crypto() + .make_user_key_connector_registration(user_id) + .unwrap(); + + // Initialize a new client using the key connector key + let unlock_client = Client::new(None); + unlock_client + .crypto() + .initialize_user_crypto(InitUserCryptoRequest { + user_id: Some(user_id), + kdf_params: Kdf::default(), + email: email.to_owned(), + account_cryptographic_state: make_keys_response.account_cryptographic_state, + method: InitUserCryptoMethod::KeyConnector { + user_key: make_keys_response.key_connector_key_wrapped_user_key, + master_key: make_keys_response.key_connector_key.to_base64(), + }, + }) + .await + .expect("initializing user crypto with key connector key should succeed"); + + // Verify we can retrieve the user encryption key + let retrieved_key = unlock_client + .crypto() + .get_user_encryption_key() + .await + .expect("should be able to get user encryption key"); + + // The retrieved key should be a valid symmetric key + let retrieved_symmetric_key = SymmetricCryptoKey::try_from(retrieved_key) + .expect("retrieved key should be valid symmetric key"); + + assert_eq!(retrieved_symmetric_key, make_keys_response.user_key); + } } diff --git a/crates/bitwarden-core/src/key_management/key_connector.rs b/crates/bitwarden-core/src/key_management/key_connector.rs index eaa2c465f..973761030 100644 --- a/crates/bitwarden-core/src/key_management/key_connector.rs +++ b/crates/bitwarden-core/src/key_management/key_connector.rs @@ -1,9 +1,11 @@ +use std::sync::Arc; + use bitwarden_crypto::KeyConnectorKey; use reqwest::Method; use serde::{Deserialize, Serialize}; use thiserror::Error; -use crate::{ApiError, Client}; +use crate::{ApiError, client::internal::InternalClient}; #[allow(missing_docs)] #[derive(Debug, Error)] @@ -14,74 +16,101 @@ pub enum KeyConnectorApiError { Api(#[from] ApiError), } -/// Sends the key connector key to the Key Connector API. -/// If a key already exists, it will be updated; otherwise, a new key will be created for the user. -pub async fn key_connector_api_post_or_put_key_connector_key( - client: &Client, - key_connector_url: &str, - key_connector_key: &KeyConnectorKey, -) -> Result<(), KeyConnectorApiError> { - let request = KeyConnectorKeyRequestModel { - key: key_connector_key.to_base64().to_string(), - }; +/// Client for interacting with the Key Connector API. +pub struct KeyConnectorApiClient { + #[doc(hidden)] + internal_client: Arc, + /// The base URL of the Key Connector API. + key_connector_url: String, +} - if get_key_connector_user_keys(client, key_connector_url) - .await - .is_ok() - { - put_key_connector_user_keys(client, key_connector_url, request).await?; - } else { - post_key_connector_user_keys(client, key_connector_url, request).await?; +#[cfg_attr(test, mockall::automock)] +impl KeyConnectorApiClient { + /// Create a new Key Connector API client. + pub fn new(internal_client: &Arc, key_connector_url: &str) -> Self { + Self { + internal_client: Arc::clone(internal_client), + key_connector_url: key_connector_url.to_string(), + } } - Ok(()) -} + /// Sends the key connector key to the Key Connector API. + /// If a key already exists, it will be updated; otherwise, a new key will be created for the + /// user. + pub async fn post_or_put_key_connector_key( + &self, + key_connector_key: &KeyConnectorKey, + ) -> Result<(), KeyConnectorApiError> { + let request = KeyConnectorKeyRequestModel { + key: key_connector_key.to_base64().to_string(), + }; -async fn get_key_connector_user_keys( - client: &Client, - key_connector_url: &str, -) -> Result { - let response = - request_key_connector_user_keys(client, Method::GET, key_connector_url, None::<()>).await?; + if self.get_key_connector_user_keys().await.is_ok() { + self.put_key_connector_user_keys(request).await?; + } else { + self.post_key_connector_user_keys(request).await?; + } - let body = response.text().await.map_err(KeyConnectorApiError::from)?; - let response_model = serde_json::from_str::(&body)?; - Ok(response_model) -} + Ok(()) + } -async fn post_key_connector_user_keys( - client: &Client, - key_connector_url: &str, - request_model: KeyConnectorKeyRequestModel, -) -> Result<(), KeyConnectorApiError> { - request_key_connector_user_keys(client, Method::POST, key_connector_url, Some(request_model)) + async fn get_key_connector_user_keys( + &self, + ) -> Result { + let response = request_key_connector_user_keys( + &self.internal_client, + Method::GET, + &self.key_connector_url, + None::<()>, + ) .await?; - Ok(()) -} + let body = response.text().await.map_err(KeyConnectorApiError::from)?; + let response_model = serde_json::from_str::(&body)?; + Ok(response_model) + } -async fn put_key_connector_user_keys( - client: &Client, - key_connector_url: &str, - request_model: KeyConnectorKeyRequestModel, -) -> Result<(), KeyConnectorApiError> { - request_key_connector_user_keys(client, Method::PUT, key_connector_url, Some(request_model)) + async fn post_key_connector_user_keys( + &self, + request_model: KeyConnectorKeyRequestModel, + ) -> Result<(), KeyConnectorApiError> { + request_key_connector_user_keys( + &self.internal_client, + Method::POST, + &self.key_connector_url, + Some(request_model), + ) .await?; - Ok(()) + Ok(()) + } + + async fn put_key_connector_user_keys( + &self, + request_model: KeyConnectorKeyRequestModel, + ) -> Result<(), KeyConnectorApiError> { + request_key_connector_user_keys( + &self.internal_client, + Method::PUT, + &self.key_connector_url, + Some(request_model), + ) + .await?; + + Ok(()) + } } async fn request_key_connector_user_keys( - client: &Client, + client: &InternalClient, method: Method, key_connector_url: &str, body: Option, ) -> Result { let url = format!("{}/user-keys", key_connector_url); - let config = client.internal.get_api_configurations().await; + let config = client.get_api_configurations().await; let mut request = client - .internal .get_http_client() .request(method, url) .header(reqwest::header::CONTENT_TYPE, "application/json") @@ -126,3 +155,201 @@ struct KeyConnectorKeyResponseModel { #[serde(rename = "key", alias = "Key")] pub key: String, } + +#[cfg(test)] +mod tests { + use wiremock::{ + Mock, MockServer, ResponseTemplate, + matchers::{header, method, path}, + }; + + use super::*; + use crate::{Client, ClientSettings, DeviceType}; + + const ACCESS_TOKEN: &str = "test_access_token"; + + async fn setup_mock_server_with_auth() -> (MockServer, Client) { + let server = MockServer::start().await; + + let settings = ClientSettings { + identity_url: format!("http://{}/identity", server.address()), + api_url: format!("http://{}/api", server.address()), + user_agent: "Bitwarden Rust-SDK [TEST]".into(), + device_type: DeviceType::SDK, + bitwarden_client_version: None, + }; + + let client = Client::new(Some(settings)); + + // Set up authentication token + client + .internal + .set_tokens(ACCESS_TOKEN.to_string(), None, 3600); + + (server, client) + } + + #[tokio::test] + async fn test_post_when_key_doesnt_exist() { + let (server, client) = setup_mock_server_with_auth().await; + let key_connector_url = format!("http://{}", server.address()); + + // Mock GET to return 404 (key doesn't exist) + Mock::given(method("GET")) + .and(path("/user-keys")) + .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) + .respond_with(ResponseTemplate::new(404)) + .expect(1) + .mount(&server) + .await; + + // Mock POST to succeed + Mock::given(method("POST")) + .and(path("/user-keys")) + .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) + .and(header("content-type", "application/json")) + .respond_with(ResponseTemplate::new(200)) + .expect(1) + .mount(&server) + .await; + + let key = KeyConnectorKey::make(); + let key_connector_api = + KeyConnectorApiClient::new(&client.internal, key_connector_url.as_str()); + let result = key_connector_api.post_or_put_key_connector_key(&key).await; + + assert!(result.is_ok()); + } + + #[tokio::test] + async fn test_put_when_key_exists() { + let (server, client) = setup_mock_server_with_auth().await; + let key_connector_url = format!("http://{}", server.address()); + + // Mock GET to return 200 (key exists) + Mock::given(method("GET")) + .and(path("/user-keys")) + .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "key": "existing_key_connector_key" + }))) + .expect(1) + .mount(&server) + .await; + + // Mock PUT to succeed + Mock::given(method("PUT")) + .and(path("/user-keys")) + .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) + .and(header("content-type", "application/json")) + .respond_with(ResponseTemplate::new(200)) + .expect(1) + .mount(&server) + .await; + + let key = KeyConnectorKey::make(); + let key_connector_api = + KeyConnectorApiClient::new(&client.internal, key_connector_url.as_str()); + let result = key_connector_api.post_or_put_key_connector_key(&key).await; + + assert!(result.is_ok()); + } + + #[tokio::test] + async fn test_error_when_both_get_and_post_fail() { + let (server, client) = setup_mock_server_with_auth().await; + let key_connector_url = format!("http://{}", server.address()); + + // Mock GET to return 500 (server error) + Mock::given(method("GET")) + .and(path("/user-keys")) + .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) + .respond_with(ResponseTemplate::new(500)) + .expect(1) + .mount(&server) + .await; + + // Mock POST to also fail (since GET failed, POST will be attempted) + Mock::given(method("POST")) + .and(path("/user-keys")) + .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) + .respond_with(ResponseTemplate::new(500)) + .expect(1) + .mount(&server) + .await; + + let key = KeyConnectorKey::make(); + let key_connector_api = + KeyConnectorApiClient::new(&client.internal, key_connector_url.as_str()); + let result = key_connector_api.post_or_put_key_connector_key(&key).await; + + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), KeyConnectorApiError::Api(_))); + } + + #[tokio::test] + async fn test_error_when_post_fails() { + let (server, client) = setup_mock_server_with_auth().await; + let key_connector_url = format!("http://{}", server.address()); + + // Mock GET to return 404 (key doesn't exist) + Mock::given(method("GET")) + .and(path("/user-keys")) + .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) + .respond_with(ResponseTemplate::new(404)) + .expect(1) + .mount(&server) + .await; + + // Mock POST to fail with 500 + Mock::given(method("POST")) + .and(path("/user-keys")) + .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) + .respond_with(ResponseTemplate::new(500)) + .expect(1) + .mount(&server) + .await; + + let key = KeyConnectorKey::make(); + let key_connector_api = + KeyConnectorApiClient::new(&client.internal, key_connector_url.as_str()); + let result = key_connector_api.post_or_put_key_connector_key(&key).await; + + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), KeyConnectorApiError::Api(_))); + } + + #[tokio::test] + async fn test_error_when_put_fails() { + let (server, client) = setup_mock_server_with_auth().await; + let key_connector_url = format!("http://{}", server.address()); + + // Mock GET to return 200 (key exists) + Mock::given(method("GET")) + .and(path("/user-keys")) + .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "key": "existing_key_connector_key" + }))) + .expect(1) + .mount(&server) + .await; + + // Mock PUT to fail with 500 + Mock::given(method("PUT")) + .and(path("/user-keys")) + .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) + .respond_with(ResponseTemplate::new(500)) + .expect(1) + .mount(&server) + .await; + + let key = KeyConnectorKey::make(); + let key_connector_api = + KeyConnectorApiClient::new(&client.internal, key_connector_url.as_str()); + let result = key_connector_api.post_or_put_key_connector_key(&key).await; + + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), KeyConnectorApiError::Api(_))); + } +} diff --git a/crates/bitwarden-core/src/key_management/mod.rs b/crates/bitwarden-core/src/key_management/mod.rs index 27b33507c..e783d3019 100644 --- a/crates/bitwarden-core/src/key_management/mod.rs +++ b/crates/bitwarden-core/src/key_management/mod.rs @@ -40,8 +40,10 @@ pub use user_decryption::UserDecryptionData; #[cfg(feature = "internal")] mod key_connector; +#[cfg(test)] +pub use key_connector::MockKeyConnectorApiClient; #[cfg(feature = "internal")] -pub use key_connector::{KeyConnectorApiError, key_connector_api_post_or_put_key_connector_key}; +pub use key_connector::{KeyConnectorApiClient, KeyConnectorApiError}; use crate::OrganizationId; diff --git a/crates/bitwarden-crypto/src/keys/key_connector_key.rs b/crates/bitwarden-crypto/src/keys/key_connector_key.rs index ec5000e38..1c75e00cf 100644 --- a/crates/bitwarden-crypto/src/keys/key_connector_key.rs +++ b/crates/bitwarden-crypto/src/keys/key_connector_key.rs @@ -5,7 +5,10 @@ use generic_array::GenericArray; use rand::Rng; use typenum::U32; -use crate::{EncString, SymmetricCryptoKey, keys::utils::stretch_key}; +use crate::{ + BitwardenLegacyKeyBytes, CryptoError, EncString, KeyDecryptable, SymmetricCryptoKey, + keys::utils::stretch_key, +}; /// Key connector key, used to protect the user key. #[derive(Clone)] @@ -31,9 +34,38 @@ impl KeyConnectorKey { &self, user_key: &SymmetricCryptoKey, ) -> crate::error::Result { - let stretched_master_key = stretch_key(&self.0)?; + let stretched_key = stretch_key(&self.0)?; let user_key_bytes = user_key.to_encoded(); - EncString::encrypt_aes256_hmac(user_key_bytes.as_ref(), &stretched_master_key) + EncString::encrypt_aes256_hmac(user_key_bytes.as_ref(), &stretched_key) + } + + /// Unwraps the user key with this key connector key. + pub fn decrypt_user_key( + &self, + user_key: EncString, + ) -> crate::error::Result { + let dec: Vec = match user_key { + // Legacy. user_keys were encrypted using `Aes256Cbc_B64` a long time ago. We've since + // moved to using `Aes256Cbc_HmacSha256_B64`. However, we still need to support + // decrypting these old keys. + EncString::Aes256Cbc_B64 { .. } => { + let legacy_key = SymmetricCryptoKey::Aes256CbcKey(super::Aes256CbcKey { + enc_key: Box::pin(GenericArray::clone_from_slice(&self.0)), + }); + user_key.decrypt_with_key(&legacy_key)? + } + EncString::Aes256Cbc_HmacSha256_B64 { .. } => { + let stretched_key = SymmetricCryptoKey::Aes256CbcHmacKey(stretch_key(&self.0)?); + user_key.decrypt_with_key(&stretched_key)? + } + _ => { + return Err(CryptoError::OperationNotSupported( + crate::error::UnsupportedOperationError::EncryptionNotImplementedForKey, + )); + } + }; + + SymmetricCryptoKey::try_from(&BitwardenLegacyKeyBytes::from(dec)) } } @@ -42,3 +74,123 @@ impl std::fmt::Debug for KeyConnectorKey { f.debug_struct("KeyConnectorKey").finish() } } + +#[cfg(test)] +mod tests { + use bitwarden_encoding::B64; + use coset::iana::KeyOperation; + use rand_chacha::rand_core::SeedableRng; + + use super::KeyConnectorKey; + use crate::{BitwardenLegacyKeyBytes, SymmetricCryptoKey, UserKey}; + + const KEY_CONNECTOR_KEY_BYTES: [u8; 32] = [ + 31, 79, 104, 226, 150, 71, 177, 90, 194, 80, 172, 209, 17, 129, 132, 81, 138, 167, 69, 167, + 254, 149, 2, 27, 39, 197, 64, 42, 22, 195, 86, 75, + ]; + + #[test] + fn test_make_two_different_keys() { + let key1 = KeyConnectorKey::make(); + let key2 = KeyConnectorKey::make(); + assert_ne!(key1.0.as_slice(), key2.0.as_slice()); + } + + #[test] + fn test_to_base64() { + let key = KeyConnectorKey(Box::pin(KEY_CONNECTOR_KEY_BYTES.into())); + + assert_eq!( + "H09o4pZHsVrCUKzREYGEUYqnRaf+lQIbJ8VAKhbDVks=", + key.to_base64().to_string() + ); + } + + #[test] + fn test_encrypt_decrypt_user_key_aes256_cbc_hmac() { + let rng = rand_chacha::ChaCha8Rng::from_seed([0u8; 32]); + + let key_connector_key = KeyConnectorKey(Box::pin(KEY_CONNECTOR_KEY_BYTES.into())); + + let user_key = SymmetricCryptoKey::make_aes256_cbc_hmac_key_internal(rng); + let wrapped_user_key = key_connector_key.encrypt_user_key(&user_key).unwrap(); + let user_key = UserKey::new(user_key); + + let decrypted_user_key = key_connector_key + .decrypt_user_key(wrapped_user_key) + .unwrap(); + + let SymmetricCryptoKey::Aes256CbcHmacKey(user_key_unwrapped) = &decrypted_user_key else { + panic!("User key is not an Aes256CbcHmacKey"); + }; + + assert_eq!( + user_key_unwrapped.enc_key.as_slice(), + [ + 62, 0, 239, 47, 137, 95, 64, 214, 127, 91, 184, 232, 31, 9, 165, 161, 44, 132, 14, + 195, 206, 154, 127, 59, 24, 27, 225, 136, 239, 113, 26, 30 + ] + ); + assert_eq!( + user_key_unwrapped.mac_key.as_slice(), + [ + 152, 76, 225, 114, 185, 33, 111, 65, 159, 68, 83, 103, 69, 109, 86, 25, 49, 74, 66, + 163, 218, 134, 176, 1, 56, 123, 253, 184, 14, 12, 254, 66 + ] + ); + + assert_eq!( + decrypted_user_key, user_key.0, + "Decrypted key doesn't match user key" + ); + } + + #[test] + fn test_encrypt_decrypt_user_key_xchacha20_poly1305() { + let key_connector_key = KeyConnectorKey(Box::pin(KEY_CONNECTOR_KEY_BYTES.into())); + + let user_key_b64: B64 = "pQEEAlDib+JxbqMBlcd3KTUesbufAzoAARFvBIQDBAUGIFggt79surJXmqhPhYuuqi9ZyPfieebmtw2OsmN5SDrb4yUB".parse() + .unwrap(); + let user_key = + SymmetricCryptoKey::try_from(&BitwardenLegacyKeyBytes::from(&user_key_b64)).unwrap(); + let wrapped_user_key = key_connector_key.encrypt_user_key(&user_key).unwrap(); + let user_key = UserKey::new(user_key); + + let decrypted_user_key = key_connector_key + .decrypt_user_key(wrapped_user_key) + .unwrap(); + + let SymmetricCryptoKey::XChaCha20Poly1305Key(user_key_unwrapped) = &decrypted_user_key + else { + panic!("User key is not an XChaCha20Poly1305Key"); + }; + + assert_eq!( + user_key_unwrapped.enc_key.as_slice(), + [ + 183, 191, 108, 186, 178, 87, 154, 168, 79, 133, 139, 174, 170, 47, 89, 200, 247, + 226, 121, 230, 230, 183, 13, 142, 178, 99, 121, 72, 58, 219, 227, 37 + ] + ); + assert_eq!( + user_key_unwrapped.key_id.as_slice(), + [ + 226, 111, 226, 113, 110, 163, 1, 149, 199, 119, 41, 53, 30, 177, 187, 159 + ] + ); + assert_eq!( + user_key_unwrapped.supported_operations, + [ + KeyOperation::Encrypt, + KeyOperation::Decrypt, + KeyOperation::WrapKey, + KeyOperation::UnwrapKey + ] + ); + + assert_eq!( + decrypted_user_key, user_key.0, + "Decrypted key doesn't match user key" + ); + } +} From 282f98eb7eb9d5c8c4de5d85416da26a7992c0df Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Sat, 20 Dec 2025 13:27:05 +0100 Subject: [PATCH 06/10] bitwarden-api-key-connector client create --- Cargo.lock | 41 +- Cargo.toml | 1 + crates/bitwarden-api-key-connector/Cargo.toml | 24 ++ .../src/apis/configuration.rs | 24 ++ .../src/apis/mod.rs | 75 ++++ .../src/apis/user_keys_api.rs | 196 ++++++++++ crates/bitwarden-api-key-connector/src/lib.rs | 4 + .../src/models/mod.rs | 2 + .../src/models/user_key_request_model.rs | 7 + .../src/models/user_key_response_model.rs | 7 + crates/bitwarden-auth/Cargo.toml | 4 +- crates/bitwarden-auth/src/registration.rs | 113 +++++- crates/bitwarden-core/Cargo.toml | 3 +- crates/bitwarden-core/src/client/client.rs | 4 +- crates/bitwarden-core/src/client/internal.rs | 28 +- .../src/key_management/key_connector.rs | 355 ------------------ .../bitwarden-core/src/key_management/mod.rs | 7 - 17 files changed, 501 insertions(+), 394 deletions(-) create mode 100644 crates/bitwarden-api-key-connector/Cargo.toml create mode 100644 crates/bitwarden-api-key-connector/src/apis/configuration.rs create mode 100644 crates/bitwarden-api-key-connector/src/apis/mod.rs create mode 100644 crates/bitwarden-api-key-connector/src/apis/user_keys_api.rs create mode 100644 crates/bitwarden-api-key-connector/src/lib.rs create mode 100644 crates/bitwarden-api-key-connector/src/models/mod.rs create mode 100644 crates/bitwarden-api-key-connector/src/models/user_key_request_model.rs create mode 100644 crates/bitwarden-api-key-connector/src/models/user_key_response_model.rs delete mode 100644 crates/bitwarden-core/src/key_management/key_connector.rs diff --git a/Cargo.lock b/Cargo.lock index ec9fe94b9..b417ce8cb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -522,11 +522,25 @@ dependencies = [ "uuid", ] +[[package]] +name = "bitwarden-api-key-connector" +version = "2.0.0" +dependencies = [ + "async-trait", + "mockall", + "reqwest", + "serde", + "serde_json", + "tokio", + "wiremock", +] + [[package]] name = "bitwarden-auth" version = "2.0.0" dependencies = [ "bitwarden-api-api", + "bitwarden-api-key-connector", "bitwarden-core", "bitwarden-crypto", "bitwarden-encoding", @@ -541,6 +555,7 @@ dependencies = [ "tracing", "tsify", "uniffi", + "uuid", "wasm-bindgen", "wasm-bindgen-futures", "wiremock", @@ -596,6 +611,7 @@ dependencies = [ "async-trait", "bitwarden-api-api", "bitwarden-api-identity", + "bitwarden-api-key-connector", "bitwarden-crypto", "bitwarden-encoding", "bitwarden-error", @@ -604,7 +620,6 @@ dependencies = [ "bitwarden-uuid", "chrono", "getrandom 0.2.16", - "mockall", "rand 0.8.5", "rand_chacha 0.3.1", "reqwest", @@ -624,7 +639,6 @@ dependencies = [ "uuid", "wasm-bindgen", "wasm-bindgen-futures", - "wiremock", "zeroize", "zxcvbn", ] @@ -1933,12 +1947,12 @@ dependencies = [ [[package]] name = "deadpool" -version = "0.10.0" +version = "0.12.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fb84100978c1c7b37f09ed3ce3e5f843af02c2a2c431bae5b19230dad2c1b490" +checksum = "0be2b1d1d6ec8d846f05e137292d0b89133caf95ef33695424c09568bdd39b1b" dependencies = [ - "async-trait", "deadpool-runtime", + "lazy_static", "num_cpus", "tokio", ] @@ -2803,13 +2817,14 @@ dependencies = [ [[package]] name = "hyper" -version = "1.6.0" +version = "1.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cc2b571658e38e0c01b1fdca3bbbe93c00d3d71693ff2770043f8c29bc7d6f80" +checksum = "2ab2d4f250c3d7b1c9fcdff1cece94ea4e2dfbec68614f7b87cb205f24ca9d11" dependencies = [ + "atomic-waker", "bytes", "futures-channel", - "futures-util", + "futures-core", "h2", "http", "http-body", @@ -2817,6 +2832,7 @@ dependencies = [ "httpdate", "itoa", "pin-project-lite", + "pin-utils", "smallvec", "tokio", "want", @@ -5908,7 +5924,7 @@ dependencies = [ "serde", "tempfile", "textwrap", - "toml 0.8.23", + "toml 0.9.9+spec-1.0.0", "uniffi_internal_macros", "uniffi_meta", "uniffi_pipeline", @@ -5961,7 +5977,7 @@ dependencies = [ "quote", "serde", "syn", - "toml 0.8.23", + "toml 0.9.9+spec-1.0.0", "uniffi_meta", ] @@ -6874,12 +6890,11 @@ dependencies = [ [[package]] name = "wiremock" -version = "0.6.4" +version = "0.6.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a2b8b99d4cdbf36b239a9532e31fe4fb8acc38d1897c1761e161550a7dc78e6a" +checksum = "08db1edfb05d9b3c1542e521aea074442088292f00b5f28e435c714a98f85031" dependencies = [ "assert-json-diff", - "async-trait", "base64 0.22.1", "deadpool", "futures", diff --git a/Cargo.toml b/Cargo.toml index 3d423fa13..03a8ed056 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,6 +22,7 @@ keywords = ["bitwarden"] bitwarden = { path = "crates/bitwarden", version = "=1.0.0" } bitwarden-api-api = { path = "crates/bitwarden-api-api", version = "=2.0.0" } bitwarden-api-identity = { path = "crates/bitwarden-api-identity", version = "=2.0.0" } +bitwarden-api-key-connector = { path = "crates/bitwarden-api-key-connector", version = "=2.0.0" } bitwarden-auth = { path = "crates/bitwarden-auth", version = "=2.0.0" } bitwarden-cli = { path = "crates/bitwarden-cli", version = "=2.0.0" } bitwarden-collections = { path = "crates/bitwarden-collections", version = "=2.0.0" } diff --git a/crates/bitwarden-api-key-connector/Cargo.toml b/crates/bitwarden-api-key-connector/Cargo.toml new file mode 100644 index 000000000..f3d90aac8 --- /dev/null +++ b/crates/bitwarden-api-key-connector/Cargo.toml @@ -0,0 +1,24 @@ +[package] +name = "bitwarden-api-key-connector" +description = "Api bindings for the Bitwarden Key Connector API." +categories = ["api-bindings"] + +version.workspace = true +authors.workspace = true +edition.workspace = true +rust-version.workspace = true +homepage.workspace = true +repository.workspace = true +license-file.workspace = true +keywords.workspace = true + +[dependencies] +async-trait = { workspace = true } +mockall = { version = ">=0.13.1, <0.15" } +reqwest = { workspace = true } +tokio = { workspace = true } +serde = { workspace = true } +serde_json = { workspace = true } + +[dev-dependencies] +wiremock = { workspace = true } \ No newline at end of file diff --git a/crates/bitwarden-api-key-connector/src/apis/configuration.rs b/crates/bitwarden-api-key-connector/src/apis/configuration.rs new file mode 100644 index 000000000..d0cf2aac1 --- /dev/null +++ b/crates/bitwarden-api-key-connector/src/apis/configuration.rs @@ -0,0 +1,24 @@ +#[derive(Debug, Clone)] +pub struct Configuration { + pub base_path: String, + pub user_agent: Option, + pub client: reqwest::Client, + pub oauth_access_token: Option, +} + +impl Configuration { + pub fn new() -> Configuration { + Configuration::default() + } +} + +impl Default for Configuration { + fn default() -> Self { + Configuration { + base_path: "https://key-connector.bitwarden.com".to_owned(), + user_agent: Some("api/key-connector/rust".to_owned()), + client: reqwest::Client::new(), + oauth_access_token: None, + } + } +} diff --git a/crates/bitwarden-api-key-connector/src/apis/mod.rs b/crates/bitwarden-api-key-connector/src/apis/mod.rs new file mode 100644 index 000000000..6c7a3c3a7 --- /dev/null +++ b/crates/bitwarden-api-key-connector/src/apis/mod.rs @@ -0,0 +1,75 @@ +use std::sync::Arc; + +use crate::apis::configuration::Configuration; + +pub mod configuration; +pub mod user_keys_api; + +#[derive(Debug, Clone)] +pub struct ResponseContent { + pub status: reqwest::StatusCode, + pub content: String, +} + +#[allow(missing_docs)] +#[derive(Debug)] +pub enum Error { + Reqwest(reqwest::Error), + Serde(serde_json::Error), + Io(std::io::Error), + ResponseError(ResponseContent), +} + +impl From for Error { + fn from(e: reqwest::Error) -> Self { + Error::Reqwest(e) + } +} + +impl From for Error { + fn from(e: serde_json::Error) -> Self { + Error::Serde(e) + } +} + +impl From for Error { + fn from(e: std::io::Error) -> Self { + Error::Io(e) + } +} + +pub enum ApiClient { + Real(ApiClientReal), + Mock(ApiClientMock), +} + +pub struct ApiClientReal { + user_keys_api: user_keys_api::UserKeysApiClient, +} + +pub struct ApiClientMock { + pub user_keys_api: user_keys_api::MockUserKeysApi, +} + +impl ApiClient { + pub fn new(configuration: &Arc) -> Self { + Self::Real(ApiClientReal { + user_keys_api: user_keys_api::UserKeysApiClient::new(configuration.clone()), + }) + } + + pub fn new_mocked(func: impl FnOnce(&mut ApiClientMock)) -> Self { + let mut mock = ApiClientMock { + user_keys_api: user_keys_api::MockUserKeysApi::new(), + }; + func(&mut mock); + Self::Mock(mock) + } + + pub fn user_keys_api(&self) -> &dyn user_keys_api::UserKeysApi { + match self { + ApiClient::Real(real) => &real.user_keys_api, + ApiClient::Mock(mock) => &mock.user_keys_api, + } + } +} diff --git a/crates/bitwarden-api-key-connector/src/apis/user_keys_api.rs b/crates/bitwarden-api-key-connector/src/apis/user_keys_api.rs new file mode 100644 index 000000000..b5387f924 --- /dev/null +++ b/crates/bitwarden-api-key-connector/src/apis/user_keys_api.rs @@ -0,0 +1,196 @@ +use std::sync::Arc; + +use async_trait::async_trait; +use configuration::Configuration; +use mockall::automock; +use reqwest::Method; +use serde::Serialize; + +use crate::{ + apis::{Error, configuration}, + models::{ + user_key_request_model::UserKeyKeyRequestModel, + user_key_response_model::UserKeyResponseModel, + }, +}; + +#[automock] +#[cfg_attr(target_arch = "wasm32", async_trait(?Send))] +#[cfg_attr(not(target_arch = "wasm32"), async_trait::async_trait)] +pub trait UserKeysApi: Send + Sync { + /// GET /user-keys + async fn get_user_key(&self) -> Result; + + /// POST /user-keys + async fn post_user_key(&self, request_model: UserKeyKeyRequestModel) -> Result<(), Error>; + + /// PUT /user-keys + async fn put_user_key(&self, request_model: UserKeyKeyRequestModel) -> Result<(), Error>; +} + +pub struct UserKeysApiClient { + configuration: Arc, +} + +impl UserKeysApiClient { + pub fn new(configuration: Arc) -> Self { + Self { configuration } + } +} + +#[cfg_attr(target_arch = "wasm32", async_trait(?Send))] +#[cfg_attr(not(target_arch = "wasm32"), async_trait)] +impl UserKeysApi for UserKeysApiClient { + async fn get_user_key(&self) -> Result { + let response = request(&self.configuration, Method::GET, None::<()>).await?; + + let body = response.text().await?; + let response_model = serde_json::from_str::(&body)?; + Ok(response_model) + } + + async fn post_user_key(&self, request_model: UserKeyKeyRequestModel) -> Result<(), Error> { + request(&self.configuration, Method::POST, Some(request_model)).await?; + + Ok(()) + } + + async fn put_user_key(&self, request_model: UserKeyKeyRequestModel) -> Result<(), Error> { + request(&self.configuration, Method::PUT, Some(request_model)).await?; + + Ok(()) + } +} + +async fn request( + configuration: &Arc, + method: Method, + body: Option, +) -> Result { + let url = format!("{}/user-keys", configuration.base_path); + + let mut request = configuration + .client + .request(method, url) + .header(reqwest::header::CONTENT_TYPE, "application/json") + .header(reqwest::header::ACCEPT, "application/json"); + + if let Some(ref user_agent) = configuration.user_agent { + request = request.header(reqwest::header::USER_AGENT, user_agent.clone()); + } + if let Some(ref access_token) = configuration.oauth_access_token { + request = request.bearer_auth(access_token.clone()); + } + if let Some(ref body) = body { + request = + request.body(serde_json::to_string(&body).expect("Serialize should be infallible")) + } + + let response = request.send().await?; + + Ok(response.error_for_status()?) +} + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use wiremock::{ + Mock, MockServer, ResponseTemplate, + matchers::{header, method, path}, + }; + + use crate::{ + apis::{ + configuration::Configuration, + user_keys_api::{UserKeysApi, UserKeysApiClient}, + }, + models::user_key_request_model::UserKeyKeyRequestModel, + }; + + const ACCESS_TOKEN: &str = "test_access_token"; + const KEY_CONNECTOR_KEY: &str = "test_key_connector_key"; + + async fn setup_mock_server_with_auth() -> (MockServer, Configuration) { + let server = MockServer::start().await; + + let configuration = Configuration { + base_path: format!("http://{}", server.address()), + user_agent: Some("Bitwarden Rust-SDK [TEST]".to_string()), + client: reqwest::Client::new(), + oauth_access_token: Some(ACCESS_TOKEN.to_string()), + }; + + (server, configuration) + } + + #[tokio::test] + async fn test_get() { + let (server, configuration) = setup_mock_server_with_auth().await; + + Mock::given(method("GET")) + .and(path("/user-keys")) + .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "key": KEY_CONNECTOR_KEY.to_string() + }))) + .expect(1) + .mount(&server) + .await; + + let api_client = UserKeysApiClient::new(Arc::new(configuration)); + + let result = api_client.get_user_key().await; + + assert!(result.is_ok()); + assert_eq!(KEY_CONNECTOR_KEY, result.unwrap().key); + } + + #[tokio::test] + async fn test_post() { + let (server, configuration) = setup_mock_server_with_auth().await; + + Mock::given(method("POST")) + .and(path("/user-keys")) + .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) + .and(header("content-type", "application/json")) + .respond_with(ResponseTemplate::new(200)) + .expect(1) + .mount(&server) + .await; + + let request_model = UserKeyKeyRequestModel { + key: KEY_CONNECTOR_KEY.to_string(), + }; + + let api_client = UserKeysApiClient::new(Arc::new(configuration)); + + let result = api_client.post_user_key(request_model).await; + + assert!(result.is_ok()); + } + + #[tokio::test] + async fn test_put() { + let (server, configuration) = setup_mock_server_with_auth().await; + + Mock::given(method("PUT")) + .and(path("/user-keys")) + .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) + .and(header("content-type", "application/json")) + .respond_with(ResponseTemplate::new(200)) + .expect(1) + .mount(&server) + .await; + + let request_model = UserKeyKeyRequestModel { + key: KEY_CONNECTOR_KEY.to_string(), + }; + + let api_client = UserKeysApiClient::new(Arc::new(configuration)); + + let result = api_client.put_user_key(request_model).await; + + assert!(result.is_ok()); + } +} diff --git a/crates/bitwarden-api-key-connector/src/lib.rs b/crates/bitwarden-api-key-connector/src/lib.rs new file mode 100644 index 000000000..733dbfabd --- /dev/null +++ b/crates/bitwarden-api-key-connector/src/lib.rs @@ -0,0 +1,4 @@ +//! Client for interacting with the Key Connector API. + +pub mod apis; +pub mod models; diff --git a/crates/bitwarden-api-key-connector/src/models/mod.rs b/crates/bitwarden-api-key-connector/src/models/mod.rs new file mode 100644 index 000000000..832270c9a --- /dev/null +++ b/crates/bitwarden-api-key-connector/src/models/mod.rs @@ -0,0 +1,2 @@ +pub mod user_key_request_model; +pub mod user_key_response_model; diff --git a/crates/bitwarden-api-key-connector/src/models/user_key_request_model.rs b/crates/bitwarden-api-key-connector/src/models/user_key_request_model.rs new file mode 100644 index 000000000..b00ccb738 --- /dev/null +++ b/crates/bitwarden-api-key-connector/src/models/user_key_request_model.rs @@ -0,0 +1,7 @@ +use serde::{Deserialize, Serialize}; + +#[derive(Clone, Default, Debug, PartialEq, Serialize, Deserialize)] +pub struct UserKeyKeyRequestModel { + #[serde(rename = "key", alias = "Key")] + pub key: String, +} diff --git a/crates/bitwarden-api-key-connector/src/models/user_key_response_model.rs b/crates/bitwarden-api-key-connector/src/models/user_key_response_model.rs new file mode 100644 index 000000000..a2c4f5fb9 --- /dev/null +++ b/crates/bitwarden-api-key-connector/src/models/user_key_response_model.rs @@ -0,0 +1,7 @@ +use serde::{Deserialize, Serialize}; + +#[derive(Clone, Default, Debug, PartialEq, Serialize, Deserialize)] +pub struct UserKeyResponseModel { + #[serde(rename = "key", alias = "Key")] + pub key: String, +} diff --git a/crates/bitwarden-auth/Cargo.toml b/crates/bitwarden-auth/Cargo.toml index 4f3022b74..c0791f90d 100644 --- a/crates/bitwarden-auth/Cargo.toml +++ b/crates/bitwarden-auth/Cargo.toml @@ -25,7 +25,8 @@ uniffi = ["bitwarden-core/uniffi", "dep:uniffi"] # Uniffi bindings # Note: dependencies must be alphabetized to pass the cargo sort check in the CI pipeline. [dependencies] -bitwarden-api-api = { workspace = true } +bitwarden-api-api = { workspace = true, features = ["mockall"] } +bitwarden-api-key-connector = { workspace = true } bitwarden-core = { workspace = true, features = ["internal"] } bitwarden-crypto = { workspace = true } bitwarden-encoding = { workspace = true } @@ -37,6 +38,7 @@ thiserror = { workspace = true } tracing = { workspace = true } tsify = { workspace = true, optional = true } uniffi = { workspace = true, optional = true } +uuid = { workspace = true } wasm-bindgen = { workspace = true, optional = true } wasm-bindgen-futures = { workspace = true, optional = true } diff --git a/crates/bitwarden-auth/src/registration.rs b/crates/bitwarden-auth/src/registration.rs index 97acfbcc3..8b4ebac7b 100644 --- a/crates/bitwarden-auth/src/registration.rs +++ b/crates/bitwarden-auth/src/registration.rs @@ -9,7 +9,7 @@ use bitwarden_api_api::models::SetKeyConnectorKeyRequestModel; use bitwarden_core::{ Client, OrganizationId, UserId, key_management::{ - AccountCryptographyMakeKeysError, KeyConnectorApiClient, + AccountCryptographyMakeKeysError, account_cryptographic_state::WrappedAccountCryptographicState, }, }; @@ -55,14 +55,13 @@ impl RegistrationClient { user_id: UserId, ) -> Result { let client = &self.client.internal; - let api_client = &client.get_api_configurations().await.api_client; - let key_connector_api_client = - KeyConnectorApiClient::new(client, key_connector_url.as_str()); + let configuration = &client.get_api_configurations().await; + let key_connector_client = client.get_key_connector_client(key_connector_url); internal_post_keys_for_key_connector_registration( self, - api_client, - &key_connector_api_client, + &configuration.api_client, + &key_connector_client, org_id, user_id, ) @@ -73,7 +72,7 @@ impl RegistrationClient { async fn internal_post_keys_for_key_connector_registration( registration_client: &RegistrationClient, api_client: &bitwarden_api_api::apis::ApiClient, - key_connector_api_client: &KeyConnectorApiClient, + key_connector_api_client: &bitwarden_api_key_connector::apis::ApiClient, org_id: OrganizationId, user_id: UserId, ) -> Result { @@ -86,13 +85,35 @@ async fn internal_post_keys_for_key_connector_registration( .map_err(UserRegistrationError::AccountCryptographyMakeKeys)?; info!("Posting key connector key to key connector server"); - key_connector_api_client - .post_or_put_key_connector_key(®istration_crypto_result.key_connector_key) + let request = + bitwarden_api_key_connector::models::user_key_request_model::UserKeyKeyRequestModel { + key: registration_crypto_result + .key_connector_key + .to_base64() + .to_string(), + }; + (if key_connector_api_client + .user_keys_api() + .get_user_key() .await - .map_err(|e| { - error!("Failed to post key connector key to key connector server: {e:?}"); - UserRegistrationError::KeyConnectorApi - })?; + .is_ok() + { + info!("User's key connector key exists, updating"); + key_connector_api_client + .user_keys_api() + .put_user_key(request) + .await + } else { + info!("User's key connector key does not exist, creating"); + key_connector_api_client + .user_keys_api() + .post_user_key(request) + .await + }) + .map_err(|e| { + error!("Failed to post key connector key to key connector server: {e:?}"); + UserRegistrationError::KeyConnectorApi + })?; info!("Posting user account cryptographic state to server"); let request = SetKeyConnectorKeyRequestModel { @@ -158,3 +179,69 @@ pub enum UserRegistrationError { #[error(transparent)] AccountCryptographyMakeKeys(#[from] AccountCryptographyMakeKeysError), } + +#[cfg(test)] +mod tests { + use bitwarden_core::{ + Client, OrganizationId, UserId, client::test_accounts::test_bitwarden_com_account, + }; + + use crate::registration::{ + RegistrationClient, internal_post_keys_for_key_connector_registration, + }; + + const TEST_USER_ID: &str = "060000fb-0922-4dd3-b170-6e15cb5df8c8"; + const TEST_ORG_ID: &str = "1bc9ac1e-f5aa-45f2-94bf-b181009709b8"; + + #[tokio::test] + async fn test_post_keys_for_key_connector_registration_success() { + let client = Client::init_test_account(test_bitwarden_com_account()).await; + let registration_client = RegistrationClient::new(client); + + let api_client = bitwarden_api_api::apis::ApiClient::new_mocked(|mock| { + mock.accounts_key_management_api + .expect_post_set_key_connector_key() + .once() + .returning(move |_body| Ok(())); + }); + + let key_connector_api_client = + bitwarden_api_key_connector::apis::ApiClient::new_mocked(|mock| { + mock.user_keys_api + .expect_get_user_key() + .once() + .returning(move || { + Err(bitwarden_api_key_connector::apis::Error::ResponseError( + bitwarden_api_key_connector::apis::ResponseContent { + status: reqwest::StatusCode::NOT_FOUND, + content: "Not Found".to_string(), + }, + )) + }); + mock.user_keys_api + .expect_post_user_key() + .once() + .returning(move |_body| Ok(())); + }); + + let result = internal_post_keys_for_key_connector_registration( + ®istration_client, + &api_client, + &key_connector_api_client, + OrganizationId::new(uuid::uuid!(TEST_ORG_ID)), + UserId::new(uuid::uuid!(TEST_USER_ID)), + ) + .await; + assert!(result.is_ok()); + + // Assert that the mock expectations were met + if let bitwarden_api_api::apis::ApiClient::Mock(mut mock) = api_client { + mock.accounts_key_management_api.checkpoint(); + } + if let bitwarden_api_key_connector::apis::ApiClient::Mock(mut mock) = + key_connector_api_client + { + mock.user_keys_api.checkpoint(); + } + } +} diff --git a/crates/bitwarden-core/Cargo.toml b/crates/bitwarden-core/Cargo.toml index b3ae95611..da3d0f550 100644 --- a/crates/bitwarden-core/Cargo.toml +++ b/crates/bitwarden-core/Cargo.toml @@ -41,6 +41,7 @@ wasm = [ async-trait = { workspace = true } bitwarden-api-api = { workspace = true } bitwarden-api-identity = { workspace = true } +bitwarden-api-key-connector = { workspace = true } bitwarden-crypto = { workspace = true } bitwarden-encoding = { workspace = true } bitwarden-error = { workspace = true } @@ -76,10 +77,8 @@ rustls = { version = "0.23.19", default-features = false } rustls-platform-verifier = "0.6.0" [dev-dependencies] -mockall = { version = ">=0.13.1, <0.15" } rand_chacha = ">=0.3.1, <0.4.0" tokio = { workspace = true, features = ["rt"] } -wiremock = { workspace = true } zeroize = { version = ">=1.7.0, <2.0", features = ["derive", "aarch64"] } [lints] diff --git a/crates/bitwarden-core/src/client/client.rs b/crates/bitwarden-core/src/client/client.rs index 9720f1bb1..ea93116b2 100644 --- a/crates/bitwarden-core/src/client/client.rs +++ b/crates/bitwarden-core/src/client/client.rs @@ -104,8 +104,8 @@ impl Client { let api = bitwarden_api_api::apis::configuration::Configuration { base_path: settings.api_url, - user_agent: Some(settings.user_agent), - client, + user_agent: Some(settings.user_agent.clone()), + client: client.clone(), basic_auth: None, oauth_access_token: None, bearer_access_token: None, diff --git a/crates/bitwarden-core/src/client/internal.rs b/crates/bitwarden-core/src/client/internal.rs index 73d5d0aee..c0904c459 100644 --- a/crates/bitwarden-core/src/client/internal.rs +++ b/crates/bitwarden-core/src/client/internal.rs @@ -74,10 +74,26 @@ impl ApiConfigurations { let mut api = self.api_config.clone(); identity.oauth_access_token = Some(token.clone()); - api.oauth_access_token = Some(token); + api.oauth_access_token = Some(token.clone()); *self = ApiConfigurations::new(identity, api, self.device_type); } + + pub(crate) fn get_key_connector_client( + self: &Arc, + key_connector_url: String, + ) -> bitwarden_api_key_connector::apis::ApiClient { + let api = self.api_config.clone(); + + let key_connector = bitwarden_api_key_connector::apis::configuration::Configuration { + base_path: key_connector_url, + user_agent: api.user_agent, + client: api.client, + oauth_access_token: api.oauth_access_token, + }; + + bitwarden_api_key_connector::apis::ApiClient::new(&Arc::new(key_connector)) + } } /// Access and refresh tokens used for authentication and authorization. @@ -216,6 +232,16 @@ impl InternalClient { } } + pub fn get_key_connector_client( + &self, + key_connector_url: String, + ) -> bitwarden_api_key_connector::apis::ApiClient { + self.__api_configurations + .read() + .expect("RwLock is not poisoned") + .get_key_connector_client(key_connector_url) + } + #[allow(missing_docs)] pub async fn get_api_configurations(&self) -> Arc { // At the moment we ignore the error result from the token renewal, if it fails, diff --git a/crates/bitwarden-core/src/key_management/key_connector.rs b/crates/bitwarden-core/src/key_management/key_connector.rs deleted file mode 100644 index 973761030..000000000 --- a/crates/bitwarden-core/src/key_management/key_connector.rs +++ /dev/null @@ -1,355 +0,0 @@ -use std::sync::Arc; - -use bitwarden_crypto::KeyConnectorKey; -use reqwest::Method; -use serde::{Deserialize, Serialize}; -use thiserror::Error; - -use crate::{ApiError, client::internal::InternalClient}; - -#[allow(missing_docs)] -#[derive(Debug, Error)] -pub enum KeyConnectorApiError { - #[error("Invalid Key Connector URL")] - InvalidKeyConnectorUrl, - #[error(transparent)] - Api(#[from] ApiError), -} - -/// Client for interacting with the Key Connector API. -pub struct KeyConnectorApiClient { - #[doc(hidden)] - internal_client: Arc, - /// The base URL of the Key Connector API. - key_connector_url: String, -} - -#[cfg_attr(test, mockall::automock)] -impl KeyConnectorApiClient { - /// Create a new Key Connector API client. - pub fn new(internal_client: &Arc, key_connector_url: &str) -> Self { - Self { - internal_client: Arc::clone(internal_client), - key_connector_url: key_connector_url.to_string(), - } - } - - /// Sends the key connector key to the Key Connector API. - /// If a key already exists, it will be updated; otherwise, a new key will be created for the - /// user. - pub async fn post_or_put_key_connector_key( - &self, - key_connector_key: &KeyConnectorKey, - ) -> Result<(), KeyConnectorApiError> { - let request = KeyConnectorKeyRequestModel { - key: key_connector_key.to_base64().to_string(), - }; - - if self.get_key_connector_user_keys().await.is_ok() { - self.put_key_connector_user_keys(request).await?; - } else { - self.post_key_connector_user_keys(request).await?; - } - - Ok(()) - } - - async fn get_key_connector_user_keys( - &self, - ) -> Result { - let response = request_key_connector_user_keys( - &self.internal_client, - Method::GET, - &self.key_connector_url, - None::<()>, - ) - .await?; - - let body = response.text().await.map_err(KeyConnectorApiError::from)?; - let response_model = serde_json::from_str::(&body)?; - Ok(response_model) - } - - async fn post_key_connector_user_keys( - &self, - request_model: KeyConnectorKeyRequestModel, - ) -> Result<(), KeyConnectorApiError> { - request_key_connector_user_keys( - &self.internal_client, - Method::POST, - &self.key_connector_url, - Some(request_model), - ) - .await?; - - Ok(()) - } - - async fn put_key_connector_user_keys( - &self, - request_model: KeyConnectorKeyRequestModel, - ) -> Result<(), KeyConnectorApiError> { - request_key_connector_user_keys( - &self.internal_client, - Method::PUT, - &self.key_connector_url, - Some(request_model), - ) - .await?; - - Ok(()) - } -} - -async fn request_key_connector_user_keys( - client: &InternalClient, - method: Method, - key_connector_url: &str, - body: Option, -) -> Result { - let url = format!("{}/user-keys", key_connector_url); - - let config = client.get_api_configurations().await; - let mut request = client - .get_http_client() - .request(method, url) - .header(reqwest::header::CONTENT_TYPE, "application/json") - .header(reqwest::header::ACCEPT, "application/json"); - - if let Some(ref user_agent) = config.api_config.user_agent { - request = request.header(reqwest::header::USER_AGENT, user_agent.clone()); - } - if let Some(ref access_token) = config.api_config.oauth_access_token { - request = request.bearer_auth(access_token.clone()); - } - if let Some(ref body) = body { - request = - request.body(serde_json::to_string(&body).expect("Serialize should be infallible")) - } - - let response = request.send().await.map_err(KeyConnectorApiError::from)?; - - Ok(response.error_for_status()?) -} - -impl From for KeyConnectorApiError { - fn from(e: reqwest::Error) -> Self { - KeyConnectorApiError::Api(ApiError::Reqwest(e)) - } -} - -impl From for KeyConnectorApiError { - fn from(e: serde_json::Error) -> Self { - KeyConnectorApiError::Api(ApiError::Serde(e)) - } -} - -#[derive(Clone, Default, Debug, PartialEq, Serialize, Deserialize)] -struct KeyConnectorKeyRequestModel { - #[serde(rename = "key", alias = "Key")] - pub key: String, -} - -#[derive(Clone, Default, Debug, PartialEq, Serialize, Deserialize)] -struct KeyConnectorKeyResponseModel { - #[serde(rename = "key", alias = "Key")] - pub key: String, -} - -#[cfg(test)] -mod tests { - use wiremock::{ - Mock, MockServer, ResponseTemplate, - matchers::{header, method, path}, - }; - - use super::*; - use crate::{Client, ClientSettings, DeviceType}; - - const ACCESS_TOKEN: &str = "test_access_token"; - - async fn setup_mock_server_with_auth() -> (MockServer, Client) { - let server = MockServer::start().await; - - let settings = ClientSettings { - identity_url: format!("http://{}/identity", server.address()), - api_url: format!("http://{}/api", server.address()), - user_agent: "Bitwarden Rust-SDK [TEST]".into(), - device_type: DeviceType::SDK, - bitwarden_client_version: None, - }; - - let client = Client::new(Some(settings)); - - // Set up authentication token - client - .internal - .set_tokens(ACCESS_TOKEN.to_string(), None, 3600); - - (server, client) - } - - #[tokio::test] - async fn test_post_when_key_doesnt_exist() { - let (server, client) = setup_mock_server_with_auth().await; - let key_connector_url = format!("http://{}", server.address()); - - // Mock GET to return 404 (key doesn't exist) - Mock::given(method("GET")) - .and(path("/user-keys")) - .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) - .respond_with(ResponseTemplate::new(404)) - .expect(1) - .mount(&server) - .await; - - // Mock POST to succeed - Mock::given(method("POST")) - .and(path("/user-keys")) - .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) - .and(header("content-type", "application/json")) - .respond_with(ResponseTemplate::new(200)) - .expect(1) - .mount(&server) - .await; - - let key = KeyConnectorKey::make(); - let key_connector_api = - KeyConnectorApiClient::new(&client.internal, key_connector_url.as_str()); - let result = key_connector_api.post_or_put_key_connector_key(&key).await; - - assert!(result.is_ok()); - } - - #[tokio::test] - async fn test_put_when_key_exists() { - let (server, client) = setup_mock_server_with_auth().await; - let key_connector_url = format!("http://{}", server.address()); - - // Mock GET to return 200 (key exists) - Mock::given(method("GET")) - .and(path("/user-keys")) - .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) - .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ - "key": "existing_key_connector_key" - }))) - .expect(1) - .mount(&server) - .await; - - // Mock PUT to succeed - Mock::given(method("PUT")) - .and(path("/user-keys")) - .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) - .and(header("content-type", "application/json")) - .respond_with(ResponseTemplate::new(200)) - .expect(1) - .mount(&server) - .await; - - let key = KeyConnectorKey::make(); - let key_connector_api = - KeyConnectorApiClient::new(&client.internal, key_connector_url.as_str()); - let result = key_connector_api.post_or_put_key_connector_key(&key).await; - - assert!(result.is_ok()); - } - - #[tokio::test] - async fn test_error_when_both_get_and_post_fail() { - let (server, client) = setup_mock_server_with_auth().await; - let key_connector_url = format!("http://{}", server.address()); - - // Mock GET to return 500 (server error) - Mock::given(method("GET")) - .and(path("/user-keys")) - .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) - .respond_with(ResponseTemplate::new(500)) - .expect(1) - .mount(&server) - .await; - - // Mock POST to also fail (since GET failed, POST will be attempted) - Mock::given(method("POST")) - .and(path("/user-keys")) - .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) - .respond_with(ResponseTemplate::new(500)) - .expect(1) - .mount(&server) - .await; - - let key = KeyConnectorKey::make(); - let key_connector_api = - KeyConnectorApiClient::new(&client.internal, key_connector_url.as_str()); - let result = key_connector_api.post_or_put_key_connector_key(&key).await; - - assert!(result.is_err()); - assert!(matches!(result.unwrap_err(), KeyConnectorApiError::Api(_))); - } - - #[tokio::test] - async fn test_error_when_post_fails() { - let (server, client) = setup_mock_server_with_auth().await; - let key_connector_url = format!("http://{}", server.address()); - - // Mock GET to return 404 (key doesn't exist) - Mock::given(method("GET")) - .and(path("/user-keys")) - .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) - .respond_with(ResponseTemplate::new(404)) - .expect(1) - .mount(&server) - .await; - - // Mock POST to fail with 500 - Mock::given(method("POST")) - .and(path("/user-keys")) - .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) - .respond_with(ResponseTemplate::new(500)) - .expect(1) - .mount(&server) - .await; - - let key = KeyConnectorKey::make(); - let key_connector_api = - KeyConnectorApiClient::new(&client.internal, key_connector_url.as_str()); - let result = key_connector_api.post_or_put_key_connector_key(&key).await; - - assert!(result.is_err()); - assert!(matches!(result.unwrap_err(), KeyConnectorApiError::Api(_))); - } - - #[tokio::test] - async fn test_error_when_put_fails() { - let (server, client) = setup_mock_server_with_auth().await; - let key_connector_url = format!("http://{}", server.address()); - - // Mock GET to return 200 (key exists) - Mock::given(method("GET")) - .and(path("/user-keys")) - .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) - .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ - "key": "existing_key_connector_key" - }))) - .expect(1) - .mount(&server) - .await; - - // Mock PUT to fail with 500 - Mock::given(method("PUT")) - .and(path("/user-keys")) - .and(header("authorization", format!("Bearer {ACCESS_TOKEN}"))) - .respond_with(ResponseTemplate::new(500)) - .expect(1) - .mount(&server) - .await; - - let key = KeyConnectorKey::make(); - let key_connector_api = - KeyConnectorApiClient::new(&client.internal, key_connector_url.as_str()); - let result = key_connector_api.post_or_put_key_connector_key(&key).await; - - assert!(result.is_err()); - assert!(matches!(result.unwrap_err(), KeyConnectorApiError::Api(_))); - } -} diff --git a/crates/bitwarden-core/src/key_management/mod.rs b/crates/bitwarden-core/src/key_management/mod.rs index e783d3019..5a3640817 100644 --- a/crates/bitwarden-core/src/key_management/mod.rs +++ b/crates/bitwarden-core/src/key_management/mod.rs @@ -38,13 +38,6 @@ mod user_decryption; #[cfg(feature = "internal")] pub use user_decryption::UserDecryptionData; -#[cfg(feature = "internal")] -mod key_connector; -#[cfg(test)] -pub use key_connector::MockKeyConnectorApiClient; -#[cfg(feature = "internal")] -pub use key_connector::{KeyConnectorApiClient, KeyConnectorApiError}; - use crate::OrganizationId; key_ids! { From 5c0971afb2531a292989cb35bda5ea7ccca3856f Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Sat, 20 Dec 2025 18:26:52 +0100 Subject: [PATCH 07/10] incorrect type of org identifier --- crates/bitwarden-auth/src/registration.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/crates/bitwarden-auth/src/registration.rs b/crates/bitwarden-auth/src/registration.rs index 8b4ebac7b..125ce924a 100644 --- a/crates/bitwarden-auth/src/registration.rs +++ b/crates/bitwarden-auth/src/registration.rs @@ -7,7 +7,7 @@ use bitwarden_api_api::models::SetKeyConnectorKeyRequestModel; use bitwarden_core::{ - Client, OrganizationId, UserId, + Client, UserId, key_management::{ AccountCryptographyMakeKeysError, account_cryptographic_state::WrappedAccountCryptographicState, @@ -51,7 +51,7 @@ impl RegistrationClient { pub async fn post_keys_for_key_connector_registration( &self, key_connector_url: String, - org_id: OrganizationId, + sso_org_identifier: String, user_id: UserId, ) -> Result { let client = &self.client.internal; @@ -62,7 +62,7 @@ impl RegistrationClient { self, &configuration.api_client, &key_connector_client, - org_id, + sso_org_identifier, user_id, ) .await @@ -73,7 +73,7 @@ async fn internal_post_keys_for_key_connector_registration( registration_client: &RegistrationClient, api_client: &bitwarden_api_api::apis::ApiClient, key_connector_api_client: &bitwarden_api_key_connector::apis::ApiClient, - org_id: OrganizationId, + sso_org_identifier: String, user_id: UserId, ) -> Result { // First call crypto API to get all keys @@ -123,7 +123,7 @@ async fn internal_post_keys_for_key_connector_registration( .to_string(), ), account_keys: Some(Box::new(registration_crypto_result.account_keys_request)), - ..SetKeyConnectorKeyRequestModel::new(org_id.to_string()) + ..SetKeyConnectorKeyRequestModel::new(sso_org_identifier.to_string()) }; api_client .accounts_key_management_api() @@ -182,16 +182,14 @@ pub enum UserRegistrationError { #[cfg(test)] mod tests { - use bitwarden_core::{ - Client, OrganizationId, UserId, client::test_accounts::test_bitwarden_com_account, - }; + use bitwarden_core::{Client, UserId, client::test_accounts::test_bitwarden_com_account}; use crate::registration::{ RegistrationClient, internal_post_keys_for_key_connector_registration, }; const TEST_USER_ID: &str = "060000fb-0922-4dd3-b170-6e15cb5df8c8"; - const TEST_ORG_ID: &str = "1bc9ac1e-f5aa-45f2-94bf-b181009709b8"; + const TEST_SSO_ORG_IDENTIFIER: &str = "test-org"; #[tokio::test] async fn test_post_keys_for_key_connector_registration_success() { @@ -228,7 +226,7 @@ mod tests { ®istration_client, &api_client, &key_connector_api_client, - OrganizationId::new(uuid::uuid!(TEST_ORG_ID)), + TEST_SSO_ORG_IDENTIFIER.to_string(), UserId::new(uuid::uuid!(TEST_USER_ID)), ) .await; From 5600567a3a60170f562d58d2a37b6ed7df185b3b Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Sun, 21 Dec 2025 08:23:10 +0100 Subject: [PATCH 08/10] resolving merge conflicts --- Cargo.lock | 15 ++ crates/bitwarden-auth/src/registration.rs | 155 +++++++++++++++--- .../src/key_management/crypto.rs | 107 +++++++++++- .../src/key_management/crypto_client.rs | 119 +------------- .../bitwarden-core/src/key_management/mod.rs | 2 +- 5 files changed, 259 insertions(+), 139 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e2eef8832..64d68e10b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -522,11 +522,25 @@ dependencies = [ "uuid", ] +[[package]] +name = "bitwarden-api-key-connector" +version = "2.0.0" +dependencies = [ + "async-trait", + "mockall", + "reqwest", + "serde", + "serde_json", + "tokio", + "wiremock", +] + [[package]] name = "bitwarden-auth" version = "2.0.0" dependencies = [ "bitwarden-api-api", + "bitwarden-api-key-connector", "bitwarden-core", "bitwarden-crypto", "bitwarden-encoding", @@ -598,6 +612,7 @@ dependencies = [ "async-trait", "bitwarden-api-api", "bitwarden-api-identity", + "bitwarden-api-key-connector", "bitwarden-crypto", "bitwarden-encoding", "bitwarden-error", diff --git a/crates/bitwarden-auth/src/registration.rs b/crates/bitwarden-auth/src/registration.rs index b178cc9de..e87711a2b 100644 --- a/crates/bitwarden-auth/src/registration.rs +++ b/crates/bitwarden-auth/src/registration.rs @@ -7,14 +7,11 @@ use bitwarden_api_api::models::{ DeviceKeysRequestModel, KeysRequestModel, OrganizationUserResetPasswordEnrollmentRequestModel, - SetKeyConnectorKeyRequestModel + SetKeyConnectorKeyRequestModel, }; use bitwarden_core::{ Client, OrganizationId, UserId, - key_management::{ - AccountCryptographyMakeKeysError, - account_cryptographic_state::WrappedAccountCryptographicState, - }, + key_management::account_cryptographic_state::WrappedAccountCryptographicState, }; use bitwarden_crypto::EncString; use bitwarden_encoding::B64; @@ -82,7 +79,7 @@ impl RegistrationClient { key_connector_url: String, sso_org_identifier: String, user_id: UserId, - ) -> Result { + ) -> Result { let client = &self.client.internal; let configuration = &client.get_api_configurations().await; let key_connector_client = client.get_key_connector_client(key_connector_url); @@ -94,7 +91,7 @@ impl RegistrationClient { sso_org_identifier, user_id, ) - .await + .await } } @@ -222,21 +219,20 @@ pub struct TdeRegistrationResponse { pub user_key: B64, } - async fn internal_post_keys_for_key_connector_registration( registration_client: &RegistrationClient, api_client: &bitwarden_api_api::apis::ApiClient, key_connector_api_client: &bitwarden_api_key_connector::apis::ApiClient, sso_org_identifier: String, user_id: UserId, -) -> Result { +) -> Result { // First call crypto API to get all keys info!("Initializing account cryptography"); let registration_crypto_result = registration_client .client .crypto() .make_user_key_connector_registration(user_id) - .map_err(UserRegistrationError::AccountCryptographyMakeKeys)?; + .map_err(|_| RegistrationError::Crypto)?; info!("Posting key connector key to key connector server"); let request = @@ -264,10 +260,10 @@ async fn internal_post_keys_for_key_connector_registration( .post_user_key(request) .await }) - .map_err(|e| { - error!("Failed to post key connector key to key connector server: {e:?}"); - UserRegistrationError::KeyConnectorApi - })?; + .map_err(|e| { + error!("Failed to post key connector key to key connector server: {e:?}"); + RegistrationError::KeyConnectorApi + })?; info!("Posting user account cryptographic state to server"); let request = SetKeyConnectorKeyRequestModel { @@ -285,7 +281,7 @@ async fn internal_post_keys_for_key_connector_registration( .await .map_err(|e| { error!("Failed to post account cryptographic state to server: {e:?}"); - UserRegistrationError::Api + RegistrationError::Api })?; info!("User initialized!"); @@ -347,6 +343,7 @@ mod tests { const TEST_USER_ID: &str = "060000fb-0922-4dd3-b170-6e15cb5df8c8"; const TEST_ORG_ID: &str = "1bc9ac1e-f5aa-45f2-94bf-b181009709b8"; const TEST_DEVICE_ID: &str = "test-device-id"; + const TEST_SSO_ORG_IDENTIFIER: &str = "test-org"; const TEST_ORG_PUBLIC_KEY: &[u8] = &[ 48, 130, 1, 34, 48, 13, 6, 9, 42, 134, 72, 134, 247, 13, 1, 1, 1, 5, 0, 3, 130, 1, 15, 0, @@ -626,15 +623,12 @@ mod tests { } } - const TEST_USER_ID: &str = "060000fb-0922-4dd3-b170-6e15cb5df8c8"; - const TEST_SSO_ORG_IDENTIFIER: &str = "test-org"; - #[tokio::test] async fn test_post_keys_for_key_connector_registration_success() { - let client = Client::init_test_account(test_bitwarden_com_account()).await; + let client = Client::new(None); let registration_client = RegistrationClient::new(client); - let api_client = bitwarden_api_api::apis::ApiClient::new_mocked(|mock| { + let api_client = ApiClient::new_mocked(|mock| { mock.accounts_key_management_api .expect_post_set_key_connector_key() .once() @@ -671,7 +665,126 @@ mod tests { assert!(result.is_ok()); // Assert that the mock expectations were met - if let bitwarden_api_api::apis::ApiClient::Mock(mut mock) = api_client { + if let ApiClient::Mock(mut mock) = api_client { + mock.accounts_key_management_api.checkpoint(); + } + if let bitwarden_api_key_connector::apis::ApiClient::Mock(mut mock) = + key_connector_api_client + { + mock.user_keys_api.checkpoint(); + } + } + + #[tokio::test] + async fn test_post_keys_for_key_connector_registration_key_connector_api_failure() { + let client = Client::new(None); + let registration_client = RegistrationClient::new(client); + + let api_client = ApiClient::new_mocked(|mock| { + // Should not be called if Key Connector API fails + mock.accounts_key_management_api + .expect_post_set_key_connector_key() + .never(); + }); + + let key_connector_api_client = + bitwarden_api_key_connector::apis::ApiClient::new_mocked(|mock| { + mock.user_keys_api + .expect_get_user_key() + .once() + .returning(move || { + Err(bitwarden_api_key_connector::apis::Error::ResponseError( + bitwarden_api_key_connector::apis::ResponseContent { + status: reqwest::StatusCode::NOT_FOUND, + content: "Not Found".to_string(), + }, + )) + }); + mock.user_keys_api + .expect_post_user_key() + .once() + .returning(move |_body| { + Err(bitwarden_api_key_connector::apis::Error::Serde( + serde_json::Error::io(std::io::Error::other("API error")), + )) + }); + }); + + let result = internal_post_keys_for_key_connector_registration( + ®istration_client, + &api_client, + &key_connector_api_client, + TEST_SSO_ORG_IDENTIFIER.to_string(), + UserId::new(uuid::uuid!(TEST_USER_ID)), + ) + .await; + + assert!(result.is_err()); + assert!(matches!( + result.unwrap_err(), + RegistrationError::KeyConnectorApi + )); + + // Assert that the mock expectations were met + if let ApiClient::Mock(mut mock) = api_client { + mock.accounts_key_management_api.checkpoint(); + } + if let bitwarden_api_key_connector::apis::ApiClient::Mock(mut mock) = + key_connector_api_client + { + mock.user_keys_api.checkpoint(); + } + } + + #[tokio::test] + async fn test_post_keys_for_key_connector_registration_api_failure() { + let client = Client::new(None); + let registration_client = RegistrationClient::new(client); + + let api_client = ApiClient::new_mocked(|mock| { + mock.accounts_key_management_api + .expect_post_set_key_connector_key() + .once() + .returning(move |_body| { + Err(bitwarden_api_api::apis::Error::Serde( + serde_json::Error::io(std::io::Error::other("API error")), + )) + }); + }); + + let key_connector_api_client = + bitwarden_api_key_connector::apis::ApiClient::new_mocked(|mock| { + mock.user_keys_api + .expect_get_user_key() + .once() + .returning(move || { + Err(bitwarden_api_key_connector::apis::Error::ResponseError( + bitwarden_api_key_connector::apis::ResponseContent { + status: reqwest::StatusCode::NOT_FOUND, + content: "Not Found".to_string(), + }, + )) + }); + mock.user_keys_api + .expect_post_user_key() + .once() + .returning(move |_body| Ok(())); + }); + + let result = internal_post_keys_for_key_connector_registration( + ®istration_client, + &api_client, + &key_connector_api_client, + TEST_SSO_ORG_IDENTIFIER.to_string(), + UserId::new(uuid::uuid!(TEST_USER_ID)), + ) + .await; + + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), RegistrationError::Api)); + + // Assert that the mock expectations were met + if let ApiClient::Mock(mut mock) = api_client { mock.accounts_key_management_api.checkpoint(); } if let bitwarden_api_key_connector::apis::ApiClient::Mock(mut mock) = diff --git a/crates/bitwarden-core/src/key_management/crypto.rs b/crates/bitwarden-core/src/key_management/crypto.rs index 8be76d1d1..badb9b61e 100644 --- a/crates/bitwarden-core/src/key_management/crypto.rs +++ b/crates/bitwarden-core/src/key_management/crypto.rs @@ -9,10 +9,11 @@ use std::collections::HashMap; use bitwarden_api_api::models::AccountKeysRequestModel; use bitwarden_crypto::{ AsymmetricCryptoKey, AsymmetricPublicCryptoKey, CoseSerializable, CryptoError, DeviceKey, - EncString, Kdf, KeyDecryptable, KeyEncryptable, MasterKey, Pkcs8PrivateKeyBytes, - PrimitiveEncryptable, RotateableKeySet, SignatureAlgorithm, SignedPublicKey, SigningKey, - SpkiPublicKeyBytes, SymmetricCryptoKey, TrustDeviceResponse, UnsignedSharedKey, UserKey, - dangerous_get_v2_rotated_account_keys, derive_symmetric_key_from_prf, + EncString, Kdf, KeyConnectorKey, KeyDecryptable, KeyEncryptable, MasterKey, + Pkcs8PrivateKeyBytes, PrimitiveEncryptable, RotateableKeySet, SignatureAlgorithm, + SignedPublicKey, SigningKey, SpkiPublicKeyBytes, SymmetricCryptoKey, TrustDeviceResponse, + UnsignedSharedKey, UserKey, dangerous_get_v2_rotated_account_keys, + derive_symmetric_key_from_prf, safe::{PasswordProtectedKeyEnvelope, PasswordProtectedKeyEnvelopeError}, }; use bitwarden_encoding::B64; @@ -865,7 +866,7 @@ pub struct MakeTdeRegistrationResponse { pub reset_password_key: UnsignedSharedKey, } -/// Errors that can occur when making keys for TDE registration. +/// Errors that can occur when making keys for TDE or Key Connector registration. #[bitwarden_error(flat)] #[derive(Debug, thiserror::Error)] pub enum MakeKeysError { @@ -918,6 +919,52 @@ pub(crate) fn make_user_tde_registration( }) } +/// The response from `make_user_key_connector_registration`. +pub struct MakeKeyConnectorRegistrationResponse { + /// The account cryptographic state + pub account_cryptographic_state: WrappedAccountCryptographicState, + /// Encrypted user's user key, wrapped with the key connector key + pub key_connector_key_wrapped_user_key: EncString, + /// The user's user key + pub user_key: SymmetricCryptoKey, + /// The request model for the account cryptographic state (also called Account Keys) + pub account_keys_request: AccountKeysRequestModel, + /// The key connector key used for unlocking + pub key_connector_key: KeyConnectorKey, +} + +/// Create the data needed to register for Key Connector +pub(crate) fn make_user_key_connector_registration( + client: &Client, + user_id: UserId, +) -> Result { + let mut ctx = client.internal.get_key_store().context_mut(); + let (user_key_id, wrapped_state) = WrappedAccountCryptographicState::make(&mut ctx, user_id) + .map_err(MakeKeysError::AccountCryptographyInitialization)?; + #[expect(deprecated)] + let user_key = ctx.dangerous_get_symmetric_key(user_key_id)?.to_owned(); + + // Key Connector unlock method + let key_connector_key = KeyConnectorKey::make(); + + let wrapped_user_key = key_connector_key + .encrypt_user_key(&user_key) + .map_err(MakeKeysError::Crypto)?; + + let cryptography_state_request_model = + wrapped_state + .to_request_model(&user_key_id, &mut ctx) + .map_err(MakeKeysError::AccountCryptographyInitialization)?; + + Ok(MakeKeyConnectorRegistrationResponse { + account_cryptographic_state: wrapped_state, + key_connector_key_wrapped_user_key: wrapped_user_key, + user_key, + account_keys_request: cryptography_state_request_model, + key_connector_key, + }) +} + #[cfg(test)] mod tests { use std::num::NonZeroU32; @@ -926,6 +973,7 @@ mod tests { use super::*; use crate::Client; + const TEST_VECTOR_USER_KEY_V2_B64: &str = "pQEEAlACHUUoybNAuJoZzqNMxz2bAzoAARFvBIQDBAUGIFggAvGl4ifaUAomQdCdUPpXLHtypiQxHjZwRHeI83caZM4B"; const TEST_VECTOR_PRIVATE_KEY_V2: &str = "7.g1gdowE6AAERbwMZARwEUAIdRSjJs0C4mhnOo0zHPZuhBVgYthGLGqVLPeidY8mNMxpLJn3fyeSxyaWsWQTR6pxmRV2DyGZXly/0l9KK+Rsfetl9wvYIz0O4/RW3R6wf7eGxo5XmicV3WnFsoAmIQObxkKWShxFyjzg+ocKItQDzG7Gp6+MW4biTrAlfK51ML/ZS+PCjLmgI1QQr4eMHjiwA2TBKtKkxfjoTJkMXECpRVLEXOo8/mbIGYkuabbSA7oU+TJ0yXlfKDtD25gnyO7tjW/0JMFUaoEKRJOuKoXTN4n/ks4Hbxk0X5/DzfG05rxWad2UNBjNg7ehW99WrQ+33ckdQFKMQOri/rt8JzzrF1k11/jMJ+Y2TADKNHr91NalnUX+yqZAAe3sRt5Pv5ZhLIwRMKQi/1NrLcsQPRuUnogVSPOoMnE/eD6F70iU60Z6pvm1iBw2IvELZcrs/oxpO2SeCue08fIZW/jNZokbLnm90tQ7QeZTUpiPALhUgfGOa3J9VOJ7jQGCqDjd9CzV2DCVfhKCapeTbldm+RwEWBz5VvorH5vMx1AzbPRJxdIQuxcg3NqRrXrYC7fyZljWaPB9qP1tztiPtd1PpGEgxLByIfR6fqyZMCvOBsWbd0H6NhF8mNVdDw60+skFRdbRBTSCjCtKZeLVuVFb8ioH45PR5oXjtx4atIDzu6DKm6TTMCbR6DjZuZZ8GbwHxuUD2mDD3pAFhaof9kR3lQdjy7Zb4EzUUYskQxzcLPcqzp9ZgB3Rg91SStBCCMhdQ6AnhTy+VTGt/mY5AbBXNRSL6fI0r+P9K8CcEI4bNZCDkwwQr5v4O4ykSUzIvmVU0zKzDngy9bteIZuhkvGUoZlQ9UATNGPhoLfqq2eSvqEXkCbxTVZ5D+Ww9pHmWeVcvoBhcl5MvicfeQt++dY3tPjIfZq87nlugG4HiNbcv9nbVpgwe3v8cFetWXQgnO4uhx8JHSwGoSuxHFZtl2sdahjTHavRHnYjSABEFrViUKgb12UDD5ow1GAL62wVdSJKRf9HlLbJhN3PBxuh5L/E0wy1wGA9ecXtw/R1ktvXZ7RklGAt1TmNzZv6vI2J/CMXvndOX9rEpjKMbwbIDAjQ9PxiWdcnmc5SowT9f6yfIjbjXnRMWWidPAua7sgrtej4HP4Qjz1fpgLMLCRyF97tbMTmsAI5Cuj98Buh9PwcdyXj5SbVuHdJS1ehv9b5SWPsD4pwOm3+otVNK6FTazhoUl47AZoAoQzXfsXxrzqYzvF0yJkCnk9S1dcij1L569gQ43CJO6o6jIZFJvA4EmZDl95ELu+BC+x37Ip8dq4JLPsANDVSqvXO9tfDUIXEx25AaOYhW2KAUoDve/fbsU8d0UZR1o/w+ZrOQwawCIPeVPtbh7KFRVQi/rPI+Abl6XR6qMJbKPegliYGUuGF2oEMEc6QLTsMRCEPuw0S3kxbNfVPqml8nGhB2r8zUHBY1diJEmipVghnwH74gIKnyJ2C9nKjV8noUfKzqyV8vxUX2G5yXgodx8Jn0cWs3XhWuApFla9z4R28W/4jA1jK2WQMlx+b6xKUWgRk8+fYsc0HSt2fDrQ9pLpnjb8ME59RCxSPV++PThpnR2JtastZBZur2hBIJsGILCAmufUU4VC4gBKPhNfu/OK4Ktgz+uQlUa9fEC/FnkpTRQPxHuQjSQSNrIIyW1bIRBtnwjvvvNoui9FZJ"; #[allow(unused)] @@ -1716,4 +1764,53 @@ mod tests { "decrypted admin reset key should match the user's encryption key" ); } + + #[tokio::test] + async fn test_make_user_key_connector_registration_success() { + let user_id = UserId::new_v4(); + let email = "test@bitwarden.com"; + let registration_client = Client::new(None); + + let make_keys_response = + make_user_key_connector_registration(®istration_client, user_id); + assert!(make_keys_response.is_ok()); + let make_keys_response = make_keys_response.unwrap(); + + // Initialize a new client using the key connector key + let unlock_client = Client::new(None); + unlock_client + .crypto() + .initialize_user_crypto(InitUserCryptoRequest { + user_id: Some(user_id), + kdf_params: Kdf::default(), + email: email.to_owned(), + account_cryptographic_state: make_keys_response.account_cryptographic_state, + method: InitUserCryptoMethod::KeyConnector { + user_key: make_keys_response + .key_connector_key_wrapped_user_key + .clone(), + master_key: make_keys_response.key_connector_key.to_base64(), + }, + }) + .await + .expect("initializing user crypto with key connector key should succeed"); + + // Verify we can retrieve the user encryption key + let retrieved_key = unlock_client + .crypto() + .get_user_encryption_key() + .await + .expect("should be able to get user encryption key"); + + // The retrieved key should be a valid symmetric key + let retrieved_symmetric_key = SymmetricCryptoKey::try_from(retrieved_key) + .expect("retrieved key should be valid symmetric key"); + + assert_eq!(retrieved_symmetric_key, make_keys_response.user_key); + + let decrypted_user_key = make_keys_response + .key_connector_key + .decrypt_user_key(make_keys_response.key_connector_key_wrapped_user_key); + assert_eq!(retrieved_symmetric_key, decrypted_user_key.unwrap()); + } } diff --git a/crates/bitwarden-core/src/key_management/crypto_client.rs b/crates/bitwarden-core/src/key_management/crypto_client.rs index e1d0c13d0..117cda32a 100644 --- a/crates/bitwarden-core/src/key_management/crypto_client.rs +++ b/crates/bitwarden-core/src/key_management/crypto_client.rs @@ -1,27 +1,21 @@ -use bitwarden_api_api::models::AccountKeysRequestModel; #[cfg(feature = "wasm")] use bitwarden_crypto::safe::PasswordProtectedKeyEnvelope; -use bitwarden_crypto::{ - CryptoError, Decryptable, Kdf, KeyConnectorKey, RotateableKeySet, SymmetricCryptoKey, -}; +use bitwarden_crypto::{CryptoError, Decryptable, Kdf, RotateableKeySet}; #[cfg(feature = "internal")] use bitwarden_crypto::{EncString, UnsignedSharedKey}; use bitwarden_encoding::B64; -use bitwarden_error::bitwarden_error; #[cfg(feature = "wasm")] use wasm_bindgen::prelude::*; use super::crypto::{ DeriveKeyConnectorError, DeriveKeyConnectorRequest, EnrollAdminPasswordResetError, - MakeKeyPairResponse, VerifyAsymmetricKeysRequest, VerifyAsymmetricKeysResponse, - derive_key_connector, make_key_pair, verify_asymmetric_keys, + MakeKeyConnectorRegistrationResponse, MakeKeyPairResponse, VerifyAsymmetricKeysRequest, + VerifyAsymmetricKeysResponse, derive_key_connector, make_key_pair, + make_user_key_connector_registration, verify_asymmetric_keys, }; #[cfg(feature = "internal")] use crate::key_management::{ SymmetricKeyId, - account_cryptographic_state::{ - AccountCryptographyInitializationError, WrappedAccountCryptographicState, - }, crypto::{ DerivePinKeyResponse, InitOrgCryptoRequest, InitUserCryptoRequest, UpdatePasswordResponse, derive_pin_key, derive_pin_user_key, enroll_admin_password_reset, get_user_encryption_key, @@ -216,68 +210,14 @@ impl CryptoClient { /// Creates a new V2 account cryptographic state for Key Connector registration. /// This generates fresh cryptographic keys (private key, signing key, signed public key, /// and security state) wrapped with a new user key. - /// - /// Returns the wrapped account cryptographic state that can be used for registration, - /// key connector key wrapped user key, key connector key and decrypted user key. pub fn make_user_key_connector_registration( &self, user_id: UserId, - ) -> Result { - let mut ctx = self.client.internal.get_key_store().context_mut(); - let (user_key_id, wrapped_state) = - WrappedAccountCryptographicState::make(&mut ctx, user_id) - .map_err(AccountCryptographyMakeKeysError::AccountCryptographyInitialization)?; - #[expect(deprecated)] - let user_key = ctx.dangerous_get_symmetric_key(user_key_id)?.to_owned(); - - // Key Connector unlock method - let key_connector_key = KeyConnectorKey::make(); - - let wrapped_user_key = key_connector_key - .encrypt_user_key(&user_key) - .map_err(AccountCryptographyMakeKeysError::Crypto)?; - - let cryptography_state_request_model = wrapped_state - .to_request_model(&user_key_id, &mut ctx) - .map_err(AccountCryptographyMakeKeysError::AccountCryptographyInitialization)?; - - Ok(MakeKeyConnectorRegistrationResponse { - account_cryptographic_state: wrapped_state, - key_connector_key_wrapped_user_key: wrapped_user_key, - user_key, - account_keys_request: cryptography_state_request_model, - key_connector_key, - }) + ) -> Result { + make_user_key_connector_registration(&self.client, user_id) } } -/// The response from `make_user_key_connector_registration`. -pub struct MakeKeyConnectorRegistrationResponse { - /// The account cryptographic state - pub account_cryptographic_state: WrappedAccountCryptographicState, - /// Encrypted user's user key, wrapped with the key connector key - pub key_connector_key_wrapped_user_key: EncString, - /// The user's user key - pub user_key: SymmetricCryptoKey, - /// The request model for the account cryptographic state (also called Account Keys) - pub account_keys_request: AccountKeysRequestModel, - /// The key connector key used for unlocking - pub key_connector_key: KeyConnectorKey, -} - -// TODO move to crypto.rs -/// Errors that can occur during account cryptography key generation. -#[bitwarden_error(flat)] -#[derive(Debug, thiserror::Error)] -pub enum AccountCryptographyMakeKeysError { - /// Failed to initialize account cryptography - #[error("Failed to initialize account cryptography: {0}")] - AccountCryptographyInitialization(#[from] AccountCryptographyInitializationError), - /// Generic crypto error - #[error("Cryptography error: {0}")] - Crypto(#[from] CryptoError), -} - impl Client { /// Access to crypto functionality. pub fn crypto(&self) -> CryptoClient { @@ -292,10 +232,7 @@ mod tests { use bitwarden_crypto::{BitwardenLegacyKeyBytes, SymmetricCryptoKey}; use super::*; - use crate::{ - client::test_accounts::test_bitwarden_com_account, - key_management::crypto::InitUserCryptoMethod, - }; + use crate::client::test_accounts::test_bitwarden_com_account; #[tokio::test] async fn test_enroll_pin_envelope() { @@ -325,46 +262,4 @@ mod tests { let user_key_final = SymmetricCryptoKey::try_from(&secret).expect("valid user key"); assert_eq!(user_key_initial, user_key_final); } - - #[tokio::test] - async fn test_make_user_key_connector_registration_success() { - let user_id = UserId::new_v4(); - let email = "test@bitwarden.com"; - let client = Client::init_test_account(test_bitwarden_com_account()).await; - - let make_keys_response = client - .crypto() - .make_user_key_connector_registration(user_id) - .unwrap(); - - // Initialize a new client using the key connector key - let unlock_client = Client::new(None); - unlock_client - .crypto() - .initialize_user_crypto(InitUserCryptoRequest { - user_id: Some(user_id), - kdf_params: Kdf::default(), - email: email.to_owned(), - account_cryptographic_state: make_keys_response.account_cryptographic_state, - method: InitUserCryptoMethod::KeyConnector { - user_key: make_keys_response.key_connector_key_wrapped_user_key, - master_key: make_keys_response.key_connector_key.to_base64(), - }, - }) - .await - .expect("initializing user crypto with key connector key should succeed"); - - // Verify we can retrieve the user encryption key - let retrieved_key = unlock_client - .crypto() - .get_user_encryption_key() - .await - .expect("should be able to get user encryption key"); - - // The retrieved key should be a valid symmetric key - let retrieved_symmetric_key = SymmetricCryptoKey::try_from(retrieved_key) - .expect("retrieved key should be valid symmetric key"); - - assert_eq!(retrieved_symmetric_key, make_keys_response.user_key); - } } diff --git a/crates/bitwarden-core/src/key_management/mod.rs b/crates/bitwarden-core/src/key_management/mod.rs index 5a3640817..bd7c140a5 100644 --- a/crates/bitwarden-core/src/key_management/mod.rs +++ b/crates/bitwarden-core/src/key_management/mod.rs @@ -19,7 +19,7 @@ pub mod crypto; #[cfg(feature = "internal")] mod crypto_client; #[cfg(feature = "internal")] -pub use crypto_client::{AccountCryptographyMakeKeysError, CryptoClient}; +pub use crypto_client::CryptoClient; #[cfg(feature = "internal")] mod master_password; From c5d68b0be130df1323bed1d1b86cadde6cb9e5af Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Sun, 21 Dec 2025 08:32:06 +0100 Subject: [PATCH 09/10] lint --- crates/bitwarden-api-key-connector/Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bitwarden-api-key-connector/Cargo.toml b/crates/bitwarden-api-key-connector/Cargo.toml index f3d90aac8..24f6a9917 100644 --- a/crates/bitwarden-api-key-connector/Cargo.toml +++ b/crates/bitwarden-api-key-connector/Cargo.toml @@ -16,9 +16,9 @@ keywords.workspace = true async-trait = { workspace = true } mockall = { version = ">=0.13.1, <0.15" } reqwest = { workspace = true } -tokio = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } +tokio = { workspace = true } [dev-dependencies] -wiremock = { workspace = true } \ No newline at end of file +wiremock = { workspace = true } From f7aee37818f23f80a713c2b3545e120e64867917 Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Sun, 21 Dec 2025 08:40:48 +0100 Subject: [PATCH 10/10] revert unwanted changes after merge conflicts resolution --- crates/bitwarden-core/Cargo.toml | 2 +- crates/bitwarden-core/src/client/internal.rs | 2 +- .../account_cryptographic_state.rs | 58 +++++++++++++++++-- 3 files changed, 55 insertions(+), 7 deletions(-) diff --git a/crates/bitwarden-core/Cargo.toml b/crates/bitwarden-core/Cargo.toml index da3d0f550..1b8b22784 100644 --- a/crates/bitwarden-core/Cargo.toml +++ b/crates/bitwarden-core/Cargo.toml @@ -77,7 +77,7 @@ rustls = { version = "0.23.19", default-features = false } rustls-platform-verifier = "0.6.0" [dev-dependencies] -rand_chacha = ">=0.3.1, <0.4.0" +rand_chacha = "0.3.1" tokio = { workspace = true, features = ["rt"] } zeroize = { version = ">=1.7.0, <2.0", features = ["derive", "aarch64"] } diff --git a/crates/bitwarden-core/src/client/internal.rs b/crates/bitwarden-core/src/client/internal.rs index 43796404e..38390b95e 100644 --- a/crates/bitwarden-core/src/client/internal.rs +++ b/crates/bitwarden-core/src/client/internal.rs @@ -74,7 +74,7 @@ impl ApiConfigurations { let mut api = self.api_config.clone(); identity.oauth_access_token = Some(token.clone()); - api.oauth_access_token = Some(token.clone()); + api.oauth_access_token = Some(token); *self = ApiConfigurations::new(identity, api, self.device_type); } diff --git a/crates/bitwarden-core/src/key_management/account_cryptographic_state.rs b/crates/bitwarden-core/src/key_management/account_cryptographic_state.rs index 75455cca7..892f1e2c6 100644 --- a/crates/bitwarden-core/src/key_management/account_cryptographic_state.rs +++ b/crates/bitwarden-core/src/key_management/account_cryptographic_state.rs @@ -221,11 +221,16 @@ impl WrappedAccountCryptographicState { return Err(AccountCryptographyInitializationError::WrongUserKeyType); } - let private_key_id = ctx - .unwrap_private_key(user_key, private_key) - .map_err(|_| AccountCryptographyInitializationError::WrongUserKey)?; + // Some users have unreadable V1 private keys. In this case, we set no keys to + // state. + if let Ok(private_key_id) = ctx.unwrap_private_key(user_key, private_key) { + ctx.persist_asymmetric_key(private_key_id, AsymmetricKeyId::UserPrivateKey)?; + } else { + tracing::warn!( + "V1 private key could not be unwrapped, skipping setting private key" + ); + } - ctx.persist_asymmetric_key(private_key_id, AsymmetricKeyId::UserPrivateKey)?; ctx.persist_symmetric_key(user_key, SymmetricKeyId::User)?; } WrappedAccountCryptographicState::V2 { @@ -290,7 +295,7 @@ impl WrappedAccountCryptographicState { mod tests { use std::{str::FromStr, sync::RwLock}; - use bitwarden_crypto::KeyStore; + use bitwarden_crypto::{KeyStore, PrimitiveEncryptable}; use super::*; use crate::key_management::{AsymmetricKeyId, SigningKeyId, SymmetricKeyId}; @@ -467,4 +472,47 @@ mod tests { model.security_state.unwrap().security_version as u64 ); } + + #[test] + fn test_set_to_context_v1_corrupt_private_key() { + // Test that a V1 account with a corrupt private key (valid EncString but invalid key data) + // can still initialize, but skips setting the private key + let temp_store: KeyStore = KeyStore::default(); + let mut temp_ctx = temp_store.context_mut(); + + let user_key = temp_ctx.make_symmetric_key(SymmetricKeyAlgorithm::Aes256CbcHmac); + let corrupt_private_key = "not a private key" + .encrypt(&mut temp_ctx, user_key) + .unwrap(); + + // Construct the V1 wrapped state with corrupt private key + let wrapped = WrappedAccountCryptographicState::V1 { + private_key: corrupt_private_key, + }; + + #[expect(deprecated)] + let user_key_material = temp_ctx + .dangerous_get_symmetric_key(user_key) + .unwrap() + .to_owned(); + drop(temp_ctx); + drop(temp_store); + + // Now attempt to set this wrapped state into a fresh store + let store: KeyStore = KeyStore::default(); + let mut ctx = store.context_mut(); + let user_key = ctx.add_local_symmetric_key(user_key_material); + let security_state = RwLock::new(None); + + wrapped + .set_to_context(&security_state, user_key, &store, ctx) + .unwrap(); + + let ctx = store.context(); + + // The user symmetric key should be set + assert!(ctx.has_symmetric_key(SymmetricKeyId::User)); + // But the private key should NOT be set (due to corruption) + assert!(!ctx.has_asymmetric_key(AsymmetricKeyId::UserPrivateKey)); + } }