Skip to content

[PM-18104] Implement generic key decryption in KeyStoreContext #152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

[PM-18104] Implement generic key decryption in KeyStoreContext #152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bde9f09
Implement generic key decryption in KeyStoreContext
dani-garcia Feb 5, 2025
820463a
Fix doclinks
dani-garcia Feb 7, 2025
ce39bca
Fix MSRV
dani-garcia Feb 7, 2025
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
2 changes: 1 addition & 1 deletion crates/bitwarden-core/src/auth/auth_client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@
let ctx = key_store.context();
// FIXME: [PM-18099] Once DeviceKey deals with KeyIds, this should be updated
#[allow(deprecated)]
let user_key = ctx.dangerous_get_symmetric_key(SymmetricKeyId::User)?;
let user_key = ctx.dangerous_get_key(SymmetricKeyId::User)?;

Check warning on line 194 in crates/bitwarden-core/src/auth/auth_client.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-core/src/auth/auth_client.rs#L194 

Added line #L194 was not covered by tests

Ok(DeviceKey::trust_device(user_key)?)
}
Expand Down
6 changes: 3 additions & 3 deletions crates/bitwarden-core/src/auth/auth_request.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ pub(crate) fn approve_auth_request(

// FIXME: [PM-18110] This should be removed once the key store can handle public key encryption
#[allow(deprecated)]
let key = ctx.dangerous_get_symmetric_key(SymmetricKeyId::User)?;
let key = ctx.dangerous_get_key(SymmetricKeyId::User)?;

Ok(AsymmetricEncString::encrypt_rsa2048_oaep_sha1(
&key.to_vec(),
Expand Down Expand Up @@ -258,7 +258,7 @@ mod tests {
let key_store = existing_device.internal.get_key_store();
let ctx = key_store.context();
#[allow(deprecated)]
ctx.dangerous_get_symmetric_key(SymmetricKeyId::User)
ctx.dangerous_get_key(SymmetricKeyId::User)
.unwrap()
.to_base64()
};
Expand All @@ -267,7 +267,7 @@ mod tests {
let key_store = new_device.internal.get_key_store();
let ctx = key_store.context();
#[allow(deprecated)]
ctx.dangerous_get_symmetric_key(SymmetricKeyId::User)
ctx.dangerous_get_key(SymmetricKeyId::User)
.unwrap()
.to_base64()
};
Expand Down
2 changes: 1 addition & 1 deletion crates/bitwarden-core/src/auth/password/validate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ pub(crate) fn validate_password_user_key(
let ctx = key_store.context();
// FIXME: [PM-18099] Once MasterKey deals with KeyIds, this should be updated
#[allow(deprecated)]
let existing_key = ctx.dangerous_get_symmetric_key(SymmetricKeyId::User)?;
let existing_key = ctx.dangerous_get_key(SymmetricKeyId::User)?;

if user_key.to_vec() != existing_key.to_vec() {
return Err(AuthValidateError::WrongUserKey);
Expand Down
2 changes: 1 addition & 1 deletion crates/bitwarden-core/src/auth/pin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub(crate) fn validate_pin(
let ctx = key_store.context();
// FIXME: [PM-18099] Once PinKey deals with KeyIds, this should be updated
#[allow(deprecated)]
let user_key = ctx.dangerous_get_symmetric_key(SymmetricKeyId::User)?;
let user_key = ctx.dangerous_get_key(SymmetricKeyId::User)?;

let pin_key = PinKey::derive(pin.as_bytes(), email.as_bytes(), kdf)?;

Expand Down
2 changes: 1 addition & 1 deletion crates/bitwarden-core/src/auth/renew.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
let key_store = client.get_key_store();
let ctx = key_store.context();
#[allow(deprecated)]
if let Ok(enc_key) = ctx.dangerous_get_symmetric_key(SymmetricKeyId::User) {
if let Ok(enc_key) = ctx.dangerous_get_key(SymmetricKeyId::User) {

Check warning on line 80 in crates/bitwarden-core/src/auth/renew.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-core/src/auth/renew.rs#L80 

Added line #L80 was not covered by tests
let state =
ClientState::new(r.access_token.clone(), enc_key.to_base64());
_ = state::set(state_file, access_token, state);
Expand Down
10 changes: 5 additions & 5 deletions crates/bitwarden-core/src/client/encryption_settings.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,9 @@
#[allow(deprecated)]
{
let mut ctx = store.context_mut();
ctx.set_symmetric_key(SymmetricKeyId::User, user_key)?;
ctx.set_key(SymmetricKeyId::User, user_key)?;
if let Some(private_key) = private_key {
ctx.set_asymmetric_key(AsymmetricKeyId::UserPrivateKey, private_key)?;
ctx.set_key(AsymmetricKeyId::UserPrivateKey, private_key)?;
}
}

Expand All @@ -87,7 +87,7 @@
#[allow(deprecated)]
store
.context_mut()
.set_symmetric_key(SymmetricKeyId::User, key)
.set_key(SymmetricKeyId::User, key)

Check warning on line 90 in crates/bitwarden-core/src/client/encryption_settings.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-core/src/client/encryption_settings.rs#L90 

Added line #L90 was not covered by tests
.expect("Mutable context");
}

Expand All @@ -103,7 +103,7 @@
return Ok(());
}

if !ctx.has_asymmetric_key(AsymmetricKeyId::UserPrivateKey) {
if !ctx.has_key(AsymmetricKeyId::UserPrivateKey) {
return Err(EncryptionSettingsError::MissingPrivateKey);
}

Expand All @@ -113,7 +113,7 @@

// Decrypt the org keys with the private key
for (org_id, org_enc_key) in org_enc_keys {
ctx.decrypt_symmetric_key_with_asymmetric_key(
ctx.decrypt_key_into_store(
AsymmetricKeyId::UserPrivateKey,
SymmetricKeyId::Organization(org_id),
&org_enc_key,
Expand Down
6 changes: 3 additions & 3 deletions crates/bitwarden-core/src/key_management/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ pub fn create_test_crypto_with_user_key(key: SymmetricCryptoKey) -> KeyStore<Key
#[allow(deprecated)]
store
.context_mut()
.set_symmetric_key(SymmetricKeyId::User, key.clone())
.set_key(SymmetricKeyId::User, key.clone())
.expect("Mutable context");

store
Expand All @@ -58,13 +58,13 @@ pub fn create_test_crypto_with_user_and_org_key(
#[allow(deprecated)]
store
.context_mut()
.set_symmetric_key(SymmetricKeyId::User, key.clone())
.set_key(SymmetricKeyId::User, key.clone())
.expect("Mutable context");

#[allow(deprecated)]
store
.context_mut()
.set_symmetric_key(SymmetricKeyId::Organization(org_id), org_key.clone())
.set_key(SymmetricKeyId::Organization(org_id), org_key.clone())
.expect("Mutable context");

store
Expand Down
26 changes: 12 additions & 14 deletions crates/bitwarden-core/src/mobile/crypto.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@
let ctx = key_store.context();
// This is needed because the mobile clients need access to the user encryption key
#[allow(deprecated)]
let user_key = ctx.dangerous_get_symmetric_key(SymmetricKeyId::User)?;
let user_key = ctx.dangerous_get_key(SymmetricKeyId::User)?;

Check warning on line 226 in crates/bitwarden-core/src/mobile/crypto.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-core/src/mobile/crypto.rs#L226 

Added line #L226 was not covered by tests

Ok(user_key.to_base64())
}
Expand All @@ -246,7 +246,7 @@
let ctx = key_store.context();
// FIXME: [PM-18099] Once MasterKey deals with KeyIds, this should be updated
#[allow(deprecated)]
let user_key = ctx.dangerous_get_symmetric_key(SymmetricKeyId::User)?;
let user_key = ctx.dangerous_get_key(SymmetricKeyId::User)?;

let login_method = client
.internal
Expand Down Expand Up @@ -294,7 +294,7 @@
let ctx = key_store.context();
// FIXME: [PM-18099] Once PinKey deals with KeyIds, this should be updated
#[allow(deprecated)]
let user_key = ctx.dangerous_get_symmetric_key(SymmetricKeyId::User)?;
let user_key = ctx.dangerous_get_key(SymmetricKeyId::User)?;

let login_method = client
.internal
Expand All @@ -317,7 +317,7 @@
let ctx = key_store.context();
// FIXME: [PM-18099] Once PinKey deals with KeyIds, this should be updated
#[allow(deprecated)]
let user_key = ctx.dangerous_get_symmetric_key(SymmetricKeyId::User)?;
let user_key = ctx.dangerous_get_key(SymmetricKeyId::User)?;

let pin: String = encrypted_pin.decrypt_with_key(user_key)?;
let login_method = client
Expand Down Expand Up @@ -370,7 +370,7 @@
let ctx = key_store.context();
// FIXME: [PM-18110] This should be removed once the key store can handle public key encryption
#[allow(deprecated)]
let key = ctx.dangerous_get_symmetric_key(SymmetricKeyId::User)?;
let key = ctx.dangerous_get_key(SymmetricKeyId::User)?;

Ok(AsymmetricEncString::encrypt_rsa2048_oaep_sha1(
&key.to_vec(),
Expand Down Expand Up @@ -581,7 +581,7 @@
let key_store = client.internal.get_key_store();
let ctx = key_store.context();
#[allow(deprecated)]
ctx.dangerous_get_symmetric_key(SymmetricKeyId::User)
ctx.dangerous_get_key(SymmetricKeyId::User)
.unwrap()
.to_base64()
};
Expand All @@ -590,7 +590,7 @@
let key_store = client2.internal.get_key_store();
let ctx = key_store.context();
#[allow(deprecated)]
ctx.dangerous_get_symmetric_key(SymmetricKeyId::User)
ctx.dangerous_get_key(SymmetricKeyId::User)
.unwrap()
.to_base64()
};
Expand Down Expand Up @@ -646,7 +646,7 @@
let key_store = client.internal.get_key_store();
let ctx = key_store.context();
#[allow(deprecated)]
ctx.dangerous_get_symmetric_key(SymmetricKeyId::User)
ctx.dangerous_get_key(SymmetricKeyId::User)
.unwrap()
.to_base64()
};
Expand All @@ -655,7 +655,7 @@
let key_store = client2.internal.get_key_store();
let ctx = key_store.context();
#[allow(deprecated)]
ctx.dangerous_get_symmetric_key(SymmetricKeyId::User)
ctx.dangerous_get_key(SymmetricKeyId::User)
.unwrap()
.to_base64()
};
Expand Down Expand Up @@ -688,7 +688,7 @@
let key_store = client.internal.get_key_store();
let ctx = key_store.context();
#[allow(deprecated)]
ctx.dangerous_get_symmetric_key(SymmetricKeyId::User)
ctx.dangerous_get_key(SymmetricKeyId::User)
.unwrap()
.to_base64()
};
Expand All @@ -697,7 +697,7 @@
let key_store = client3.internal.get_key_store();
let ctx = key_store.context();
#[allow(deprecated)]
ctx.dangerous_get_symmetric_key(SymmetricKeyId::User)
ctx.dangerous_get_key(SymmetricKeyId::User)
.unwrap()
.to_base64()
};
Expand Down Expand Up @@ -740,9 +740,7 @@
let key_store = client.internal.get_key_store();
let ctx = key_store.context();
#[allow(deprecated)]
let expected = ctx
.dangerous_get_symmetric_key(SymmetricKeyId::User)
.unwrap();
let expected = ctx.dangerous_get_key(SymmetricKeyId::User).unwrap();

assert_eq!(&decrypted, &expected.to_vec());
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bitwarden-core/src/platform/generate_fingerprint.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ pub(crate) fn generate_user_fingerprint(
// FIXME: [PM-18110] This should be removed once the key store can handle public keys and
// fingerprints
#[allow(deprecated)]
let private_key = ctx.dangerous_get_asymmetric_key(AsymmetricKeyId::UserPrivateKey)?;
let private_key = ctx.dangerous_get_key(AsymmetricKeyId::UserPrivateKey)?;

let public_key = private_key.to_public_der()?;
let fingerprint = fingerprint(&fingerprint_material, &public_key)?;
Expand Down
Loading