Skip to content

Commit

Permalink
Revert "Implement Eq and PartialEq for Sensitive and their inner types (
Browse files Browse the repository at this point in the history
  • Loading branch information
Hinton committed May 17, 2024
1 parent df99792 commit dc9f252
Show file tree
Hide file tree
Showing 17 changed files with 51 additions and 76 deletions.
11 changes: 5 additions & 6 deletions crates/bitwarden-crypto/src/enc_string/asymmetric.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use crate::{
error::{CryptoError, EncStringParseError, Result},
rsa::encrypt_rsa2048_oaep_sha1,
AsymmetricCryptoKey, AsymmetricEncryptable, DecryptedString, DecryptedVec, KeyDecryptable,
SensitiveVec,
};

// This module is a workaround to avoid deprecated warnings that come from the ZeroizeOnDrop
Expand Down Expand Up @@ -147,10 +146,10 @@ impl serde::Serialize for AsymmetricEncString {
impl AsymmetricEncString {
/// Encrypt and produce a [AsymmetricEncString::Rsa2048_OaepSha1_B64] variant.
pub fn encrypt_rsa2048_oaep_sha1(
data_dec: SensitiveVec,
data_dec: &[u8],
key: &dyn AsymmetricEncryptable,
) -> Result<AsymmetricEncString> {
let enc = encrypt_rsa2048_oaep_sha1(key.to_public_key(), data_dec.expose())?;
let enc = encrypt_rsa2048_oaep_sha1(key.to_public_key(), data_dec)?;
Ok(AsymmetricEncString::Rsa2048_OaepSha1_B64 { data: enc })
}

Expand Down Expand Up @@ -255,7 +254,7 @@ XKZBokBGnjFnTnKcs7nv/O8=
assert_eq!(enc_string.enc_type(), 3);

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

#[test]
Expand All @@ -267,7 +266,7 @@ XKZBokBGnjFnTnKcs7nv/O8=
assert_eq!(enc_string.enc_type(), 4);

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

#[test]
Expand All @@ -279,7 +278,7 @@ XKZBokBGnjFnTnKcs7nv/O8=
assert_eq!(enc_string.enc_type(), 6);

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

#[test]
Expand Down
6 changes: 3 additions & 3 deletions crates/bitwarden-crypto/src/enc_string/symmetric.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ mod tests {
let cipher = test_string.to_owned().encrypt_with_key(&key).unwrap();

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

#[test]
Expand Down Expand Up @@ -415,7 +415,7 @@ mod tests {
assert_eq!(enc_string.enc_type(), 0);

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

#[test]
Expand All @@ -428,7 +428,7 @@ mod tests {
assert_eq!(enc_string.enc_type(), 1);

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

#[test]
Expand Down
10 changes: 5 additions & 5 deletions crates/bitwarden-crypto/src/keys/asymmetric_crypto_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,8 @@ DnqOsltgPomWZ7xVfMkm9niL2OA=
assert_eq!(pem_key.key, der_key.key);

// Check that the keys can be converted back to DER
assert_eq!(der_key_vec, der_key.to_der().unwrap().as_slice());
assert_eq!(der_key_vec, pem_key.to_der().unwrap().as_slice());
assert_eq!(&der_key.to_der().unwrap(), der_key_vec.expose());
assert_eq!(&pem_key.to_der().unwrap(), der_key_vec.expose());
}

#[test]
Expand Down Expand Up @@ -224,12 +224,12 @@ DnqOsltgPomWZ7xVfMkm9niL2OA=
let private_key = AsymmetricCryptoKey::from_der(private_key).unwrap();
let public_key = AsymmetricPublicCryptoKey::from_der(public_key).unwrap();

let plaintext = SensitiveString::test("Hello, world!");
let plaintext = "Hello, world!";
let encrypted =
AsymmetricEncString::encrypt_rsa2048_oaep_sha1(plaintext.clone().into(), &public_key)
AsymmetricEncString::encrypt_rsa2048_oaep_sha1(plaintext.as_bytes(), &public_key)
.unwrap();
let decrypted: DecryptedString = encrypted.decrypt_with_key(&private_key).unwrap();

assert_eq!(plaintext, decrypted);
assert_eq!(plaintext, decrypted.expose());
}
}
2 changes: 1 addition & 1 deletion crates/bitwarden-crypto/src/keys/device_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ impl DeviceKey {
let data = user_key.to_vec();

let protected_user_key =
AsymmetricEncString::encrypt_rsa2048_oaep_sha1(data, &device_private_key)?;
AsymmetricEncString::encrypt_rsa2048_oaep_sha1(data.expose(), &device_private_key)?;

let protected_device_public_key = device_private_key
.to_public_der()?
Expand Down
4 changes: 2 additions & 2 deletions crates/bitwarden-crypto/src/keys/master_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,10 +190,10 @@ mod tests {
let master_key = MasterKey::derive(&password, salt, &kdf).unwrap();

assert_eq!(
"ZF6HjxUTSyBHsC+HXSOhZoXN+UuMnygV5YkWXCY4VmM=",
master_key
.derive_master_key_hash(&password, HashPurpose::ServerAuthorization)
.unwrap(),
"ZF6HjxUTSyBHsC+HXSOhZoXN+UuMnygV5YkWXCY4VmM=",
);
}

Expand All @@ -210,10 +210,10 @@ mod tests {
let master_key = MasterKey::derive(&password, salt, &kdf).unwrap();

assert_eq!(
"PR6UjYmjmppTYcdyTiNbAhPJuQQOmynKbdEl1oyi/iQ=",
master_key
.derive_master_key_hash(&password, HashPurpose::ServerAuthorization)
.unwrap(),
"PR6UjYmjmppTYcdyTiNbAhPJuQQOmynKbdEl1oyi/iQ=",
);
}

Expand Down
4 changes: 2 additions & 2 deletions crates/bitwarden-crypto/src/keys/shareable_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ mod tests {
"test_key",
None,
);
assert_eq!(key.to_base64(), "4PV6+PcmF2w7YHRatvyMcVQtI7zvCyssv/wFWmzjiH6Iv9altjmDkuBD1aagLVaLezbthbSe+ktR+U6qswxNnQ==");
assert_eq!(key.to_base64().expose(), "4PV6+PcmF2w7YHRatvyMcVQtI7zvCyssv/wFWmzjiH6Iv9altjmDkuBD1aagLVaLezbthbSe+ktR+U6qswxNnQ==");

let key = derive_shareable_key(
Sensitive::new(Box::new(*b"67t9b5g67$%Dh89n")),
"test_key",
Some("test"),
);
assert_eq!(key.to_base64(), "F9jVQmrACGx9VUPjuzfMYDjr726JtL300Y3Yg+VYUnVQtQ1s8oImJ5xtp1KALC9h2nav04++1LDW4iFD+infng==");
assert_eq!(key.to_base64().expose(), "F9jVQmrACGx9VUPjuzfMYDjr726JtL300Y3Yg+VYUnVQtQ1s8oImJ5xtp1KALC9h2nav04++1LDW4iFD+infng==");
}
}
2 changes: 1 addition & 1 deletion crates/bitwarden-crypto/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
//! let encrypted = data.clone().encrypt_with_key(&key)?;
//! let decrypted: DecryptedString = encrypted.decrypt_with_key(&key)?;
//!
//! assert_eq!(decrypted, data);
//! assert_eq!(&data, decrypted.expose());
//! Ok(())
//! }
//! ```
Expand Down
23 changes: 1 addition & 22 deletions crates/bitwarden-crypto/src/sensitive/sensitive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::CryptoError;
///
/// Internally [`Sensitive`] contains a [`Box`] which ensures the value is placed on the heap. It
/// implements the [`Drop`] trait which calls `zeroize` on the inner value.
#[derive(Eq, Clone, Zeroize, ZeroizeOnDrop)]
#[derive(PartialEq, Clone, Zeroize, ZeroizeOnDrop)]
pub struct Sensitive<V: Zeroize> {
pub(super) value: Box<V>,
}
Expand Down Expand Up @@ -144,27 +144,6 @@ impl<V: Zeroize + Serialize> fmt::Debug for Sensitive<V> {
}
}

