Skip to content

Commit

Permalink
[PM-6262] Add basic feature flags support to enable cipher key encryp…
Browse files Browse the repository at this point in the history
…tion (#638)

## Type of change
```
- [ ] Bug fix
- [x] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other
```

## Objective
Implement support for some basic feature flagging for the mobile
clients, the only flag supported is `enableCipherKeyEncryption`
  • Loading branch information
dani-garcia authored Mar 1, 2024
1 parent 5d131f5 commit 29089c5
Show file tree
Hide file tree
Showing 7 changed files with 173 additions and 6 deletions.
6 changes: 6 additions & 0 deletions crates/bitwarden-uniffi/src/platform/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,10 @@ impl ClientPlatform {
.platform()
.user_fingerprint(fingerprint_material)?)
}

/// Load feature flags into the client
pub async fn load_flags(&self, flags: std::collections::HashMap<String, bool>) -> Result<()> {
self.0 .0.write().await.load_flags(flags);
Ok(())
}
}
24 changes: 21 additions & 3 deletions crates/bitwarden/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,12 @@ use super::AccessToken;
#[cfg(feature = "secrets")]
use crate::auth::login::{AccessTokenLoginRequest, AccessTokenLoginResponse};
#[cfg(feature = "internal")]
use crate::platform::{
get_user_api_key, sync, SecretVerificationRequest, SyncRequest, SyncResponse,
UserApiKeyResponse,
use crate::{
client::flags::Flags,
platform::{
get_user_api_key, sync, SecretVerificationRequest, SyncRequest, SyncResponse,
UserApiKeyResponse,
},
};
use crate::{
client::{
Expand Down Expand Up @@ -77,6 +80,9 @@ pub struct Client {
pub(crate) token_expires_on: Option<i64>,
pub(crate) login_method: Option<LoginMethod>,

#[cfg(feature = "internal")]
flags: Flags,

/// Use Client::get_api_configurations() to access this.
/// It should only be used directly in renew_token
#[doc(hidden)]
Expand Down Expand Up @@ -138,6 +144,8 @@ impl Client {
refresh_token: None,
token_expires_on: None,
login_method: None,
#[cfg(feature = "internal")]
flags: Flags::default(),
__api_configurations: ApiConfigurations {
identity,
api,
Expand All @@ -148,6 +156,16 @@ impl Client {
}
}

#[cfg(feature = "internal")]
pub fn load_flags(&mut self, flags: std::collections::HashMap<String, bool>) {
self.flags = Flags::load_from_map(flags);
}

#[cfg(feature = "mobile")]
pub(crate) fn get_flags(&self) -> &Flags {
&self.flags
}

pub(crate) async fn get_api_configurations(&mut self) -> &ApiConfigurations {
// At the moment we ignore the error result from the token renewal, if it fails,
// the token will end up expiring and the next operation is going to fail anyway.
Expand Down
43 changes: 43 additions & 0 deletions crates/bitwarden/src/client/flags.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
#[derive(Debug, Default, Clone, serde::Deserialize)]
pub struct Flags {
#[serde(default, rename = "enableCipherKeyEncryption")]
pub enable_cipher_key_encryption: bool,
}

impl Flags {
pub fn load_from_map(map: std::collections::HashMap<String, bool>) -> Self {
let map = map
.into_iter()
.map(|(k, v)| (k, serde_json::Value::Bool(v)))
.collect();
serde_json::from_value(serde_json::Value::Object(map)).expect("Valid map")
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_load_empty_map() {
let map = std::collections::HashMap::new();
let flags = Flags::load_from_map(map);
assert!(!flags.enable_cipher_key_encryption);
}

#[test]
fn test_load_valid_map() {
let mut map = std::collections::HashMap::new();
map.insert("enableCipherKeyEncryption".into(), true);
let flags = Flags::load_from_map(map);
assert!(flags.enable_cipher_key_encryption);
}

#[test]
fn test_load_invalid_map() {
let mut map = std::collections::HashMap::new();
map.insert("thisIsNotAFlag".into(), true);
let flags = Flags::load_from_map(map);
assert!(!flags.enable_cipher_key_encryption);
}
}
3 changes: 3 additions & 0 deletions crates/bitwarden/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,8 @@ mod client;
pub mod client_settings;
pub(crate) mod encryption_settings;

#[cfg(feature = "internal")]
mod flags;

pub use access_token::AccessToken;
pub use client::Client;
15 changes: 12 additions & 3 deletions crates/bitwarden/src/mobile/vault/client_ciphers.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use bitwarden_crypto::{Decryptable, Encryptable};
use bitwarden_crypto::{Decryptable, Encryptable, LocateKey};

use super::client_vault::ClientVault;
use crate::{
error::Result,
error::{Error, Result},
vault::{Cipher, CipherListView, CipherView},
Client,
};
Expand All @@ -12,9 +12,18 @@ pub struct ClientCiphers<'a> {
}

impl<'a> ClientCiphers<'a> {
pub async fn encrypt(&self, cipher_view: CipherView) -> Result<Cipher> {
pub async fn encrypt(&self, mut cipher_view: CipherView) -> Result<Cipher> {
let enc = self.client.get_encryption_settings()?;

// TODO: Once this flag is removed, the key generation logic should
// be moved directly into the KeyEncryptable implementation
if cipher_view.key.is_none() && self.client.get_flags().enable_cipher_key_encryption {
let key = cipher_view
.locate_key(enc, &None)
.ok_or(Error::VaultLocked)?;
cipher_view.generate_cipher_key(key)?;
}

let cipher = cipher_view.encrypt(enc, &None)?;

Ok(cipher)
Expand Down
77 changes: 77 additions & 0 deletions crates/bitwarden/src/vault/cipher/cipher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,18 @@ impl Cipher {
}
}

impl CipherView {
pub fn generate_cipher_key(&mut self, key: &SymmetricCryptoKey) -> Result<()> {
let ciphers_key = Cipher::get_cipher_key(key, &self.key)?;
let key = ciphers_key.as_ref().unwrap_or(key);

let new_key = SymmetricCryptoKey::generate(rand::thread_rng());

self.key = Some(new_key.to_vec().encrypt_with_key(key)?);
Ok(())
}
}

impl KeyDecryptable<SymmetricCryptoKey, CipherListView> for Cipher {
fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result<CipherListView, CryptoError> {
let ciphers_key = Cipher::get_cipher_key(key, &self.key)?;
Expand Down Expand Up @@ -401,3 +413,68 @@ impl From<bitwarden_api_api::models::CipherRepromptType> for CipherRepromptType
}
}
}

#[cfg(test)]
mod tests {

use super::*;

#[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,
}),
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
let cipher = generate_cipher();
let no_key_cipher_enc = cipher.encrypt_with_key(&key).unwrap();
let no_key_cipher_dec: CipherView = no_key_cipher_enc.decrypt_with_key(&key).unwrap();
assert!(no_key_cipher_dec.key.is_none());
assert_eq!(no_key_cipher_dec.name, original_cipher.name);

let mut cipher = generate_cipher();
cipher.generate_cipher_key(&key).unwrap();

// Check that the cipher gets encrypted correctly when it's assigned it's own key
let key_cipher_enc = cipher.encrypt_with_key(&key).unwrap();
let key_cipher_dec: CipherView = key_cipher_enc.decrypt_with_key(&key).unwrap();
assert!(key_cipher_dec.key.is_some());
assert_eq!(key_cipher_dec.name, original_cipher.name);
}
}
11 changes: 11 additions & 0 deletions languages/kotlin/doc.md
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,17 @@ Fingerprint using logged in user&#x27;s public key

**Output**: std::result::Result<String,BitwardenError>

### `load_flags`

Load feature flags into the client

**Arguments**:

- self:
- flags: std::collections::HashMap<String,>

**Output**: std::result::Result<,BitwardenError>

## ClientSends

### `encrypt`
Expand Down

0 comments on commit 29089c5

Please sign in to comment.