## 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: ```go 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 ```go 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 ```go // 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 ```go 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: 1. Rate-Limit-Umgehung beim Login (verschiedene gefaelschte IPs) 2. 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 ```go 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 ```go 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 1. **HKDF-Schluessel-Trennung (SEC-08):** Korrekt implementiert mit zwei unabhaengigen Ableitungen. 2. **Rollenhierarchie (SEC-01):** Konsistent in server.go und auth.go. Privilege-Escalation-Checks in allen drei User-Management-Handlern vorhanden. 3. **BUG-1 Fix (Tenant-LDAP Allowlists):** Beide Endpunkte (domain_admin und superadmin) pruefen sowohl `default_role` als auch `group_mappings[].role`. 4. **Login-Reihenfolge:** Korrekt: lokal -> tenant LDAP -> global LDAP. 5. **Mail-ID-Validierung (SEC-22):** Alle 5 Handler pruefen vor dem Store-Zugriff. Regex einmalig kompiliert. 6. **SMTP Fail-Closed (SEC-26):** Leere Allowlist blockiert alle Verbindungen. 7. **Export Tenant-Isolation (SEC-05):** PDF und ZIP Export pruefen Tenant-Zugehoerigkeit. Superadmin hat Vollzugriff. 8. **Tenant-LDAP Endpunkte:** Authentifizierung und Rollenpruefung korrekt. Passwort wird maskiert. 9. **SQL-Injection:** Keine String-Konkatenation in Queries gefunden.