diff --git a/Cargo.lock b/Cargo.lock index 5bcdfd657..81a6dfdee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -932,6 +932,7 @@ dependencies = [ "serde_repr", "sha1", "sha2", + "subtle", "thiserror 2.0.12", "tokio", "tsify", diff --git a/Cargo.toml b/Cargo.toml index 8e221f56f..4dbe8c2f4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -73,6 +73,7 @@ serde_json = ">=1.0.96, <2.0" serde_qs = ">=0.12.0, <0.16" serde_repr = ">=0.1.12, <0.2" serde-wasm-bindgen = ">=0.6.0, <0.7" +subtle = ">=2.5.0, <3.0" syn = ">=2.0.87, <3" thiserror = ">=1.0.40, <3" tokio = { version = "1.36.0", features = ["macros"] } diff --git a/crates/bitwarden-core/src/client/encryption_settings.rs b/crates/bitwarden-core/src/client/encryption_settings.rs index d7411bc86..b7e1f3700 100644 --- a/crates/bitwarden-core/src/client/encryption_settings.rs +++ b/crates/bitwarden-core/src/client/encryption_settings.rs @@ -170,6 +170,7 @@ impl EncryptionSettings { let security_state: SecurityState = security_state .verify_and_unwrap(&signing_key.to_verifying_key()) .map_err(|_| EncryptionSettingsError::InvalidSecurityState)?; + store.set_security_state_version(security_state.version()); *sdk_security_state.write().expect("RwLock not poisoned") = Some(security_state); #[allow(deprecated)] diff --git a/crates/bitwarden-core/src/key_management/mod.rs b/crates/bitwarden-core/src/key_management/mod.rs index 2b14cecac..44e7d8134 100644 --- a/crates/bitwarden-core/src/key_management/mod.rs +++ b/crates/bitwarden-core/src/key_management/mod.rs @@ -28,7 +28,9 @@ pub(crate) use master_password::{MasterPasswordAuthenticationData, MasterPasswor #[cfg(feature = "internal")] mod security_state; #[cfg(feature = "internal")] -pub use security_state::{SecurityState, SignedSecurityState}; +pub use security_state::{ + MINIMUM_ENFORCE_ICON_URI_HASH_VERSION, SecurityState, SignedSecurityState, +}; #[cfg(feature = "internal")] mod user_decryption; #[cfg(feature = "internal")] diff --git a/crates/bitwarden-core/src/key_management/security_state.rs b/crates/bitwarden-core/src/key_management/security_state.rs index f5927638c..717c411ec 100644 --- a/crates/bitwarden-core/src/key_management/security_state.rs +++ b/crates/bitwarden-core/src/key_management/security_state.rs @@ -31,6 +31,9 @@ use serde::{Deserialize, Serialize}; use crate::UserId; +/// Icon URI hashes are enforced starting with this security state version. +pub const MINIMUM_ENFORCE_ICON_URI_HASH_VERSION: u64 = 2; + #[cfg(feature = "wasm")] #[wasm_bindgen::prelude::wasm_bindgen(typescript_custom_section)] const TS_CUSTOM_TYPES: &'static str = r#" diff --git a/crates/bitwarden-crypto/Cargo.toml b/crates/bitwarden-crypto/Cargo.toml index 05f064588..c83ec9b57 100644 --- a/crates/bitwarden-crypto/Cargo.toml +++ b/crates/bitwarden-crypto/Cargo.toml @@ -55,7 +55,7 @@ serde_bytes = { workspace = true } serde_repr.workspace = true sha1 = ">=0.10.5, <0.11" sha2 = ">=0.10.6, <0.11" -subtle = ">=2.5.0, <3.0" +subtle = { workspace = true } thiserror = { workspace = true } tsify = { workspace = true, optional = true } typenum = ">=1.18.0, <1.19.0" diff --git a/crates/bitwarden-crypto/src/store/context.rs b/crates/bitwarden-crypto/src/store/context.rs index 6e602f704..4a657baa6 100644 --- a/crates/bitwarden-crypto/src/store/context.rs +++ b/crates/bitwarden-crypto/src/store/context.rs @@ -82,6 +82,8 @@ pub struct KeyStoreContext<'a, Ids: KeyIds> { pub(super) local_asymmetric_keys: Box>, pub(super) local_signing_keys: Box>, + pub(super) security_state_version: u64, + // Make sure the context is !Send & !Sync pub(super) _phantom: std::marker::PhantomData<(Cell<()>, RwLockReadGuard<'static, ()>)>, } @@ -122,6 +124,13 @@ impl KeyStoreContext<'_, Ids> { self.local_signing_keys.clear(); } + /// Returns the version of the security state of the key context. This describes the user's + /// encryption version and can be used to disable certain old / dangerous format features + /// safely. + pub fn get_security_state_version(&self) -> u64 { + self.security_state_version + } + /// Remove all symmetric keys from the context for which the predicate returns false /// This will also remove the keys from the global store if this context has write access pub fn retain_symmetric_keys(&mut self, f: fn(Ids::Symmetric) -> bool) { diff --git a/crates/bitwarden-crypto/src/store/mod.rs b/crates/bitwarden-crypto/src/store/mod.rs index a61bdd5cd..6ce365e0d 100644 --- a/crates/bitwarden-crypto/src/store/mod.rs +++ b/crates/bitwarden-crypto/src/store/mod.rs @@ -113,6 +113,7 @@ struct KeyStoreInner { symmetric_keys: Box>, asymmetric_keys: Box>, signing_keys: Box>, + security_state_version: u64, } /// Create a new key store with the best available implementation for the current platform. @@ -123,6 +124,7 @@ impl Default for KeyStore { symmetric_keys: create_store(), asymmetric_keys: create_store(), signing_keys: create_store(), + security_state_version: 1, })), } } @@ -138,6 +140,12 @@ impl KeyStore { keys.signing_keys.clear(); } + /// Sets the security state version for this store. + pub fn set_security_state_version(&self, version: u64) { + let mut data = self.inner.write().expect("RwLock is poisoned"); + data.security_state_version = version; + } + /// Initiate an encryption/decryption context. This context will have read only access to the /// global keys, and will have its own local key stores with read/write access. This /// context-local store will be cleared when the context is dropped. @@ -170,11 +178,14 @@ impl KeyStore { /// - [KeyStoreContext::encapsulate_key_unsigned] /// - [KeyStoreContext::derive_shareable_key] pub fn context(&'_ self) -> KeyStoreContext<'_, Ids> { + let data = self.inner.read().expect("RwLock is poisoned"); + let security_state_version = data.security_state_version; KeyStoreContext { - global_keys: GlobalKeys::ReadOnly(self.inner.read().expect("RwLock is poisoned")), + global_keys: GlobalKeys::ReadOnly(data), local_symmetric_keys: create_store(), local_asymmetric_keys: create_store(), local_signing_keys: create_store(), + security_state_version, _phantom: std::marker::PhantomData, } } @@ -200,11 +211,14 @@ impl KeyStore { /// you're not holding references to the context across await points, as that would cause the /// future to also not be [Send]. pub fn context_mut(&'_ self) -> KeyStoreContext<'_, Ids> { + let inner = self.inner.write().expect("RwLock is poisoned"); + let security_state_version = inner.security_state_version; KeyStoreContext { - global_keys: GlobalKeys::ReadWrite(self.inner.write().expect("RwLock is poisoned")), + global_keys: GlobalKeys::ReadWrite(inner), local_symmetric_keys: create_store(), local_asymmetric_keys: create_store(), local_signing_keys: create_store(), + security_state_version, _phantom: std::marker::PhantomData, } } diff --git a/crates/bitwarden-vault/Cargo.toml b/crates/bitwarden-vault/Cargo.toml index e6eb2abda..8e1ee7e10 100644 --- a/crates/bitwarden-vault/Cargo.toml +++ b/crates/bitwarden-vault/Cargo.toml @@ -50,6 +50,7 @@ serde_json = { workspace = true } serde_repr = { workspace = true } sha1 = ">=0.10.5, <0.11" sha2 = ">=0.10.6, <0.11" +subtle = { workspace = true } thiserror = { workspace = true } tsify = { workspace = true, optional = true } uniffi = { workspace = true, optional = true } diff --git a/crates/bitwarden-vault/src/cipher/cipher.rs b/crates/bitwarden-vault/src/cipher/cipher.rs index 7d5fce9f8..235421401 100644 --- a/crates/bitwarden-vault/src/cipher/cipher.rs +++ b/crates/bitwarden-vault/src/cipher/cipher.rs @@ -2,7 +2,7 @@ use bitwarden_api_api::models::{CipherDetailsResponseModel, CipherResponseModel} use bitwarden_collections::collection::CollectionId; use bitwarden_core::{ MissingFieldError, OrganizationId, UserId, - key_management::{KeyIds, SymmetricKeyId}, + key_management::{KeyIds, MINIMUM_ENFORCE_ICON_URI_HASH_VERSION, SymmetricKeyId}, require, }; use bitwarden_crypto::{ @@ -394,7 +394,10 @@ impl Decryptable for Cipher { }; // For compatibility we only remove URLs with invalid checksums if the cipher has a key - if cipher.key.is_some() { + // or the user is on Crypto V2 + if cipher.key.is_some() + || ctx.get_security_state_version() >= MINIMUM_ENFORCE_ICON_URI_HASH_VERSION + { cipher.remove_invalid_checksums(); } diff --git a/crates/bitwarden-vault/src/cipher/login.rs b/crates/bitwarden-vault/src/cipher/login.rs index cd6ecca38..4f48ed80c 100644 --- a/crates/bitwarden-vault/src/cipher/login.rs +++ b/crates/bitwarden-vault/src/cipher/login.rs @@ -11,6 +11,7 @@ use bitwarden_encoding::B64; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; use serde_repr::{Deserialize_repr, Serialize_repr}; +use subtle::ConstantTimeEq; #[cfg(feature = "wasm")] use tsify::Tsify; #[cfg(feature = "wasm")] @@ -70,7 +71,7 @@ impl LoginUriView { use sha2::Digest; let uri_hash = sha2::Sha256::new().chain_update(uri.as_bytes()).finalize(); - uri_hash.as_slice() == cs.as_bytes() + uri_hash.as_slice().ct_eq(cs.as_bytes()).into() } pub(crate) fn generate_checksum(&mut self) {