diff --git a/crates/bitwarden/src/tool/generators/client_generator.rs b/crates/bitwarden/src/tool/generators/client_generator.rs index e74a408dd..fcfb39a71 100644 --- a/crates/bitwarden/src/tool/generators/client_generator.rs +++ b/crates/bitwarden/src/tool/generators/client_generator.rs @@ -27,7 +27,7 @@ impl<'a> ClientGenerator<'a> { /// lowercase: true, /// uppercase: true, /// numbers: true, - /// length: Some(20), + /// length: 20, /// ..Default::default() /// }; /// let password = Client::new(None).generator().password(input).await.unwrap(); @@ -36,7 +36,8 @@ impl<'a> ClientGenerator<'a> { /// } /// ``` pub async fn password(&self, input: PasswordGeneratorRequest) -> Result { - password(input) + let charset = input.validate_options()?; + Ok(password(charset)) } pub async fn passphrase(&self, input: PassphraseGeneratorRequest) -> Result { diff --git a/crates/bitwarden/src/tool/generators/password.rs b/crates/bitwarden/src/tool/generators/password.rs index cea7adab4..24752f704 100644 --- a/crates/bitwarden/src/tool/generators/password.rs +++ b/crates/bitwarden/src/tool/generators/password.rs @@ -1,5 +1,5 @@ use crate::error::{Error, Result}; -use rand::{seq::SliceRandom, RngCore}; +use rand::{distributions::Distribution, seq::SliceRandom, RngCore}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -26,12 +26,11 @@ pub struct PasswordGeneratorRequest { /// The length of the generated password. /// Note that the password length must be greater than the sum of all the minimums. - /// The default value when unset is 16. - pub length: Option, + pub length: u8, /// When set to true, the generated password will not contain ambiguous characters. /// The ambiguous characters are: I, O, l, 0, 1 - pub avoid_ambiguous: Option, // TODO: Should we rename this to include_all_characters? + pub avoid_ambiguous: bool, // TODO: Should we rename this to include_all_characters? /// The minimum number of lowercase characters in the generated password. /// When set, the value must be between 1 and 9. This value is ignored is lowercase is false @@ -47,6 +46,8 @@ pub struct PasswordGeneratorRequest { pub min_special: Option, } +const DEFAULT_PASSWORD_LENGTH: u8 = 16; + // We need to implement this manually so we can set one character set to true. // Otherwise the default implementation will fail to generate a password. impl Default for PasswordGeneratorRequest { @@ -56,8 +57,8 @@ impl Default for PasswordGeneratorRequest { uppercase: false, numbers: false, special: false, - length: None, - avoid_ambiguous: None, + length: DEFAULT_PASSWORD_LENGTH, + avoid_ambiguous: false, min_lowercase: None, min_uppercase: None, min_number: None, @@ -79,144 +80,182 @@ pub struct PassphraseGeneratorRequest { pub include_number: Option, } -const DEFAULT_PASSWORD_LENGTH: u8 = 16; - const UPPER_CHARS_AMBIGUOUS: &[char] = &['I', 'O']; -const UPPER_CHARS: &[char] = &[ - 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'J', 'K', 'L', 'M', 'N', 'P', 'Q', 'R', 'S', 'T', 'U', - 'V', 'W', 'X', 'Y', 'Z', -]; const LOWER_CHARS_AMBIGUOUS: &[char] = &['l']; -const LOWER_CHARS: &[char] = &[ - 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', - 'u', 'v', 'w', 'x', 'y', 'z', -]; const NUMBER_CHARS_AMBIGUOUS: &[char] = &['0', '1']; -const NUMBER_CHARS: &[char] = &['2', '3', '4', '5', '6', '7', '8', '9']; const SPECIAL_CHARS: &[char] = &['!', '@', '#', '$', '%', '^', '&', '*']; -struct PasswordGeneratorCharSet { - lower: Vec, - upper: Vec, - number: Vec, - special: Vec, - all: Vec, -} +// We don't want the validated struct to be accessible, yet at the same time it needs to be public +// to be used as a return type, so we define it in a private module to make it innaccessible. +mod private { + use std::collections::BTreeSet; + + use rand::prelude::Distribution; -impl PasswordGeneratorCharSet { - fn new(lower: bool, upper: bool, number: bool, special: bool, avoid_ambiguous: bool) -> Self { - fn chars( - enabled: bool, - chars: &[char], - ambiguous: &[char], - avoid_ambiguous: bool, - ) -> Vec { - if !enabled { - return Vec::new(); + // Note: We are using a BTreeSet to have consistent ordering between runs. This is not + // important during normal execution, but it's necessary for the tests to be repeatable. + #[derive(Clone, Default)] + pub struct CharSet(pub(super) BTreeSet); + impl CharSet { + pub fn include(self, other: impl IntoIterator) -> Self { + self.include_if(true, other) + } + pub fn include_if( + mut self, + predicate: bool, + other: impl IntoIterator, + ) -> Self { + if predicate { + self.0.extend(other); } - let mut chars = chars.to_vec(); - if !avoid_ambiguous { - chars.extend_from_slice(ambiguous); + self + } + pub fn exclude_if<'a>( + self, + predicate: bool, + other: impl IntoIterator, + ) -> Self { + if predicate { + let other: BTreeSet<_> = other.into_iter().copied().collect(); + Self(self.0.difference(&other).copied().collect()) + } else { + self } - chars } - let lower = chars(lower, LOWER_CHARS, LOWER_CHARS_AMBIGUOUS, avoid_ambiguous); - let upper = chars(upper, UPPER_CHARS, UPPER_CHARS_AMBIGUOUS, avoid_ambiguous); - let number = chars( - number, - NUMBER_CHARS, - NUMBER_CHARS_AMBIGUOUS, - avoid_ambiguous, + } + impl<'a> IntoIterator for &'a CharSet { + type Item = char; + type IntoIter = std::iter::Copied>; + fn into_iter(self) -> Self::IntoIter { + self.0.iter().copied() + } + } + impl Distribution for CharSet { + fn sample(&self, rng: &mut R) -> char { + let idx = rng.gen_range(0..self.0.len()); + *self.0.iter().nth(idx).expect("Valid index") + } + } + + pub struct PasswordGeneratorOptions { + pub(super) lower: (CharSet, usize), + pub(super) upper: (CharSet, usize), + pub(super) number: (CharSet, usize), + pub(super) special: (CharSet, usize), + pub(super) all: (CharSet, usize), + + pub(super) length: usize, + } +} +use private::{CharSet, PasswordGeneratorOptions}; + +impl PasswordGeneratorRequest { + // TODO: Add password generator policy checks + pub fn validate_options(self) -> Result { + // We always have to have at least one character set enabled + if !self.lowercase && !self.uppercase && !self.numbers && !self.special { + return Err(Error::Internal( + "At least one character set must be enabled", + )); + } + + // Make sure the minimum values are zero when the character + // set is disabled, and at least one when it's enabled + fn get_minimum(min: Option, enabled: bool) -> usize { + if enabled { + usize::max(min.unwrap_or(1) as usize, 1) + } else { + 0 + } + } + + let length = self.length as usize; + let min_lowercase = get_minimum(self.min_lowercase, self.lowercase); + let min_uppercase = get_minimum(self.min_uppercase, self.uppercase); + let min_number = get_minimum(self.min_number, self.numbers); + let min_special = get_minimum(self.min_special, self.special); + + // Check that the minimum lengths aren't larger than the password length + let minimum_length = min_lowercase + min_uppercase + min_number + min_special; + if minimum_length > length { + return Err(Error::Internal( + "Password length can't be less than the sum of the minimums", + )); + } + + let lower = ( + CharSet::default() + .include_if(self.lowercase, 'a'..='z') + .exclude_if(self.avoid_ambiguous, LOWER_CHARS_AMBIGUOUS), + min_lowercase, ); - let special = chars(special, SPECIAL_CHARS, &[], avoid_ambiguous); - let all = lower - .iter() - .chain(&upper) - .chain(&number) - .chain(&special) - .copied() - .collect(); - Self { + let upper = ( + CharSet::default() + .include_if(self.uppercase, 'A'..='Z') + .exclude_if(self.avoid_ambiguous, UPPER_CHARS_AMBIGUOUS), + min_uppercase, + ); + + let number = ( + CharSet::default() + .include_if(self.numbers, '0'..='9') + .exclude_if(self.avoid_ambiguous, NUMBER_CHARS_AMBIGUOUS), + min_number, + ); + + let special = ( + CharSet::default().include_if(self.special, SPECIAL_CHARS.iter().copied()), + min_special, + ); + + let all = ( + CharSet::default() + .include(&lower.0) + .include(&upper.0) + .include(&number.0) + .include(&special.0), + length - minimum_length, + ); + + Ok(PasswordGeneratorOptions { lower, upper, number, special, all, - } + length, + }) } } /// Implementation of the random password generator. This is not accessible to the public API. /// See [`ClientGenerator::password`](crate::ClientGenerator::password) for the API function. -pub(super) fn password(input: PasswordGeneratorRequest) -> Result { +pub(super) fn password(input: PasswordGeneratorOptions) -> String { password_with_rng(rand::thread_rng(), input) } -pub(super) fn password_with_rng( - mut rng: impl RngCore, - input: PasswordGeneratorRequest, -) -> Result { - // We always have to have at least one character set enabled - if !input.lowercase && !input.uppercase && !input.numbers && !input.special { - return Err(Error::Internal( - "At least one character set must be enabled", - )); - } +pub(super) fn password_with_rng(mut rng: impl RngCore, input: PasswordGeneratorOptions) -> String { + let mut buf: Vec = Vec::with_capacity(input.length); - // Generate all character dictionaries - let chars = PasswordGeneratorCharSet::new( - input.lowercase, - input.uppercase, - input.numbers, - input.special, - input.avoid_ambiguous.unwrap_or(false), - ); - - // Make sure the minimum values are zero when the character - // set is disabled, and at least one when it's enabled - fn get_minimum(min: Option, enabled: bool) -> u8 { - if enabled { - u8::max(min.unwrap_or(1), 1) - } else { - 0 - } - } - let min_lowercase = get_minimum(input.min_lowercase, input.lowercase); - let min_uppercase = get_minimum(input.min_uppercase, input.uppercase); - let min_number = get_minimum(input.min_number, input.numbers); - let min_special = get_minimum(input.min_special, input.special); - - // Check that the minimum lengths aren't larger than the password length - let min_length = min_lowercase + min_uppercase + min_number + min_special; - let length = input.length.unwrap_or(DEFAULT_PASSWORD_LENGTH); - if min_length > length { - return Err(Error::Internal( - "Password length can't be less than the sum of the minimums", - )); - } + let (set, qty) = &input.all; + buf.extend(set.sample_iter(&mut rng).take(*qty)); - // Generate the minimum chars of each type, then generate the rest to fill the expected length - let mut buf = Vec::with_capacity(length as usize); + let (set, qty) = &input.upper; + buf.extend(set.sample_iter(&mut rng).take(*qty)); - for _ in 0..min_lowercase { - buf.push(*chars.lower.choose(&mut rng).expect("slice is not empty")); - } - for _ in 0..min_uppercase { - buf.push(*chars.upper.choose(&mut rng).expect("slice is not empty")); - } - for _ in 0..min_number { - buf.push(*chars.number.choose(&mut rng).expect("slice is not empty")); - } - for _ in 0..min_special { - buf.push(*chars.special.choose(&mut rng).expect("slice is not empty")); - } - for _ in min_length..length { - buf.push(*chars.all.choose(&mut rng).expect("slice is not empty")); - } + let (set, qty) = &input.lower; + buf.extend(set.sample_iter(&mut rng).take(*qty)); + + let (set, qty) = &input.number; + buf.extend(set.sample_iter(&mut rng).take(*qty)); + + let (set, qty) = &input.special; + buf.extend(set.sample_iter(&mut rng).take(*qty)); buf.shuffle(&mut rng); - Ok(buf.iter().collect()) + + buf.iter().collect() } pub(super) fn passphrase(_input: PassphraseGeneratorRequest) -> Result { @@ -225,109 +264,153 @@ pub(super) fn passphrase(_input: PassphraseGeneratorRequest) -> Result { #[cfg(test)] mod test { - use std::collections::HashSet; + use std::collections::BTreeSet; use rand::SeedableRng; use super::*; - // We convert the slices to HashSets to be able to use `is_subset` - fn to_set(chars: &[char]) -> HashSet { - chars.iter().copied().collect() + // We convert the slices to BTreeSets to be able to use `is_subset` + fn ref_to_set<'a>(chars: impl IntoIterator) -> BTreeSet { + chars.into_iter().copied().collect() + } + fn to_set(chars: impl IntoIterator) -> BTreeSet { + chars.into_iter().collect() } #[test] - fn test_password_characters_all() { - let set = PasswordGeneratorCharSet::new(true, true, true, true, true); - assert_eq!(set.lower, LOWER_CHARS); - assert_eq!(set.upper, UPPER_CHARS); - assert_eq!(set.number, NUMBER_CHARS); - assert_eq!(set.special, SPECIAL_CHARS); + fn test_password_gen_all_charsets_enabled() { + let mut rng = rand_chacha::ChaCha8Rng::from_seed([0u8; 32]); + + let options = PasswordGeneratorRequest { + lowercase: true, + uppercase: true, + numbers: true, + special: true, + avoid_ambiguous: false, + ..Default::default() + } + .validate_options() + .unwrap(); + + assert_eq!(to_set(&options.lower.0), to_set('a'..='z')); + assert_eq!(to_set(&options.upper.0), to_set('A'..='Z')); + assert_eq!(to_set(&options.number.0), to_set('0'..='9')); + assert_eq!(to_set(&options.special.0), ref_to_set(SPECIAL_CHARS)); + + let pass = password_with_rng(&mut rng, options); + assert_eq!(pass, "Z!^B5r%hUa23dFM@"); } #[test] - fn test_password_characters_all_ambiguous() { - let set = PasswordGeneratorCharSet::new(true, true, true, true, false); - assert!(to_set(&set.lower).is_superset(&to_set(LOWER_CHARS))); - assert!(to_set(&set.lower).is_superset(&to_set(LOWER_CHARS_AMBIGUOUS))); - assert!(to_set(&set.upper).is_superset(&to_set(UPPER_CHARS))); - assert!(to_set(&set.upper).is_superset(&to_set(UPPER_CHARS_AMBIGUOUS))); - assert!(to_set(&set.number).is_superset(&to_set(NUMBER_CHARS))); - assert!(to_set(&set.number).is_superset(&to_set(NUMBER_CHARS_AMBIGUOUS))); - assert_eq!(set.special, SPECIAL_CHARS); + fn test_password_gen_only_letters_enabled() { + let mut rng = rand_chacha::ChaCha8Rng::from_seed([0u8; 32]); + + let options = PasswordGeneratorRequest { + lowercase: true, + uppercase: true, + numbers: false, + special: false, + avoid_ambiguous: false, + ..Default::default() + } + .validate_options() + .unwrap(); + + assert_eq!(to_set(&options.lower.0), to_set('a'..='z')); + assert_eq!(to_set(&options.upper.0), to_set('A'..='Z')); + assert_eq!(to_set(&options.number.0), to_set([])); + assert_eq!(to_set(&options.special.0), to_set([])); + + let pass = password_with_rng(&mut rng, options); + assert_eq!(pass, "NQiFrGufQMiNUAmj"); } #[test] - fn test_password_characters_lower() { - let set = PasswordGeneratorCharSet::new(true, false, false, false, true); - assert_eq!(set.lower, LOWER_CHARS); - assert_eq!(set.upper, Vec::new()); - assert_eq!(set.number, Vec::new()); - assert_eq!(set.special, Vec::new()); + fn test_password_gen_only_numbers_and_lower_enabled_no_ambiguous() { + let mut rng = rand_chacha::ChaCha8Rng::from_seed([0u8; 32]); + + let options = PasswordGeneratorRequest { + lowercase: true, + uppercase: false, + numbers: true, + special: false, + avoid_ambiguous: true, + ..Default::default() + } + .validate_options() + .unwrap(); + + assert!(to_set(&options.lower.0).is_subset(&to_set('a'..='z'))); + assert!(to_set(&options.lower.0).is_disjoint(&ref_to_set(LOWER_CHARS_AMBIGUOUS))); + + assert!(to_set(&options.number.0).is_subset(&to_set('0'..='9'))); + assert!(to_set(&options.number.0).is_disjoint(&ref_to_set(NUMBER_CHARS_AMBIGUOUS))); + + assert_eq!(to_set(&options.upper.0), to_set([])); + assert_eq!(to_set(&options.special.0), to_set([])); + + let pass = password_with_rng(&mut rng, options); + assert_eq!(pass, "mnjabfz5ct272prf"); } #[test] - fn test_password_characters_upper_ambiguous() { - // Only uppercase including ambiguous - let set = PasswordGeneratorCharSet::new(false, true, false, false, false); - assert_eq!(set.lower, Vec::new()); - assert!(to_set(&set.upper).is_superset(&to_set(UPPER_CHARS))); - assert!(to_set(&set.upper).is_superset(&to_set(UPPER_CHARS_AMBIGUOUS))); - assert_eq!(set.number, Vec::new()); - assert_eq!(set.special, Vec::new()); + fn test_password_gen_only_upper_and_special_enabled_no_ambiguous() { + let mut rng = rand_chacha::ChaCha8Rng::from_seed([0u8; 32]); + + let options = PasswordGeneratorRequest { + lowercase: false, + uppercase: true, + numbers: false, + special: true, + avoid_ambiguous: true, + ..Default::default() + } + .validate_options() + .unwrap(); + + assert!(to_set(&options.upper.0).is_subset(&to_set('A'..='Z'))); + assert!(to_set(&options.upper.0).is_disjoint(&ref_to_set(UPPER_CHARS_AMBIGUOUS))); + + assert_eq!(to_set(&options.special.0), ref_to_set(SPECIAL_CHARS)); + + assert_eq!(to_set(&options.lower.0), to_set([])); + assert_eq!(to_set(&options.number.0), to_set([])); + + let pass = password_with_rng(&mut rng, options); + assert_eq!(pass, "B*GBQANS%UZPQD!K"); } #[test] - fn test_password_gen() { + fn test_password_gen_minimum_limits() { let mut rng = rand_chacha::ChaCha8Rng::from_seed([0u8; 32]); - let pass = password_with_rng( - &mut rng, - PasswordGeneratorRequest { - lowercase: true, - uppercase: true, - numbers: true, - special: true, - ..Default::default() - }, - ) - .unwrap(); - assert_eq!(pass, "xfZPr&wXCiFta8DM"); - - let pass = password_with_rng( - &mut rng, - PasswordGeneratorRequest { - lowercase: true, - uppercase: true, - numbers: false, - special: false, - length: Some(20), - avoid_ambiguous: Some(false), - min_lowercase: Some(1), - min_uppercase: Some(1), - min_number: None, - min_special: None, - }, - ) - .unwrap(); - assert_eq!(pass, "jvpFStaIdRUoENAeTmJw"); - - let pass = password_with_rng( - &mut rng, - PasswordGeneratorRequest { - lowercase: false, - uppercase: false, - numbers: true, - special: true, - length: Some(5), - avoid_ambiguous: Some(true), - min_lowercase: None, - min_uppercase: None, - min_number: Some(3), - min_special: Some(2), - }, - ) + let options = PasswordGeneratorRequest { + lowercase: true, + uppercase: true, + numbers: true, + special: true, + avoid_ambiguous: false, + length: 24, + min_lowercase: Some(5), + min_uppercase: Some(5), + min_number: Some(5), + min_special: Some(5), + } + .validate_options() .unwrap(); - assert_eq!(pass, "^878%"); + + assert_eq!(to_set(&options.lower.0), to_set('a'..='z')); + assert_eq!(to_set(&options.upper.0), to_set('A'..='Z')); + assert_eq!(to_set(&options.number.0), to_set('0'..='9')); + assert_eq!(to_set(&options.special.0), ref_to_set(SPECIAL_CHARS)); + + assert_eq!(options.lower.1, 5); + assert_eq!(options.upper.1, 5); + assert_eq!(options.number.1, 5); + assert_eq!(options.special.1, 5); + + let pass = password_with_rng(&mut rng, options); + assert_eq!(pass, "236q5!a#R%PG5rI%k1!*@uRt"); } } diff --git a/crates/bw/src/main.rs b/crates/bw/src/main.rs index 73e64a4aa..71ec66880 100644 --- a/crates/bw/src/main.rs +++ b/crates/bw/src/main.rs @@ -199,7 +199,7 @@ async fn process_commands() -> Result<()> { uppercase: args.uppercase, numbers: args.numbers, special: args.special, - length: Some(args.length), + length: args.length, ..Default::default() }) .await?;