From 27f51b6be812901491c287b6498cb8697e134320 Mon Sep 17 00:00:00 2001 From: Florian Wilde Date: Mon, 27 Jul 2026 00:14:10 +0200 Subject: [PATCH] fix(auth): Einmalkennwoerter nicht zweimal annehmen Ein TOTP-Code blieb rund 90 Sekunden gueltig und liess sich darin beliebig oft verwenden. RFC 6238 verlangt in Abschnitt 5.2 ausdruecklich das Gegenteil: derselbe Code darf kein zweites Mal angenommen werden. Ohne diese Sperre kann ein abgefangener Code - ueber die Schulter gesehen, von einer weiterleitenden Phishing-Seite, von Schadsoftware vom Bildschirm gelesen - die restliche Gueltigkeitsdauer ueber wiederverwendet werden. verifyTotp gibt dafuer jetzt ueber matchTotp das Zeitfenster zurueck, das gepasst hat; angenommen wird nur, was neuer ist als das zuletzt eingeloeste. Der Vergleich bleibt dabei in konstanter Zeit - es wird weiterhin nicht fruehzeitig abgebrochen. Auch die Aktivierung verbrennt ihr Fenster: der Code, der eben scharfgeschaltet hat, soll nicht Sekunden spaeter noch zum Anmelden taugen. Co-Authored-By: Claude Opus 5 (1M context) --- .../20260727000000_totp_replay/migration.sql | 2 + prisma/schema.prisma | 4 ++ src/app/api/account/totp/route.ts | 11 +++-- src/app/api/auth/mfa/route.ts | 27 ++++++++++-- src/lib/auth/mfa.ts | 3 ++ src/lib/auth/totp.test.ts | 31 +++++++++++++ src/lib/auth/totp.ts | 44 +++++++++++++------ 7 files changed, 101 insertions(+), 21 deletions(-) create mode 100644 prisma/migrations/20260727000000_totp_replay/migration.sql diff --git a/prisma/migrations/20260727000000_totp_replay/migration.sql b/prisma/migrations/20260727000000_totp_replay/migration.sql new file mode 100644 index 0000000..b29daa9 --- /dev/null +++ b/prisma/migrations/20260727000000_totp_replay/migration.sql @@ -0,0 +1,2 @@ +-- AlterTable +ALTER TABLE "User" ADD COLUMN "totpLastCounter" INTEGER; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index b95114c..909b5fd 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -128,6 +128,10 @@ model User { /// sperrt sich aus, wer die Einrichtung abbricht. totpSecretCipher String? totpEnabledAt DateTime? + /// Zuletzt eingeloestes Zeitfenster. RFC 6238 verlangt, denselben Code + /// kein zweites Mal anzunehmen – ohne diesen Merker liesse sich ein + /// abgefangener Code die restliche Gueltigkeit ueber wiederverwenden. + totpLastCounter Int? sessions Session[] credentials Credential[] diff --git a/src/app/api/account/totp/route.ts b/src/app/api/account/totp/route.ts index 45f4fe9..5ae3c67 100644 --- a/src/app/api/account/totp/route.ts +++ b/src/app/api/account/totp/route.ts @@ -4,7 +4,7 @@ import { z } from "zod"; import { requireUserApi } from "@/lib/auth/guard"; import { verifyPassword } from "@/lib/auth/password"; import { countUnusedRecoveryCodes, issueRecoveryCodes } from "@/lib/auth/recovery"; -import { generateTotpSecret, groupSecret, otpauthUri, verifyTotp } from "@/lib/auth/totp"; +import { generateTotpSecret, groupSecret, matchTotp, otpauthUri } from "@/lib/auth/totp"; import { APP_NAME } from "@/lib/config"; import { encryptSecret, tryDecryptSecret } from "@/lib/crypto"; import { prisma } from "@/lib/db"; @@ -109,7 +109,8 @@ export async function PUT(request: Request) { return jsonError("Es liegt kein Schluessel vor. Bitte die Einrichtung neu starten.", 409); } - if (!verifyTotp(secret, parsed.data.code)) { + const fenster = matchTotp(secret, parsed.data.code); + if (fenster === null) { return jsonError( "Der Code stimmt nicht. Geht die Uhr des Geraets richtig?", 401, @@ -118,7 +119,9 @@ export async function PUT(request: Request) { await prisma.user.update({ where: { id: auth.user.id }, - data: { totpEnabledAt: new Date() }, + // Das Fenster gleich mitverbrennen: der Code, der eben aktiviert hat, + // soll nicht Sekunden spaeter noch zum Anmelden taugen. + data: { totpEnabledAt: new Date(), totpLastCounter: fenster }, }); return NextResponse.json({ @@ -157,7 +160,7 @@ export async function DELETE(request: Request) { await prisma.$transaction([ prisma.user.update({ where: { id: auth.user.id }, - data: { totpSecretCipher: null, totpEnabledAt: null }, + data: { totpSecretCipher: null, totpEnabledAt: null, totpLastCounter: null }, }), prisma.recoveryCode.deleteMany({ where: { userId: auth.user.id } }), prisma.pendingLogin.deleteMany({ where: { userId: auth.user.id } }), diff --git a/src/app/api/auth/mfa/route.ts b/src/app/api/auth/mfa/route.ts index 4f48c01..503e9c3 100644 --- a/src/app/api/auth/mfa/route.ts +++ b/src/app/api/auth/mfa/route.ts @@ -8,8 +8,9 @@ import { } from "@/lib/auth/mfa"; import { redeemRecoveryCode } from "@/lib/auth/recovery"; import { createSession, setSessionCookie } from "@/lib/auth/session"; -import { verifyTotp } from "@/lib/auth/totp"; +import { matchTotp } from "@/lib/auth/totp"; import { noteSuccessfulLogin } from "@/lib/auth/users"; +import { prisma } from "@/lib/db"; import { LIMITS, rateLimit } from "@/lib/security/rateLimit"; import { clientIp, jsonError, readJsonBody, tooManyRequests } from "@/lib/security/request"; @@ -63,9 +64,19 @@ export async function POST(request: Request) { // Wiederherstellungscode. Beide Wege zu probieren waere unnoetig – die // Formate ueberschneiden sich nicht. const istZiffern = /^[\d\s]+$/.test(eingabe); - const erfolg = istZiffern - ? Boolean(pending.user.totpSecret) && verifyTotp(pending.user.totpSecret!, eingabe) - : await redeemRecoveryCode(pending.user.id, eingabe); + + // Bei den Ziffern zaehlt zusaetzlich, dass das Zeitfenster neuer ist als + // das zuletzt eingeloeste. Sonst liesse sich ein abgefangener Code die + // restliche Gueltigkeitsdauer ueber wiederverwenden – RFC 6238, 5.2. + const fenster = + istZiffern && pending.user.totpSecret + ? matchTotp(pending.user.totpSecret, eingabe) + : null; + const frisch = + fenster !== null && + (pending.user.totpLastCounter === null || fenster > pending.user.totpLastCounter); + + const erfolg = istZiffern ? frisch : await redeemRecoveryCode(pending.user.id, eingabe); if (!erfolg) { const weiterMoeglich = await noteMfaFailure(pending.id); @@ -83,6 +94,14 @@ export async function POST(request: Request) { await discardPendingLogin(pending.id); await noteSuccessfulLogin(pending.user.id); + // Das eingeloeste Fenster merken, damit derselbe Code verbrannt ist. + if (fenster !== null) { + await prisma.user.update({ + where: { id: pending.user.id }, + data: { totpLastCounter: fenster }, + }); + } + const session = await createSession(pending.user.id, { userAgent: request.headers.get("user-agent"), ip, diff --git a/src/lib/auth/mfa.ts b/src/lib/auth/mfa.ts index 834ea64..16b713e 100644 --- a/src/lib/auth/mfa.ts +++ b/src/lib/auth/mfa.ts @@ -75,6 +75,8 @@ export interface PendingLoginState { isActive: boolean; /** Entschluesselter TOTP-Schluessel; null, wenn er nicht lesbar ist. */ totpSecret: string | null; + /** Zuletzt eingeloestes Zeitfenster – gegen die Wiederverwendung. */ + totpLastCounter: number | null; }; } @@ -104,6 +106,7 @@ export async function readPendingLogin(): Promise { role: pending.user.role, isActive: pending.user.isActive, totpSecret: tryDecryptSecret(pending.user.totpSecretCipher), + totpLastCounter: pending.user.totpLastCounter, }, }; } diff --git a/src/lib/auth/totp.test.ts b/src/lib/auth/totp.test.ts index 538fea4..584f044 100644 --- a/src/lib/auth/totp.test.ts +++ b/src/lib/auth/totp.test.ts @@ -6,6 +6,7 @@ import { counterFor, generateTotpSecret, groupSecret, + matchTotp, otpauthUri, totpCode, verifyTotp, @@ -124,3 +125,33 @@ describe("Einrichtung", () => { expect(groupSecret("ABCDEFGH")).toBe("ABCD EFGH"); }); }); + +describe("matchTotp", () => { + const SECRET = base32Encode(Buffer.from("12345678901234567890", "utf8")); + const JETZT = new Date(1111111109_000); + + /** + * Der Zaehler ist noetig, um einen verbrauchten Code zu entwerten. RFC 6238 + * verlangt das in Abschnitt 5.2 ausdruecklich – sonst laesst sich ein + * abgefangener Code die restliche Gueltigkeitsdauer ueber wiederverwenden. + */ + it("gibt das Zeitfenster zurueck, das gepasst hat", () => { + expect(matchTotp(SECRET, "081804", JETZT)).toBe(37037036); + }); + + it("nennt bei den Nachbarfenstern deren eigene Nummer", () => { + // Genau daran haengt die Entwertung: der Code aus dem vorigen Fenster + // darf den Merker nicht auf einen hoeheren Stand setzen. + const naechstes = new Date(JETZT.getTime() + TOTP_PERIOD * 1000); + expect(matchTotp(SECRET, "081804", naechstes)).toBe(37037036); + }); + + it("liefert null bei falschem Code", () => { + expect(matchTotp(SECRET, "000000", JETZT)).toBeNull(); + }); + + it("stimmt mit verifyTotp ueberein", () => { + expect(matchTotp(SECRET, "081804", JETZT) !== null).toBe(verifyTotp(SECRET, "081804", JETZT)); + expect(matchTotp(SECRET, "000000", JETZT) !== null).toBe(verifyTotp(SECRET, "000000", JETZT)); + }); +}); diff --git a/src/lib/auth/totp.ts b/src/lib/auth/totp.ts index ff570a6..90a8212 100644 --- a/src/lib/auth/totp.ts +++ b/src/lib/auth/totp.ts @@ -102,26 +102,44 @@ export function counterFor(at: Date = new Date()): number { } /** - * Prueft einen eingegebenen Code. Vergleicht in konstanter Zeit und probiert - * die Nachbarfenster mit, damit eine leicht abweichende Uhr nicht stoert. + * Prueft einen eingegebenen Code und gibt das Zeitfenster zurueck, das + * gepasst hat – oder null. + * + * Das Fenster wird gebraucht, um einen verbrauchten Code zu entwerten: + * RFC 6238 verlangt in Abschnitt 5.2 ausdruecklich, dass derselbe Code kein + * zweites Mal angenommen wird. Ohne diese Zahl liesse sich ein abgefangener + * Code die restliche Gueltigkeitsdauer ueber wiederverwenden. + * + * Verglichen wird in konstanter Zeit, und die Nachbarfenster werden + * mitprobiert, damit eine leicht abweichende Uhr nicht stoert. */ +export function matchTotp( + secret: string, + code: string, + at: Date = new Date(), + window: number = TOTP_WINDOW, +): number | null { + const eingabe = code.replace(/\s/g, ""); + if (!/^\d+$/.test(eingabe) || eingabe.length !== TOTP_DIGITS) return null; + + const jetzt = counterFor(at); + let treffer: number | null = null; + for (let schritt = -window; schritt <= window; schritt += 1) { + // Nicht fruehzeitig abbrechen: sonst haengt die Laufzeit davon ab, + // welches Fenster gepasst hat. + if (constantEquals(totpCode(secret, jetzt + schritt), eingabe)) treffer = jetzt + schritt; + } + return treffer; +} + +/** Wie matchTotp, wenn nur interessiert *ob* der Code stimmt. */ export function verifyTotp( secret: string, code: string, at: Date = new Date(), window: number = TOTP_WINDOW, ): boolean { - const eingabe = code.replace(/\s/g, ""); - if (!/^\d+$/.test(eingabe) || eingabe.length !== TOTP_DIGITS) return false; - - const jetzt = counterFor(at); - let treffer = false; - for (let schritt = -window; schritt <= window; schritt += 1) { - // Nicht fruehzeitig abbrechen: sonst haengt die Laufzeit davon ab, - // welches Fenster gepasst hat. - if (constantEquals(totpCode(secret, jetzt + schritt), eingabe)) treffer = true; - } - return treffer; + return matchTotp(secret, code, at, window) !== null; } function constantEquals(a: string, b: string): boolean {