Compare commits
4
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
a663669286 | ||
|
|
6d380fbf98 | ||
|
|
1798cba96c | ||
|
|
37d49ade11 |
@@ -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)" pixad:latest
|
docker run -p 8080:8080 -e PIXA_SIGNING_KEY="$(openssl rand -base64 32)" pixa: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,6 +29,25 @@ P1: implement blocked networks configuration to extend SSRF protection
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-09-21 validate dimensions and fit mode on the encrypted-URL
|
||||||
|
route and the token generator (closes #62): added a shared
|
||||||
|
`ValidateImageRequest` in `internal/imgcache` enforcing the
|
||||||
|
`MaxDimension` bound and `ValidateFitMode`, applied by both the
|
||||||
|
`/v1/image/` and `/v1/e/` routes, so an over-limit size or an unknown
|
||||||
|
fit mode is a 400 rather than an out-of-memory or a 500 from the
|
||||||
|
processor; the URL generator now checks every numeric form field and
|
||||||
|
rejects a non-numeric or out-of-range `width`, `height`, `quality`,
|
||||||
|
or `ttl` with a 400 naming the field instead of coercing it to `0`,
|
||||||
|
and `width`/`height` are bounds-checked so an unusable token cannot be
|
||||||
|
minted
|
||||||
|
- 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
|
||||||
|
|||||||
+98
-17
@@ -2,6 +2,8 @@ package handlers
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"crypto/subtle"
|
"crypto/subtle"
|
||||||
|
"errors"
|
||||||
|
"fmt"
|
||||||
"html/template"
|
"html/template"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/url"
|
"net/url"
|
||||||
@@ -13,6 +15,11 @@ import (
|
|||||||
"sneak.berlin/go/pixa/internal/templates"
|
"sneak.berlin/go/pixa/internal/templates"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// errInvalidFormField reports a generator form field whose value is
|
||||||
|
// non-numeric or out of range. The offending field name is wrapped in so the
|
||||||
|
// response can name it.
|
||||||
|
var errInvalidFormField = errors.New("invalid")
|
||||||
|
|
||||||
// HandleRoot serves the login page or generator page based on authentication state.
|
// HandleRoot serves the login page or generator page based on authentication state.
|
||||||
func (s *Handlers) HandleRoot() http.HandlerFunc {
|
func (s *Handlers) HandleRoot() http.HandlerFunc {
|
||||||
return func(w http.ResponseWriter, r *http.Request) {
|
return func(w http.ResponseWriter, r *http.Request) {
|
||||||
@@ -98,18 +105,26 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc {
|
|||||||
// Validate source URL
|
// Validate source URL
|
||||||
parsed, err := url.Parse(sourceURL)
|
parsed, err := url.Parse(sourceURL)
|
||||||
if err != nil || parsed.Host == "" {
|
if err != nil || parsed.Host == "" {
|
||||||
s.renderGeneratorWithForm(w, r, "Invalid source URL", r.Form)
|
s.renderGeneratorWithForm(w, r, "Invalid source URL", r.Form,
|
||||||
|
http.StatusBadRequest)
|
||||||
|
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
payload, expiresAt, ttl := buildGeneratePayload(parsed, r.Form)
|
payload, expiresAt, ttl, err := buildGeneratePayload(parsed, r.Form)
|
||||||
|
if err != nil {
|
||||||
|
s.renderGeneratorWithForm(w, r, err.Error(), r.Form,
|
||||||
|
http.StatusBadRequest)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
// Generate encrypted token
|
// Generate encrypted token
|
||||||
token, err := s.encGen.Generate(payload)
|
token, err := s.encGen.Generate(payload)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
s.log.Error("failed to generate encrypted URL", "error", err)
|
s.log.Error("failed to generate encrypted URL", "error", err)
|
||||||
s.renderGeneratorWithForm(w, r, "Failed to generate URL", r.Form)
|
s.renderGeneratorWithForm(w, r, "Failed to generate URL", r.Form,
|
||||||
|
http.StatusInternalServerError)
|
||||||
|
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -137,17 +152,38 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// buildGeneratePayload parses the numeric form fields and assembles the
|
// buildGeneratePayload parses the numeric form fields and assembles the
|
||||||
// encrypted URL payload. ttl=0 means never expires (ExpiresAt stays 0).
|
// encrypted URL payload. ttl=0 means never expires (ExpiresAt stays 0). A
|
||||||
|
// non-numeric or out-of-range field, or an unrecognized fit mode, is a client
|
||||||
|
// error naming the offending field, so an unusable token is never minted.
|
||||||
func buildGeneratePayload(
|
func buildGeneratePayload(
|
||||||
parsed *url.URL, form url.Values,
|
parsed *url.URL, form url.Values,
|
||||||
) (*encurl.Payload, time.Time, int) {
|
) (*encurl.Payload, time.Time, int, error) {
|
||||||
width, _ := strconv.Atoi(form.Get("width"))
|
width, err := parseFormDimension(form, "width")
|
||||||
height, _ := strconv.Atoi(form.Get("height"))
|
if err != nil {
|
||||||
quality, _ := strconv.Atoi(form.Get("quality"))
|
return nil, time.Time{}, 0, err
|
||||||
ttl, _ := strconv.Atoi(form.Get("ttl"))
|
}
|
||||||
|
|
||||||
if quality <= 0 {
|
height, err := parseFormDimension(form, "height")
|
||||||
quality = 85
|
if err != nil {
|
||||||
|
return nil, time.Time{}, 0, err
|
||||||
|
}
|
||||||
|
|
||||||
|
quality, err := parseFormCount(form, "quality", encurl.DefaultQuality)
|
||||||
|
if err != nil {
|
||||||
|
return nil, time.Time{}, 0, err
|
||||||
|
}
|
||||||
|
|
||||||
|
ttl, err := parseFormCount(form, "ttl", 0)
|
||||||
|
if err != nil {
|
||||||
|
return nil, time.Time{}, 0, err
|
||||||
|
}
|
||||||
|
|
||||||
|
fitMode := imgcache.FitMode(form.Get("fit"))
|
||||||
|
|
||||||
|
err = imgcache.ValidateFitMode(fitMode)
|
||||||
|
if err != nil {
|
||||||
|
return nil, time.Time{}, 0,
|
||||||
|
fmt.Errorf("%w: %s", imgcache.ErrInvalidFitMode, form.Get("fit"))
|
||||||
}
|
}
|
||||||
|
|
||||||
var (
|
var (
|
||||||
@@ -168,11 +204,45 @@ func buildGeneratePayload(
|
|||||||
Height: height,
|
Height: height,
|
||||||
Format: imgcache.ImageFormat(form.Get("format")),
|
Format: imgcache.ImageFormat(form.Get("format")),
|
||||||
Quality: quality,
|
Quality: quality,
|
||||||
FitMode: imgcache.FitMode(form.Get("fit")),
|
FitMode: fitMode,
|
||||||
ExpiresAt: expiresAtUnix,
|
ExpiresAt: expiresAtUnix,
|
||||||
}
|
}
|
||||||
|
|
||||||
return payload, expiresAt, ttl
|
return payload, expiresAt, ttl, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// parseFormDimension reads an optional width or height form field. An empty
|
||||||
|
// value means "original size" (0). A non-numeric, negative, or over-limit
|
||||||
|
// value is rejected with an error naming the field.
|
||||||
|
func parseFormDimension(form url.Values, field string) (int, error) {
|
||||||
|
raw := form.Get(field)
|
||||||
|
if raw == "" {
|
||||||
|
return 0, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
value, err := strconv.Atoi(raw)
|
||||||
|
if err != nil || value < 0 || value > imgcache.MaxDimension {
|
||||||
|
return 0, fmt.Errorf("%w %s", errInvalidFormField, field)
|
||||||
|
}
|
||||||
|
|
||||||
|
return value, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// parseFormCount reads an optional non-negative integer form field, returning
|
||||||
|
// def when the field is empty and an error naming the field when the value is
|
||||||
|
// non-numeric or negative.
|
||||||
|
func parseFormCount(form url.Values, field string, def int) (int, error) {
|
||||||
|
raw := form.Get(field)
|
||||||
|
if raw == "" {
|
||||||
|
return def, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
value, err := strconv.Atoi(raw)
|
||||||
|
if err != nil || value < 0 {
|
||||||
|
return 0, fmt.Errorf("%w %s", errInvalidFormField, field)
|
||||||
|
}
|
||||||
|
|
||||||
|
return value, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// generatorData holds template data for the generator page.
|
// generatorData holds template data for the generator page.
|
||||||
@@ -212,6 +282,15 @@ func (s *Handlers) renderLogin(
|
|||||||
|
|
||||||
func (s *Handlers) renderGenerator(
|
func (s *Handlers) renderGenerator(
|
||||||
w http.ResponseWriter, r *http.Request, data *generatorData,
|
w http.ResponseWriter, r *http.Request, data *generatorData,
|
||||||
|
) {
|
||||||
|
s.renderGeneratorStatus(w, r, data, http.StatusOK)
|
||||||
|
}
|
||||||
|
|
||||||
|
// renderGeneratorStatus renders the generator page with an explicit HTTP
|
||||||
|
// status. The status is written before the body so both it and the
|
||||||
|
// Content-Type header take effect; a rejected form uses 400.
|
||||||
|
func (s *Handlers) renderGeneratorStatus(
|
||||||
|
w http.ResponseWriter, r *http.Request, data *generatorData, status int,
|
||||||
) {
|
) {
|
||||||
w.Header().Set("Content-Type", "text/html; charset=utf-8")
|
w.Header().Set("Content-Type", "text/html; charset=utf-8")
|
||||||
|
|
||||||
@@ -221,17 +300,19 @@ func (s *Handlers) renderGenerator(
|
|||||||
|
|
||||||
data.CSRFField = csrfField(r)
|
data.CSRFField = csrfField(r)
|
||||||
|
|
||||||
|
w.WriteHeader(status)
|
||||||
|
|
||||||
err := templates.Render(w, "generator.html", data)
|
err := templates.Render(w, "generator.html", data)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
s.log.Error("failed to render generator template", "error", err)
|
s.log.Error("failed to render generator template", "error", err)
|
||||||
http.Error(w, "Internal server error", http.StatusInternalServerError)
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func (s *Handlers) renderGeneratorWithForm(
|
func (s *Handlers) renderGeneratorWithForm(
|
||||||
w http.ResponseWriter, r *http.Request, errorMsg string, form url.Values,
|
w http.ResponseWriter, r *http.Request, errorMsg string,
|
||||||
|
form url.Values, status int,
|
||||||
) {
|
) {
|
||||||
s.renderGenerator(w, r, &generatorData{
|
s.renderGeneratorStatus(w, r, &generatorData{
|
||||||
Error: errorMsg,
|
Error: errorMsg,
|
||||||
FormURL: form.Get("url"),
|
FormURL: form.Get("url"),
|
||||||
FormWidth: form.Get("width"),
|
FormWidth: form.Get("width"),
|
||||||
@@ -240,7 +321,7 @@ func (s *Handlers) renderGeneratorWithForm(
|
|||||||
FormQuality: form.Get("quality"),
|
FormQuality: form.Get("quality"),
|
||||||
FormFit: form.Get("fit"),
|
FormFit: form.Get("fit"),
|
||||||
FormTTL: form.Get("ttl"),
|
FormTTL: form.Get("ttl"),
|
||||||
})
|
}, status)
|
||||||
}
|
}
|
||||||
|
|
||||||
func (s *Handlers) buildGeneratedURL(r *http.Request, token, format string) string {
|
func (s *Handlers) buildGeneratedURL(r *http.Request, token, format string) string {
|
||||||
|
|||||||
@@ -0,0 +1,66 @@
|
|||||||
|
package handlers
|
||||||
|
|
||||||
|
import (
|
||||||
|
"maps"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"net/url"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
// generatePost submits the /generate form with a valid session and CSRF token
|
||||||
|
// plus the caller's extra fields, returning the recorder.
|
||||||
|
func generatePost(
|
||||||
|
t *testing.T, extra url.Values,
|
||||||
|
) *httptest.ResponseRecorder {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
h, srv := newCSRFTestRouter(t)
|
||||||
|
|
||||||
|
sessionCookie := newSessionCookie(t, h)
|
||||||
|
cookies, token := csrfCredentials(t, srv, []*http.Cookie{sessionCookie})
|
||||||
|
cookies = append(cookies, sessionCookie)
|
||||||
|
|
||||||
|
form := url.Values{
|
||||||
|
sourceURLField: {testSourceURL},
|
||||||
|
csrfTokenField: {token},
|
||||||
|
}
|
||||||
|
maps.Copy(form, extra)
|
||||||
|
|
||||||
|
return postForm(srv, "/generate", cookies, form)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestGeneratePostRejectsNonNumericWidth verifies that a non-numeric width is
|
||||||
|
// rejected with 400 naming the field rather than being coerced to 0 and
|
||||||
|
// minting a 0-width token.
|
||||||
|
func TestGeneratePostRejectsNonNumericWidth(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
rec := generatePost(t, url.Values{"width": {"abc"}})
|
||||||
|
|
||||||
|
if rec.Code != http.StatusBadRequest {
|
||||||
|
t.Fatalf("status = %d, want %d", rec.Code, http.StatusBadRequest)
|
||||||
|
}
|
||||||
|
|
||||||
|
if strings.Contains(rec.Body.String(), "/v1/e/") {
|
||||||
|
t.Error("a token was generated for non-numeric width")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestGeneratePostRejectsOverLimitWidth verifies that a width beyond
|
||||||
|
// MaxDimension is rejected at generation time so an unusable token cannot be
|
||||||
|
// minted.
|
||||||
|
func TestGeneratePostRejectsOverLimitWidth(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
rec := generatePost(t, url.Values{"width": {"100000"}})
|
||||||
|
|
||||||
|
if rec.Code != http.StatusBadRequest {
|
||||||
|
t.Fatalf("status = %d, want %d", rec.Code, http.StatusBadRequest)
|
||||||
|
}
|
||||||
|
|
||||||
|
if strings.Contains(rec.Body.String(), "/v1/e/") {
|
||||||
|
t.Error("a token was generated for an over-limit width")
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,45 @@
|
|||||||
|
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)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,177 @@
|
|||||||
|
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")
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -110,13 +110,6 @@ func (s *Handlers) parseImageRequest(
|
|||||||
|
|
||||||
if fit := query.Get("fit"); fit != "" {
|
if fit := query.Get("fit"); fit != "" {
|
||||||
req.FitMode = imgcache.FitMode(fit)
|
req.FitMode = imgcache.FitMode(fit)
|
||||||
|
|
||||||
fitErr := imgcache.ValidateFitMode(req.FitMode)
|
|
||||||
if fitErr != nil {
|
|
||||||
s.respondError(w, "invalid fit mode: "+fit, http.StatusBadRequest)
|
|
||||||
|
|
||||||
return nil, false
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Default quality if not set
|
// Default quality if not set
|
||||||
@@ -129,6 +122,18 @@ func (s *Handlers) parseImageRequest(
|
|||||||
req.FitMode = imgcache.FitCover
|
req.FitMode = imgcache.FitCover
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Enforce dimension and fit-mode bounds, shared with the encrypted-URL
|
||||||
|
// route. Dimensions are already bounded by the path parser above; this
|
||||||
|
// also rejects an unrecognized fit mode with 400 instead of letting it
|
||||||
|
// reach the processor as a 500.
|
||||||
|
err = imgcache.ValidateImageRequest(req)
|
||||||
|
if err != nil {
|
||||||
|
s.respondError(w, "invalid image request: "+err.Error(),
|
||||||
|
http.StatusBadRequest)
|
||||||
|
|
||||||
|
return nil, false
|
||||||
|
}
|
||||||
|
|
||||||
return req, true
|
return req, true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -50,6 +50,19 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc {
|
|||||||
// Convert payload to ImageRequest
|
// Convert payload to ImageRequest
|
||||||
req := payload.ToImageRequest()
|
req := payload.ToImageRequest()
|
||||||
|
|
||||||
|
// Apply the same dimension and fit-mode bounds as the plain image
|
||||||
|
// route: a sealed payload is trusted for its origin, not for staying
|
||||||
|
// within limits, so an over-limit size or unknown fit mode is a 400
|
||||||
|
// here rather than an out-of-memory or a 500 from the processor.
|
||||||
|
err = imgcache.ValidateImageRequest(req)
|
||||||
|
if err != nil {
|
||||||
|
s.log.Debug("encrypted URL failed validation", "error", err)
|
||||||
|
s.respondError(w, "invalid encrypted URL: "+err.Error(),
|
||||||
|
http.StatusBadRequest)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
// Log the request
|
// Log the request
|
||||||
s.log.Debug("encrypted image request",
|
s.log.Debug("encrypted image request",
|
||||||
"host", req.SourceHost,
|
"host", req.SourceHost,
|
||||||
|
|||||||
@@ -0,0 +1,98 @@
|
|||||||
|
package handlers
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"log/slog"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/go-chi/chi/v5"
|
||||||
|
|
||||||
|
"sneak.berlin/go/pixa/internal/encurl"
|
||||||
|
"sneak.berlin/go/pixa/internal/imgcache"
|
||||||
|
)
|
||||||
|
|
||||||
|
// newEncTestServer builds a router serving the encrypted-URL route with a
|
||||||
|
// generator seeded by the shared test signing key. The image service is left
|
||||||
|
// nil: these tests exercise validation that rejects a token before any image
|
||||||
|
// is fetched, so the handler must never reach the service.
|
||||||
|
func newEncTestServer(t *testing.T) (*encurl.Generator, http.Handler) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
encGen, err := encurl.NewGenerator(testSigningKey)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("encurl.NewGenerator() error = %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
h := &Handlers{
|
||||||
|
log: slog.New(slog.DiscardHandler),
|
||||||
|
encGen: encGen,
|
||||||
|
}
|
||||||
|
|
||||||
|
r := chi.NewRouter()
|
||||||
|
r.Get("/v1/e/{token}/*", h.HandleImageEnc())
|
||||||
|
|
||||||
|
return encGen, r
|
||||||
|
}
|
||||||
|
|
||||||
|
// getEncToken issues a GET for the given token and returns the recorder.
|
||||||
|
func getEncToken(srv http.Handler, token string) *httptest.ResponseRecorder {
|
||||||
|
req := httptest.NewRequestWithContext(
|
||||||
|
context.Background(), http.MethodGet, "/v1/e/"+token+"/img.jpg", nil)
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
srv.ServeHTTP(rec, req)
|
||||||
|
|
||||||
|
return rec
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestHandleImageEnc_OverLimitDimension_Returns400 verifies that a decrypted
|
||||||
|
// token requesting a dimension beyond MaxDimension is rejected with 400
|
||||||
|
// instead of reaching the image processor and libvips.
|
||||||
|
func TestHandleImageEnc_OverLimitDimension_Returns400(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
encGen, srv := newEncTestServer(t)
|
||||||
|
|
||||||
|
token, err := encGen.Generate(&encurl.Payload{
|
||||||
|
SourceHost: "cdn.example.com",
|
||||||
|
SourcePath: "/photo.jpg",
|
||||||
|
Width: 100000,
|
||||||
|
Height: 100000,
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Generate() error = %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
rec := getEncToken(srv, token)
|
||||||
|
|
||||||
|
if rec.Code != http.StatusBadRequest {
|
||||||
|
t.Fatalf("status = %d, want %d", rec.Code, http.StatusBadRequest)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestHandleImageEnc_InvalidFitMode_Returns400 verifies that a decrypted token
|
||||||
|
// carrying an unrecognized fit mode is rejected with 400 rather than surfacing
|
||||||
|
// as a 500 from the image processor's default branch.
|
||||||
|
func TestHandleImageEnc_InvalidFitMode_Returns400(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
encGen, srv := newEncTestServer(t)
|
||||||
|
|
||||||
|
token, err := encGen.Generate(&encurl.Payload{
|
||||||
|
SourceHost: "cdn.example.com",
|
||||||
|
SourcePath: "/photo.jpg",
|
||||||
|
Width: 800,
|
||||||
|
Height: 600,
|
||||||
|
FitMode: imgcache.FitMode("bogus"),
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Generate() error = %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
rec := getEncToken(srv, token)
|
||||||
|
|
||||||
|
if rec.Code != http.StatusBadRequest {
|
||||||
|
t.Fatalf("status = %d, want %d", rec.Code, http.StatusBadRequest)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -59,6 +59,23 @@ func ValidateFitMode(fit FitMode) error {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ValidateImageRequest checks that a request's dimensions are within
|
||||||
|
// MaxDimension and its fit mode is recognized. Both the plain /v1/image/
|
||||||
|
// route and the encrypted /v1/e/ route validate through this function so a
|
||||||
|
// request from either source enforces identical bounds, regardless of how it
|
||||||
|
// was constructed. A width or height of 0 means "original size" and is valid.
|
||||||
|
func ValidateImageRequest(req *ImageRequest) error {
|
||||||
|
if req.Size.Width < 0 || req.Size.Height < 0 {
|
||||||
|
return ErrInvalidSize
|
||||||
|
}
|
||||||
|
|
||||||
|
if req.Size.Width > MaxDimension || req.Size.Height > MaxDimension {
|
||||||
|
return ErrDimensionTooLarge
|
||||||
|
}
|
||||||
|
|
||||||
|
return ValidateFitMode(req.FitMode)
|
||||||
|
}
|
||||||
|
|
||||||
// ImageRequest represents a request for a processed image
|
// ImageRequest represents a request for a processed image
|
||||||
type ImageRequest struct {
|
type ImageRequest struct {
|
||||||
// SourceHost is the origin host (e.g., "cdn.example.com")
|
// SourceHost is the origin host (e.g., "cdn.example.com")
|
||||||
|
|||||||
@@ -0,0 +1,64 @@
|
|||||||
|
package imgcache
|
||||||
|
|
||||||
|
import (
|
||||||
|
"errors"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestValidateImageRequest(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
req ImageRequest
|
||||||
|
wantErr error
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "within bounds",
|
||||||
|
req: ImageRequest{Size: Size{Width: 800, Height: 600}, FitMode: FitCover},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "original size and empty fit",
|
||||||
|
req: ImageRequest{Size: Size{Width: 0, Height: 0}},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "width over limit",
|
||||||
|
req: ImageRequest{Size: Size{Width: MaxDimension + 1, Height: 600}},
|
||||||
|
wantErr: ErrDimensionTooLarge,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "height over limit",
|
||||||
|
req: ImageRequest{Size: Size{Width: 800, Height: MaxDimension + 1}},
|
||||||
|
wantErr: ErrDimensionTooLarge,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "negative width",
|
||||||
|
req: ImageRequest{Size: Size{Width: -1, Height: 600}},
|
||||||
|
wantErr: ErrInvalidSize,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "invalid fit mode",
|
||||||
|
req: ImageRequest{Size: Size{Width: 800, Height: 600}, FitMode: "bogus"},
|
||||||
|
wantErr: ErrInvalidFitMode,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
err := ValidateImageRequest(&tt.req)
|
||||||
|
if tt.wantErr == nil {
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ValidateImageRequest() error = %v, want nil", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
if !errors.Is(err, tt.wantErr) {
|
||||||
|
t.Fatalf("ValidateImageRequest() error = %v, want %v", err, tt.wantErr)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
+27
-11
@@ -9,24 +9,40 @@ import (
|
|||||||
|
|
||||||
// HTTP server configuration constants.
|
// HTTP server configuration constants.
|
||||||
const (
|
const (
|
||||||
HTTPReadTimeout = 30 * time.Second
|
HTTPReadTimeout = 30 * time.Second
|
||||||
HTTPWriteTimeout = 60 * 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
|
||||||
|
// 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
|
||||||
)
|
)
|
||||||
|
|
||||||
func (s *Server) serveUntilShutdown() {
|
// newHTTPServer builds the http.Server with the hardening timeouts and
|
||||||
listenAddr := fmt.Sprintf(":%d", s.config.Port)
|
// limits applied. It is separate from serveUntilShutdown so the
|
||||||
s.httpServer = &http.Server{
|
// configuration can be asserted in a test without binding a listener.
|
||||||
Addr: listenAddr,
|
func (s *Server) newHTTPServer() *http.Server {
|
||||||
ReadTimeout: HTTPReadTimeout,
|
return &http.Server{
|
||||||
WriteTimeout: HTTPWriteTimeout,
|
Addr: fmt.Sprintf(":%d", s.config.Port),
|
||||||
MaxHeaderBytes: HTTPMaxHeaderBytes,
|
ReadTimeout: HTTPReadTimeout,
|
||||||
Handler: s,
|
ReadHeaderTimeout: HTTPReadHeaderTimeout,
|
||||||
|
WriteTimeout: HTTPWriteTimeout,
|
||||||
|
IdleTimeout: HTTPIdleTimeout,
|
||||||
|
MaxHeaderBytes: HTTPMaxHeaderBytes,
|
||||||
|
Handler: s,
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func (s *Server) serveUntilShutdown() {
|
||||||
|
s.httpServer = s.newHTTPServer()
|
||||||
|
|
||||||
s.SetupRoutes()
|
s.SetupRoutes()
|
||||||
|
|
||||||
s.log.Info("http begin listen", "listenaddr", listenAddr)
|
s.log.Info("http begin listen", "listenaddr", s.httpServer.Addr)
|
||||||
|
|
||||||
err := s.httpServer.ListenAndServe()
|
err := s.httpServer.ListenAndServe()
|
||||||
if err != nil && !errors.Is(err, http.ErrServerClosed) {
|
if err != nil && !errors.Is(err, http.ErrServerClosed) {
|
||||||
|
|||||||
@@ -0,0 +1,65 @@
|
|||||||
|
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,6 +8,7 @@ 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"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -46,8 +47,10 @@ 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.
|
// covers the login POST, where no session exists yet. LimitBody caps
|
||||||
|
// 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