From 1ccf11b664a6d5210a4a140438ccb5b3597e05c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa?= Date: Tue, 9 Apr 2024 14:28:19 +0200 Subject: [PATCH] [PM-6764] Move cipher to organization (#695) ## Type of change ``` - [ ] Bug fix - [x] New feature development - [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc) - [ ] Build/deploy pipeline (DevOps) - [ ] Other ``` ## Objective Added function to allow moving a cipher to an organization, which requires reencrypting the cipher key if it has one. --- Cargo.lock | 1 + crates/bitwarden-uniffi/Cargo.toml | 1 + crates/bitwarden-uniffi/src/uniffi_support.rs | 2 + crates/bitwarden-uniffi/src/vault/ciphers.rs | 18 ++ .../src/mobile/vault/client_ciphers.rs | 11 ++ crates/bitwarden/src/vault/cipher/cipher.rs | 157 +++++++++++++----- languages/kotlin/doc.md | 26 ++- 7 files changed, 177 insertions(+), 39 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cc24ecfc1..47e2b9e9d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -515,6 +515,7 @@ dependencies = [ "env_logger", "schemars", "uniffi", + "uuid", ] [[package]] diff --git a/crates/bitwarden-uniffi/Cargo.toml b/crates/bitwarden-uniffi/Cargo.toml index 8dfddb820..90a106bce 100644 --- a/crates/bitwarden-uniffi/Cargo.toml +++ b/crates/bitwarden-uniffi/Cargo.toml @@ -29,6 +29,7 @@ chrono = { version = ">=0.4.26, <0.5", features = [ env_logger = "0.11.1" schemars = { version = ">=0.8, <0.9", optional = true } uniffi = "=0.26.1" +uuid = ">=1.3.3, <2" [build-dependencies] uniffi = { version = "=0.26.1", features = ["build"] } diff --git a/crates/bitwarden-uniffi/src/uniffi_support.rs b/crates/bitwarden-uniffi/src/uniffi_support.rs index a49e347b6..f487dfaa0 100644 --- a/crates/bitwarden-uniffi/src/uniffi_support.rs +++ b/crates/bitwarden-uniffi/src/uniffi_support.rs @@ -1,4 +1,5 @@ use bitwarden_crypto::{AsymmetricEncString, EncString, SensitiveString}; +use uuid::Uuid; // Forward the type definitions to the main bitwarden crate type DateTime = chrono::DateTime; @@ -6,3 +7,4 @@ uniffi::ffi_converter_forward!(DateTime, bitwarden::UniFfiTag, crate::UniFfiTag) uniffi::ffi_converter_forward!(EncString, bitwarden::UniFfiTag, crate::UniFfiTag); uniffi::ffi_converter_forward!(AsymmetricEncString, bitwarden::UniFfiTag, crate::UniFfiTag); uniffi::ffi_converter_forward!(SensitiveString, bitwarden::UniFfiTag, crate::UniFfiTag); +uniffi::ffi_converter_forward!(Uuid, bitwarden::UniFfiTag, crate::UniFfiTag); diff --git a/crates/bitwarden-uniffi/src/vault/ciphers.rs b/crates/bitwarden-uniffi/src/vault/ciphers.rs index eb8543947..dcf224a38 100644 --- a/crates/bitwarden-uniffi/src/vault/ciphers.rs +++ b/crates/bitwarden-uniffi/src/vault/ciphers.rs @@ -1,6 +1,7 @@ use std::sync::Arc; use bitwarden::vault::{Cipher, CipherListView, CipherView}; +use uuid::Uuid; use crate::{Client, Result}; @@ -47,4 +48,21 @@ impl ClientCiphers { .decrypt_list(ciphers) .await?) } + + /// Move a cipher to an organization, reencrypting the cipher key if necessary + pub async fn move_to_organization( + &self, + cipher: CipherView, + organization_id: Uuid, + ) -> Result { + Ok(self + .0 + .0 + .read() + .await + .vault() + .ciphers() + .move_to_organization(cipher, organization_id) + .await?) + } } diff --git a/crates/bitwarden/src/mobile/vault/client_ciphers.rs b/crates/bitwarden/src/mobile/vault/client_ciphers.rs index c35cf3080..71203374f 100644 --- a/crates/bitwarden/src/mobile/vault/client_ciphers.rs +++ b/crates/bitwarden/src/mobile/vault/client_ciphers.rs @@ -1,4 +1,5 @@ use bitwarden_crypto::{Decryptable, Encryptable, LocateKey}; +use uuid::Uuid; use super::client_vault::ClientVault; use crate::{ @@ -44,6 +45,16 @@ impl<'a> ClientCiphers<'a> { Ok(cipher_views) } + + pub async fn move_to_organization( + &self, + mut cipher_view: CipherView, + organization_id: Uuid, + ) -> Result { + let enc = self.client.get_encryption_settings()?; + cipher_view.move_to_organization(enc, organization_id)?; + Ok(cipher_view) + } } impl<'a> ClientVault<'a> { diff --git a/crates/bitwarden/src/vault/cipher/cipher.rs b/crates/bitwarden/src/vault/cipher/cipher.rs index fa85b5969..f245fdc3a 100644 --- a/crates/bitwarden/src/vault/cipher/cipher.rs +++ b/crates/bitwarden/src/vault/cipher/cipher.rs @@ -1,6 +1,6 @@ use bitwarden_api_api::models::CipherDetailsResponseModel; use bitwarden_crypto::{ - CryptoError, EncString, KeyContainer, KeyDecryptable, KeyEncryptable, LocateKey, + CryptoError, EncString, KeyContainer, KeyDecryptable, KeyEncryptable, LocateKey, SensitiveVec, SymmetricCryptoKey, }; use chrono::{DateTime, Utc}; @@ -325,6 +325,29 @@ impl CipherView { uris.retain(|u| u.is_checksum_valid()); } } + + pub fn move_to_organization( + &mut self, + enc: &dyn KeyContainer, + organization_id: Uuid, + ) -> Result<()> { + // If the cipher has a key, we need to re-encrypt it with the new organization key + if let Some(cipher_key) = &mut self.key { + let old_key = enc + .get_key(&self.organization_id) + .ok_or(Error::VaultLocked)?; + + let new_key = enc + .get_key(&Some(organization_id)) + .ok_or(Error::VaultLocked)?; + + let dec_cipher_key = SensitiveVec::new(Box::new(cipher_key.decrypt_with_key(old_key)?)); + *cipher_key = dec_cipher_key.expose().encrypt_with_key(new_key)?; + } + + self.organization_id = Some(organization_id); + Ok(()) + } } impl KeyDecryptable for Cipher { @@ -443,49 +466,51 @@ impl From for CipherRepromptType #[cfg(test)] mod tests { + use std::collections::HashMap; + use super::*; + fn generate_cipher() -> CipherView { + CipherView { + r#type: CipherType::Login, + login: Some(login::LoginView { + username: Some("test_username".to_string()), + password: Some("test_password".to_string()), + password_revision_date: None, + uris: None, + totp: None, + autofill_on_page_load: None, + fido2_credentials: None, + }), + id: "fd411a1a-fec8-4070-985d-0e6560860e69".parse().ok(), + organization_id: None, + folder_id: None, + collection_ids: vec![], + key: None, + name: "My test login".to_string(), + notes: None, + identity: None, + card: None, + secure_note: None, + favorite: false, + reprompt: CipherRepromptType::None, + organization_use_totp: true, + edit: true, + view_password: true, + local_data: None, + attachments: None, + fields: None, + password_history: None, + creation_date: "2024-01-30T17:55:36.150Z".parse().unwrap(), + deleted_date: None, + revision_date: "2024-01-30T17:55:36.150Z".parse().unwrap(), + } + } + #[test] fn test_generate_cipher_key() { let key = SymmetricCryptoKey::generate(rand::thread_rng()); - fn generate_cipher() -> CipherView { - CipherView { - r#type: CipherType::Login, - login: Some(login::LoginView { - username: Some("test_username".to_string()), - password: Some("test_password".to_string()), - password_revision_date: None, - uris: None, - totp: None, - autofill_on_page_load: None, - fido2_credentials: None, - }), - id: "fd411a1a-fec8-4070-985d-0e6560860e69".parse().ok(), - organization_id: None, - folder_id: None, - collection_ids: vec![], - key: None, - name: "My test login".to_string(), - notes: None, - identity: None, - card: None, - secure_note: None, - favorite: false, - reprompt: CipherRepromptType::None, - organization_use_totp: true, - edit: true, - view_password: true, - local_data: None, - attachments: None, - fields: None, - password_history: None, - creation_date: "2024-01-30T17:55:36.150Z".parse().unwrap(), - deleted_date: None, - revision_date: "2024-01-30T17:55:36.150Z".parse().unwrap(), - } - } - let original_cipher = generate_cipher(); // Check that the cipher gets encrypted correctly without it's own key @@ -504,4 +529,60 @@ mod tests { assert!(key_cipher_dec.key.is_some()); assert_eq!(key_cipher_dec.name, original_cipher.name); } + + struct MockKeyContainer(HashMap, SymmetricCryptoKey>); + impl KeyContainer for MockKeyContainer { + fn get_key<'a>(&'a self, org_id: &Option) -> Option<&'a SymmetricCryptoKey> { + self.0.get(org_id) + } + } + + #[test] + fn test_move_user_cipher_to_org() { + let org = uuid::Uuid::new_v4(); + + let enc = MockKeyContainer(HashMap::from([ + (None, SymmetricCryptoKey::generate(rand::thread_rng())), + (Some(org), SymmetricCryptoKey::generate(rand::thread_rng())), + ])); + + // Create a cipher with a user key + let mut cipher = generate_cipher(); + cipher + .generate_cipher_key(enc.get_key(&None).unwrap()) + .unwrap(); + + cipher.move_to_organization(&enc, org).unwrap(); + assert_eq!(cipher.organization_id, Some(org)); + + // Check that the cipher can be encrypted/decrypted with the new org key + let org_key = enc.get_key(&Some(org)).unwrap(); + let cipher_enc = cipher.encrypt_with_key(org_key).unwrap(); + let cipher_dec: CipherView = cipher_enc.decrypt_with_key(org_key).unwrap(); + + assert_eq!(cipher_dec.name, "My test login"); + } + + #[test] + fn test_move_user_cipher_to_org_manually() { + let org = uuid::Uuid::new_v4(); + + let enc = MockKeyContainer(HashMap::from([ + (None, SymmetricCryptoKey::generate(rand::thread_rng())), + (Some(org), SymmetricCryptoKey::generate(rand::thread_rng())), + ])); + + // Create a cipher with a user key + let mut cipher = generate_cipher(); + cipher + .generate_cipher_key(enc.get_key(&None).unwrap()) + .unwrap(); + + cipher.organization_id = Some(org); + + // Check that the cipher can not be encrypted, as the + // cipher key is tied to the user key and not the org key + let org_key = enc.get_key(&Some(org)).unwrap(); + assert!(cipher.encrypt_with_key(org_key).is_err()); + } } diff --git a/languages/kotlin/doc.md b/languages/kotlin/doc.md index 5dce0dbb4..e12914f64 100644 --- a/languages/kotlin/doc.md +++ b/languages/kotlin/doc.md @@ -132,6 +132,18 @@ Generate keys needed for registration process **Output**: std::result::Result +### `make_register_tde_keys` + +Generate keys needed for TDE process + +**Arguments**: + +- self: +- org_public_key: String +- remember_device: + +**Output**: std::result::Result + ### `validate_password` Validate the user password @@ -288,6 +300,18 @@ Decrypt cipher list **Output**: std::result::Result +### `move_to_organization` + +Move a cipher to an organization, reencrypting the cipher key if necessary + +**Arguments**: + +- self: +- cipher: [CipherView](#cipherview) +- organization_id: Uuid + +**Output**: std::result::Result + ## ClientCollections ### `decrypt` @@ -346,7 +370,7 @@ as it can be used to decrypt all of the user's data - self: -**Output**: std::result::Result +**Output**: std::result::Result ### `update_password`