0037-totp-last-used-step.md 17892 octets

RFC 0037 — Persister l’anti-rejeu TOTP (last_used_step)

  • Auteur : EBO (origine audit Gitrust / patch 1.0.1)
  • Date : 2026-09-18
  • Statut : Implemented (2026-10-01 ; Accepted 2026-09-18)
  • Apps consommatrices concernées : Gitrust (déploiements multi-réplicas) ; tout consommateur de rustwarden-core qui appelle TotpService::verify_code / verify_and_enable (API POST /api/v1/auth/2fa/verify, UI login 2FA, apps headless)
  • Crate cible : rustwarden-core
  • Origine : constat M-7 du plan docs/plans/2026-09-17-securisation-upstream-audit-gitrust.md (Task 14). RFC obligatoire : changement de schéma DB (RUSTWARDEN_EVOLUTION_PROCESS.md §2).

Summary

Persister en base le garde anti-rejeu TOTP livré in-process en 1.0.1 (matching_totp_step + cache OnceLock) via une colonne user_totp.last_used_step, mise à jour par un UPDATE conditionnel SeaORM, pour que deux réplicas n’acceptent plus le même code.

Motivation

Problème concret (audit M-7)

totp-rs avec pas de 30 s et skew ±1 accepte le même code pendant ~90 s. Le patch 1.0.1 ferme le rejeu dans un processus :

  • matching_totp_step (pub(crate), ~L34–62) : TOTP::check (skew ±1, constant_time_eq), puis identification du pas qui a matché parmi t-1, t, t+1 (unix_time / 30).
  • totp_replay_guard_accept (~L67–84) : cache OnceLock<Mutex<HashMap<(Uuid, u64), Instant>>>, TTL 90 s. Même (user_id, step) → false (sémantique identique à un code invalide, pas d’oracle).
  • TotpService::verify_code (~L249–268) : matching_totp_step puis le garde ; Ok(false) si rejeu.

Ce cache ne se partage pas entre processus. Deux réplicas Gitrust (ou un rolling restart) acceptent le même code une fois chacun. CHANGELOG.md 1.0.1 le documente : « Pas de migration last_used_step (RFC 0037) ».

Quel step stocker

Ne pas écrire now / 30 en aveugle. Un code de la fenêtre précédente (t-1) serait accepté, last_used_step passerait à t, et un second code du pas courant pourrait encore passer — ou l’inverse. Il faut retenir le pas renvoyé par matching_totp_step (celui dont totp.generate(step * 30) == code).

Qui est impacté

  • Gitrust et tout déploiement rustwarden-core à plus d’une instance.
  • Les utilisateurs 2FA : un code OTP déjà consommé reste invalide partout, pas seulement sur le réplica qui l’a vu.
  • Les opérateurs : une migration SeaORM (BIGINT NULL) au bump qui livre cette RFC.

Si on ne fait rien

Le garde 1.0.1 reste correct en instance unique. En multi-réplicas, M-7 reste ouvert. Gitrust devrait réimplémenter un store partagé (Redis, etc.) en divergence du framework.

Urgence

Pas un patch 1.0.1 (migration). Bloquant pour les déploiements HA dès que 1.0.1 est en prod. Pas d’incident unique documenté ; durcissement authn (critère RFC : schéma DB).

Non-goals

  • Pas de relais Evilginx live : un MITM interactif présente le code une seule fois ; ni le garde in-process ni last_used_step ne l’arrêtent.
  • Pas de changement d’algorithme / digits / période / skew : SHA1, 6 digits, 30 s, skew 1 (TOTP::new actuel).
  • Pas de backup codes : déjà one-shot (verify_backup_code retire le hash consommé).
  • Pas de Redis / store externe obligatoire : PostgreSQL (déjà la source de vérité user_totp) suffit.
  • Pas d’oracle : rejeu et code faux renvoient tous les deux Ok(false) / TotpFailed selon le call site actuel — pas de 409 distinct.
  • Pas de CHECK SQL sur la monotonie : la monotonie est garantie par le WHERE de l’UPDATE.
  • Pas de SQL brut applicatif : SeaORM uniquement (update_many + filtres). La migration utilise SchemaManager::alter_table.

API proposée

Aucun nouvel item pub obligatoire. Comportement de TotpService::verify_code (et verify_and_enable) enrichi. Schéma + modèle.

Schéma

Migration crates/rustwarden-core/src/migrations/m20260918_000010_totp_last_used_step.rs, enregistrée dans migrations/mod.rs (pub mod + core_migrations()). Prochain numéro après m20260510_000009_fediverse_interop.

