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 @@ -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"] }
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 @@ -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")]
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 @@ -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"
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 @@ -82,6 +82,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 @@ -122,6 +124,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
18 changes: 16 additions & 2 deletions crates/bitwarden-crypto/src/store/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ struct KeyStoreInner<Ids: KeyIds> {
symmetric_keys: Box<dyn StoreBackend<Ids::Symmetric>>,
asymmetric_keys: Box<dyn StoreBackend<Ids::Asymmetric>>,
signing_keys: Box<dyn StoreBackend<Ids::Signing>>,
security_state_version: u64,
}

/// Create a new key store with the best available implementation for the current platform.
Expand All @@ -123,6 +124,7 @@ impl<Ids: KeyIds> Default for KeyStore<Ids> {
symmetric_keys: create_store(),
asymmetric_keys: create_store(),
signing_keys: create_store(),
security_state_version: 1,
})),
}
}
Expand All @@ -138,6 +140,12 @@ 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 data = self.inner.write().expect("RwLock is poisoned");
data.security_state_version = version;
Copy link
Member

Choose a reason for hiding this comment

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

Non-blocking question: Would we want to add some rollback protection here in the future? Something like:

if version < data.security_state_version {
    return Err(...);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! However, I do think this should be considered when sync and at least the crypto state is managed in SDK.

}

/// 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 @@ -170,11 +178,14 @@ impl<Ids: KeyIds> KeyStore<Ids> {
/// - [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,
}
}
Expand All @@ -200,11 +211,14 @@ impl<Ids: KeyIds> KeyStore<Ids> {
/// 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,
}
}
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 @@ -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 }
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 @@ -394,7 +394,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