fix(security): behebe F-01/F-02/W-03/W-04 aus Security-Audit + PROJ-24 TOTP 2FA
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>
This commit is contained in:
@@ -91,7 +91,7 @@ func newTestEnv(t *testing.T) *testEnv {
|
||||
users.Create(userstore.CreateUserRequest{Username: "auditor", Email: "auditor@x.com", Password: "auditorpass", Role: userstore.RoleAuditor})
|
||||
users.Create(userstore.CreateUserRequest{Username: "user1", Email: "user1@x.com", Password: "userpass", Role: userstore.RoleUser})
|
||||
|
||||
authMgr := auth.New(users, nil, "test-secret-must-be-long-enough-32")
|
||||
authMgr := auth.New(users, nil, "test-secret-must-be-long-enough-32", "0000000000000000000000000000000000000000000000000000000000000000")
|
||||
cfg := config.APIConfig{Bind: ":18080", Secret: "test-secret-must-be-long-enough-32"}
|
||||
srv := api.New(cfg, store, idx, authMgr, users, audlog, logger)
|
||||
|
||||
|
||||
+73
-9
@@ -194,6 +194,13 @@ func (s *Server) routes() {
|
||||
s.mux.HandleFunc("POST /api/pop3/test", s.auth(s.handleTestPop3))
|
||||
s.mux.HandleFunc("POST /api/pop3/{id}/import", s.auth(s.handleStartPop3Import))
|
||||
s.mux.HandleFunc("GET /api/pop3/{id}/progress", s.auth(s.handlePop3Progress))
|
||||
|
||||
// PROJ-24: TOTP 2FA routes
|
||||
s.mux.HandleFunc("GET /api/auth/totp/setup", s.auth(s.handleTOTPSetupGet))
|
||||
s.mux.HandleFunc("POST /api/auth/totp/setup", s.auth(s.handleTOTPSetupPost))
|
||||
s.mux.HandleFunc("DELETE /api/auth/totp", s.auth(s.handleTOTPDisable))
|
||||
s.mux.HandleFunc("POST /api/auth/totp", s.handleTOTPLogin) // no auth middleware — uses pending token
|
||||
s.mux.HandleFunc("POST /api/admin/users/{id}/totp/reset", s.authAdmin(s.handleTOTPReset))
|
||||
}
|
||||
|
||||
// ServeHTTP implements http.Handler.
|
||||
@@ -236,7 +243,7 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
token, user, err := s.authMgr.Login(req.Username, req.Password)
|
||||
token, user, totpRequired, err := s.authMgr.Login(req.Username, req.Password)
|
||||
if err != nil {
|
||||
_ = s.users.RecordLoginAttempt(req.Username, remoteIP(r))
|
||||
s.audlog.Log(audit.Entry{
|
||||
@@ -244,12 +251,28 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) {
|
||||
Username: req.Username,
|
||||
IPAddress: remoteIP(r),
|
||||
Success: false,
|
||||
Detail: err.Error(),
|
||||
Detail: classifyLoginError(err),
|
||||
})
|
||||
writeError(w, http.StatusUnauthorized, "invalid credentials")
|
||||
return
|
||||
}
|
||||
|
||||
// PROJ-24: If TOTP is enabled, return a pending token instead of a full session.
|
||||
if totpRequired {
|
||||
s.audlog.Log(audit.Entry{
|
||||
EventType: audit.EventLogin,
|
||||
Username: user.Username,
|
||||
IPAddress: remoteIP(r),
|
||||
Success: true,
|
||||
Detail: "totp_pending",
|
||||
})
|
||||
writeJSON(w, http.StatusAccepted, map[string]interface{}{
|
||||
"totp_required": true,
|
||||
"session_token": token,
|
||||
})
|
||||
return
|
||||
}
|
||||
|
||||
_ = s.users.UpdateLastLogin(user.ID)
|
||||
|
||||
s.audlog.Log(audit.Entry{
|
||||
@@ -404,7 +427,8 @@ func (s *Server) handleCreateUser(w http.ResponseWriter, r *http.Request) {
|
||||
TenantID: tenantID,
|
||||
})
|
||||
if err != nil {
|
||||
writeError(w, http.StatusBadRequest, err.Error())
|
||||
s.logger.Error("create user failed", "err", err)
|
||||
writeError(w, http.StatusBadRequest, "user creation failed")
|
||||
return
|
||||
}
|
||||
|
||||
@@ -477,7 +501,8 @@ func (s *Server) handleUpdateUser(w http.ResponseWriter, r *http.Request) {
|
||||
Password: req.Password,
|
||||
})
|
||||
if err != nil {
|
||||
writeError(w, http.StatusBadRequest, err.Error())
|
||||
s.logger.Error("update user failed", "err", err)
|
||||
writeError(w, http.StatusBadRequest, "user update failed")
|
||||
return
|
||||
}
|
||||
|
||||
@@ -532,7 +557,8 @@ func (s *Server) handleDeleteUser(w http.ResponseWriter, r *http.Request) {
|
||||
writeError(w, http.StatusConflict, "cannot delete the last active admin")
|
||||
return
|
||||
}
|
||||
writeError(w, http.StatusInternalServerError, err.Error())
|
||||
s.logger.Error("delete user failed", "err", err)
|
||||
writeError(w, http.StatusInternalServerError, "user deletion failed")
|
||||
return
|
||||
}
|
||||
|
||||
@@ -876,9 +902,45 @@ func tenantFromCtx(ctx context.Context) *int64 {
|
||||
return v
|
||||
}
|
||||
|
||||
// sanitizeFilename strips characters that could be used for HTTP header injection
|
||||
// (quotes, newlines, control chars) from attachment filenames coming from parsed
|
||||
// e-mails. Only alphanumerics, spaces, dots, hyphens, and underscores are kept.
|
||||
func sanitizeFilename(name string) string {
|
||||
var b strings.Builder
|
||||
for _, r := range name {
|
||||
if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') ||
|
||||
r == '.' || r == '-' || r == '_' || r == ' ' {
|
||||
b.WriteRune(r)
|
||||
}
|
||||
}
|
||||
return b.String()
|
||||
}
|
||||
|
||||
// classifyLoginError maps internal login errors to safe audit-log categories.
|
||||
// Raw error messages must not be stored in audit logs since auditor-role
|
||||
// users can read them via GET /api/audit and internal details (LDAP hostnames,
|
||||
// port numbers, etc.) would be exposed.
|
||||
func classifyLoginError(err error) string {
|
||||
if err == nil {
|
||||
return ""
|
||||
}
|
||||
msg := err.Error()
|
||||
switch {
|
||||
case strings.Contains(msg, "not found"), strings.Contains(msg, "invalid password"),
|
||||
strings.Contains(msg, "invalid credentials"):
|
||||
return "invalid_password"
|
||||
case strings.Contains(msg, "ldap"), strings.Contains(msg, "LDAP"):
|
||||
return "ldap_error"
|
||||
case strings.Contains(msg, "disabled"), strings.Contains(msg, "inactive"):
|
||||
return "account_disabled"
|
||||
default:
|
||||
return "unknown"
|
||||
}
|
||||
}
|
||||
|
||||
func remoteIP(r *http.Request) string {
|
||||
if fwd := r.Header.Get("X-Forwarded-For"); fwd != "" {
|
||||
return strings.Split(fwd, ",")[0]
|
||||
return strings.TrimSpace(strings.Split(fwd, ",")[0])
|
||||
}
|
||||
return r.RemoteAddr
|
||||
}
|
||||
@@ -1043,7 +1105,7 @@ func (s *Server) handleGetAttachment(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
|
||||
a := pm.Attachments[idx]
|
||||
filename := a.Filename
|
||||
filename := sanitizeFilename(a.Filename)
|
||||
if filename == "" {
|
||||
filename = fmt.Sprintf("attachment-%d", idx)
|
||||
}
|
||||
@@ -1538,7 +1600,8 @@ func (s *Server) handleSyncNow(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
|
||||
if err := s.imapScheduler.TriggerSync(r.Context(), id); err != nil {
|
||||
writeError(w, http.StatusConflict, err.Error())
|
||||
s.logger.Error("trigger sync failed", "err", err)
|
||||
writeError(w, http.StatusConflict, "sync already running or failed to start")
|
||||
return
|
||||
}
|
||||
|
||||
@@ -1914,7 +1977,8 @@ func (s *Server) handleSecurityFix(w http.ResponseWriter, r *http.Request) {
|
||||
if _, err := os.Stat(jailPath); os.IsNotExist(err) {
|
||||
jailConf := "[sshd]\nenabled = true\nmaxretry = 5\nbantime = 3600\nfindtime = 600\n"
|
||||
if err := os.WriteFile(jailPath, []byte(jailConf), 0644); err != nil {
|
||||
writeError(w, http.StatusInternalServerError, "could not write jail.local: "+err.Error())
|
||||
s.logger.Error("could not write jail.local", "err", err)
|
||||
writeError(w, http.StatusInternalServerError, "security config update failed")
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,262 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"encoding/base64"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"strconv"
|
||||
|
||||
"github.com/archivmail/internal/audit"
|
||||
"github.com/archivmail/internal/auth"
|
||||
)
|
||||
|
||||
// ── PROJ-24: TOTP 2FA Handlers ───────────────────────────────────────────
|
||||
|
||||
// handleTOTPSetupGet generates a new TOTP secret and QR code for the current user.
|
||||
// GET /api/auth/totp/setup
|
||||
func (s *Server) handleTOTPSetupGet(w http.ResponseWriter, r *http.Request) {
|
||||
sess := sessionFromCtx(r.Context())
|
||||
if sess.UserID == 0 {
|
||||
writeError(w, http.StatusUnauthorized, "not authenticated")
|
||||
return
|
||||
}
|
||||
|
||||
secret, otpauthURL, qrPNG, err := auth.GenerateSecret(sess.Username, "archivmail")
|
||||
if err != nil {
|
||||
s.logger.Error("totp setup: generate secret failed", "err", err)
|
||||
writeError(w, http.StatusInternalServerError, "failed to generate TOTP secret")
|
||||
return
|
||||
}
|
||||
|
||||
// Encrypt the secret with AES-256-GCM before storing
|
||||
encryptedSecret, err := s.authMgr.EncryptAES([]byte(secret))
|
||||
if err != nil {
|
||||
s.logger.Error("totp setup: encrypt secret failed", "err", err)
|
||||
writeError(w, http.StatusInternalServerError, "failed to encrypt TOTP secret")
|
||||
return
|
||||
}
|
||||
|
||||
// Store encrypted secret in DB (not yet activated)
|
||||
if err := s.users.SetTOTPSecret(r.Context(), sess.UserID, encryptedSecret); err != nil {
|
||||
s.logger.Error("totp setup: store secret failed", "err", err)
|
||||
writeError(w, http.StatusInternalServerError, "failed to store TOTP secret")
|
||||
return
|
||||
}
|
||||
|
||||
resp := map[string]interface{}{
|
||||
"secret": secret,
|
||||
"otpauth_url": otpauthURL,
|
||||
}
|
||||
if len(qrPNG) > 0 {
|
||||
resp["qr_code"] = base64.StdEncoding.EncodeToString(qrPNG)
|
||||
}
|
||||
|
||||
writeJSON(w, http.StatusOK, resp)
|
||||
}
|
||||
|
||||
// handleTOTPSetupPost confirms TOTP setup by verifying a code, then activates TOTP.
|
||||
// POST /api/auth/totp/setup { "code": "123456" }
|
||||
func (s *Server) handleTOTPSetupPost(w http.ResponseWriter, r *http.Request) {
|
||||
sess := sessionFromCtx(r.Context())
|
||||
if sess.UserID == 0 {
|
||||
writeError(w, http.StatusUnauthorized, "not authenticated")
|
||||
return
|
||||
}
|
||||
|
||||
var req struct {
|
||||
Code string `json:"code"`
|
||||
}
|
||||
if err := json.NewDecoder(r.Body).Decode(&req); err != nil || req.Code == "" {
|
||||
writeError(w, http.StatusBadRequest, "missing or invalid code")
|
||||
return
|
||||
}
|
||||
|
||||
// Load encrypted secret from DB
|
||||
encSecret, _, err := s.users.GetTOTPSecret(r.Context(), sess.UserID)
|
||||
if err != nil || len(encSecret) == 0 {
|
||||
writeError(w, http.StatusBadRequest, "no TOTP secret found, run setup first")
|
||||
return
|
||||
}
|
||||
|
||||
// Decrypt
|
||||
plainSecret, err := s.authMgr.DecryptAESForHandler(encSecret)
|
||||
if err != nil {
|
||||
s.logger.Error("totp setup confirm: decrypt failed", "err", err)
|
||||
writeError(w, http.StatusInternalServerError, "failed to decrypt TOTP secret")
|
||||
return
|
||||
}
|
||||
|
||||
// Validate code
|
||||
if !auth.ValidateTOTP(string(plainSecret), req.Code) {
|
||||
writeError(w, http.StatusBadRequest, "invalid TOTP code")
|
||||
return
|
||||
}
|
||||
|
||||
// Activate TOTP
|
||||
if err := s.users.EnableTOTP(r.Context(), sess.UserID); err != nil {
|
||||
s.logger.Error("totp setup confirm: enable failed", "err", err)
|
||||
writeError(w, http.StatusInternalServerError, "failed to enable TOTP")
|
||||
return
|
||||
}
|
||||
|
||||
s.audlog.Log(audit.Entry{
|
||||
EventType: audit.EventUserMgmt,
|
||||
Username: sess.Username,
|
||||
IPAddress: remoteIP(r),
|
||||
Detail: "totp_enabled",
|
||||
Success: true,
|
||||
})
|
||||
|
||||
writeJSON(w, http.StatusOK, map[string]bool{"ok": true})
|
||||
}
|
||||
|
||||
// handleTOTPDisable deactivates TOTP for the current user (requires a valid code).
|
||||
// DELETE /api/auth/totp { "code": "123456" }
|
||||
func (s *Server) handleTOTPDisable(w http.ResponseWriter, r *http.Request) {
|
||||
sess := sessionFromCtx(r.Context())
|
||||
if sess.UserID == 0 {
|
||||
writeError(w, http.StatusUnauthorized, "not authenticated")
|
||||
return
|
||||
}
|
||||
|
||||
var req struct {
|
||||
Code string `json:"code"`
|
||||
}
|
||||
if err := json.NewDecoder(r.Body).Decode(&req); err != nil || req.Code == "" {
|
||||
writeError(w, http.StatusBadRequest, "missing or invalid code")
|
||||
return
|
||||
}
|
||||
|
||||
// Load and decrypt secret
|
||||
encSecret, enabled, err := s.users.GetTOTPSecret(r.Context(), sess.UserID)
|
||||
if err != nil || !enabled || len(encSecret) == 0 {
|
||||
writeError(w, http.StatusBadRequest, "TOTP is not enabled")
|
||||
return
|
||||
}
|
||||
|
||||
plainSecret, err := s.authMgr.DecryptAESForHandler(encSecret)
|
||||
if err != nil {
|
||||
s.logger.Error("totp disable: decrypt failed", "err", err)
|
||||
writeError(w, http.StatusInternalServerError, "failed to decrypt TOTP secret")
|
||||
return
|
||||
}
|
||||
|
||||
if !auth.ValidateTOTP(string(plainSecret), req.Code) {
|
||||
writeError(w, http.StatusBadRequest, "invalid TOTP code")
|
||||
return
|
||||
}
|
||||
|
||||
if err := s.users.DisableTOTP(r.Context(), sess.UserID); err != nil {
|
||||
s.logger.Error("totp disable: failed", "err", err)
|
||||
writeError(w, http.StatusInternalServerError, "failed to disable TOTP")
|
||||
return
|
||||
}
|
||||
|
||||
s.audlog.Log(audit.Entry{
|
||||
EventType: audit.EventUserMgmt,
|
||||
Username: sess.Username,
|
||||
IPAddress: remoteIP(r),
|
||||
Detail: "totp_disabled",
|
||||
Success: true,
|
||||
})
|
||||
|
||||
writeJSON(w, http.StatusOK, map[string]bool{"ok": true})
|
||||
}
|
||||
|
||||
// handleTOTPLogin completes a TOTP-pending login by validating the code.
|
||||
// POST /api/auth/totp { "session_token": "...", "code": "123456" }
|
||||
func (s *Server) handleTOTPLogin(w http.ResponseWriter, r *http.Request) {
|
||||
var req struct {
|
||||
SessionToken string `json:"session_token"`
|
||||
Code string `json:"code"`
|
||||
}
|
||||
if err := json.NewDecoder(r.Body).Decode(&req); err != nil || req.SessionToken == "" || req.Code == "" {
|
||||
writeError(w, http.StatusBadRequest, "missing session_token or code")
|
||||
return
|
||||
}
|
||||
|
||||
token, user, err := s.authMgr.ValidateTOTPLogin(req.SessionToken, req.Code)
|
||||
if err != nil {
|
||||
s.audlog.Log(audit.Entry{
|
||||
EventType: audit.EventLogin,
|
||||
IPAddress: remoteIP(r),
|
||||
Success: false,
|
||||
Detail: "totp_login_failed: " + err.Error(),
|
||||
})
|
||||
writeError(w, http.StatusUnauthorized, "invalid TOTP code or expired session")
|
||||
return
|
||||
}
|
||||
|
||||
_ = s.users.UpdateLastLogin(user.ID)
|
||||
|
||||
s.audlog.Log(audit.Entry{
|
||||
EventType: audit.EventLogin,
|
||||
Username: user.Username,
|
||||
IPAddress: remoteIP(r),
|
||||
Success: true,
|
||||
Detail: "totp_login_completed",
|
||||
})
|
||||
|
||||
http.SetCookie(w, &http.Cookie{
|
||||
Name: sessionCookieName,
|
||||
Value: token,
|
||||
Path: "/",
|
||||
MaxAge: 8 * 3600,
|
||||
HttpOnly: true,
|
||||
SameSite: http.SameSiteStrictMode,
|
||||
})
|
||||
|
||||
writeJSON(w, http.StatusOK, map[string]interface{}{
|
||||
"user": map[string]interface{}{
|
||||
"id": user.ID,
|
||||
"username": user.Username,
|
||||
"email": user.Email,
|
||||
"role": user.Role,
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
// handleTOTPReset allows an admin to reset TOTP for a user.
|
||||
// POST /api/admin/users/{id}/totp/reset
|
||||
func (s *Server) handleTOTPReset(w http.ResponseWriter, r *http.Request) {
|
||||
id, err := strconv.ParseInt(r.PathValue("id"), 10, 64)
|
||||
if err != nil {
|
||||
writeError(w, http.StatusBadRequest, "invalid user id")
|
||||
return
|
||||
}
|
||||
|
||||
sess := sessionFromCtx(r.Context())
|
||||
|
||||
// Fetch target user
|
||||
target, err := s.users.GetByID(id)
|
||||
if err != nil {
|
||||
writeError(w, http.StatusNotFound, "user not found")
|
||||
return
|
||||
}
|
||||
|
||||
// SEC-02: Tenant isolation — domain_admin can only reset TOTP for users in their own tenant.
|
||||
if sess.TenantID != nil {
|
||||
if target.TenantID == nil || *target.TenantID != *sess.TenantID {
|
||||
writeError(w, http.StatusForbidden, "access denied")
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
// Reset TOTP
|
||||
if err := s.users.ResetTOTP(r.Context(), id, sess.Username); err != nil {
|
||||
s.logger.Error("totp reset: failed", "err", err, "target_user", id, "admin", sess.Username)
|
||||
writeError(w, http.StatusInternalServerError, "failed to reset TOTP")
|
||||
return
|
||||
}
|
||||
|
||||
s.audlog.Log(audit.Entry{
|
||||
EventType: audit.EventUserMgmt,
|
||||
Username: sess.Username,
|
||||
IPAddress: remoteIP(r),
|
||||
Detail: fmt.Sprintf("totp_reset_by_admin: TOTP reset by %s for user %s (id=%d)", sess.Username, target.Username, id),
|
||||
Success: true,
|
||||
})
|
||||
|
||||
writeJSON(w, http.StatusOK, map[string]bool{"ok": true})
|
||||
}
|
||||
@@ -53,7 +53,8 @@ type uploadJobSnapshot struct {
|
||||
func (s *Server) handleUpload(w http.ResponseWriter, r *http.Request) {
|
||||
// 512 MB max total upload
|
||||
if err := r.ParseMultipartForm(512 << 20); err != nil {
|
||||
writeError(w, http.StatusBadRequest, "multipart parse failed: "+err.Error())
|
||||
s.logger.Error("multipart parse failed", "err", err)
|
||||
writeError(w, http.StatusBadRequest, "multipart parse failed")
|
||||
return
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user