PD-238 — Revue de sécurité¶

Date : 2026-02-06 Auteur : agent-adversarial (red team) Scope : MFA Management — Keycloak Admin API integration Commit de base : dev branch, post-PD-238 implementation

Résumé exécutif¶

• Statut global : PASS_WITH_OBSERVATIONS
• Nombre de vulnérabilités critiques : 0
• Nombre de vulnérabilités majeures : 2
• Nombre d'observations mineures : 4

L'implémentation PD-238 présente une posture de sécurité globalement solide. Les trois invariants audités (INV-238-09, INV-238-10, INV-238-02) sont conformes. Deux points majeurs méritent attention : le fail-open du rate limiter Redis et l'implémentation custom du constant-time comparison. Quatre observations mineures sont documentées.

Vérification des invariants¶

INV-238-09 : Secrets jamais loggés¶

• Statut : CONFORME

Méthodologie : Recherche exhaustive par grep de tous les appels console.log, logger.log, logger.debug, logger.info, logger.warn, logger.error dans le périmètre audité, croisée avec les mots-clés secret, qrCodeUri, recoveryCodes, password, token.

Observations :

1. Aucun console.log trouvé dans l'ensemble du périmètre src/modules/auth/mfa/.

2. Logs service keycloak-admin.service.ts — Tous les messages de log utilisent des messages génériques sans données sensibles :

3. Ligne 239 : Getting MFA status for user (pas de userId loggé)
4. Ligne 288 : Initializing TOTP for user (pas de secret)
5. Ligne 336 : Verifying TOTP for user (pas de code TOTP)
6. Ligne 379 : Disabling MFA for user
7. Ligne 415 : Regenerating recovery codes for user

8. Lignes 296-297 : Commentaires explicites // INV-238-09: Do NOT log the response containing secrets

9. Logs service mfa-management.service.ts — Même rigueur :

10. Aucun log ne contient secret, qrCodeUri, recoveryCodes, ni le code TOTP

11. Ligne 178 : No TOTP session found for user, init may have expired — correct, pas de fuite

12. Logs reauth.service.ts — Le mot password n'apparaît jamais dans aucun appel logger :

13. Ligne 75 : Reauth attempt for userId=${userId} — seul userId loggé
14. Ligne 93 : Reauth failed: Invalid password userId=${userId} — mentionne le mot "password" dans le message descriptif, mais ne logge pas la valeur du password. Acceptable mais voir OBS-03.

15. Ligne 185 : Password verification error: ${error.message} — logge le message d'erreur, qui ne devrait pas contenir le password (provient d'opérations crypto). Acceptable mais surveiller les messages d'erreur des bibliothèques crypto.

16. Logs reauth-token.guard.ts — Les messages de log concernant les tokens sont descriptifs :

17. Reauth token absent, Reauth token signature invalid or expired, etc.

18. Aucune valeur de token n'est loggée.

19. Logs mfa-exception.filter.ts — Ligne 57 : logge exception.code et exception.message qui sont des constantes définies dans mfa-management.errors.ts, jamais des secrets.

20. DTOs avec @Exclude() — Protection contre la sérialisation accidentelle :

21. TotpInitResponseDto : class-level @Exclude(), champs sensibles avec @Expose() explicite
22. TotpVerifyResponseDto : class-level @Exclude(), recoveryCodes avec @Expose() explicite
23. RecoveryRegenerateResponseDto : class-level @Exclude(), recoveryCodes avec @Expose() explicite

Note : L'annotation @Exclude() au niveau de la classe avec @Expose() sélectif fonctionne uniquement si ClassSerializerInterceptor est activé globalement ou au niveau du controller. Aucun ClassSerializerInterceptor n'a été trouvé dans le codebase (ni dans main.ts, ni comme APP_INTERCEPTOR). Cela signifie que les annotations @Exclude()/@Expose() sont documentaires mais ne sont pas fonctionnellement actives. Voir VUL-02.

1. ReauthRequestDto — Le champ password n'a pas de @Exclude(). Ce DTO est un DTO d'entrée (request body), pas de réponse, donc le risque est limité. Cependant, si un intercepteur de logging serialise le body de la requête, le password serait exposé. Voir OBS-01.

