From 7d552dbdc8d8d7536ac7f7d3a8c8ad9a7d316f83 Mon Sep 17 00:00:00 2001 From: Jeremy Lin Date: Mon, 24 Jan 2022 01:17:00 -0800 Subject: [PATCH] Increase length limit for email token generation The current limit of 19 is an artifact of the implementation, which can be easily rewritten in terms of a more general string generation function. The new limit is 255 (max value of a `u8`); using a larger type would probably be overkill. --- .env.template | 2 +- src/api/core/accounts.rs | 2 +- src/api/core/two_factor/email.rs | 18 ++---------------- src/config.rs | 8 ++------ src/crypto.rs | 28 +++++++++------------------- 5 files changed, 15 insertions(+), 43 deletions(-) diff --git a/.env.template b/.env.template index 1f4c937f..8da88cdc 100644 --- a/.env.template +++ b/.env.template @@ -185,7 +185,7 @@ # EMAIL_EXPIRATION_TIME=600 ## Email token size -## Number of digits in an email token (min: 6, max: 19). +## Number of digits in an email 2FA token (min: 6, max: 255). ## Note that the Bitwarden clients are hardcoded to mention 6 digit codes regardless of this setting! # EMAIL_TOKEN_SIZE=6 diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs index ead05478..32c81e93 100644 --- a/src/api/core/accounts.rs +++ b/src/api/core/accounts.rs @@ -381,7 +381,7 @@ fn post_email_token(data: JsonUpcase, headers: Headers, conn: Db err!("Email domain not allowed"); } - let token = crypto::generate_token(6)?; + let token = crypto::generate_email_token(6); if CONFIG.mail_enabled() { if let Err(e) = mail::send_change_email(&data.NewEmail, &token) { diff --git a/src/api/core/two_factor/email.rs b/src/api/core/two_factor/email.rs index 998aeccf..0753f62f 100644 --- a/src/api/core/two_factor/email.rs +++ b/src/api/core/two_factor/email.rs @@ -58,7 +58,7 @@ pub fn send_token(user_uuid: &str, conn: &DbConn) -> EmptyResult { let type_ = TwoFactorType::Email as i32; let mut twofactor = TwoFactor::find_by_user_and_type(user_uuid, type_, conn).map_res("Two factor not found")?; - let generated_token = crypto::generate_token(CONFIG.email_token_size())?; + let generated_token = crypto::generate_email_token(CONFIG.email_token_size()); let mut twofactor_data = EmailTokenData::from_json(&twofactor.data)?; twofactor_data.set_token(generated_token); @@ -123,7 +123,7 @@ fn send_email(data: JsonUpcase, headers: Headers, conn: DbConn) - tf.delete(&conn)?; } - let generated_token = crypto::generate_token(CONFIG.email_token_size())?; + let generated_token = crypto::generate_email_token(CONFIG.email_token_size()); let twofactor_data = EmailTokenData::new(data.Email, generated_token); // Uses EmailVerificationChallenge as type to show that it's not verified yet. @@ -309,18 +309,4 @@ mod tests { // If it's smaller than 3 characters it should only show asterisks. assert_eq!(result, "***@example.ext"); } - - #[test] - fn test_token() { - let result = crypto::generate_token(19).unwrap(); - - assert_eq!(result.chars().count(), 19); - } - - #[test] - fn test_token_too_large() { - let result = crypto::generate_token(20); - - assert!(result.is_err(), "too large token should give an error"); - } } diff --git a/src/config.rs b/src/config.rs index 92fe8b9d..9e1f5e56 100644 --- a/src/config.rs +++ b/src/config.rs @@ -593,8 +593,8 @@ make_config! { email_2fa: _enable_email_2fa { /// Enabled |> Disabling will prevent users from setting up new email 2FA and using existing email 2FA configured _enable_email_2fa: bool, true, auto, |c| c._enable_smtp && c.smtp_host.is_some(); - /// Email token size |> Number of digits in an email token (min: 6, max: 19). Note that the Bitwarden clients are hardcoded to mention 6 digit codes regardless of this setting. - email_token_size: u32, true, def, 6; + /// Email token size |> Number of digits in an email 2FA token (min: 6, max: 255). Note that the Bitwarden clients are hardcoded to mention 6 digit codes regardless of this setting. + email_token_size: u8, true, def, 6; /// Token expiration time |> Maximum time in seconds a token is valid. The time the user has to open email client and copy token. email_expiration_time: u64, true, def, 600; /// Maximum attempts |> Maximum attempts before an email token is reset and a new email will need to be sent @@ -668,10 +668,6 @@ fn validate_config(cfg: &ConfigItems) -> Result<(), Error> { if cfg._enable_email_2fa && cfg.email_token_size < 6 { err!("`EMAIL_TOKEN_SIZE` has a minimum size of 6") } - - if cfg._enable_email_2fa && cfg.email_token_size > 19 { - err!("`EMAIL_TOKEN_SIZE` has a maximum size of 19") - } } // Check if the icon blacklist regex is valid diff --git a/src/crypto.rs b/src/crypto.rs index e30439fc..be9680cb 100644 --- a/src/crypto.rs +++ b/src/crypto.rs @@ -6,8 +6,6 @@ use std::num::NonZeroU32; use data_encoding::HEXLOWER; use ring::{digest, hmac, pbkdf2}; -use crate::error::Error; - static DIGEST_ALG: pbkdf2::Algorithm = pbkdf2::PBKDF2_HMAC_SHA256; const OUTPUT_LEN: usize = digest::SHA256_OUTPUT_LEN; @@ -65,6 +63,12 @@ pub fn get_random_string(alphabet: &[u8], num_chars: usize) -> String { .collect() } +/// Generates a random numeric string. +pub fn get_random_string_numeric(num_chars: usize) -> String { + const ALPHABET: &[u8] = b"0123456789"; + get_random_string(ALPHABET, num_chars) +} + /// Generates a random alphanumeric string. pub fn get_random_string_alphanum(num_chars: usize) -> String { const ALPHABET: &[u8] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZ\ @@ -87,23 +91,9 @@ pub fn generate_attachment_id() -> String { generate_id(10) // 80 bits } -pub fn generate_token(token_size: u32) -> Result { - // A u64 can represent all whole numbers up to 19 digits long. - if token_size > 19 { - err!("Token size is limited to 19 digits") - } - - let low: u64 = 0; - let high: u64 = 10u64.pow(token_size); - - // Generate a random number in the range [low, high), then format it as a - // token of fixed width, left-padding with 0 as needed. - use rand::{thread_rng, Rng}; - let mut rng = thread_rng(); - let number: u64 = rng.gen_range(low..high); - let token = format!("{:0size$}", number, size = token_size as usize); - - Ok(token) +/// Generates a numeric token for email-based verifications. +pub fn generate_email_token(token_size: u8) -> String { + get_random_string_numeric(token_size as usize) } /// Generates a personal API key.