Skip to content

Commit a3eb918

Browse files
authored
[PM-23085] Add encrypt_with_key method to cipher_client (#350)
## 🎟️ Tracking [PM-23085](https://bitwarden.atlassian.net/browse/PM-23085) ## 📔 Objective Implement `encrypt_with_key` for the `cipher_client` so that the TS clients can re-encrypt a cipher with a new user key during key rotation. Once key rotation is implemented fully in the SDK ([PM-23084](https://bitwarden.atlassian.net/browse/PM-23084)) this method can either be removed or updated to accept a symmetric key identifier instead of the raw base64 encoded key. ## ⏰ Reminders before review - Contributor guidelines followed - All formatters and local linters executed and passed - Written new unit and / or integration tests where applicable - Protected functional changes with optionality (feature flags) - Used internationalization (i18n) for all UI strings - CI builds passed - Communicated to DevOps any deployment requirements - Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team ## 🦮 Reviewer guidelines <!-- Suggested interactions but feel free to use (or not) as you desire! --> - 👍 (`:+1:`) or similar for great changes - 📝 (`:memo:`) or ℹ️ (`:information_source:`) for notes or general info - ❓ (`:question:`) for questions - 🤔 (`:thinking:`) or 💭 (`:thought_balloon:`) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion - 🎨 (`:art:`) for suggestions / improvements - ❌ (`:x:`) or ⚠️ (`:warning:`) for more significant problems or concerns needing attention - 🌱 (`:seedling:`) or ♻️ (`:recycle:`) for future improvements or indications of technical debt - ⛏ (`:pick:`) for minor or nitpick changes [PM-23085]: https://bitwarden.atlassian.net/browse/PM-23085?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [PM-23084]: https://bitwarden.atlassian.net/browse/PM-23084?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
1 parent e6b15dc commit a3eb918

File tree

2 files changed

+198
-15
lines changed

2 files changed

+198
-15
lines changed

crates/bitwarden-vault/src/cipher/cipher.rs

Lines changed: 79 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ use super::{
2828
secure_note, ssh_key,
2929
};
3030
use crate::{
31-
password_history, Fido2CredentialFullView, Fido2CredentialView, Login, LoginView,
31+
password_history, EncryptError, Fido2CredentialFullView, Fido2CredentialView, Login, LoginView,
3232
VaultParseError,
3333
};
3434

@@ -42,6 +42,8 @@ pub enum CipherError {
4242
VaultLocked(#[from] VaultLockedError),
4343
#[error(transparent)]
4444
CryptoError(#[from] CryptoError),
45+
#[error(transparent)]
46+
EncryptError(#[from] EncryptError),
4547
#[error("This cipher contains attachments without keys. Those attachments will need to be reuploaded to complete the operation")]
4648
AttachmentsWithoutKeys,
4749
}
@@ -526,32 +528,55 @@ impl CipherView {
526528
Ok(())
527529
}
528530

529-
#[allow(missing_docs)]
531+
/// Moves the cipher to an organization by re-encrypting the cipher keys with the organization
532+
/// key and assigning the organization ID to the cipher.
533+
///
534+
/// # Arguments
535+
/// * `ctx` - The key store context where the cipher keys will be re-encrypted
536+
/// * `organization_id` - The ID of the organization to move the cipher to
530537
pub fn move_to_organization(
531538
&mut self,
532539
ctx: &mut KeyStoreContext<KeyIds>,
533540
organization_id: Uuid,
534541
) -> Result<(), CipherError> {
535-
let old_key = self.key_identifier();
536542
let new_key = SymmetricKeyId::Organization(organization_id);
537543

544+
self.reencrypt_cipher_keys(ctx, new_key)?;
545+
self.organization_id = Some(organization_id);
546+
547+
Ok(())
548+
}
549+
550+
/// Re-encrypt the cipher key(s) using a new wrapping key.
551+
///
552+
/// If the cipher has a cipher key, it will be re-encrypted with the new wrapping key.
553+
/// Otherwise, the cipher will re-encrypt all attachment keys and FIDO2 credential keys
554+
pub fn reencrypt_cipher_keys(
555+
&mut self,
556+
ctx: &mut KeyStoreContext<KeyIds>,
557+
new_wrapping_key: SymmetricKeyId,
558+
) -> Result<(), CipherError> {
559+
let old_key = self.key_identifier();
560+
538561
// If any attachment is missing a key we can't reencrypt the attachment keys
539562
if self.attachments.iter().flatten().any(|a| a.key.is_none()) {
540563
return Err(CipherError::AttachmentsWithoutKeys);
541564
}
542565

543-
// If the cipher has a key, we need to re-encrypt it with the new organization key
544-
if let Some(cipher_key) = &mut self.key {
545-
let tmp_cipher_key_id = SymmetricKeyId::Local("cipher_key");
546-
ctx.unwrap_symmetric_key(old_key, tmp_cipher_key_id, cipher_key)?;
547-
*cipher_key = ctx.wrap_symmetric_key(new_key, tmp_cipher_key_id)?;
566+
// If the cipher has a key, reencrypt it with the new wrapping key
567+
if self.key.is_some() {
568+
// Decrypt the current cipher key using the existing wrapping key
569+
let cipher_key = Cipher::decrypt_cipher_key(ctx, old_key, &self.key)?;
570+
571+
// Wrap the cipher key with the new wrapping key
572+
self.key = Some(ctx.wrap_symmetric_key(new_wrapping_key, cipher_key)?);
548573
} else {
549-
// If the cipher does not have a key, we need to reencrypt all attachment keys
550-
self.reencrypt_attachment_keys(ctx, old_key, new_key)?;
551-
self.reencrypt_fido2_credentials(ctx, old_key, new_key)?;
574+
// The cipher does not have a key, we must reencrypt all attachment keys and FIDO2
575+
// credentials individually
576+
self.reencrypt_attachment_keys(ctx, old_key, new_wrapping_key)?;
577+
self.reencrypt_fido2_credentials(ctx, old_key, new_wrapping_key)?;
552578
}
553579

554-
self.organization_id = Some(organization_id);
555580
Ok(())
556581
}
557582

@@ -999,6 +1024,48 @@ mod tests {
9991024
assert!(cipher.attachments.unwrap()[0].key.is_none());
10001025
}
10011026

1027+
#[test]
1028+
fn test_reencrypt_cipher_key() {
1029+
let old_key = SymmetricCryptoKey::make_aes256_cbc_hmac_key();
1030+
let new_key = SymmetricCryptoKey::make_aes256_cbc_hmac_key();
1031+
let key_store = create_test_crypto_with_user_key(old_key);
1032+
let mut ctx = key_store.context_mut();
1033+
1034+
let mut cipher = generate_cipher();
1035+
cipher
1036+
.generate_cipher_key(&mut ctx, cipher.key_identifier())
1037+
.unwrap();
1038+
1039+
// Re-encrypt the cipher key with a new wrapping key
1040+
let new_key_id: SymmetricKeyId = SymmetricKeyId::Local("new_cipher_key");
1041+
#[allow(deprecated)]
1042+
ctx.set_symmetric_key(new_key_id, new_key).unwrap();
1043+
1044+
cipher.reencrypt_cipher_keys(&mut ctx, new_key_id).unwrap();
1045+
1046+
// Check that the cipher key can be unwrapped with the new key
1047+
assert!(cipher.key.is_some());
1048+
assert!(ctx
1049+
.unwrap_symmetric_key(new_key_id, new_key_id, &cipher.key.unwrap())
1050+
.is_ok());
1051+
}
1052+
1053+
#[test]
1054+
fn test_reencrypt_cipher_key_ignores_missing_key() {
1055+
let key = SymmetricCryptoKey::make_aes256_cbc_hmac_key();
1056+
let key_store = create_test_crypto_with_user_key(key);
1057+
let mut ctx = key_store.context_mut();
1058+
let mut cipher = generate_cipher();
1059+
1060+
// The cipher does not have a key, so re-encryption should not add one
1061+
cipher
1062+
.reencrypt_cipher_keys(&mut ctx, SymmetricKeyId::Local("new_cipher_key"))
1063+
.unwrap();
1064+
1065+
// Check that the cipher key is still None
1066+
assert!(cipher.key.is_none());
1067+
}
1068+
10021069
#[test]
10031070
fn test_move_user_cipher_to_org() {
10041071
let org = uuid::Uuid::new_v4();

crates/bitwarden-vault/src/cipher/cipher_client.rs

Lines changed: 119 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
use bitwarden_core::{Client, OrganizationId};
2-
use bitwarden_crypto::IdentifyKey;
1+
use bitwarden_core::{key_management::SymmetricKeyId, Client, OrganizationId};
2+
use bitwarden_crypto::{CompositeEncryptable, IdentifyKey, SymmetricCryptoKey};
33
#[cfg(feature = "wasm")]
44
use wasm_bindgen::prelude::*;
55

@@ -46,6 +46,57 @@ impl CiphersClient {
4646
})
4747
}
4848

49+
/// Encrypt a cipher with the provided key. This should only be used when rotating encryption
50+
/// keys in the Web client.
51+
///
52+
/// Until key rotation is fully implemented in the SDK, this method must be provided the new
53+
/// symmetric key in base64 format. See PM-23084
54+
///
55+
/// If the cipher has a CipherKey, it will be re-encrypted with the new key.
56+
/// If the cipher does not have a CipherKey and CipherKeyEncryption is enabled, one will be
57+
/// generated using the new key. Otherwise, the cipher's data will be encrypted with the new
58+
/// key directly.
59+
#[cfg(feature = "wasm")]
60+
pub fn encrypt_cipher_for_rotation(
61+
&self,
62+
mut cipher_view: CipherView,
63+
new_key_b64: String,
64+
) -> Result<EncryptionContext, CipherError> {
65+
let new_key = SymmetricCryptoKey::try_from(new_key_b64)?;
66+
67+
let user_id = self
68+
.client
69+
.internal
70+
.get_user_id()
71+
.ok_or(EncryptError::MissingUserId)?;
72+
let key_store = self.client.internal.get_key_store();
73+
let mut ctx = key_store.context();
74+
75+
// Set the new key in the key store context
76+
const NEW_KEY_ID: SymmetricKeyId = SymmetricKeyId::Local("new_cipher_key");
77+
#[allow(deprecated)]
78+
ctx.set_symmetric_key(NEW_KEY_ID, new_key)?;
79+
80+
if cipher_view.key.is_none()
81+
&& self
82+
.client
83+
.internal
84+
.get_flags()
85+
.enable_cipher_key_encryption
86+
{
87+
cipher_view.generate_cipher_key(&mut ctx, NEW_KEY_ID)?;
88+
} else {
89+
cipher_view.reencrypt_cipher_keys(&mut ctx, NEW_KEY_ID)?;
90+
}
91+
92+
let cipher = cipher_view.encrypt_composite(&mut ctx, NEW_KEY_ID)?;
93+
94+
Ok(EncryptionContext {
95+
cipher,
96+
encrypted_for: user_id,
97+
})
98+
}
99+
49100
#[allow(missing_docs)]
50101
pub fn decrypt(&self, cipher: Cipher) -> Result<CipherView, DecryptError> {
51102
let key_store = self.client.internal.get_key_store();
@@ -127,9 +178,10 @@ impl CiphersClient {
127178
mod tests {
128179

129180
use bitwarden_core::client::test_accounts::test_bitwarden_com_account;
181+
use bitwarden_crypto::CryptoError;
130182

131183
use super::*;
132-
use crate::{Attachment, CipherRepromptType, CipherType, Login, VaultClientExt};
184+
use crate::{Attachment, CipherRepromptType, CipherType, Login, LoginView, VaultClientExt};
133185

134186
fn test_cipher() -> Cipher {
135187
Cipher {
@@ -170,6 +222,46 @@ mod tests {
170222
}
171223
}
172224

225+
fn test_cipher_view() -> CipherView {
226+
let test_id: uuid::Uuid = "fd411a1a-fec8-4070-985d-0e6560860e69".parse().unwrap();
227+
CipherView {
228+
r#type: CipherType::Login,
229+
login: Some(LoginView {
230+
username: Some("test_username".to_string()),
231+
password: Some("test_password".to_string()),
232+
password_revision_date: None,
233+
uris: None,
234+
totp: None,
235+
autofill_on_page_load: None,
236+
fido2_credentials: None,
237+
}),
238+
id: Some(test_id),
239+
organization_id: None,
240+
folder_id: None,
241+
collection_ids: vec![],
242+
key: None,
243+
name: "My test login".to_string(),
244+
notes: None,
245+
identity: None,
246+
card: None,
247+
secure_note: None,
248+
ssh_key: None,
249+
favorite: false,
250+
reprompt: CipherRepromptType::None,
251+
organization_use_totp: true,
252+
edit: true,
253+
permissions: None,
254+
view_password: true,
255+
local_data: None,
256+
attachments: None,
257+
fields: None,
258+
password_history: None,
259+
creation_date: "2024-01-30T17:55:36.150Z".parse().unwrap(),
260+
deleted_date: None,
261+
revision_date: "2024-01-30T17:55:36.150Z".parse().unwrap(),
262+
}
263+
}
264+
173265
fn test_attachment_legacy() -> Attachment {
174266
Attachment {
175267
id: Some("uf7bkexzag04d3cw04jsbqqkbpbwhxs0".to_string()),
@@ -417,4 +509,28 @@ mod tests {
417509

418510
assert_eq!(content, b"Hello");
419511
}
512+
513+
#[tokio::test]
514+
async fn test_encrypt_cipher_for_rotation() {
515+
let client = Client::init_test_account(test_bitwarden_com_account()).await;
516+
517+
let new_key = SymmetricCryptoKey::make_aes256_cbc_hmac_key();
518+
519+
let cipher_view = test_cipher_view();
520+
let new_key_b64 = new_key.to_base64();
521+
522+
let ctx = client
523+
.vault()
524+
.ciphers()
525+
.encrypt_cipher_for_rotation(cipher_view, new_key_b64)
526+
.unwrap();
527+
528+
assert!(ctx.cipher.key.is_some());
529+
530+
// Decrypting the cipher "normally" will fail because it was encrypted with a new key
531+
assert!(matches!(
532+
client.vault().ciphers().decrypt(ctx.cipher).err(),
533+
Some(DecryptError::Crypto(CryptoError::InvalidMac))
534+
));
535+
}
420536
}

0 commit comments

Comments
 (0)