Fichiers audités : - src/modules/auth/mfa/services/keycloak-admin.service.ts - src/modules/auth/mfa/services/mfa-management.service.ts - src/modules/auth/mfa/controllers/mfa-management.controller.ts - src/modules/auth/services/reauth.service.ts - src/modules/auth/controllers/reauth.controller.ts - src/modules/auth/mfa/guards/reauth-token.guard.ts - src/modules/auth/guards/mfa-rate-limit.guard.ts - src/modules/auth/mfa/errors/mfa-management.errors.ts - src/modules/auth/mfa/filters/mfa-exception.filter.ts - src/modules/auth/mfa/dto/*.ts - src/modules/auth/dto/reauth.dto.ts

INV-238-10 : Pas de stockage local¶

• Statut : CONFORME

Méthodologie : Recherche de toute opération de persistance (TypeORM .save(), .insert(), .create(), .update(), repository) liée aux mots-clés secret, totp, recovery, qrCode. Analyse des opérations Redis.

Observations :

1. Aucune insertion en base de données de secrets TOTP, qrCodeUri, ou codes de récupération. Les services MFA n'injectent aucun Repository TypeORM pour des entités liées au MFA. La seule injection TypeORM dans reauth.service.ts est pour User (vérification du mot de passe), pas pour stocker des secrets MFA.

2. Redis — sessionId uniquement :

3. mfa-management.service.ts:320 : redis.set(key, sessionId, 'EX', TOTP_SESSION_TTL_SECONDS)
4. Le TTL est de 300 secondes (5 minutes) — CONFORME
5. La clé stockée est mfa:totp:session:{userId} avec pour valeur le sessionId Keycloak uniquement
6. Le secret TOTP, le qrCodeUri et les recovery codes ne sont jamais stockés dans Redis

7. Le sessionId est supprimé après activation réussie (deleteSessionId, ligne 349)

8. Pattern pass-through : Le service keycloak-admin.service.ts retourne les secrets directement depuis Keycloak sans les persister :

9. initTotp() : retourne le TotpInitResult en mémoire transitoire
10. verifyAndActivateTotp() : retourne les recoveryCodes en mémoire transitoire
11. regenerateRecoveryCodes() : retourne les codes en mémoire transitoire

12. Aucune variable de classe ne stocke ces secrets

13. Token admin Keycloak : Le accessToken est stocké en mémoire (propriété de l'instance). C'est un token de service account, pas un secret MFA utilisateur. Acceptable car il s'agit d'un cache applicatif standard pour l'authentification service-to-service.

Fichiers audités : - src/modules/auth/mfa/services/keycloak-admin.service.ts - src/modules/auth/mfa/services/mfa-management.service.ts - src/modules/auth/mfa/controllers/mfa-management.controller.ts

INV-238-02 : Pas d'accès croisé¶

• Statut : CONFORME

Méthodologie : Analyse de tous les endpoints pour vérifier que le userId est systématiquement extrait du JWT (sub claim) et jamais d'un paramètre client (URL param, query param, body).

Observations :

1. Controller MFA (mfa-management.controller.ts) — Tous les 5 endpoints utilisent @CurrentUser() user: JwtPayload et passent user.sub au service :
2. getStatus() : user.sub → ligne 84
3. initTotp() : user.sub → ligne 117
4. verifyTotp() : user.sub → ligne 153
5. disable() : user.sub → ligne 193

6. regenerateRecoveryCodes() : user.sub → ligne 235

7. Controller Reauth (reauth.controller.ts) — Utilise @CurrentUser() user: JwtPayload et passe user.sub :

8. reauth() : user.sub → ligne 75

9. Aucun paramètre URL userId ou keycloakUserId — Recherche exhaustive de @Param et @Query.*userId dans les controllers : aucun résultat.

10. Décorateur @CurrentUser() — Extrait request.user qui est peuplé par OidcJwtAuthGuard à partir du JWT validé. Le claim sub provient exclusivement du token JWT signé, impossible à falsifier sans la clé de signature.

11. Guard ReauthTokenGuard — Vérifie que le sub du reauth token correspond au sub du JWT principal (ligne 81 : payload.sub !== principalUserId). Cela empêche un attaquant qui obtiendrait un reauth token d'un autre utilisateur de l'utiliser.

12. Services — Les méthodes de service prennent un userId: string en paramètre, qui provient toujours de user.sub via le controller. Aucun chemin alternatif n'existe.

Fichiers audités : - src/modules/auth/mfa/controllers/mfa-management.controller.ts - src/modules/auth/controllers/reauth.controller.ts - src/modules/auth/decorators/current-user.decorator.ts - src/modules/auth/mfa/guards/reauth-token.guard.ts

Analyse OWASP Top 10¶

A01:2021 — Broken Access Control¶

Statut : Bien couvert

• Tous les endpoints MFA sont protégés par OidcJwtAuthGuard (au niveau controller class)
• Les opérations sensibles (disable, regenerate) requièrent un ReauthTokenGuard additionnel
• Le userId est toujours extrait du JWT, jamais du client
• Le ReauthTokenGuard vérifie la correspondance sub entre le token principal et le reauth token
• Rate limiting appliqué via MfaRateLimitGuard sur tous les endpoints

Observation : Le guard MfaRateLimitGuard a un fail-open en cas d'indisponibilité Redis (ligne 91-93). Voir VUL-01.

A02:2021 — Cryptographic Failures¶

Statut : Acceptable avec observations

• Le reauth token utilise HS256 (symétrique) via JwtModule — acceptable pour un token interne court-lived (5 min)
• La vérification SRP utilise SHA3-256 pour le hashing, cohérent avec le protocole SRP
• L'implémentation constantTimeEqual est custom (voir VUL-03 ci-dessous)
• Les secrets TOTP sont gérés par Keycloak, pas par l'application

Observation : Le reauth token utilise la même clé JWT (security.jwt.secret) que les tokens principaux. Cela n'est pas critique car le champ purpose: 'reauth' est vérifié, mais une clé dédiée offrirait une meilleure séparation. Voir OBS-04.

A03:2021 — Injection¶

Statut : Bien couvert

• Les DTOs d'entrée utilisent class-validator avec des contraintes strictes :
• TotpVerifyRequestDto.code : @IsString(), @Length(6, 6), @Matches(/^[0-9]+$/)
• ReauthRequestDto.password : @IsString(), @MinLength(1)
• ValidationPipe global avec whitelist: true et forbidNonWhitelisted: true dans main.ts
• Les requêtes vers Keycloak utilisent JSON.stringify(body), pas de concaténation de chaînes dans les URLs (le keycloakUserId provient du JWT, pas d'un input utilisateur direct)
• Pas de requêtes SQL directes, utilisation de TypeORM avec paramètres

Pas de vulnérabilité d'injection identifiée.

A07:2021 — Identification and Authentication Failures¶

Statut : Bien couvert

• L'authentification JWT est obligatoire sur tous les endpoints (class-level guard)
• La réauthentification par mot de passe est requise pour les opérations destructives
• Le reauth token a une durée de vie courte (5 min, configurable)
• Le token TOTP session a un TTL de 5 min dans Redis
• Rate limiting prévient le brute-force sur les codes TOTP
• Le ReauthTokenGuard vérifie la signature JWT, le purpose, le sub, et l'expiration
• Les erreurs d'authentification ne révèlent pas si l'utilisateur existe (Re-authentication failed vs User not found dans les logs seulement, pas dans la réponse HTTP)

Observation : Le ReauthService distingue USER_NOT_FOUND et INVALID_PASSWORD dans les logs (lignes 84 et 93), mais renvoie le même MfaReauthFailedError au client. C'est correct pour la réponse HTTP, mais les logs pourraient être utilisés pour de l'énumération d'utilisateurs par un attaquant avec accès aux logs.

Vulnérabilités identifiées¶

[MAJEURE] VUL-01 — Rate limiter fail-open en cas d'indisponibilité Redis¶

• Description : Le MfaRateLimitGuard retourne true (autorise la requête) lorsque Redis n'est pas disponible (mfa-rate-limit.guard.ts:91-93). Un attaquant qui pourrait provoquer l'indisponibilité de Redis (ou un incident réseau) pourrait contourner le rate limiting et tenter un brute-force sur le code TOTP ou le mot de passe de réauthentification.

• Impact : Brute-force possible sur /user/mfa/totp/verify (code TOTP 6 digits = 1M combinaisons) et /auth/reauth (mots de passe) en cas d'indisponibilité Redis.

• Fichier concerné : src/modules/auth/guards/mfa-rate-limit.guard.ts:89-93

• Remédiation proposée :

• Implémenter un rate limiter in-memory en fallback (compteur local avec Map + TTL)
• OU basculer vers un fail-closed qui rejette les requêtes si Redis est indisponible, avec monitoring/alerting sur cet état
• Ajouter un compteur de tentatives TOTP côté Keycloak comme defense-in-depth
• Documenter la décision architecturale si le fail-open est intentionnel (disponibilité vs sécurité)

[MAJEURE] VUL-02 — @Exclude() / @Expose() non fonctionnels sans ClassSerializerInterceptor¶

• Description : Les DTOs TotpInitResponseDto, TotpVerifyResponseDto et RecoveryRegenerateResponseDto utilisent les décorateurs @Exclude() et @Expose() de class-transformer pour empêcher la sérialisation accidentelle des secrets. Cependant, aucun ClassSerializerInterceptor n'est enregistré (ni globalement dans main.ts, ni au niveau des controllers). Les décorateurs @Exclude()/@Expose() nécessitent cet intercepteur pour être fonctionnellement actifs.

En l'état actuel, si un middleware ou intercepteur tiers appelle JSON.stringify() ou plainToInstance() sur ces DTOs, les champs ne seront pas filtrés. Les annotations sont purement documentaires.

• Impact : La protection contre la sérialisation accidentelle des secrets (INV-238-09 couche DTO) n'est pas active. Tout intercepteur de logging, middleware de monitoring, ou sérialisation manuelle inclura les secrets dans le résultat.

• Fichier concerné :

• src/modules/auth/mfa/dto/totp-init.dto.ts
• src/modules/auth/mfa/dto/totp-verify.dto.ts
• src/modules/auth/mfa/dto/recovery-regenerate.dto.ts

• src/main.ts (absence de ClassSerializerInterceptor)

• Remédiation proposée :

• Ajouter ClassSerializerInterceptor globalement dans main.ts :

  app.useGlobalInterceptors(new ClassSerializerInterceptor(app.get(Reflector)));
  

• OU l'ajouter au niveau du MfaManagementController :

  @UseInterceptors(ClassSerializerInterceptor)
  

• Ajouter un test unitaire vérifiant que la sérialisation d'un TotpInitResponseDto sans @Expose() exclut bien les champs sensibles

[MINEURE] VUL-03 — constantTimeEqual custom au lieu de crypto.timingSafeEqual¶

• Description : Le ReauthService implémente une fonction constantTimeEqual custom (ligne 214-225) pour la comparaison des verifiers SRP. Bien que l'implémentation semble correcte (XOR byte-by-byte), elle présente un early-return sur la comparaison de longueur (ligne 215 : if (a.length !== b.length) return false), ce qui constitue une fuite d'information potentielle sur la longueur du verifier.

De plus, Node.js fournit crypto.timingSafeEqual() qui est implémenté en C et audité, donc plus fiable qu'une implémentation userland en JavaScript.

• Impact : Faible — Un attaquant devrait pouvoir mesurer des variations de timing de l'ordre de la nanoseconde sur le réseau, ce qui est extrêmement difficile en pratique. Le early-return sur la longueur est mitigé par le fait que les verifiers SRP ont toujours une longueur déterministe pour un même paramètre N.

• Fichier concerné : src/modules/auth/services/reauth.service.ts:214-225

• Remédiation proposée :

  import { timingSafeEqual } from 'crypto';
  
  private constantTimeEqual(a: string, b: string): boolean {
    const bufA = Buffer.from(a, 'utf8');
    const bufB = Buffer.from(b, 'utf8');
    if (bufA.length !== bufB.length) return false;
    return timingSafeEqual(bufA, bufB);
  }
  

[MINEURE] OBS-01 — ReauthRequestDto sans @Exclude() sur le champ password¶

• Description : Le DTO ReauthRequestDto ne dispose pas d'un décorateur @Exclude() sur le champ password. Bien que ce soit un DTO d'entrée (request body), si un intercepteur de logging ou d'audit serialise automatiquement le body des requêtes, le mot de passe sera inclus en clair.

• Impact : Faible — Dépend de la présence d'intercepteurs de logging du body. Le AccessAuditInterceptor présent dans le codebase pourrait potentiellement loguer le body.

• Fichier concerné : src/modules/auth/dto/reauth.dto.ts

• Remédiation proposée : Ajouter @Exclude() au niveau de la classe et @Expose() seulement sur les champs non-sensibles, ou ne pas exposer le password via le toString/toJSON. Alternativement, vérifier que l'AccessAuditInterceptor exclut explicitement les bodies des endpoints d'authentification.

[MINEURE] OBS-02 — Pas de MfaRateLimitGuard sur POST /auth/reauth¶

• Description : L'endpoint POST /auth/reauth n'utilise pas le MfaRateLimitGuard. Il est protégé par OidcJwtAuthGuard uniquement. Un attaquant avec un JWT valide pourrait tenter de brute-forcer le mot de passe via la réauthentification sans limitation de débit spécifique.

Il est possible qu'un RateLimitGuard générique couvre ce cas via le module AuthModule, mais il n'est pas explicitement appliqué sur le endpoint POST /auth/reauth dans reauth.controller.ts.

• Impact : Moyen — Un attaquant authentifié pourrait tenter un nombre illimité de mots de passe via /auth/reauth.

• Fichier concerné : src/modules/auth/controllers/reauth.controller.ts

• Remédiation proposée : Appliquer @UseGuards(MfaRateLimitGuard) ou un guard de rate limiting dédié sur l'endpoint POST /auth/reauth. Considérer un rate limit plus restrictif (ex: 5 tentatives / 5 min) car cet endpoint accepte un mot de passe.

[MINEURE] OBS-03 — Messages de log distinguant USER_NOT_FOUND et INVALID_PASSWORD¶

• Description : Dans reauth.service.ts, les logs distinguent Reauth failed: User not found (ligne 84) et Reauth failed: Invalid password (ligne 93). Bien que la réponse HTTP soit identique (MfaReauthFailedError), un attaquant avec accès aux logs (via compromission du système de logging) pourrait utiliser ces messages pour déterminer si un userId existe.

• Impact : Très faible — Nécessite un accès aux logs de l'application, ce qui implique déjà une compromission partielle.

• Fichier concerné : src/modules/auth/services/reauth.service.ts:84,93

• Remédiation proposée : Unifier les messages de log : Reauth failed for userId=${userId} sans distinguer la raison. La raison peut être émise via l'event de sécurité (déjà fait via emitReauthFailed) pour le monitoring, mais le logger textuel ne devrait pas distinguer.

[MINEURE] OBS-04 — Clé JWT partagée entre tokens principaux et reauth tokens¶

• Description : Le MfaModule configure JwtModule avec la même clé security.jwt.secret que celle utilisée pour les tokens principaux dans AuthModule. Bien que le champ purpose: 'reauth' soit vérifié dans le ReauthTokenGuard, le partage de clé signifie qu'un token principal pourrait théoriquement être présenté comme reauth token si le guard ne vérifie pas le purpose (ce qu'il fait correctement).

• Impact : Très faible — Le ReauthTokenGuard vérifie payload.purpose === 'reauth' (ligne 74), donc un token principal (sans champ purpose) serait rejeté. La séparation de clés est une bonne pratique de defense-in-depth mais non critique ici.

• Fichier concerné : src/modules/auth/mfa/mfa.module.ts:61-71

• Remédiation proposée : Considérer l'utilisation d'une clé JWT dédiée (security.jwt.reauthSecret) pour les reauth tokens. Cela offre une isolation cryptographique : la compromission de la clé principale ne compromettrait pas les reauth tokens et vice-versa.

Synthèse des fichiers audités¶

Recommandations¶

Priorité haute¶

1. [VUL-01] Implémenter un fallback in-memory pour le rate limiter — Le fail-open actuel est un choix de disponibilité qui compromet la sécurité. Ajouter un rate limiter local en mémoire comme fallback quand Redis est indisponible. Un simple Map<string, { count: number, resetAt: number }> suffit pour un fallback basique.

2. [VUL-02] Activer ClassSerializerInterceptor — Soit globalement dans main.ts, soit au niveau du controller MFA. Valider par un test que les champs sensibles sont bien exclus lors de la sérialisation. C'est une protection defense-in-depth essentielle pour INV-238-09.

Priorité moyenne¶

1. [OBS-02] Ajouter un rate limiting sur POST /auth/reauth — Cet endpoint accepte un mot de passe et doit être protégé contre le brute-force au même titre que les endpoints MFA.

2. [VUL-03] Utiliser crypto.timingSafeEqual au lieu de l'implémentation custom. C'est une correction simple qui élimine un risque théorique de timing attack.

Priorité basse¶

1. [OBS-01] Protéger le champ password du ReauthRequestDto contre la sérialisation accidentelle, ou vérifier que les intercepteurs d'audit excluent le body de ce endpoint.

2. [OBS-03] Unifier les messages de log de réauthentification pour ne pas distinguer USER_NOT_FOUND et INVALID_PASSWORD dans les logs textuels.

3. [OBS-04] Évaluer la séparation des clés JWT pour les reauth tokens. C'est du defense-in-depth non critique mais conforme aux bonnes pratiques.

Threat Model — Vecteurs d'attaque évalués¶

Revue effectuée par agent-adversarial le 2026-02-06. Prochaine revue recommandée après correction des VUL-01 et VUL-02.