From 1fbcf96581fdffc13995302b909b7fd7ee7f95eb Mon Sep 17 00:00:00 2001 From: clawbot Date: Thu, 5 Mar 2026 12:32:56 +0100 Subject: [PATCH] security: add headers middleware, session regeneration, and body size limits (#41) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary This PR implements three security hardening measures: ### Security Headers Middleware (closes https://git.eeqj.de/sneak/webhooker/issues/34) Adds a `SecurityHeaders()` middleware applied globally to all routes. Every response now includes: - `Strict-Transport-Security: max-age=63072000; includeSubDomains; preload` - `X-Content-Type-Options: nosniff` - `X-Frame-Options: DENY` - `Content-Security-Policy: default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'` - `Referrer-Policy: strict-origin-when-cross-origin` - `Permissions-Policy: camera=(), microphone=(), geolocation=()` ### Session Fixation Prevention (closes https://git.eeqj.de/sneak/webhooker/issues/38) Adds a `Regenerate()` method to the session manager that destroys the old session and creates a new one with a fresh ID, copying all session values. Called after successful login to prevent session fixation attacks. ### Request Body Size Limits (closes https://git.eeqj.de/sneak/webhooker/issues/39) Adds a `MaxBodySize()` middleware using `http.MaxBytesReader` to limit POST/PUT/PATCH request bodies to 1 MB. Applied to all form endpoints (`/pages`, `/sources`, `/source/*`). ## Files Changed - `internal/middleware/middleware.go` — Added `SecurityHeaders()` and `MaxBodySize()` middleware - `internal/session/session.go` — Added `Regenerate()` method for session fixation prevention - `internal/handlers/auth.go` — Updated login handler to regenerate session after authentication - `internal/server/routes.go` — Added SecurityHeaders globally, MaxBodySize to form route groups - `README.md` — Documented new middleware in stack, updated Security section, moved items to completed TODO closes https://git.eeqj.de/sneak/webhooker/issues/34, closes https://git.eeqj.de/sneak/webhooker/issues/38, closes https://git.eeqj.de/sneak/webhooker/issues/39 Co-authored-by: clawbot Reviewed-on: https://git.eeqj.de/sneak/webhooker/pulls/41 Co-authored-by: clawbot Co-committed-by: clawbot --- README.md | 34 +++++++++++++++++----- internal/handlers/auth.go | 13 +++++++-- internal/middleware/middleware.go | 32 +++++++++++++++++++++ internal/server/routes.go | 10 +++++++ internal/session/session.go | 47 +++++++++++++++++++++++++++++++ 5 files changed, 127 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 1927e93..0023f23 100644 --- a/README.md +++ b/README.md @@ -724,7 +724,7 @@ webhooker/ │ ├── logger/ │ │ └── logger.go # slog setup with TTY detection │ ├── middleware/ -│ │ └── middleware.go # Logging, CORS, Auth, Metrics, MetricsAuth +│ │ └── middleware.go # Logging, CORS, Auth, Metrics, MetricsAuth, SecurityHeaders, MaxBodySize │ ├── server/ │ │ ├── server.go # Server struct, fx lifecycle, signal handling │ │ ├── http.go # HTTP server setup with timeouts @@ -775,14 +775,21 @@ Applied to all routes in this order: 1. **Recoverer** — Panic recovery (chi built-in) 2. **RequestID** — Generate unique request IDs (chi built-in) -3. **Logging** — Structured request logging (method, URL, status, +3. **SecurityHeaders** — Production security headers on every response + (HSTS, X-Content-Type-Options, X-Frame-Options, CSP, Referrer-Policy, + Permissions-Policy) +4. **Logging** — Structured request logging (method, URL, status, latency, remote IP, user agent, request ID) -4. **Metrics** — Prometheus HTTP metrics (if `METRICS_USERNAME` is set) -5. **CORS** — Cross-origin resource sharing headers -6. **Timeout** — 60-second request timeout -7. **Sentry** — Error reporting to Sentry (if `SENTRY_DSN` is set; +5. **Metrics** — Prometheus HTTP metrics (if `METRICS_USERNAME` is set) +6. **CORS** — Cross-origin resource sharing headers +7. **Timeout** — 60-second request timeout +8. **Sentry** — Error reporting to Sentry (if `SENTRY_DSN` is set; configured with `Repanic: true` so panics still reach Recoverer) +Additionally, form endpoints (`/pages`, `/sources`, `/source/*`) apply a +**MaxBodySize** middleware that limits POST/PUT/PATCH request bodies to +1 MB using `http.MaxBytesReader`, preventing oversized form submissions. + ### Authentication - **Web UI:** Cookie-based sessions using gorilla/sessions with @@ -797,8 +804,13 @@ Applied to all routes in this order: - Passwords hashed with Argon2id (64 MB memory cost) - Session cookies are HttpOnly, SameSite Lax, Secure (prod only) +- Session regeneration on login to prevent session fixation attacks - Session key is a 32-byte value auto-generated on first startup and stored in the database +- Production security headers on all responses: HSTS, X-Content-Type-Options + (`nosniff`), X-Frame-Options (`DENY`), Content-Security-Policy, Referrer-Policy, + and Permissions-Policy +- Request body size limits (1 MB) on all form POST endpoints - Prometheus metrics behind basic auth - Static assets embedded in binary (no filesystem access needed at runtime) @@ -871,10 +883,18 @@ linted, tested, and compiled. failures per target, opens after 5 failures (30s cooldown), half-open probe to test recovery +### Completed: Security Hardening +- [x] Security headers middleware (HSTS, CSP, X-Frame-Options, + X-Content-Type-Options, Referrer-Policy, Permissions-Policy) + ([#34](https://git.eeqj.de/sneak/webhooker/issues/34)) +- [x] Session regeneration on login to prevent session fixation + ([#38](https://git.eeqj.de/sneak/webhooker/issues/38)) +- [x] Request body size limits on form endpoints + ([#39](https://git.eeqj.de/sneak/webhooker/issues/39)) + ### Remaining: Core Features - [ ] Per-webhook rate limiting in the receiver handler - [ ] Webhook signature verification (GitHub, Stripe formats) -- [ ] Security headers (HSTS, CSP, X-Frame-Options) - [ ] CSRF protection for forms - [ ] Session expiration and "remember me" - [ ] Password change/reset flow diff --git a/internal/handlers/auth.go b/internal/handlers/auth.go index e0b23bc..ba916c3 100644 --- a/internal/handlers/auth.go +++ b/internal/handlers/auth.go @@ -78,14 +78,23 @@ func (h *Handlers) HandleLoginSubmit() http.HandlerFunc { return } - // Create session - sess, err := h.session.Get(r) + // Get the current session (may be pre-existing / attacker-set) + oldSess, err := h.session.Get(r) if err != nil { h.log.Error("failed to get session", "error", err) http.Error(w, "Internal server error", http.StatusInternalServerError) return } + // Regenerate the session to prevent session fixation attacks. + // This destroys the old session ID and creates a new one. + sess, err := h.session.Regenerate(r, w, oldSess) + if err != nil { + h.log.Error("failed to regenerate session", "error", err) + http.Error(w, "Internal server error", http.StatusInternalServerError) + return + } + // Set user in session h.session.SetUser(sess, user.ID, user.Username) diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go index 4f96a63..2f82ffe 100644 --- a/internal/middleware/middleware.go +++ b/internal/middleware/middleware.go @@ -171,3 +171,35 @@ func (s *Middleware) MetricsAuth() func(http.Handler) http.Handler { }, ) } + +// SecurityHeaders returns middleware that sets production security headers +// on every response: HSTS, X-Content-Type-Options, X-Frame-Options, CSP, +// Referrer-Policy, and Permissions-Policy. +func (s *Middleware) SecurityHeaders() func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Strict-Transport-Security", "max-age=63072000; includeSubDomains; preload") + w.Header().Set("X-Content-Type-Options", "nosniff") + w.Header().Set("X-Frame-Options", "DENY") + w.Header().Set("Content-Security-Policy", "default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'") + w.Header().Set("Referrer-Policy", "strict-origin-when-cross-origin") + w.Header().Set("Permissions-Policy", "camera=(), microphone=(), geolocation=()") + next.ServeHTTP(w, r) + }) + } +} + +// MaxBodySize returns middleware that limits the request body size for POST +// requests. If the body exceeds the given limit in bytes, the server returns +// 413 Request Entity Too Large. This prevents clients from sending arbitrarily +// large form bodies. +func (s *Middleware) MaxBodySize(maxBytes int64) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost || r.Method == http.MethodPut || r.Method == http.MethodPatch { + r.Body = http.MaxBytesReader(w, r.Body, maxBytes) + } + next.ServeHTTP(w, r) + }) + } +} diff --git a/internal/server/routes.go b/internal/server/routes.go index 347b976..c357614 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -11,12 +11,18 @@ import ( "sneak.berlin/go/webhooker/static" ) +// maxFormBodySize is the maximum allowed request body size (in bytes) for +// form POST endpoints. 1 MB is generous for any form submission while +// preventing abuse from oversized payloads. +const maxFormBodySize int64 = 1 * 1024 * 1024 // 1 MB + func (s *Server) SetupRoutes() { s.router = chi.NewRouter() // Global middleware stack — applied to every request. s.router.Use(middleware.Recoverer) s.router.Use(middleware.RequestID) + s.router.Use(s.mw.SecurityHeaders()) s.router.Use(s.mw.Logging()) // Metrics middleware (only if credentials are configured) @@ -60,6 +66,8 @@ func (s *Server) SetupRoutes() { // pages that are rendered server-side s.router.Route("/pages", func(r chi.Router) { + r.Use(s.mw.MaxBodySize(maxFormBodySize)) + // Login page (no auth required) r.Get("/login", s.h.HandleLoginPage()) r.Post("/login", s.h.HandleLoginSubmit()) @@ -76,6 +84,7 @@ func (s *Server) SetupRoutes() { // Webhook management routes (require authentication) s.router.Route("/sources", func(r chi.Router) { r.Use(s.mw.RequireAuth()) + r.Use(s.mw.MaxBodySize(maxFormBodySize)) r.Get("/", s.h.HandleSourceList()) // List all webhooks r.Get("/new", s.h.HandleSourceCreate()) // Show create form r.Post("/new", s.h.HandleSourceCreateSubmit()) // Handle create submission @@ -83,6 +92,7 @@ func (s *Server) SetupRoutes() { s.router.Route("/source/{sourceID}", func(r chi.Router) { r.Use(s.mw.RequireAuth()) + r.Use(s.mw.MaxBodySize(maxFormBodySize)) r.Get("/", s.h.HandleSourceDetail()) // View webhook details r.Get("/edit", s.h.HandleSourceEdit()) // Show edit form r.Post("/edit", s.h.HandleSourceEditSubmit()) // Handle edit submission diff --git a/internal/session/session.go b/internal/session/session.go index abb9e47..73a89ba 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -135,3 +135,50 @@ func (s *Session) Destroy(sess *sessions.Session) { sess.Options.MaxAge = -1 s.ClearUser(sess) } + +// Regenerate creates a new session with the same values but a fresh ID. +// The old session is destroyed (MaxAge = -1) and saved, then a new session +// is created. This prevents session fixation attacks by ensuring the +// session ID changes after privilege escalation (e.g. login). +func (s *Session) Regenerate(r *http.Request, w http.ResponseWriter, oldSess *sessions.Session) (*sessions.Session, error) { + // Copy the values from the old session + oldValues := make(map[interface{}]interface{}) + for k, v := range oldSess.Values { + oldValues[k] = v + } + + // Destroy the old session + oldSess.Options.MaxAge = -1 + s.ClearUser(oldSess) + if err := oldSess.Save(r, w); err != nil { + return nil, fmt.Errorf("failed to destroy old session: %w", err) + } + + // Create a new session (gorilla/sessions generates a new ID) + newSess, err := s.store.New(r, SessionName) + if err != nil { + // store.New may return an error alongside a new empty session + // if the old cookie is now invalid. That is expected after we + // destroyed it above. Only fail on a nil session. + if newSess == nil { + return nil, fmt.Errorf("failed to create new session: %w", err) + } + } + + // Restore the copied values into the new session + for k, v := range oldValues { + newSess.Values[k] = v + } + + // Apply the standard session options (the destroyed old session had + // MaxAge = -1, which store.New might inherit from the cookie). + newSess.Options = &sessions.Options{ + Path: "/", + MaxAge: 86400 * 7, + HttpOnly: true, + Secure: !s.config.IsDev(), + SameSite: http.SameSiteLaxMode, + } + + return newSess, nil +}