Skip to content

Commit

Permalink
Add sensitive to password strength and password policy
Browse files Browse the repository at this point in the history
  • Loading branch information
dani-garcia committed Apr 30, 2024
1 parent faa3444 commit 817cbe3
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 21 deletions.
25 changes: 25 additions & 0 deletions crates/bitwarden-crypto/src/sensitive/sensitive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,31 @@ impl SensitiveString {

Ok(value)
}

#[inline(always)]
pub fn len(&self) -> usize {
self.value.len()
}

Check warning on line 122 in crates/bitwarden-crypto/src/sensitive/sensitive.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-crypto/src/sensitive/sensitive.rs#L120-L122

Added lines #L120 - L122 were not covered by tests

#[inline(always)]
pub fn is_empty(&self) -> bool {
self.value.is_empty()
}

Check warning on line 127 in crates/bitwarden-crypto/src/sensitive/sensitive.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-crypto/src/sensitive/sensitive.rs#L125-L127

Added lines #L125 - L127 were not covered by tests

// The predicate is specifically a fn() and not a closure to forbid capturing values
// from the environment, which would make it easier to accidentally leak some data.
// For example, the following won't compile with fn() but would work with impl Fn():
// ```
// let mut chars = Mutex::new(Vec::new());
// self.any_chars(|c| {chars.lock().unwrap().push(c); true});
// ```
// Note that this is not a perfect solution, as it is still possible to leak the characters by
// using a global variable or a static variable. Also `char` implements Copy so it's hard to
// ensure the compiler is not making a copy of the character.
#[inline(always)]
pub fn any_chars(&self, predicate: fn(char) -> bool) -> bool {
self.value.chars().any(predicate)
}
}

