Skip to content

Commit

Permalink
[PM-5791] Change decrypt to return Sensitive (#536)
Browse files Browse the repository at this point in the history 
Updates the decrypt methods to return a `Sensitive` value which
implements `zeroize` and `zeroize on drop` to ensure the values are
zeroed on drop within the rust ecosystem.
  • Loading branch information
@Hinton
Hinton authored Apr 25, 2024
1 parent b164389 commit 51db07e
Show file tree
Hide file tree
Showing 35 changed files with 770 additions and 514 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

42 changes: 24 additions & 18 deletions crates/bitwarden-crypto/src/enc_string/asymmetric.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use super::{from_b64_vec, split_enc_string};
use crate::{
error::{CryptoError, EncStringParseError, Result},
rsa::encrypt_rsa2048_oaep_sha1,
AsymmetricCryptoKey, AsymmetricEncryptable, KeyDecryptable,
AsymmetricCryptoKey, AsymmetricEncryptable, DecryptedString, DecryptedVec, KeyDecryptable,
};

// This module is a workaround to avoid deprecated warnings that come from the ZeroizeOnDrop
Expand Down Expand Up @@ -166,8 +166,8 @@ impl AsymmetricEncString {
}
}

impl KeyDecryptable<AsymmetricCryptoKey, Vec<u8>> for AsymmetricEncString {
fn decrypt_with_key(&self, key: &AsymmetricCryptoKey) -> Result<Vec<u8>> {
impl KeyDecryptable<AsymmetricCryptoKey, DecryptedVec> for AsymmetricEncString {
fn decrypt_with_key(&self, key: &AsymmetricCryptoKey) -> Result<DecryptedVec> {
use AsymmetricEncString::*;
match self {
Rsa2048_OaepSha256_B64 { data } => key.key.decrypt(Oaep::new::<sha2::Sha256>(), data),
Expand All @@ -181,14 +181,15 @@ impl KeyDecryptable<AsymmetricCryptoKey, Vec<u8>> for AsymmetricEncString {
key.key.decrypt(Oaep::new::<sha1::Sha1>(), data)
}
}
.map(|v| DecryptedVec::new(Box::new(v)))
.map_err(|_| CryptoError::KeyDecrypt)
}
}

impl KeyDecryptable<AsymmetricCryptoKey, String> for AsymmetricEncString {
fn decrypt_with_key(&self, key: &AsymmetricCryptoKey) -> Result<String> {
let dec: Vec<u8> = self.decrypt_with_key(key)?;
String::from_utf8(dec).map_err(|_| CryptoError::InvalidUtf8String)
impl KeyDecryptable<AsymmetricCryptoKey, DecryptedString> for AsymmetricEncString {
fn decrypt_with_key(&self, key: &AsymmetricCryptoKey) -> Result<DecryptedString> {
let dec: DecryptedVec = self.decrypt_with_key(key)?;
dec.try_into()
}
}

Expand All @@ -209,8 +210,11 @@ mod tests {
use schemars::schema_for;

use super::{AsymmetricCryptoKey, AsymmetricEncString, KeyDecryptable};
use crate::{DecryptedString, SensitiveString};

const RSA_PRIVATE_KEY: &str = "-----BEGIN PRIVATE KEY-----
fn rsa_private_key_string() -> SensitiveString {
SensitiveString::test(
"-----BEGIN PRIVATE KEY-----
MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCXRVrCX+2hfOQS
8HzYUS2oc/jGVTZpv+/Ryuoh9d8ihYX9dd0cYh2tl6KWdFc88lPUH11Oxqy20Rk2
e5r/RF6T9yM0Me3NPnaKt+hlhLtfoc0h86LnhD56A9FDUfuI0dVnPcrwNv0YJIo9
Expand All @@ -237,42 +241,44 @@ AoEZ18y6/KIjpSMpqC92Nnk/EBM9EYe6Cf4eA9ApAoGAeqEUg46UTlJySkBKURGp
Is3v1kkf5I0X8DnOhwb+HPxNaiEdmO7ckm8+tPVgppLcG0+tMdLjigFQiDUQk2y3
WjyxP5ZvXu7U96jaJRI8PFMoE06WeVYcdIzrID2HvqH+w0UQJFrLJ/0Mn4stFAEz
XKZBokBGnjFnTnKcs7nv/O8=
-----END PRIVATE KEY-----";
-----END PRIVATE KEY-----",
)
}

#[test]
fn test_enc_string_rsa2048_oaep_sha256_b64() {
let private_key = AsymmetricCryptoKey::from_pem(RSA_PRIVATE_KEY).unwrap();
let private_key = AsymmetricCryptoKey::from_pem(rsa_private_key_string()).unwrap();
let enc_str: &str = "3.YFqzW9LL/uLjCnl0RRLtndzGJ1FV27mcwQwGjfJPOVrgCX9nJSUYCCDd0iTIyOZ/zRxG47b6L1Z3qgkEfcxjmrSBq60gijc3E2TBMAg7OCLVcjORZ+i1sOVOudmOPWro6uA8refMrg4lqbieDlbLMzjVEwxfi5WpcL876cD0vYyRwvLO3bzFrsE7x33HHHtZeOPW79RqMn5efsB5Dj9wVheC9Ix9AYDjbo+rjg9qR6guwKmS7k2MSaIQlrDR7yu8LP+ePtiSjx+gszJV5jQGfcx60dtiLQzLS/mUD+RmU7B950Bpx0H7x56lT5yXZbWK5YkoP6qd8B8D2aKbP68Ywg==";
let enc_string: AsymmetricEncString = enc_str.parse().unwrap();

assert_eq!(enc_string.enc_type(), 3);

let res: String = enc_string.decrypt_with_key(&private_key).unwrap();
assert_eq!(res, "EncryptMe!");
let res: DecryptedString = enc_string.decrypt_with_key(&private_key).unwrap();
assert_eq!(res.expose(), "EncryptMe!");
}

#[test]
fn test_enc_string_rsa2048_oaep_sha1_b64() {
let private_key = AsymmetricCryptoKey::from_pem(RSA_PRIVATE_KEY).unwrap();
let private_key = AsymmetricCryptoKey::from_pem(rsa_private_key_string()).unwrap();
let enc_str: &str = "4.ZheRb3PCfAunyFdQYPfyrFqpuvmln9H9w5nDjt88i5A7ug1XE0LJdQHCIYJl0YOZ1gCOGkhFu/CRY2StiLmT3iRKrrVBbC1+qRMjNNyDvRcFi91LWsmRXhONVSPjywzrJJXglsztDqGkLO93dKXNhuKpcmtBLsvgkphk/aFvxbaOvJ/FHdK/iV0dMGNhc/9tbys8laTdwBlI5xIChpRcrfH+XpSFM88+Bu03uK67N9G6eU1UmET+pISJwJvMuIDMqH+qkT7OOzgL3t6I0H2LDj+CnsumnQmDsvQzDiNfTR0IgjpoE9YH2LvPXVP2wVUkiTwXD9cG/E7XeoiduHyHjw==";
let enc_string: AsymmetricEncString = enc_str.parse().unwrap();

assert_eq!(enc_string.enc_type(), 4);

let res: String = enc_string.decrypt_with_key(&private_key).unwrap();
assert_eq!(res, "EncryptMe!");
let res: DecryptedString = enc_string.decrypt_with_key(&private_key).unwrap();
assert_eq!(res.expose(), "EncryptMe!");
}

#[test]
fn test_enc_string_rsa2048_oaep_sha1_hmac_sha256_b64() {
let private_key = AsymmetricCryptoKey::from_pem(RSA_PRIVATE_KEY).unwrap();
let private_key = AsymmetricCryptoKey::from_pem(rsa_private_key_string()).unwrap();
let enc_str: &str = "6.ThnNc67nNr7GELyuhGGfsXNP2zJnNqhrIsjntEQ27r2qmn8vwdHbTbfO0cwt6YgSibDN0PjiCZ1O3Wb/IFq+vwvyRwFqF9145wBF8CQCbkhV+M0XvO99kh0daovtt120Nve/5ETI5PbPag9VdalKRQWZypJaqQHm5TAQVf4F5wtLlCLMBkzqTk+wkFe7BPMTGn07T+O3eJbTxXvyMZewQ7icJF0MZVA7VyWX9qElmZ89FCKowbf1BMr5pbcQ+0KdXcSVW3to43VkTp7k7COwsuH3M/i1AuVP5YN8ixjyRpvaeGqX/ap2nCHK2Wj5VxgCGT7XEls6ZknnAp9nB9qVjQ==|s3ntw5H/KKD/qsS0lUghTHl5Sm9j6m7YEdNHf0OeAFQ=";
let enc_string: AsymmetricEncString = enc_str.parse().unwrap();

assert_eq!(enc_string.enc_type(), 6);

let res: String = enc_string.decrypt_with_key(&private_key).unwrap();
assert_eq!(res, "EncryptMe!");
let res: DecryptedString = enc_string.decrypt_with_key(&private_key).unwrap();
assert_eq!(res.expose(), "EncryptMe!");
}

#[test]
Expand Down
49 changes: 32 additions & 17 deletions crates/bitwarden-crypto/src/enc_string/symmetric.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use serde::Deserialize;
use super::{check_length, from_b64, from_b64_vec, split_enc_string};
use crate::{
error::{CryptoError, EncStringParseError, Result},
KeyDecryptable, KeyEncryptable, LocateKey, SymmetricCryptoKey,
DecryptedString, DecryptedVec, KeyDecryptable, KeyEncryptable, LocateKey, SymmetricCryptoKey,
};

/// # Encrypted string primitive
Expand Down Expand Up @@ -233,11 +233,15 @@ impl KeyEncryptable<SymmetricCryptoKey, EncString> for &[u8] {
}
}

impl KeyDecryptable<SymmetricCryptoKey, Vec<u8>> for EncString {
fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result<Vec<u8>> {
impl KeyDecryptable<SymmetricCryptoKey, DecryptedVec> for EncString {
fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result<DecryptedVec> {
match self {
EncString::AesCbc256_B64 { iv, data } => {
let dec = crate::aes::decrypt_aes256(iv, data.clone(), &key.key)?;
let dec = DecryptedVec::new(Box::new(crate::aes::decrypt_aes256(
iv,
data.clone(),
&key.key,
)?));
Ok(dec)
}
EncString::AesCbc128_HmacSha256_B64 { iv, mac, data } => {
Expand All @@ -247,13 +251,24 @@ impl KeyDecryptable<SymmetricCryptoKey, Vec<u8>> for EncString {
// When refactoring the key handling, this should be fixed.
let enc_key = key.key[0..16].into();
let mac_key = key.key[16..32].into();
let dec = crate::aes::decrypt_aes128_hmac(iv, mac, data.clone(), mac_key, enc_key)?;
let dec = DecryptedVec::new(Box::new(crate::aes::decrypt_aes128_hmac(
iv,
mac,
data.clone(),
mac_key,
enc_key,
)?));
Ok(dec)
}
EncString::AesCbc256_HmacSha256_B64 { iv, mac, data } => {
let mac_key = key.mac_key.as_ref().ok_or(CryptoError::InvalidMac)?;
let dec =
crate::aes::decrypt_aes256_hmac(iv, mac, data.clone(), mac_key, &key.key)?;
let dec = DecryptedVec::new(Box::new(crate::aes::decrypt_aes256_hmac(
iv,
mac,
data.clone(),
mac_key,
&key.key,
)?));
Ok(dec)
}
}
Expand All @@ -266,10 +281,10 @@ impl KeyEncryptable<SymmetricCryptoKey, EncString> for String {
}
}

impl KeyDecryptable<SymmetricCryptoKey, String> for EncString {
fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result<String> {
let dec: Vec<u8> = self.decrypt_with_key(key)?;
String::from_utf8(dec).map_err(|_| CryptoError::InvalidUtf8String)
impl KeyDecryptable<SymmetricCryptoKey, DecryptedString> for EncString {
fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result<DecryptedString> {
let dec: DecryptedVec = self.decrypt_with_key(key)?;
dec.try_into()
}
}

Expand Down Expand Up @@ -301,8 +316,8 @@ mod tests {
let test_string = "encrypted_test_string";
let cipher = test_string.to_owned().encrypt_with_key(&key).unwrap();

let decrypted_str: String = cipher.decrypt_with_key(&key).unwrap();
assert_eq!(decrypted_str, test_string);
let decrypted_str: SensitiveString = cipher.decrypt_with_key(&key).unwrap();
assert_eq!(decrypted_str.expose(), test_string);
}

#[test]
Expand Down Expand Up @@ -399,8 +414,8 @@ mod tests {
let enc_string: EncString = enc_str.parse().unwrap();
assert_eq!(enc_string.enc_type(), 0);

let dec_str: String = enc_string.decrypt_with_key(&key).unwrap();
assert_eq!(dec_str, "EncryptMe!");
let dec_str: SensitiveString = enc_string.decrypt_with_key(&key).unwrap();
assert_eq!(dec_str.expose(), "EncryptMe!");
}

#[test]
Expand All @@ -412,8 +427,8 @@ mod tests {
let enc_string: EncString = enc_str.parse().unwrap();
assert_eq!(enc_string.enc_type(), 1);

let dec_str: String = enc_string.decrypt_with_key(&key).unwrap();
assert_eq!(dec_str, "EncryptMe!");
let dec_str: SensitiveString = enc_string.decrypt_with_key(&key).unwrap();
assert_eq!(dec_str.expose(), "EncryptMe!");
}

#[test]
Expand Down
Loading

0 comments on commit 51db07e

Please sign in to comment.