[PM-9540] Trim and lowercase email before using as salt (#883)

We should trim and convert emails to lowercase prior to using them as salt for the login and registration methods. Which matches the behaviour of our existing applications, https://github.com/bitwarden/clients/blob/main/apps/cli/src/auth/commands/login.command.ts#L575.
bitwarden · Jul 8, 2024 · 70ec880 · 70ec880
1 parent 8180ba7
commit 70ec880
Show file tree

Hide file tree

Showing 16 changed files with 129 additions and 97 deletions.
diff --git a/crates/bitwarden-core/src/auth/auth_request.rs b/crates/bitwarden-core/src/auth/auth_request.rs
@@ -115,7 +115,7 @@ fn test_auth_request() {
 mod tests {
     use std::num::NonZeroU32;
 
-    use bitwarden_crypto::Kdf;
+    use bitwarden_crypto::{Kdf, MasterKey};
 
     use super::*;
     use crate::mobile::crypto::{AuthRequestMethod, InitUserCryptoMethod, InitUserCryptoRequest};
@@ -124,9 +124,9 @@ mod tests {
     fn test_approve() {
         let client = Client::new(None);
 
-        let master_key = bitwarden_crypto::MasterKey::derive(
-            "asdfasdfasdf".as_bytes(),
-            "[email protected]".as_bytes(),
+        let master_key = MasterKey::derive(
+            "asdfasdfasdf",
+            "[email protected]",
             &Kdf::PBKDF2 {
                 iterations: NonZeroU32::new(600_000).unwrap(),
             },
@@ -200,9 +200,7 @@ mod tests {
         // Initialize an existing client which is unlocked
         let existing_device = Client::new(None);
 
-        let master_key =
-            bitwarden_crypto::MasterKey::derive("asdfasdfasdf".as_bytes(), email.as_bytes(), &kdf)
-                .unwrap();
+        let master_key = MasterKey::derive("asdfasdfasdf", email, &kdf).unwrap();
 
         existing_device
             .internal

diff --git a/crates/bitwarden-core/src/auth/login/api_key.rs b/crates/bitwarden-core/src/auth/login/api_key.rs
@@ -38,7 +38,7 @@ pub(crate) async fn login_api_key(
             r.expires_in,
         );
 
-        let master_key = MasterKey::derive(input.password.as_bytes(), email.as_bytes(), &kdf)?;
+        let master_key = MasterKey::derive(&input.password, &email, &kdf)?;
 
         client
             .internal

diff --git a/crates/bitwarden-core/src/auth/login/password.rs b/crates/bitwarden-core/src/auth/login/password.rs
@@ -30,11 +30,7 @@ pub(crate) async fn login_password(
 
     info!("password logging in");
 
-    let master_key = MasterKey::derive(
-        input.password.as_bytes(),
-        input.email.as_bytes(),
-        &input.kdf,
-    )?;
+    let master_key = MasterKey::derive(&input.password, &input.email, &input.kdf)?;
     let password_hash = master_key
         .derive_master_key_hash(input.password.as_bytes(), HashPurpose::ServerAuthorization)?;
 

diff --git a/crates/bitwarden-core/src/auth/mod.rs b/crates/bitwarden-core/src/auth/mod.rs
@@ -39,7 +39,7 @@ fn determine_password_hash(
     password: &str,
     purpose: HashPurpose,
 ) -> Result<String> {
-    let master_key = MasterKey::derive(password.as_bytes(), email.as_bytes(), kdf)?;
+    let master_key = MasterKey::derive(password, email, kdf)?;
     Ok(master_key.derive_master_key_hash(password.as_bytes(), purpose)?)
 }
 

diff --git a/crates/bitwarden-core/src/auth/password/validate.rs b/crates/bitwarden-core/src/auth/password/validate.rs
@@ -56,7 +56,7 @@ pub(crate) fn validate_password_user_key(
         match login_method {
             UserLoginMethod::Username { email, kdf, .. }
             | UserLoginMethod::ApiKey { email, kdf, .. } => {
-                let master_key = MasterKey::derive(password.as_bytes(), email.as_bytes(), kdf)?;
+                let master_key = MasterKey::derive(&password, email, kdf)?;
                 let user_key = master_key
                     .decrypt_user_key(encrypted_user_key.parse()?)
                     .map_err(|_| "wrong password")?;
@@ -117,7 +117,7 @@ mod tests {
     fn test_validate_password_user_key() {
         use std::num::NonZeroU32;
 
-        use bitwarden_crypto::Kdf;
+        use bitwarden_crypto::{Kdf, MasterKey};
 
         use crate::client::{Client, LoginMethod, UserLoginMethod};
 
@@ -137,9 +137,7 @@ mod tests {
                 client_id: "1".to_string(),
             }));
 
-        let master_key =
-            bitwarden_crypto::MasterKey::derive(password.as_bytes(), email.as_bytes(), &kdf)
-                .unwrap();
+        let master_key = MasterKey::derive(password, email, &kdf).unwrap();
 
         let user_key = "2.Q/2PhzcC7GdeiMHhWguYAQ==|GpqzVdr0go0ug5cZh1n+uixeBC3oC90CIe0hd/HWA/pTRDZ8ane4fmsEIcuc8eMKUt55Y2q/fbNzsYu41YTZzzsJUSeqVjT8/iTQtgnNdpo=|dwI+uyvZ1h/iZ03VQ+/wrGEFYVewBUUl/syYgjsNMbE=";
         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=".parse().unwrap();
@@ -162,13 +160,13 @@ mod tests {
     fn test_validate_password_user_key_wrong_password() {
         use std::num::NonZeroU32;
 
-        use bitwarden_crypto::Kdf;
+        use bitwarden_crypto::{Kdf, MasterKey};
 
         use crate::client::{Client, LoginMethod, UserLoginMethod};
 
         let client = Client::new(None);
 
-        let password = b"asdfasdfasdf";
+        let password = "asdfasdfasdf";
         let email = "[email protected]";
         let kdf = Kdf::PBKDF2 {
             iterations: NonZeroU32::new(600_000).unwrap(),
@@ -182,8 +180,7 @@ mod tests {
                 client_id: "1".to_string(),
             }));
 
-        let master_key =
-            bitwarden_crypto::MasterKey::derive(password, email.as_bytes(), &kdf).unwrap();
+        let master_key = MasterKey::derive(password, email, &kdf).unwrap();
 
         let user_key = "2.Q/2PhzcC7GdeiMHhWguYAQ==|GpqzVdr0go0ug5cZh1n+uixeBC3oC90CIe0hd/HWA/pTRDZ8ane4fmsEIcuc8eMKUt55Y2q/fbNzsYu41YTZzzsJUSeqVjT8/iTQtgnNdpo=|dwI+uyvZ1h/iZ03VQ+/wrGEFYVewBUUl/syYgjsNMbE=";
         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=".parse().unwrap();

diff --git a/crates/bitwarden-core/src/auth/register.rs b/crates/bitwarden-core/src/auth/register.rs
@@ -57,7 +57,7 @@ pub(super) fn make_register_keys(
     password: String,
     kdf: Kdf,
 ) -> Result<RegisterKeyResponse> {
-    let master_key = MasterKey::derive(password.as_bytes(), email.as_bytes(), &kdf)?;
+    let master_key = MasterKey::derive(&password, &email, &kdf)?;
     let master_password_hash =
         master_key.derive_master_key_hash(password.as_bytes(), HashPurpose::ServerAuthorization)?;
     let (user_key, encrypted_user_key) = master_key.make_user_key()?;

diff --git a/crates/bitwarden-core/src/client/internal.rs b/crates/bitwarden-core/src/client/internal.rs
@@ -3,7 +3,7 @@ use std::sync::{Arc, RwLock};
 #[cfg(any(feature = "internal", feature = "secrets"))]
 use bitwarden_crypto::SymmetricCryptoKey;
 #[cfg(feature = "internal")]
-use bitwarden_crypto::{AsymmetricEncString, EncString, Kdf, MasterKey};
+use bitwarden_crypto::{AsymmetricEncString, EncString, Kdf, MasterKey, PinKey};
 use chrono::Utc;
 use uuid::Uuid;
 
@@ -213,7 +213,7 @@ impl InternalClient {
     #[cfg(feature = "internal")]
     pub(crate) fn initialize_user_crypto_pin(
         &self,
-        pin_key: MasterKey,
+        pin_key: PinKey,
         pin_protected_user_key: EncString,
         private_key: EncString,
     ) -> Result<()> {

diff --git a/crates/bitwarden-core/src/mobile/client_kdf.rs b/crates/bitwarden-core/src/mobile/client_kdf.rs
@@ -3,7 +3,7 @@ use bitwarden_crypto::{HashPurpose, Kdf};
 use crate::{error::Result, mobile::kdf::hash_password, Client};
 
 pub struct ClientKdf<'a> {
-    pub(crate) client: &'a crate::Client,
+    pub(crate) _client: &'a crate::Client,
 }
 
 impl<'a> ClientKdf<'a> {
@@ -14,12 +14,12 @@ impl<'a> ClientKdf<'a> {
         kdf_params: Kdf,
         purpose: HashPurpose,
     ) -> Result<String> {
-        hash_password(self.client, email, password, kdf_params, purpose).await
+        hash_password(email, password, kdf_params, purpose).await
     }
 }
 
 impl<'a> Client {
     pub fn kdf(&'a self) -> ClientKdf<'a> {
-        ClientKdf { client: self }
+        ClientKdf { _client: self }
     }
 }
diff --git a/crates/bitwarden-core/src/mobile/crypto.rs b/crates/bitwarden-core/src/mobile/crypto.rs
@@ -85,7 +85,7 @@ pub enum AuthRequestMethod {
 
 #[cfg(feature = "internal")]
 pub async fn initialize_user_crypto(client: &Client, req: InitUserCryptoRequest) -> Result<()> {
-    use bitwarden_crypto::DeviceKey;
+    use bitwarden_crypto::{DeviceKey, PinKey};
 
     use crate::auth::{auth_request_decrypt_master_key, auth_request_decrypt_user_key};
 
@@ -95,8 +95,7 @@ pub async fn initialize_user_crypto(client: &Client, req: InitUserCryptoRequest)
         InitUserCryptoMethod::Password { password, user_key } => {
             let user_key: EncString = user_key.parse()?;
 
-            let master_key =
-                MasterKey::derive(password.as_bytes(), req.email.as_bytes(), &req.kdf_params)?;
+            let master_key = MasterKey::derive(&password, &req.email, &req.kdf_params)?;
             client
                 .internal
                 .initialize_user_crypto_master_key(master_key, user_key, private_key)?;
@@ -111,7 +110,7 @@ pub async fn initialize_user_crypto(client: &Client, req: InitUserCryptoRequest)
             pin,
             pin_protected_user_key,
         } => {
-            let pin_key = MasterKey::derive(pin.as_bytes(), req.email.as_bytes(), &req.kdf_params)?;
+            let pin_key = PinKey::derive(pin.as_bytes(), req.email.as_bytes(), &req.kdf_params)?;
             client.internal.initialize_user_crypto_pin(
                 pin_key,
                 pin_protected_user_key,
@@ -216,7 +215,7 @@ pub fn update_password(client: &Client, new_password: String) -> Result<UpdatePa
         LoginMethod::User(
             UserLoginMethod::Username { email, kdf, .. }
             | UserLoginMethod::ApiKey { email, kdf, .. },
-        ) => MasterKey::derive(new_password.as_bytes(), email.as_bytes(), kdf)?,
+        ) => MasterKey::derive(&new_password, email, kdf)?,
         #[cfg(feature = "secrets")]
         LoginMethod::ServiceAccount(_) => return Err(Error::NotAuthenticated),
     };
@@ -283,11 +282,13 @@ fn derive_pin_protected_user_key(
     login_method: &LoginMethod,
     user_key: &SymmetricCryptoKey,
 ) -> Result<EncString> {
+    use bitwarden_crypto::PinKey;
+
     let derived_key = match login_method {
         LoginMethod::User(
             UserLoginMethod::Username { email, kdf, .. }
             | UserLoginMethod::ApiKey { email, kdf, .. },
-        ) => MasterKey::derive(pin.as_bytes(), email.as_bytes(), kdf)?,
+        ) => PinKey::derive(pin.as_bytes(), email.as_bytes(), kdf)?,
         #[cfg(feature = "secrets")]
         LoginMethod::ServiceAccount(_) => return Err(Error::NotAuthenticated),
     };
@@ -504,9 +505,9 @@ mod tests {
 
         let client = Client::new(None);
 
-        let master_key = bitwarden_crypto::MasterKey::derive(
-            "asdfasdfasdf".as_bytes(),
-            "[email protected]".as_bytes(),
+        let master_key = MasterKey::derive(
+            "asdfasdfasdf",
+            "[email protected]",
             &Kdf::PBKDF2 {
                 iterations: NonZeroU32::new(600_000).unwrap(),
             },

diff --git a/crates/bitwarden-core/src/mobile/kdf.rs b/crates/bitwarden-core/src/mobile/kdf.rs
@@ -1,15 +1,14 @@
 use bitwarden_crypto::{HashPurpose, Kdf, MasterKey};
 
-use crate::{error::Result, Client};
+use crate::error::Result;
 
 pub async fn hash_password(
-    _client: &Client,
     email: String,
     password: String,
     kdf_params: Kdf,
     purpose: HashPurpose,
 ) -> Result<String> {
-    let master_key = MasterKey::derive(password.as_bytes(), email.as_bytes(), &kdf_params)?;
+    let master_key = MasterKey::derive(&password, &email, &kdf_params)?;
 
     Ok(master_key.derive_master_key_hash(password.as_bytes(), purpose)?)
 }
diff --git a/crates/bitwarden-core/src/platform/generate_fingerprint.rs b/crates/bitwarden-core/src/platform/generate_fingerprint.rs
@@ -54,7 +54,7 @@ pub(crate) fn generate_user_fingerprint(
 mod tests {
     use std::num::NonZeroU32;
 
-    use bitwarden_crypto::Kdf;
+    use bitwarden_crypto::{Kdf, MasterKey};
 
     use super::*;
     use crate::Client;
@@ -67,9 +67,9 @@ mod tests {
 
         let client = Client::new(None);
 
-        let master_key = bitwarden_crypto::MasterKey::derive(
-            "asdfasdfasdf".as_bytes(),
-            "[email protected]".as_bytes(),
+        let master_key = MasterKey::derive(
+            "asdfasdfasdf",
+            "[email protected]",
             &Kdf::PBKDF2 {
                 iterations: NonZeroU32::new(600_000).unwrap(),
             },

diff --git a/crates/bitwarden-core/src/platform/get_user_api_key.rs b/crates/bitwarden-core/src/platform/get_user_api_key.rs
@@ -52,7 +52,7 @@ fn get_secret_verification_request(
             .master_password
             .as_ref()
             .map(|p| {
-                let master_key = MasterKey::derive(p.as_bytes(), email.as_bytes(), kdf)?;
+                let master_key = MasterKey::derive(p, email, kdf)?;
 
                 master_key.derive_master_key_hash(p.as_bytes(), HashPurpose::ServerAuthorization)
             })