From 27a71d899284da50e9a08ca87a0068f991dafe44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa?= Date: Fri, 19 Jul 2024 14:40:18 +0200 Subject: [PATCH] [PM-9527] Add PIN validation support (#912) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 🎟ī¸ Tracking https://bitwarden.atlassian.net/browse/PM-9527 ## 📔 Objective Fede mentioned on Slack some time ago that we currently don't have a simple way to validate the user's PIN like we do with the password. Instead the way to do it was to call `init_crypto` and check that the result is not an error. This requires more data than needed and does a lot of unnecessary operations, so I think it makes sense to expose a simpler way to do PIN verification. In this PR we're decrypting the PIN protected user key and comparing it with the stored user key, similar to what we're doing with `validate_password_user_key`. ## ⏰ Reminders before review - Contributor guidelines followed - All formatters and local linters executed and passed - Written new unit and / or integration tests where applicable - Protected functional changes with optionality (feature flags) - Used internationalization (i18n) for all UI strings - CI builds passed - Communicated to DevOps any deployment requirements - Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team ## đŸĻŽ Reviewer guidelines - 👍 (`:+1:`) or similar for great changes - 📝 (`:memo:`) or ℹī¸ (`:information_source:`) for notes or general info - ❓ (`:question:`) for questions - 🤔 (`:thinking:`) or 💭 (`:thought_balloon:`) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion - 🎨 (`:art:`) for suggestions / improvements - ❌ (`:x:`) or ⚠ī¸ (`:warning:`) for more significant problems or concerns needing attention - 🌱 (`:seedling:`) or â™ģī¸ (`:recycle:`) for future improvements or indications of technical debt - ⛏ (`:pick:`) for minor or nitpick changes --- crates/bitwarden-core/src/auth/client_auth.rs | 7 +- crates/bitwarden-core/src/auth/mod.rs | 2 + crates/bitwarden-core/src/auth/pin.rs | 101 ++++++++++++++++++ crates/bitwarden-uniffi/src/auth/mod.rs | 13 ++- 4 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 crates/bitwarden-core/src/auth/pin.rs diff --git a/crates/bitwarden-core/src/auth/client_auth.rs b/crates/bitwarden-core/src/auth/client_auth.rs index 1afc39c8c..6f1afb135 100644 --- a/crates/bitwarden-core/src/auth/client_auth.rs +++ b/crates/bitwarden-core/src/auth/client_auth.rs @@ -1,5 +1,5 @@ #[cfg(feature = "internal")] -use bitwarden_crypto::{AsymmetricEncString, DeviceKey, Kdf, TrustDeviceResponse}; +use bitwarden_crypto::{AsymmetricEncString, DeviceKey, EncString, Kdf, TrustDeviceResponse}; #[cfg(feature = "internal")] use crate::auth::login::NewAuthRequestResponse; @@ -16,6 +16,7 @@ use crate::auth::{ password_strength, satisfies_policy, validate_password, validate_password_user_key, MasterPasswordPolicyOptions, }, + pin::validate_pin, register::{make_register_keys, register}, tde::{make_register_tde_keys, RegisterTdeKeyResponse}, AuthRequestResponse, RegisterKeyResponse, RegisterRequest, @@ -116,6 +117,10 @@ impl<'a> ClientAuth<'a> { validate_password_user_key(self.client, password, encrypted_user_key) } + pub fn validate_pin(&self, pin: String, pin_protected_user_key: EncString) -> Result { + validate_pin(self.client, pin, pin_protected_user_key) + } + pub fn new_auth_request(&self, email: &str) -> Result { new_auth_request(email) } diff --git a/crates/bitwarden-core/src/auth/mod.rs b/crates/bitwarden-core/src/auth/mod.rs index 5b3d615e6..e52870570 100644 --- a/crates/bitwarden-core/src/auth/mod.rs +++ b/crates/bitwarden-core/src/auth/mod.rs @@ -8,6 +8,8 @@ mod jwt_token; pub mod login; #[cfg(feature = "internal")] pub mod password; +#[cfg(feature = "internal")] +pub mod pin; pub mod renew; pub use access_token::AccessToken; pub use jwt_token::JWTToken; diff --git a/crates/bitwarden-core/src/auth/pin.rs b/crates/bitwarden-core/src/auth/pin.rs new file mode 100644 index 000000000..ee093bfa6 --- /dev/null +++ b/crates/bitwarden-core/src/auth/pin.rs @@ -0,0 +1,101 @@ +use bitwarden_crypto::{EncString, PinKey}; + +use crate::{ + client::{LoginMethod, UserLoginMethod}, + error::{Error, Result}, + Client, +}; + +pub(crate) fn validate_pin( + client: &Client, + pin: String, + pin_protected_user_key: EncString, +) -> Result { + let login_method = client + .internal + .get_login_method() + .ok_or(Error::NotAuthenticated)?; + + #[allow(irrefutable_let_patterns)] + let LoginMethod::User(login_method) = login_method.as_ref() else { + return Err(Error::NotAuthenticated); + }; + + match login_method { + UserLoginMethod::Username { email, kdf, .. } + | UserLoginMethod::ApiKey { email, kdf, .. } => { + let enc = client.internal.get_encryption_settings()?; + let user_key = enc.get_key(&None)?; + + let pin_key = PinKey::derive(pin.as_bytes(), email.as_bytes(), kdf)?; + + let Ok(decrypted_key) = pin_key.decrypt_user_key(pin_protected_user_key) else { + return Ok(false); + }; + + Ok(user_key.to_vec() == decrypted_key.to_vec()) + } + } +} + +#[cfg(test)] +mod tests { + use std::num::NonZeroU32; + + use bitwarden_crypto::{Kdf, MasterKey}; + + use super::*; + use crate::client::{Client, LoginMethod, UserLoginMethod}; + + fn init_client() -> Client { + let client = Client::new(None); + + let password = "asdfasdfasdf"; + let email = "test@bitwarden.com"; + let kdf = Kdf::PBKDF2 { + iterations: NonZeroU32::new(600_000).unwrap(), + }; + + client + .internal + .set_login_method(LoginMethod::User(UserLoginMethod::Username { + email: email.to_string(), + kdf: kdf.clone(), + client_id: "1".to_string(), + })); + + 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(); + + client + .internal + .initialize_user_crypto_master_key(master_key, user_key.parse().unwrap(), private_key) + .unwrap(); + + client + } + + #[test] + fn test_validate_valid_pin() { + let pin = "1234".to_string(); + let pin_protected_user_key = "2.BXgvdBUeEMyvumqAJkAzPA==|JScDPoqOkVdrC1X755Ubt8tS9pC/thvrvNf5CyNcRg8HZtZ466EcRo7aCqwUzLyTVNRkbCYtFYT+09acGGHur8tGuS7Kmg/pYeaUo4K0UKI=|NpIFg5P9z0SN1MffbixD9OQE0l+NiNmnRQJs/kTsyoQ=" + .parse() + .unwrap(); + + let client = init_client(); + assert!(validate_pin(&client, pin.clone(), pin_protected_user_key).unwrap()); + } + + #[test] + fn test_validate_invalid_pin() { + let pin = "1234".to_string(); + let pin_protected_user_key = "2.BXgvdBUeEMyvumqAJkAyPA==|JScDPoqOkVdrC1X755Ubt8tS9pC/thvrvNf5CyNcRg8HZtZ466EcRo7aCqwUzLyTVNRkbCYtFYT+09acGGHur8tGuS7Kmg/pYeaUo4K0UKI=|NpIFg5P9z0SN1MffbixD9OQE0l+NiNmnRQJs/kTsyoQ=" + .parse() + .unwrap(); + + let client = init_client(); + assert!(!validate_pin(&client, pin.clone(), pin_protected_user_key).unwrap()); + } +} diff --git a/crates/bitwarden-uniffi/src/auth/mod.rs b/crates/bitwarden-uniffi/src/auth/mod.rs index a70eebb8f..644580c5d 100644 --- a/crates/bitwarden-uniffi/src/auth/mod.rs +++ b/crates/bitwarden-uniffi/src/auth/mod.rs @@ -4,7 +4,7 @@ use bitwarden::auth::{ password::MasterPasswordPolicyOptions, AuthRequestResponse, RegisterKeyResponse, RegisterTdeKeyResponse, }; -use bitwarden_crypto::{AsymmetricEncString, HashPurpose, Kdf, TrustDeviceResponse}; +use bitwarden_crypto::{AsymmetricEncString, EncString, HashPurpose, Kdf, TrustDeviceResponse}; use crate::{error::Result, Client}; @@ -110,6 +110,17 @@ impl ClientAuth { .validate_password_user_key(password, encrypted_user_key)?) } + /// Validate the user PIN + /// + /// To validate the user PIN, you need to have the user's pin_protected_user_key. This key is + /// obtained when enabling PIN unlock on the account with the `derive_pin_key` method. + /// + /// This works by comparing the decrypted user key with the current user key, so the client must + /// be unlocked. + pub fn validate_pin(&self, pin: String, pin_protected_user_key: EncString) -> Result { + Ok(self.0 .0.auth().validate_pin(pin, pin_protected_user_key)?) + } + /// Initialize a new auth request pub fn new_auth_request(&self, email: String) -> Result { Ok(self.0 .0.auth().new_auth_request(&email)?)