impl<V: Zeroize + PartialEq<V>> PartialEq<Sensitive<V>> for Sensitive<V> {
fn eq(&self, other: &Self) -> bool {
self.value.eq(&other.value)
}
}
impl<V: Zeroize + PartialEq<V>> PartialEq<V> for Sensitive<V> {
fn eq(&self, other: &V) -> bool {
self.value.as_ref().eq(other)
}
}
impl PartialEq<&str> for SensitiveString {
fn eq(&self, other: &&str) -> bool {
self.value.as_ref().eq(other)
}
}
impl PartialEq<&[u8]> for SensitiveVec {
fn eq(&self, other: &&[u8]) -> bool {
self.value.as_ref().eq(other)
}
}

/// Unfortunately once we serialize a `SensitiveString` we can't control the future memory.
impl<V: Zeroize + Serialize> Serialize for Sensitive<V> {
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
Expand Down
2 changes: 1 addition & 1 deletion crates/bitwarden/src/auth/access_token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ mod tests {
"ec2c1d46-6a4b-4751-a310-af9601317f2d"
);
assert_eq!(token.client_secret, "C2IgxjjLF7qSshsbwe8JGcbM075YXw");
assert_eq!(token.encryption_key.to_base64(), "H9/oIRLtL9nGCQOVDjSMoEbJsjWXSOCb3qeyDt6ckzS3FhyboEDWyTP/CQfbIszNmAVg2ExFganG1FVFGXO/Jg==");
assert_eq!(token.encryption_key.to_base64().expose(), "H9/oIRLtL9nGCQOVDjSMoEbJsjWXSOCb3qeyDt6ckzS3FhyboEDWyTP/CQfbIszNmAVg2ExFganG1FVFGXO/Jg==");
}

