scripte/archivmail
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(security): Kritische Sicherheitslücken beheben (SEC-01/02/03/05/08/17/22/26/28)

Browse Source 
- SEC-01: Privilege Escalation verhindert — Rollenhierarchie in Create/Update/DeleteUser
- SEC-02: Tenant-Isolation in Update/DeleteUser — domain_admin nur eigene Nutzer
- SEC-03: IMAP/POP3 Owner-Check via auth.HasRole statt direktem String-Vergleich
- SEC-05: Export PDF/ZIP prüft Tenant-Zugehörigkeit vor Dateiausgabe
- SEC-08: HKDF-SHA256 trennt JWT-Secret von AES-Key (archivmail-jwt-v1 / archivmail-aes-v1)
- SEC-17: handleSecurityFix erfordert requireRole(superadmin)
- SEC-22: Mail-ID Regex [0-9a-f]{64} in allen Handlern (Path-Traversal-Schutz)
- SEC-26: SMTP Fail-Closed — leere AllowedIPs blockiert alles statt zu erlauben
- SEC-28: handleGetRaw — Parse-Fehler bricht ab statt Fallthrough zu Dateizugriff

BREAKING: IMAP/POP3/LDAP-Passwörter müssen nach Deploy einmalig neu eingegeben
werden (neuer AES-Key). JWT-Sessions laufen ab (einmaliges Re-Login nötig).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
sysops
2026-03-18 00:05:47 +01:00
parent 143db65755
commit 46d7bfe608
6 changed files with 200 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/api/export.go
+43 -1
View File
@@ -327,6 +327,11 @@ func buildMailPDF(id string, pm *mailparser.ParsedMail, rawSize int) []byte {
func (s *Server) handleExportPDF(w http.ResponseWriter, r *http.Request) {
id := r.PathValue("id")
// SEC-22: Validate mail ID format to prevent path traversal.
if !isValidMailID(id) {
writeError(w, http.StatusBadRequest, "invalid mail id")
return
}
raw, err := s.store.Load(id)
if err != nil {
@@ -340,8 +345,18 @@ func (s *Server) handleExportPDF(w http.ResponseWriter, r *http.Request) {
return
}
// User role: only own mailbox
sess := sessionFromCtx(r.Context())
// SEC-05: Tenant isolation for PDF export.
if sess.TenantID != nil {
mailTenant, _ := s.store.GetTenantForMail(r.Context(), id)
if mailTenant == nil || *mailTenant != *sess.TenantID {
writeError(w, http.StatusForbidden, "access denied")
return
}
}
// User role: only own mailbox
if sess.Role == userstore.RoleUser {
u, err := s.users.GetByUsername(sess.Username)
if err != nil || !mailBelongsToUser(pm, u.Email) {
@@ -394,8 +409,35 @@ func (s *Server) handleExportZIP(w http.ResponseWriter, r *http.Request) {
return
}
// SEC-22: Validate all mail IDs before processing.
for _, id := range req.IDs {
if !isValidMailID(id) {
writeError(w, http.StatusBadRequest, "invalid mail id")
return
}
}
sess := sessionFromCtx(r.Context())
// SEC-05: Tenant isolation — verify all requested IDs belong to caller's tenant.
if sess.TenantID != nil {
allowedIDs, err := s.store.GetAllIDsByTenant(r.Context(), sess.TenantID)
if err != nil {
writeError(w, http.StatusInternalServerError, "tenant check failed")
return
}
allowed := make(map[string]struct{}, len(allowedIDs))
for _, aid := range allowedIDs {
allowed[aid] = struct{}{}
}
for _, id := range req.IDs {
if _, ok := allowed[id]; !ok {
writeError(w, http.StatusForbidden, "access denied")
return
}
}
}
// For RoleUser, look up user email once for access checks
var userEmail string
if sess.Role == userstore.RoleUser {
internal/api/server.go
+119 -21
View File
@@ -17,6 +17,8 @@ import (
"syscall"
"time"
"regexp"
"github.com/archivmail/config"
"github.com/archivmail/internal/audit"
"github.com/archivmail/internal/auth"
@@ -31,6 +33,27 @@ import (
"github.com/archivmail/pkg/mailparser"
)
// SEC-22: Compiled regex for mail ID validation to prevent path traversal.
var mailIDRegex = regexp.MustCompile(`^[0-9a-f]{64}$`)
// isValidMailID validates that a mail ID matches the expected hex format.
func isValidMailID(id string) bool {
return mailIDRegex.MatchString(id)
}
// roleLevel returns the privilege level for a role string.
// Hierarchy: superadmin=5 > admin=4 > domain_admin=3 > auditor=2 > user=1
func roleLevel(role string) int {
levels := map[string]int{
userstore.RoleUser: 1,
userstore.RoleAuditor: 2,
userstore.RoleDomainAdmin: 3,
userstore.RoleAdmin: 4,
userstore.RoleSuperAdmin: 5,
}
return levels[role]
}
type contextKey string
const (
@@ -132,7 +155,8 @@ func (s *Server) routes() {
s.mux.HandleFunc("GET /api/admin/system/stats", s.authAdmin(s.handleSystemStats))
s.mux.HandleFunc("GET /api/admin/security/audit", s.authAdmin(s.handleSecurityAudit))
s.mux.HandleFunc("POST /api/admin/security/fix", s.authAdmin(s.handleSecurityFix))
// SEC-17: Security fix actions require superadmin, not just domain_admin.
s.mux.HandleFunc("POST /api/admin/security/fix", s.auth(s.requireRole(userstore.RoleSuperAdmin, s.handleSecurityFix)))
// Export routes
s.mux.HandleFunc("GET /api/export/pdf/{id}", s.auth(s.requireMailAccess(s.handleExportPDF)))
@@ -350,18 +374,33 @@ func (s *Server) handleCreateUser(w http.ResponseWriter, r *http.Request) {
return
}
// SEC-01: Privilege escalation check — caller must not assign a role
// at or above their own level.
sess := sessionFromCtx(r.Context())
if roleLevel(req.Role) >= roleLevel(sess.Role) {
writeError(w, http.StatusForbidden, "insufficient privileges to assign this role")
return
}
// SEC-02: Tenant isolation — non-superadmin users can only create users
// within their own tenant.
var tenantID *int64
if sess.TenantID != nil {
tenantID = sess.TenantID
}
user, err := s.users.Create(userstore.CreateUserRequest{
Username: req.Username,
Email: req.Email,
Password: req.Password,
Role: req.Role,
TenantID: tenantID,
})
if err != nil {
writeError(w, http.StatusBadRequest, err.Error())
return
}
sess := sessionFromCtx(r.Context())
s.audlog.Log(audit.Entry{
EventType: audit.EventUserMgmt,
Username: sess.Username,
@@ -397,6 +436,33 @@ func (s *Server) handleUpdateUser(w http.ResponseWriter, r *http.Request) {
return
}
sess := sessionFromCtx(r.Context())
// SEC-02: Tenant isolation — load target user and verify same tenant.
target, err := s.users.GetByID(id)
if err != nil {
writeError(w, http.StatusNotFound, "user not found")
return
}
if sess.TenantID != nil {
if target.TenantID == nil || *target.TenantID != *sess.TenantID {
writeError(w, http.StatusForbidden, "access denied")
return
}
}
// SEC-01: Privilege escalation check — caller must not assign a role
// at or above their own level, and must not modify users at or above
// their own level.
if roleLevel(target.Role) >= roleLevel(sess.Role) {
writeError(w, http.StatusForbidden, "insufficient privileges to modify this user")
return
}
if req.Role != nil && roleLevel(*req.Role) >= roleLevel(sess.Role) {
writeError(w, http.StatusForbidden, "insufficient privileges to assign this role")
return
}
updated, err := s.users.Update(id, userstore.UpdateUserRequest{
Email: req.Email,
Role: req.Role,
@@ -408,7 +474,6 @@ func (s *Server) handleUpdateUser(w http.ResponseWriter, r *http.Request) {
return
}
sess := sessionFromCtx(r.Context())
s.audlog.Log(audit.Entry{
EventType: audit.EventUserMgmt,
Username: sess.Username,
@@ -440,6 +505,21 @@ func (s *Server) handleDeleteUser(w http.ResponseWriter, r *http.Request) {
return
}
// SEC-02: Tenant isolation — domain_admin can only delete users in their own tenant.
sess := sessionFromCtx(r.Context())
if sess.TenantID != nil {
if target.TenantID == nil || *target.TenantID != *sess.TenantID {
writeError(w, http.StatusForbidden, "access denied")
return
}
}
// SEC-01: Cannot delete users at or above own privilege level.
if roleLevel(target.Role) >= roleLevel(sess.Role) {
writeError(w, http.StatusForbidden, "insufficient privileges to delete this user")
return
}
if err := s.users.DeleteSafe(id); err != nil {
if err.Error() == "userstore: cannot delete last admin" {
writeError(w, http.StatusConflict, "cannot delete the last active admin")
@@ -459,7 +539,6 @@ func (s *Server) handleDeleteUser(w http.ResponseWriter, r *http.Request) {
}
}
sess := sessionFromCtx(r.Context())
s.audlog.Log(audit.Entry{
EventType: audit.EventUserMgmt,
Username: sess.Username,
@@ -808,6 +887,11 @@ func (s *Server) requireMailAccess(next http.HandlerFunc) http.HandlerFunc {
func (s *Server) handleGetMail(w http.ResponseWriter, r *http.Request) {
id := r.PathValue("id")
// SEC-22: Validate mail ID format to prevent path traversal.
if !isValidMailID(id) {
writeError(w, http.StatusBadRequest, "invalid mail id")
return
}
raw, err := s.store.Load(id)
if err != nil {
@@ -892,6 +976,11 @@ func (s *Server) handleGetMail(w http.ResponseWriter, r *http.Request) {
func (s *Server) handleGetAttachment(w http.ResponseWriter, r *http.Request) {
id := r.PathValue("id")
// SEC-22: Validate mail ID format to prevent path traversal.
if !isValidMailID(id) {
writeError(w, http.StatusBadRequest, "invalid mail id")
return
}
indexStr := r.PathValue("index")
idx, err := strconv.Atoi(indexStr)
if err != nil {
@@ -950,6 +1039,11 @@ func (s *Server) handleGetAttachment(w http.ResponseWriter, r *http.Request) {
func (s *Server) handleGetRaw(w http.ResponseWriter, r *http.Request) {
id := r.PathValue("id")
// SEC-22: Validate mail ID format to prevent path traversal.
if !isValidMailID(id) {
writeError(w, http.StatusBadRequest, "invalid mail id")
return
}
raw, err := s.store.Load(id)
if err != nil {
@@ -968,15 +1062,17 @@ func (s *Server) handleGetRaw(w http.ResponseWriter, r *http.Request) {
}
}
// Access check for user role
// SEC-28: Access check for user role — parse failure must NOT grant access.
if sess.Role == userstore.RoleUser {
pm, err := mailparser.Parse(raw)
if err == nil {
u, err := s.users.GetByUsername(sess.Username)
if err != nil || !mailBelongsToUser(pm, u.Email) {
writeError(w, http.StatusForbidden, "access denied")
return
}
if err != nil {
writeError(w, http.StatusInternalServerError, "failed to parse mail")
return
}
u, err := s.users.GetByUsername(sess.Username)
if err != nil || !mailBelongsToUser(pm, u.Email) {
writeError(w, http.StatusForbidden, "access denied")
return
}
}
@@ -1182,7 +1278,8 @@ func (s *Server) handleListImap(w http.ResponseWriter, r *http.Request) {
return
}
sess := sessionFromCtx(r.Context())
isAdmin := sess.Role == userstore.RoleAdmin
// SEC-03: Use HasRole to correctly check admin privileges (domain_admin, admin, superadmin).
isAdmin := auth.HasRole(sess.Role, userstore.RoleDomainAdmin)
accounts, err := s.imapStore.List(r.Context(), sess.Username, isAdmin)
if err != nil {
writeError(w, http.StatusInternalServerError, "failed to list IMAP accounts")
@@ -1265,7 +1362,7 @@ func (s *Server) handleDeleteImap(w http.ResponseWriter, r *http.Request) {
}
sess := sessionFromCtx(r.Context())
if acc.Owner != sess.Username && sess.Role != userstore.RoleAdmin {
if acc.Owner != sess.Username && !auth.HasRole(sess.Role, userstore.RoleDomainAdmin) {
writeError(w, http.StatusForbidden, "access denied")
return
}
@@ -1353,7 +1450,7 @@ func (s *Server) handleStartImport(w http.ResponseWriter, r *http.Request) {
}
sess := sessionFromCtx(r.Context())
if acc.Owner != sess.Username && sess.Role != userstore.RoleAdmin {
if acc.Owner != sess.Username && !auth.HasRole(sess.Role, userstore.RoleDomainAdmin) {
writeError(w, http.StatusForbidden, "access denied")
return
}
@@ -1389,7 +1486,7 @@ func (s *Server) handleImapProgress(w http.ResponseWriter, r *http.Request) {
}
sess := sessionFromCtx(r.Context())
if acc.Owner != sess.Username && sess.Role != userstore.RoleAdmin {
if acc.Owner != sess.Username && !auth.HasRole(sess.Role, userstore.RoleDomainAdmin) {
writeError(w, http.StatusForbidden, "access denied")
return
}
@@ -1417,7 +1514,7 @@ func (s *Server) handleSyncNow(w http.ResponseWriter, r *http.Request) {
}
sess := sessionFromCtx(r.Context())
if acc.Owner != sess.Username && sess.Role != userstore.RoleAdmin {
if acc.Owner != sess.Username && !auth.HasRole(sess.Role, userstore.RoleDomainAdmin) {
writeError(w, http.StatusForbidden, "access denied")
return
}
@@ -1452,7 +1549,7 @@ func (s *Server) handleUpdateImapInterval(w http.ResponseWriter, r *http.Request
}
sess := sessionFromCtx(r.Context())
if acc.Owner != sess.Username && sess.Role != userstore.RoleAdmin {
if acc.Owner != sess.Username && !auth.HasRole(sess.Role, userstore.RoleDomainAdmin) {
writeError(w, http.StatusForbidden, "access denied")
return
}
@@ -1860,7 +1957,8 @@ func (s *Server) handleListPop3(w http.ResponseWriter, r *http.Request) {
return
}
sess := sessionFromCtx(r.Context())
isAdmin := sess.Role == userstore.RoleAdmin
// SEC-03: Use HasRole to correctly check admin privileges (domain_admin, admin, superadmin).
isAdmin := auth.HasRole(sess.Role, userstore.RoleDomainAdmin)
accounts, err := s.pop3Store.List(r.Context(), sess.Username, isAdmin)
if err != nil {
writeError(w, http.StatusInternalServerError, "failed to list POP3 accounts")
@@ -1940,7 +2038,7 @@ func (s *Server) handleDeletePop3(w http.ResponseWriter, r *http.Request) {
}
sess := sessionFromCtx(r.Context())
if acc.Owner != sess.Username && sess.Role != userstore.RoleAdmin {
if acc.Owner != sess.Username && !auth.HasRole(sess.Role, userstore.RoleDomainAdmin) {
writeError(w, http.StatusForbidden, "access denied")
return
}
@@ -2032,7 +2130,7 @@ func (s *Server) handleStartPop3Import(w http.ResponseWriter, r *http.Request) {
}
sess := sessionFromCtx(r.Context())
if acc.Owner != sess.Username && sess.Role != userstore.RoleAdmin {
if acc.Owner != sess.Username && !auth.HasRole(sess.Role, userstore.RoleDomainAdmin) {
writeError(w, http.StatusForbidden, "access denied")
return
}
@@ -2068,7 +2166,7 @@ func (s *Server) handlePop3Progress(w http.ResponseWriter, r *http.Request) {
}
sess := sessionFromCtx(r.Context())
if acc.Owner != sess.Username && sess.Role != userstore.RoleAdmin {
if acc.Owner != sess.Username && !auth.HasRole(sess.Role, userstore.RoleDomainAdmin) {
writeError(w, http.StatusForbidden, "access denied")
return
}