// crates/rustwarden-core/src/migrations/m20260918_000010_totp_last_used_step.rs
use sea_orm_migration::prelude::*;

#[derive(DeriveMigrationName)]
pub struct Migration;

#[async_trait::async_trait]
impl MigrationTrait for Migration {
    async fn up(&self, manager: &SchemaManager) -> Result<(), DbErr> {
        manager
            .alter_table(
                Table::alter()
                    .table(UserTotp::Table)
                    .add_column(
                        ColumnDef::new(UserTotp::LastUsedStep)
                            .big_integer()
                            .null(),
                    )
                    .to_owned(),
            )
            .await
    }

    async fn down(&self, manager: &SchemaManager) -> Result<(), DbErr> {
        manager
            .alter_table(
                Table::alter()
                    .table(UserTotp::Table)
                    .drop_column(UserTotp::LastUsedStep)
                    .to_owned(),
            )
            .await
    }
}

#[derive(Iden)]
enum UserTotp {
    Table,
    LastUsedStep,
}

Équivalent SQL (documentation ; non exécuté par l’app) :

ALTER TABLE user_totp ADD COLUMN last_used_step BIGINT NULL;

Réversible : DROP COLUMN last_used_step. Les lignes existantes restent NULL → premier code post-migration toujours accepté.

Modèle

user_totp::Model aujourd’hui : id, user_id, secret_encrypted, enabled, backup_codes_encrypted, backup_codes_remaining, enabled_at, created_at, updated_at. Après Accept :

/// Last TOTP time-step (`unix_epoch / 30`) that was successfully consumed.
/// `None` until the first accepted code after this column exists.
pub last_used_step: Option<i64>,

Type i64 : SeaORM mappe PostgreSQL BIGINT. matching_totp_step renvoie u64 ; conversion i64::try_from(step) — un pas Unix/30 tient dans i64 pour des siècles. Échec de conversion → traiter comme code invalide (Ok(false)), pas de panic.

setup (TotpService insert enabled=false) : last_used_step: NotSet (NULL).

Concurrence (source de vérité)

Succès ssi rows_affected == 1 :

UPDATE user_totp
SET last_used_step = $step
WHERE user_id = $id
  AND (last_used_step IS NULL OR last_used_step < $step)

Comparaison stricte < : le même pas est un rejeu. Un pas plus grand (fenêtre suivante) passe.

Proposition SeaORM (pas de SQL applicatif) :

use chrono::Utc;
use sea_orm::{
    ColumnTrait, Condition, EntityTrait, QueryFilter, sea_query::Expr,
};
use crate::models::user_totp;

/// Consume `step` for `user_id`. Returns true iff this replica won the CAS.
async fn claim_totp_step(
    db: &sea_orm::DatabaseConnection,
    user_id: uuid::Uuid,
    step: u64,
) -> Result<bool, crate::error::AppError> {
    let step_i64 = i64::try_from(step).map_err(|_| {
        crate::error::AppError::Internal("TOTP step does not fit in i64".into())
    })?;

    let result = user_totp::Entity::update_many()
        .col_expr(user_totp::Column::LastUsedStep, Expr::value(step_i64))
        .col_expr(user_totp::Column::UpdatedAt, Expr::value(Utc::now()))
        .filter(user_totp::Column::UserId.eq(user_id))
        .filter(
            Condition::any()
                .add(user_totp::Column::LastUsedStep.is_null())
                .add(user_totp::Column::LastUsedStep.lt(step_i64)),
        )
        .exec(db)
        .await?;

    Ok(result.rows_affected == 1)
}

Les snippets ci-dessus sont proposés (Draft) : ils ne compilent pas tant que la colonne n’existe pas.

Flux verify_code après Accept

Ordre (évite de polluer le cache in-process si le CAS DB perd) :

  1. Charger user_totp enabled=true (inchangé) ; sinon AppError::TotpNotEnabled.
  2. matching_totp_step(&secret, code) → None ⇒ Ok(false) (aucun UPDATE).
  3. claim_totp_step(db, user_id, step) → false ⇒ Ok(false) (rejeu inter-réplicas ou même pas).
  4. Optionnel : totp_replay_guard_accept après le CAS gagné, comme court-circuit intra-processus pour les rejeux suivants sur ce réplica (évite un UPDATE no-op). Le CAS DB reste autoritatif.
  5. Ok(true).

verify_and_enable (~L208) appelle aujourd’hui verify_totp_code sans garde in-process. Après Accept : après match, claim_totp_step puis enabled=true. Le code de setup ne peut pas servir immédiatement de login sur un autre réplica.

