Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
a663669286 | ||
|
|
6d380fbf98 |
@@ -29,6 +29,17 @@ P1: implement blocked networks configuration to extend SSRF protection
|
||||
|
||||
# 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
|
||||
|
||||
+98
-17
@@ -2,6 +2,8 @@ package handlers
|
||||
|
||||
import (
|
||||
"crypto/subtle"
|
||||
"errors"
|
||||
"fmt"
|
||||
"html/template"
|
||||
"net/http"
|
||||
"net/url"
|
||||
@@ -13,6 +15,11 @@ import (
|
||||
"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.
|
||||
func (s *Handlers) HandleRoot() http.HandlerFunc {
|
||||
return func(w http.ResponseWriter, r *http.Request) {
|
||||
@@ -98,18 +105,26 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc {
|
||||
// Validate source URL
|
||||
parsed, err := url.Parse(sourceURL)
|
||||
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
|
||||
}
|
||||
|
||||
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
|
||||
token, err := s.encGen.Generate(payload)
|
||||
if err != nil {
|
||||
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
|
||||
}
|
||||
@@ -137,17 +152,38 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc {
|
||||
}
|
||||
|
||||
// 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(
|
||||
parsed *url.URL, form url.Values,
|
||||
) (*encurl.Payload, time.Time, int) {
|
||||
width, _ := strconv.Atoi(form.Get("width"))
|
||||
height, _ := strconv.Atoi(form.Get("height"))
|
||||
quality, _ := strconv.Atoi(form.Get("quality"))
|
||||
ttl, _ := strconv.Atoi(form.Get("ttl"))
|
||||
) (*encurl.Payload, time.Time, int, error) {
|
||||
width, err := parseFormDimension(form, "width")
|
||||
if err != nil {
|
||||
return nil, time.Time{}, 0, err
|
||||
}
|
||||
|
||||
if quality <= 0 {
|
||||
quality = 85
|
||||
height, err := parseFormDimension(form, "height")
|
||||
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 (
|
||||
@@ -168,11 +204,45 @@ func buildGeneratePayload(
|
||||
Height: height,
|
||||
Format: imgcache.ImageFormat(form.Get("format")),
|
||||
Quality: quality,
|
||||
FitMode: imgcache.FitMode(form.Get("fit")),
|
||||
FitMode: fitMode,
|
||||
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.
|
||||
@@ -212,6 +282,15 @@ func (s *Handlers) renderLogin(
|
||||
|
||||
func (s *Handlers) renderGenerator(
|
||||
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")
|
||||
|
||||
@@ -221,17 +300,19 @@ func (s *Handlers) renderGenerator(
|
||||
|
||||
data.CSRFField = csrfField(r)
|
||||
|
||||
w.WriteHeader(status)
|
||||
|
||||
err := templates.Render(w, "generator.html", data)
|
||||
if err != nil {
|
||||
s.log.Error("failed to render generator template", "error", err)
|
||||
http.Error(w, "Internal server error", http.StatusInternalServerError)
|
||||
}
|
||||
}
|
||||
|
||||
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,
|
||||
FormURL: form.Get("url"),
|
||||
FormWidth: form.Get("width"),
|
||||
@@ -240,7 +321,7 @@ func (s *Handlers) renderGeneratorWithForm(
|
||||
FormQuality: form.Get("quality"),
|
||||
FormFit: form.Get("fit"),
|
||||
FormTTL: form.Get("ttl"),
|
||||
})
|
||||
}, status)
|
||||
}
|
||||
|
||||
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")
|
||||
}
|
||||
}
|
||||
@@ -110,13 +110,6 @@ func (s *Handlers) parseImageRequest(
|
||||
|
||||
if fit := query.Get("fit"); 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
|
||||
@@ -129,6 +122,18 @@ func (s *Handlers) parseImageRequest(
|
||||
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
|
||||
}
|
||||
|
||||
|
||||
@@ -50,6 +50,19 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc {
|
||||
// Convert payload to ImageRequest
|
||||
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
|
||||
s.log.Debug("encrypted image request",
|
||||
"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
|
||||
type ImageRequest struct {
|
||||
// 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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user