fix: security hardening for middleware, crypto, and backup handlers
- Remove CORS origin reflection (SEC-C1 CRITICAL) - Add Content-Security-Policy header (SEC-H2) - Fix rate limiter memory leak with periodic stale IP cleanup (SEC-H5) - Enforce minimum 32-char ENCRYPTION_KEY (SEC-H4) - Validate backup type against allowlist (SEC-M6) - Fix backup download path traversal with path containment check (SEC-C2 CRITICAL)
This commit is contained in:
+24
-2
@@ -6,6 +6,7 @@ import (
|
||||
"net/http"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/alexei/docker-watcher/internal/store"
|
||||
@@ -69,14 +70,35 @@ func (s *Server) downloadBackup(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
|
||||
filePath := s.backupEngine.FilePath(backup)
|
||||
if _, err := os.Stat(filePath); err != nil {
|
||||
|
||||
// Validate the resolved path stays within the backup directory to prevent path traversal.
|
||||
absPath, err := filepath.Abs(filePath)
|
||||
if err != nil {
|
||||
respondError(w, http.StatusInternalServerError, "failed to resolve backup path")
|
||||
return
|
||||
}
|
||||
absBackupDir, _ := filepath.Abs(s.backupEngine.BackupDir())
|
||||
if !strings.HasPrefix(absPath, absBackupDir+string(filepath.Separator)) {
|
||||
respondError(w, http.StatusForbidden, "access denied")
|
||||
return
|
||||
}
|
||||
|
||||
f, err := os.Open(absPath)
|
||||
if err != nil {
|
||||
respondError(w, http.StatusNotFound, "backup file not found on disk")
|
||||
return
|
||||
}
|
||||
defer f.Close()
|
||||
|
||||
stat, err := f.Stat()
|
||||
if err != nil {
|
||||
respondError(w, http.StatusInternalServerError, "failed to read backup file")
|
||||
return
|
||||
}
|
||||
|
||||
w.Header().Set("Content-Type", "application/octet-stream")
|
||||
w.Header().Set("Content-Disposition", "attachment; filename=\""+filepath.Base(backup.Filename)+"\"")
|
||||
http.ServeFile(w, r, filePath)
|
||||
http.ServeContent(w, r, filepath.Base(backup.Filename), stat.ModTime(), f)
|
||||
}
|
||||
|
||||
// deleteBackup handles DELETE /api/backups/{id}.
|
||||
|
||||
+34
-15
@@ -45,23 +45,20 @@ func securityHeaders(next http.Handler) http.Handler {
|
||||
w.Header().Set("X-Content-Type-Options", "nosniff")
|
||||
w.Header().Set("X-Frame-Options", "DENY")
|
||||
w.Header().Set("Referrer-Policy", "strict-origin-when-cross-origin")
|
||||
w.Header().Set("Content-Security-Policy", "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; connect-src 'self'; font-src 'self'")
|
||||
next.ServeHTTP(w, r)
|
||||
})
|
||||
}
|
||||
|
||||
// cors is an HTTP middleware that restricts CORS to same-origin requests.
|
||||
// The frontend is served from the same origin, so no wildcard is needed.
|
||||
// cors is an HTTP middleware that handles CORS for same-origin requests.
|
||||
// The frontend is served from the same origin, so cross-origin requests are not expected.
|
||||
func cors(next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
origin := r.Header.Get("Origin")
|
||||
if origin != "" {
|
||||
// Only allow the same origin (frontend is served from the same host).
|
||||
w.Header().Set("Access-Control-Allow-Origin", origin)
|
||||
w.Header().Set("Vary", "Origin")
|
||||
w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS")
|
||||
w.Header().Set("Access-Control-Allow-Headers", "Content-Type, Authorization")
|
||||
w.Header().Set("Access-Control-Allow-Credentials", "true")
|
||||
}
|
||||
// The frontend is served from the same origin, so cross-origin
|
||||
// requests are not expected. We do NOT reflect the Origin header
|
||||
// back, as that would allow any website to make credentialed requests.
|
||||
// If cross-origin support is needed in the future, maintain an
|
||||
// explicit allowlist of trusted origins here.
|
||||
|
||||
if r.Method == http.MethodOptions {
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
@@ -84,12 +81,16 @@ func limitBody(next http.Handler) http.Handler {
|
||||
|
||||
// rateLimiter provides per-IP rate limiting for login endpoints.
|
||||
type rateLimiter struct {
|
||||
mu sync.Mutex
|
||||
attempts map[string][]time.Time
|
||||
mu sync.Mutex
|
||||
attempts map[string][]time.Time
|
||||
lastCleanup time.Time
|
||||
}
|
||||
|
||||
func newRateLimiter() *rateLimiter {
|
||||
return &rateLimiter{attempts: make(map[string][]time.Time)}
|
||||
return &rateLimiter{
|
||||
attempts: make(map[string][]time.Time),
|
||||
lastCleanup: time.Now(),
|
||||
}
|
||||
}
|
||||
|
||||
// allow checks if the IP is allowed to make another request.
|
||||
@@ -101,7 +102,25 @@ func (rl *rateLimiter) allow(ip string) bool {
|
||||
now := time.Now()
|
||||
window := now.Add(-1 * time.Minute)
|
||||
|
||||
// Clean old entries.
|
||||
// Periodically clean all stale IPs to prevent memory leak.
|
||||
if now.Sub(rl.lastCleanup) > 5*time.Minute {
|
||||
for k, times := range rl.attempts {
|
||||
filtered := times[:0]
|
||||
for _, t := range times {
|
||||
if t.After(window) {
|
||||
filtered = append(filtered, t)
|
||||
}
|
||||
}
|
||||
if len(filtered) == 0 {
|
||||
delete(rl.attempts, k)
|
||||
} else {
|
||||
rl.attempts[k] = filtered
|
||||
}
|
||||
}
|
||||
rl.lastCleanup = now
|
||||
}
|
||||
|
||||
// Clean old entries for this IP.
|
||||
filtered := rl.attempts[ip][:0]
|
||||
for _, t := range rl.attempts[ip] {
|
||||
if t.After(window) {
|
||||
|
||||
@@ -40,6 +40,14 @@ func (e *Engine) BackupDir() string {
|
||||
// CreateBackup creates a new database backup using VACUUM INTO.
|
||||
// Returns the backup metadata record.
|
||||
func (e *Engine) CreateBackup(backupType string) (store.Backup, error) {
|
||||
// Validate backup type to prevent path traversal via filename.
|
||||
switch backupType {
|
||||
case "manual", "auto", "pre-restore":
|
||||
// valid
|
||||
default:
|
||||
return store.Backup{}, fmt.Errorf("invalid backup type: %q", backupType)
|
||||
}
|
||||
|
||||
e.mu.Lock()
|
||||
defer e.mu.Unlock()
|
||||
|
||||
|
||||
@@ -28,6 +28,9 @@ func KeyFromEnv() ([32]byte, error) {
|
||||
if raw == "" {
|
||||
return [32]byte{}, ErrNoKey
|
||||
}
|
||||
if len(raw) < 32 {
|
||||
return [32]byte{}, fmt.Errorf("ENCRYPTION_KEY must be at least 32 characters long (got %d)", len(raw))
|
||||
}
|
||||
return DeriveKey(raw), nil
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user