Signatures inchangées :

impl TotpService {
    pub async fn verify_code(
        db: &DatabaseConnection,
        user_id: Uuid,
        code: &str,
    ) -> Result<bool, AppError> { /* ... */ }

    pub async fn verify_and_enable(
        db: &DatabaseConnection,
        user_id: Uuid,
        code: &str,
    ) -> Result<Vec<String>, AppError> { /* ... */ }
}

Usage consommateur

Aucun changement d’appel. Gitrust continue :

let accepted = rustwarden_core::services::totp_service::TotpService::verify_code(
    db, user_id, code,
).await?;
if !accepted {
    // invalid or replay — same branch, no oracle
}

Analyse de compatibilité

Checklist :

  • API existante non modifiée → non-breaking compile (signatures verify_code / verify_and_enable inchangées).
  • API existante étendue avec défaut → N/A (pas de nouveau pub requis).
  • API existante modifiée ou supprimée → non.
  • Nouveau comportement par défaut → un code déjà consommé sur le réplica A est refusé sur B (voulu). Instance unique : déjà le cas via le garde 1.0.1.
  • Changement de schéma DB → oui : last_used_step BIGINT NULL, migration up/down. Pas de rewrite de données. NULL = grandfathering du premier code.
  • Changement de format sérialisé → non (user_totp n’est pas un DTO JSON public).

SemVer : mineure (schéma + comportement de durcissement, signatures stables). Processus §1 : un changement de défaut peut être breaking ; ici le 1.0.1 a déjà le même rejet en mono-instance. Documenter en Security dans le CHANGELOG.

Alternatives considérées

Minimum 3 alternatives avec analyse pros/cons, y compris "ne rien faire". Sans ça la RFC est incomplète : on ne peut pas juger une proposition sans son coût relatif.

Alternative A (recommandée) : colonne last_used_step + CAS SeaORM

Pros : correct dès 2 réplicas sans Redis. Step matché (pas now/30). Atomicité PostgreSQL. Réversible (NULL / DROP COLUMN). Réutilise matching_totp_step déjà testé. Pas d’oracle. Constitution : pas de SQL applicatif.

Cons : un UPDATE de plus sur le hot path 2FA (déjà un SELECT + decrypt). Migration à jouer sur toutes les apps. Tests de concurrence plus lourds.

Alternative B : garde in-process uniquement (statut 1.0.1)

Pros : zéro migration, déjà livré, TTL 90 s, tests unitaires verts (matching_totp_step_*, totp_replay_guard_accept).

Cons : insuffisant dès 2 réplicas (M-7). Rolling restart vide le OnceLock. Sticky sessions ne sont pas une garantie de sécu. Gitrust devrait forker un store.

Alternative C (ne rien faire / persister rien)

Pros : zéro coût, zéro risque de migration.

Cons : rejeu inter-instances ouvert à vie ; dette consommateur ; contredit l’audit M-7 et le CHANGELOG qui pointe déjà cette RFC.

Testing plan

Tests unitaires (étendre totp_service.rs tests ~L623+) :

  • Happy path : matching_totp_step renvoie le pas courant ; après claim_totp_step une fois, last_used_step == Some(step).
  • Erreur 1 — input invalide : code "1" / 5 digits → matching_totp_step == None, aucun UPDATE (assert DB inchangée).
  • Erreur 2 — contrainte violée (rejeu même pas) : second claim_totp_step / verify_code avec le même code → Ok(false), rows_affected == 0, last_used_step inchangé.
  • Erreur 3 — fenêtre plus ancienne : accepter t, puis présenter le code de t-1 → matching_totp_step peut encore matcher le secret, mais last_used_step < t-1 est faux → Ok(false).
  • Erreur 4 (timeout / panic path) : i64::try_from n’est pas exercé en pratique ; tester claim_totp_step si update_many renvoie DbErr → AppError::Database, pas Ok(true) fail-open.

Tests d’intégration (service / DB) :

  • verify_code happy path sur DB de test : premier code true, rejeu false.
  • Deux tâches tokio concurrentes, même (user_id, step) : exactement une true.
  • Migration up : colonne nullable présente ; rows existantes NULL ; premier verify OK. Migration down : colonne absente, données user_totp restantes intactes (secret_encrypted, enabled).
  • verify_and_enable pose last_used_step ; un verify_code immédiat avec le même code → false.

