diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index a5a73ef9b..b4a8552c8 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -61,6 +61,7 @@ crates/bitwarden-vault/** @bitwarden/team-vault-dev # Team-owned folders in other crates (to be avoided if possible) crates/bitwarden-wasm-internal/integration-tests/tests/organizations/** @bitwarden/team-admin-console-dev crates/bitwarden-wasm-internal/integration-tests/tests/unlock/** @bitwarden/team-key-management-dev +crates/bitwarden-wasm-internal/integration-tests/tests/user-crypto-management/** @bitwarden/team-key-management-dev crates/bitwarden-wasm-internal/src/pure_crypto.rs @bitwarden/team-key-management-dev crates/bitwarden-core/src/key_management/** @bitwarden/team-key-management-dev @bitwarden/team-platform-dev crates/bitwarden-ipc/src/crypto_provider/noise/ @bitwarden/team-key-management-dev diff --git a/Cargo.lock b/Cargo.lock index c48d08180..e684e7d0b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -716,6 +716,7 @@ dependencies = [ "rsa", "schemars 1.2.1", "serde", + "serde-wasm-bindgen", "serde_bytes", "serde_json", "serde_repr", diff --git a/crates/bitwarden-core/src/client/builder.rs b/crates/bitwarden-core/src/client/builder.rs index 584429a6e..5d604024e 100644 --- a/crates/bitwarden-core/src/client/builder.rs +++ b/crates/bitwarden-core/src/client/builder.rs @@ -24,6 +24,8 @@ pub struct ClientBuilder { token_handler: Arc, state_registry: Option, middleware: Vec>, + #[cfg(feature = "test-fixtures")] + api_configurations: Option>, } impl ClientBuilder { @@ -34,9 +36,20 @@ impl ClientBuilder { token_handler: Arc::new(NoopTokenHandler), state_registry: None, middleware: Vec::new(), + #[cfg(feature = "test-fixtures")] + api_configurations: None, } } + /// Overrides the [`ApiConfigurations`] used by the client being built, allowing tests to inject + /// a mocked [`bitwarden_api_api::apis::ApiClient`] via + /// [`ApiConfigurations::from_api_client`]. Only available for testing. + #[cfg(feature = "test-fixtures")] + pub fn with_api_configurations(mut self, api_configurations: Arc) -> Self { + self.api_configurations = Some(api_configurations); + self + } + /// Sets the [`ClientSettings`] for the client being built. pub fn with_settings(mut self, settings: ClientSettings) -> Self { self.settings = Some(settings); @@ -132,11 +145,18 @@ impl ClientBuilder { client: bw_http_client, }; + #[cfg(feature = "test-fixtures")] + let api_configurations = self + .api_configurations + .unwrap_or_else(|| ApiConfigurations::new(identity, api, settings.device_type)); + #[cfg(not(feature = "test-fixtures"))] + let api_configurations = ApiConfigurations::new(identity, api, settings.device_type); + let client = Client { internal: Arc::new(InternalClient { user_id: OnceLock::new(), token_handler: self.token_handler, - api_configurations: ApiConfigurations::new(identity, api, settings.device_type), + api_configurations, external_http_client, key_store, #[cfg(feature = "internal")] diff --git a/crates/bitwarden-core/src/client/test_accounts.rs b/crates/bitwarden-core/src/client/test_accounts.rs index 20e19fc17..3c785b9f0 100644 --- a/crates/bitwarden-core/src/client/test_accounts.rs +++ b/crates/bitwarden-core/src/client/test_accounts.rs @@ -10,6 +10,7 @@ use bitwarden_test::MemoryRepository; #[cfg(feature = "test-fixtures")] use crate::{ Client, ClientSettings, UserId, + client::ApiConfigurations, key_management::{ LocalUserDataKeyState, MasterPasswordUnlockData, UserKeyState, account_cryptographic_state::WrappedAccountCryptographicState, @@ -22,7 +23,11 @@ impl Client { /// Creates a client with the necessary state repositories for testing. /// Does not initialize any crypto state. pub fn new_test(settings: Option) -> Self { - let client = Client::new(settings); + Self::register_test_repositories(Client::new(settings)) + } + + /// Registers the state repositories required by the test-account crypto initialization. + fn register_test_repositories(client: Client) -> Self { client .platform() .state() @@ -40,8 +45,27 @@ impl Client { } pub async fn init_test_account(account: TestAccount) -> Self { - let client = Client::new_test(None); + Self::init_test_account_on(Client::new_test(None), account).await + } + + /// Creates a test account client backed by a mocked + /// [`bitwarden_api_api::apis::ApiClient`], so tests can assert on API requests made by + /// sub-clients that operate on a full [`Client`]. + pub async fn init_test_account_with_api_client( + account: TestAccount, + api_client: bitwarden_api_api::apis::ApiClient, + ) -> Self { + let client = Self::register_test_repositories( + Client::builder() + .with_api_configurations(std::sync::Arc::new(ApiConfigurations::from_api_client( + api_client, + ))) + .build(), + ); + Self::init_test_account_on(client, account).await + } + async fn init_test_account_on(client: Client, account: TestAccount) -> Self { client .flags() .load(HashMap::from([( diff --git a/crates/bitwarden-core/src/key_management/crypto.rs b/crates/bitwarden-core/src/key_management/crypto.rs index 458835cb0..ef1e22dec 100644 --- a/crates/bitwarden-core/src/key_management/crypto.rs +++ b/crates/bitwarden-core/src/key_management/crypto.rs @@ -471,6 +471,8 @@ pub(super) async fn get_user_encryption_key(client: &Client) -> Result for Kdf { + type Error = serde_wasm_bindgen::Error; + + fn try_from(value: wasm_bindgen::JsValue) -> Result { + serde_wasm_bindgen::from_value(value) + } +} + impl Kdf { /// Default KDF for new encryption V1 accounts. pub fn default_pbkdf2() -> Kdf { diff --git a/crates/bitwarden-uniffi/src/crypto.rs b/crates/bitwarden-uniffi/src/crypto.rs index fb2aa5e3d..22bd7ef1c 100644 --- a/crates/bitwarden-uniffi/src/crypto.rs +++ b/crates/bitwarden-uniffi/src/crypto.rs @@ -101,7 +101,9 @@ impl CryptoClient { /// Create the data necessary to update the user's kdf settings. The user's encryption key is /// re-encrypted for the password under the new kdf settings. This returns the new encrypted /// user key and the new password hash but does not update sdk state. + /// Note: This is deprecated. Please use the user-crypto-management client instead. pub async fn make_update_kdf(&self, password: String, kdf: Kdf) -> Result { + #[allow(deprecated)] Ok(self.0.make_update_kdf(password, kdf).await?) } diff --git a/crates/bitwarden-uniffi/swift/integration-tests/Tests/IntegrationTests/Utils.swift b/crates/bitwarden-uniffi/swift/integration-tests/Tests/IntegrationTests/Utils.swift index 6056433ca..a42fdf6d5 100644 --- a/crates/bitwarden-uniffi/swift/integration-tests/Tests/IntegrationTests/Utils.swift +++ b/crates/bitwarden-uniffi/swift/integration-tests/Tests/IntegrationTests/Utils.swift @@ -62,6 +62,7 @@ actor InMemoryStateBridge: StateBridgeForeignImpl { private var v2UpgradeToken: V2UpgradeToken? private var accountCryptographicState: WrappedAccountCryptographicState? private var masterpasswordUnlockData: MasterPasswordUnlockData? + private var kdfConfig: Kdf? func setUserKey(value: SymmetricCryptoKey) { userKey = value } func getUserKey() -> SymmetricCryptoKey? { userKey } @@ -90,6 +91,10 @@ actor InMemoryStateBridge: StateBridgeForeignImpl { func setMasterpasswordUnlockData(value: MasterPasswordUnlockData) { masterpasswordUnlockData = value } func getMasterpasswordUnlockData() -> MasterPasswordUnlockData? { masterpasswordUnlockData } func clearMasterpasswordUnlockData() { masterpasswordUnlockData = nil } + + func setKdfConfig(value: Kdf) { kdfConfig = value } + func getKdfConfig() -> Kdf? { kdfConfig } + func clearKdfConfig() { kdfConfig = nil } } final class MockTokenProvider: ClientManagedTokens { diff --git a/crates/bitwarden-user-crypto-management/Cargo.toml b/crates/bitwarden-user-crypto-management/Cargo.toml index 94615ddc5..3a2191755 100644 --- a/crates/bitwarden-user-crypto-management/Cargo.toml +++ b/crates/bitwarden-user-crypto-management/Cargo.toml @@ -54,7 +54,10 @@ wasm-bindgen-futures = { workspace = true, optional = true } [dev-dependencies] bitwarden-api-api = { workspace = true, features = ["mockall"] } -bitwarden-core = { workspace = true, features = ["internal-test-utils"] } +bitwarden-core = { workspace = true, features = [ + "internal-test-utils", + "test-fixtures", +] } bitwarden-test = { workspace = true } serde_json = { workspace = true } tokio = { workspace = true, features = ["rt"] } diff --git a/crates/bitwarden-user-crypto-management/src/change_kdf.rs b/crates/bitwarden-user-crypto-management/src/change_kdf.rs new file mode 100644 index 000000000..1fa3675a6 --- /dev/null +++ b/crates/bitwarden-user-crypto-management/src/change_kdf.rs @@ -0,0 +1,240 @@ +//! Client operation for changing the account's KDF (key derivation function) settings. + +use bitwarden_api_api::models::ChangeKdfRequestModel; +use bitwarden_core::{ + ApiError, NotAuthenticatedError, + key_management::{ + MasterPasswordAuthenticationData, MasterPasswordError, MasterPasswordUnlockData, + SymmetricKeySlotId, + }, +}; +use bitwarden_crypto::Kdf; +use bitwarden_error::bitwarden_error; +use thiserror::Error; +use tracing::error; +#[cfg(feature = "wasm")] +use wasm_bindgen::prelude::*; + +use crate::UserCryptoManagementClient; + +#[cfg_attr(feature = "wasm", wasm_bindgen)] +#[cfg_attr(feature = "uniffi", uniffi::export(async_runtime = "tokio"))] +impl UserCryptoManagementClient { + /// Changes the account's KDF settings, and sets them on the server. + pub async fn change_kdf(&self, password: String, new_kdf: Kdf) -> Result<(), ChangeKdfError> { + let bridge = self.client.km_state_bridge(); + + let current_unlock_data = bridge + .get_masterpassword_unlock_data() + .await + .ok_or(ChangeKdfError::MissingMasterPasswordUnlockData)?; + let salt = current_unlock_data.salt; + let current_kdf = current_unlock_data.kdf; + + // Re-derive the authentication and unlock data. The key store context must not be held + // across the await on the server request below, so it is scoped to this block. + let (old_authentication_data, authentication_data, unlock_data) = { + let ctx = self.client.internal.get_key_store().context(); + + let old_authentication_data = + MasterPasswordAuthenticationData::derive(&password, ¤t_kdf, &salt)?; + let authentication_data = + MasterPasswordAuthenticationData::derive(&password, &new_kdf, &salt)?; + let unlock_data = MasterPasswordUnlockData::derive( + &password, + &new_kdf, + &salt, + SymmetricKeySlotId::User, + &ctx, + )?; + + (old_authentication_data, authentication_data, unlock_data) + }; + + // Build and post the change-KDF request to the server. The old authentication hash proves + // possession of the current password; the new authentication and unlock data replace it. + let request = ChangeKdfRequestModel { + master_password_hash: old_authentication_data + .master_password_authentication_hash + .to_string(), + authentication_data: Box::new((&authentication_data).into()), + unlock_data: Box::new((&unlock_data).into()), + }; + + self.client + .internal + .get_api_configurations() + .api_client + .accounts_api() + .post_kdf(Some(request)) + .await + .map_err(|e| { + error!("Failed to post change-kdf request: {e:?}"); + ApiError::from(e) + })?; + + // Persist the new unlock data and KDF config to client-managed state via the state bridge, + // then update the KDF held in the internal client so in-memory state stays consistent. + bridge.set_masterpassword_unlock_data(&unlock_data).await; + bridge.set_kdf_config(&new_kdf).await; + self.client + .internal + .set_user_master_password_unlock(unlock_data) + .await?; + + Ok(()) + } +} + +/// Errors that can occur while changing the account KDF settings. +#[derive(Debug, Error)] +#[bitwarden_error(flat)] +pub enum ChangeKdfError { + /// Deriving the new authentication or unlock data failed. + #[error(transparent)] + MasterPassword(#[from] MasterPasswordError), + /// The current master-password unlock data is not available in the state bridge. + #[error("Master password unlock data is not available")] + MissingMasterPasswordUnlockData, + /// The client is not authenticated with a master password. + #[error(transparent)] + NotAuthenticated(#[from] NotAuthenticatedError), + /// The server rejected the change-KDF request. + #[error(transparent)] + Api(#[from] ApiError), +} + +#[cfg(test)] +mod tests { + use std::num::NonZeroU32; + + use bitwarden_api_api::apis::ApiClient; + use bitwarden_core::{ + Client, + client::test_accounts::test_bitwarden_com_account, + key_management::{ + MasterPasswordUnlockData, state_bridge::test_support::InMemoryStateBridge, + }, + }; + use bitwarden_crypto::Kdf; + + use super::*; + use crate::UserCryptoManagementClientExt; + + const TEST_PASSWORD: &str = "asdfasdfasdf"; + const TEST_EMAIL: &str = "test@bitwarden.com"; + // A valid EncString; only the salt/kdf of the seeded unlock data are used by derivation. + const TEST_WRAPPED_USER_KEY: &str = "2.Q/2PhzcC7GdeiMHhWguYAQ==|GpqzVdr0go0ug5cZh1n+uixeBC3oC90CIe0hd/HWA/pTRDZ8ane4fmsEIcuc8eMKUt55Y2q/fbNzsYu41YTZzzsJUSeqVjT8/iTQtgnNdpo=|dwI+uyvZ1h/iZ03VQ+/wrGEFYVewBUUl/syYgjsNMbE="; + + fn new_kdf() -> Kdf { + Kdf::PBKDF2 { + iterations: NonZeroU32::new(700_000).unwrap(), + } + } + + // Sets the master-password unlock data in the state bridge to a fixed value for testing. + async fn test_unlock_data(client: &Client) { + let current_kdf = client.internal.get_kdf().await.unwrap(); + client + .km_state_bridge() + .set_masterpassword_unlock_data(&MasterPasswordUnlockData { + kdf: current_kdf, + master_key_wrapped_user_key: TEST_WRAPPED_USER_KEY.parse().unwrap(), + salt: TEST_EMAIL.to_string(), + }) + .await; + } + + #[tokio::test] + async fn test_change_kdf_success_posts_and_persists() { + let api_client = ApiClient::new_mocked(|mock| { + mock.accounts_api + .expect_post_kdf() + .once() + .returning(|body| { + let body = body.expect("body should be Some"); + // The unlock/authentication data must carry the new KDF settings. + assert_eq!(body.authentication_data.kdf.iterations, 700_000); + assert_eq!(body.unlock_data.kdf.iterations, 700_000); + assert!(!body.master_password_hash.is_empty()); + Ok(()) + }); + }); + + let client = + Client::init_test_account_with_api_client(test_bitwarden_com_account(), api_client) + .await; + client + .km_state_bridge() + .register_bridge(Box::new(InMemoryStateBridge::default())); + test_unlock_data(&client).await; + + client + .user_crypto_management() + .change_kdf(TEST_PASSWORD.to_string(), new_kdf()) + .await + .unwrap(); + + // The new KDF config and unlock data are persisted to state. + let bridge = client.km_state_bridge(); + assert_eq!(bridge.get_kdf_config().await, Some(new_kdf())); + let unlock = bridge + .get_masterpassword_unlock_data() + .await + .expect("unlock data persisted"); + assert_eq!(unlock.kdf, new_kdf()); + // The internal client's KDF is updated as well. + assert_eq!(client.internal.get_kdf().await.unwrap(), new_kdf()); + } + + #[tokio::test] + async fn test_change_kdf_api_failure_does_not_persist() { + let api_client = ApiClient::new_mocked(|mock| { + mock.accounts_api + .expect_post_kdf() + .once() + .returning(|_body| Err(std::io::Error::other("Simulated error").into())); + }); + + let client = + Client::init_test_account_with_api_client(test_bitwarden_com_account(), api_client) + .await; + client + .km_state_bridge() + .register_bridge(Box::new(InMemoryStateBridge::default())); + test_unlock_data(&client).await; + + let current_kdf = client.internal.get_kdf().await.unwrap(); + + let result = client + .user_crypto_management() + .change_kdf(TEST_PASSWORD.to_string(), new_kdf()) + .await; + + assert!(matches!(result, Err(ChangeKdfError::Api(_)))); + // The KDF config is untouched when the server call fails. + assert_eq!(client.km_state_bridge().get_kdf_config().await, None); + assert_eq!(client.internal.get_kdf().await.unwrap(), current_kdf); + } + + #[tokio::test] + async fn test_change_kdf_missing_unlock_data_errors() { + let api_client = ApiClient::new_mocked(|_mock| {}); + let client = + Client::init_test_account_with_api_client(test_bitwarden_com_account(), api_client) + .await; + client + .km_state_bridge() + .register_bridge(Box::new(InMemoryStateBridge::default())); + + let result = client + .user_crypto_management() + .change_kdf(TEST_PASSWORD.to_string(), new_kdf()) + .await; + + assert!(matches!( + result, + Err(ChangeKdfError::MissingMasterPasswordUnlockData) + )); + } +} diff --git a/crates/bitwarden-user-crypto-management/src/lib.rs b/crates/bitwarden-user-crypto-management/src/lib.rs index bc895edbe..379c2af85 100644 --- a/crates/bitwarden-user-crypto-management/src/lib.rs +++ b/crates/bitwarden-user-crypto-management/src/lib.rs @@ -4,11 +4,13 @@ #[cfg(feature = "uniffi")] uniffi::setup_scaffolding!(); +mod change_kdf; mod key_connector_migration; mod key_rotation; mod pin_settings; mod public_key_encryption_key_pair_regeneration; mod user_crypto_management_client; +pub use change_kdf::ChangeKdfError; pub use pin_settings::PinSettingsClient; pub use user_crypto_management_client::{ UserCryptoManagementClient, UserCryptoManagementClientExt, diff --git a/crates/bitwarden-wasm-internal/integration-tests/tests/user-crypto-management/change-kdf.test.ts b/crates/bitwarden-wasm-internal/integration-tests/tests/user-crypto-management/change-kdf.test.ts new file mode 100644 index 000000000..f48807d64 --- /dev/null +++ b/crates/bitwarden-wasm-internal/integration-tests/tests/user-crypto-management/change-kdf.test.ts @@ -0,0 +1,291 @@ +import { + ChangeKdfError, + ClientSettings, + Kdf, + MasterPasswordUnlockData, + PasswordManagerClient, + WasmStateBridge, + isChangeKdfError, +} from "@bitwarden/sdk-internal"; + +import { HttpMock, installHttpMock } from "../http-mock"; +import { + MASTER_KEY_WRAPPED_USER_KEY, + TEST_EMAIL, + TEST_KDF_PARAMS, + TEST_PASSWORD, + initializeUserCrypto, + makeInitializedPasswordmanagerClient, + makePasswordManagerClient, + makeStateBridge, + seedMasterPasswordUnlockData, +} from "../utils"; + +// Nothing listens here; every request is served by the fetch mock. A concrete host keeps the +// SDK's request URLs parseable and makes an unmocked route fail loudly rather than escape to +// the network. +const SETTINGS: ClientSettings = { + apiUrl: "http://localhost:4000", + identityUrl: "http://localhost:4000/identity", +}; + +const ROUTE = "POST /accounts/kdf"; + +const NEW_PBKDF2: Kdf = { pBKDF2: { iterations: 700_000 } }; +const NEW_ARGON2: Kdf = { argon2id: { iterations: 3, memory: 16, parallelism: 4 } }; + +const PBKDF2_SHA256 = 0; +const ARGON2ID = 1; + +const TIMEOUT = 60_000; + +/** Guards the user-key comparisons below against passing on two `undefined`s. */ +const BASE64_PATTERN = /^[A-Za-z0-9+/]{40,}={0,2}$/; + +/** A client unlocked under {@link TEST_KDF_PARAMS} with its master-password state seeded. */ +async function setup(): Promise<{ stateBridge: WasmStateBridge; client: PasswordManagerClient }> { + const stateBridge = makeStateBridge(); + const client = await makeInitializedPasswordmanagerClient(stateBridge, SETTINGS); + await seedMasterPasswordUnlockData(stateBridge); + return { stateBridge, client }; +} + +/** The happy-path stand-in for `POST /accounts/kdf`, which answers with an empty 200. */ +const okRoutes = () => ({ [ROUTE]: () => ({}) }); + +/** Awaits a rejection and narrows it to a {@link ChangeKdfError}. */ +async function rejection(promise: Promise): Promise { + const thrown = await promise.then( + () => undefined, + (error) => error, + ); + if (!isChangeKdfError(thrown)) { + throw new Error(`expected a ChangeKdfError, got ${thrown}`); + } + return thrown; +} + +describe("change kdf", () => { + let mock: HttpMock; + + afterEach(() => { + expect(mock.unmatched.map((request) => request.route)).toEqual([]); + mock.restore(); + }); + + describe("request", () => { + it( + "posts the re-derived authentication and unlock data under the new KDF", + async () => { + mock = installHttpMock(okRoutes()); + const { client } = await setup(); + + await client.user_crypto_management().change_kdf(TEST_PASSWORD, NEW_PBKDF2); + + expect(mock.routes()).toEqual([ROUTE]); + + const posted = mock.bodyFor(ROUTE); + expect(Object.keys(posted).sort()).toEqual([ + "authenticationData", + "masterPasswordHash", + "unlockData", + ]); + expect(Object.keys(posted.authenticationData).sort()).toEqual([ + "kdf", + "masterPasswordAuthenticationHash", + "salt", + ]); + expect(Object.keys(posted.unlockData).sort()).toEqual([ + "kdf", + "masterKeyWrappedUserKey", + "salt", + ]); + + // Both halves carry the new KDF; memory and parallelism are omitted for PBKDF2. + const kdf = { kdfType: PBKDF2_SHA256, iterations: 700_000 }; + expect(posted.authenticationData.kdf).toEqual(kdf); + expect(posted.unlockData.kdf).toEqual(kdf); + + // The salt is carried over from the current unlock data, not re-derived. + expect(posted.authenticationData.salt).toBe(TEST_EMAIL); + expect(posted.unlockData.salt).toBe(TEST_EMAIL); + + // The user key is re-wrapped under the master key derived with the new KDF. + expect(posted.unlockData.masterKeyWrappedUserKey).toMatch(/^2\./); + expect(posted.unlockData.masterKeyWrappedUserKey).not.toBe(MASTER_KEY_WRAPPED_USER_KEY); + }, + TIMEOUT, + ); + + it( + "proves possession with a hash derived under the old KDF", + async () => { + mock = installHttpMock(okRoutes()); + const { client } = await setup(); + + await client.user_crypto_management().change_kdf(TEST_PASSWORD, NEW_PBKDF2); + + const posted = mock.bodyFor(ROUTE); + // `masterPasswordHash` is the old-KDF hash the server authenticates the change with; + // `authenticationData` is what replaces it. Same password, different KDF, so they differ. + expect(posted.masterPasswordHash).not.toBe( + posted.authenticationData.masterPasswordAuthenticationHash, + ); + expect(posted.masterPasswordHash).not.toBe(""); + }, + TIMEOUT, + ); + + it( + "converts the argon2id variant across the boundary", + async () => { + mock = installHttpMock(okRoutes()); + const { stateBridge, client } = await setup(); + + await client.user_crypto_management().change_kdf(TEST_PASSWORD, NEW_ARGON2); + + const posted = mock.bodyFor(ROUTE); + const kdf = { kdfType: ARGON2ID, iterations: 3, memory: 16, parallelism: 4 }; + expect(posted.authenticationData.kdf).toEqual(kdf); + expect(posted.unlockData.kdf).toEqual(kdf); + + // Round trip: the argon2id variant survives the trip out to the bridge as well. + expect(await stateBridge.get_kdf_config()).toEqual(NEW_ARGON2); + }, + TIMEOUT, + ); + }); + + describe("persisted state", () => { + it( + "writes the new KDF config and unlock data to the state bridge", + async () => { + mock = installHttpMock(okRoutes()); + const { stateBridge, client } = await setup(); + + await client.user_crypto_management().change_kdf(TEST_PASSWORD, NEW_PBKDF2); + + expect(await stateBridge.get_kdf_config()).toEqual(NEW_PBKDF2); + + const persisted = await stateBridge.get_masterpassword_unlock_data(); + expect(persisted).toBeDefined(); + expect(persisted!.kdf).toEqual(NEW_PBKDF2); + expect(persisted!.salt).toBe(TEST_EMAIL); + // Exactly what was posted, so client and server cannot disagree about the wrapped key. + expect(persisted!.masterKeyWrappedUserKey).toBe( + mock.bodyFor(ROUTE).unlockData.masterKeyWrappedUserKey, + ); + }, + TIMEOUT, + ); + + it( + "leaves the persisted unlock data usable: a fresh client recovers the same user key", + async () => { + mock = installHttpMock(okRoutes()); + const { stateBridge, client } = await setup(); + const userKey = await client.crypto().get_user_encryption_key(); + expect(userKey).toMatch(BASE64_PATTERN); + + await client.user_crypto_management().change_kdf(TEST_PASSWORD, NEW_PBKDF2); + const persisted = await stateBridge.get_masterpassword_unlock_data(); + + // A new client over a fresh bridge, unlocked from nothing but the persisted data. + const next = makePasswordManagerClient(makeStateBridge(), SETTINGS); + await initializeUserCrypto( + next, + { + masterPasswordUnlock: { + password: TEST_PASSWORD, + master_password_unlock: persisted as MasterPasswordUnlockData, + }, + }, + NEW_PBKDF2, + ); + + // Changing the KDF re-wraps the user key; it must not change it. + expect(await next.crypto().get_user_encryption_key()).toBe(userKey); + }, + TIMEOUT, + ); + }); + + describe("failures", () => { + it( + "leaves state untouched when the server rejects the change", + async () => { + mock = installHttpMock({ + [ROUTE]: () => ({ status: 400, json: { message: "kdf change rejected" } }), + }); + const { stateBridge, client } = await setup(); + + const error = await rejection( + client.user_crypto_management().change_kdf(TEST_PASSWORD, NEW_PBKDF2), + ); + + expect(error.variant).toBe("Api"); + expect(mock.routes()).toEqual([ROUTE]); + // Nothing is persisted, so the account is not left half-migrated. + expect(await stateBridge.get_kdf_config()).toBeNull(); + const unlockData = await stateBridge.get_masterpassword_unlock_data(); + expect(unlockData!.kdf).toEqual(TEST_KDF_PARAMS); + expect(unlockData!.masterKeyWrappedUserKey).toBe(MASTER_KEY_WRAPPED_USER_KEY); + }, + TIMEOUT, + ); + + it( + "errors before making a request when the unlock data is missing", + async () => { + mock = installHttpMock(okRoutes()); + // No `seedMasterPasswordUnlockData`: the bridge has no master-password state. + const client = await makeInitializedPasswordmanagerClient(makeStateBridge(), SETTINGS); + + const error = await rejection( + client.user_crypto_management().change_kdf(TEST_PASSWORD, NEW_PBKDF2), + ); + + expect(error.variant).toBe("MissingMasterPasswordUnlockData"); + expect(mock.requests).toEqual([]); + }, + TIMEOUT, + ); + + it( + "errors before making a request when the new KDF is below the allowed minimum", + async () => { + mock = installHttpMock(okRoutes()); + const { stateBridge, client } = await setup(); + const belowMinimum: Kdf = { argon2id: { iterations: 1, memory: 16, parallelism: 1 } }; + + const error = await rejection( + client.user_crypto_management().change_kdf(TEST_PASSWORD, belowMinimum), + ); + + expect(error.variant).toBe("MasterPassword"); + expect(mock.requests).toEqual([]); + expect(await stateBridge.get_kdf_config()).toBeNull(); + }, + TIMEOUT, + ); + }); + + it( + "never sends the password or the user key", + async () => { + mock = installHttpMock(okRoutes()); + const { client } = await setup(); + const userKey = await client.crypto().get_user_encryption_key(); + expect(userKey).toMatch(BASE64_PATTERN); + + await client.user_crypto_management().change_kdf(TEST_PASSWORD, NEW_PBKDF2); + + expect(mock.requests).not.toEqual([]); + for (const request of mock.requests) { + expect(request.body).not.toContain(TEST_PASSWORD); + expect(request.body).not.toContain(userKey); + } + }, + TIMEOUT, + ); +}); diff --git a/crates/bitwarden-wasm-internal/integration-tests/tests/utils.ts b/crates/bitwarden-wasm-internal/integration-tests/tests/utils.ts index 1ec2020c6..9627d0951 100644 --- a/crates/bitwarden-wasm-internal/integration-tests/tests/utils.ts +++ b/crates/bitwarden-wasm-internal/integration-tests/tests/utils.ts @@ -23,6 +23,7 @@ import { SharedUnlockLeader, InitUserCryptoMethod, ClientSettings, + Kdf, } from "@bitwarden/sdk-internal"; import { ORG_ACCOUNT_KDF_PARAMS, @@ -46,6 +47,9 @@ export function makeStateBridge(): WasmStateBridge { let v2UpgradeToken: V2UpgradeToken | null; let accountCryptographicState: WrappedAccountCryptographicState | null; let masterPasswordUnlockData: MasterPasswordUnlockData | null; + // Initialized, unlike the slots above, so an untouched bridge reports `null` rather than + // `undefined` — tests assert on the absence of a KDF config after a failed change. + let kdfConfig: Kdf | null = null; return { set_user_key: async (v: SymmetricKey) => { @@ -103,6 +107,14 @@ export function makeStateBridge(): WasmStateBridge { clear_masterpassword_unlock_data: async () => { masterPasswordUnlockData = null; }, + + set_kdf_config: async (v: Kdf) => { + kdfConfig = v; + }, + get_kdf_config: async () => kdfConfig, + clear_kdf_config: async () => { + kdfConfig = null; + }, }; } @@ -159,23 +171,36 @@ export function initializeCryptoDefault(client: PasswordManagerClient) { export function initializeUserCrypto( client: PasswordManagerClient, initUserCryptoMethod: InitUserCryptoMethod, + kdfParams: Kdf = TEST_KDF_PARAMS, ) { return client.crypto().initialize_user_crypto({ userId: TEST_USER_ID, - kdfParams: TEST_KDF_PARAMS, + kdfParams, email: TEST_EMAIL, accountCryptographicState: { V1: { private_key: encstring(PRIVATE_KEY) } }, method: initUserCryptoMethod, }); } +export function seedMasterPasswordUnlockData( + stateBridge: WasmStateBridge, + kdf: Kdf = TEST_KDF_PARAMS, +): Promise { + return stateBridge.set_masterpassword_unlock_data({ + kdf, + masterKeyWrappedUserKey: encstring(MASTER_KEY_WRAPPED_USER_KEY), + salt: TEST_EMAIL, + }); +} + /** * Makes a password manager client with an initialized crypto state for testing. */ export async function makeInitializedPasswordmanagerClient( stateBridge: WasmStateBridge, + settings?: ClientSettings, ): Promise { - const client = makePasswordManagerClient(stateBridge); + const client = makePasswordManagerClient(stateBridge, settings); await initializeCryptoDefault(client); return client; }