QA-15 F-06: Pruefprotokoll
CI / Web-Barrierefreiheitspruefung (axe, Tastatur, Reflow, Textabstaende) (push) Canceled after 0s
CI / Web-Barrierefreiheitspruefung (axe, Tastatur, Reflow, Textabstaende) (pull_request) Canceled after 0s
CI / Core-Modul (Build/Vet/Test) (push) Canceled after 0s
CI / DMS Security-Test-Gate (AC5, Merge-Gate) (push) Canceled after 0s
CI / DMS-Modul (Lint/Build/Test/Coverage/Artefakt) (push) Canceled after 0s
CI / DMS-Modul Supply-Chain-Scan (govulncheck) (push) Canceled after 0s
CI / Web (dms-browser) (push) Canceled after 0s
CI / Web (pdf-viewer) (push) Canceled after 0s
CI / Web (shl) (push) Canceled after 0s
CI / Core-Modul (Build/Vet/Test) (pull_request) Canceled after 0s
CI / DMS Security-Test-Gate (AC5, Merge-Gate) (pull_request) Canceled after 0s
CI / DMS-Modul (Lint/Build/Test/Coverage/Artefakt) (pull_request) Canceled after 0s
CI / DMS-Modul Supply-Chain-Scan (govulncheck) (pull_request) Canceled after 0s
CI / Web (dms-browser) (pull_request) Canceled after 0s
CI / Web (pdf-viewer) (pull_request) Canceled after 0s
CI / Web (shl) (pull_request) Canceled after 0s
Core-Schnittstellen-Vertragstests / contract-tests (pull_request) Canceled after 0s
Supply-Chain-Scan (Go) / govulncheck (pull_request) Canceled after 0s
Supply-Chain-Scan (npm) / npm-audit (pull_request) Canceled after 0s
Core-Pflichttest-Gate / pflichttest-gate (pull_request) Canceled after 0s
RBAC-Rollenhierarchie-Pflichtgate / rbac-policy-invariant (pull_request) Canceled after 0s

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
patrickandClaude Sonnet 5 committed 2026-09-22 09:07:44 +02:00
1 parent 85c093e509
commit c0de004e57
1 file changed
+131
@@ -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).