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

Fix make_user_key which previously didn't stretch the users key #315

Merged
merged 4 commits into from
Nov 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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.

1 change: 1 addition & 0 deletions crates/bitwarden/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,6 @@ bitwarden-api-identity = { path = "../bitwarden-api-identity", version = "=0.2.2
bitwarden-api-api = { path = "../bitwarden-api-api", version = "=0.2.2" }

[dev-dependencies]
rand_chacha = "0.3.1"
tokio = { version = "1.28.2", features = ["rt", "macros"] }
wiremock = "0.5.18"
6 changes: 3 additions & 3 deletions crates/bitwarden/src/auth/register.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ pub(super) fn make_register_keys(

#[cfg_attr(feature = "mobile", derive(uniffi::Record))]
pub struct RegisterKeyResponse {
master_password_hash: String,
encrypted_user_key: String,
keys: RsaKeyPair,
pub master_password_hash: String,
pub encrypted_user_key: String,
pub keys: RsaKeyPair,
}
10 changes: 5 additions & 5 deletions crates/bitwarden/src/client/client.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
use std::time::{Duration, Instant};

use reqwest::header::{self};
use uuid::Uuid;

#[cfg(feature = "secrets")]
use crate::auth::login::{access_token_login, AccessTokenLoginRequest, AccessTokenLoginResponse};
#[cfg(feature = "internal")]
use crate::{
auth::login::{
Expand All @@ -13,11 +18,6 @@ use crate::{
SecretVerificationRequest, SyncRequest, SyncResponse, UserApiKeyResponse,
},
};
use reqwest::header::{self};
use uuid::Uuid;

#[cfg(feature = "secrets")]
use crate::auth::login::{access_token_login, AccessTokenLoginRequest, AccessTokenLoginResponse};
use crate::{
auth::renew::renew_token,
client::{
Expand Down
75 changes: 65 additions & 10 deletions crates/bitwarden/src/crypto/master_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ use rand::Rng;
use sha2::Digest;

use super::{
encrypt_aes256, hkdf_expand, EncString, KeyDecryptable, PbkdfSha256Hmac, SymmetricCryptoKey,
UserKey, PBKDF_SHA256_HMAC_OUT_SIZE,
encrypt_aes256_hmac, hkdf_expand, EncString, KeyDecryptable, PbkdfSha256Hmac,
SymmetricCryptoKey, UserKey, PBKDF_SHA256_HMAC_OUT_SIZE,
};
use crate::{client::kdf::Kdf, error::Result, util::BASE64_ENGINE};

Expand Down Expand Up @@ -41,13 +41,7 @@ impl MasterKey {
}

pub(crate) fn make_user_key(&self) -> Result<(UserKey, EncString)> {
let mut user_key = [0u8; 64];
rand::thread_rng().fill(&mut user_key);

let protected = encrypt_aes256(&user_key, self.0.key)?;

let u: &[u8] = &user_key;
Ok((UserKey::new(SymmetricCryptoKey::try_from(u)?), protected))
make_user_key(rand::thread_rng(), self)
}

pub(crate) fn decrypt_user_key(&self, user_key: EncString) -> Result<SymmetricCryptoKey> {
Expand All @@ -58,6 +52,22 @@ impl MasterKey {
}
}

/// Generate a new random user key and encrypt it with the master key.
fn make_user_key(
mut rng: impl rand::RngCore,
master_key: &MasterKey,
) -> Result<(UserKey, EncString)> {
let mut user_key = [0u8; 64];
rng.fill(&mut user_key);

let stretched_key = stretch_master_key(master_key)?;
let protected =
encrypt_aes256_hmac(&user_key, stretched_key.mac_key.unwrap(), stretched_key.key)?;

let u: &[u8] = &user_key;
Ok((UserKey::new(SymmetricCryptoKey::try_from(u)?), protected))
}

/// Derive a generic key from a secret and salt using the provided KDF.
fn derive_key(secret: &[u8], salt: &[u8], kdf: &Kdf) -> Result<SymmetricCryptoKey> {
let hash = match kdf {
Expand Down Expand Up @@ -112,7 +122,9 @@ fn stretch_master_key(master_key: &MasterKey) -> Result<SymmetricCryptoKey> {
mod tests {
use std::num::NonZeroU32;

use super::{stretch_master_key, HashPurpose, MasterKey};
use rand::SeedableRng;

use super::{make_user_key, stretch_master_key, HashPurpose, MasterKey};
use crate::{client::kdf::Kdf, crypto::SymmetricCryptoKey};

#[test]
Expand Down Expand Up @@ -225,4 +237,47 @@ mod tests {
.unwrap(),
);
}

#[test]
fn test_make_user_key() {
let mut rng = rand_chacha::ChaCha8Rng::from_seed([0u8; 32]);

let master_key = MasterKey(SymmetricCryptoKey {
key: [
31, 79, 104, 226, 150, 71, 177, 90, 194, 80, 172, 209, 17, 129, 132, 81, 138, 167,
69, 167, 254, 149, 2, 27, 39, 197, 64, 42, 22, 195, 86, 75,
]
.into(),
mac_key: None,
});

let (user_key, protected) = make_user_key(&mut rng, &master_key).unwrap();

assert_eq!(
user_key.0.key.as_slice(),
[
62, 0, 239, 47, 137, 95, 64, 214, 127, 91, 184, 232, 31, 9, 165, 161, 44, 132, 14,
195, 206, 154, 127, 59, 24, 27, 225, 136, 239, 113, 26, 30
]
);
assert_eq!(
user_key.0.mac_key.unwrap().as_slice(),
[
152, 76, 225, 114, 185, 33, 111, 65, 159, 68, 83, 103, 69, 109, 86, 25, 49, 74, 66,
163, 218, 134, 176, 1, 56, 123, 253, 184, 14, 12, 254, 66
]
);

// Ensure we can decrypt the key and get back the same key
let decrypted = master_key.decrypt_user_key(protected).unwrap();

assert_eq!(
decrypted.key, user_key.0.key,
"Decrypted key doesn't match user key"
);
assert_eq!(
decrypted.mac_key, user_key.0.mac_key,
"Decrypted key doesn't match user key"
);
}
}
2 changes: 1 addition & 1 deletion crates/bitwarden/src/crypto/user_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{
error::Result,
};

pub(crate) struct UserKey(SymmetricCryptoKey);
pub(crate) struct UserKey(pub(super) SymmetricCryptoKey);

impl UserKey {
pub(crate) fn new(key: SymmetricCryptoKey) -> Self {
Expand Down
35 changes: 35 additions & 0 deletions crates/bitwarden/tests/register.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/// Integration test for registering a new user and unlocking the vault
#[cfg(feature = "mobile")]
#[tokio::test]
async fn test_register_initialize_crypto() {
use std::num::NonZeroU32;

use bitwarden::{client::kdf::Kdf, mobile::crypto::InitCryptoRequest, Client};

let mut client = Client::new(None);

let email = "[email protected]";
let password = "test123";
let kdf = Kdf::PBKDF2 {
iterations: NonZeroU32::new(600_000).unwrap(),
};

let register_response = client
.auth()
.make_register_keys(email.to_owned(), password.to_owned(), kdf.clone())
.unwrap();

// Ensure we can initialize the crypto with the new keys
client
.crypto()
.initialize_crypto(InitCryptoRequest {
kdf_params: kdf,
email: email.to_owned(),
password: password.to_owned(),
user_key: register_response.encrypted_user_key,
private_key: register_response.keys.private.to_string(),
organization_keys: Default::default(),
})
.await
.unwrap();
}