Tests de non-régression :

  • Code consommateur compile (signatures inchangées).
  • Backup codes : verify_backup_code ne lit/écrit pas last_used_step.
  • Garde in-process 1.0.1 : tests totp_replay_guard_accept restent verts (court-circuit optionnel).
  • Baseline Draft : cargo test -p rustwarden-core totp_service --lib (garde + matching_totp_step) — à relancer après impl.

Doctests :

  • Pas de nouvel item pub. Enrichir la rustdoc de TotpService::verify_code (anglais) : replay → Ok(false). Exemple ignore si DB requise ; un doctest exécutable n’est pas réaliste sans secret + horloge.

Benchs : comparer latence verify_code avant/après (SELECT+decrypt vs +UPDATE). Régression > 10 % bloque le merge sans justification. Attendu : un UPDATE PK user_id ≪ bcrypt backup codes.

Les exemples claim_totp_step ne sont pas exécutables avant l’implémentation.

Migration guide

Section à copier telle quelle dans le CHANGELOG.md au moment du merge.

### API changes

**Before (v1.0.1)** :
`TotpService::verify_code` refusait le rejeu du même pas TOTP **dans le
processus** (cache 90 s). Deux réplicas pouvaient accepter le même code.
Pas de colonne `last_used_step`.

**After (vX.Y.Z)** :
Colonne `user_totp.last_used_step BIGINT NULL`. Après `matching_totp_step`
(pas qui a matché, pas `now/30`), un `UPDATE` conditionnel SeaORM
(`last_used_step IS NULL OR last_used_step < $step`) consomme le pas.
Succès ssi `rows_affected == 1` ; sinon `Ok(false)` (même sémantique
qu'un code invalide). `verify_and_enable` consomme aussi le pas.

Aucune signature publique changée.

### Migration steps

1. Bump `rustwarden-core` à la version qui livre RFC 0037.
2. Jouer les migrations (`cargo run --bin migrate` ou `auto_migrate`) :
   `m20260918_000010_totp_last_used_step`.
3. Aucun rewrite de données. Les secrets TOTP existants restent valides.
4. Redémarrer **tous** les réplicas après la migration (rolling OK : un
   réplica pré-colonne crashera au SELECT/UPDATE tant que le schéma n'est
   pas à jour — migrer avant le binaire).
5. Gitrust : aucun workaround Redis à conserver pour ce rejeu.

Open questions

Liste des points en suspens. La RFC ne peut pas être acceptée tant qu’ils ne sont pas résolus :

  • Garde in-process après CAS : le conserver comme court-circuit (recommandé) ou le retirer une fois la colonne livrée (moins de chemins) ? Résolu (2026-09-18) : conservée comme court-circuit intra-processus après CAS gagné ; le CAS DB reste autoritatif.
  • verify_and_enable : confirmer qu’il doit claim_totp_step (recommandé) pour bloquer le replay setup→login. Résolu (2026-09-18) : confirmé — verify_and_enable consomme aussi le pas via claim_totp_step (bloque le rejeu setup→login).
  • SemVer : minor Security vs major ? Signatures stables, schéma additif nullable. Résolu (2026-09-18) : 1.1.0 minor Security (schéma additif nullable, signatures stables).
  • Tracking GitHub : ouvrir l’issue avant Under Review. Résolu (2026-09-18) : suivi interne via docs/plans/2026-09-17-securisation-upstream-audit-gitrust.md.

Tracking issue

À ouvrir sur le dépôt Rustwarden (labels area:auth + security + rfc) en référençant l’audit M-7, le CHANGELOG 1.0.1 et Gitrust comme premier consommateur HA. Pas d’URL tant que le Draft n’est pas soumis.


Score d'évaluation (rempli par les reviewers)

Notes proposées par l’auteur (Draft) ; les reviewers confirment ou ajustent. Seuil Accepted : ≥ 25/35.

CritèreNote (1-5)Commentaire
Généralité5Tout consommateur 2FA multi-instance ; Gitrust n’a plus à forker un store.
Stabilité API5Signatures inchangées ; colonne nullable ; NULL = premier code OK.
Performance4Un UPDATE PK de plus sur le path 2FA ; pas de bcrypt. Bench à coller au merge.
Sécurité5Ferme M-7 inter-réplicas ; pas d’oracle ; step matché (pas now/30) ; CAS.
Complexité d'implémentation4Migration SeaORM + update_many + tests concurrence ; pas d’unsafe.
Charge de maintenance4Une colonne, pas de Redis ; garde in-process optionnel à ne pas faire dériver.
Documentation4RFC + CHANGELOG + rustdoc verify_code ; migration up/down.
Total31/35(≥ 25 pour Accepted)