Browse Source

Share TOTP code verification between user and admin 2FA

Extract a single `verify_totp()` helper in `two_factor::authenticator` that
performs the time-window check, and use it from both `validate_totp_code`
(user 2FA) and the admin-page TOTP check instead of duplicating the loop.
Each caller keeps its own last-used storage (DB vs. in-memory) and drift
policy. The user path now compares in constant time, and a unit test for the
helper is added.
pull/7444/head
tom27052006 1 day ago
parent
commit
c99c468eb7
  1. 24
      src/api/admin.rs
  2. 108
      src/api/core/two_factor/authenticator.rs

24
src/api/admin.rs

@ -271,7 +271,7 @@ fn validate_totp(code: Option<&str>) -> bool {
}
fn check_totp_code(secret: &str, code: &str) -> bool {
use totp_lite::{Sha1, totp_custom};
use two_factor::authenticator::{TotpValidation, verify_totp};
if code.len() != 6 || !code.chars().all(char::is_numeric) {
return false;
@ -284,22 +284,18 @@ fn check_totp_code(secret: &str, code: &str) -> bool {
let current_timestamp = chrono::Utc::now().timestamp();
let mut last_used = ADMIN_TOTP_LAST_USED.lock().expect("admin TOTP mutex poisoned");
// Allow one step of time drift in either direction, like the user 2FA in `two_factor::authenticator` does
for step in -1..=1i64 {
let time_step = current_timestamp / 30 + step;
let time = (current_timestamp + step * 30).cast_unsigned();
let generated = totp_custom::<Sha1>(30, 6, &decoded_secret, time);
if crate::crypto::ct_eq(&generated, code) {
if time_step <= *last_used {
warn!("This admin TOTP code has already been used");
return false;
}
// Reuse the shared verifier; allow one step of time drift in either direction, like the user 2FA does.
match verify_totp(&decoded_secret, code, current_timestamp, *last_used, 1) {
TotpValidation::Accepted(time_step) => {
*last_used = time_step;
return true;
true
}
TotpValidation::Reused => {
warn!("This admin TOTP code has already been used");
false
}
TotpValidation::Rejected => false,
}
false
}
#[get("/totp")]

108
src/api/core/two_factor/authenticator.rs

@ -112,6 +112,41 @@ pub async fn validate_totp_code_str(
validate_totp_code(user_id, totp_code, secret, ip, conn).await
}
/// The outcome of checking a TOTP `code` against the allowed time window.
pub(crate) enum TotpValidation {
/// The code is valid. Contains the accepted time step, which must be stored as the
/// new "last used" value so the same code cannot be reused.
Accepted(i64),
/// The code matched a time step that has already been used (replay protection).
Reused,
/// The code did not match any step within the allowed window.
Rejected,
}
/// Verifies a 6-digit TOTP `code` against the already base32-decoded `secret`, allowing
/// `steps` (>= 0) time steps of ±30 seconds drift in either direction and rejecting any
/// time step that is not newer than `last_used`. The comparison is constant-time.
/// Shared by the user 2FA flow and the admin-page 2FA.
pub(crate) fn verify_totp(secret: &[u8], code: &str, timestamp: i64, last_used: i64, steps: i64) -> TotpValidation {
use totp_lite::{Sha1, totp_custom};
for step in -steps..=steps {
let time_step = timestamp / 30i64 + step;
// The generator needs the time as an u64; we only ever deal with times >= 0.
let time: u64 = (timestamp + step * 30i64).cast_unsigned();
let generated = totp_custom::<Sha1>(30, 6, secret, time);
if crypto::ct_eq(&generated, code) {
return if time_step > last_used {
TotpValidation::Accepted(time_step)
} else {
TotpValidation::Reused
};
}
}
TotpValidation::Rejected
}
pub async fn validate_totp_code(
user_id: &UserId,
totp_code: &str,
@ -119,8 +154,6 @@ pub async fn validate_totp_code(
ip: &ClientIp,
conn: &DbConn,
) -> EmptyResult {
use totp_lite::{Sha1, totp_custom};
let Ok(decoded_secret) = BASE32.decode(secret.as_bytes()) else {
err!("Invalid TOTP secret")
};
@ -140,17 +173,10 @@ pub async fn validate_totp_code(
let current_time = chrono::Utc::now();
let current_timestamp = current_time.timestamp();
for step in -steps..=steps {
let time_step = current_timestamp / 30i64 + step;
// We need to calculate the time offsite and cast it as an u64.
// Since we only have times into the future and the totp generator needs an u64 instead of the default i64.
let time: u64 = (current_timestamp + step * 30i64).cast_unsigned();
let generated = totp_custom::<Sha1>(30, 6, &decoded_secret, time);
// Check 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.
match verify_totp(&decoded_secret, totp_code, current_timestamp, twofactor.last_used, steps) {
TotpValidation::Accepted(time_step) => {
// If the accepted step is not the current one the time is drifted either server or client side.
let step = time_step - current_timestamp / 30i64;
if step != 0 {
warn!("TOTP Time drift detected. The step offset is {step}");
}
@ -159,25 +185,27 @@ pub async fn validate_totp_code(
// This will also save a newly created twofactor if the code is correct.
twofactor.last_used = time_step;
twofactor.save(conn).await?;
return Ok(());
} else if generated == totp_code && time_step <= twofactor.last_used {
Ok(())
}
TotpValidation::Reused => {
warn!("This TOTP or a TOTP code within {steps} steps back or forward has already been used!");
err!(
format!("Invalid TOTP code! Server time: {} IP: {}", current_time.format("%F %T UTC"), ip.ip),
ErrorEvent {
event: EventType::UserFailedLogIn2fa
}
);
)
}
}
// Else no valid code received, deny access
err!(
format!("Invalid TOTP code! Server time: {} IP: {}", current_time.format("%F %T UTC"), ip.ip),
ErrorEvent {
event: EventType::UserFailedLogIn2fa
// Else no valid code received, deny access
TotpValidation::Rejected => {
err!(
format!("Invalid TOTP code! Server time: {} IP: {}", current_time.format("%F %T UTC"), ip.ip),
ErrorEvent {
event: EventType::UserFailedLogIn2fa
}
)
}
);
}
}
#[derive(Debug, Deserialize)]
@ -217,3 +245,35 @@ async fn disable_authenticator(data: Json<DisableAuthenticatorData>, headers: He
"object": "twoFactorProvider"
})))
}
#[cfg(test)]
mod tests {
use super::{TotpValidation, verify_totp};
use data_encoding::BASE32;
use totp_lite::{Sha1, totp_custom};
fn code_at(secret: &[u8], timestamp: i64) -> String {
totp_custom::<Sha1>(30, 6, secret, timestamp.cast_unsigned())
}
#[test]
fn totp_accepts_rejects_and_detects_reuse() {
let secret = BASE32.decode(b"JBSWY3DPEHPK3PXP").unwrap();
let ts = 1_700_000_000i64;
let step = ts / 30;
let current = code_at(&secret, ts);
// A fresh current code is accepted and reports the current time step.
assert!(matches!(verify_totp(&secret, &current, ts, 0, 1), TotpValidation::Accepted(s) if s == step));
// The same code is rejected as reused once its step has been recorded.
assert!(matches!(verify_totp(&secret, &current, ts, step, 1), TotpValidation::Reused));
// A wrong code is rejected.
assert!(matches!(verify_totp(&secret, "000000", ts, 0, 1), TotpValidation::Rejected));
// The previous 30s window is accepted with one step of drift...
let previous = code_at(&secret, ts - 30);
assert!(matches!(verify_totp(&secret, &previous, ts, 0, 1), TotpValidation::Accepted(_)));
// ...but rejected when drift is disabled (steps = 0).
assert!(matches!(verify_totp(&secret, &previous, ts, 0, 0), TotpValidation::Rejected));
}
}

Loading…
Cancel
Save