From b286352d07804957f9eaf024a676cac6327a36ab Mon Sep 17 00:00:00 2001 From: sysops Date: Fri, 3 Jul 2026 22:23:27 +0200 Subject: [PATCH] =?UTF-8?q?fix(PROJ-64):=20Session-Invalidation=20bei=20Pa?= =?UTF-8?q?sswort-Change=20+=20Datei-Permissions=20geh=C3=A4rtet?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security-Audit deckte zwei Medium-Findings auf: JWTs blieben bis zu 8h nach Passwort-Change/-Reset oder Admin-TOTP-Reset gültig (kein Session-Invalidation), und archivierte Mails/Anhänge wurden mit 0644/0755 statt 0600/0700 geschrieben. - users.tokens_valid_after (neue Spalte) wird bei SetPassword() und InvalidateTokensBefore() gesetzt; ValidateToken() lehnt JWTs mit iat davor ab. - Admin-TOTP-Reset revoked jetzt aktive Sessions des Zielnutzers. - Mail-/Attachment-Dateien und ihre Verzeichnisse nur noch für den archivmail-Service-Account lesbar. Co-Authored-By: Claude Sonnet 5 --- features/INDEX.md | 7 ++-- ...4-session-invalidation-file-permissions.md | 40 +++++++++++++++++++ internal/api/totp_handlers.go | 6 +++ internal/auth/auth.go | 15 +++++++ internal/storage/attachments.go | 4 +- internal/storage/storage.go | 6 +-- internal/userstore/userstore.go | 27 ++++++++++++- 7 files changed, 96 insertions(+), 9 deletions(-) create mode 100644 features/PROJ-64-session-invalidation-file-permissions.md diff --git a/features/INDEX.md b/features/INDEX.md index 050204f..642a4dc 100644 --- a/features/INDEX.md +++ b/features/INDEX.md @@ -63,10 +63,10 @@ | PROJ-44 | OCR-GUI-Integration (Status, Download, Such-Highlight) | Deployed | [PROJ-44](PROJ-44-ocr-gui-integration.md) | 2026-05-08 | | PROJ-45 | IMAP Per-Folder UID-Tracking + UIDVALIDITY-Check | Deployed | [PROJ-45](PROJ-45-imap-folder-uid-tracking.md) | 2026-05-11 | | PROJ-46 | E-Mail als primärer Login-Identifier für Tenant-User | Planned | [PROJ-46](PROJ-46-email-login-tenant-user.md) | 2026-06-13 | -| PROJ-47 | Tenant-Voll-Export per CLI | In Review | [PROJ-47](PROJ-47-tenant-voll-export-cli.md) | 2026-06-13 | +| PROJ-47 | Tenant-Voll-Export per CLI | Deployed | [PROJ-47](PROJ-47-tenant-voll-export-cli.md) | 2026-06-13 | | PROJ-48 | Audit-Log Unveränderbarkeit (Nachbesserung PROJ-11) | Deployed | [PROJ-48](PROJ-48-audit-log-unveraenderbarkeit.md) | 2026-06-13 | | PROJ-49 | Verschlüsselungspflicht at-rest (Healthcheck & Warnung) | Deployed | [PROJ-49](PROJ-49-verschluesselungspflicht.md) | 2026-06-13 | -| PROJ-50 | DSGVO-Löschersuchen für Mail-Inhalte (GoBD-Vorrang) | In Review | [PROJ-50](PROJ-50-dsgvo-loeschersuchen.md) | 2026-06-13 | +| PROJ-50 | DSGVO-Löschersuchen für Mail-Inhalte (GoBD-Vorrang) | Deployed | [PROJ-50](PROJ-50-dsgvo-loeschersuchen.md) | 2026-06-13 | | PROJ-51 | Aufbewahrungsfristen nach Dokumentenart (Retention-Kategorien) | Deployed | [PROJ-51](PROJ-51-retention-kategorien.md) | 2026-06-13 | | PROJ-52 | Vollständigkeits-Reconciliation (Zähl-Report) | Planned | [PROJ-52](PROJ-52-vollstaendigkeits-reconciliation.md) | 2026-06-13 | | PROJ-53 | Konfigurierbare Listenanzahl pro Seite | Deployed | [PROJ-53](PROJ-53-konfigurierbare-listenanzahl.md) | 2026-06-14 | @@ -78,7 +78,8 @@ | PROJ-61 | Fix Cross-Tenant Stored XSS via Mandanten-Logo (Sicherheitsbug) | Deployed | [PROJ-61](PROJ-61-fix-tenant-logo-xss-idor.md) | 2026-06-25 | | PROJ-62 | Fix Cross-Tenant IDOR bei POP3-Konto-Löschung/-Import (Sicherheitsbug) | Deployed | [PROJ-62](PROJ-62-fix-pop3-tenant-idor.md) | 2026-06-25 | | PROJ-63 | Defensive Tenant-Scope-Härtung der Tenant-Verwaltungs-Endpunkte (FUND-2) | Deployed | [PROJ-63](PROJ-63-harden-tenant-admin-scope.md) | 2026-06-25 | +| PROJ-64 | Session-Invalidation bei Passwort-Change + Datei-Permissions-Härtung (Security-Audit) | Deployed | [PROJ-64](PROJ-64-session-invalidation-file-permissions.md) | 2026-07-03 | -## Next Available ID: PROJ-64 +## Next Available ID: PROJ-65 diff --git a/features/PROJ-64-session-invalidation-file-permissions.md b/features/PROJ-64-session-invalidation-file-permissions.md new file mode 100644 index 0000000..26678c1 --- /dev/null +++ b/features/PROJ-64-session-invalidation-file-permissions.md @@ -0,0 +1,40 @@ +# PROJ-64: Session-Invalidation bei Passwort-Change + Datei-Permissions-Härtung (Security-Audit-Nachbesserung) + +## Status: Deployed +**Created:** 2026-07-03 +**Last Updated:** 2026-07-03 + +## Hintergrund +Vollständiges Codebase-Security-Audit (2026-07-03, 4 parallele Fokus-Reviews: Auth/Authz, Injection, Tenant-Isolation, Crypto/Storage) ergab zwei Medium-/Medium-High-Findings. Injection- und Tenant-Isolation-Review lieferten keine ausnutzbaren Befunde (bestehende Fixes aus PROJ-55/61/62/63 halten). + +**Finding 1 — Kein Session-Invalidation bei Passwort-Change/Reset (Medium-High):** +`SetPassword()` (Passwort-Change, Passwort-Reset) und Admin-TOTP-Reset aktualisierten nur den bcrypt-Hash bzw. TOTP-Status, invalidierten aber keine bereits ausgestellten JWTs. Die Token-Blacklist arbeitet nur per-JTI (Logout), es gab keinen "invalidate all sessions"-Mechanismus. Ein gestohlenes JWT/Cookie blieb bis zu 8h nach Passwort-Reset gültig — die Standard-Gegenmaßnahme "Passwort ändern, um Angreifer auszusperren" griff nicht. + +**Finding 2 — Mail-Dateien world-readable (Medium):** +`internal/storage/storage.go` und `attachments.go` schrieben archivierte Mails/Anhänge mit Mode `0644` (Verzeichnisse `0755`) — lesbar für jeden lokalen User/Group, nicht nur den archivmail-Service-Account. Falls Encryption nicht konfiguriert ist (dokumentierter Fallback), lag Mail-Inhalt im Klartext für jeden lokalen Account offen. + +## Acceptance Criteria +- [x] Neue Spalte `users.tokens_valid_after` (TIMESTAMPTZ, nullable), idempotent per `ALTER TABLE ... ADD COLUMN IF NOT EXISTS` in `initSchema`. +- [x] `SetPassword()` setzt `tokens_valid_after = NOW()` bei jedem Passwort-Change/-Reset. +- [x] Neue Methode `InvalidateTokensBefore(ctx, userID)` — von Admin-TOTP-Reset aufgerufen. +- [x] `ValidateToken()` vergleicht JWT-Claim `iat` gegen `tokens_valid_after`; liegt `iat` davor, wird der Token als revoked abgelehnt (`auth: token revoked (credentials changed)`). +- [x] Bestehendes Verhalten unverändert für User ohne gesetztes `tokens_valid_after` (NULL-Check, No-op). +- [x] Mail-Dateien: `os.WriteFile` von `0o644` auf `0o600`. +- [x] Attachment-Dateien: gleiche Änderung in `attachments.go`. +- [x] Storage-Verzeichnisse (`store/`, `attachments/`, `meta/`, Shard-Verzeichnisse): `os.MkdirAll` von `0o755` auf `0o700`. +- [x] Build- und Schema-Verifikation auf Testserver 192.168.1.132 (kein lokaler Go-Toolchain verfügbar). + +## Implementierungsnotizen (2026-07-03) +- `internal/userstore/userstore.go`: Spalte + `SetPassword()` erweitert + `InvalidateTokensBefore()` + `TokensValidAfter()` ergänzt. +- `internal/auth/auth.go` (`ValidateToken`): `iat`-Vergleich gegen `tokens_valid_after` ergänzt, zusätzlicher DB-Roundtrip pro Request-Validierung (gleiche Größenordnung wie bestehender Blacklist-Check). +- `internal/api/totp_handlers.go` (`handleTOTPReset`): Aufruf von `InvalidateTokensBefore()` nach erfolgreichem TOTP-Reset ergänzt (Fehler nur geloggt, kein Abbruch — Reset selbst war bereits erfolgreich). +- `internal/api/onboarding_handlers.go` (Passwort-Reset) und `internal/api/profile_handlers.go` (Passwort-Change) benötigten keine Änderung — beide rufen `SetPassword()` auf, das die Invalidation jetzt intern miterledigt. +- `internal/storage/storage.go`: Zeilen 72 (Init-MkdirAll), 413 (Shard-MkdirAll), 446 (WriteFile) gehärtet. +- `internal/storage/attachments.go`: Zeilen 45 (MkdirAll), 49 (WriteFile) gehärtet. +- Build (`CGO_ENABLED=0 go build -buildvcs=false`) und Schema-Migration (`tokens_valid_after`-Spalte, klaglos angelegt) auf 192.168.1.132 verifiziert, Dienste liefen danach unverändert weiter. Kein Produktiv-Deploy im Rahmen dieser Verifikation. + +## Tech Design +Übersprungen (additive Härtung, kein neues Subsystem — Muster analog zu bestehender Blacklist-/TOTP-Reset-Logik). + +## Priorität +Medium — kein akuter Exploit im laufenden Betrieb, aber schließt eine reale Lücke in der Incident-Response ("Passwort ändern" wirkte bisher nicht sofort) und eine konkrete Klartext-Offenlegung bei fehlender Encryption-Konfiguration. diff --git a/internal/api/totp_handlers.go b/internal/api/totp_handlers.go index 38d0347..96ff2b1 100644 --- a/internal/api/totp_handlers.go +++ b/internal/api/totp_handlers.go @@ -254,6 +254,12 @@ func (s *Server) handleTOTPReset(w http.ResponseWriter, r *http.Request) { return } + // PROJ-64: invalidate any already-issued JWTs for the target user — an admin + // resetting TOTP is an account-takeover response and must revoke live sessions. + if err := s.users.InvalidateTokensBefore(r.Context(), id); err != nil { + s.logger.Error("totp reset: failed to invalidate tokens", "err", err, "target_user", id) + } + s.audlog.Log(audit.Entry{ EventType: audit.EventUserMgmt, Username: sess.Username, diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 0e1e0c5..d05d43a 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -375,6 +375,21 @@ func (m *Manager) ValidateToken(tokenStr string) (*Session, error) { } } + // PROJ-64: reject tokens issued before a password change / admin TOTP reset, + // closing the session-hijack window that a stateless-only JWT leaves open. + var iat time.Time + switch v := claims["iat"].(type) { + case float64: + iat = time.Unix(int64(v), 0) + case int64: + iat = time.Unix(v, 0) + } + if validAfter, err := m.store.TokensValidAfter(context.Background(), userID); err == nil && validAfter != nil { + if iat.Before(*validAfter) { + return nil, errors.New("auth: token revoked (credentials changed)") + } + } + return &Session{ UserID: userID, Username: username, diff --git a/internal/storage/attachments.go b/internal/storage/attachments.go index 07e531d..9b201cf 100644 --- a/internal/storage/attachments.go +++ b/internal/storage/attachments.go @@ -42,11 +42,11 @@ func (s *Store) saveAttachments(ctx context.Context, emailID string, pm *mailpar } attPath := s.attachmentPath(hash) - if err := os.MkdirAll(filepath.Dir(attPath), 0o755); err != nil { + if err := os.MkdirAll(filepath.Dir(attPath), 0o700); err != nil { return fmt.Errorf("storage: attachment mkdir: %w", err) } if _, statErr := os.Stat(attPath); os.IsNotExist(statErr) { - if err := os.WriteFile(attPath, toWrite, 0o644); err != nil { + if err := os.WriteFile(attPath, toWrite, 0o600); err != nil { return fmt.Errorf("storage: attachment write: %w", err) } } diff --git a/internal/storage/storage.go b/internal/storage/storage.go index b7388d5..273508f 100644 --- a/internal/storage/storage.go +++ b/internal/storage/storage.go @@ -69,7 +69,7 @@ type MailWithUID struct { // and connects to PostgreSQL. func New(cfg Config) (*Store, error) { for _, sub := range []string{"store", "attachments", "meta"} { - if err := os.MkdirAll(filepath.Join(cfg.Dir, sub), 0o755); err != nil { + if err := os.MkdirAll(filepath.Join(cfg.Dir, sub), 0o700); err != nil { return nil, fmt.Errorf("storage: mkdir %s: %w", sub, err) } } @@ -410,7 +410,7 @@ func (s *Store) Save(ctx context.Context, raw []byte, _ time.Time, tenantID *int id := fmt.Sprintf("%x", sum[:]) // 64 hex chars path := s.filePath(id) - if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { return "", fmt.Errorf("storage: mkdir shard: %w", err) } @@ -443,7 +443,7 @@ func (s *Store) Save(ctx context.Context, raw []byte, _ time.Time, tenantID *int toWrite = toStore } - if err := os.WriteFile(path, toWrite, 0o644); err != nil { + if err := os.WriteFile(path, toWrite, 0o600); err != nil { return "", fmt.Errorf("storage: write: %w", err) } diff --git a/internal/userstore/userstore.go b/internal/userstore/userstore.go index a6c6cb4..a814cdf 100644 --- a/internal/userstore/userstore.go +++ b/internal/userstore/userstore.go @@ -129,6 +129,14 @@ func (s *Store) initSchema(ctx context.Context) error { _, err = s.pool.Exec(ctx, ` ALTER TABLE users ADD COLUMN IF NOT EXISTS list_page_size INT NOT NULL DEFAULT 25; `) + if err != nil { + return err + } + // PROJ-64: tokens_valid_after invalidiert alle vor diesem Zeitpunkt ausgestellten JWTs + // (Passwort-Change/Reset, Admin-TOTP-Reset) — schließt Session-Hijack-Fenster. + _, err = s.pool.Exec(ctx, ` + ALTER TABLE users ADD COLUMN IF NOT EXISTS tokens_valid_after TIMESTAMPTZ; + `) return err } @@ -207,10 +215,27 @@ func (s *Store) SetPassword(ctx context.Context, id int64, newPassword string) e if err != nil { return fmt.Errorf("userstore: bcrypt: %w", err) } - _, err = s.pool.Exec(ctx, `UPDATE users SET password_hash=$1 WHERE id=$2`, string(hash), id) + _, err = s.pool.Exec(ctx, `UPDATE users SET password_hash=$1, tokens_valid_after=NOW() WHERE id=$2`, string(hash), id) return err } +// InvalidateTokensBefore sets tokens_valid_after=NOW() so all JWTs issued before +// this call are rejected on next use (PROJ-64). Used e.g. after admin TOTP reset. +func (s *Store) InvalidateTokensBefore(ctx context.Context, id int64) error { + _, err := s.pool.Exec(ctx, `UPDATE users SET tokens_valid_after=NOW() WHERE id=$1`, id) + return err +} + +// TokensValidAfter returns the tokens_valid_after timestamp for a user, or nil if unset. +func (s *Store) TokensValidAfter(ctx context.Context, id int64) (*time.Time, error) { + var t *time.Time + err := s.pool.QueryRow(ctx, `SELECT tokens_valid_after FROM users WHERE id=$1`, id).Scan(&t) + if err != nil { + return nil, err + } + return t, nil +} + // GetByID retrieves a user by their numeric ID. func (s *Store) GetByID(id int64) (*User, error) { ctx := context.Background()