F-01: err.Error() wird nicht mehr an HTTP-Clients gesendet.
Stattdessen generische Fehlermeldungen + Server-Log.
Betrifft: handleCreateUser, handleUpdateUser, handleDeleteUser,
handleSyncNow, handleSecurityConfig, handleUpload.
F-02: Login-Audit-Log enthält keinen rohen err.Error() mehr.
Neue classifyLoginError() Funktion: invalid_password / ldap_error /
account_disabled / unknown — schützt vor LDAP-Info-Leak via Audit-API.
W-03: remoteIP() trimmt jetzt Leerzeichen aus X-Forwarded-For.
Vollständige Lösung erfordert Proxy-Konfiguration (W-03 bleibt WARN).
W-04: Attachment-Dateiname wird durch sanitizeFilename() bereinigt.
Nur [a-zA-Z0-9._- ] erlaubt — verhindert Header-Injection.
PROJ-24: TOTP 2FA vollständig implementiert:
- internal/auth/totp.go: GenerateSecret, ValidateTOTP, QRCodeSVG
- internal/api/totp_handlers.go: Setup, Login-Step2, Admin-Reset
- internal/userstore: SetTOTPSecret, EnableTOTP, DisableTOTP, ResetTOTP
- Login-Flow: totp_pending JWT → /api/auth/totp → vollwertiger JWT
- AES-256-GCM verschlüsseltes Secret in users.totp_secret
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
12 KiB
Security-Audit Bericht -- archivmail (2026-03-18)
Auditor: Security Engineer (Code-Review) Scope: Juengste Commits -- HKDF, Rollenhierarchie, Tenant-LDAP, Mail-ID-Validation, SMTP, Export
Zusammenfassung
PASS: 27 | FAIL: 2 | WARN: 5
Ergebnisse
| # | Bereich | Pruefpunkt | Ergebnis | Details |
|---|---|---|---|---|
| 1 | SEC-08 HKDF | Zwei verschiedene Salts fuer JWT und AES | PASS | archivmail-jwt-v1 und archivmail-aes-v1 korrekt in cmd/archivmail/main.go:82,87 |
| 2 | SEC-08 HKDF | jwtSecret wird fuer auth.New() verwendet | PASS | main.go:165: auth.New(users, ldapSt, jwtSecret) |
| 3 | SEC-08 HKDF | aesKey wird fuer ldapcfg, imap, pop3 verwendet | PASS | main.go:157,243,261: alle drei Stores erhalten aesKey |
| 4 | SEC-08 HKDF | apiCfg.Secret = jwtSecret (nicht cfg.API.Secret) | PASS | main.go:170: Secret: jwtSecret -- korrekt abgeleitet |
| 5 | SEC-08 HKDF | Keine direkte Nutzung von cfg.API.Secret als Schluessel | PASS | Grep zeigt nur main.go:80 (masterKey Zuweisung) und Kommentar -- kein Durchreichen |
| 6 | SEC-01 Rollen | roleLevel() Hierarchie korrekt | PASS | server.go:46-55: superadmin=5, admin=4, domain_admin=3, auditor=2, user=1. Konsistent mit auth.go:284-293 |
| 7 | SEC-01 Rollen | handleCreateUser prueft roleLevel | PASS | server.go:387: roleLevel(req.Role) >= roleLevel(sess.Role) -- domain_admin (3) kann keinen superadmin (5) anlegen |
| 8 | SEC-01 Rollen | handleUpdateUser prueft roleLevel fuer Ziel UND neue Rolle | PASS | server.go:464,468: Beide Checks vorhanden (target.Role und req.Role) |
| 9 | SEC-01 Rollen | handleDeleteUser prueft roleLevel | PASS | server.go:525: roleLevel(target.Role) >= roleLevel(sess.Role) |
| 10 | SEC-01 Rollen | domain_admin kann sich nicht zu superadmin befoerdern | PASS | handleUpdateUser Zeile 468: Selbst-Update wird durch roleLevel(target.Role) >= roleLevel(sess.Role) in Zeile 464 blockiert (eigene Rolle = domain_admin (3) >= domain_admin (3) = true -> 403) |
| 11 | BUG-1 Fix | handleSaveTenantLDAP Allowlist | PASS | ldap_tenants.go:452: Allowlist {"user": true, "auditor": true} korrekt |
| 12 | BUG-1 Fix | handleSaveTenantLDAP prueft group_mappings | PASS | ldap_tenants.go:457-461: Jedes GroupMapping wird gegen Allowlist geprueft |
| 13 | BUG-1 Fix | handleAdminSaveTenantLDAP Allowlist | PASS | ldap_tenants.go:593: Allowlist {"user": true, "auditor": true, "domain_admin": true} -- korrekt, superadmin ausgeschlossen |
| 14 | BUG-1 Fix | handleAdminSaveTenantLDAP prueft group_mappings | PASS | ldap_tenants.go:598-602: Jedes GroupMapping wird geprueft |
| 15 | BUG-1 Fix | Leerer default_role erlaubt | PASS | ldap_tenants.go:453: cfg.DefaultRole != "" -- leerer String wird nicht geprueft, also erlaubt |
| 16 | Login LDAP | Reihenfolge: lokal -> tenant LDAP -> global LDAP | PASS | auth.go:66-155: (1) local VerifyPassword (2) tenant LDAP ab Zeile 74 (3) global LDAP ab Zeile 119 |
| 17 | Login LDAP | Globales LDAP kann Tenant-LDAP nicht ueberschreiben | PASS | Tenant-LDAP (Step 2) wird vor globalem LDAP (Step 3) geprueft. Bei Erfolg in Step 2 wird sofort returned |
| 18 | SEC-22 | handleGetMail: isValidMailID vor Store-Zugriff | PASS | server.go:908-912 |
| 19 | SEC-22 | handleGetAttachment: isValidMailID vor Store-Zugriff | PASS | server.go:997-1001 |
| 20 | SEC-22 | handleGetRaw: isValidMailID vor Store-Zugriff | PASS | server.go:1060-1064 |
| 21 | SEC-22 | handleExportPDF: isValidMailID vor Store-Zugriff | PASS | export.go:331-334 |
| 22 | SEC-22 | handleExportZIP: isValidMailID fuer alle IDs | PASS | export.go:413-418 |
| 23 | SEC-22 | Regex als Paket-Variable kompiliert | PASS | server.go:37: var mailIDRegex = regexp.MustCompile(...) -- einmalig kompiliert |
| 24 | SEC-26 | isAllowed: leere AllowedIPs -> false | PASS | smtpd.go:317-318: len(d.cfg.AllowedIPs) == 0 returns false (fail-closed) |
| 25 | SEC-26 | Kommentar in config.go vorhanden | PASS | config.go:59-61: Erklaert allowed_ips: ["0.0.0.0/0", "::/0"] |
| 26 | SEC-05 | handleExportPDF: Tenant-Isolation | PASS | export.go:351-357: Prueft sess.TenantID != nil und vergleicht mit Mail-Tenant |
| 27 | SEC-05 | handleExportZIP: Tenant-Isolation | PASS | export.go:423-439: Laedt alle IDs via GetAllIDsByTenant und prueft jede angeforderte ID |
| 28 | SEC-05 | superadmin (TenantID==nil) sieht alle Mails | PASS | Alle Tenant-Checks starten mit if sess.TenantID != nil -- superadmin hat nil -> kein Check -> Vollzugriff |
| 29 | Tenant-LDAP Auth | /api/tenant/ldap ohne Session -> 401 | PASS | ldap_tenants.go:396: s.authAdmin(...) = authMiddleware(tenantMiddleware(requireRole(RoleDomainAdmin, ...))) -- authMiddleware blockt ohne Token |
| 30 | Tenant-LDAP Auth | user-Rolle kann /api/tenant/ldap nicht aufrufen | PASS | requireRole(RoleDomainAdmin, ...) -> HasRole("user", "domain_admin") = false (1 < 3) -> HTTP 403 |
| 31 | Tenant-LDAP Auth | GET /api/tenant/ldap gibt bind_password NICHT im Klartext zurueck | PASS | handleGetTenantLDAP ruft tenantLdapStore.Get() auf -> tenant_store.go:77-78: Password wird zu "..." maskiert |
| 32 | SEC-01 Hierarchie | Hierarchie-Diskrepanz Spec vs. Code | WARN | Audit-Aufgabe nennt superadmin=5 > domain_admin=4 > admin=3, Code hat admin=4 > domain_admin=3. Code-Hierarchie ist logisch korrekt (globaler admin > tenant domain_admin). Spec-Text im Audit-Auftrag weicht ab. |
| 33 | Allgemein | SQL Injection | PASS | Keine String-Konkatenation in SQL-Queries gefunden. Alle Stores verwenden pgx parameterized queries |
| 34 | Allgemein | Fehlerausgaben mit err.Error() | FAIL | Siehe Finding F-01 |
| 35 | Allgemein | Audit-Log fuer sicherheitsrelevante Aktionen | WARN | Siehe Finding W-01 |
Kritische Findings (FAIL)
F-01: Interne Fehlerdetails an Client (Severity: MEDIUM, Priority: P2)
Datei: internal/api/server.go
Zeilen: 407, 480, 535, 1541, 1917
Mehrere Handler geben err.Error() direkt an den HTTP-Client weiter:
- Zeile 407 (
handleCreateUser):writeError(w, http.StatusBadRequest, err.Error()) - Zeile 480 (
handleUpdateUser):writeError(w, http.StatusBadRequest, err.Error()) - Zeile 535 (
handleDeleteUser):writeError(w, http.StatusInternalServerError, err.Error()) - Zeile 1541:
writeError(w, http.StatusConflict, err.Error()) - Zeile 1917:
"could not write jail.local: "+err.Error()
Ebenso in internal/api/upload.go:56: "multipart parse failed: "+err.Error()
Risiko: Interne Implementierungsdetails, Datenbank-Fehlermeldungen oder Dateipfade koennten an Angreifer weitergegeben werden. Dies erleichtert Reconnaissance.
Fix-Vorschlag: Generische Fehlermeldungen an den Client senden, Details nur ins Server-Log schreiben:
s.logger.Error("create user failed", "err", err)
writeError(w, http.StatusBadRequest, "user creation failed")
F-02: Login-Handler gibt err.Error() im Audit-Detail weiter (Severity: LOW, Priority: P3)
Datei: internal/api/server.go
Zeile: 247
Detail: err.Error(),
Der Audit-Log-Eintrag bei fehlgeschlagenem Login enthaelt den vollstaendigen Fehlertext. Dies ist fuer das Audit-Log akzeptabel (intern), aber der gleiche err.Error() koennte bei unsachgemaesser Audit-Log-Exposition Details verraten. In Kombination mit dem Audit-API-Endpoint (GET /api/audit, zugaenglich fuer auditor-Rolle) koennten LDAP-Fehlermeldungen mit internen Hostnamen/Ports sichtbar werden.
Fix-Vorschlag: Fehlerdetails im Audit-Log auf vordefinierte Kategorien beschraenken: "invalid_password", "user_not_found", "ldap_error".
Warnungen (WARN)
W-01: Fehlende Audit-Logs bei einigen sicherheitsrelevanten Aktionen
Betroffene Handler:
handleUpdateTenant(ldap_tenants.go:247-289): Kein Audit-Log-Eintrag bei Tenant-Update.handleAddTenantDomain/handleRemoveTenantDomain: Kein Audit-Log-Eintrag.handleGetAttachment,handleGetRaw: Kein Audit-Log fuer einzelne Zugriffe (nur Export wird geloggt).
Empfehlung: Audit-Log-Eintraege fuer Tenant-Aenderungen und Domain-Zuweisungen hinzufuegen. Mail-Lesezugriffe sind optional, aber fuer GoBD-Compliance empfehlenswert.
W-02: Cookie Secure-Flag nicht gesetzt
Datei: internal/api/server.go
Zeile: 269
// Secure: true -- enable when TLS is terminated at this server
Das Secure-Flag ist auskommentiert. Im On-Premise-Deployment ohne TLS-Terminierung am Go-Server ist dies verstaendlich, aber wenn ein Reverse-Proxy (nginx/caddy) TLS terminiert, koennte das Cookie ueber eine unverschluesselte Verbindung gesendet werden.
Empfehlung: Konfigurierbar machen oder automatisch setzen wenn hinter TLS-Proxy (X-Forwarded-Proto: https).
W-03: X-Forwarded-For Header nicht validiert
Datei: internal/api/server.go
Zeile: 880-884
func remoteIP(r *http.Request) string {
if fwd := r.Header.Get("X-Forwarded-For"); fwd != "" {
return strings.Split(fwd, ",")[0]
}
return r.RemoteAddr
}
Ein Angreifer kann den X-Forwarded-For-Header faelschen, um eine andere IP in Audit-Logs und Rate-Limiting zu injizieren. Dies ermoeglicht:
- Rate-Limit-Umgehung beim Login (verschiedene gefaelschte IPs)
- Falsche IP-Adressen in Audit-Logs
Empfehlung: Nur dem aeussersten Proxy-Hop vertrauen oder die Anzahl vertrauenswuerdiger Proxies konfigurierbar machen. Alternativ r.RemoteAddr verwenden wenn kein Proxy konfiguriert ist.
W-04: Attachment-Filename ohne Sanitierung im Content-Disposition
Datei: internal/api/server.go
Zeile: 1052
w.Header().Set("Content-Disposition", fmt.Sprintf(`attachment; filename="%s"`, filename))
Der Attachment-Filename kommt direkt aus der geparsten E-Mail. Ein boesartiger Absender koennte einen Dateinamen mit " oder Newlines senden, was zu Header-Injection fuehren koennte.
Ebenso in export.go:377 (dort ist die ID gekuerzt -- sicher) und server.go:1098 (ID-basiert -- sicher).
Empfehlung: Dateinamen sanitieren: Nur alphanumerische Zeichen, Punkt, Bindestrich und Unterstrich erlauben. Alternativ RFC 6266 filename* mit Encoding verwenden.
W-05: HKDF ohne Info-Parameter
Datei: cmd/archivmail/main.go
Zeilen: 82, 87
hkdf.New(sha256.New, masterKey, []byte("archivmail-jwt-v1"), nil)
Der HKDF-Aufruf nutzt den Salt-Parameter zur Schluesseltrennung, aber der info-Parameter ist nil. Gemaess RFC 5869 ist der info-Parameter fuer die Kontextbindung vorgesehen, waehrend salt fuer die Randomisierung dient. Funktional ist dies korrekt und sicher, da die verschiedenen Salt-Werte eine wirksame Schluesseltrennung gewaehrleisten. Es waere jedoch konzeptionell sauberer, den Kontext (archivmail-jwt-v1) als info und einen festen Salt zu verwenden.
Empfehlung: Kein sofortiger Handlungsbedarf -- die aktuelle Implementierung ist kryptographisch sicher. Bei einem zukuenftigen Refactoring koennte der Kontext in den info-Parameter verschoben werden.
Geprueft und fuer gut befunden
- HKDF-Schluessel-Trennung (SEC-08): Korrekt implementiert mit zwei unabhaengigen Ableitungen.
- Rollenhierarchie (SEC-01): Konsistent in server.go und auth.go. Privilege-Escalation-Checks in allen drei User-Management-Handlern vorhanden.
- BUG-1 Fix (Tenant-LDAP Allowlists): Beide Endpunkte (domain_admin und superadmin) pruefen sowohl
default_roleals auchgroup_mappings[].role. - Login-Reihenfolge: Korrekt: lokal -> tenant LDAP -> global LDAP.
- Mail-ID-Validierung (SEC-22): Alle 5 Handler pruefen vor dem Store-Zugriff. Regex einmalig kompiliert.
- SMTP Fail-Closed (SEC-26): Leere Allowlist blockiert alle Verbindungen.
- Export Tenant-Isolation (SEC-05): PDF und ZIP Export pruefen Tenant-Zugehoerigkeit. Superadmin hat Vollzugriff.
- Tenant-LDAP Endpunkte: Authentifizierung und Rollenpruefung korrekt. Passwort wird maskiert.
- SQL-Injection: Keine String-Konkatenation in Queries gefunden.