Skip to content

Commit

Permalink
Revert "Convert to_der to use Sensitive (#748)"
Browse files Browse the repository at this point in the history
This reverts commit 34b7213.
  • Loading branch information
Hinton committed May 17, 2024
1 parent 45de0bd commit df99792
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 61 deletions.
57 changes: 26 additions & 31 deletions crates/bitwarden-crypto/src/keys/asymmetric_crypto_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ pub struct AsymmetricPublicCryptoKey {

impl AsymmetricPublicCryptoKey {
/// Build a public key from the SubjectPublicKeyInfo DER.
pub fn from_der(der: &[u8]) -> Result<Self> {
pub fn from_der(der: SensitiveVec) -> Result<Self> {
Ok(Self {
key: rsa::RsaPublicKey::from_public_key_der(der)
key: rsa::RsaPublicKey::from_public_key_der(der.expose())
.map_err(|_| CryptoError::InvalidKey)?,
})
}
Expand Down Expand Up @@ -86,29 +86,24 @@ impl AsymmetricCryptoKey {
})
}

pub fn to_der(&self) -> Result<SensitiveVec> {
pub fn to_der(&self) -> Result<Vec<u8>> {
use rsa::pkcs8::EncodePrivateKey;

// SecretDocument implements ZeroizeOnDrop
let key = self
Ok(self
.key
.to_pkcs8_der()
.map_err(|_| CryptoError::InvalidKey)?;

Ok(SensitiveVec::new(Box::new(key.as_bytes().to_owned())))
.map_err(|_| CryptoError::InvalidKey)?
.as_bytes()
.to_owned())
}

pub fn to_public_der(&self) -> Result<Vec<u8>> {
use rsa::pkcs8::EncodePublicKey;

// SecretDocument implements ZeroizeOnDrop
let key = self
Ok(self
.to_public_key()
.to_public_key_der()
.map_err(|_| CryptoError::InvalidKey)?;

// Public keys are considered not sensitive
Ok(key.as_bytes().to_owned())
.map_err(|_| CryptoError::InvalidKey)?
.as_bytes()
.to_owned())
}
}

Expand All @@ -129,7 +124,7 @@ impl std::fmt::Debug for AsymmetricCryptoKey {

#[cfg(test)]
mod tests {
use base64::{engine::general_purpose::STANDARD, Engine};
use base64::engine::general_purpose::STANDARD;

use crate::{
AsymmetricCryptoKey, AsymmetricEncString, AsymmetricPublicCryptoKey, DecryptedString,
Expand Down Expand Up @@ -177,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());
assert_eq!(der_key_vec, pem_key.to_der().unwrap());
assert_eq!(der_key_vec, der_key.to_der().unwrap().as_slice());
assert_eq!(der_key_vec, pem_key.to_der().unwrap().as_slice());
}

#[test]
Expand Down Expand Up @@ -214,20 +209,20 @@ DnqOsltgPomWZ7xVfMkm9niL2OA=
.decode_base64(STANDARD)
.unwrap();

let public_key = STANDARD
.decode(concat!(
"MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEArvcXfr5pCD6KhzXo7BWc",
"5Hdcbgp9U6hk0+wDYQBJ2yP8mlbd3GiN9JMFAtliE6BaTYLuxI9Mdk7XmDoKy63X",
"AuI8tUon5imL/792Wca3f3qrbZh9pOfPKWp7HkcByty1ZO8QPlEYUP24y4DzOfVd",
"LkdZfs9X5qKHiTxc+VklzTm3PSap4eORTQ/lP1GB10y0qJk5+44GRcSQSr3ku6ui",
"2re8AJ2GQhdnZz5oWaCb/kij5bQPBwBrIEBlgRdaeasVdR6wFJPJAQZxtqWo9MPK",
"eVDOkaQ3Qrryh+49S4rln3592/WeHYM5hO47DJr86ELcqcyCmksYas7xTqHfVfHS",
"XQIDAQAB"
))
.unwrap();
let public_key = SensitiveString::test(concat!(
"MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEArvcXfr5pCD6KhzXo7BWc",
"5Hdcbgp9U6hk0+wDYQBJ2yP8mlbd3GiN9JMFAtliE6BaTYLuxI9Mdk7XmDoKy63X",
"AuI8tUon5imL/792Wca3f3qrbZh9pOfPKWp7HkcByty1ZO8QPlEYUP24y4DzOfVd",
"LkdZfs9X5qKHiTxc+VklzTm3PSap4eORTQ/lP1GB10y0qJk5+44GRcSQSr3ku6ui",
"2re8AJ2GQhdnZz5oWaCb/kij5bQPBwBrIEBlgRdaeasVdR6wFJPJAQZxtqWo9MPK",
"eVDOkaQ3Qrryh+49S4rln3592/WeHYM5hO47DJr86ELcqcyCmksYas7xTqHfVfHS",
"XQIDAQAB",
))
.decode_base64(STANDARD)
.unwrap();

let private_key = AsymmetricCryptoKey::from_der(private_key).unwrap();
let public_key = AsymmetricPublicCryptoKey::from_der(&public_key).unwrap();
let public_key = AsymmetricPublicCryptoKey::from_der(public_key).unwrap();

let plaintext = SensitiveString::test("Hello, world!");
let encrypted =
Expand Down
1 change: 0 additions & 1 deletion crates/bitwarden-crypto/src/keys/device_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ impl DeviceKey {

let protected_device_private_key = device_private_key
.to_der()?
.expose()
.encrypt_with_key(&device_key.0)?;

Ok(TrustDeviceResponse {
Expand Down
35 changes: 15 additions & 20 deletions crates/bitwarden/src/auth/auth_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use crate::{error::Error, Client};
pub struct AuthRequestResponse {
/// Base64 encoded private key
/// This key is temporarily passed back and will most likely not be available in the future
pub private_key: SensitiveString,
pub private_key: String,
/// Base64 encoded public key
pub public_key: String,
/// Fingerprint of the public key
Expand All @@ -35,10 +35,10 @@ pub(crate) fn new_auth_request(email: &str) -> Result<AuthRequestResponse, Error
let spki = key.to_public_der()?;

let fingerprint = fingerprint(email, &spki)?;
let b64 = STANDARD.encode(spki);
let b64 = STANDARD.encode(&spki);

Ok(AuthRequestResponse {
private_key: key.to_der()?.encode_base64(STANDARD),
private_key: STANDARD.encode(key.to_der()?),
public_key: b64,
fingerprint,
access_code: password(PasswordGeneratorRequest {
Expand All @@ -55,11 +55,12 @@ pub(crate) fn new_auth_request(email: &str) -> Result<AuthRequestResponse, Error
/// Decrypt the user key using the private key generated previously.
#[cfg(feature = "mobile")]
pub(crate) fn auth_request_decrypt_user_key(
private_key: SensitiveString,
private_key: String,
user_key: AsymmetricEncString,
) -> Result<SymmetricCryptoKey, Error> {
use bitwarden_crypto::DecryptedVec;

let private_key = SensitiveString::new(Box::new(private_key));
let key = AsymmetricCryptoKey::from_der(private_key.decode_base64(STANDARD)?)?;
let key: DecryptedVec = user_key.decrypt_with_key(&key)?;

Expand All @@ -69,12 +70,13 @@ pub(crate) fn auth_request_decrypt_user_key(
/// Decrypt the user key using the private key generated previously.
#[cfg(feature = "mobile")]
pub(crate) fn auth_request_decrypt_master_key(
private_key: SensitiveString,
private_key: String,
master_key: AsymmetricEncString,
user_key: EncString,
) -> Result<SymmetricCryptoKey, Error> {
use bitwarden_crypto::{DecryptedVec, MasterKey};

let private_key = SensitiveString::new(Box::new(private_key));
let key = AsymmetricCryptoKey::from_der(private_key.decode_base64(STANDARD)?)?;
let master_key: DecryptedVec = master_key.decrypt_with_key(&key)?;
let master_key = MasterKey::new(SymmetricCryptoKey::try_from(master_key)?);
Expand All @@ -89,7 +91,8 @@ pub(crate) fn approve_auth_request(
client: &mut Client,
public_key: String,
) -> Result<AsymmetricEncString, Error> {
let public_key = AsymmetricPublicCryptoKey::from_der(&STANDARD.decode(public_key)?)?;
let public_key = SensitiveString::new(Box::new(public_key));
let public_key = AsymmetricPublicCryptoKey::from_der(public_key.decode_base64(STANDARD)?)?;

let enc = client.get_encryption_settings()?;
let key = enc.get_key(&None).ok_or(Error::VaultLocked)?;
Expand All @@ -111,9 +114,9 @@ fn test_auth_request() {
67, 35, 61, 245, 93,
]);

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

let encrypted =
AsymmetricEncString::encrypt_rsa2048_oaep_sha1(secret.clone(), &private_key).unwrap();
Expand All @@ -127,7 +130,6 @@ fn test_auth_request() {
mod tests {
use std::num::NonZeroU32;

use base64::Engine;
use bitwarden_crypto::{Kdf, SensitiveVec};

use super::*;
Expand Down Expand Up @@ -167,11 +169,7 @@ mod tests {
let private_key = "MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCzLtEUdxfcLxDj84yaGFsVF5hZ8Hjlb08NMQDy1RnBma06I3ZESshLYzVz4r/gegMn9OOltfV/Yxlyvida8oW6qdlfJ7AVz6Oa8pV7BiL40C7b76+oqraQpyYw2HChANB1AhXL9SqWngKmLZwjA7qiCrmcc0kZHeOb4KnKtp9iVvPVs+8veFvKgYO4ba2AAOHKFdR0W55/agXfAy+fWUAkC8mc9ikyJdQWaPV6OZvC2XFkOseBQm9Rynudh3BQpoWiL6w620efe7t5k+02/EyOFJL9f/XEEjM/+Yo0t3LAfkuhHGeKiRST59Xc9hTEmyJTeVXROtz+0fjqOp3xkaObAgMBAAECggEACs4xhnO0HaZhh1/iH7zORMIRXKeyxP2LQiTR8xwN5JJ9wRWmGAR9VasS7EZFTDidIGVME2u/h4s5EqXnhxfO+0gGksVvgNXJ/qw87E8K2216g6ZNo6vSGA7H1GH2voWwejJ4/k/cJug6dz2S402rRAKh2Wong1arYHSkVlQp3diiMa5FHAOSE+Cy09O2ZsaF9IXQYUtlW6AVXFrBEPYH2kvkaPXchh8VETMijo6tbvoKLnUHe+wTaDMls7hy8exjtVyI59r3DNzjy1lNGaGb5QSnFMXR+eHhPZc844Wv02MxC15zKABADrl58gpJyjTl6XpDdHCYGsmGpVGH3X9TQQKBgQDz/9beFjzq59ve6rGwn+EtnQfSsyYT+jr7GN8lNEXb3YOFXBgPhfFIcHRh2R00Vm9w2ApfAx2cd8xm2I6HuvQ1Os7g26LWazvuWY0Qzb+KaCLQTEGH1RnTq6CCG+BTRq/a3J8M4t38GV5TWlzv8wr9U4dl6FR4efjb65HXs1GQ4QKBgQC7/uHfrOTEHrLeIeqEuSl0vWNqEotFKdKLV6xpOvNuxDGbgW4/r/zaxDqt0YBOXmRbQYSEhmO3oy9J6XfE1SUln0gbavZeW0HESCAmUIC88bDnspUwS9RxauqT5aF8ODKN/bNCWCnBM1xyonPOs1oT1nyparJVdQoG//Y7vkB3+wKBgBqLqPq8fKAp3XfhHLfUjREDVoiLyQa/YI9U42IOz9LdxKNLo6p8rgVthpvmnRDGnpUuS+KOWjhdqDVANjF6G3t3DG7WNl8Rh5Gk2H4NhFswfSkgQrjebFLlBy9gjQVCWXt8KSmjvPbiY6q52Aaa8IUjA0YJAregvXxfopxO+/7BAoGARicvEtDp7WWnSc1OPoj6N14VIxgYcI7SyrzE0d/1x3ffKzB5e7qomNpxKzvqrVP8DzG7ydh8jaKPmv1MfF8tpYRy3AhmN3/GYwCnPqT75YYrhcrWcVdax5gmQVqHkFtIQkRSCIftzPLlpMGKha/YBV8c1fvC4LD0NPh/Ynv0gtECgYEAyOZg95/kte0jpgUEgwuMrzkhY/AaUJULFuR5MkyvReEbtSBQwV5tx60+T95PHNiFooWWVXiLMsAgyI2IbkxVR1Pzdri3gWK5CTfqb7kLuaj/B7SGvBa2Sxo478KS5K8tBBBWkITqo+wLC0mn3uZi1dyMWO1zopTA+KtEGF2dtGQ=";

let enc_user_key = "4.dxbd5OMwi/Avy7DQxvLV+Z7kDJgHBtg/jAbgYNO7QU0Zii4rLFNco2lS5aS9z42LTZHc2p5HYwn2ZwkZNfHsQ6//d5q40MDgGYJMKBXOZP62ZHhct1XsvYBmtcUtIOm5j2HSjt2pjEuGAc1LbyGIWRJJQ3Lp1ULbL2m71I+P23GF36JyOM8SUWvpvxE/3+qqVhRFPG2VqMCYa2kLLxwVfUmpV+KKjX1TXsrq6pfJIwHNwHw4h7MSfD8xTy2bx4MiBt638Z9Vt1pGsSQkh9RgPvCbnhuCpZQloUgJ8ByLVEcrlKx3yaaxiQXvte+ZhuOI7rGdjmoVoOzisooje4JgYw==".parse().unwrap();
let dec = auth_request_decrypt_user_key(
SensitiveString::new(Box::new(private_key.to_owned())),
enc_user_key,
)
.unwrap();
let dec = auth_request_decrypt_user_key(private_key.to_owned(), enc_user_key).unwrap();

assert_eq!(
dec.to_vec(),
Expand All @@ -189,12 +187,9 @@ mod tests {

let enc_master_key = "4.dxbd5OMwi/Avy7DQxvLV+Z7kDJgHBtg/jAbgYNO7QU0Zii4rLFNco2lS5aS9z42LTZHc2p5HYwn2ZwkZNfHsQ6//d5q40MDgGYJMKBXOZP62ZHhct1XsvYBmtcUtIOm5j2HSjt2pjEuGAc1LbyGIWRJJQ3Lp1ULbL2m71I+P23GF36JyOM8SUWvpvxE/3+qqVhRFPG2VqMCYa2kLLxwVfUmpV+KKjX1TXsrq6pfJIwHNwHw4h7MSfD8xTy2bx4MiBt638Z9Vt1pGsSQkh9RgPvCbnhuCpZQloUgJ8ByLVEcrlKx3yaaxiQXvte+ZhuOI7rGdjmoVoOzisooje4JgYw==".parse().unwrap();
let enc_user_key = "2.Q/2PhzcC7GdeiMHhWguYAQ==|GpqzVdr0go0ug5cZh1n+uixeBC3oC90CIe0hd/HWA/pTRDZ8ane4fmsEIcuc8eMKUt55Y2q/fbNzsYu41YTZzzsJUSeqVjT8/iTQtgnNdpo=|dwI+uyvZ1h/iZ03VQ+/wrGEFYVewBUUl/syYgjsNMbE=".parse().unwrap();
let dec = auth_request_decrypt_master_key(
SensitiveString::new(Box::new(private_key.to_owned())),
enc_master_key,
enc_user_key,
)
.unwrap();
let dec =
auth_request_decrypt_master_key(private_key.to_owned(), enc_master_key, enc_user_key)
.unwrap();

assert_eq!(
dec.to_vec(),
Expand Down
4 changes: 2 additions & 2 deletions crates/bitwarden/src/auth/login/auth_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use bitwarden_api_api::{
apis::auth_requests_api::{auth_requests_id_response_get, auth_requests_post},
models::{AuthRequestCreateRequestModel, AuthRequestType},
};
use bitwarden_crypto::{Kdf, SensitiveString};
use bitwarden_crypto::Kdf;
use uuid::Uuid;

use crate::{
Expand All @@ -22,7 +22,7 @@ pub struct NewAuthRequestResponse {
device_identifier: String,
auth_request_id: Uuid,
access_code: String,
private_key: SensitiveString,
private_key: String,
}

pub(crate) async fn send_new_auth_request(
Expand Down
10 changes: 6 additions & 4 deletions crates/bitwarden/src/auth/tde.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use base64::{engine::general_purpose::STANDARD, Engine};
use base64::engine::general_purpose::STANDARD;
use bitwarden_crypto::{
AsymmetricEncString, AsymmetricPublicCryptoKey, DeviceKey, EncString, Kdf, SymmetricCryptoKey,
TrustDeviceResponse, UserKey,
AsymmetricEncString, AsymmetricPublicCryptoKey, DeviceKey, EncString, Kdf, SensitiveString,
SymmetricCryptoKey, TrustDeviceResponse, UserKey,
};

use crate::{error::Result, Client};
Expand All @@ -15,7 +15,9 @@ pub(super) fn make_register_tde_keys(
org_public_key: String,
remember_device: bool,
) -> Result<RegisterTdeKeyResponse> {
let public_key = AsymmetricPublicCryptoKey::from_der(&STANDARD.decode(org_public_key)?)?;
let public_key = AsymmetricPublicCryptoKey::from_der(
SensitiveString::new(Box::new(org_public_key)).decode_base64(STANDARD)?,
)?;

let mut rng = rand::thread_rng();

Expand Down
7 changes: 4 additions & 3 deletions crates/bitwarden/src/mobile/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ pub enum InitUserCryptoMethod {
},
AuthRequest {
/// Private Key generated by the `crate::auth::new_auth_request`.
request_private_key: SensitiveString,
request_private_key: String,

method: AuthRequestMethod,
},
Expand Down Expand Up @@ -306,10 +306,11 @@ pub(super) fn enroll_admin_password_reset(
client: &mut Client,
public_key: String,
) -> Result<AsymmetricEncString> {
use base64::{engine::general_purpose::STANDARD, Engine};
use base64::engine::general_purpose::STANDARD;
use bitwarden_crypto::AsymmetricPublicCryptoKey;

let public_key = AsymmetricPublicCryptoKey::from_der(STANDARD.decode(public_key)?.as_slice())?;
let public_key = SensitiveString::new(Box::new(public_key));
let public_key = AsymmetricPublicCryptoKey::from_der(public_key.decode_base64(STANDARD)?)?;
let enc = client.get_encryption_settings()?;
let key = enc.get_key(&None).ok_or(Error::VaultLocked)?;

Expand Down

0 comments on commit df99792

Please sign in to comment.