Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,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"] }
Expand Down
1 change: 1 addition & 0 deletions crates/bitwarden-core/src/client/encryption_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
4 changes: 3 additions & 1 deletion crates/bitwarden-core/src/key_management/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ pub(crate) use non_generic_wrappers::*;
#[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")]
Expand Down
3 changes: 3 additions & 0 deletions crates/bitwarden-core/src/key_management/security_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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#"
Expand Down
2 changes: 1 addition & 1 deletion crates/bitwarden-crypto/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,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"
Expand Down
9 changes: 9 additions & 0 deletions crates/bitwarden-crypto/src/store/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ pub struct KeyStoreContext<'a, Ids: KeyIds> {
pub(super) local_asymmetric_keys: Box<dyn StoreBackend<Ids::Asymmetric>>,
pub(super) local_signing_keys: Box<dyn StoreBackend<Ids::Signing>>,

pub(super) security_state_version: u64,

// Make sure the context is !Send & !Sync
pub(super) _phantom: std::marker::PhantomData<(Cell<()>, RwLockReadGuard<'static, ()>)>,
}
Expand Down Expand Up @@ -121,6 +123,13 @@ impl<Ids: KeyIds> 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) {
Expand Down
19 changes: 19 additions & 0 deletions crates/bitwarden-crypto/src/store/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ pub struct KeyStore<Ids: KeyIds> {
// We use an Arc<> to make it easier to pass this store around, as we can
// clone it instead of passing references
inner: Arc<RwLock<KeyStoreInner<Ids>>>,
security_state_version: Arc<RwLock<u64>>,
Copy link
Member

@dani-garcia dani-garcia Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this instead be a part of KeyStoreInner to reuse the existing lock?

Having two locks seems wasteful and I feel like could lead to accidental deadlocks. In fact the current code in context_mut() smells like it could cause a deadlock to me, as we get a write lock for the global_keys and then we try to read the security state while holding the lock. If any other part of the codebase read those two in the opposite order, we'd have issues.

}

/// [KeyStore] contains sensitive data, provide a dummy [Debug] implementation.
Expand All @@ -124,6 +125,7 @@ impl<Ids: KeyIds> Default for KeyStore<Ids> {
asymmetric_keys: create_store(),
signing_keys: create_store(),
})),
security_state_version: Arc::new(RwLock::new(1)),
}
}
}
Expand All @@ -138,6 +140,15 @@ impl<Ids: KeyIds> KeyStore<Ids> {
keys.signing_keys.clear();
}

/// Sets the security state version for this store.
pub fn set_security_state_version(&self, version: u64) {
let mut ver = self
.security_state_version
.write()
.expect("RwLock is poisoned");
*ver = 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.
Expand Down Expand Up @@ -175,6 +186,10 @@ impl<Ids: KeyIds> KeyStore<Ids> {
local_symmetric_keys: create_store(),
local_asymmetric_keys: create_store(),
local_signing_keys: create_store(),
security_state_version: *self
.security_state_version
.read()
.expect("RwLock is poisoned"),
_phantom: std::marker::PhantomData,
}
}
Expand Down Expand Up @@ -205,6 +220,10 @@ impl<Ids: KeyIds> KeyStore<Ids> {
local_symmetric_keys: create_store(),
local_asymmetric_keys: create_store(),
local_signing_keys: create_store(),
security_state_version: *self
.security_state_version
.read()
.expect("RwLock is poisoned"),
_phantom: std::marker::PhantomData,
}
}
Expand Down
1 change: 1 addition & 0 deletions crates/bitwarden-vault/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,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 }
Expand Down
7 changes: 5 additions & 2 deletions crates/bitwarden-vault/src/cipher/cipher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -389,7 +389,10 @@ impl Decryptable<KeyIds, SymmetricKeyId, CipherView> 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();
}

Expand Down
3 changes: 2 additions & 1 deletion crates/bitwarden-vault/src/cipher/login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -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) {
Expand Down
Loading