Skip to content

Commit

Permalink
[PM-6760] Add support for URI checksums (#662)
Browse files Browse the repository at this point in the history
## Type of change
```
- [ ] Bug fix
- [x] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other
```

## Objective
Validate the URI checksums of ciphers. This is done in a similar way to
the `enable_cipher_key_encryption` flag check
  • Loading branch information
dani-garcia committed Mar 21, 2024
1 parent c66892b commit 67e743f
Show file tree
Hide file tree
Showing 2 changed files with 112 additions and 3 deletions.
32 changes: 29 additions & 3 deletions crates/bitwarden/src/vault/cipher/cipher.rs
Expand Up @@ -139,10 +139,15 @@ pub struct CipherListView {
}

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

// For compatibility reasons, we only create checksums for ciphers that have a key
if ciphers_key.is_some() {
self.generate_checksums();
}

Ok(Cipher {
id: self.id,
organization_id: self.organization_id,
Expand Down Expand Up @@ -177,7 +182,7 @@ impl KeyDecryptable<SymmetricCryptoKey, CipherView> for Cipher {
let ciphers_key = Cipher::get_cipher_key(key, &self.key)?;
let key = ciphers_key.as_ref().unwrap_or(key);

Ok(CipherView {
let mut cipher = CipherView {
id: self.id,
organization_id: self.organization_id,
folder_id: self.folder_id,
Expand All @@ -202,7 +207,14 @@ impl KeyDecryptable<SymmetricCryptoKey, CipherView> for Cipher {
creation_date: self.creation_date,
deleted_date: self.deleted_date,
revision_date: self.revision_date,
})
};

// For compatibility we only remove URLs with invalid checksums if the cipher has a key
if ciphers_key.is_some() {
cipher.remove_invalid_checksums();
}

Ok(cipher)
}
}

Expand Down Expand Up @@ -299,6 +311,20 @@ impl CipherView {
self.key = Some(new_key.to_vec().encrypt_with_key(key)?);
Ok(())
}

pub fn generate_checksums(&mut self) {
if let Some(uris) = self.login.as_mut().and_then(|l| l.uris.as_mut()) {
for uri in uris {
uri.generate_checksum();
}
}
}

pub fn remove_invalid_checksums(&mut self) {
if let Some(uris) = self.login.as_mut().and_then(|l| l.uris.as_mut()) {
uris.retain(|u| u.is_checksum_valid());
}
}
}

impl KeyDecryptable<SymmetricCryptoKey, CipherListView> for Cipher {
Expand Down
83 changes: 83 additions & 0 deletions crates/bitwarden/src/vault/cipher/login.rs
@@ -1,3 +1,4 @@
use base64::{engine::general_purpose::STANDARD, Engine};
use bitwarden_api_api::models::{CipherLoginModel, CipherLoginUriModel};
use bitwarden_crypto::{
CryptoError, EncString, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey,
Expand Down Expand Up @@ -28,6 +29,7 @@ pub enum UriMatchType {
pub struct LoginUri {
pub uri: Option<EncString>,
pub r#match: Option<UriMatchType>,
pub uri_checksum: Option<EncString>,
}

#[derive(Serialize, Deserialize, Debug, JsonSchema)]
Expand All @@ -36,6 +38,35 @@ pub struct LoginUri {
pub struct LoginUriView {
pub uri: Option<String>,
pub r#match: Option<UriMatchType>,
pub uri_checksum: Option<String>,
}

impl LoginUriView {
pub(crate) fn is_checksum_valid(&self) -> bool {
let Some(uri) = &self.uri else {
return false;
};
let Some(cs) = &self.uri_checksum else {
return false;
};
let Ok(cs) = STANDARD.decode(cs) else {
return false;
};

use sha2::Digest;
let uri_hash = sha2::Sha256::new().chain_update(uri.as_bytes()).finalize();

uri_hash.as_slice() == cs
}

pub(crate) fn generate_checksum(&mut self) {
if let Some(uri) = &self.uri {
use sha2::Digest;
let uri_hash = sha2::Sha256::new().chain_update(uri.as_bytes()).finalize();
let uri_hash = STANDARD.encode(uri_hash.as_slice());
self.uri_checksum = Some(uri_hash);
}
}
}

#[derive(Serialize, Deserialize, Debug, JsonSchema, Clone)]
Expand Down Expand Up @@ -93,6 +124,7 @@ impl KeyEncryptable<SymmetricCryptoKey, LoginUri> for LoginUriView {
Ok(LoginUri {
uri: self.uri.encrypt_with_key(key)?,
r#match: self.r#match,
uri_checksum: self.uri_checksum.encrypt_with_key(key)?,
})
}
}
Expand All @@ -116,6 +148,7 @@ impl KeyDecryptable<SymmetricCryptoKey, LoginUriView> for LoginUri {
Ok(LoginUriView {
uri: self.uri.decrypt_with_key(key)?,
r#match: self.r#match,
uri_checksum: self.uri_checksum.decrypt_with_key(key)?,
})
}
}
Expand Down Expand Up @@ -166,6 +199,7 @@ impl TryFrom<CipherLoginUriModel> for LoginUri {
Ok(Self {
uri: EncString::try_from_optional(uri.uri)?,
r#match: uri.r#match.map(|m| m.into()),
uri_checksum: EncString::try_from_optional(uri.uri_checksum)?,
})
}
}
Expand Down Expand Up @@ -208,3 +242,52 @@ impl TryFrom<bitwarden_api_api::models::CipherFido2CredentialModel> for Fido2Cre
})
}
}

#[cfg(test)]
mod tests {
#[test]
fn test_valid_checksum() {
let uri = super::LoginUriView {
uri: Some("https://example.com".to_string()),
r#match: Some(super::UriMatchType::Domain),
uri_checksum: Some("EAaArVRs5qV39C9S3zO0z9ynVoWeZkuNfeMpsVDQnOk=".to_string()),
};
assert!(uri.is_checksum_valid());
}

#[test]
fn test_invalid_checksum() {
let uri = super::LoginUriView {
uri: Some("https://example.com".to_string()),
r#match: Some(super::UriMatchType::Domain),
uri_checksum: Some("UtSgIv8LYfEdOu7yqjF7qXWhmouYGYC8RSr7/ryZg5Q=".to_string()),
};
assert!(!uri.is_checksum_valid());
}

#[test]
fn test_missing_checksum() {
let uri = super::LoginUriView {
uri: Some("https://example.com".to_string()),
r#match: Some(super::UriMatchType::Domain),
uri_checksum: None,
};
assert!(!uri.is_checksum_valid());
}

#[test]
fn test_generate_checksum() {
let mut uri = super::LoginUriView {
uri: Some("https://test.com".to_string()),
r#match: Some(super::UriMatchType::Domain),
uri_checksum: None,
};

uri.generate_checksum();

assert_eq!(
uri.uri_checksum.unwrap().as_str(),
"OWk2vQvwYD1nhLZdA+ltrpBWbDa2JmHyjUEWxRZSS8w="
);
}
}

0 comments on commit 67e743f

Please sign in to comment.