diff --git a/docs/QA-15-F06-TOKENFORMAT-PRUEFPROTOKOLL.md b/docs/QA-15-F06-TOKENFORMAT-PRUEFPROTOKOLL.md new file mode 100644 index 00000000..6b2aee65 --- /dev/null +++ b/docs/QA-15-F06-TOKENFORMAT-PRUEFPROTOKOLL.md @@ -0,0 +1,131 @@ +# QA-15 F-06: Zweitfaktor — Klartext-Geheimnisse, Wiederholungsschutz, Sitzungswiderruf + +Umsetzung des Befunds F-06 aus `docs/SICHERHEITSPRUEFUNG-GESAMTPROJEKT.md` +(PR #110) sowie der zugehörigen User-Entscheidung vom 2026-09-22 ("Ja, +umsetzen" — TOTP-Verschlüsselung UND Sitzungstoken-Rotation). + +Basis: `origin/integration/gesamtprojekt` (Head `d86db43`). +Branch: `feature/qa-15-f06-token-format-haertung`. + +## Umsetzung + +**F-06 (c) — Klartext-Secret in der Datenbank** + +- Neue Spalte `totp_credentials.secret_ciphertext` (Migration + `migrations/tenant/0009_totp_secret_encryption`), `secret` wird nullable. + Eine CHECK-Constraint erzwingt, dass immer genau eine der beiden Spalten + gesetzt ist. +- `internal/totp.Store` bekommt eine `Cipher`-Schnittstelle + (`Encrypt`/`Decrypt`, dieselbe Form wie `moduletrust.SecretCipher`). + `NewEncryptedStore(pool, cipher)` verschlüsselt neue/geänderte Secrets; + `NewStore(pool)` (ohne Cipher) bleibt nur für Altbestand-Lesepfade + erhalten — jeder Schreibversuch ohne Cipher schlägt mit + `ErrSecretUnavailable` fehl (fail closed, kein stiller Klartext-Fallback). +- `cmd/account-api`: neuer, EIGENER Master-Key + `NEXARCH_ACCOUNT_TOTP_ENCRYPTION_KEY` (separat vom + Modul-Signaturschlüssel — unterschiedliche Betriebszyklen). Ohne + gesetzten Schlüssel läuft der Dienst weiter (Altbestand bleibt lesbar), + aber jede neue TOTP-Einrichtung schlägt fehl. +- Nachverschlüsselungs-Werkzeug: `account-api -encrypt-totp-secrets` + verschlüsselt bestehende Klartextzeilen EINE NACH DER ANDEREN (kein + Transaktionsrahmen über alle Zeilen, damit ein Abbruch nicht bereits + erledigte Zeilen zurückrollt). Das Geheimnis selbst ändert sich dabei + nicht — bestehende Authenticator-Apps bleiben gültig. + +**F-06 (b) — Code innerhalb seines 90-Sekunden-Fensters mehrfach nutzbar** + +- Neue Spalte `totp_credentials.last_used_step` (gleiche Migration). +- `totp.ValidateStep` liefert zusätzlich zum Ok-Wert den getroffenen + HOTP-Zeitschritt. `Store.VerifyLoginCode` lehnt einen Code ab, dessen + Schritt `<= last_used_step` ist, und aktualisiert den Wert atomar + (`WHERE last_used_step IS NULL OR last_used_step < $step`) — verhindert + ein Wettrennen zwischen zwei parallelen Anfragen mit demselben Code. +- `ConfirmSetup` (Einrichtung) bleibt unverändert — dort ist ein doppelter + Versuch durch `confirmed=true` ohnehin nur einmal wirksam. + +**Sitzungstoken-Rotation (User-Entscheidung)** + +- Umgesetzt als der konkrete, im Code bereits versprochene, aber nicht + eingelöste Fix: `internal/auth.ChangePasswordHandler` widerruft nach + erfolgreicher Passwortänderung ALLE zuvor ausgestellten Tokens des + Nutzers (`RevocationStore.RevokeAllForUser`) und stellt dem Aufrufer ein + frisches Token aus (`WithSessionRotation`), damit die eigene Anfrage + nicht durch die eigene Aktion ausgesperrt wird. +- Bewusst NICHT umgesetzt: ein umfassenderes Session-Cookie-Modell. Die + bestehende Architektur (kurzlebige, zustandslose JWTs mit gezieltem + jti-Widerruf und Massenwiderruf, IAM-17) ist bereits die "Rotation" im + eigentlichen Sinn — jeder Login stellt ohnehin ein neues Token aus. + Klassische Session-Fixation (Angreifer erzwingt eine bekannte Session-ID + vor dem Login) greift hier nicht, weil kein Token vor der + Authentifizierung existiert. Die tatsächliche Lücke war die + dokumentierte, aber ungenutzte `RevokeAllForUser`-Absicht bei + Passwortänderung — genau die ist jetzt geschlossen. + +## Prüfungen auf Host 204 (Wegwerf-Cluster `17/coredev`) + +| Prüfung | Ergebnis | +|---|---| +| `go build ./...`, `go vet ./...` | sauber | +| `gofmt -l` (geänderte Verzeichnisse) | leer | +| `internal/totp` (inkl. neuer Tests) | `ok`, 16,4s | +| `internal/auth` (inkl. neuer Tests) | `ok`, 13,3s | +| `scripts/migrations-fresh-check.sh` | PASS, Core Mandant jetzt 19 Migrationen, 88 Tabellen gesamt, keine Kollision | +| `scripts/e2e-core-dms.sh` | 10/10 Teilfälle, `TestCoreE2E_TokenJWKSAndPolicy` | +| `scripts/e2e-module-keys.sh` | 8/8 Prüfungen | + +Neue Tests (Auszug): + +- `TestBeginSetup_StoresSecretEncryptedNotInPlaintext` — Rohabfrage der + Spalten nach Einrichtung: `secret` ist `NULL`, `secret_ciphertext` + nicht leer und ungleich dem Klartext. +- `TestBeginSetup_WithoutCipher_FailsClosed` — ohne Cipher entsteht keine + Zeile. +- `TestVerifyLoginCode_WrongCipher_FailsClosed` — Chiffretext eines + fremden Schlüssels ist nicht lesbar. +- `TestVerifyLoginCode_RejectsReplayOfSameCode` — derselbe Code wird beim + zweiten Versuch abgelehnt. +- `TestChangePassword_RevokesOldSessionAndIssuesNewOne` — altes Token + nach Passwortänderung widerrufen, neues Token in der Antwort gültig und + nicht widerrufen. + +## QA und Entscheidungen (bis zu 10 Iterationen, User-Regel 2026-09-22) + +0 Iterationen am Produktivcode nötig (Build/Vet/Tests bestanden nach der +ersten vollständigen Fassung); 2 Iterationen an Tests (fehlende +`secret_ciphertext`/`last_used_step`-Spalten im handgeschriebenen +Test-Schema; `setupTenantDB`-Hilfsfunktion ohne Widerrufstabellen für den +neuen Rotationstest — beide durch Anpassung der Tests behoben, kein +Produktcode betroffen). + +Getroffene Entscheidungen: + +1. **Eigener Master-Key statt Wiederverwendung des + Modul-Signaturschlüssels.** Alternative: `NEXARCH_ACCOUNT_MODULE_KEY_ENCRYPTION_KEY` + mitnutzen. Verworfen, weil Modul-Signaturschlüssel automatisch rotieren + und TOTP-Secrets nicht — eine gemeinsame Variable würde die + Betriebszyklen unnötig koppeln. +2. **`secret` bleibt als Legacy-Spalte bestehen, keine Zwangs-Migration + beim Programmstart.** Alternative: harter Cutover mit Pflicht-Skript + vor dem ersten Start. Verworfen — würde den Dienst ohne Betriebseingriff + startunfähig machen; stattdessen lesbar bleibend plus explizites + Werkzeug (`-encrypt-totp-secrets`). +3. **Sitzungsrotation nur bei Passwortänderung, nicht bei jedem TOTP-Login.** + Ein 30-Minuten-Token ist bereits kurzlebig; eine Rotation bei jedem + Login böte kaum zusätzlichen Schutz, würde aber Clients zwingen, das + Token laufend zu ersetzen. Passwortänderung ist der Fall mit echtem + Bedrohungsmodell (gestohlenes, noch gültiges Token soll nach + Kompromittierungs-Reaktion nicht weiter gelten). + +## Offene Punkte / Grenzen + +- **Uhren-Reihenfolge bei der Rotation:** `RevokeAllForUser` setzt + `revoked_before = now()` (Postgres-Zeit), das neue Token trägt die + Go-Prozesszeit. Bei erheblichem Taktversatz zwischen App- und DB-Server + könnte ein Token theoretisch knapp vor dem Stichtag ausgestellt und + sofort als widerrufen gelten. In der Praxis liegt zwischen beiden + Aufrufen mindestens ein DB-Roundtrip; nicht separat gehärtet. +- **`-encrypt-totp-secrets` ist nicht auf 204/131 gegen echten Altbestand + gelaufen** (es gibt keinen), nur gegen synthetische Testzeilen über die + Store-Tests. +- **Kein Alarm bei einem Massenwiderruf** (F-06 sieht das nicht vor, + bleibt wie zuvor).