diff --git a/go.mod b/go.mod index ce9b477..f3648e0 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/getsentry/sentry-go v0.40.0 github.com/go-chi/chi/v5 v5.2.3 github.com/go-chi/cors v1.2.2 + github.com/gorilla/csrf v1.7.3 github.com/gorilla/securecookie v1.1.2 github.com/prometheus/client_golang v1.23.2 github.com/slok/go-http-metrics v0.13.0 diff --git a/go.sum b/go.sum index 002a2b4..af3e229 100644 --- a/go.sum +++ b/go.sum @@ -175,6 +175,8 @@ github.com/googleapis/enterprise-certificate-proxy v0.3.6 h1:GW/XbdyBFQ8Qe+YAmFU github.com/googleapis/enterprise-certificate-proxy v0.3.6/go.mod h1:MkHOF77EYAE7qfSuSS9PU6g4Nt4e11cnsDUowfwewLA= github.com/googleapis/gax-go/v2 v2.14.2 h1:eBLnkZ9635krYIPD+ag1USrOAI0Nr0QYF3+/3GqO0k0= github.com/googleapis/gax-go/v2 v2.14.2/go.mod h1:ON64QhlJkhVtSqp4v1uaK92VyZ2gmvDQsweuyLV+8+w= +github.com/gorilla/csrf v1.7.3 h1:BHWt6FTLZAb2HtWT5KDBf6qgpZzvtbp9QWDRKZMXJC0= +github.com/gorilla/csrf v1.7.3/go.mod h1:F1Fj3KG23WYHE6gozCmBAezKookxbIvUJT+121wTuLk= github.com/gorilla/securecookie v1.1.2 h1:YCIWL56dvtr73r6715mJs5ZvhtnY73hBvEF8kXD8ePA= github.com/gorilla/securecookie v1.1.2/go.mod h1:NfCASbcHqRSY+3a8tlWJwsQap2VX5pwzwo4h3eOamfo= github.com/grpc-ecosystem/grpc-gateway/v2 v2.26.3 h1:5ZPtiqj0JL5oKWmcsq4VMaAW5ukBEgSGXEN89zeH1Jo= diff --git a/internal/handlers/auth.go b/internal/handlers/auth.go index 977756a..5e74e1f 100644 --- a/internal/handlers/auth.go +++ b/internal/handlers/auth.go @@ -2,6 +2,7 @@ package handlers import ( "crypto/subtle" + "html/template" "net/http" "net/url" "strconv" @@ -23,13 +24,13 @@ func (s *Handlers) HandleRoot() http.HandlerFunc { // Check if authenticated if s.sessMgr.IsAuthenticated(r) { - s.renderGenerator(w, nil) + s.renderGenerator(w, r, nil) return } // Show login page - s.renderLogin(w, "") + s.renderLogin(w, r, "") } } @@ -37,7 +38,7 @@ func (s *Handlers) HandleRoot() http.HandlerFunc { func (s *Handlers) handleLoginPost(w http.ResponseWriter, r *http.Request) { err := r.ParseForm() if err != nil { - s.renderLogin(w, "Invalid form data") + s.renderLogin(w, r, "Invalid form data") return } @@ -47,7 +48,7 @@ func (s *Handlers) handleLoginPost(w http.ResponseWriter, r *http.Request) { // Constant-time comparison to prevent timing attacks if subtle.ConstantTimeCompare([]byte(submittedKey), []byte(s.config.SigningKey)) != 1 { s.log.Warn("failed login attempt", "remote_addr", r.RemoteAddr) - s.renderLogin(w, "Invalid signing key") + s.renderLogin(w, r, "Invalid signing key") return } @@ -56,7 +57,7 @@ func (s *Handlers) handleLoginPost(w http.ResponseWriter, r *http.Request) { err = s.sessMgr.CreateSession(w) if err != nil { s.log.Error("failed to create session", "error", err) - s.renderLogin(w, "Failed to create session") + s.renderLogin(w, r, "Failed to create session") return } @@ -87,7 +88,7 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc { err := r.ParseForm() if err != nil { - s.renderGenerator(w, &generatorData{Error: "Invalid form data"}) + s.renderGenerator(w, r, &generatorData{Error: "Invalid form data"}) return } @@ -97,7 +98,7 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc { // Validate source URL parsed, err := url.Parse(sourceURL) if err != nil || parsed.Host == "" { - s.renderGeneratorWithForm(w, "Invalid source URL", r.Form) + s.renderGeneratorWithForm(w, r, "Invalid source URL", r.Form) return } @@ -108,7 +109,7 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc { token, err := s.encGen.Generate(payload) if err != nil { s.log.Error("failed to generate encrypted URL", "error", err) - s.renderGeneratorWithForm(w, "Failed to generate URL", r.Form) + s.renderGeneratorWithForm(w, r, "Failed to generate URL", r.Form) return } @@ -121,7 +122,7 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc { expiresAtStr = expiresAt.Format(time.RFC3339) } - s.renderGenerator(w, &generatorData{ + s.renderGenerator(w, r, &generatorData{ GeneratedURL: generatedURL, ExpiresAt: expiresAtStr, FormURL: sourceURL, @@ -186,15 +187,20 @@ type generatorData struct { FormQuality string FormFit string FormTTL string + CSRFField template.HTML } -func (s *Handlers) renderLogin(w http.ResponseWriter, errorMsg string) { +func (s *Handlers) renderLogin( + w http.ResponseWriter, r *http.Request, errorMsg string, +) { w.Header().Set("Content-Type", "text/html; charset=utf-8") data := struct { - Error string + Error string + CSRFField template.HTML }{ - Error: errorMsg, + Error: errorMsg, + CSRFField: csrfField(r), } err := templates.Render(w, "login.html", data) @@ -204,13 +210,17 @@ func (s *Handlers) renderLogin(w http.ResponseWriter, errorMsg string) { } } -func (s *Handlers) renderGenerator(w http.ResponseWriter, data *generatorData) { +func (s *Handlers) renderGenerator( + w http.ResponseWriter, r *http.Request, data *generatorData, +) { w.Header().Set("Content-Type", "text/html; charset=utf-8") if data == nil { data = &generatorData{} } + data.CSRFField = csrfField(r) + err := templates.Render(w, "generator.html", data) if err != nil { s.log.Error("failed to render generator template", "error", err) @@ -219,9 +229,9 @@ func (s *Handlers) renderGenerator(w http.ResponseWriter, data *generatorData) { } func (s *Handlers) renderGeneratorWithForm( - w http.ResponseWriter, errorMsg string, form url.Values, + w http.ResponseWriter, r *http.Request, errorMsg string, form url.Values, ) { - s.renderGenerator(w, &generatorData{ + s.renderGenerator(w, r, &generatorData{ Error: errorMsg, FormURL: form.Get("url"), FormWidth: form.Get("width"), diff --git a/internal/handlers/auth_csrf_internal_test.go b/internal/handlers/auth_csrf_internal_test.go new file mode 100644 index 0000000..161253b --- /dev/null +++ b/internal/handlers/auth_csrf_internal_test.go @@ -0,0 +1,273 @@ +package handlers + +import ( + "context" + "log/slog" + "net/http" + "net/http/httptest" + "net/url" + "regexp" + "strings" + "testing" + + "github.com/go-chi/chi/v5" + + "sneak.berlin/go/pixa/internal/config" + "sneak.berlin/go/pixa/internal/encurl" + "sneak.berlin/go/pixa/internal/session" +) + +// testSigningKey is a throwaway signing key for the CSRF flow tests. It +// seeds the session manager, the encrypted-URL generator, and the CSRF +// token key, exactly as the real signing key does in production. +const testSigningKey = "test-signing-key-0123456789abcdef" + +// Form field names used in the CSRF flow tests. +const ( + loginKeyField = "key" + // gorilla/csrf's default form field name, not a credential. + csrfTokenField = "gorilla.csrf.Token" //nolint:gosec // G101 false positive +) + +// csrfFieldPattern extracts the token rendered by csrf.TemplateField into +// the form. The field name is gorilla/csrf's default. +var csrfFieldPattern = regexp.MustCompile( + `name="gorilla\.csrf\.Token" value="([^"]+)"`) + +// newCSRFTestRouter builds a router that mirrors the production wiring for +// the CSRF-protected UI routes (see server.SetupRoutes): the login and +// generator forms and their POST targets sit behind the real CSRF +// middleware. Requests are marked plaintext (Debug: true) so the flow runs +// over httptest's http transport without an https Referer. +func newCSRFTestRouter(t *testing.T) (*Handlers, http.Handler) { + t.Helper() + + cfg := &config.Config{SigningKey: testSigningKey, Debug: true} + + sessMgr, err := session.NewManager(testSigningKey) + if err != nil { + t.Fatalf("session.NewManager() error = %v", err) + } + + encGen, err := encurl.NewGenerator(testSigningKey) + if err != nil { + t.Fatalf("encurl.NewGenerator() error = %v", err) + } + + protect, err := newCSRFProtect(testSigningKey, cfg.Debug) + if err != nil { + t.Fatalf("newCSRFProtect() error = %v", err) + } + + h := &Handlers{ + log: slog.New(slog.DiscardHandler), + config: cfg, + sessMgr: sessMgr, + encGen: encGen, + csrfProtect: protect, + } + + r := chi.NewRouter() + r.Group(func(r chi.Router) { + r.Use(h.CSRF()) + r.Get("/", h.HandleRoot()) + r.Post("/", h.HandleRoot()) + r.Post("/generate", h.HandleGenerateURL()) + }) + + return h, r +} + +// csrfCredentials performs a GET that renders a form and returns the CSRF +// cookies the middleware set and the token embedded in the form. Passing +// the authenticated session cookie renders the generator form instead of +// the login form. +func csrfCredentials( + t *testing.T, srv http.Handler, reqCookies []*http.Cookie, +) ([]*http.Cookie, string) { + t.Helper() + + req := httptest.NewRequestWithContext( + context.Background(), http.MethodGet, "/", nil) + for _, c := range reqCookies { + req.AddCookie(c) + } + + rec := httptest.NewRecorder() + srv.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("GET / status = %d, want %d", rec.Code, http.StatusOK) + } + + match := csrfFieldPattern.FindStringSubmatch(rec.Body.String()) + if match == nil { + t.Fatalf("no CSRF token field found in rendered form") + } + + return rec.Result().Cookies(), match[1] +} + +// postForm submits form values with the given cookies and returns the +// recorder. +func postForm( + srv http.Handler, path string, + cookies []*http.Cookie, form url.Values, +) *httptest.ResponseRecorder { + req := httptest.NewRequestWithContext( + context.Background(), http.MethodPost, path, + strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + for _, c := range cookies { + req.AddCookie(c) + } + + rec := httptest.NewRecorder() + srv.ServeHTTP(rec, req) + + return rec +} + +// TestLoginPostRejectedWithoutToken verifies that POST / with no CSRF token +// is rejected. This is login CSRF: no session cookie exists yet, so the +// protection must rest on a token bound to a pre-session cookie. +func TestLoginPostRejectedWithoutToken(t *testing.T) { + t.Parallel() + + _, srv := newCSRFTestRouter(t) + + rec := postForm(srv, "/", nil, url.Values{loginKeyField: {testSigningKey}}) + + if rec.Code != http.StatusForbidden { + t.Errorf("POST / without token status = %d, want %d", + rec.Code, http.StatusForbidden) + } +} + +// TestLoginPostRejectedWithForeignToken verifies that a token that does not +// match the request's CSRF cookie is rejected: a token minted for one +// cookie cannot authorize a request carrying a different cookie. +func TestLoginPostRejectedWithForeignToken(t *testing.T) { + t.Parallel() + + _, srv := newCSRFTestRouter(t) + + cookiesA, _ := csrfCredentials(t, srv, nil) + _, tokenB := csrfCredentials(t, srv, nil) + + rec := postForm(srv, "/", cookiesA, url.Values{ + loginKeyField: {testSigningKey}, + csrfTokenField: {tokenB}, + }) + + if rec.Code != http.StatusForbidden { + t.Errorf("POST / with foreign token status = %d, want %d", + rec.Code, http.StatusForbidden) + } +} + +// TestLoginPostAcceptedWithValidToken verifies that POST / with a matching +// cookie and token succeeds: the login is processed and a session is +// established (303 redirect). +func TestLoginPostAcceptedWithValidToken(t *testing.T) { + t.Parallel() + + _, srv := newCSRFTestRouter(t) + + cookies, token := csrfCredentials(t, srv, nil) + + rec := postForm(srv, "/", cookies, url.Values{ + loginKeyField: {testSigningKey}, + csrfTokenField: {token}, + }) + + if rec.Code != http.StatusSeeOther { + t.Fatalf("POST / with valid token status = %d, want %d", + rec.Code, http.StatusSeeOther) + } + + var authed bool + + for _, c := range rec.Result().Cookies() { + if c.Name == session.CookieName && c.Value != "" { + authed = true + } + } + + if !authed { + t.Error("valid login did not set a session cookie") + } +} + +// TestGeneratePostRejectedWithoutToken verifies that POST /generate is +// rejected without a CSRF token even when the request carries a valid +// authenticated session. The session cookie is not sufficient; the policy +// requires a CSRF token on this cookie-authenticated form. +func TestGeneratePostRejectedWithoutToken(t *testing.T) { + t.Parallel() + + h, srv := newCSRFTestRouter(t) + + sessionCookie := newSessionCookie(t, h) + + rec := postForm(srv, "/generate", + []*http.Cookie{sessionCookie}, + url.Values{"url": {"https://example.com/a.jpg"}}) + + if rec.Code != http.StatusForbidden { + t.Errorf("POST /generate without token status = %d, want %d", + rec.Code, http.StatusForbidden) + } +} + +// TestGeneratePostAcceptedWithValidToken verifies that POST /generate +// succeeds with a valid session and a matching CSRF cookie and token. +func TestGeneratePostAcceptedWithValidToken(t *testing.T) { + t.Parallel() + + h, srv := newCSRFTestRouter(t) + + sessionCookie := newSessionCookie(t, h) + + cookies, token := csrfCredentials(t, srv, []*http.Cookie{sessionCookie}) + cookies = append(cookies, sessionCookie) + + rec := postForm(srv, "/generate", cookies, url.Values{ + "url": {"https://example.com/a.jpg"}, + "format": {"jpeg"}, + csrfTokenField: {token}, + }) + + if rec.Code != http.StatusOK { + t.Fatalf("POST /generate with valid token status = %d, want %d", + rec.Code, http.StatusOK) + } + + if !strings.Contains(rec.Body.String(), "/v1/e/") { + t.Error("generator response did not contain a generated URL") + } +} + +// newSessionCookie creates an authenticated session cookie via the +// handler's session manager. +func newSessionCookie(t *testing.T, h *Handlers) *http.Cookie { + t.Helper() + + rec := httptest.NewRecorder() + + err := h.sessMgr.CreateSession(rec) + if err != nil { + t.Fatalf("CreateSession() error = %v", err) + } + + for _, c := range rec.Result().Cookies() { + if c.Name == session.CookieName { + return c + } + } + + t.Fatalf("session manager did not set a %q cookie", session.CookieName) + + return nil +} diff --git a/internal/handlers/csrf.go b/internal/handlers/csrf.go new file mode 100644 index 0000000..166abdc --- /dev/null +++ b/internal/handlers/csrf.go @@ -0,0 +1,65 @@ +package handlers + +import ( + "html/template" + "net/http" + + "github.com/gorilla/csrf" + + "sneak.berlin/go/pixa/internal/seal" +) + +// csrfKeySalt provides domain separation for the CSRF authentication key, +// derived from the signing key so tokens survive restarts without extra +// configuration and never reuse the session or encrypted-URL key material. +const csrfKeySalt = "pixa-csrf-v1" + +// newCSRFProtect builds the CSRF-protection middleware for the +// state-mutating HTML form routes. The token lives in its own cookie, +// independent of the session cookie, so it also protects the login POST +// where no session exists yet (login CSRF). +// +// When plaintext is true (local HTTP development), requests are marked +// plaintext so the library neither demands an https Referer nor sets a +// Secure cookie the browser would withhold over http. In production the +// service runs behind a TLS-terminating proxy, so plaintext is false and +// the library enforces its https Referer origin check. +func newCSRFProtect( + signingKey string, plaintext bool, +) (func(http.Handler) http.Handler, error) { + key, err := seal.DeriveKey([]byte(signingKey), csrfKeySalt) + if err != nil { + return nil, err + } + + protect := csrf.Protect( + key[:], + csrf.Path("/"), + csrf.Secure(!plaintext), + csrf.SameSite(csrf.SameSiteStrictMode), + ) + + if !plaintext { + return protect, nil + } + + return func(next http.Handler) http.Handler { + protected := protect(next) + + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + protected.ServeHTTP(w, csrf.PlaintextHTTPRequest(r)) + }) + }, nil +} + +// CSRF returns the CSRF-protection middleware for the login and generator +// form routes. +func (s *Handlers) CSRF() func(http.Handler) http.Handler { + return s.csrfProtect +} + +// csrfField returns the hidden form input carrying the CSRF token for the +// given request, to be embedded verbatim in a rendered form. +func csrfField(r *http.Request) template.HTML { + return csrf.TemplateField(r) +} diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index 0398357..5323167 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -31,23 +31,30 @@ type Params struct { // Handlers provides HTTP request handlers. type Handlers struct { - log *slog.Logger - hc *healthcheck.Healthcheck - db *database.Database - config *config.Config - imgSvc *imgcache.Service - imgCache *imgcache.Cache - sessMgr *session.Manager - encGen *encurl.Generator + log *slog.Logger + hc *healthcheck.Healthcheck + db *database.Database + config *config.Config + imgSvc *imgcache.Service + imgCache *imgcache.Cache + sessMgr *session.Manager + encGen *encurl.Generator + csrfProtect func(http.Handler) http.Handler } // New creates a new Handlers instance. func New(lc fx.Lifecycle, params Params) (*Handlers, error) { + csrfProtect, err := newCSRFProtect(params.Config.SigningKey, params.Config.Debug) + if err != nil { + return nil, err + } + s := &Handlers{ - log: params.Logger.Get(), - hc: params.Healthcheck, - db: params.Database, - config: params.Config, + log: params.Logger.Get(), + hc: params.Healthcheck, + db: params.Database, + config: params.Config, + csrfProtect: csrfProtect, } lc.Append(fx.Hook{ diff --git a/internal/server/routes.go b/internal/server/routes.go index 4736dbc..1fe054e 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -44,11 +44,17 @@ func (s *Server) SetupRoutes() { // Static files (Tailwind CSS, etc.) s.router.Handle("/static/*", http.StripPrefix("/static/", static.Handler())) - // Login/generator UI - s.router.Get("/", s.h.HandleRoot()) - s.router.Post("/", s.h.HandleRoot()) + // Login/generator UI. The form routes carry CSRF protection; the + // token cookie is independent of the session cookie, so it also + // covers the login POST, where no session exists yet. + s.router.Group(func(r chi.Router) { + r.Use(s.h.CSRF()) + r.Get("/", s.h.HandleRoot()) + r.Post("/", s.h.HandleRoot()) + r.Post("/generate", s.h.HandleGenerateURL()) + }) + s.router.Get("/logout", s.h.HandleLogout()) - s.router.Post("/generate", s.h.HandleGenerateURL()) // Main image proxy route // /v1/image///x. diff --git a/internal/templates/generator.html b/internal/templates/generator.html index ede9951..d82424d 100644 --- a/internal/templates/generator.html +++ b/internal/templates/generator.html @@ -47,6 +47,7 @@ {{end}}
+ {{ .CSRFField }}