Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PM-5791] Change decrypt to return Sensitive #536

Merged
merged 29 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
8c869f2
Explore introducing a Decrypted struct
Hinton Jan 19, 2024
74a5b08
Replace with std::mem::take
Hinton Jan 19, 2024
c57bea6
Merge branch 'main' of github.com:bitwarden/sdk into ps/zeroize-decry…
Hinton Jan 25, 2024
d5f2ae5
Document and introduce type alias
Hinton Jan 25, 2024
97ecc2c
Format
Hinton Jan 25, 2024
449466a
Add Decrypted to asymmetric
Hinton Jan 25, 2024
ed4d92b
Use Decrypt everywhere
Hinton Jan 25, 2024
18cf0d6
Use boxed value and fix tests
Hinton Jan 26, 2024
3d247ad
Merge branch 'main' of github.com:bitwarden/sdk into ps/zeroize-decry…
Hinton Jan 26, 2024
b848625
Resolve review feedback
Hinton Jan 29, 2024
d85fc9f
Extract a common sensitive type.
Hinton Jan 29, 2024
d7eedaa
Format
Hinton Jan 29, 2024
cb5f43a
Merge branch 'main' into ps/zeroize-decrypted
dani-garcia Mar 7, 2024
ba14e8c
Update exporters to use Sensitive
dani-garcia Mar 7, 2024
19fecae
Add support for base64 encode/decode for the Sensitive types
dani-garcia Mar 7, 2024
cece11e
Merge branch 'main' into ps/zeroize-decrypted
dani-garcia Mar 11, 2024
ef8ba37
Make base64 memory test required
dani-garcia Mar 11, 2024
621cf72
Merge branch 'main' of github.com:bitwarden/sdk into ps/zeroize-decry…
Hinton Apr 22, 2024
b87bd95
Use ::test in tests
Hinton Apr 22, 2024
37b18ad
Mark send key as sensitive
Hinton Apr 22, 2024
b2c3ff3
Merge branch 'main' of github.com:bitwarden/sdk into ps/zeroize-decry…
Hinton Apr 22, 2024
1f731a8
Remove accidentally duplicated generate_cipher
Hinton Apr 22, 2024
ef08b7c
Convert Send::derive_shareable_key to expect SensitiveVec
Hinton Apr 23, 2024
16ea5c3
Fix compile
Hinton Apr 23, 2024
31a02f8
Refactor sub_title to not expose temporary values
Hinton Apr 23, 2024
456f54f
Merge branch 'main' of github.com:bitwarden/sdk into ps/zeroize-decry…
Hinton Apr 23, 2024
0d40054
Shore up uri checksum
Hinton Apr 23, 2024
da787a2
Remove converter for DecryptedString
Hinton Apr 25, 2024
7baa6f1
Review feedback
Hinton Apr 25, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

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
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
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
Loading