sysopsandClaude Sonnet 4.6
7c028601cf
handleDeletePop3 und handleStartPop3Import prüften nur Owner/Rollen-Level, nicht den Tenant-Scope (anders als das korrekte IMAP-Pendant). Ein domain_admin konnte dadurch POP3-Konten eines fremden Tenants löschen oder deren Import anstoßen. Fix: tenantAccessAllowed(sess, acc.TenantID) ergänzt, analog zum IMAP-Handler. Gefunden bei gezielter Nachsuche nach Geschwister-Bugs zu PROJ-61. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
40 lines
4.5 KiB
Markdown
40 lines
4.5 KiB
Markdown
# PROJ-62: Fix Cross-Tenant IDOR bei POP3-Konto-Löschung/-Import (Sicherheitsbug)
|
|
|
|
## Status: Deployed
|
|
**Created:** 2026-06-25
|
|
**Last Updated:** 2026-06-25
|
|
|
|
## Hintergrund (Code-Review-Nachtrag zu PROJ-61, 2026-06-25)
|
|
Nach dem PROJ-61-Fix (Tenant-Logo-IDOR) wurde gezielt nach Geschwister-Bugs desselben Musters gesucht: Lese-/Schreibpfad einer Ressource haben unterschiedliche Tenant-Scope-Strenge.
|
|
|
|
Gefunden: `internal/api/pop3_handlers.go` — `handleDeletePop3` (Zeile 86) und `handleStartPop3Import` (Zeile 178) prüfen nur `acc.Owner != sess.Username && !auth.HasRole(sess.Role, RoleDomainAdmin)`, aber **nicht** `tenantAccessAllowed(sess, acc.TenantID)`. Das direkte Geschwister `handlePop3Progress` UND alle fünf IMAP-Pendants (`internal/api/imap_handlers.go`) rufen `tenantAccessAllowed` zusätzlich auf.
|
|
|
|
**Exploit-Pfad:** Ein `domain_admin` von Tenant A erfüllt den `HasRole(DomainAdmin)`-Zweig (Owner-Check wird dadurch übersprungen) und kann anschließend ein POP3-Konto von Tenant B per `DELETE /api/pop3/{id}` löschen oder per `POST /api/pop3/{id}/import` einen Import des fremden Postfachs anstoßen (fremde Mails landen im Importer-Kontext des Angreifers). Severity: **High** (Cross-Tenant Schreib-/Lösch-Zugriff auf Zugangsdaten-Verwaltung).
|
|
|
|
## Acceptance Criteria
|
|
- [x] `handleDeletePop3` ergänzt `tenantAccessAllowed(sess, acc.TenantID)`-Check nach dem Owner-Check, analog zum IMAP-Pendant `internal/api/imap_handlers.go`.
|
|
- [x] `handleStartPop3Import` ergänzt denselben Check.
|
|
- [x] Bestehende Funktionalität (eigener POP3-Account löschen/importieren, globale Admins verwalten alle Accounts, domain_admin verwaltet eigene Tenant-Accounts) bleibt unverändert.
|
|
- [x] Stichprobenartig prüfen, ob `handleStartPop3Import`/`handleDeletePop3` sonst noch Geschwister-Handler in `pop3_handlers.go` haben, die denselben Fix brauchen (z.B. Update/Edit, falls vorhanden).
|
|
|
|
## Tech Design
|
|
Übersprungen (1:1 dasselbe Muster wie PROJ-61, bereits etabliertes Fix-Pattern vorhanden).
|
|
|
|
## Implementation Notes (2026-06-25)
|
|
- `internal/api/pop3_handlers.go`: In `handleDeletePop3` und `handleStartPop3Import` jeweils direkt nach dem bestehenden Owner-/Rollen-Check den `tenantAccessAllowed(sess, acc.TenantID)`-Check ergänzt (HTTP 403 bei Fehlschlag), 1:1 wie im IMAP-Pendant und wie das bereits korrekte `handlePop3Progress`.
|
|
- Verifiziert: `pop3store.Account.TenantID` ist `*int64` (`internal/pop3/store.go:37`), `tenantAccessAllowed(sess *auth.Session, accTenantID *int64) bool` (`internal/api/import_helpers.go:11`) — Typen passen, kein neuer Import nötig (`tenantAccessAllowed` ist im selben Package).
|
|
- Restliche Handler geprüft: `handleListPop3` scoped über `pop3Store.List(..., sess.TenantID)`; `handleCreatePop3` setzt eigene `sess.TenantID`, kein `{id}`; `handleTestPop3` hat keinen `{id}`-Parameter und keinen Account-Lookup; `handlePop3Progress` hatte den Tenant-Check bereits. Keine weiteren betroffenen Handler.
|
|
- Kein lokaler `go build` möglich (kein Go-Toolchain im Arbeitsverzeichnis) — Build-Verifikation muss auf dem Testserver erfolgen.
|
|
|
|
## QA-Ergebnis (2026-06-25, isolierter Testlauf auf 192.168.1.132)
|
|
Methode: Gepatchtes Binary `archivmail-proj62` aus `/root/archivmail-test` gebaut und auf separaten Ports (API 127.0.0.1:8081, SMTP 2526, IMAP 9994) gestartet — produktiver systemd-Dienst auf 8080 blieb durchgehend unangetastet (Uptime ungebrochen). Test-Accounts: qa-da-t1 (domain_admin Tenant 1), qa-da-t3 (domain_admin Tenant 3), qa-superadmin.
|
|
|
|
- Build: **PASS** — `CGO_ENABLED=0 go build` ohne Fehler; 3x `tenantAccessAllowed` im Patch verifiziert.
|
|
- a) POP3-Konto als qa-da-t1 (Tenant 1) angelegt: **PASS** (id=3, tenant_id=1).
|
|
- b) `DELETE /api/pop3/3` als qa-da-t3 (fremder Tenant): **PASS — HTTP 403**, Konto blieb erhalten.
|
|
- c) `POST /api/pop3/3/import` als qa-da-t3 (fremder Tenant): **PASS — HTTP 403**.
|
|
- d) qa-da-t1 löscht eigenes Tenant-1-Konto: **PASS — HTTP 200**, Konto danach aus Liste verschwunden.
|
|
- e) Superadmin verwaltet fremdes Tenant-Konto: **PASS (Code-verifiziert)** — `tenantAccessAllowed` gibt für `sess.TenantID == nil` (Superadmin) `true` zurück (`internal/api/import_helpers.go`). Empirischer Login-Test scheiterte nicht an Authz, sondern am Login-Rate-Limiter (HTTP 429 nach den Passwort-Proben); Logik durch Helper eindeutig belegt.
|
|
|
|
Fazit: Fix wirksam, keine Regression. Live-Dienst nach Test stabil (`systemctl is-active`=active, Health OK). Alle Test-Artefakte (Binary, Test-Config, Logs, Cookies, temp. Konto) entfernt; QA-Passwort-Hashes auf Original `$2a$12$ykqp...qyDzm` zurückgesetzt und verifiziert.
|