max / makenotwork
1 file changed,
+29 insertions,
-14 deletions
| @@ -178,22 +178,37 @@ pub(super) async fn verify_two_factor( | |||
| 178 | 178 | let mut verified = false; | |
| 179 | 179 | ||
| 180 | 180 | // Try TOTP verification first (6-digit numeric codes). The stored secret is | |
| 181 | - | // encrypted at rest; decrypt it (legacy plaintext rows pass through) before | |
| 182 | - | // building the verifier. | |
| 181 | + | // encrypted at rest and MUST carry the `enc:v1:` prefix; there is no legacy | |
| 182 | + | // plaintext fallback (see `crypto::decrypt_totp_secret`). A secret that fails | |
| 183 | + | // to decrypt (un-migrated plaintext row, wrong key, tampering) must NOT 500 | |
| 184 | + | // the whole login — that would also skip the backup-code path below and lock | |
| 185 | + | // the user out entirely. Log it and fall through to backup codes instead. | |
| 183 | 186 | if let Some(ref stored_secret) = user.totp_secret { | |
| 184 | - | let secret = crate::crypto::decrypt_totp_secret(stored_secret, &state.config.signing_secret)?; | |
| 185 | - | let totp = build_totp(&secret, &user.email)?; | |
| 186 | - | // Find the actual step that matched (not just wall-clock step) to | |
| 187 | - | // prevent replay across the skew boundary. | |
| 188 | - | let now = chrono::Utc::now().timestamp() as u64; | |
| 189 | - | let matched_step = find_matching_step(&totp, &code, now); | |
| 190 | - | if let Some(step) = matched_step { | |
| 191 | - | let last_step = db::totp::get_totp_last_used_step(&state.db, user_id).await?.unwrap_or(0); | |
| 192 | - | if step > last_step { | |
| 193 | - | db::totp::set_totp_last_used_step(&state.db, user_id, step).await?; | |
| 194 | - | verified = true; | |
| 187 | + | match crate::crypto::decrypt_totp_secret(stored_secret, &state.config.signing_secret) { | |
| 188 | + | Ok(secret) => { | |
| 189 | + | let totp = build_totp(&secret, &user.email)?; | |
| 190 | + | // Find the actual step that matched (not just wall-clock step) to | |
| 191 | + | // prevent replay across the skew boundary. | |
| 192 | + | let now = chrono::Utc::now().timestamp() as u64; | |
| 193 | + | let matched_step = find_matching_step(&totp, &code, now); | |
| 194 | + | if let Some(step) = matched_step { | |
| 195 | + | let last_step = db::totp::get_totp_last_used_step(&state.db, user_id).await?.unwrap_or(0); | |
| 196 | + | if step > last_step { | |
| 197 | + | db::totp::set_totp_last_used_step(&state.db, user_id, step).await?; | |
| 198 | + | verified = true; | |
| 199 | + | } | |
| 200 | + | // If step <= last_step, the code was already used — fall through to backup codes | |
| 201 | + | } | |
| 202 | + | } | |
| 203 | + | Err(e) => { | |
| 204 | + | // Decrypt failure is not a verification failure for the user to | |
| 205 | + | // see — fall through to backup codes, which are the recovery path. | |
| 206 | + | tracing::error!( | |
| 207 | + | user_id = %user_id, | |
| 208 | + | error = ?e, | |
| 209 | + | "TOTP secret failed to decrypt; falling through to backup codes", | |
| 210 | + | ); | |
| 195 | 211 | } | |
| 196 | - | // If step <= last_step, the code was already used — fall through to backup codes | |
| 197 | 212 | } | |
| 198 | 213 | } | |
| 199 | 214 |