Compare commits
5
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
11c8a96766 | ||
|
|
22293d733f | ||
|
|
62a0beaea8 | ||
|
|
fa7f28c62b | ||
|
|
83869f221a |
@@ -24,7 +24,7 @@ $EDITOR config.yml # replace the signing_key placeholder
|
|||||||
|
|
||||||
# or build and run via Docker
|
# or build and run via Docker
|
||||||
make docker
|
make docker
|
||||||
docker run -p 8080:8080 -e PIXA_SIGNING_KEY="$(openssl rand -base64 32)" pixa:latest
|
docker run -p 8080:8080 -e PIXA_SIGNING_KEY="$(openssl rand -base64 32)" pixad:latest
|
||||||
```
|
```
|
||||||
|
|
||||||
A container is configured two ways. The signing key comes from the
|
A container is configured two ways. The signing key comes from the
|
||||||
|
|||||||
@@ -29,14 +29,6 @@ P1: implement blocked networks configuration to extend SSRF protection
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
- 2026-09-21 http.Server hardening (closes #92): added
|
|
||||||
`HTTPReadHeaderTimeout` (10s, bounds the slowloris header dribble) and
|
|
||||||
`HTTPIdleTimeout` (120s, bounds keep-alive reuse) alongside the
|
|
||||||
existing timeouts and wired them onto the server; added a `LimitBody`
|
|
||||||
middleware capping the two form POST bodies (`POST /`, `POST /generate`)
|
|
||||||
at `MaxFormBytes` (1 MiB) and returning 413, applied ahead of the CSRF
|
|
||||||
middleware so an oversized body is refused as 413 rather than being read
|
|
||||||
as a missing CSRF token (403); left `WriteTimeout` at 60s unchanged
|
|
||||||
- 2026-08-07 update golangci-lint to v2.12.2 with the canonical
|
- 2026-08-07 update golangci-lint to v2.12.2 with the canonical
|
||||||
`.golangci.yml` (v2 schema, `default: all` minus six disabled
|
`.golangci.yml` (v2 schema, `default: all` minus six disabled
|
||||||
linters, `lll` 88, tests included): bumped the pinned
|
linters, `lll` 88, tests included): bumped the pinned
|
||||||
|
|||||||
@@ -1,45 +0,0 @@
|
|||||||
package handlers
|
|
||||||
|
|
||||||
import (
|
|
||||||
"errors"
|
|
||||||
"net/http"
|
|
||||||
)
|
|
||||||
|
|
||||||
// MaxFormBytes bounds the request body accepted on the HTML form POST
|
|
||||||
// routes (POST / and POST /generate). The forms carry a handful of short
|
|
||||||
// fields, so 1 MiB is generous while making the bound explicit rather than
|
|
||||||
// resting on ParseForm's incidental 10 MB cap.
|
|
||||||
const MaxFormBytes = 1 << 20 // 1 MiB
|
|
||||||
|
|
||||||
// LimitBody returns middleware that caps the request body on POST requests
|
|
||||||
// at maxBytes and rejects an oversized body with 413 Request Entity Too
|
|
||||||
// Large.
|
|
||||||
//
|
|
||||||
// It parses the form here, before the CSRF middleware reads the token from
|
|
||||||
// it. The CSRF middleware reads the token with PostFormValue, which
|
|
||||||
// swallows a parse error, so if the body were only capped there an
|
|
||||||
// oversized body would read as a missing token and be refused as 403. By
|
|
||||||
// parsing under the cap first, an oversized body is refused as 413. A
|
|
||||||
// successful parse is cached on the request, so the CSRF check and the
|
|
||||||
// handler reuse it rather than reading the body again.
|
|
||||||
func (s *Handlers) LimitBody(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.Body = http.MaxBytesReader(w, r.Body, maxBytes)
|
|
||||||
|
|
||||||
err := r.ParseForm()
|
|
||||||
|
|
||||||
var tooLarge *http.MaxBytesError
|
|
||||||
if errors.As(err, &tooLarge) {
|
|
||||||
http.Error(w, "Request body too large",
|
|
||||||
http.StatusRequestEntityTooLarge)
|
|
||||||
|
|
||||||
return
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
next.ServeHTTP(w, r)
|
|
||||||
})
|
|
||||||
}
|
|
||||||
}
|
|
||||||
@@ -1,177 +0,0 @@
|
|||||||
package handlers
|
|
||||||
|
|
||||||
import (
|
|
||||||
"log/slog"
|
|
||||||
"net/http"
|
|
||||||
"net/url"
|
|
||||||
"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"
|
|
||||||
)
|
|
||||||
|
|
||||||
// Form field names and a throwaway source image URL for the body-limit
|
|
||||||
// tests.
|
|
||||||
const (
|
|
||||||
sourceURLField = "url"
|
|
||||||
testSourceURL = "https://example.com/a.jpg"
|
|
||||||
)
|
|
||||||
|
|
||||||
// newBodyLimitTestRouter mirrors the production wiring for the form POST
|
|
||||||
// routes (see server.SetupRoutes): LimitBody sits in front of the CSRF
|
|
||||||
// middleware, which sits in front of the handlers. maxBytes is the body
|
|
||||||
// cap under test, so a test can trip the limit with a small body.
|
|
||||||
func newBodyLimitTestRouter(
|
|
||||||
t *testing.T, maxBytes int64,
|
|
||||||
) (*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.LimitBody(maxBytes))
|
|
||||||
r.Use(h.CSRF())
|
|
||||||
r.Get("/", h.HandleRoot())
|
|
||||||
r.Post("/", h.HandleRoot())
|
|
||||||
r.Post("/generate", h.HandleGenerateURL())
|
|
||||||
})
|
|
||||||
|
|
||||||
return h, r
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestOversizedLoginPostRejectedBeforeCSRF is the core regression: an
|
|
||||||
// oversized POST / carrying an otherwise valid CSRF cookie and token must
|
|
||||||
// be rejected with 413. If the body limit ran after CSRF, the truncated
|
|
||||||
// body would read as a missing token and return 403; if it ran after the
|
|
||||||
// handler, a valid token would return 303. Getting 413 proves the limit
|
|
||||||
// fires before CSRF parses the form.
|
|
||||||
func TestOversizedLoginPostRejectedBeforeCSRF(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
_, srv := newBodyLimitTestRouter(t, 16)
|
|
||||||
|
|
||||||
cookies, token := csrfCredentials(t, srv, nil)
|
|
||||||
|
|
||||||
rec := postForm(srv, "/", cookies, url.Values{
|
|
||||||
loginKeyField: {testSigningKey},
|
|
||||||
csrfTokenField: {token},
|
|
||||||
})
|
|
||||||
|
|
||||||
if rec.Code != http.StatusRequestEntityTooLarge {
|
|
||||||
t.Errorf("oversized POST / status = %d, want %d",
|
|
||||||
rec.Code, http.StatusRequestEntityTooLarge)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestOversizedGeneratePostRejectedBeforeCSRF is the same regression for
|
|
||||||
// POST /generate, which also parses a form behind CSRF.
|
|
||||||
func TestOversizedGeneratePostRejectedBeforeCSRF(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
h, srv := newBodyLimitTestRouter(t, 16)
|
|
||||||
|
|
||||||
sessionCookie := newSessionCookie(t, h)
|
|
||||||
|
|
||||||
cookies, token := csrfCredentials(t, srv, []*http.Cookie{sessionCookie})
|
|
||||||
cookies = append(cookies, sessionCookie)
|
|
||||||
|
|
||||||
rec := postForm(srv, "/generate", cookies, url.Values{
|
|
||||||
sourceURLField: {testSourceURL},
|
|
||||||
csrfTokenField: {token},
|
|
||||||
})
|
|
||||||
|
|
||||||
if rec.Code != http.StatusRequestEntityTooLarge {
|
|
||||||
t.Errorf("oversized POST /generate status = %d, want %d",
|
|
||||||
rec.Code, http.StatusRequestEntityTooLarge)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestWithinLimitLoginPostSucceeds verifies the limit does not disturb a
|
|
||||||
// normal request: under the production cap, a valid login still parses and
|
|
||||||
// establishes a session (303). This guards against the body limit
|
|
||||||
// consuming or corrupting the form the CSRF check and handler depend on.
|
|
||||||
func TestWithinLimitLoginPostSucceeds(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
_, srv := newBodyLimitTestRouter(t, MaxFormBytes)
|
|
||||||
|
|
||||||
cookies, token := csrfCredentials(t, srv, nil)
|
|
||||||
|
|
||||||
rec := postForm(srv, "/", cookies, url.Values{
|
|
||||||
loginKeyField: {testSigningKey},
|
|
||||||
csrfTokenField: {token},
|
|
||||||
})
|
|
||||||
|
|
||||||
if rec.Code != http.StatusSeeOther {
|
|
||||||
t.Fatalf("within-limit POST / 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("within-limit valid login did not set a session cookie")
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestWithinLimitGeneratePostSucceeds is the same non-regression check for
|
|
||||||
// POST /generate.
|
|
||||||
func TestWithinLimitGeneratePostSucceeds(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
h, srv := newBodyLimitTestRouter(t, MaxFormBytes)
|
|
||||||
|
|
||||||
sessionCookie := newSessionCookie(t, h)
|
|
||||||
|
|
||||||
cookies, token := csrfCredentials(t, srv, []*http.Cookie{sessionCookie})
|
|
||||||
cookies = append(cookies, sessionCookie)
|
|
||||||
|
|
||||||
rec := postForm(srv, "/generate", cookies, url.Values{
|
|
||||||
sourceURLField: {testSourceURL},
|
|
||||||
"format": {"jpeg"},
|
|
||||||
csrfTokenField: {token},
|
|
||||||
})
|
|
||||||
|
|
||||||
if rec.Code != http.StatusOK {
|
|
||||||
t.Fatalf("within-limit POST /generate status = %d, want %d",
|
|
||||||
rec.Code, http.StatusOK)
|
|
||||||
}
|
|
||||||
|
|
||||||
if !strings.Contains(rec.Body.String(), "/v1/e/") {
|
|
||||||
t.Error("within-limit generate response did not contain a generated URL")
|
|
||||||
}
|
|
||||||
}
|
|
||||||
+5
-21
@@ -10,39 +10,23 @@ import (
|
|||||||
// HTTP server configuration constants.
|
// HTTP server configuration constants.
|
||||||
const (
|
const (
|
||||||
HTTPReadTimeout = 30 * time.Second
|
HTTPReadTimeout = 30 * time.Second
|
||||||
// HTTPReadHeaderTimeout bounds the request-header read on its own,
|
|
||||||
// short, so a slowloris client dribbling headers is dropped well
|
|
||||||
// before it ties up a connection for the whole ReadTimeout window.
|
|
||||||
HTTPReadHeaderTimeout = 10 * time.Second
|
|
||||||
HTTPWriteTimeout = 60 * time.Second
|
HTTPWriteTimeout = 60 * time.Second
|
||||||
// HTTPIdleTimeout bounds how long an idle keep-alive connection is
|
|
||||||
// held open, so idle connections cannot accumulate without limit on a
|
|
||||||
// service targeting high concurrency.
|
|
||||||
HTTPIdleTimeout = 120 * time.Second
|
|
||||||
HTTPMaxHeaderBytes = 8 << 10 // 8KB
|
HTTPMaxHeaderBytes = 8 << 10 // 8KB
|
||||||
)
|
)
|
||||||
|
|
||||||
// newHTTPServer builds the http.Server with the hardening timeouts and
|
func (s *Server) serveUntilShutdown() {
|
||||||
// limits applied. It is separate from serveUntilShutdown so the
|
listenAddr := fmt.Sprintf(":%d", s.config.Port)
|
||||||
// configuration can be asserted in a test without binding a listener.
|
s.httpServer = &http.Server{
|
||||||
func (s *Server) newHTTPServer() *http.Server {
|
Addr: listenAddr,
|
||||||
return &http.Server{
|
|
||||||
Addr: fmt.Sprintf(":%d", s.config.Port),
|
|
||||||
ReadTimeout: HTTPReadTimeout,
|
ReadTimeout: HTTPReadTimeout,
|
||||||
ReadHeaderTimeout: HTTPReadHeaderTimeout,
|
|
||||||
WriteTimeout: HTTPWriteTimeout,
|
WriteTimeout: HTTPWriteTimeout,
|
||||||
IdleTimeout: HTTPIdleTimeout,
|
|
||||||
MaxHeaderBytes: HTTPMaxHeaderBytes,
|
MaxHeaderBytes: HTTPMaxHeaderBytes,
|
||||||
Handler: s,
|
Handler: s,
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
func (s *Server) serveUntilShutdown() {
|
|
||||||
s.httpServer = s.newHTTPServer()
|
|
||||||
|
|
||||||
s.SetupRoutes()
|
s.SetupRoutes()
|
||||||
|
|
||||||
s.log.Info("http begin listen", "listenaddr", s.httpServer.Addr)
|
s.log.Info("http begin listen", "listenaddr", listenAddr)
|
||||||
|
|
||||||
err := s.httpServer.ListenAndServe()
|
err := s.httpServer.ListenAndServe()
|
||||||
if err != nil && !errors.Is(err, http.ErrServerClosed) {
|
if err != nil && !errors.Is(err, http.ErrServerClosed) {
|
||||||
|
|||||||
@@ -1,65 +0,0 @@
|
|||||||
package server
|
|
||||||
|
|
||||||
import (
|
|
||||||
"testing"
|
|
||||||
"time"
|
|
||||||
|
|
||||||
"sneak.berlin/go/pixa/internal/config"
|
|
||||||
)
|
|
||||||
|
|
||||||
// TestNewHTTPServerTimeouts verifies that the constructed http.Server
|
|
||||||
// carries every hardening timeout wired onto it, including the slowloris
|
|
||||||
// defense (ReadHeaderTimeout) and the keep-alive bound (IdleTimeout). This
|
|
||||||
// guards against a field being defined but never set on the server, so
|
|
||||||
// each assertion compares the server field to its constant.
|
|
||||||
func TestNewHTTPServerTimeouts(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
s := &Server{config: &config.Config{Port: 8080}}
|
|
||||||
|
|
||||||
srv := s.newHTTPServer()
|
|
||||||
|
|
||||||
fields := []struct {
|
|
||||||
name string
|
|
||||||
got time.Duration
|
|
||||||
want time.Duration
|
|
||||||
}{
|
|
||||||
{"ReadTimeout", srv.ReadTimeout, HTTPReadTimeout},
|
|
||||||
{"ReadHeaderTimeout", srv.ReadHeaderTimeout, HTTPReadHeaderTimeout},
|
|
||||||
{"WriteTimeout", srv.WriteTimeout, HTTPWriteTimeout},
|
|
||||||
{"IdleTimeout", srv.IdleTimeout, HTTPIdleTimeout},
|
|
||||||
}
|
|
||||||
|
|
||||||
for _, f := range fields {
|
|
||||||
if f.got != f.want {
|
|
||||||
t.Errorf("%s = %v, want %v", f.name, f.got, f.want)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
if srv.MaxHeaderBytes != HTTPMaxHeaderBytes {
|
|
||||||
t.Errorf("MaxHeaderBytes = %d, want %d",
|
|
||||||
srv.MaxHeaderBytes, HTTPMaxHeaderBytes)
|
|
||||||
}
|
|
||||||
|
|
||||||
if srv.Handler != s {
|
|
||||||
t.Error("Handler is not the server")
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestHardeningTimeoutValues pins the intent behind the two new timeouts
|
|
||||||
// without hard-coding brittle exact durations: the header-read phase is
|
|
||||||
// bounded strictly shorter than the whole-request read (the slowloris
|
|
||||||
// dribble), and idle keep-alive connections are bounded rather than held
|
|
||||||
// open forever.
|
|
||||||
func TestHardeningTimeoutValues(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
if HTTPReadHeaderTimeout <= 0 || HTTPReadHeaderTimeout > HTTPReadTimeout {
|
|
||||||
t.Errorf("ReadHeaderTimeout = %v, want positive and <= ReadTimeout %v",
|
|
||||||
HTTPReadHeaderTimeout, HTTPReadTimeout)
|
|
||||||
}
|
|
||||||
|
|
||||||
if HTTPIdleTimeout <= 0 {
|
|
||||||
t.Errorf("IdleTimeout = %v, want positive bound", HTTPIdleTimeout)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
@@ -8,7 +8,6 @@ import (
|
|||||||
"github.com/go-chi/chi/v5/middleware"
|
"github.com/go-chi/chi/v5/middleware"
|
||||||
"github.com/prometheus/client_golang/prometheus/promhttp"
|
"github.com/prometheus/client_golang/prometheus/promhttp"
|
||||||
|
|
||||||
"sneak.berlin/go/pixa/internal/handlers"
|
|
||||||
"sneak.berlin/go/pixa/internal/static"
|
"sneak.berlin/go/pixa/internal/static"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -47,10 +46,8 @@ func (s *Server) SetupRoutes() {
|
|||||||
|
|
||||||
// Login/generator UI. The form routes carry CSRF protection; the
|
// Login/generator UI. The form routes carry CSRF protection; the
|
||||||
// token cookie is independent of the session cookie, so it also
|
// token cookie is independent of the session cookie, so it also
|
||||||
// covers the login POST, where no session exists yet. LimitBody caps
|
// covers the login POST, where no session exists yet.
|
||||||
// the POST body ahead of CSRF, which reads its token from that body.
|
|
||||||
s.router.Group(func(r chi.Router) {
|
s.router.Group(func(r chi.Router) {
|
||||||
r.Use(s.h.LimitBody(handlers.MaxFormBytes))
|
|
||||||
r.Use(s.h.CSRF())
|
r.Use(s.h.CSRF())
|
||||||
r.Get("/", s.h.HandleRoot())
|
r.Get("/", s.h.HandleRoot())
|
||||||
r.Post("/", s.h.HandleRoot())
|
r.Post("/", s.h.HandleRoot())
|
||||||
|
|||||||
Reference in New Issue
Block a user