impl<T: Zeroize + AsRef<[u8]>> Sensitive<T> {
Expand Down
4 changes: 2 additions & 2 deletions crates/bitwarden-uniffi/src/auth/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ impl ClientAuth {
/// **API Draft:** Calculate Password Strength
pub async fn password_strength(
&self,
password: String,
password: SensitiveString,

Check warning on line 21 in crates/bitwarden-uniffi/src/auth/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-uniffi/src/auth/mod.rs#L21

Added line #L21 was not covered by tests
email: String,
additional_inputs: Vec<String>,
) -> u8 {
Expand All @@ -34,7 +34,7 @@ impl ClientAuth {
/// Evaluate if the provided password satisfies the provided policy
pub async fn satisfies_policy(
&self,
password: String,
password: SensitiveString,

Check warning on line 37 in crates/bitwarden-uniffi/src/auth/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-uniffi/src/auth/mod.rs#L37

Added line #L37 was not covered by tests
strength: u8,
policy: MasterPasswordPolicyOptions,
) -> bool {
Expand Down
4 changes: 2 additions & 2 deletions crates/bitwarden/src/auth/client_auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl<'a> ClientAuth<'a> {
impl<'a> ClientAuth<'a> {
pub async fn password_strength(
&self,
password: String,
password: SensitiveString,

Check warning on line 52 in crates/bitwarden/src/auth/client_auth.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/client_auth.rs#L52

Added line #L52 was not covered by tests
email: String,
additional_inputs: Vec<String>,
) -> u8 {
Expand All @@ -58,7 +58,7 @@ impl<'a> ClientAuth<'a> {

pub async fn satisfies_policy(
&self,
password: String,
password: SensitiveString,

Check warning on line 61 in crates/bitwarden/src/auth/client_auth.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/client_auth.rs#L61

Added line #L61 was not covered by tests
strength: u8,
policy: &MasterPasswordPolicyOptions,
) -> bool {
Expand Down
27 changes: 15 additions & 12 deletions crates/bitwarden/src/auth/password/policy.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use bitwarden_crypto::SensitiveString;
use schemars::JsonSchema;

/// Validate the provided password passes the provided Master Password Requirements Policy.
pub(crate) fn satisfies_policy(
password: String,
password: SensitiveString,
strength: u8,
policy: &MasterPasswordPolicyOptions,
) -> bool {
Expand All @@ -14,19 +15,19 @@ pub(crate) fn satisfies_policy(
return false;
}

if policy.require_upper && password.to_lowercase() == password {
if policy.require_upper && !password.any_chars(char::is_uppercase) {
return false;
}

if policy.require_lower && password.to_uppercase() == password {
if policy.require_lower && !password.any_chars(char::is_lowercase) {
return false;
}

if policy.require_numbers && !password.chars().any(|c| c.is_numeric()) {
if policy.require_numbers && !password.any_chars(char::is_numeric) {
return false;
}

if policy.require_special && !password.chars().any(|c| "!@#$%^&*".contains(c)) {
if policy.require_special && !password.any_chars(|c| "!@#$%^&*".contains(c)) {
return false;
}

Expand All @@ -53,11 +54,13 @@ pub struct MasterPasswordPolicyOptions {
#[cfg(test)]
mod tests {

use bitwarden_crypto::SensitiveString;

use super::{satisfies_policy, MasterPasswordPolicyOptions};

#[test]
fn satisfies_policy_gives_success() {
let password = "lkasfo!icbb$2323ALKJCO22".to_string();
let password = SensitiveString::test("lkasfo!icbb$2323ALKJCO22");
let options = MasterPasswordPolicyOptions {
min_complexity: 3,
min_length: 5,
Expand All @@ -74,7 +77,7 @@ mod tests {

#[test]
fn satisfies_policy_evaluates_strength() {
let password = "password123".to_string();
let password = SensitiveString::test("password123");
let options = MasterPasswordPolicyOptions {
min_complexity: 3,
min_length: 0,
Expand All @@ -91,7 +94,7 @@ mod tests {

#[test]
fn satisfies_policy_evaluates_length() {
let password = "password123".to_string();
let password = SensitiveString::test("password123");
let options = MasterPasswordPolicyOptions {
min_complexity: 0,
min_length: 20,
Expand All @@ -108,7 +111,7 @@ mod tests {

#[test]
fn satisfies_policy_evaluates_upper() {
let password = "password123".to_string();
let password = SensitiveString::test("password123");
let options = MasterPasswordPolicyOptions {
min_complexity: 0,
min_length: 0,
Expand All @@ -125,7 +128,7 @@ mod tests {

#[test]
fn satisfies_policy_evaluates_lower() {
let password = "ABCDEFG123".to_string();
let password = SensitiveString::test("ABCDEFG123");
let options = MasterPasswordPolicyOptions {
min_complexity: 0,
min_length: 0,
Expand All @@ -142,7 +145,7 @@ mod tests {

#[test]
fn satisfies_policy_evaluates_numbers() {
let password = "password".to_string();
let password = SensitiveString::test("password");
let options = MasterPasswordPolicyOptions {
min_complexity: 0,
min_length: 0,
Expand All @@ -159,7 +162,7 @@ mod tests {

#[test]
fn satisfies_policy_evaluates_special() {
let password = "Password123".to_string();
let password = SensitiveString::test("Password123");
let options = MasterPasswordPolicyOptions {
min_complexity: 0,
min_length: 0,
Expand Down
14 changes: 9 additions & 5 deletions crates/bitwarden/src/auth/password/strength.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use bitwarden_crypto::SensitiveString;
use zxcvbn::zxcvbn;

const GLOBAL_INPUTS: [&str; 3] = ["bitwarden", "bit", "warden"];

pub(crate) fn password_strength(
password: String,
password: SensitiveString,
email: String,
additional_inputs: Vec<String>,
) -> u8 {
Expand All @@ -13,7 +14,7 @@ pub(crate) fn password_strength(
let mut arr: Vec<_> = inputs.iter().map(String::as_str).collect();
arr.extend(GLOBAL_INPUTS);

zxcvbn(&password, &arr).map_or(0, |e| e.score())
zxcvbn(password.expose(), &arr).map_or(0, |e| e.score())
}

fn email_to_user_inputs(email: &str) -> Vec<String> {
Expand All @@ -31,6 +32,8 @@ fn email_to_user_inputs(email: &str) -> Vec<String> {

#[cfg(test)]
mod tests {
use bitwarden_crypto::SensitiveString;

use super::{email_to_user_inputs, password_strength};

#[test]
Expand All @@ -44,7 +47,8 @@ mod tests {
];

for (password, email, expected) in cases {
let result = password_strength(password.to_owned(), email.to_owned(), vec![]);
let result =
password_strength(SensitiveString::test(password), email.to_owned(), vec![]);
assert_eq!(expected, result, "{email}: {password}");
}
}
Expand All @@ -54,14 +58,14 @@ mod tests {
let password = "asdfjkhkjwer!";

let result = password_strength(
password.to_owned(),
SensitiveString::test(password),
"[email protected]".to_owned(),
vec![],
);
assert_eq!(result, 4);

let result = password_strength(
password.to_owned(),
SensitiveString::test(password),
"[email protected]".to_owned(),
vec![],
);
Expand Down

0 comments on commit 817cbe3

Please sign in to comment.