From ebf40099f2fc73cc5309ddd7fff6a679dc9ae839 Mon Sep 17 00:00:00 2001 From: BlackDex Date: Thu, 10 Oct 2019 17:32:20 +0200 Subject: [PATCH 1/2] Updated authenticator TOTP - Added security check for previouse used codes - Allow TOTP codes with 1 step back and forward when there is a time drift. This means in total 3 codes could be valid. But only newer codes then the previouse used codes are excepted after that. --- .../down.sql | 0 .../up.sql | 1 + .../down.sql | 0 .../up.sql | 1 + .../down.sql | 0 .../up.sql | 1 + src/api/core/two_factor/authenticator.rs | 60 ++++++++++++++++--- src/api/identity.rs | 2 +- src/db/models/two_factor.rs | 2 + src/db/schemas/mysql/schema.rs | 1 + src/db/schemas/postgresql/schema.rs | 1 + src/db/schemas/sqlite/schema.rs | 1 + 12 files changed, 60 insertions(+), 10 deletions(-) create mode 100644 migrations/mysql/2019-10-10-083032_add_column_to_twofactor/down.sql create mode 100644 migrations/mysql/2019-10-10-083032_add_column_to_twofactor/up.sql create mode 100644 migrations/postgresql/2019-10-10-083032_add_column_to_twofactor/down.sql create mode 100644 migrations/postgresql/2019-10-10-083032_add_column_to_twofactor/up.sql create mode 100644 migrations/sqlite/2019-10-10-083032_add_column_to_twofactor/down.sql create mode 100644 migrations/sqlite/2019-10-10-083032_add_column_to_twofactor/up.sql diff --git a/migrations/mysql/2019-10-10-083032_add_column_to_twofactor/down.sql b/migrations/mysql/2019-10-10-083032_add_column_to_twofactor/down.sql new file mode 100644 index 00000000..e69de29b diff --git a/migrations/mysql/2019-10-10-083032_add_column_to_twofactor/up.sql b/migrations/mysql/2019-10-10-083032_add_column_to_twofactor/up.sql new file mode 100644 index 00000000..cfcd6ca2 --- /dev/null +++ b/migrations/mysql/2019-10-10-083032_add_column_to_twofactor/up.sql @@ -0,0 +1 @@ +ALTER TABLE twofactor ADD COLUMN last_used INTEGER NOT NULL DEFAULT 0; \ No newline at end of file diff --git a/migrations/postgresql/2019-10-10-083032_add_column_to_twofactor/down.sql b/migrations/postgresql/2019-10-10-083032_add_column_to_twofactor/down.sql new file mode 100644 index 00000000..e69de29b diff --git a/migrations/postgresql/2019-10-10-083032_add_column_to_twofactor/up.sql b/migrations/postgresql/2019-10-10-083032_add_column_to_twofactor/up.sql new file mode 100644 index 00000000..cfcd6ca2 --- /dev/null +++ b/migrations/postgresql/2019-10-10-083032_add_column_to_twofactor/up.sql @@ -0,0 +1 @@ +ALTER TABLE twofactor ADD COLUMN last_used INTEGER NOT NULL DEFAULT 0; \ No newline at end of file diff --git a/migrations/sqlite/2019-10-10-083032_add_column_to_twofactor/down.sql b/migrations/sqlite/2019-10-10-083032_add_column_to_twofactor/down.sql new file mode 100644 index 00000000..e69de29b diff --git a/migrations/sqlite/2019-10-10-083032_add_column_to_twofactor/up.sql b/migrations/sqlite/2019-10-10-083032_add_column_to_twofactor/up.sql new file mode 100644 index 00000000..14d3c683 --- /dev/null +++ b/migrations/sqlite/2019-10-10-083032_add_column_to_twofactor/up.sql @@ -0,0 +1 @@ +ALTER TABLE twofactor ADD COLUMN last_used INTEGER NOT NULL DEFAULT 0; diff --git a/src/api/core/two_factor/authenticator.rs b/src/api/core/two_factor/authenticator.rs index eeb91c46..7b4c5f92 100644 --- a/src/api/core/two_factor/authenticator.rs +++ b/src/api/core/two_factor/authenticator.rs @@ -77,7 +77,7 @@ fn activate_authenticator(data: JsonUpcase, headers: He let twofactor = TwoFactor::new(user.uuid.clone(), type_, key.to_uppercase()); // Validate the token provided with the key - validate_totp_code(token, &twofactor.data)?; + validate_totp_code(&user.uuid, token, &twofactor.data, &conn)?; _generate_recover_code(&mut user, &conn); twofactor.save(&conn)?; @@ -94,27 +94,69 @@ fn activate_authenticator_put(data: JsonUpcase, headers activate_authenticator(data, headers, conn) } -pub fn validate_totp_code_str(totp_code: &str, secret: &str) -> EmptyResult { +pub fn validate_totp_code_str(user_uuid: &str, totp_code: &str, secret: &str, conn: &DbConn) -> EmptyResult { let totp_code: u64 = match totp_code.parse() { Ok(code) => code, _ => err!("TOTP code is not a number"), }; - validate_totp_code(totp_code, secret) + validate_totp_code(user_uuid, totp_code, secret, &conn) } -pub fn validate_totp_code(totp_code: u64, secret: &str) -> EmptyResult { - use oath::{totp_raw_now, HashType}; +pub fn validate_totp_code(user_uuid: &str, totp_code: u64, secret: &str, conn: &DbConn) -> EmptyResult { + use oath::{totp_raw_custom_time, HashType}; + use std::time::{UNIX_EPOCH, SystemTime}; let decoded_secret = match BASE32.decode(secret.as_bytes()) { Ok(s) => s, Err(_) => err!("Invalid TOTP secret"), }; - let generated = totp_raw_now(&decoded_secret, 6, 0, 30, &HashType::SHA1); - if generated != totp_code { - err!("Invalid TOTP code"); + let mut twofactor = TwoFactor::find_by_user_and_type(&user_uuid, TwoFactorType::Authenticator as i32, &conn)?; + + // Get the current system time in UNIX Epoch (UTC) + let current_time: u64 = SystemTime::now().duration_since(UNIX_EPOCH) + .expect("Earlier than 1970-01-01 00:00:00 UTC").as_secs(); + + // First check the current time for a valid token. + let time_step_now = (current_time / 30) as i32; + let generated_now = totp_raw_custom_time(&decoded_secret, 6, 0, 30, current_time, &HashType::SHA1); + if generated_now == totp_code && time_step_now > twofactor.last_used { + twofactor.last_used = time_step_now; + twofactor.save(&conn)?; + return Ok(()); + } else if generated_now == totp_code && time_step_now <= twofactor.last_used { + warn!("This or a future TOTP code has already been used!"); + err!("Invalid TOTP code!"); } - Ok(()) + // Check for time drifted codes + // First check the previous TOTP code + let time_step_prev = ((current_time - 30) / 30) as i32; + let generated_prev = totp_raw_custom_time(&decoded_secret, 6, 0, 30, current_time - 30, &HashType::SHA1); + if generated_prev == totp_code && time_step_prev > twofactor.last_used { + info!("TOTP Time drift detected. Token is valide for one step on the past."); + twofactor.last_used = time_step_prev; + twofactor.save(&conn)?; + return Ok(()); + } else if generated_prev == totp_code && time_step_prev <= twofactor.last_used { + warn!("This or a future TOTP code has already been used!"); + err!("Invalid TOTP code!"); + } + + // Second check the next TOTP code + let time_step_next = ((current_time + 30) / 30) as i32; + let generated_next = totp_raw_custom_time(&decoded_secret, 6, 0, 30, current_time + 30, &HashType::SHA1); + if generated_next == totp_code && time_step_next > twofactor.last_used { + info!("TOTP Time drift detected. Token is valide for one step on the future."); + twofactor.last_used = time_step_next; + twofactor.save(&conn)?; + return Ok(()); + } else if generated_next == totp_code && time_step_next <= twofactor.last_used { + warn!("This or a previous TOTP code has already been used!"); + err!("Invalid TOTP code!"); + } + + // Else no valide code received, deny access + err!("Invalid TOTP code!"); } diff --git a/src/api/identity.rs b/src/api/identity.rs index 2da203dd..03460ebe 100644 --- a/src/api/identity.rs +++ b/src/api/identity.rs @@ -197,7 +197,7 @@ fn twofactor_auth( let mut remember = data.two_factor_remember.unwrap_or(0); match TwoFactorType::from_i32(selected_id) { - Some(TwoFactorType::Authenticator) => _tf::authenticator::validate_totp_code_str(twofactor_code, &selected_data?)?, + Some(TwoFactorType::Authenticator) => _tf::authenticator::validate_totp_code_str(user_uuid, twofactor_code, &selected_data?, conn)?, Some(TwoFactorType::U2f) => _tf::u2f::validate_u2f_login(user_uuid, twofactor_code, conn)?, Some(TwoFactorType::YubiKey) => _tf::yubikey::validate_yubikey_login(twofactor_code, &selected_data?)?, Some(TwoFactorType::Duo) => _tf::duo::validate_duo_login(data.username.as_ref().unwrap(), twofactor_code, conn)?, diff --git a/src/db/models/two_factor.rs b/src/db/models/two_factor.rs index d869245c..ecc3c69a 100644 --- a/src/db/models/two_factor.rs +++ b/src/db/models/two_factor.rs @@ -19,6 +19,7 @@ pub struct TwoFactor { pub atype: i32, pub enabled: bool, pub data: String, + pub last_used: i32, } #[allow(dead_code)] @@ -47,6 +48,7 @@ impl TwoFactor { atype: atype as i32, enabled: true, data, + last_used: 0, } } diff --git a/src/db/schemas/mysql/schema.rs b/src/db/schemas/mysql/schema.rs index 51ec1880..2e705112 100644 --- a/src/db/schemas/mysql/schema.rs +++ b/src/db/schemas/mysql/schema.rs @@ -92,6 +92,7 @@ table! { atype -> Integer, enabled -> Bool, data -> Text, + last_used -> Integer, } } diff --git a/src/db/schemas/postgresql/schema.rs b/src/db/schemas/postgresql/schema.rs index 1bc924c9..0112c59a 100644 --- a/src/db/schemas/postgresql/schema.rs +++ b/src/db/schemas/postgresql/schema.rs @@ -92,6 +92,7 @@ table! { atype -> Integer, enabled -> Bool, data -> Text, + last_used -> Integer, } } diff --git a/src/db/schemas/sqlite/schema.rs b/src/db/schemas/sqlite/schema.rs index 750cecd3..43abe5c4 100644 --- a/src/db/schemas/sqlite/schema.rs +++ b/src/db/schemas/sqlite/schema.rs @@ -92,6 +92,7 @@ table! { atype -> Integer, enabled -> Bool, data -> Text, + last_used -> Integer, } } From 9466f0269607098304c03f87e2ed116b93a2c01d Mon Sep 17 00:00:00 2001 From: BlackDex Date: Sat, 12 Oct 2019 15:28:28 +0200 Subject: [PATCH 2/2] Recoded TOTP time drift validation --- src/api/core/two_factor/authenticator.rs | 59 ++++++++++-------------- 1 file changed, 24 insertions(+), 35 deletions(-) diff --git a/src/api/core/two_factor/authenticator.rs b/src/api/core/two_factor/authenticator.rs index 7b4c5f92..2622b965 100644 --- a/src/api/core/two_factor/authenticator.rs +++ b/src/api/core/two_factor/authenticator.rs @@ -118,43 +118,32 @@ pub fn validate_totp_code(user_uuid: &str, totp_code: u64, secret: &str, conn: & let current_time: u64 = SystemTime::now().duration_since(UNIX_EPOCH) .expect("Earlier than 1970-01-01 00:00:00 UTC").as_secs(); - // First check the current time for a valid token. - let time_step_now = (current_time / 30) as i32; - let generated_now = totp_raw_custom_time(&decoded_secret, 6, 0, 30, current_time, &HashType::SHA1); - if generated_now == totp_code && time_step_now > twofactor.last_used { - twofactor.last_used = time_step_now; - twofactor.save(&conn)?; - return Ok(()); - } else if generated_now == totp_code && time_step_now <= twofactor.last_used { - warn!("This or a future TOTP code has already been used!"); - err!("Invalid TOTP code!"); - } + // The amount of steps back and forward in time + let steps = 1; + for step in -steps..=steps { - // Check for time drifted codes - // First check the previous TOTP code - let time_step_prev = ((current_time - 30) / 30) as i32; - let generated_prev = totp_raw_custom_time(&decoded_secret, 6, 0, 30, current_time - 30, &HashType::SHA1); - if generated_prev == totp_code && time_step_prev > twofactor.last_used { - info!("TOTP Time drift detected. Token is valide for one step on the past."); - twofactor.last_used = time_step_prev; - twofactor.save(&conn)?; - return Ok(()); - } else if generated_prev == totp_code && time_step_prev <= twofactor.last_used { - warn!("This or a future TOTP code has already been used!"); - err!("Invalid TOTP code!"); - } + let time_step = (current_time / 30) as i32 + step; + // We need to calculate the time offsite and cast it as an i128. + // Else we can't do math with it on a default u64 variable. + let time_offset: i128 = (step * 30).into(); + let generated = totp_raw_custom_time(&decoded_secret, 6, 0, 30, (current_time as i128 + time_offset) as u64, &HashType::SHA1); - // Second check the next TOTP code - let time_step_next = ((current_time + 30) / 30) as i32; - let generated_next = totp_raw_custom_time(&decoded_secret, 6, 0, 30, current_time + 30, &HashType::SHA1); - if generated_next == totp_code && time_step_next > twofactor.last_used { - info!("TOTP Time drift detected. Token is valide for one step on the future."); - twofactor.last_used = time_step_next; - twofactor.save(&conn)?; - return Ok(()); - } else if generated_next == totp_code && time_step_next <= twofactor.last_used { - warn!("This or a previous TOTP code has already been used!"); - err!("Invalid TOTP code!"); + // Check the the given code equals the generated and if the time_step is larger then the one last used. + if generated == totp_code && time_step > twofactor.last_used { + + // If the step does not equals 0 the time is drifted either server or client side. + if step != 0 { + info!("TOTP Time drift detected. The step offset is {}", step); + } + + // Save the last used time step so only totp time steps higher then this one are allowed. + twofactor.last_used = time_step; + twofactor.save(&conn)?; + return Ok(()); + } else if generated == totp_code && time_step <= twofactor.last_used { + warn!("This or a TOTP code within {} steps back and forward has already been used!", steps); + err!("Invalid TOTP Code!"); + } } // Else no valide code received, deny access