Skip to content

Commit

Permalink
Fix device keys incorrectly being treated as strings (#564)
Browse files Browse the repository at this point in the history
Device keys are generated from random u8 vectors. We incorrectly
attempted to parse these as strings when decrypting them in the last
trust device step.

Added a complete test which confirms the whole device approval process.
  • Loading branch information
Hinton authored Jan 31, 2024
1 parent 878cd4a commit 8b468ec
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 8 deletions.
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.

1 change: 1 addition & 0 deletions crates/bitwarden/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,4 @@ reqwest = { version = "*", features = [
rand_chacha = "0.3.1"
tokio = { version = "1.35.1", features = ["rt", "macros"] }
wiremock = "0.5.22"
zeroize = { version = ">=1.7.0, <2.0", features = ["derive", "aarch64"] }
86 changes: 78 additions & 8 deletions crates/bitwarden/src/auth/auth_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ pub(crate) fn auth_request_decrypt_user_key(
user_key: AsymmetricEncString,
) -> Result<SymmetricCryptoKey, Error> {
let key = AsymmetricCryptoKey::from_der(&STANDARD.decode(private_key)?)?;
let key: String = user_key.decrypt_with_key(&key)?;
let mut key: Vec<u8> = user_key.decrypt_with_key(&key)?;

Ok(key.parse()?)
Ok(SymmetricCryptoKey::try_from(key.as_mut_slice())?)
}

/// Approve an auth request.
Expand All @@ -83,20 +83,25 @@ pub(crate) fn approve_auth_request(

#[test]
fn test_auth_request() {
use zeroize::Zeroizing;

let request = new_auth_request("[email protected]").unwrap();

let secret =
"w2LO+nwV4oxwswVYCxlOfRUseXfvU03VzvKQHrqeklPgiMZrspUe6sOBToCnDn9Ay0tuCBn8ykVVRb7PWhub2Q==";
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 =
AsymmetricCryptoKey::from_der(&STANDARD.decode(&request.private_key).unwrap()).unwrap();

let encrypted =
AsymmetricEncString::encrypt_rsa2048_oaep_sha1(secret.as_bytes(), &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_base64(), secret);
assert_eq!(decrypted.to_vec(), Zeroizing::new(secret.to_owned()));
}

#[cfg(test)]
Expand All @@ -106,7 +111,10 @@ mod tests {
use bitwarden_crypto::Kdf;

use super::*;
use crate::client::{LoginMethod, UserLoginMethod};
use crate::{
client::{LoginMethod, UserLoginMethod},
mobile::crypto::{InitUserCryptoMethod, InitUserCryptoRequest},
};

#[test]
fn test_approve() {
Expand Down Expand Up @@ -134,4 +142,66 @@ mod tests {

approve_auth_request(&mut client, public_key.to_owned()).unwrap();
}

#[tokio::test]
async fn test_device_login() {
let kdf = Kdf::PBKDF2 {
iterations: NonZeroU32::new(600_000).unwrap(),
};
let email = "[email protected]";

let user_key = "2.Q/2PhzcC7GdeiMHhWguYAQ==|GpqzVdr0go0ug5cZh1n+uixeBC3oC90CIe0hd/HWA/pTRDZ8ane4fmsEIcuc8eMKUt55Y2q/fbNzsYu41YTZzzsJUSeqVjT8/iTQtgnNdpo=|dwI+uyvZ1h/iZ03VQ+/wrGEFYVewBUUl/syYgjsNMbE=".parse().unwrap();
let private_key = "2.yN7l00BOlUE0Sb0M//Q53w==|EwKG/BduQRQ33Izqc/ogoBROIoI5dmgrxSo82sgzgAMIBt3A2FZ9vPRMY+GWT85JiqytDitGR3TqwnFUBhKUpRRAq4x7rA6A1arHrFp5Tp1p21O3SfjtvB3quiOKbqWk6ZaU1Np9HwqwAecddFcB0YyBEiRX3VwF2pgpAdiPbSMuvo2qIgyob0CUoC/h4Bz1be7Qa7B0Xw9/fMKkB1LpOm925lzqosyMQM62YpMGkjMsbZz0uPopu32fxzDWSPr+kekNNyLt9InGhTpxLmq1go/pXR2uw5dfpXc5yuta7DB0EGBwnQ8Vl5HPdDooqOTD9I1jE0mRyuBpWTTI3FRnu3JUh3rIyGBJhUmHqGZvw2CKdqHCIrQeQkkEYqOeJRJVdBjhv5KGJifqT3BFRwX/YFJIChAQpebNQKXe/0kPivWokHWwXlDB7S7mBZzhaAPidZvnuIhalE2qmTypDwHy22FyqV58T8MGGMchcASDi/QXI6kcdpJzPXSeU9o+NC68QDlOIrMVxKFeE7w7PvVmAaxEo0YwmuAzzKy9QpdlK0aab/xEi8V4iXj4hGepqAvHkXIQd+r3FNeiLfllkb61p6WTjr5urcmDQMR94/wYoilpG5OlybHdbhsYHvIzYoLrC7fzl630gcO6t4nM24vdB6Ymg9BVpEgKRAxSbE62Tqacxqnz9AcmgItb48NiR/He3n3ydGjPYuKk/ihZMgEwAEZvSlNxYONSbYrIGDtOY+8Nbt6KiH3l06wjZW8tcmFeVlWv+tWotnTY9IqlAfvNVTjtsobqtQnvsiDjdEVtNy/s2ci5TH+NdZluca2OVEr91Wayxh70kpM6ib4UGbfdmGgCo74gtKvKSJU0rTHakQ5L9JlaSDD5FamBRyI0qfL43Ad9qOUZ8DaffDCyuaVyuqk7cz9HwmEmvWU3VQ+5t06n/5kRDXttcw8w+3qClEEdGo1KeENcnXCB32dQe3tDTFpuAIMLqwXs6FhpawfZ5kPYvLPczGWaqftIs/RXJ/EltGc0ugw2dmTLpoQhCqrcKEBDoYVk0LDZKsnzitOGdi9mOWse7Se8798ib1UsHFUjGzISEt6upestxOeupSTOh0v4+AjXbDzRUyogHww3V+Bqg71bkcMxtB+WM+pn1XNbVTyl9NR040nhP7KEf6e9ruXAtmrBC2ah5cFEpLIot77VFZ9ilLuitSz+7T8n1yAh1IEG6xxXxninAZIzi2qGbH69O5RSpOJuJTv17zTLJQIIc781JwQ2TTwTGnx5wZLbffhCasowJKd2EVcyMJyhz6ru0PvXWJ4hUdkARJs3Xu8dus9a86N8Xk6aAPzBDqzYb1vyFIfBxP0oO8xFHgd30Cgmz8UrSE3qeWRrF8ftrI6xQnFjHBGWD/JWSvd6YMcQED0aVuQkuNW9ST/DzQThPzRfPUoiL10yAmV7Ytu4fR3x2sF0Yfi87YhHFuCMpV/DsqxmUizyiJuD938eRcH8hzR/VO53Qo3UIsqOLcyXtTv6THjSlTopQ+JOLOnHm1w8dzYbLN44OG44rRsbihMUQp+wUZ6bsI8rrOnm9WErzkbQFbrfAINdoCiNa6cimYIjvvnMTaFWNymqY1vZxGztQiMiHiHYwTfwHTXrb9j0uPM=|09J28iXv9oWzYtzK2LBT6Yht4IT4MijEkk0fwFdrVQ4=";

// Initialize an existing client which is unlocked
let mut existing_device = Client::new(None);
existing_device.set_login_method(LoginMethod::User(UserLoginMethod::Username {
client_id: "123".to_owned(),
email: email.to_owned(),
kdf: kdf.clone(),
}));

existing_device
.initialize_user_crypto("asdfasdfasdf", user_key, private_key.parse().unwrap())
.unwrap();

// Initialize a new device which will request to be logged in
let mut new_device = Client::new(None);

// Initialize an auth request, and approve it on the existing device
let auth_req = new_auth_request(email).unwrap();
let approved_req = approve_auth_request(&mut existing_device, auth_req.public_key).unwrap();

// Unlock the vault using the approved request
new_device
.crypto()
.initialize_user_crypto(InitUserCryptoRequest {
kdf_params: kdf,
email: email.to_owned(),
private_key: private_key.to_owned(),
method: InitUserCryptoMethod::AuthRequest {
request_private_key: auth_req.private_key,
protected_user_key: approved_req,
},
})
.await
.unwrap();

// We can validate that the vault is unlocked correctly by confirming the user key is the
// same
assert_eq!(
existing_device
.get_encryption_settings()
.unwrap()
.get_key(&None)
.unwrap()
.to_base64(),
new_device
.get_encryption_settings()
.unwrap()
.get_key(&None)
.unwrap()
.to_base64()
);
}
}

0 comments on commit 8b468ec

Please sign in to comment.