#[test]
Expand Down
21 changes: 9 additions & 12 deletions crates/bitwarden/src/auth/auth_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ pub(crate) fn approve_auth_request(
let key = enc.get_key(&None).ok_or(Error::VaultLocked)?;

Ok(AsymmetricEncString::encrypt_rsa2048_oaep_sha1(
key.to_vec(),
key.to_vec().expose(),
&public_key,
)?)
}
Expand All @@ -107,23 +107,22 @@ pub(crate) fn approve_auth_request(
fn test_auth_request() {
let request = new_auth_request("[email protected]").unwrap();

let secret = bitwarden_crypto::SensitiveVec::test(&[
let secret: &[u8] = &[
111, 32, 97, 169, 4, 241, 174, 74, 239, 206, 113, 86, 174, 68, 216, 238, 52, 85, 156, 27,
134, 149, 54, 55, 91, 147, 45, 130, 131, 237, 51, 31, 191, 106, 155, 14, 160, 82, 47, 40,
96, 31, 114, 127, 212, 187, 167, 110, 205, 116, 198, 243, 218, 72, 137, 53, 248, 43, 255,
67, 35, 61, 245, 93,
]);
];

let private_key = SensitiveString::new(Box::new(request.private_key.clone()));
let private_key =
AsymmetricCryptoKey::from_der(private_key.decode_base64(STANDARD).unwrap()).unwrap();

let encrypted =
AsymmetricEncString::encrypt_rsa2048_oaep_sha1(secret.clone(), &private_key).unwrap();
let encrypted = AsymmetricEncString::encrypt_rsa2048_oaep_sha1(secret, &private_key).unwrap();

let decrypted = auth_request_decrypt_user_key(request.private_key, encrypted).unwrap();

assert_eq!(decrypted.to_vec(), secret);
assert_eq!(decrypted.to_vec().expose(), secret);
}

#[cfg(test)]
Expand Down Expand Up @@ -172,12 +171,11 @@ mod tests {
let dec = auth_request_decrypt_user_key(private_key.to_owned(), enc_user_key).unwrap();

assert_eq!(
dec.to_vec(),
[
dec.to_vec().expose(),
&[
201, 37, 234, 213, 21, 75, 40, 70, 149, 213, 234, 16, 19, 251, 162, 245, 161, 74,
34, 245, 211, 151, 211, 192, 95, 10, 117, 50, 88, 223, 23, 157
]
.as_slice()
);
}

Expand All @@ -192,14 +190,13 @@ mod tests {
.unwrap();

assert_eq!(
dec.to_vec(),
[
dec.to_vec().expose(),
&[
109, 128, 172, 147, 206, 123, 134, 95, 16, 36, 155, 113, 201, 18, 186, 230, 216,
212, 173, 188, 74, 11, 134, 131, 137, 242, 105, 178, 105, 126, 52, 139, 248, 91,
215, 21, 128, 91, 226, 222, 165, 67, 251, 34, 83, 81, 77, 147, 225, 76, 13, 41,
102, 45, 183, 218, 106, 89, 254, 208, 251, 101, 130, 10,
]
.as_slice()
);
}

Expand Down
2 changes: 1 addition & 1 deletion crates/bitwarden/src/auth/tde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub(super) fn make_register_tde_keys(
let key_pair = user_key.make_key_pair()?;

let admin_reset =
AsymmetricEncString::encrypt_rsa2048_oaep_sha1(user_key.0.to_vec(), &public_key)?;
AsymmetricEncString::encrypt_rsa2048_oaep_sha1(user_key.0.to_vec().expose(), &public_key)?;

Check warning on line 28 in crates/bitwarden/src/auth/tde.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/tde.rs#L28

Added line #L28 was not covered by tests

let device_key = if remember_device {
Some(DeviceKey::trust_device(&user_key.0)?)
Expand Down
4 changes: 2 additions & 2 deletions crates/bitwarden/src/mobile/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ pub(super) fn enroll_admin_password_reset(
let key = enc.get_key(&None).ok_or(Error::VaultLocked)?;

Ok(AsymmetricEncString::encrypt_rsa2048_oaep_sha1(
key.to_vec(),
key.to_vec().expose(),
&public_key,
)?)
}
Expand Down Expand Up @@ -537,6 +537,6 @@ mod tests {
.unwrap()
.get_key(&None)
.unwrap();
assert_eq!(decrypted, expected.to_vec());
assert_eq!(decrypted.expose(), expected.to_vec().expose());
}
}
8 changes: 4 additions & 4 deletions crates/bitwarden/src/tool/exporters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ mod tests {
f.id,
"fd411a1a-fec8-4070-985d-0e6560860e69".parse().unwrap()
);
assert_eq!(f.name, "test_name");
assert_eq!(f.name.expose(), "test_name");
}

#[test]
Expand Down Expand Up @@ -274,7 +274,7 @@ mod tests {
"fd411a1a-fec8-4070-985d-0e6560860e69".parse().unwrap()
);
assert_eq!(cipher.folder_id, None);
assert_eq!(cipher.name, "My login");
assert_eq!(cipher.name.expose(), "My login");
assert_eq!(cipher.notes, None);
assert!(!cipher.favorite);
assert_eq!(cipher.reprompt, 0);
Expand All @@ -290,8 +290,8 @@ mod tests {
assert_eq!(cipher.deleted_date, None);

if let bitwarden_exporters::CipherType::Login(l) = cipher.r#type {
assert_eq!(l.username.unwrap(), "test_username");
assert_eq!(l.password.unwrap(), "test_password");
assert_eq!(l.username.unwrap().expose(), "test_username");
assert_eq!(l.password.unwrap().expose(), "test_password");
assert!(l.login_uris.is_empty());
assert_eq!(l.totp, None);
} else {
Expand Down
2 changes: 1 addition & 1 deletion crates/bitwarden/src/vault/cipher/attachment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,6 @@ mod tests {
.decrypt_with_key(&user_key)
.unwrap();

assert_eq!(dec, original);
assert_eq!(dec.expose(), &original);
}
}
Loading

0 comments on commit dc9f252

Please sign in to comment.