Take the signature, the expiry and the URL scheme off the image request (closes #314)
check / check (push) Canceled after 0s

The image request now holds only its source and its transform, exactly what
the variant key covers. The /v1/image/ handler checks the allowlist and
verifies the signature itself with Signer.Verify(req, sig, expires). Both
image handlers keep the URL's expiry as a local value for Cache-Control. The
image service holds no signer, allowlist or AllowHTTP, and ValidateRequest is
gone. The fetcher takes the source and builds its URL with Source.URL from its
own AllowHTTP, so no URL string is built and parsed again. chooseAutoFormat
returns the format it chooses instead of writing into the request. The signed
URL format and every route's answers are unchanged.

Model: opus-5-5
This commit is contained in:
2026-10-26 12:18:23 +00:00
parent 63690a9ec9
commit 8a03d0dbe7
46 changed files with 753 additions and 908 deletions
+21
View File
@@ -30,6 +30,27 @@ P2: security: per-origin rate limiting
# Completed Steps # Completed Steps
- 2026-10-26 the image request holds only what names the cached object (closes
#314): its source and its transform, which the variant key covers. The signed
URL format, the signing algorithm and every route's answers are unchanged.
- The `/v1/image/` handler checks `allowlist_hosts` and verifies the
signature itself, with `Signer.Verify(req, sig, expires)`.
- Each image handler keeps its URL's expiry as a value of its own, for
`Cache-Control`. `encurl`'s `Generate` takes the expiry and its `Parse`
returns it.
- The fetcher takes the source and builds its URL with `Source.URL`, over
http only when the fetcher's own `AllowHTTP` is set. `validateURL` takes
that URL, so no URL string is built and parsed again.
- `chooseAutoFormat` returns the format it chooses, and the handler puts it
into the request it parsed.
- The image service serves images through `Get`, and has `WaitForProcessing`
for shutdown.
- Removed:
- `Signature`, `Expires`, `AllowHTTP` and `SourceURL` of the image
request
- `Service.ValidateRequest`, and the service's signer, allowlist and
`AllowHTTP`
- `extractHost` of the fetcher and `urlToFSPath` of the mock fetcher
- 2026-10-26 pixad's fx graph builds the image pipeline (closes #313). - 2026-10-26 pixad's fx graph builds the image pipeline (closes #313).
`database.New` now opens the database, so the parts below can read it while `database.New` now opens the database, so the parts below can read it while
they are built; its stop hook still runs after the cache's. The cache's they are built; its stop hook still runs after the cache's. The cache's
+17 -16
View File
@@ -59,12 +59,14 @@ func NewGenerator(signingKey string) (*Generator, error) {
return &Generator{key: key}, nil return &Generator{key: key}, nil
} }
// Generate creates an encrypted URL token from the request's source, // Generate creates an encrypted URL token from the request's source and
// transform and expiry, each as the request holds it; an empty format, a // transform, each as the request holds it, and from expires; an empty format,
// quality of 0 or an empty fit mode is left out of the token, and a zero // a quality of 0 or an empty fit mode is left out of the token, and a zero
// Expires makes a token that never expires. The token is CBOR-encoded, // expires makes a token that never expires. The token is CBOR-encoded,
// encrypted with NaCl secretbox, and base64url-encoded. // encrypted with NaCl secretbox, and base64url-encoded.
func (g *Generator) Generate(req *imagerequest.Request) (string, error) { func (g *Generator) Generate(
req *imagerequest.Request, expires time.Time,
) (string, error) {
p := &payload{ p := &payload{
SourceHost: req.Source.Host, SourceHost: req.Source.Host,
SourcePath: req.Source.Path, SourcePath: req.Source.Path,
@@ -76,8 +78,8 @@ func (g *Generator) Generate(req *imagerequest.Request) (string, error) {
FitMode: req.Transform.FitMode, FitMode: req.Transform.FitMode,
} }
if !req.Expires.IsZero() { if !expires.IsZero() {
p.ExpiresAt = req.Expires.Unix() p.ExpiresAt = expires.Unix()
} }
// CBOR encode the payload // CBOR encode the payload
@@ -91,19 +93,19 @@ func (g *Generator) Generate(req *imagerequest.Request) (string, error) {
} }
// Parse decrypts and validates an encrypted URL token and returns the // Parse decrypts and validates an encrypted URL token and returns the
// request it holds, with default values for omitted optional fields. A token // request it holds, with default values for omitted optional fields, and its
// that never expires gives the zero Expires. // expiry. A token that never expires gives the zero time.
// Returns ErrExpired if the token has expired, or other errors for invalid tokens. // Returns ErrExpired if the token has expired, or other errors for invalid tokens.
func (g *Generator) Parse(token string) (*imagerequest.Request, error) { func (g *Generator) Parse(token string) (*imagerequest.Request, time.Time, error) {
// Decrypt // Decrypt
data, err := seal.Decrypt(g.key, token) data, err := seal.Decrypt(g.key, token)
if err != nil { if err != nil {
if errors.Is(err, seal.ErrDecryptionFailed) || if errors.Is(err, seal.ErrDecryptionFailed) ||
errors.Is(err, seal.ErrInvalidPayload) { errors.Is(err, seal.ErrInvalidPayload) {
return nil, ErrDecryptFailed return nil, time.Time{}, ErrDecryptFailed
} }
return nil, err return nil, time.Time{}, err
} }
// CBOR decode // CBOR decode
@@ -111,12 +113,12 @@ func (g *Generator) Parse(token string) (*imagerequest.Request, error) {
err = cbor.Unmarshal(data, &p) err = cbor.Unmarshal(data, &p)
if err != nil { if err != nil {
return nil, ErrInvalidFormat return nil, time.Time{}, ErrInvalidFormat
} }
// Check expiration (0 = never expires) // Check expiration (0 = never expires)
if p.ExpiresAt != 0 && time.Now().Unix() > p.ExpiresAt { if p.ExpiresAt != 0 && time.Now().Unix() > p.ExpiresAt {
return nil, ErrExpired return nil, time.Time{}, ErrExpired
} }
format := p.Format format := p.Format
@@ -151,6 +153,5 @@ func (g *Generator) Parse(token string) (*imagerequest.Request, error) {
Quality: quality, Quality: quality,
FitMode: fitMode, FitMode: fitMode,
}, },
Expires: expires, }, expires, nil
}, nil
} }
+15 -20
View File
@@ -36,10 +36,10 @@ func TestGenerator_GenerateAndParse(t *testing.T) {
Quality: 90, Quality: 90,
FitMode: imagerequest.FitContain, FitMode: imagerequest.FitContain,
}, },
Expires: time.Now().Add(time.Hour),
} }
expires := time.Now().Add(time.Hour)
token, err := gen.Generate(req) token, err := gen.Generate(req, expires)
if err != nil { if err != nil {
t.Fatalf("Generate() error = %v", err) t.Fatalf("Generate() error = %v", err)
} }
@@ -48,7 +48,7 @@ func TestGenerator_GenerateAndParse(t *testing.T) {
t.Fatal("Generate() returned empty token") t.Fatal("Generate() returned empty token")
} }
parsed, err := gen.Parse(token) parsed, parsedExpires, err := gen.Parse(token)
if err != nil { if err != nil {
t.Fatalf("Parse() error = %v", err) t.Fatalf("Parse() error = %v", err)
} }
@@ -91,8 +91,8 @@ func TestGenerator_GenerateAndParse(t *testing.T) {
parsed.Transform.FitMode, req.Transform.FitMode) parsed.Transform.FitMode, req.Transform.FitMode)
} }
if parsed.Expires.Unix() != req.Expires.Unix() { if parsedExpires.Unix() != expires.Unix() {
t.Errorf("ExpiresAt = %d, want %d", parsed.Expires.Unix(), req.Expires.Unix()) t.Errorf("ExpiresAt = %d, want %d", parsedExpires.Unix(), expires.Unix())
} }
} }
@@ -106,15 +106,14 @@ func TestGenerator_Parse_Expired(t *testing.T) {
Host: testSourceHost, Host: testSourceHost,
Path: testSourcePath, Path: testSourcePath,
}, },
Expires: time.Now().Add(-time.Hour), // Already expired
} }
token, err := gen.Generate(req) token, err := gen.Generate(req, time.Now().Add(-time.Hour)) // Already expired
if err != nil { if err != nil {
t.Fatalf("Generate() error = %v", err) t.Fatalf("Generate() error = %v", err)
} }
_, err = gen.Parse(token) _, _, err = gen.Parse(token)
if err == nil { if err == nil {
t.Error("Parse() should fail for expired token") t.Error("Parse() should fail for expired token")
} }
@@ -129,7 +128,7 @@ func TestGenerator_Parse_InvalidToken(t *testing.T) {
gen, _ := encurl.NewGenerator("test-signing-key-12345") gen, _ := encurl.NewGenerator("test-signing-key-12345")
_, err := gen.Parse("not-a-valid-token") _, _, err := gen.Parse("not-a-valid-token")
if err == nil { if err == nil {
t.Error("Parse() should fail for invalid token") t.Error("Parse() should fail for invalid token")
} }
@@ -145,10 +144,9 @@ func TestGenerator_Parse_TamperedToken(t *testing.T) {
Host: testSourceHost, Host: testSourceHost,
Path: testSourcePath, Path: testSourcePath,
}, },
Expires: time.Now().Add(time.Hour),
} }
token, _ := gen.Generate(req) token, _ := gen.Generate(req, time.Now().Add(time.Hour))
// Tamper with the token // Tamper with the token
tampered := []byte(token) tampered := []byte(token)
@@ -156,7 +154,7 @@ func TestGenerator_Parse_TamperedToken(t *testing.T) {
tampered[10] ^= 0x01 tampered[10] ^= 0x01
} }
_, err := gen.Parse(string(tampered)) _, _, err := gen.Parse(string(tampered))
if err == nil { if err == nil {
t.Error("Parse() should fail for tampered token") t.Error("Parse() should fail for tampered token")
} }
@@ -173,12 +171,11 @@ func TestGenerator_Parse_WrongKey(t *testing.T) {
Host: testSourceHost, Host: testSourceHost,
Path: testSourcePath, Path: testSourcePath,
}, },
Expires: time.Now().Add(time.Hour),
} }
token, _ := gen1.Generate(req) token, _ := gen1.Generate(req, time.Now().Add(time.Hour))
_, err := gen2.Parse(token) _, _, err := gen2.Parse(token)
if err == nil { if err == nil {
t.Error("Parse() should fail with different signing key") t.Error("Parse() should fail with different signing key")
} }
@@ -195,13 +192,12 @@ func TestGenerator_Parse_Defaults(t *testing.T) {
Host: testSourceHost, Host: testSourceHost,
Path: testSourcePath, Path: testSourcePath,
}, },
Expires: time.Now().Add(time.Hour), }, time.Now().Add(time.Hour))
})
if err != nil { if err != nil {
t.Fatalf("Generate() error = %v", err) t.Fatalf("Generate() error = %v", err)
} }
req, err := gen.Parse(token) req, _, err := gen.Parse(token)
if err != nil { if err != nil {
t.Fatalf("Parse() error = %v", err) t.Fatalf("Parse() error = %v", err)
} }
@@ -232,10 +228,9 @@ func TestGenerator_TokenIsURLSafe(t *testing.T) {
Host: testSourceHost, Host: testSourceHost,
Path: testSourcePath, Path: testSourcePath,
}, },
Expires: time.Now().Add(time.Hour),
} }
token, _ := gen.Generate(req) token, _ := gen.Generate(req, time.Now().Add(time.Hour))
// Check that token only contains URL-safe characters // Check that token only contains URL-safe characters
for _, c := range token { for _, c := range token {
+14 -17
View File
@@ -153,20 +153,20 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc {
return return
} }
// The image URL must fetch the source URL as it was written // The image URL must fetch the source URL as it was written. The
req.AllowHTTP = s.config.AllowHTTP // fetcher fetches over http only when allow_http is set.
fetchURL := req.SourceURL() fetchURL := req.Source.URL(s.config.AllowHTTP)
if !fetchedAsWritten(parsed, fetchURL) { if !fetchedAsWritten(parsed, fetchURL) {
s.renderGeneratorWithForm(w, r, "Source URL cannot be fetched as "+ s.renderGeneratorWithForm(w, r, "Source URL cannot be fetched as "+
"written: pixa would fetch "+fetchURL, "written: pixa would fetch "+fetchURL.String(),
r.Form, http.StatusBadRequest) r.Form, http.StatusBadRequest)
return return
} }
// Generate encrypted token // Generate encrypted token
token, err := s.encGen.Generate(req) token, err := s.encGen.Generate(req, expiresAt)
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,
@@ -226,25 +226,23 @@ func parseSourceURL(sourceURL string) (*url.URL, error) {
// in RawPath when it is not the decoded path percent-encoded again. // in RawPath when it is not the decoded path percent-encoded again.
// EscapedPath ignores RawPath when it also holds a character net/url encodes, // EscapedPath ignores RawPath when it also holds a character net/url encodes,
// such as a space, so RawPath is compared itself. // such as a space, so RawPath is compared itself.
func fetchedAsWritten(source *url.URL, fetchURL string) bool { func fetchedAsWritten(source, fetchURL *url.URL) bool {
writtenPath := source.RawPath writtenPath := source.RawPath
if writtenPath == "" { if writtenPath == "" {
writtenPath = source.EscapedPath() writtenPath = source.EscapedPath()
} }
fetched, err := url.Parse(fetchURL) return source.User == nil && !source.ForceQuery &&
fetchURL.EscapedPath() == writtenPath &&
return err == nil && source.User == nil && !source.ForceQuery && fetchURL.RawQuery == source.RawQuery
fetched.EscapedPath() == writtenPath &&
fetched.RawQuery == source.RawQuery
} }
// buildGenerateRequest parses the numeric form fields and assembles the // buildGenerateRequest parses the numeric form fields and assembles the
// image request the encrypted URL's token is made from. ttl=0 means never // image request the encrypted URL's token is made from, and its expiry.
// expires (Expires stays the zero time). A non-numeric or out-of-range // ttl=0 means never expires (the expiry is the zero time). A non-numeric or
// width, height, quality or ttl, or an unrecognized fit mode, is a client // out-of-range width, height, quality or ttl, or an unrecognized fit mode, is
// error naming the offending field. The format field is passed through // a client error naming the offending field. The format field is passed
// unchecked. // through unchecked.
func buildGenerateRequest( func buildGenerateRequest(
parsed *url.URL, form url.Values, parsed *url.URL, form url.Values,
) (*imagerequest.Request, time.Time, int, error) { ) (*imagerequest.Request, time.Time, int, error) {
@@ -294,7 +292,6 @@ func buildGenerateRequest(
Quality: quality, Quality: quality,
FitMode: fitMode, FitMode: fitMode,
}, },
Expires: expiresAt,
} }
return req, expiresAt, ttl, nil return req, expiresAt, ttl, nil
+5 -3
View File
@@ -19,11 +19,13 @@ import (
"github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5"
"github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus"
"sneak.berlin/go/pixa/internal/allowlist"
"sneak.berlin/go/pixa/internal/database" "sneak.berlin/go/pixa/internal/database"
"sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/imageprocessor" "sneak.berlin/go/pixa/internal/imageprocessor"
"sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imagerequest"
"sneak.berlin/go/pixa/internal/imgcache" "sneak.berlin/go/pixa/internal/imgcache"
"sneak.berlin/go/pixa/internal/signature"
) )
// countingHost is the allowlisted host whose photoPath the server of // countingHost is the allowlisted host whose photoPath the server of
@@ -137,8 +139,6 @@ func newCountingDatabaseServer(
MaxInputBytes: 50 << 20, MaxInputBytes: 50 << 20,
MaxConcurrentProcessing: 2, MaxConcurrentProcessing: 2,
}), }),
SigningKey: testSigningKey,
Allowlist: []string{countingHost},
MaxResponseSize: 50 << 20, MaxResponseSize: 50 << 20,
Logger: slog.New(slog.DiscardHandler), Logger: slog.New(slog.DiscardHandler),
}) })
@@ -159,7 +159,7 @@ func newCountingDatabaseServer(
Quality: 85, Quality: 85,
FitMode: imagerequest.FitCover, FitMode: imagerequest.FitCover,
}, },
}) }, time.Time{})
if err != nil { if err != nil {
tb.Fatalf("encGen.Generate() error = %v", err) tb.Fatalf("encGen.Generate() error = %v", err)
} }
@@ -168,6 +168,8 @@ func newCountingDatabaseServer(
log: slog.New(slog.DiscardHandler), log: slog.New(slog.DiscardHandler),
imgSvc: svc, imgSvc: svc,
encGen: encGen, encGen: encGen,
signer: signature.New(testSigningKey),
allowlist: allowlist.New([]string{countingHost}),
} }
r := chi.NewRouter() r := chi.NewRouter()
+13 -6
View File
@@ -13,11 +13,13 @@ import (
"github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5"
"github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus"
"sneak.berlin/go/pixa/internal/allowlist"
"sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/httpfetcher" "sneak.berlin/go/pixa/internal/httpfetcher"
"sneak.berlin/go/pixa/internal/imageprocessor" "sneak.berlin/go/pixa/internal/imageprocessor"
"sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imagerequest"
"sneak.berlin/go/pixa/internal/imgcache" "sneak.berlin/go/pixa/internal/imgcache"
"sneak.berlin/go/pixa/internal/signature"
) )
// mismatchedSourcePath is the path of the source that // mismatchedSourcePath is the path of the source that
@@ -27,8 +29,9 @@ import (
const mismatchedSourcePath = "/images/source.png" const mismatchedSourcePath = "/images/source.png"
// newRoutesFetchingThrough returns both image routes, and the Handlers behind // newRoutesFetchingThrough returns both image routes, and the Handlers behind
// them, whose image service fetches through fetcher and has allowlistedHost on // them, whose image service fetches through fetcher, and which have
// its allowlist. The handlers and the image service log to log. // allowlistedHost on their allowlist. The handlers and the image service log
// to log.
func newRoutesFetchingThrough( func newRoutesFetchingThrough(
t *testing.T, fetcher httpfetcher.Fetcher, log *slog.Logger, t *testing.T, fetcher httpfetcher.Fetcher, log *slog.Logger,
) (*Handlers, http.Handler) { ) (*Handlers, http.Handler) {
@@ -50,8 +53,6 @@ func newRoutesFetchingThrough(
MaxInputBytes: 50 << 20, MaxInputBytes: 50 << 20,
MaxConcurrentProcessing: 2, MaxConcurrentProcessing: 2,
}), }),
SigningKey: testSigningKey,
Allowlist: []string{allowlistedHost},
MaxResponseSize: 50 << 20, MaxResponseSize: 50 << 20,
Logger: log, Logger: log,
}) })
@@ -64,7 +65,13 @@ func newRoutesFetchingThrough(
t.Fatalf("encurl.NewGenerator() error = %v", err) t.Fatalf("encurl.NewGenerator() error = %v", err)
} }
h := &Handlers{log: log, imgSvc: svc, encGen: encGen} h := &Handlers{
log: log,
imgSvc: svc,
encGen: encGen,
signer: signature.New(testSigningKey),
allowlist: allowlist.New([]string{allowlistedHost}),
}
r := chi.NewRouter() r := chi.NewRouter()
r.Get("/v1/image/*", h.HandleImage()) r.Get("/v1/image/*", h.HandleImage())
@@ -93,7 +100,7 @@ func mismatchedSourceEncURL(t *testing.T, h *Handlers) string {
Size: imagerequest.Size{Width: 50, Height: 50}, Size: imagerequest.Size{Width: 50, Height: 50},
Format: imagerequest.FormatJPEG, Format: imagerequest.FormatJPEG,
}, },
}) }, time.Time{})
if err != nil { if err != nil {
t.Fatalf("Generate() error = %v", err) t.Fatalf("Generate() error = %v", err)
} }
+11 -12
View File
@@ -18,16 +18,17 @@ var (
"image/jxl, image/avif, image/webp or image/jpeg") "image/jxl, image/avif, image/webp or image/jpeg")
) )
// chooseAutoFormat replaces the format auto in req with the format // chooseAutoFormat returns the format to serve for the requested format: for
// formatForAccept chooses from r's Accept header, and adds Vary: Accept to the // the format auto, the format formatForAccept chooses from r's Accept header,
// and any other format as it is. For auto it adds Vary: Accept to the
// response, which then depends on that header. It answers 400 for an Accept // response, which then depends on that header. It answers 400 for an Accept
// header that is not valid and 406 for one that allows none of the formats, // header that is not valid and 406 for one that allows none of the formats,
// and reports whether req can be served. Any other format is left as it is. // and reports whether the image can be served.
func (s *Handlers) chooseAutoFormat( func (s *Handlers) chooseAutoFormat(
w http.ResponseWriter, r *http.Request, req *imagerequest.Request, w http.ResponseWriter, r *http.Request, requested imagerequest.Format,
) bool { ) (imagerequest.Format, bool) {
if req.Transform.Format != imagerequest.FormatAuto { if requested != imagerequest.FormatAuto {
return true return requested, true
} }
w.Header().Add("Vary", "Accept") w.Header().Add("Vary", "Accept")
@@ -36,18 +37,16 @@ func (s *Handlers) chooseAutoFormat(
if errors.Is(err, errNotAcceptable) { if errors.Is(err, errNotAcceptable) {
s.respondError(w, err.Error(), http.StatusNotAcceptable) s.respondError(w, err.Error(), http.StatusNotAcceptable)
return false return "", false
} }
if err != nil { if err != nil {
s.respondError(w, err.Error(), http.StatusBadRequest) s.respondError(w, err.Error(), http.StatusBadRequest)
return false return "", false
} }
req.Transform.Format = format return format, true
return true
} }
// formatForAccept returns the format an auto URL is served in for the Accept // formatForAccept returns the format an auto URL is served in for the Accept
@@ -101,8 +101,7 @@ func autoPhotoURLs(t *testing.T, h *Handlers) (string, string) {
Size: imagerequest.Size{Width: 50, Height: 50}, Size: imagerequest.Size{Width: 50, Height: 50},
Format: imagerequest.FormatAuto, Format: imagerequest.FormatAuto,
}, },
Expires: time.Now().Add(time.Minute), }, time.Now().Add(time.Minute))
})
if err != nil { if err != nil {
t.Fatalf("Generate() error = %v", err) t.Fatalf("Generate() error = %v", err)
} }
+8
View File
@@ -16,6 +16,7 @@ import (
"sneak.berlin/go/pixa/internal/imgcache" "sneak.berlin/go/pixa/internal/imgcache"
"sneak.berlin/go/pixa/internal/logger" "sneak.berlin/go/pixa/internal/logger"
"sneak.berlin/go/pixa/internal/session" "sneak.berlin/go/pixa/internal/session"
"sneak.berlin/go/pixa/internal/signature"
) )
// Params defines dependencies for Handlers. // Params defines dependencies for Handlers.
@@ -41,6 +42,11 @@ type Handlers struct {
encGen *encurl.Generator encGen *encurl.Generator
csrfProtect func(http.Handler) http.Handler csrfProtect func(http.Handler) http.Handler
// signer checks the signature of a /v1/image/ URL with signing_key.
signer *signature.Signer
// allowlist matches the hosts of allowlist_hosts, which a /v1/image/
// URL may name without a signature.
allowlist *allowlist.HostAllowList
// refererBlocklist matches the hosts of referer_blocklist; its IsAllowed // refererBlocklist matches the hosts of referer_blocklist; its IsAllowed
// reports whether a URL's host is on that list. // reports whether a URL's host is on that list.
refererBlocklist *allowlist.HostAllowList refererBlocklist *allowlist.HostAllowList
@@ -61,6 +67,8 @@ func New(_ fx.Lifecycle, params Params) (*Handlers, error) {
sessMgr: params.SessionManager, sessMgr: params.SessionManager,
encGen: params.EncryptedURLGenerator, encGen: params.EncryptedURLGenerator,
csrfProtect: csrfProtect, csrfProtect: csrfProtect,
signer: signature.New(params.Config.SigningKey),
allowlist: allowlist.New(params.Config.AllowlistHosts),
refererBlocklist: allowlist.New(params.Config.RefererBlocklist), refererBlocklist: allowlist.New(params.Config.RefererBlocklist),
}, nil }, nil
} }
+7 -5
View File
@@ -19,10 +19,13 @@ import (
"github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5"
"github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus"
"sneak.berlin/go/pixa/internal/allowlist"
"sneak.berlin/go/pixa/internal/database" "sneak.berlin/go/pixa/internal/database"
"sneak.berlin/go/pixa/internal/httpfetcher" "sneak.berlin/go/pixa/internal/httpfetcher"
"sneak.berlin/go/pixa/internal/imageprocessor" "sneak.berlin/go/pixa/internal/imageprocessor"
"sneak.berlin/go/pixa/internal/imagerequest"
"sneak.berlin/go/pixa/internal/imgcache" "sneak.berlin/go/pixa/internal/imgcache"
"sneak.berlin/go/pixa/internal/signature"
) )
// testFixtures contains test data. // testFixtures contains test data.
@@ -62,8 +65,6 @@ func setupTestHandler(t *testing.T) *testFixtures {
MaxInputBytes: 50 << 20, MaxInputBytes: 50 << 20,
MaxConcurrentProcessing: 2, MaxConcurrentProcessing: 2,
}), }),
SigningKey: "test-signing-key-must-be-32-chars",
Allowlist: []string{goodHost},
MaxResponseSize: 50 << 20, MaxResponseSize: 50 << 20,
}) })
if err != nil { if err != nil {
@@ -73,6 +74,8 @@ func setupTestHandler(t *testing.T) *testFixtures {
h := &Handlers{ h := &Handlers{
imgSvc: svc, imgSvc: svc,
log: slog.Default(), log: slog.Default(),
signer: signature.New("test-signing-key-must-be-32-chars"),
allowlist: allowlist.New([]string{goodHost}),
} }
return &testFixtures{ return &testFixtures{
@@ -127,10 +130,9 @@ func newMockFetcher(fs fs.FS) *mockFetcher {
} }
func (f *mockFetcher) Fetch( func (f *mockFetcher) Fetch(
_ context.Context, url string, _ context.Context, source imagerequest.Source,
) (*httpfetcher.FetchResult, error) { ) (*httpfetcher.FetchResult, error) {
// Remove https:// prefix path := source.Host + source.Path
path := url[8:] // Remove "https://"
data, err := fs.ReadFile(f.fs, path) data, err := fs.ReadFile(f.fs, path)
if err != nil { if err != nil {
+50 -22
View File
@@ -7,6 +7,7 @@ import (
"net/http" "net/http"
"net/url" "net/url"
"strconv" "strconv"
"strings"
"time" "time"
"github.com/go-chi/chi/v5/middleware" "github.com/go-chi/chi/v5/middleware"
@@ -27,13 +28,13 @@ func (s *Handlers) HandleImage() http.HandlerFunc {
return return
} }
req, ok := s.parseImageRequest(w, r) req, sig, expires, ok := s.parseImageRequest(w, r)
if !ok { if !ok {
return return
} }
// Validate signature if required // Validate signature if required
err := s.imgSvc.ValidateRequest(req) err := s.validateSignature(req, sig, expires)
if err != nil { if err != nil {
s.log.Warn("signature validation failed", s.log.Warn("signature validation failed",
"host", req.Source.Host, "host", req.Source.Host,
@@ -46,10 +47,13 @@ func (s *Handlers) HandleImage() http.HandlerFunc {
} }
// The signature covers the format auto, not the format chosen // The signature covers the format auto, not the format chosen
if !s.chooseAutoFormat(w, r, req) { format, ok := s.chooseAutoFormat(w, r, req.Transform.Format)
if !ok {
return return
} }
req.Transform.Format = format
// Get cache key for logging // Get cache key for logging
cacheKey := imagerequest.CacheKey(req) cacheKey := imagerequest.CacheKey(req)
@@ -65,7 +69,7 @@ func (s *Handlers) HandleImage() http.HandlerFunc {
defer func() { _ = resp.Content.Close() }() defer func() { _ = resp.Content.Close() }()
s.writeImageResponse(w, r, req, resp, cacheKey, startTime) s.writeImageResponse(w, r, req, expires, resp, cacheKey, startTime)
} }
} }
@@ -82,11 +86,11 @@ func (s *Handlers) HandleRobotsTxt() http.HandlerFunc {
} }
// parseImageRequest parses the image URL's path and query parameters into // parseImageRequest parses the image URL's path and query parameters into
// an image request. On invalid input it writes an error response and // an image request, and returns it with the URL's signature and expiry. On
// returns false. // invalid input it writes an error response and returns false.
func (s *Handlers) parseImageRequest( func (s *Handlers) parseImageRequest(
w http.ResponseWriter, r *http.Request, w http.ResponseWriter, r *http.Request,
) (*imagerequest.Request, bool) { ) (*imagerequest.Request, string, time.Time, bool) {
// The path is parsed decoded once, as r.URL.Path holds it, and not from // The path is parsed decoded once, as r.URL.Path holds it, and not from
// chi's wildcard: chi routes on the path as the client wrote it whenever // chi's wildcard: chi routes on the path as the client wrote it whenever
// that is not Go's own encoding of it, so its wildcard would still hold // that is not Go's own encoding of it, so its wildcard would still hold
@@ -100,7 +104,7 @@ func (s *Handlers) parseImageRequest(
) )
s.respondError(w, "invalid image URL: "+err.Error(), http.StatusBadRequest) s.respondError(w, "invalid image URL: "+err.Error(), http.StatusBadRequest)
return nil, false return nil, "", time.Time{}, false
} }
// Parse signature params from query string. r.URL.Query() would silently // Parse signature params from query string. r.URL.Query() would silently
@@ -113,7 +117,7 @@ func (s *Handlers) parseImageRequest(
s.respondError(w, fmt.Sprintf("invalid query string %q: %v", s.respondError(w, fmt.Sprintf("invalid query string %q: %v",
r.URL.RawQuery, err), http.StatusBadRequest) r.URL.RawQuery, err), http.StatusBadRequest)
return nil, false return nil, "", time.Time{}, false
} }
for name, values := range query { for name, values := range query {
@@ -121,17 +125,15 @@ func (s *Handlers) parseImageRequest(
s.respondError(w, fmt.Sprintf("invalid %s: given more than once", s.respondError(w, fmt.Sprintf("invalid %s: given more than once",
name), http.StatusBadRequest) name), http.StatusBadRequest)
return nil, false return nil, "", time.Time{}, false
} }
} }
req.Signature = query.Get("sig") expires, err := parseExpires(query)
req.Expires, err = parseExpires(query)
if err != nil { if err != nil {
s.respondError(w, err.Error(), http.StatusBadRequest) s.respondError(w, err.Error(), http.StatusBadRequest)
return nil, false return nil, "", time.Time{}, false
} }
// Parse optional quality and fit params. Only a q missing from the URL is // Parse optional quality and fit params. Only a q missing from the URL is
@@ -143,7 +145,7 @@ func (s *Handlers) parseImageRequest(
s.respondError(w, `invalid q: not a number, got ""`, s.respondError(w, `invalid q: not a number, got ""`,
http.StatusBadRequest) http.StatusBadRequest)
return nil, false return nil, "", time.Time{}, false
} }
req.Transform.Quality, err = parseFormInt(query, "q", req.Transform.Quality, err = parseFormInt(query, "q",
@@ -152,7 +154,7 @@ func (s *Handlers) parseImageRequest(
s.respondError(w, fmt.Sprintf("%v, got %q", err, qStr), s.respondError(w, fmt.Sprintf("%v, got %q", err, qStr),
http.StatusBadRequest) http.StatusBadRequest)
return nil, false return nil, "", time.Time{}, false
} }
// Only a fit missing from the URL is cover. A fit in the URL that is not a // Only a fit missing from the URL is cover. A fit in the URL that is not a
@@ -162,7 +164,7 @@ func (s *Handlers) parseImageRequest(
if query.Has("fit") && req.Transform.FitMode == "" { if query.Has("fit") && req.Transform.FitMode == "" {
s.respondError(w, `invalid fit: not a fit mode, got ""`, http.StatusBadRequest) s.respondError(w, `invalid fit: not a fit mode, got ""`, http.StatusBadRequest)
return nil, false return nil, "", time.Time{}, false
} }
// Default fit mode if not set // Default fit mode if not set
@@ -179,10 +181,35 @@ func (s *Handlers) parseImageRequest(
s.respondError(w, "invalid image request: "+err.Error(), s.respondError(w, "invalid image request: "+err.Error(),
http.StatusBadRequest) http.StatusBadRequest)
return nil, false return nil, "", time.Time{}, false
} }
return req, true return req, query.Get("sig"), expires, true
}
// validateSignature returns nil when the source host of req is on
// allowlist_hosts and names no port. Otherwise it verifies sig, the URL's
// signature of req, and expires, the URL's expiry.
func (s *Handlers) validateSignature(
req *imagerequest.Request, sig string, expires time.Time,
) error {
// An allowlist entry names no port, so a host followed by ":", with or
// without a port, needs a signature. Go's URL type reads the port as the
// fetcher does: in an IPv6 address written without brackets, such as
// 2001:db8::1:8443, it is what follows the last ":". This is checked here
// and not in allowlist.IsAllowed, which also matches referer_blocklist,
// where a Referer must be blocked whatever its port. Port and IsAllowed
// read only the host of a URL.
sourceURL := &url.URL{Host: req.Source.Host}
hostHasPort := sourceURL.Port() != "" ||
strings.HasSuffix(sourceURL.Host, ":")
if !hostHasPort && s.allowlist.IsAllowed(sourceURL) {
return nil
}
// Signature required for non-allowed hosts
return s.signer.Verify(req, sig, expires)
} }
// parseExpires reads the exp query parameter, a Unix time in seconds. An exp // parseExpires reads the exp query parameter, a Unix time in seconds. An exp
@@ -341,10 +368,11 @@ func notModified(w http.ResponseWriter, r *http.Request, etag string) bool {
} }
// writeImageResponse writes headers and streams the image content, // writeImageResponse writes headers and streams the image content,
// handling conditional and HEAD requests. // handling conditional and HEAD requests. expires is the URL's expiry, which
// ends the Cache-Control max-age.
func (s *Handlers) writeImageResponse( func (s *Handlers) writeImageResponse(
w http.ResponseWriter, r *http.Request, w http.ResponseWriter, r *http.Request,
req *imagerequest.Request, resp *imgcache.ImageResponse, req *imagerequest.Request, expires time.Time, resp *imgcache.ImageResponse,
cacheKey imagerequest.VariantKey, startTime time.Time, cacheKey imagerequest.VariantKey, startTime time.Time,
) { ) {
// Set response headers // Set response headers
@@ -355,7 +383,7 @@ func (s *Handlers) writeImageResponse(
} }
// Cache control headers // Cache control headers
w.Header().Set("Cache-Control", cacheControl(req.Expires)) w.Header().Set("Cache-Control", cacheControl(expires))
w.Header().Set("X-Pixa-Cache", string(resp.CacheStatus)) w.Header().Set("X-Pixa-Cache", string(resp.CacheStatus))
if notModified(w, r, resp.ETag) { if notModified(w, r, resp.ETag) {
@@ -14,10 +14,12 @@ import (
"github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5"
"github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus"
"sneak.berlin/go/pixa/internal/allowlist"
"sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/imageprocessor" "sneak.berlin/go/pixa/internal/imageprocessor"
"sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imagerequest"
"sneak.berlin/go/pixa/internal/imgcache" "sneak.berlin/go/pixa/internal/imgcache"
"sneak.berlin/go/pixa/internal/signature"
) )
// photoPath is the path of the JPEG that newSignedHostServer serves. // photoPath is the path of the JPEG that newSignedHostServer serves.
@@ -52,7 +54,6 @@ func newSignedHostServer(
MaxInputBytes: 50 << 20, MaxInputBytes: 50 << 20,
MaxConcurrentProcessing: 2, MaxConcurrentProcessing: 2,
}), }),
SigningKey: testSigningKey,
MaxResponseSize: 50 << 20, MaxResponseSize: 50 << 20,
Logger: log, Logger: log,
}) })
@@ -69,6 +70,8 @@ func newSignedHostServer(
log: log, log: log,
imgSvc: svc, imgSvc: svc,
encGen: encGen, encGen: encGen,
signer: signature.New(testSigningKey),
allowlist: allowlist.New(nil),
} }
r := chi.NewRouter() r := chi.NewRouter()
@@ -202,8 +205,7 @@ func TestHandleImageEnc_MaxAge(t *testing.T) {
Size: imagerequest.Size{Width: 50, Height: 50}, Size: imagerequest.Size{Width: 50, Height: 50},
Format: imagerequest.FormatJPEG, Format: imagerequest.FormatJPEG,
}, },
Expires: tt.expires, }, tt.expires)
})
if err != nil { if err != nil {
t.Fatalf("Generate() error = %v", err) t.Fatalf("Generate() error = %v", err)
} }
+26 -20
View File
@@ -14,6 +14,7 @@ import (
"github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5"
"github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus"
"sneak.berlin/go/pixa/internal/allowlist"
"sneak.berlin/go/pixa/internal/httpfetcher" "sneak.berlin/go/pixa/internal/httpfetcher"
"sneak.berlin/go/pixa/internal/imageprocessor" "sneak.berlin/go/pixa/internal/imageprocessor"
"sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imagerequest"
@@ -26,7 +27,7 @@ import (
const allowlistedHost = "allowed.example.com" const allowlistedHost = "allowed.example.com"
// newImageRoute returns the image route of a Handlers whose service fetches // newImageRoute returns the image route of a Handlers whose service fetches
// with fetcher and checks signatures with testSigningKey. // with fetcher, and which checks signatures with testSigningKey.
func newImageRoute(t *testing.T, fetcher httpfetcher.Fetcher) http.Handler { func newImageRoute(t *testing.T, fetcher httpfetcher.Fetcher) http.Handler {
t.Helper() t.Helper()
@@ -46,15 +47,18 @@ func newImageRoute(t *testing.T, fetcher httpfetcher.Fetcher) http.Handler {
MaxInputBytes: 50 << 20, MaxInputBytes: 50 << 20,
MaxConcurrentProcessing: 2, MaxConcurrentProcessing: 2,
}), }),
SigningKey: testSigningKey,
Allowlist: []string{allowlistedHost},
MaxResponseSize: 50 << 20, MaxResponseSize: 50 << 20,
}) })
if err != nil { if err != nil {
t.Fatalf("failed to create service: %v", err) t.Fatalf("failed to create service: %v", err)
} }
h := &Handlers{imgSvc: svc, log: slog.New(slog.DiscardHandler)} h := &Handlers{
imgSvc: svc,
log: slog.New(slog.DiscardHandler),
signer: signature.New(testSigningKey),
allowlist: allowlist.New([]string{allowlistedHost}),
}
r := chi.NewRouter() r := chi.NewRouter()
r.Get("/v1/image/*", h.HandleImage()) r.Get("/v1/image/*", h.HandleImage())
@@ -90,10 +94,10 @@ func photoURLWithSig(host, sig string, expires time.Time) string {
return fmt.Sprintf("%s?sig=%s&exp=%d", photoURL(host), sig, expires.Unix()) return fmt.Sprintf("%s?sig=%s&exp=%d", photoURL(host), sig, expires.Unix())
} }
// photoSignature returns the signature of photoURL(host) at the default // photoRequest returns the image request of photoURL(host) at the default
// quality and fit, made with key and expiring at expires. // quality and fit.
func photoSignature(key, host string, expires time.Time) string { func photoRequest(host string) *imagerequest.Request {
return signature.New(key).Sign(&imagerequest.Request{ return &imagerequest.Request{
Source: imagerequest.Source{ Source: imagerequest.Source{
Host: host, Host: host,
Path: photoPath, Path: photoPath,
@@ -104,8 +108,13 @@ func photoSignature(key, host string, expires time.Time) string {
Quality: 85, Quality: 85,
FitMode: imagerequest.FitCover, FitMode: imagerequest.FitCover,
}, },
Expires: expires, }
}) }
// photoSignature returns the signature of photoURL(host) at the default
// quality and fit, made with key and expiring at expires.
func photoSignature(key, host string, expires time.Time) string {
return signature.New(key).Sign(photoRequest(host), expires)
} }
// sendGet sends a GET for target to route and returns the response. // sendGet sends a GET for target to route and returns the response.
@@ -286,10 +295,9 @@ func TestHandleImage_AllowlistOrSignature(t *testing.T) {
// host can be, and checks that each is refused with 400 naming the host as // host can be, and checks that each is refused with 400 naming the host as
// decoded: allowlistedHost with a character after it that a URL host cannot // decoded: allowlistedHost with a character after it that a URL host cannot
// hold, percent-encoded, an IPv6 address without brackets, and brackets // hold, percent-encoded, an IPv6 address without brackets, and brackets
// around something that is not an IPv6 address. A "?" or "#" after // around something that is not an IPv6 address. Later steps put the host
// allowlistedHost would otherwise end the host when the source URL is read, // unparsed into the allowlist check, the cache key and the upstream URL, so
// so the request would pass the allowlist and be fetched and cached under a // such a host is refused first.
// host that is not the one fetched.
func TestHandleImage_InvalidHost_Returns400(t *testing.T) { func TestHandleImage_InvalidHost_Returns400(t *testing.T) {
t.Parallel() t.Parallel()
@@ -360,10 +368,9 @@ func TestHandleImage_HostForms_Served(t *testing.T) {
// TestHandleImage_HashInSourcePathOrQuery_Returns400 sends image URLs whose // TestHandleImage_HashInSourcePathOrQuery_Returns400 sends image URLs whose
// source path or source query holds a "#", written as %23, and checks that // source path or source query holds a "#", written as %23, and checks that
// each is refused with 400, the one signed for that source path included. The // each is refused with 400, the one signed for that source path included.
// source URL would otherwise end at the "#" when it is fetched, so each text // A "#" starts a URL's fragment, so neither the source path nor the source
// after it would fetch the image at photoPath and cache it under keys of its // query may hold one.
// own.
func TestHandleImage_HashInSourcePathOrQuery_Returns400(t *testing.T) { func TestHandleImage_HashInSourcePathOrQuery_Returns400(t *testing.T) {
t.Parallel() t.Parallel()
@@ -381,8 +388,7 @@ func TestHandleImage_HashInSourcePathOrQuery_Returns400(t *testing.T) {
Quality: 85, Quality: 85,
FitMode: imagerequest.FitCover, FitMode: imagerequest.FitCover,
}, },
Expires: exp, }, exp)
})
tests := []struct { tests := []struct {
name string name string
@@ -38,40 +38,44 @@ func getImage(t *testing.T, fix *testFixtures, target string) int {
} }
// generateSignedURL returns the /v1/image/ URL of req, signed with key and // generateSignedURL returns the /v1/image/ URL of req, signed with key and
// expiring after ttl, and sets req's expiry. An unset quality or fit of req is // expiring after ttl. It signs a copy of req, so req itself is not changed.
// first set to 85 or cover, the values the route uses when a URL has no q or // An unset quality or fit in the copy is set to 85 or cover, the values the
// fit. The URL carries q and fit next to sig and exp, so the route verifies it // route uses when a URL has no q or fit. The URL carries q and fit next to sig
// for the quality and fit it was signed with. // and exp, so the route verifies it for the quality and fit it was signed
// with.
func generateSignedURL( func generateSignedURL(
key string, req *imagerequest.Request, ttl time.Duration, key string, req *imagerequest.Request, ttl time.Duration,
) string { ) string {
if req.Transform.Quality == 0 { signed := *req
req.Transform.Quality = 85
if signed.Transform.Quality == 0 {
signed.Transform.Quality = 85
} }
if req.Transform.FitMode == "" { if signed.Transform.FitMode == "" {
req.Transform.FitMode = imagerequest.FitCover signed.Transform.FitMode = imagerequest.FitCover
} }
req.Expires = time.Now().Add(ttl) expires := time.Now().Add(ttl)
sig := signature.New(key).Sign(req) sig := signature.New(key).Sign(&signed, expires)
// A source query follows the source path after a "?". The image route // A source query follows the source path after a "?". The image route
// reads the path decoded once, so the whole path is written in Go's own // reads the path decoded once, so the whole path is written in Go's own
// percent-encoding of a URL path, which writes "?" as %3F and "%" as %25, // percent-encoding of a URL path, which writes "?" as %3F and "%" as %25,
// and leaves "/", ";", ",", "=" and "&" as they are. // and leaves "/", ";", ",", "=" and "&" as they are.
sourcePathAndQuery := req.Source.Path sourcePathAndQuery := signed.Source.Path
if req.Source.Query != "" { if signed.Source.Query != "" {
sourcePathAndQuery += "?" + req.Source.Query sourcePathAndQuery += "?" + signed.Source.Query
} }
unescapedPath := fmt.Sprintf("/v1/image/%s%s/%dx%d.%s", unescapedPath := fmt.Sprintf("/v1/image/%s%s/%dx%d.%s",
req.Source.Host, sourcePathAndQuery, signed.Source.Host, sourcePathAndQuery,
req.Transform.Size.Width, req.Transform.Size.Height, req.Transform.Format) signed.Transform.Size.Width, signed.Transform.Size.Height,
signed.Transform.Format)
return fmt.Sprintf("%s?sig=%s&exp=%d&q=%d&fit=%s", return fmt.Sprintf("%s?sig=%s&exp=%d&q=%d&fit=%s",
(&url.URL{Path: unescapedPath}).EscapedPath(), sig, req.Expires.Unix(), (&url.URL{Path: unescapedPath}).EscapedPath(), sig, expires.Unix(),
req.Transform.Quality, req.Transform.FitMode) signed.Transform.Quality, signed.Transform.FitMode)
} }
// TestHandleImage_SignatureCoversQualityAndFit signs a URL for quality 85 // TestHandleImage_SignatureCoversQualityAndFit signs a URL for quality 85
@@ -93,8 +97,7 @@ func TestHandleImage_SignatureCoversQualityAndFit(t *testing.T) {
Quality: 85, Quality: 85,
FitMode: imagerequest.FitCover, FitMode: imagerequest.FitCover,
}, },
Expires: expires, }, expires)
})
signedURL := fmt.Sprintf("/v1/image/%s/images/photo.jpg/50x50.jpeg?sig=%s&exp=%d", signedURL := fmt.Sprintf("/v1/image/%s/images/photo.jpg/50x50.jpeg?sig=%s&exp=%d",
signedHost, sig, expires.Unix()) signedHost, sig, expires.Unix())
@@ -277,8 +280,7 @@ func TestHandleImage_SourceQueryEncodedAnyWay(t *testing.T) {
Quality: 85, Quality: 85,
FitMode: imagerequest.FitCover, FitMode: imagerequest.FitCover,
}, },
Expires: expires, }, expires)
})
tests := []struct { tests := []struct {
name string name string
@@ -344,8 +346,7 @@ func TestHandleImage_SourceURLHoldingPercent25Verifies(t *testing.T) {
Quality: 85, Quality: 85,
FitMode: imagerequest.FitCover, FitMode: imagerequest.FitCover,
}, },
Expires: expires, }, expires)
})
target := fmt.Sprintf("/v1/image/%s%s/50x50.jpeg?sig=%s&exp=%d", target := fmt.Sprintf("/v1/image/%s%s/50x50.jpeg?sig=%s&exp=%d",
signedHost, tt.writtenPathAndQuery, sig, expires.Unix()) signedHost, tt.writtenPathAndQuery, sig, expires.Unix())
+20 -13
View File
@@ -31,11 +31,18 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc {
ctx := r.Context() ctx := r.Context()
start := time.Now() start := time.Now()
req, ok := s.parseImageEncRequest(w, r) req, expires, ok := s.parseImageEncRequest(w, r)
if !ok || !s.chooseAutoFormat(w, r, req) { if !ok {
return return
} }
format, ok := s.chooseAutoFormat(w, r, req.Transform.Format)
if !ok {
return
}
req.Transform.Format = format
// Log the request // Log the request
s.log.Debug("encrypted image request", s.log.Debug("encrypted image request",
"host", req.Source.Host, "host", req.Source.Host,
@@ -64,7 +71,7 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc {
} }
// Cache headers: max-age ends at the URL's expiry // Cache headers: max-age ends at the URL's expiry
w.Header().Set("Cache-Control", cacheControl(req.Expires)) w.Header().Set("Cache-Control", cacheControl(expires))
w.Header().Set("X-Pixa-Cache", string(resp.CacheStatus)) w.Header().Set("X-Pixa-Cache", string(resp.CacheStatus))
if notModified(w, r, resp.ETag) { if notModified(w, r, resp.ETag) {
@@ -102,34 +109,34 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc {
} }
// parseImageEncRequest decrypts the token of an encrypted image URL into an // parseImageEncRequest decrypts the token of an encrypted image URL into an
// image request and checks it. On a token that is missing, does not decrypt, // image request and checks it, and returns it with the token's expiry. On a
// has expired or asks for something not valid, it writes an error response // token that is missing, does not decrypt, has expired or asks for something
// and returns false. // not valid, it writes an error response and returns false.
func (s *Handlers) parseImageEncRequest( func (s *Handlers) parseImageEncRequest(
w http.ResponseWriter, r *http.Request, w http.ResponseWriter, r *http.Request,
) (*imagerequest.Request, bool) { ) (*imagerequest.Request, time.Time, bool) {
// Extract token from URL // Extract token from URL
token := chi.URLParam(r, "token") token := chi.URLParam(r, "token")
if token == "" { if token == "" {
s.respondError(w, "missing token", http.StatusBadRequest) s.respondError(w, "missing token", http.StatusBadRequest)
return nil, false return nil, time.Time{}, false
} }
// Decrypt and validate the payload // Decrypt and validate the payload
req, err := s.encGen.Parse(token) req, expires, err := s.encGen.Parse(token)
if err != nil { if err != nil {
if errors.Is(err, encurl.ErrExpired) { if errors.Is(err, encurl.ErrExpired) {
s.log.Debug("encrypted URL expired", "error", err) s.log.Debug("encrypted URL expired", "error", err)
s.respondError(w, "URL has expired", http.StatusGone) s.respondError(w, "URL has expired", http.StatusGone)
return nil, false return nil, time.Time{}, false
} }
s.log.Debug("failed to decrypt URL", "error", err) s.log.Debug("failed to decrypt URL", "error", err)
s.respondError(w, "invalid encrypted URL", http.StatusBadRequest) s.respondError(w, "invalid encrypted URL", http.StatusBadRequest)
return nil, false return nil, time.Time{}, false
} }
// Apply the same dimension and fit-mode bounds as the plain image // Apply the same dimension and fit-mode bounds as the plain image
@@ -142,10 +149,10 @@ func (s *Handlers) parseImageEncRequest(
s.respondError(w, "invalid encrypted URL: "+err.Error(), s.respondError(w, "invalid encrypted URL: "+err.Error(),
http.StatusBadRequest) http.StatusBadRequest)
return nil, false return nil, time.Time{}, false
} }
return req, true return req, expires, true
} }
// handleImageError converts image service errors to HTTP responses. // handleImageError converts image service errors to HTTP responses.
+3 -2
View File
@@ -6,6 +6,7 @@ import (
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"testing" "testing"
"time"
"github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5"
@@ -62,7 +63,7 @@ func TestHandleImageEnc_OverLimitDimension_Returns400(t *testing.T) {
Transform: imagerequest.Transform{ Transform: imagerequest.Transform{
Size: imagerequest.Size{Width: 100000, Height: 100000}, Size: imagerequest.Size{Width: 100000, Height: 100000},
}, },
}) }, time.Time{})
if err != nil { if err != nil {
t.Fatalf("Generate() error = %v", err) t.Fatalf("Generate() error = %v", err)
} }
@@ -91,7 +92,7 @@ func TestHandleImageEnc_InvalidFitMode_Returns400(t *testing.T) {
Size: imagerequest.Size{Width: 800, Height: 600}, Size: imagerequest.Size{Width: 800, Height: 600},
FitMode: imagerequest.Fit("bogus"), FitMode: imagerequest.Fit("bogus"),
}, },
}) }, time.Time{})
if err != nil { if err != nil {
t.Fatalf("Generate() error = %v", err) t.Fatalf("Generate() error = %v", err)
} }
@@ -70,15 +70,12 @@ func TestHandleImageEnc_RejectedToken(t *testing.T) {
}, },
} }
valid, err := h.encGen.Generate(&photo) valid, err := h.encGen.Generate(&photo, time.Time{})
if err != nil { if err != nil {
t.Fatalf("Generate() error = %v", err) t.Fatalf("Generate() error = %v", err)
} }
expiredPhoto := photo expired, err := h.encGen.Generate(&photo, time.Now().Add(-time.Minute))
expiredPhoto.Expires = time.Now().Add(-time.Minute)
expired, err := h.encGen.Generate(&expiredPhoto)
if err != nil { if err != nil {
t.Fatalf("Generate() error = %v", err) t.Fatalf("Generate() error = %v", err)
} }
@@ -88,7 +85,7 @@ func TestHandleImageEnc_RejectedToken(t *testing.T) {
t.Fatalf("encurl.NewGenerator() error = %v", err) t.Fatalf("encurl.NewGenerator() error = %v", err)
} }
otherKey, err := otherGen.Generate(&photo) otherKey, err := otherGen.Generate(&photo, time.Time{})
if err != nil { if err != nil {
t.Fatalf("Generate() error = %v", err) t.Fatalf("Generate() error = %v", err)
} }
@@ -69,8 +69,7 @@ func TestImageWithoutFormat_SignedAsJXL(t *testing.T) {
Quality: encurl.DefaultQuality, Quality: encurl.DefaultQuality,
FitMode: imagerequest.FitCover, FitMode: imagerequest.FitCover,
}, },
Expires: expires, }, expires)
})
query := fmt.Sprintf("?sig=%s&exp=%d", sig, expires.Unix()) query := fmt.Sprintf("?sig=%s&exp=%d", sig, expires.Unix())
for _, size := range []string{"50x50.jxl", "50x50"} { for _, size := range []string{"50x50.jxl", "50x50"} {
@@ -94,7 +93,7 @@ func TestImageEncWithoutFormat_ServesJPEGXL(t *testing.T) {
Transform: imagerequest.Transform{ Transform: imagerequest.Transform{
Size: imagerequest.Size{Width: 50, Height: 50}, Size: imagerequest.Size{Width: 50, Height: 50},
}, },
}) }, time.Time{})
if err != nil { if err != nil {
t.Fatalf("Generate() error = %v", err) t.Fatalf("Generate() error = %v", err)
} }
@@ -15,7 +15,9 @@ import (
"sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/httpfetcher" "sneak.berlin/go/pixa/internal/httpfetcher"
"sneak.berlin/go/pixa/internal/imageprocessor" "sneak.berlin/go/pixa/internal/imageprocessor"
"sneak.berlin/go/pixa/internal/imagerequest"
"sneak.berlin/go/pixa/internal/imgcache" "sneak.berlin/go/pixa/internal/imgcache"
"sneak.berlin/go/pixa/internal/signature"
) )
// blockedReferer is a page on leech.example, which newRefererRoutes puts on // blockedReferer is a page on leech.example, which newRefererRoutes puts on
@@ -31,11 +33,11 @@ type countingFetcher struct {
// Fetch counts the fetch and passes it on. // Fetch counts the fetch and passes it on.
func (f *countingFetcher) Fetch( func (f *countingFetcher) Fetch(
ctx context.Context, url string, ctx context.Context, source imagerequest.Source,
) (*httpfetcher.FetchResult, error) { ) (*httpfetcher.FetchResult, error) {
f.fetches.Add(1) f.fetches.Add(1)
return f.Fetcher.Fetch(ctx, url) return f.Fetcher.Fetch(ctx, source)
} }
// newRefererRoutes returns both image routes of a Handlers whose // newRefererRoutes returns both image routes of a Handlers whose
@@ -65,8 +67,6 @@ func newRefererRoutes(t *testing.T) (http.Handler, *Handlers, *countingFetcher)
MaxInputBytes: 50 << 20, MaxInputBytes: 50 << 20,
MaxConcurrentProcessing: 2, MaxConcurrentProcessing: 2,
}), }),
SigningKey: testSigningKey,
Allowlist: []string{allowlistedHost},
MaxResponseSize: 50 << 20, MaxResponseSize: 50 << 20,
}) })
if err != nil { if err != nil {
@@ -82,6 +82,8 @@ func newRefererRoutes(t *testing.T) (http.Handler, *Handlers, *countingFetcher)
log: slog.New(slog.DiscardHandler), log: slog.New(slog.DiscardHandler),
imgSvc: svc, imgSvc: svc,
encGen: encGen, encGen: encGen,
signer: signature.New(testSigningKey),
allowlist: allowlist.New([]string{allowlistedHost}),
refererBlocklist: allowlist.New( refererBlocklist: allowlist.New(
[]string{"leech.example", ".hotlinker.example"}), []string{"leech.example", ".hotlinker.example"}),
} }
@@ -49,7 +49,7 @@ func encPhotoURL(t *testing.T, h *Handlers) string {
Size: imagerequest.Size{Width: 50, Height: 50}, Size: imagerequest.Size{Width: 50, Height: 50},
Format: imagerequest.FormatJPEG, Format: imagerequest.FormatJPEG,
}, },
}) }, time.Time{})
if err != nil { if err != nil {
t.Fatalf("Generate() error = %v", err) t.Fatalf("Generate() error = %v", err)
} }
@@ -0,0 +1,211 @@
package handlers
import (
"testing"
"time"
"sneak.berlin/go/pixa/internal/allowlist"
"sneak.berlin/go/pixa/internal/signature"
)
// Addresses the port tests below put on the allowlist.
const (
allowlistedIPv4 = "203.0.113.5"
allowlistedIPv6 = "2001:db8::1"
)
// newSignatureChecker returns Handlers that check signatures with
// testSigningKey and have allowlistHosts on their allowlist, as New builds
// them from signing_key and allowlist_hosts.
func newSignatureChecker(allowlistHosts ...string) *Handlers {
return &Handlers{
signer: signature.New(testSigningKey),
allowlist: allowlist.New(allowlistHosts),
}
}
func TestValidateSignature_NonAllowlistedHost_NoSignature(t *testing.T) {
t.Parallel()
h := newSignatureChecker(allowlistedHost)
// Should fail validation - not allowlisted and no signature
err := h.validateSignature(photoRequest(signedHost), "", time.Time{})
if err == nil {
t.Error("validateSignature() expected error for non-allowlisted " +
"host without signature")
}
}
func TestValidateSignature_NonAllowlistedHost_ValidSignature(t *testing.T) {
t.Parallel()
h := newSignatureChecker(allowlistedHost)
req := photoRequest(signedHost)
// Generate a valid signature
expires := time.Now().Add(time.Hour)
sig := signature.New(testSigningKey).Sign(req, expires)
// Should pass validation
err := h.validateSignature(req, sig, expires)
if err != nil {
t.Errorf("validateSignature() error = %v", err)
}
}
func TestValidateSignature_NonAllowlistedHost_ExpiredSignature(t *testing.T) {
t.Parallel()
h := newSignatureChecker(allowlistedHost)
req := photoRequest(signedHost)
// Generate an expired signature
expires := time.Now().Add(-time.Hour)
sig := signature.New(testSigningKey).Sign(req, expires)
// Should fail validation
err := h.validateSignature(req, sig, expires)
if err == nil {
t.Error("validateSignature() expected error for expired signature")
}
}
func TestValidateSignature_NonAllowlistedHost_InvalidSignature(t *testing.T) {
t.Parallel()
h := newSignatureChecker(allowlistedHost)
// Should fail validation
err := h.validateSignature(photoRequest(signedHost), "invalid-signature",
time.Now().Add(time.Hour))
if err == nil {
t.Error("validateSignature() expected error for invalid signature")
}
}
// TestValidateSignature_NoAllowlist checks that with no host on the
// allowlist, a request without a signature fails validation.
func TestValidateSignature_NoAllowlist(t *testing.T) {
t.Parallel()
h := newSignatureChecker()
err := h.validateSignature(photoRequest(signedHost), "", time.Time{})
if err == nil {
t.Error("validateSignature() expected error when no host is allowlisted")
}
}
// TestValidateSignature_SignatureExactHostMatch verifies that
// validateSignature enforces exact host matching for signatures. A
// signature for one host must not verify for a different host, even
// if they share a domain suffix.
func TestValidateSignature_SignatureExactHostMatch(t *testing.T) {
t.Parallel()
h := newSignatureChecker()
// Sign a request for signedHost
signedReq := photoRequest(signedHost)
expires := time.Now().Add(time.Hour)
sig := signature.New(testSigningKey).Sign(signedReq, expires)
// The original request should pass validation
t.Run("exact host passes", func(t *testing.T) {
t.Parallel()
err := h.validateSignature(signedReq, sig, expires)
if err != nil {
t.Errorf("validateSignature() exact host failed: %v", err)
}
})
// Try to reuse the signature with different hosts
tests := []struct {
name string
host string
}{
{"parent domain", "example.com"},
{"sibling subdomain", "images.example.com"},
{"deeper subdomain", "a." + signedHost},
{"evil suffix domain", signedHost + ".evil.com"},
{"prefixed host", "evil" + signedHost},
}
for _, tt := range tests {
t.Run(tt.name+" rejected", func(t *testing.T) {
t.Parallel()
err := h.validateSignature(photoRequest(tt.host), sig, expires)
if err == nil {
t.Errorf("validateSignature() should reject signature for "+
"host %q (signed for %q)", tt.host, signedHost)
}
})
}
}
// TestValidateSignature_HostWithPortNeedsSignature verifies that a host
// followed by a port, or by a colon with no port, matches no allowlist entry,
// so that a request for it without a signature is refused, while the same
// host with no port is allowlisted.
func TestValidateSignature_HostWithPortNeedsSignature(t *testing.T) {
t.Parallel()
h := newSignatureChecker(allowlistedHost, allowlistedIPv4, allowlistedIPv6)
tests := []struct {
host string
allowlisted bool
}{
{allowlistedHost, true},
{allowlistedHost + ":8443", false},
{allowlistedHost + ":443", false},
{allowlistedHost + ":", false},
{allowlistedIPv4, true},
{allowlistedIPv4 + ":8080", false},
{"[" + allowlistedIPv6 + "]", true},
{"[" + allowlistedIPv6 + "]:8443", false},
}
for _, tt := range tests {
t.Run(tt.host, func(t *testing.T) {
t.Parallel()
err := h.validateSignature(photoRequest(tt.host), "", time.Time{})
t.Logf("validateSignature() for host %q with no signature: %v",
tt.host, err)
if allowlisted := err == nil; allowlisted != tt.allowlisted {
t.Errorf("host %q served without a signature = %v, want %v",
tt.host, allowlisted, tt.allowlisted)
}
})
}
}
// TestValidateSignature_UnbracketedIPv6PortNeedsSignature verifies that an
// allowlisted IPv6 address written without brackets and followed by a port,
// or by a colon with no port, needs a signature. Go's URL type reads
// 2001:db8::1:8443 as the host 2001:db8::1 with the port 8443.
func TestValidateSignature_UnbracketedIPv6PortNeedsSignature(t *testing.T) {
t.Parallel()
h := newSignatureChecker(allowlistedIPv6)
for _, host := range []string{allowlistedIPv6 + ":8443", allowlistedIPv6 + ":"} {
t.Run(host, func(t *testing.T) {
t.Parallel()
err := h.validateSignature(photoRequest(host), "", time.Time{})
t.Logf("validateSignature() for host %q with no signature: %v",
host, err)
if err == nil {
t.Errorf("host %q served without a signature, want refused",
host)
}
})
}
}
@@ -51,12 +51,12 @@ func TestDialContextReplacesOnlyTheDialer(t *testing.T) {
t.Errorf("body = %q, want %q", body, imagePayload) t.Errorf("body = %q, want %q", body, imagePayload)
} }
_, err := f.Fetch(testContext(t), "http://127.0.0.1/image") _, err := f.Fetch(testContext(t), imageSourceOn("127.0.0.1"))
if !errors.Is(err, ErrSSRFBlocked) { if !errors.Is(err, ErrSSRFBlocked) {
t.Errorf("Fetch(loopback URL) error = %v, want ErrSSRFBlocked", err) t.Errorf("Fetch(loopback URL) error = %v, want ErrSSRFBlocked", err)
} }
_, err = f.Fetch(testContext(t), upstreamURL("/redirect/private")) _, err = f.Fetch(testContext(t), upstreamSource("/redirect/private"))
if !errors.Is(err, ErrSSRFBlocked) { if !errors.Is(err, ErrSSRFBlocked) {
t.Errorf("Fetch(/redirect/private) error = %v, want ErrSSRFBlocked", err) t.Errorf("Fetch(/redirect/private) error = %v, want ErrSSRFBlocked", err)
} }
+58 -6
View File
@@ -14,6 +14,8 @@ import (
"sync" "sync"
"testing" "testing"
"time" "time"
"sneak.berlin/go/pixa/internal/imagerequest"
) )
// testPublicHost is a TEST-NET-1 (RFC 5737) literal. isPrivateIP treats it as // testPublicHost is a TEST-NET-1 (RFC 5737) literal. isPrivateIP treats it as
@@ -29,9 +31,9 @@ const imagePayload = "fake-jpeg-bytes"
// would mean SSRF protection let a forbidden target reach the transport. // would mean SSRF protection let a forbidden target reach the transport.
var errUnexpectedDial = errors.New("unexpected dial target") var errUnexpectedDial = errors.New("unexpected dial target")
// upstreamURL builds a fetch URL on the fake public host for the given path. // upstreamSource returns the source at path on the fake public host.
func upstreamURL(path string) string { func upstreamSource(path string) imagerequest.Source {
return "http://" + testPublicHost + path return imagerequest.Source{Host: testPublicHost, Path: path}
} }
// recordingDialer records every address the transport asks it to dial and // recordingDialer records every address the transport asks it to dial and
@@ -158,7 +160,7 @@ func testContext(t *testing.T) context.Context {
func fetchImage(t *testing.T, f *HTTPFetcher, path string) *FetchResult { func fetchImage(t *testing.T, f *HTTPFetcher, path string) *FetchResult {
t.Helper() t.Helper()
res, err := f.Fetch(testContext(t), upstreamURL(path)) res, err := f.Fetch(testContext(t), upstreamSource(path))
if err != nil { if err != nil {
t.Fatalf("Fetch(%s) error = %v", path, err) t.Fatalf("Fetch(%s) error = %v", path, err)
} }
@@ -170,7 +172,7 @@ func fetchImage(t *testing.T, f *HTTPFetcher, path string) *FetchResult {
func fetchExpectError(t *testing.T, f *HTTPFetcher, path string) error { func fetchExpectError(t *testing.T, f *HTTPFetcher, path string) error {
t.Helper() t.Helper()
res, err := f.Fetch(testContext(t), upstreamURL(path)) res, err := f.Fetch(testContext(t), upstreamSource(path))
if err == nil { if err == nil {
_ = res.Content.Close() _ = res.Content.Close()
@@ -222,7 +224,7 @@ func TestFetchRedirectToPrivateIPBlocked(t *testing.T) {
srv := startUpstream(t) srv := startUpstream(t)
f, dialer := newServerFetcher(t, srv, nil) f, dialer := newServerFetcher(t, srv, nil)
_, err := f.Fetch(testContext(t), upstreamURL("/redirect/private")) _, err := f.Fetch(testContext(t), upstreamSource("/redirect/private"))
if !errors.Is(err, ErrSSRFBlocked) { if !errors.Is(err, ErrSSRFBlocked) {
t.Fatalf("Fetch() error = %v, want ErrSSRFBlocked", err) t.Fatalf("Fetch() error = %v, want ErrSSRFBlocked", err)
} }
@@ -590,3 +592,53 @@ func TestSSRFSafeDialerAllowsPublicTarget(t *testing.T) {
t.Errorf("public target was SSRF-blocked: %v", err) t.Errorf("public target was SSRF-blocked: %v", err)
} }
} }
// TestFetchSchemeFollowsAllowHTTP checks that the fetcher fetches a source
// over https, on port 443, unless AllowHTTP is set, and then over http, on
// port 80. The test server speaks plain HTTP, so only the fetch over http
// succeeds.
func TestFetchSchemeFollowsAllowHTTP(t *testing.T) {
t.Parallel()
tests := []struct {
allowHTTP bool
wantAddr string
}{
{false, net.JoinHostPort(testPublicHost, "443")},
{true, net.JoinHostPort(testPublicHost, "80")},
}
for _, tt := range tests {
t.Run(fmt.Sprintf("AllowHTTP %v", tt.allowHTTP), func(t *testing.T) {
t.Parallel()
srv := startUpstream(t)
dialer := &recordingDialer{target: srv.Listener.Addr().String()}
f := New(&Config{
Timeout: 30 * time.Second,
MaxResponseSize: 50 << 20,
AllowHTTP: tt.allowHTTP,
MaxConnectionsPerHost: 20,
MaxConnections: 64,
DialContext: dialer.dialContext,
})
res, err := f.Fetch(testContext(t), upstreamSource("/image"))
if err == nil {
_ = res.Content.Close()
}
dialed := dialer.dialedAddrs()
t.Logf("Fetch(): error %v, dialed %v", err, dialed)
if served := err == nil; served != tt.allowHTTP {
t.Errorf("served = %v, want %v", served, tt.allowHTTP)
}
if len(dialed) != 1 || dialed[0] != tt.wantAddr {
t.Errorf("dialed %v, want only %s", dialed, tt.wantAddr)
}
})
}
}
+21 -44
View File
@@ -20,6 +20,7 @@ import (
"time" "time"
"github.com/go-chi/chi/v5/middleware" "github.com/go-chi/chi/v5/middleware"
"sneak.berlin/go/pixa/internal/imagerequest"
) )
// Fetcher configuration constants. // Fetcher configuration constants.
@@ -91,8 +92,8 @@ var (
// Fetcher retrieves content from upstream origins. // Fetcher retrieves content from upstream origins.
type Fetcher interface { type Fetcher interface {
// Fetch retrieves content from the given URL. // Fetch retrieves the content of the source image.
Fetch(ctx context.Context, url string) (*FetchResult, error) Fetch(ctx context.Context, source imagerequest.Source) (*FetchResult, error)
} }
// FetchResult contains the result of fetching from upstream. // FetchResult contains the result of fetching from upstream.
@@ -139,7 +140,8 @@ type Config struct {
Timeout time.Duration Timeout time.Duration
// MaxResponseSize is the maximum allowed response body size. // MaxResponseSize is the maximum allowed response body size.
MaxResponseSize int64 MaxResponseSize int64
// AllowHTTP allows non-TLS connections (for testing only). // AllowHTTP makes the fetcher fetch sources over http instead of https,
// and follow redirects to http (for testing only).
AllowHTTP bool AllowHTTP bool
// MaxConnectionsPerHost limits concurrent connections to each upstream host. // MaxConnectionsPerHost limits concurrent connections to each upstream host.
MaxConnectionsPerHost int MaxConnectionsPerHost int
@@ -211,7 +213,7 @@ func New(config *Config) *HTTPFetcher {
} }
// Validate the redirect target // Validate the redirect target
err := validateURL(req.Context(), req.URL.String(), config.AllowHTTP) err := validateURL(req.Context(), req.URL, config.AllowHTTP)
if err != nil { if err != nil {
return fmt.Errorf("redirect blocked: %w", err) return fmt.Errorf("redirect blocked: %w", err)
} }
@@ -229,18 +231,22 @@ func New(config *Config) *HTTPFetcher {
} }
} }
// Fetch retrieves content from the given URL with SSRF protection. When // Fetch retrieves the source image with SSRF protection, over https, or over
// MaxConnections fetches are already in progress, it waits up to // http when AllowHTTP is set. When MaxConnections fetches are already in
// ConnectionWaitTimeout for one to finish, then fails with // progress, it waits up to ConnectionWaitTimeout for one to finish, then
// ErrTooManyConnections. // fails with ErrTooManyConnections.
func (f *HTTPFetcher) Fetch(ctx context.Context, url string) (*FetchResult, error) { func (f *HTTPFetcher) Fetch(
ctx context.Context, source imagerequest.Source,
) (*FetchResult, error) {
sourceURL := source.URL(f.config.AllowHTTP)
// Validate URL before making request // Validate URL before making request
err := validateURL(ctx, url, f.config.AllowHTTP) err := validateURL(ctx, sourceURL, f.config.AllowHTTP)
if err != nil { if err != nil {
return nil, err return nil, err
} }
release, err := f.acquireConnection(ctx, extractHost(url)) release, err := f.acquireConnection(ctx, source.Host)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@@ -254,14 +260,9 @@ func (f *HTTPFetcher) Fetch(ctx context.Context, url string) (*FetchResult, erro
} }
}() }()
parsedURL, err := neturl.Parse(url)
if err != nil {
return nil, fmt.Errorf("failed to parse URL: %w", err)
}
req := &http.Request{ req := &http.Request{
Method: http.MethodGet, Method: http.MethodGet,
URL: parsedURL, URL: sourceURL,
Header: make(http.Header), Header: make(http.Header),
} }
@@ -455,19 +456,14 @@ func (f *HTTPFetcher) isAllowedContentType(contentType string) bool {
} }
// validateURL checks if a URL is safe to fetch (not internal/private). // validateURL checks if a URL is safe to fetch (not internal/private).
func validateURL(ctx context.Context, rawURL string, allowHTTP bool) error { func validateURL(ctx context.Context, fetchURL *neturl.URL, allowHTTP bool) error {
if !allowHTTP && !strings.HasPrefix(rawURL, "https://") { if !allowHTTP && fetchURL.Scheme != "https" {
return ErrUnsupportedScheme return ErrUnsupportedScheme
} }
parsedURL, err := neturl.Parse(rawURL)
if err != nil {
return fmt.Errorf("%w: %w", ErrInvalidHost, err)
}
// Hostname drops any port, and the brackets of an IPv6 address whether // Hostname drops any port, and the brackets of an IPv6 address whether
// or not a port follows them // or not a port follows them
host := parsedURL.Hostname() host := fetchURL.Hostname()
if host == "" { if host == "" {
return ErrInvalidHost return ErrInvalidHost
} }
@@ -493,25 +489,6 @@ func validateURL(ctx context.Context, rawURL string, allowHTTP bool) error {
return nil return nil
} }
// extractHost extracts the host from a URL string.
func extractHost(rawURL string) string {
// Simple extraction without full URL parsing
url := rawURL
if idx := strings.Index(url, "://"); idx != -1 {
url = url[idx+3:]
}
if idx := strings.Index(url, "/"); idx != -1 {
url = url[:idx]
}
if idx := strings.Index(url, "?"); idx != -1 {
url = url[:idx]
}
return url
}
// isLocalhost checks if the host is localhost. // isLocalhost checks if the host is localhost.
func isLocalhost(host string) bool { func isLocalhost(host string) bool {
host = strings.ToLower(host) host = strings.ToLower(host)
@@ -5,9 +5,12 @@ import (
"errors" "errors"
"io" "io"
"net" "net"
"net/url"
"testing" "testing"
"testing/fstest" "testing/fstest"
"time" "time"
"sneak.berlin/go/pixa/internal/imagerequest"
) )
// testHost is the hostname used by mock fetch tests. // testHost is the hostname used by mock fetch tests.
@@ -49,33 +52,6 @@ func TestIsAllowedContentType(t *testing.T) {
} }
} }
func TestExtractHost(t *testing.T) {
t.Parallel()
tests := []struct {
url string
want string
}{
{"https://example.com/path", testHost},
{"http://example.com:8080/path", "example.com:8080"},
{"https://example.com", testHost},
{"https://example.com?q=1", testHost},
{"example.com/path", testHost},
{"", ""},
}
for _, tc := range tests {
t.Run(tc.url, func(t *testing.T) {
t.Parallel()
got := extractHost(tc.url)
if got != tc.want {
t.Errorf("extractHost(%q) = %q, want %q", tc.url, got, tc.want)
}
})
}
}
func TestIsLocalhost(t *testing.T) { func TestIsLocalhost(t *testing.T) {
t.Parallel() t.Parallel()
@@ -188,7 +164,7 @@ func TestIsPrivateIPJudgesEmbeddedIPv4Address(t *testing.T) {
func TestValidateURL_RejectsNonHTTPS(t *testing.T) { func TestValidateURL_RejectsNonHTTPS(t *testing.T) {
t.Parallel() t.Parallel()
err := validateURL(t.Context(), "http://example.com/path", false) err := validateURL(t.Context(), &url.URL{Scheme: "http", Host: testHost}, false)
if !errors.Is(err, ErrUnsupportedScheme) { if !errors.Is(err, ErrUnsupportedScheme) {
t.Errorf("validateURL http = %v, want ErrUnsupportedScheme", err) t.Errorf("validateURL http = %v, want ErrUnsupportedScheme", err)
} }
@@ -198,7 +174,8 @@ func TestValidateURL_AllowsHTTPWhenConfigured(t *testing.T) {
t.Parallel() t.Parallel()
// Use a host that won't resolve (explicit .invalid TLD) so we don't hit DNS. // Use a host that won't resolve (explicit .invalid TLD) so we don't hit DNS.
err := validateURL(t.Context(), "http://nonexistent.invalid/path", true) err := validateURL(t.Context(),
&url.URL{Scheme: "http", Host: "nonexistent.invalid"}, true)
// We expect a host resolution error, not ErrUnsupportedScheme. // We expect a host resolution error, not ErrUnsupportedScheme.
if errors.Is(err, ErrUnsupportedScheme) { if errors.Is(err, ErrUnsupportedScheme) {
t.Error("validateURL with AllowHTTP should not return ErrUnsupportedScheme") t.Error("validateURL with AllowHTTP should not return ErrUnsupportedScheme")
@@ -208,7 +185,7 @@ func TestValidateURL_AllowsHTTPWhenConfigured(t *testing.T) {
func TestValidateURL_RejectsLocalhost(t *testing.T) { func TestValidateURL_RejectsLocalhost(t *testing.T) {
t.Parallel() t.Parallel()
err := validateURL(t.Context(), "https://localhost/path", false) err := validateURL(t.Context(), &url.URL{Scheme: "https", Host: "localhost"}, false)
if !errors.Is(err, ErrSSRFBlocked) { if !errors.Is(err, ErrSSRFBlocked) {
t.Errorf("validateURL localhost = %v, want ErrSSRFBlocked", err) t.Errorf("validateURL localhost = %v, want ErrSSRFBlocked", err)
} }
@@ -217,7 +194,7 @@ func TestValidateURL_RejectsLocalhost(t *testing.T) {
func TestValidateURL_EmptyHost(t *testing.T) { func TestValidateURL_EmptyHost(t *testing.T) {
t.Parallel() t.Parallel()
err := validateURL(t.Context(), "https:///path", false) err := validateURL(t.Context(), &url.URL{Scheme: "https"}, false)
if !errors.Is(err, ErrInvalidHost) { if !errors.Is(err, ErrInvalidHost) {
t.Errorf("validateURL empty host = %v, want ErrInvalidHost", err) t.Errorf("validateURL empty host = %v, want ErrInvalidHost", err)
} }
@@ -232,7 +209,8 @@ func TestMockFetcher_FetchesFile(t *testing.T) {
m := NewMock(mockFS) m := NewMock(mockFS)
result, err := m.Fetch(context.Background(), "https://example.com/images/photo.jpg") result, err := m.Fetch(context.Background(),
imagerequest.Source{Host: testHost, Path: "/images/photo.jpg"})
if err != nil { if err != nil {
t.Fatalf("Fetch() error = %v", err) t.Fatalf("Fetch() error = %v", err)
} }
@@ -263,7 +241,8 @@ func TestMockFetcher_MissingFileReturnsUpstreamError(t *testing.T) {
mockFS := fstest.MapFS{} mockFS := fstest.MapFS{}
m := NewMock(mockFS) m := NewMock(mockFS)
_, err := m.Fetch(context.Background(), "https://example.com/missing.jpg") _, err := m.Fetch(context.Background(),
imagerequest.Source{Host: testHost, Path: "/missing.jpg"})
if !errors.Is(err, ErrUpstreamError) { if !errors.Is(err, ErrUpstreamError) {
t.Errorf("Fetch() error = %v, want ErrUpstreamError", err) t.Errorf("Fetch() error = %v, want ErrUpstreamError", err)
} }
@@ -280,7 +259,7 @@ func TestMockFetcher_RespectsContextCancellation(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background()) ctx, cancel := context.WithCancel(context.Background())
cancel() cancel()
_, err := m.Fetch(ctx, "https://example.com/photo.jpg") _, err := m.Fetch(ctx, imagerequest.Source{Host: testHost, Path: "/photo.jpg"})
if !errors.Is(err, context.Canceled) { if !errors.Is(err, context.Canceled) {
t.Errorf("Fetch() error = %v, want context.Canceled", err) t.Errorf("Fetch() error = %v, want context.Canceled", err)
} }
@@ -8,14 +8,20 @@ import (
"sync" "sync"
"testing" "testing"
"time" "time"
"sneak.berlin/go/pixa/internal/imagerequest"
) )
// imageURLOnPort is the fake upstream's image route on testPublicHost at // imageSourceOn is the fake upstream's image route on host.
func imageSourceOn(host string) imagerequest.Source {
return imagerequest.Source{Host: host, Path: "/image"}
}
// imageSourceOnPort is the fake upstream's image route on testPublicHost at
// port. Each port is a different host to the per-host limit, while the test // port. Each port is a different host to the per-host limit, while the test
// dialer sends every port to the one test server. // dialer sends every port to the one test server.
func imageURLOnPort(port int) string { func imageSourceOnPort(port int) imagerequest.Source {
return "http://" + net.JoinHostPort(testPublicHost, strconv.Itoa(port)) + return imageSourceOn(net.JoinHostPort(testPublicHost, strconv.Itoa(port)))
"/image"
} }
// TestFetchLimitsConnectionsToAllHostsTogether checks that MaxConnections // TestFetchLimitsConnectionsToAllHostsTogether checks that MaxConnections
@@ -39,12 +45,12 @@ func TestFetchLimitsConnectionsToAllHostsTogether(t *testing.T) {
f, _ := newServerFetcher(t, srv, cfg) f, _ := newServerFetcher(t, srv, cfg)
f.connectionWaitTimeout = 100 * time.Millisecond f.connectionWaitTimeout = 100 * time.Millisecond
first, err := f.Fetch(testContext(t), imageURLOnPort(81)) first, err := f.Fetch(testContext(t), imageSourceOnPort(81))
if err != nil { if err != nil {
t.Fatalf("first Fetch() error = %v", err) t.Fatalf("first Fetch() error = %v", err)
} }
second, err := f.Fetch(testContext(t), imageURLOnPort(82)) second, err := f.Fetch(testContext(t), imageSourceOnPort(82))
if err != nil { if err != nil {
t.Fatalf("second Fetch() error = %v", err) t.Fatalf("second Fetch() error = %v", err)
} }
@@ -53,7 +59,7 @@ func TestFetchLimitsConnectionsToAllHostsTogether(t *testing.T) {
start := time.Now() start := time.Now()
_, err = f.Fetch(testContext(t), imageURLOnPort(83)) _, err = f.Fetch(testContext(t), imageSourceOnPort(83))
if !errors.Is(err, ErrTooManyConnections) { if !errors.Is(err, ErrTooManyConnections) {
t.Fatalf("third Fetch() error = %v, want ErrTooManyConnections", err) t.Fatalf("third Fetch() error = %v, want ErrTooManyConnections", err)
} }
@@ -72,7 +78,7 @@ func TestFetchLimitsConnectionsToAllHostsTogether(t *testing.T) {
t.Fatalf("close first body: %v", err) t.Fatalf("close first body: %v", err)
} }
third, err := f.Fetch(testContext(t), imageURLOnPort(83)) third, err := f.Fetch(testContext(t), imageSourceOnPort(83))
if err != nil { if err != nil {
t.Fatalf("Fetch() after a response was closed: error = %v", err) t.Fatalf("Fetch() after a response was closed: error = %v", err)
} }
@@ -99,7 +105,7 @@ func TestFetchFreesHostSlotWhenContextEndsWaitingForConnection(t *testing.T) {
f, _ := newServerFetcher(t, srv, cfg) f, _ := newServerFetcher(t, srv, cfg)
first, err := f.Fetch(testContext(t), imageURLOnPort(81)) first, err := f.Fetch(testContext(t), imageSourceOnPort(81))
if err != nil { if err != nil {
t.Fatalf("first Fetch() error = %v", err) t.Fatalf("first Fetch() error = %v", err)
} }
@@ -109,7 +115,7 @@ func TestFetchFreesHostSlotWhenContextEndsWaitingForConnection(t *testing.T) {
ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond) ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond)
defer cancel() defer cancel()
_, err = f.Fetch(ctx, imageURLOnPort(82)) _, err = f.Fetch(ctx, imageSourceOnPort(82))
if !errors.Is(err, context.DeadlineExceeded) { if !errors.Is(err, context.DeadlineExceeded) {
t.Fatalf("second Fetch() error = %v, want context.DeadlineExceeded", err) t.Fatalf("second Fetch() error = %v, want context.DeadlineExceeded", err)
} }
@@ -134,7 +140,7 @@ func TestFetchRemovesIdleHostSemaphores(t *testing.T) {
for i := range 100 { for i := range 100 {
wg.Go(func() { wg.Go(func() {
res, err := f.Fetch(ctx, imageURLOnPort(1+i%50)) res, err := f.Fetch(ctx, imageSourceOnPort(1+i%50))
if err != nil { if err != nil {
t.Errorf("Fetch() error = %v", err) t.Errorf("Fetch() error = %v", err)
@@ -173,12 +179,12 @@ func TestFetchRemovesHostSemaphoreWhenNoConnection(t *testing.T) {
f, _ := newServerFetcher(t, srv, cfg) f, _ := newServerFetcher(t, srv, cfg)
f.connectionWaitTimeout = 100 * time.Millisecond f.connectionWaitTimeout = 100 * time.Millisecond
open, err := f.Fetch(testContext(t), imageURLOnPort(81)) open, err := f.Fetch(testContext(t), imageSourceOnPort(81))
if err != nil { if err != nil {
t.Fatalf("first Fetch() error = %v", err) t.Fatalf("first Fetch() error = %v", err)
} }
_, err = f.Fetch(testContext(t), imageURLOnPort(82)) _, err = f.Fetch(testContext(t), imageSourceOnPort(82))
if !errors.Is(err, ErrTooManyConnections) { if !errors.Is(err, ErrTooManyConnections) {
t.Fatalf("Fetch() from another host: error = %v, "+ t.Fatalf("Fetch() from another host: error = %v, "+
"want ErrTooManyConnections", err) "want ErrTooManyConnections", err)
@@ -187,7 +193,7 @@ func TestFetchRemovesHostSemaphoreWhenNoConnection(t *testing.T) {
ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond) ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond)
defer cancel() defer cancel()
_, err = f.Fetch(ctx, imageURLOnPort(81)) _, err = f.Fetch(ctx, imageSourceOnPort(81))
if !errors.Is(err, context.DeadlineExceeded) { if !errors.Is(err, context.DeadlineExceeded) {
t.Fatalf("Fetch() from the busy host: error = %v, "+ t.Fatalf("Fetch() from the busy host: error = %v, "+
"want context.DeadlineExceeded", err) "want context.DeadlineExceeded", err)
@@ -199,7 +205,7 @@ func TestFetchRemovesHostSemaphoreWhenNoConnection(t *testing.T) {
ctx, cancel = context.WithTimeout(t.Context(), 100*time.Millisecond) ctx, cancel = context.WithTimeout(t.Context(), 100*time.Millisecond)
defer cancel() defer cancel()
_, err = f.Fetch(ctx, imageURLOnPort(83)) _, err = f.Fetch(ctx, imageSourceOnPort(83))
if !errors.Is(err, context.DeadlineExceeded) { if !errors.Is(err, context.DeadlineExceeded) {
t.Fatalf("Fetch() from a host with nothing open: error = %v, "+ t.Fatalf("Fetch() from a host with nothing open: error = %v, "+
"want context.DeadlineExceeded", err) "want context.DeadlineExceeded", err)
@@ -223,14 +229,14 @@ func TestFetchReleasesConnectionOnError(t *testing.T) {
cases := []struct { cases := []struct {
name string name string
url string source imagerequest.Source
want error want error
}{ }{
{"upstream answers 500", upstreamURL("/status/500"), ErrUpstreamError}, {"upstream answers 500", upstreamSource("/status/500"), ErrUpstreamError},
{"upstream sends HTML", upstreamURL("/html"), ErrInvalidContentType}, {"upstream sends HTML", upstreamSource("/html"), ErrInvalidContentType},
// 198.51.100.7 (TEST-NET-2) passes the SSRF checks, and the test // 198.51.100.7 (TEST-NET-2) passes the SSRF checks, and the test
// dialer refuses every host but testPublicHost. // dialer refuses every host but testPublicHost.
{"connecting fails", "http://198.51.100.7/image", errUnexpectedDial}, {"connecting fails", imageSourceOn("198.51.100.7"), errUnexpectedDial},
} }
for _, tc := range cases { for _, tc := range cases {
@@ -249,7 +255,7 @@ func TestFetchReleasesConnectionOnError(t *testing.T) {
f, _ := newServerFetcher(t, srv, cfg) f, _ := newServerFetcher(t, srv, cfg)
f.connectionWaitTimeout = 100 * time.Millisecond f.connectionWaitTimeout = 100 * time.Millisecond
_, err := f.Fetch(testContext(t), tc.url) _, err := f.Fetch(testContext(t), tc.source)
if !errors.Is(err, tc.want) { if !errors.Is(err, tc.want) {
t.Fatalf("Fetch() error = %v, want %v", err, tc.want) t.Fatalf("Fetch() error = %v, want %v", err, tc.want)
} }
+9 -37
View File
@@ -7,14 +7,14 @@ import (
"io/fs" "io/fs"
"net/http" "net/http"
"strings" "strings"
)
// errEmptyURLPath is returned when a mock URL has no usable path. "sneak.berlin/go/pixa/internal/imagerequest"
var errEmptyURLPath = errors.New("empty URL path") )
// MockFetcher implements Fetcher using an embedded filesystem. // MockFetcher implements Fetcher using an embedded filesystem.
// Files are organized as: hostname/path/to/file.ext // Files are organized as: hostname/path/to/file.ext
// URLs like https://example.com/images/photo.jpg map to example.com/images/photo.jpg. // The source with host example.com and path /images/photo.jpg maps to
// example.com/images/photo.jpg; the source query is ignored.
type MockFetcher struct { type MockFetcher struct {
fs fs.FS fs fs.FS
} }
@@ -24,8 +24,10 @@ func NewMock(fsys fs.FS) *MockFetcher {
return &MockFetcher{fs: fsys} return &MockFetcher{fs: fsys}
} }
// Fetch retrieves content from the mock filesystem. // Fetch retrieves the source's content from the mock filesystem.
func (m *MockFetcher) Fetch(ctx context.Context, url string) (*FetchResult, error) { func (m *MockFetcher) Fetch(
ctx context.Context, source imagerequest.Source,
) (*FetchResult, error) {
// Check context cancellation // Check context cancellation
select { select {
case <-ctx.Done(): case <-ctx.Done():
@@ -33,11 +35,7 @@ func (m *MockFetcher) Fetch(ctx context.Context, url string) (*FetchResult, erro
default: default:
} }
// Parse URL to get filesystem path path := source.Host + source.Path
path, err := urlToFSPath(url)
if err != nil {
return nil, err
}
// Open the file // Open the file
f, err := m.fs.Open(path) f, err := m.fs.Open(path)
@@ -68,32 +66,6 @@ func (m *MockFetcher) Fetch(ctx context.Context, url string) (*FetchResult, erro
}, nil }, nil
} }
// urlToFSPath converts a URL to a filesystem path.
// https://example.com/images/photo.jpg -> example.com/images/photo.jpg
func urlToFSPath(rawURL string) (string, error) {
// Strip scheme
url := rawURL
if idx := strings.Index(url, "://"); idx != -1 {
url = url[idx+3:]
}
// Remove query string
if idx := strings.Index(url, "?"); idx != -1 {
url = url[:idx]
}
// Remove fragment
if idx := strings.Index(url, "#"); idx != -1 {
url = url[:idx]
}
if url == "" {
return "", errEmptyURLPath
}
return url, nil
}
// detectContentTypeFromPath returns the MIME type based on file extension. // detectContentTypeFromPath returns the MIME type based on file extension.
func detectContentTypeFromPath(path string) string { func detectContentTypeFromPath(path string) string {
path = strings.ToLower(path) path = strings.ToLower(path)
@@ -34,7 +34,7 @@ func TestFetchSendsRequestID(t *testing.T) {
ctx := context.WithValue(testContext(t), middleware.RequestIDKey, requestID) ctx := context.WithValue(testContext(t), middleware.RequestIDKey, requestID)
res, err := f.Fetch(ctx, upstreamURL("/image")) res, err := f.Fetch(ctx, upstreamSource("/image"))
if err != nil { if err != nil {
t.Fatalf("Fetch() error = %v", err) t.Fatalf("Fetch() error = %v", err)
} }
+15 -25
View File
@@ -1,13 +1,12 @@
// Package imagerequest holds the image request type, its parts, its checks, // Package imagerequest holds the image request type, its parts, its checks,
// the parsing of the /v1/image/ path, the variant key and the source key. It // the parsing of the /v1/image/ path, the variant key, the source key and the
// imports no other package of this repo, so every package that handles an // source's upstream URL. It imports no other package of this repo, so every
// image request can import it. // package that handles an image request can import it.
package imagerequest package imagerequest
import ( import (
"errors" "errors"
"net/url" "net/url"
"time"
) )
// ErrInvalidFitMode is returned when an invalid fit mode is provided. // ErrInvalidFitMode is returned when an invalid fit mode is provided.
@@ -85,19 +84,13 @@ func ValidateImageRequest(req *Request) error {
} }
// Request represents a request for a processed image: the source to fetch // Request represents a request for a processed image: the source to fetch
// and the transform to apply to it. // and the transform to apply to it. It holds exactly what the variant key
// covers.
type Request struct { type Request struct {
// Source is what to fetch // Source is what to fetch
Source Source Source Source
// Transform is what to do to it // Transform is what to do to it
Transform Transform Transform Transform
// Signature is the HMAC signature for non-allowlisted hosts
Signature string
// Expires is when the URL expires: the exp of a signed URL, or the expiry
// of an encrypted URL; the zero time if it has none
Expires time.Time
// AllowHTTP indicates whether HTTP (non-TLS) is allowed for this request
AllowHTTP bool
} }
// Source is the image a request fetches from the origin. // Source is the image a request fetches from the origin.
@@ -125,23 +118,20 @@ type Transform struct {
FitMode Fit FitMode Fit
} }
// SourceURL returns the full upstream URL to fetch: the source path // URL returns the source's upstream URL, with the scheme http when allowHTTP
// percent-encoded again, as net/url writes a path, so each source path is one // is true and https otherwise. net/url writes its path percent-encoded again,
// upstream path, and the source query as it is, with the characters // so each source path is one upstream path. Its query is the source query as
// escapeQuery encodes percent-encoded. Uses http:// scheme when AllowHTTP is // it is, with the characters escapeQuery encodes percent-encoded.
// true, otherwise https://. func (s Source) URL(allowHTTP bool) *url.URL {
func (r *Request) SourceURL() string {
scheme := "https" scheme := "https"
if r.AllowHTTP { if allowHTTP {
scheme = "http" scheme = "http"
} }
sourceURL := url.URL{ return &url.URL{
Scheme: scheme, Scheme: scheme,
Host: r.Source.Host, Host: s.Host,
Path: r.Source.Path, Path: s.Path,
RawQuery: escapeQuery(r.Source.Query), RawQuery: escapeQuery(s.Query),
} }
return sourceURL.String()
} }
@@ -2,57 +2,49 @@ package imagerequest
import "testing" import "testing"
func TestRequest_SourceURL_DefaultHTTPS(t *testing.T) { func TestSource_URL_DefaultHTTPS(t *testing.T) {
t.Parallel() t.Parallel()
req := &Request{ source := Source{
Source: Source{
Host: testHostCDN, Host: testHostCDN,
Path: testPathCat, Path: testPathCat,
Query: "v=2", Query: "v=2",
},
} }
got := req.SourceURL() got := source.URL(false).String()
want := "https://cdn.example.com/photos/cat.jpg?v=2" want := "https://cdn.example.com/photos/cat.jpg?v=2"
if got != want { if got != want {
t.Errorf("SourceURL() = %q, want %q", got, want) t.Errorf("URL() = %q, want %q", got, want)
} }
} }
func TestRequest_SourceURL_AllowHTTP(t *testing.T) { func TestSource_URL_AllowHTTP(t *testing.T) {
t.Parallel() t.Parallel()
req := &Request{ source := Source{
Source: Source{
Host: "localhost:8080", Host: "localhost:8080",
Path: testPathCat, Path: testPathCat,
},
AllowHTTP: true,
} }
got := req.SourceURL() got := source.URL(true).String()
want := "http://localhost:8080/photos/cat.jpg" want := "http://localhost:8080/photos/cat.jpg"
if got != want { if got != want {
t.Errorf("SourceURL() = %q, want %q", got, want) t.Errorf("URL() = %q, want %q", got, want)
} }
} }
func TestRequest_SourceURL_AllowHTTPFalse(t *testing.T) { func TestSource_URL_AllowHTTPFalse(t *testing.T) {
t.Parallel() t.Parallel()
req := &Request{ source := Source{
Source: Source{
Host: testHostCDN, Host: testHostCDN,
Path: "/img.jpg", Path: "/img.jpg",
},
AllowHTTP: false,
} }
got := req.SourceURL() got := source.URL(false).String()
if got != "https://cdn.example.com/img.jpg" { if got != "https://cdn.example.com/img.jpg" {
t.Errorf("SourceURL() = %q, want https scheme", got) t.Errorf("URL() = %q, want https scheme", got)
} }
} }
+4 -6
View File
@@ -126,9 +126,8 @@ func parseImageComponents(remainder string) (*Request, error) {
return nil, ErrMissingHost return nil, ErrMissingHost
} }
// Only the forms hostRegex describes are accepted. A "?", "#" or "@" in // Later steps put the host unparsed into the allowlist check, the cache key
// the host would make Go's URL parser, which the allowlist check and the // and the upstream URL, so only the forms hostRegex describes are accepted
// fetcher use, read a different host from the one allowlisted and cached
hostMatch := hostRegex.FindStringSubmatch(host) hostMatch := hostRegex.FindStringSubmatch(host)
if hostMatch == nil { if hostMatch == nil {
return nil, fmt.Errorf("%w: %q", ErrInvalidHost, host) return nil, fmt.Errorf("%w: %q", ErrInvalidHost, host)
@@ -147,9 +146,8 @@ func parseImageComponents(remainder string) (*Request, error) {
// host // host
host = strings.ToLower(host) host = strings.ToLower(host)
// Go's URL parser, which the fetcher uses, reads a "#" as the start of a // A "#" starts a URL's fragment, so neither the source path nor the source
// fragment and drops the rest, so each text after it would be another // query may hold one
// spelling, cached under its own keys, of one upstream request
if strings.Contains(path, "#") { if strings.Contains(path, "#") {
return nil, fmt.Errorf( return nil, fmt.Errorf(
`%w: the source path or query holds a "#"`, ErrInvalidPath) `%w: the source path or query holds a "#"`, ErrInvalidPath)
@@ -36,7 +36,7 @@ type heldFetcher struct {
} }
func (f *heldFetcher) Fetch( func (f *heldFetcher) Fetch(
ctx context.Context, url string, ctx context.Context, source imagerequest.Source,
) (*httpfetcher.FetchResult, error) { ) (*httpfetcher.FetchResult, error) {
f.fetches.Add(1) f.fetches.Add(1)
@@ -48,7 +48,7 @@ func (f *heldFetcher) Fetch(
return nil, ctx.Err() return nil, ctx.Err()
} }
return f.upstream.Fetch(ctx, url) return f.upstream.Fetch(ctx, source)
} }
// releaseFetches lets every held fetch, and every later one, go on. // releaseFetches lets every held fetch, and every later one, go on.
@@ -489,7 +489,7 @@ func TestService_Get_WaiterGetsProcessingTimedOut(t *testing.T) {
type panickingFetcher struct{} type panickingFetcher struct{}
func (panickingFetcher) Fetch( func (panickingFetcher) Fetch(
context.Context, string, context.Context, imagerequest.Source,
) (*httpfetcher.FetchResult, error) { ) (*httpfetcher.FetchResult, error) {
panic("upstream fetcher panicked") panic("upstream fetcher panicked")
} }
+5 -60
View File
@@ -7,21 +7,17 @@ import (
"fmt" "fmt"
"io" "io"
"log/slog" "log/slog"
"net/url"
"runtime/debug" "runtime/debug"
"strings"
"time" "time"
"github.com/dustin/go-humanize" "github.com/dustin/go-humanize"
"github.com/getsentry/sentry-go" "github.com/getsentry/sentry-go"
"github.com/go-chi/chi/v5/middleware" "github.com/go-chi/chi/v5/middleware"
"golang.org/x/sync/singleflight" "golang.org/x/sync/singleflight"
"sneak.berlin/go/pixa/internal/allowlist"
"sneak.berlin/go/pixa/internal/httpfetcher" "sneak.berlin/go/pixa/internal/httpfetcher"
"sneak.berlin/go/pixa/internal/imageprocessor" "sneak.berlin/go/pixa/internal/imageprocessor"
"sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imagerequest"
"sneak.berlin/go/pixa/internal/magic" "sneak.berlin/go/pixa/internal/magic"
"sneak.berlin/go/pixa/internal/signature"
) )
// Service orchestrates the cache, fetcher, and processor. // Service orchestrates the cache, fetcher, and processor.
@@ -29,10 +25,7 @@ type Service struct {
cache *Cache cache *Cache
fetcher httpfetcher.Fetcher fetcher httpfetcher.Fetcher
processor *imageprocessor.ImageProcessor processor *imageprocessor.ImageProcessor
signer *signature.Signer
allowlist *allowlist.HostAllowList
log *slog.Logger log *slog.Logger
allowHTTP bool
maxResponseSize int64 maxResponseSize int64
// variantsInProgress lets the requests that miss the same variant at the // variantsInProgress lets the requests that miss the same variant at the
// same time share one fetch and one transcode. // same time share one fetch and one transcode.
@@ -47,13 +40,6 @@ type ServiceConfig struct {
Fetcher httpfetcher.Fetcher Fetcher httpfetcher.Fetcher
// Processor converts and resizes the images // Processor converts and resizes the images
Processor *imageprocessor.ImageProcessor Processor *imageprocessor.ImageProcessor
// SigningKey is the HMAC signing key (empty disables signing)
SigningKey string
// Allowlist is the list of hosts that don't require signatures
Allowlist []string
// AllowHTTP makes the service fetch sources over http instead of
// https (testing only), so the fetcher must allow http too
AllowHTTP bool
// MaxResponseSize is the largest source accepted, in bytes; a cached // MaxResponseSize is the largest source accepted, in bytes; a cached
// source larger than this is fetched again // source larger than this is fetched again
MaxResponseSize int64 MaxResponseSize int64
@@ -61,11 +47,8 @@ type ServiceConfig struct {
Logger *slog.Logger Logger *slog.Logger
} }
// Static errors for service construction. // errCacheRequired is returned when the service is built without a cache.
var ( var errCacheRequired = errors.New("cache is required")
errCacheRequired = errors.New("cache is required")
errSigningKeyRequired = errors.New("signing key is required")
)
// NewService creates a new image service. // NewService creates a new image service.
func NewService(cfg *ServiceConfig) (*Service, error) { func NewService(cfg *ServiceConfig) (*Service, error) {
@@ -73,10 +56,6 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
return nil, errCacheRequired return nil, errCacheRequired
} }
if cfg.SigningKey == "" {
return nil, errSigningKeyRequired
}
log := cfg.Logger log := cfg.Logger
if log == nil { if log == nil {
log = slog.Default() log = slog.Default()
@@ -93,10 +72,7 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
cache: cfg.Cache, cache: cfg.Cache,
fetcher: cfg.Fetcher, fetcher: cfg.Fetcher,
processor: cfg.Processor, processor: cfg.Processor,
signer: signature.New(cfg.SigningKey),
allowlist: allowlist.New(cfg.Allowlist),
log: log, log: log,
allowHTTP: cfg.AllowHTTP,
maxResponseSize: cfg.MaxResponseSize, maxResponseSize: cfg.MaxResponseSize,
}, nil }, nil
} }
@@ -123,9 +99,6 @@ var ErrProcessingTimedOut = errors.New(
func (s *Service) Get( func (s *Service) Get(
ctx context.Context, req *imagerequest.Request, ctx context.Context, req *imagerequest.Request,
) (*ImageResponse, error) { ) (*ImageResponse, error) {
// Propagate AllowHTTP setting to the request
req.AllowHTTP = s.allowHTTP
cacheKey := imagerequest.CacheKey(req) cacheKey := imagerequest.CacheKey(req)
reader, size, contentType, err := s.cache.GetVariant(cacheKey) reader, size, contentType, err := s.cache.GetVariant(cacheKey)
@@ -169,33 +142,6 @@ func (s *Service) WaitForProcessing(ctx context.Context) int {
return s.processor.WaitForProcessing(ctx) return s.processor.WaitForProcessing(ctx)
} }
// ValidateRequest validates the request signature if required.
func (s *Service) ValidateRequest(req *imagerequest.Request) error {
// Check if host is allowed (no signature required)
sourceURL := req.SourceURL()
parsedURL, err := url.Parse(sourceURL)
if err != nil {
return fmt.Errorf("invalid source URL: %w", err)
}
// An allowlist entry names no port, so a host followed by ":", with or
// without a port, needs a signature. The port is read from the parsed
// URL, as the fetcher reads it: in an IPv6 address written without
// brackets, such as 2001:db8::1:8443, it is what follows the last ":".
// This is checked here and not in allowlist.IsAllowed, which also matches
// referer_blocklist, where a Referer must be blocked whatever its port.
hostHasPort := parsedURL.Port() != "" ||
strings.HasSuffix(parsedURL.Host, ":")
if !hostHasPort && s.allowlist.IsAllowed(parsedURL) {
return nil
}
// Signature required for non-allowed hosts
return s.signer.Verify(req)
}
// errPanicked is returned when processing a variant panicked. // errPanicked is returned when processing a variant panicked.
var errPanicked = errors.New("panic while processing image") var errPanicked = errors.New("panic while processing image")
@@ -388,11 +334,10 @@ func (s *Service) fetchAndProcess(
} }
// Fetch from upstream // Fetch from upstream
sourceURL := req.SourceURL() s.log.Debug("fetching from upstream", "host", req.Source.Host,
"path", req.Source.Path, "query", req.Source.Query)
s.log.Debug("fetching from upstream", "url", sourceURL) fetchResult, err := s.fetcher.Fetch(ctx, req.Source)
fetchResult, err := s.fetcher.Fetch(ctx, sourceURL)
if err != nil { if err != nil {
// Store negative cache for certain errors // Store negative cache for certain errors
if isNegativeCacheable(err) { if isNegativeCacheable(err) {
+5 -306
View File
@@ -9,14 +9,12 @@ import (
"sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imagerequest"
"sneak.berlin/go/pixa/internal/magic" "sneak.berlin/go/pixa/internal/magic"
"sneak.berlin/go/pixa/internal/signature"
) )
// Test data literals used repeatedly in this file (goconst). // Test data literals used repeatedly in this file (goconst).
const ( const (
testPathPhoto = "/images/photo.jpg" testPathPhoto = "/images/photo.jpg"
testPathUpload = "/uploads/image.jpg" testPathUpload = "/uploads/image.jpg"
testSigningKey = "test-signing-key-12345"
) )
func TestService_Get_AllowlistedHost(t *testing.T) { func TestService_Get_AllowlistedHost(t *testing.T) {
@@ -60,36 +58,13 @@ func TestService_Get_AllowlistedHost(t *testing.T) {
} }
} }
func TestService_Get_NonAllowlistedHost_NoSignature(t *testing.T) { // TestService_Get_NonAllowlistedHost checks that the image service serves a
// host that is not on the allowlist as any other: the /v1/image/ handler checks
// the signature for such a host before it asks the service for the image.
func TestService_Get_NonAllowlistedHost(t *testing.T) {
t.Parallel() t.Parallel()
svc, fixtures := SetupTestService(t, WithSigningKey("test-key")) svc, fixtures := SetupTestService(t)
req := &imagerequest.Request{
Source: imagerequest.Source{
Host: fixtures.OtherHost,
Path: testPathUpload,
},
Transform: imagerequest.Transform{
Size: imagerequest.Size{Width: 50, Height: 50},
Format: imagerequest.FormatJPEG,
Quality: 85,
FitMode: imagerequest.FitCover,
},
}
// Should fail validation - not allowlisted and no signature
err := svc.ValidateRequest(req)
if err == nil {
t.Error("ValidateRequest() expected error for non-allowlisted host without signature")
}
}
func TestService_Get_NonAllowlistedHost_ValidSignature(t *testing.T) {
t.Parallel()
signingKey := testSigningKey
svc, fixtures := SetupTestService(t, WithSigningKey(signingKey))
ctx := context.Background() ctx := context.Background()
req := &imagerequest.Request{ req := &imagerequest.Request{
@@ -105,17 +80,6 @@ func TestService_Get_NonAllowlistedHost_ValidSignature(t *testing.T) {
}, },
} }
// Generate a valid signature
signer := signature.New(signingKey)
req.Expires = time.Now().Add(time.Hour)
req.Signature = signer.Sign(req)
// Should pass validation
err := svc.ValidateRequest(req)
if err != nil {
t.Errorf("ValidateRequest() error = %v", err)
}
// Should fetch successfully // Should fetch successfully
resp, err := svc.Get(ctx, req) resp, err := svc.Get(ctx, req)
if err != nil { if err != nil {
@@ -134,244 +98,6 @@ func TestService_Get_NonAllowlistedHost_ValidSignature(t *testing.T) {
} }
} }
func TestService_Get_NonAllowlistedHost_ExpiredSignature(t *testing.T) {
t.Parallel()
signingKey := testSigningKey
svc, fixtures := SetupTestService(t, WithSigningKey(signingKey))
req := &imagerequest.Request{
Source: imagerequest.Source{
Host: fixtures.OtherHost,
Path: testPathUpload,
},
Transform: imagerequest.Transform{
Size: imagerequest.Size{Width: 50, Height: 50},
Format: imagerequest.FormatJPEG,
Quality: 85,
FitMode: imagerequest.FitCover,
},
}
// Generate an expired signature
signer := signature.New(signingKey)
req.Expires = time.Now().Add(-time.Hour) // Already expired
req.Signature = signer.Sign(req)
// Should fail validation
err := svc.ValidateRequest(req)
if err == nil {
t.Error("ValidateRequest() expected error for expired signature")
}
}
func TestService_Get_NonAllowlistedHost_InvalidSignature(t *testing.T) {
t.Parallel()
signingKey := testSigningKey
svc, fixtures := SetupTestService(t, WithSigningKey(signingKey))
req := &imagerequest.Request{
Source: imagerequest.Source{
Host: fixtures.OtherHost,
Path: testPathUpload,
},
Transform: imagerequest.Transform{
Size: imagerequest.Size{Width: 50, Height: 50},
Format: imagerequest.FormatJPEG,
Quality: 85,
FitMode: imagerequest.FitCover,
},
}
// Set an invalid signature
req.Signature = "invalid-signature"
req.Expires = time.Now().Add(time.Hour)
// Should fail validation
err := svc.ValidateRequest(req)
if err == nil {
t.Error("ValidateRequest() expected error for invalid signature")
}
}
// TestService_ValidateRequest_SignatureExactHostMatch verifies that
// ValidateRequest enforces exact host matching for signatures. A
// signature for one host must not verify for a different host, even
// if they share a domain suffix.
func TestService_ValidateRequest_SignatureExactHostMatch(t *testing.T) {
t.Parallel()
signingKey := "test-signing-key-must-be-32-chars"
svc, _ := SetupTestService(t,
WithSigningKey(signingKey),
WithNoAllowlist(),
)
signer := signature.New(signingKey)
// Sign a request for "cdn.example.com"
signedReq := &imagerequest.Request{
Source: imagerequest.Source{
Host: testHostCDN,
Path: testPathCat,
},
Transform: imagerequest.Transform{
Size: imagerequest.Size{Width: 50, Height: 50},
Format: imagerequest.FormatJPEG,
Quality: 85,
FitMode: imagerequest.FitCover,
},
Expires: time.Now().Add(time.Hour),
}
signedReq.Signature = signer.Sign(signedReq)
// The original request should pass validation
t.Run("exact host passes", func(t *testing.T) {
t.Parallel()
err := svc.ValidateRequest(signedReq)
if err != nil {
t.Errorf("ValidateRequest() exact host failed: %v", err)
}
})
// Try to reuse the signature with different hosts
tests := []struct {
name string
host string
}{
{"parent domain", testHostExample},
{"sibling subdomain", "images.example.com"},
{"deeper subdomain", "a.cdn.example.com"},
{"evil suffix domain", "cdn.example.com.evil.com"},
{"prefixed host", "evilcdn.example.com"},
}
for _, tt := range tests {
t.Run(tt.name+" rejected", func(t *testing.T) {
t.Parallel()
req := &imagerequest.Request{
Source: imagerequest.Source{
Host: tt.host,
Path: signedReq.Source.Path,
Query: signedReq.Source.Query,
},
Transform: signedReq.Transform,
Expires: signedReq.Expires,
Signature: signedReq.Signature,
}
err := svc.ValidateRequest(req)
if err == nil {
t.Errorf(
"ValidateRequest() should reject signature for host %q (signed for %q)",
tt.host, signedReq.Source.Host)
}
})
}
}
// TestService_ValidateRequest_HostWithPortNeedsSignature verifies that a host
// followed by a port, or by a colon with no port, matches no allowlist entry,
// so that a request for it without a signature is refused, while the same
// host with no port is allowlisted.
func TestService_ValidateRequest_HostWithPortNeedsSignature(t *testing.T) {
t.Parallel()
svc, _ := SetupTestService(t,
WithSigningKey(testSigningKey),
WithAllowlist(testHostCDN, "203.0.113.5", "2001:db8::1"),
)
tests := []struct {
host string
allowlisted bool
}{
{testHostCDN, true},
{"cdn.example.com:8443", false},
{"cdn.example.com:443", false},
{"cdn.example.com:", false},
{"203.0.113.5", true},
{"203.0.113.5:8080", false},
{"[2001:db8::1]", true},
{"[2001:db8::1]:8443", false},
}
for _, tt := range tests {
t.Run(tt.host, func(t *testing.T) {
t.Parallel()
req := &imagerequest.Request{
Source: imagerequest.Source{
Host: tt.host,
Path: testPathCat,
},
Transform: imagerequest.Transform{
Size: imagerequest.Size{Width: 50, Height: 50},
Format: imagerequest.FormatJPEG,
Quality: 85,
FitMode: imagerequest.FitCover,
},
}
err := svc.ValidateRequest(req)
t.Logf("ValidateRequest() for host %q with no signature: %v",
tt.host, err)
if allowlisted := err == nil; allowlisted != tt.allowlisted {
t.Errorf("host %q served without a signature = %v, want %v",
tt.host, allowlisted, tt.allowlisted)
}
})
}
}
// TestService_ValidateRequest_UnbracketedIPv6PortNeedsSignature verifies that
// an allowlisted IPv6 address written without brackets and followed by a
// port, or by a colon with no port, needs a signature. Go's URL parser, which
// the fetcher uses, reads 2001:db8::1:8443 as the host 2001:db8::1 with the
// port 8443.
func TestService_ValidateRequest_UnbracketedIPv6PortNeedsSignature(
t *testing.T,
) {
t.Parallel()
svc, _ := SetupTestService(t,
WithSigningKey(testSigningKey),
WithAllowlist("2001:db8::1"),
)
for _, host := range []string{"2001:db8::1:8443", "2001:db8::1:"} {
t.Run(host, func(t *testing.T) {
t.Parallel()
req := &imagerequest.Request{
Source: imagerequest.Source{
Host: host,
Path: testPathCat,
},
Transform: imagerequest.Transform{
Size: imagerequest.Size{Width: 50, Height: 50},
Format: imagerequest.FormatJPEG,
Quality: 85,
FitMode: imagerequest.FitCover,
},
}
err := svc.ValidateRequest(req)
t.Logf("ValidateRequest() for host %q with no signature: %v",
host, err)
if err == nil {
t.Errorf("host %q served without a signature, want refused",
host)
}
})
}
}
func TestService_Get_InvalidFile(t *testing.T) { func TestService_Get_InvalidFile(t *testing.T) {
t.Parallel() t.Parallel()
@@ -692,33 +418,6 @@ func TestService_Get_DifferentSizes(t *testing.T) {
} }
} }
func TestService_ValidateRequest_NoSigningKey(t *testing.T) {
t.Parallel()
// Service with no signing key - all non-allowlisted requests should fail
svc, fixtures := SetupTestService(t, WithNoAllowlist())
req := &imagerequest.Request{
Source: imagerequest.Source{
Host: fixtures.OtherHost,
Path: testPathUpload,
},
Transform: imagerequest.Transform{
Size: imagerequest.Size{Width: 50, Height: 50},
Format: imagerequest.FormatJPEG,
Quality: 85,
FitMode: imagerequest.FitCover,
},
}
err := svc.ValidateRequest(req)
if err == nil {
t.Error(
"ValidateRequest() expected error when no signing key and host not allowlisted",
)
}
}
func TestService_Get_ContextCancellation(t *testing.T) { func TestService_Get_ContextCancellation(t *testing.T) {
t.Parallel() t.Parallel()
+1 -1
View File
@@ -93,7 +93,7 @@ type fakeUpstream struct {
} }
func (u *fakeUpstream) Fetch( func (u *fakeUpstream) Fetch(
context.Context, string, context.Context, imagerequest.Source,
) (*httpfetcher.FetchResult, error) { ) (*httpfetcher.FetchResult, error) {
return &httpfetcher.FetchResult{ return &httpfetcher.FetchResult{
Content: io.NopCloser(u), Content: io.NopCloser(u),
+1 -43
View File
@@ -161,22 +161,11 @@ func generateTestGIF(t *testing.T, width, height int, c color.Color) []byte {
} }
// SetupTestService creates a Service with mock fetcher for testing. // SetupTestService creates a Service with mock fetcher for testing.
func SetupTestService( func SetupTestService(t *testing.T) (*Service, *TestFixtures) {
t *testing.T, opts ...TestServiceOption,
) (*Service, *TestFixtures) {
t.Helper() t.Helper()
mockFS, fixtures := NewTestFS(t) mockFS, fixtures := NewTestFS(t)
cfg := &testServiceConfig{
allowlist: []string{fixtures.GoodHost},
signingKey: "test-signing-key-must-be-32-chars",
}
for _, opt := range opts {
opt(cfg)
}
// Create temp directory for cache // Create temp directory for cache
tmpDir := t.TempDir() tmpDir := t.TempDir()
@@ -199,8 +188,6 @@ func SetupTestService(
MaxInputBytes: 50 << 20, MaxInputBytes: 50 << 20,
MaxConcurrentProcessing: 2, MaxConcurrentProcessing: 2,
}), }),
SigningKey: cfg.signingKey,
Allowlist: cfg.allowlist,
MaxResponseSize: 50 << 20, MaxResponseSize: 50 << 20,
}) })
if err != nil { if err != nil {
@@ -228,32 +215,3 @@ func setupServiceTestDB(t *testing.T) *sql.DB {
return db return db
} }
type testServiceConfig struct {
allowlist []string
signingKey string
}
// TestServiceOption configures the test service.
type TestServiceOption func(*testServiceConfig)
// WithAllowlist sets the allowlist for the test service.
func WithAllowlist(hosts ...string) TestServiceOption {
return func(c *testServiceConfig) {
c.allowlist = hosts
}
}
// WithSigningKey sets the signing key for the test service.
func WithSigningKey(key string) TestServiceOption {
return func(c *testServiceConfig) {
c.signingKey = key
}
}
// WithNoAllowlist removes all allowlisted hosts.
func WithNoAllowlist() TestServiceOption {
return func(c *testServiceConfig) {
c.allowlist = nil
}
}
+2 -8
View File
@@ -212,21 +212,15 @@ type imageServiceParams struct {
} }
// newImageService builds the image service from the cache, the fetcher // newImageService builds the image service from the cache, the fetcher
// and the processor, with the signing key, the allowlist and the upstream // and the processor, with the largest upstream response the config allows.
// settings of the config.
func newImageService( func newImageService(
_ fx.Lifecycle, params imageServiceParams, _ fx.Lifecycle, params imageServiceParams,
) (*imgcache.Service, error) { ) (*imgcache.Service, error) {
cfg := params.Config
return imgcache.NewService(&imgcache.ServiceConfig{ return imgcache.NewService(&imgcache.ServiceConfig{
Cache: params.Cache, Cache: params.Cache,
Fetcher: params.Fetcher, Fetcher: params.Fetcher,
Processor: params.Processor, Processor: params.Processor,
SigningKey: cfg.SigningKey, MaxResponseSize: params.Config.UpstreamMaxResponseSize,
Allowlist: cfg.AllowlistHosts,
AllowHTTP: cfg.AllowHTTP,
MaxResponseSize: cfg.UpstreamMaxResponseSize,
Logger: params.Logger.Get(), Logger: params.Logger.Get(),
}) })
} }
@@ -7,6 +7,7 @@ import (
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"testing" "testing"
"time"
"sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imagerequest"
@@ -76,7 +77,7 @@ func TestImageProxyRefusesComputedSideAboveMaximum(t *testing.T) {
Size: imagerequest.Size{Width: tt.width, Height: tt.height}, Size: imagerequest.Size{Width: tt.width, Height: tt.height},
Format: imagerequest.FormatJPEG, Format: imagerequest.FormatJPEG,
}, },
}) }, time.Time{})
if err != nil { if err != nil {
t.Fatalf("encurl Generate: %v", err) t.Fatalf("encurl Generate: %v", err)
} }
+2 -1
View File
@@ -5,6 +5,7 @@ import (
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"testing" "testing"
"time"
"sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imagerequest"
@@ -66,7 +67,7 @@ func TestImageProxyRefusesFitOutsideAboveMaximum(t *testing.T) {
Format: imagerequest.FormatJPEG, Format: imagerequest.FormatJPEG,
FitMode: imagerequest.FitOutside, FitMode: imagerequest.FitOutside,
}, },
}) }, time.Time{})
if err != nil { if err != nil {
t.Fatalf("encurl Generate: %v", err) t.Fatalf("encurl Generate: %v", err)
} }
+2 -3
View File
@@ -524,8 +524,7 @@ func signedImageURL(t *testing.T, sourceHost string) string {
Quality: 85, Quality: 85,
FitMode: imagerequest.FitCover, FitMode: imagerequest.FitCover,
}, },
Expires: expires, }, expires)
})
// Go's percent-encoding of a URL path writes the brackets of the IPv6 // Go's percent-encoding of a URL path writes the brackets of the IPv6
// host as %5B and %5D. // host as %5B and %5D.
@@ -549,7 +548,7 @@ func encryptedImageURL(t *testing.T, sourceHost string) string {
token, err := generator.Generate(&imagerequest.Request{ token, err := generator.Generate(&imagerequest.Request{
Source: imagerequest.Source{Host: sourceHost, Path: ipv6SourcePath}, Source: imagerequest.Source{Host: sourceHost, Path: ipv6SourcePath},
Transform: imagerequest.Transform{Format: imagerequest.FormatPNG}, Transform: imagerequest.Transform{Format: imagerequest.FormatPNG},
}) }, time.Time{})
if err != nil { if err != nil {
t.Fatalf("encurl Generate: %v", err) t.Fatalf("encurl Generate: %v", err)
} }
+2 -1
View File
@@ -6,6 +6,7 @@ import (
"net/http/httptest" "net/http/httptest"
"strings" "strings"
"testing" "testing"
"time"
"sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imagerequest"
@@ -45,7 +46,7 @@ func TestImageProxyRefusesSVGServedAsAVIF(t *testing.T) {
Size: imagerequest.Size{Width: 32, Height: 24}, Size: imagerequest.Size{Width: 32, Height: 24},
Format: format, Format: format,
}, },
}) }, time.Time{})
if err != nil { if err != nil {
t.Fatalf("encurl Generate: %v", err) t.Fatalf("encurl Generate: %v", err)
} }
+6 -7
View File
@@ -28,20 +28,20 @@ func TestSigner_Verify_ColonCannotMovePathQueryBoundary(t *testing.T) {
Quality: 85, Quality: 85,
FitMode: testFitCover, FitMode: testFitCover,
}, },
Expires: time.Now().Add(1 * time.Hour),
} }
signed.Signature = signer.Sign(signed) expires := time.Now().Add(1 * time.Hour)
sig := signer.Sign(signed, expires)
replayed := *signed replayed := *signed
replayed.Source.Path = "/a" replayed.Source.Path = "/a"
replayed.Source.Query = "b:" replayed.Source.Query = "b:"
t.Logf("signature for path %q query %q: %s", t.Logf("signature for path %q query %q: %s",
signed.Source.Path, signed.Source.Query, signed.Signature) signed.Source.Path, signed.Source.Query, sig)
t.Logf("signature for path %q query %q: %s", t.Logf("signature for path %q query %q: %s",
replayed.Source.Path, replayed.Source.Query, signer.Sign(&replayed)) replayed.Source.Path, replayed.Source.Query, signer.Sign(&replayed, expires))
err := signer.Verify(&replayed) err := signer.Verify(&replayed, sig, expires)
if !errors.Is(err, signature.ErrInvalid) { if !errors.Is(err, signature.ErrInvalid) {
t.Errorf("Verify() of path %q query %q with the signature for "+ t.Errorf("Verify() of path %q query %q with the signature for "+
"path %q = %v, want %v", "path %q = %v, want %v",
@@ -106,10 +106,9 @@ func TestSigner_EscapedGoldenVectors(t *testing.T) {
Quality: 85, Quality: 85,
FitMode: testFitCover, FitMode: testFitCover,
}, },
Expires: time.Unix(goldenExpiresUnix, 0),
} }
gotSignature := signer.Sign(req) gotSignature := signer.Sign(req, time.Unix(goldenExpiresUnix, 0))
t.Logf("Sign() of path %q query %q = %s", tt.path, tt.query, gotSignature) t.Logf("Sign() of path %q query %q = %s", tt.path, tt.query, gotSignature)
if gotSignature != tt.wantSignature { if gotSignature != tt.wantSignature {
+3 -4
View File
@@ -19,8 +19,8 @@ type goldenVector struct {
name string name string
req imagerequest.Request req imagerequest.Request
// wantSignature is the exact base64url (RFC 4648 URL-safe, // wantSignature is the exact base64url (RFC 4648 URL-safe,
// padded) HMAC-SHA256 signature for the request with Expires // padded) HMAC-SHA256 signature for the request expiring at
// set to goldenExpiresUnix. // goldenExpiresUnix.
wantSignature string wantSignature string
} }
@@ -124,9 +124,8 @@ func TestSigner_GoldenVectors(t *testing.T) {
t.Parallel() t.Parallel()
signReq := tt.req signReq := tt.req
signReq.Expires = time.Unix(goldenExpiresUnix, 0)
gotSignature := signer.Sign(&signReq) gotSignature := signer.Sign(&signReq, time.Unix(goldenExpiresUnix, 0))
if gotSignature != tt.wantSignature { if gotSignature != tt.wantSignature {
t.Errorf("Sign() = %q, want %q (signed byte format changed?)", t.Errorf("Sign() = %q, want %q (signed byte format changed?)",
gotSignature, tt.wantSignature) gotSignature, tt.wantSignature)
+10 -9
View File
@@ -9,10 +9,12 @@ import (
"sneak.berlin/go/pixa/internal/signature" "sneak.berlin/go/pixa/internal/signature"
) )
// signedQualityFitRequest returns a request signed for quality 85 and fit // signedQualityFitRequest returns a request, its signature for quality 85 and
// mode "cover", the effective defaults the handler applies before // fit mode "cover", the effective defaults the handler applies before
// verification. // verification, and the expiry it was signed with.
func signedQualityFitRequest(signer *signature.Signer) *imagerequest.Request { func signedQualityFitRequest(
signer *signature.Signer,
) (*imagerequest.Request, string, time.Time) {
req := &imagerequest.Request{ req := &imagerequest.Request{
Source: imagerequest.Source{ Source: imagerequest.Source{
Host: testHost, Host: testHost,
@@ -24,11 +26,10 @@ func signedQualityFitRequest(signer *signature.Signer) *imagerequest.Request {
Quality: 85, Quality: 85,
FitMode: testFitCover, FitMode: testFitCover,
}, },
Expires: time.Now().Add(1 * time.Hour),
} }
req.Signature = signer.Sign(req) expires := time.Now().Add(1 * time.Hour)
return req return req, signer.Sign(req, expires), expires
} }
// TestSigner_Verify_QualityAndFitAreSigned proves that quality and fit are // TestSigner_Verify_QualityAndFitAreSigned proves that quality and fit are
@@ -60,10 +61,10 @@ func TestSigner_Verify_QualityAndFitAreSigned(t *testing.T) {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
t.Parallel() t.Parallel()
req := signedQualityFitRequest(signer) req, sig, expires := signedQualityFitRequest(signer)
tt.tamper(req) tt.tamper(req)
err := signer.Verify(req) err := signer.Verify(req, sig, expires)
if !errors.Is(err, signature.ErrInvalid) { if !errors.Is(err, signature.ErrInvalid) {
t.Errorf("Verify() = %v, want %v", err, signature.ErrInvalid) t.Errorf("Verify() = %v, want %v", err, signature.ErrInvalid)
} }
+20 -15
View File
@@ -34,14 +34,14 @@ func New(secretKey string) *Signer {
} }
} }
// Sign generates an HMAC-SHA256 signature for the given request. // Sign generates an HMAC-SHA256 signature for the given request, expiring at
// The signature covers: host + path + query + width + height + format + // expires. The signature covers: host + path + query + width + height +
// expiration + quality + fit. The quality and fit mode are the effective // format + expiration + quality + fit. The quality and fit mode are the
// values the request resolves to: callers pass the defaults when the request // effective values the request resolves to: callers pass the defaults when
// omits those parameters, so an omitted quality or fit mode signs // the request omits those parameters, so an omitted quality or fit mode signs
// identically to that same value stated explicitly. // identically to that same value stated explicitly.
func (s *Signer) Sign(req *imagerequest.Request) string { func (s *Signer) Sign(req *imagerequest.Request, expires time.Time) string {
data := s.buildSignatureData(req) data := s.buildSignatureData(req, expires)
mac := hmac.New(sha256.New, s.secretKey) mac := hmac.New(sha256.New, s.secretKey)
mac.Write([]byte(data)) mac.Write([]byte(data))
sig := mac.Sum(nil) sig := mac.Sum(nil)
@@ -49,28 +49,31 @@ func (s *Signer) Sign(req *imagerequest.Request) string {
return base64.URLEncoding.EncodeToString(sig) return base64.URLEncoding.EncodeToString(sig)
} }
// Verify checks if the signature on the request is valid and not expired. // Verify checks that sig is the signature of the request expiring at expires,
// and that expires has not passed.
// Signatures are exact-match only: every component of the signed data // Signatures are exact-match only: every component of the signed data
// (host, path, query, dimensions, format, expiration, quality, fit) must // (host, path, query, dimensions, format, expiration, quality, fit) must
// match exactly. // match exactly.
// No suffix matching, wildcard matching, or partial matching is supported. // No suffix matching, wildcard matching, or partial matching is supported.
// A signature for "cdn.example.com" will NOT verify for "example.com" or // A signature for "cdn.example.com" will NOT verify for "example.com" or
// "other.cdn.example.com", and vice versa. // "other.cdn.example.com", and vice versa.
func (s *Signer) Verify(req *imagerequest.Request) error { func (s *Signer) Verify(
req *imagerequest.Request, sig string, expires time.Time,
) error {
// Check expiration first // Check expiration first
if req.Expires.IsZero() { if expires.IsZero() {
return ErrMissingExpiration return ErrMissingExpiration
} }
if time.Now().After(req.Expires) { if time.Now().After(expires) {
return ErrExpired return ErrExpired
} }
// Compute expected signature // Compute expected signature
expected := s.Sign(req) expected := s.Sign(req, expires)
// Constant-time comparison to prevent timing attacks // Constant-time comparison to prevent timing attacks
if !hmac.Equal([]byte(req.Signature), []byte(expected)) { if !hmac.Equal([]byte(sig), []byte(expected)) {
return ErrInvalid return ErrInvalid
} }
@@ -85,7 +88,9 @@ func (s *Signer) Verify(req *imagerequest.Request) error {
// expansion is performed. Quality and fit are the effective transform // expansion is performed. Quality and fit are the effective transform
// values, so replaying a signed URL with a different quality or fit mode // values, so replaying a signed URL with a different quality or fit mode
// fails verification. // fails verification.
func (s *Signer) buildSignatureData(req *imagerequest.Request) string { func (s *Signer) buildSignatureData(
req *imagerequest.Request, expires time.Time,
) string {
return fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%d:%s", return fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%d:%s",
req.Source.Host, req.Source.Host,
escapePercentAndColon(req.Source.Path), escapePercentAndColon(req.Source.Path),
@@ -93,7 +98,7 @@ func (s *Signer) buildSignatureData(req *imagerequest.Request) string {
req.Transform.Size.Width, req.Transform.Size.Width,
req.Transform.Size.Height, req.Transform.Size.Height,
req.Transform.Format, req.Transform.Format,
req.Expires.Unix(), expires.Unix(),
req.Transform.Quality, req.Transform.Quality,
req.Transform.FitMode, req.Transform.FitMode,
) )
+44 -52
View File
@@ -34,11 +34,11 @@ func TestSigner_Sign(t *testing.T) {
Size: imagerequest.Size{Width: 800, Height: 600}, Size: imagerequest.Size{Width: 800, Height: 600},
Format: testFormatWebP, Format: testFormatWebP,
}, },
Expires: time.Unix(1704067200, 0), // Fixed timestamp for reproducibility
} }
expires := time.Unix(1704067200, 0) // Fixed timestamp for reproducibility
sig1 := signer.Sign(req) sig1 := signer.Sign(req, expires)
sig2 := signer.Sign(req) sig2 := signer.Sign(req, expires)
// Same input should produce same signature // Same input should produce same signature
if sig1 != sig2 { if sig1 != sig2 {
@@ -62,18 +62,17 @@ func TestSigner_Sign(t *testing.T) {
Size: imagerequest.Size{Width: 800, Height: 600}, Size: imagerequest.Size{Width: 800, Height: 600},
Format: testFormatWebP, Format: testFormatWebP,
}, },
Expires: time.Unix(1704067200, 0),
} }
sig3 := signer.Sign(req2) sig3 := signer.Sign(req2, expires)
if sig1 == sig3 { if sig1 == sig3 {
t.Error("Sign() produced same signature for different input") t.Error("Sign() produced same signature for different input")
} }
} }
// validVerifyRequest returns a fully-populated request that verifies // validVerifyRequest returns a fully-populated request, and an expiry an hour
// successfully once signed. // ahead, that verify successfully once signed.
func validVerifyRequest() *imagerequest.Request { func validVerifyRequest() (*imagerequest.Request, time.Time) {
return &imagerequest.Request{ return &imagerequest.Request{
Source: imagerequest.Source{ Source: imagerequest.Source{
Host: testHost, Host: testHost,
@@ -83,13 +82,13 @@ func validVerifyRequest() *imagerequest.Request {
Size: imagerequest.Size{Width: 800, Height: 600}, Size: imagerequest.Size{Width: 800, Height: 600},
Format: testFormatWebP, Format: testFormatWebP,
}, },
Expires: time.Now().Add(1 * time.Hour), }, time.Now().Add(1 * time.Hour)
}
} }
type verifyCase struct { type verifyCase struct {
name string name string
setup func() *imagerequest.Request // setup returns the request, the signature and the expiry to verify
setup func() (*imagerequest.Request, string, time.Time)
wantErr error wantErr error
} }
@@ -97,54 +96,49 @@ func verifyCases(signer *signature.Signer) []verifyCase {
return []verifyCase{ return []verifyCase{
{ {
name: "valid signature", name: "valid signature",
setup: func() *imagerequest.Request { setup: func() (*imagerequest.Request, string, time.Time) {
req := validVerifyRequest() req, expires := validVerifyRequest()
req.Signature = signer.Sign(req)
return req return req, signer.Sign(req, expires), expires
}, },
wantErr: nil, wantErr: nil,
}, },
{ {
name: "expired signature", name: "expired signature",
setup: func() *imagerequest.Request { setup: func() (*imagerequest.Request, string, time.Time) {
req := validVerifyRequest() req, _ := validVerifyRequest()
req.Expires = time.Now().Add(-1 * time.Hour) expires := time.Now().Add(-1 * time.Hour)
req.Signature = signer.Sign(req)
return req return req, signer.Sign(req, expires), expires
}, },
wantErr: signature.ErrExpired, wantErr: signature.ErrExpired,
}, },
{ {
name: "invalid signature", name: "invalid signature",
setup: func() *imagerequest.Request { setup: func() (*imagerequest.Request, string, time.Time) {
req := validVerifyRequest() req, expires := validVerifyRequest()
req.Signature = "invalid-signature"
return req return req, "invalid-signature", expires
}, },
wantErr: signature.ErrInvalid, wantErr: signature.ErrInvalid,
}, },
{ {
name: "missing expiration", name: "missing expiration",
setup: func() *imagerequest.Request { setup: func() (*imagerequest.Request, string, time.Time) {
req := validVerifyRequest() req, _ := validVerifyRequest()
req.Expires = time.Time{}
req.Signature = "some-signature"
return req return req, "some-signature", time.Time{}
}, },
wantErr: signature.ErrMissingExpiration, wantErr: signature.ErrMissingExpiration,
}, },
{ {
name: "tampered request", name: "tampered request",
setup: func() *imagerequest.Request { setup: func() (*imagerequest.Request, string, time.Time) {
req := validVerifyRequest() req, expires := validVerifyRequest()
req.Signature = signer.Sign(req) sig := signer.Sign(req, expires)
req.Source.Path = "/photos/secret.jpg" req.Source.Path = "/photos/secret.jpg"
return req return req, sig, expires
}, },
wantErr: signature.ErrInvalid, wantErr: signature.ErrInvalid,
}, },
@@ -160,8 +154,8 @@ func TestSigner_Verify(t *testing.T) {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
t.Parallel() t.Parallel()
req := tt.setup() req, sig, expires := tt.setup()
err := signer.Verify(req) err := signer.Verify(req, sig, expires)
if tt.wantErr == nil { if tt.wantErr == nil {
if err != nil { if err != nil {
@@ -254,8 +248,9 @@ func TestSigner_Verify_ExactMatchOnly(t *testing.T) {
signer := signature.New("test-secret-key") signer := signature.New("test-secret-key")
// Base request that we'll sign, then tamper with individual fields. // Base request that we'll sign, then tamper with individual fields; it
baseReq := func() *imagerequest.Request { // is returned with its signature and expiry.
baseReq := func() (*imagerequest.Request, string, time.Time) {
req := &imagerequest.Request{ req := &imagerequest.Request{
Source: imagerequest.Source{ Source: imagerequest.Source{
Host: testHost, Host: testHost,
@@ -266,21 +261,20 @@ func TestSigner_Verify_ExactMatchOnly(t *testing.T) {
Size: imagerequest.Size{Width: 800, Height: 600}, Size: imagerequest.Size{Width: 800, Height: 600},
Format: testFormatWebP, Format: testFormatWebP,
}, },
Expires: time.Now().Add(1 * time.Hour),
} }
req.Signature = signer.Sign(req) expires := time.Now().Add(1 * time.Hour)
return req return req, signer.Sign(req, expires), expires
} }
for _, tt := range exactMatchTamperCases() { for _, tt := range exactMatchTamperCases() {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
t.Parallel() t.Parallel()
req := baseReq() req, sig, expires := baseReq()
tt.tamper(req) tt.tamper(req)
err := signer.Verify(req) err := signer.Verify(req, sig, expires)
if !errors.Is(err, signature.ErrInvalid) { if !errors.Is(err, signature.ErrInvalid) {
t.Errorf("Verify() = %v, want %v", err, signature.ErrInvalid) t.Errorf("Verify() = %v, want %v", err, signature.ErrInvalid)
} }
@@ -291,9 +285,9 @@ func TestSigner_Verify_ExactMatchOnly(t *testing.T) {
t.Run("unmodified request passes", func(t *testing.T) { t.Run("unmodified request passes", func(t *testing.T) {
t.Parallel() t.Parallel()
req := baseReq() req, sig, expires := baseReq()
err := signer.Verify(req) err := signer.Verify(req, sig, expires)
if err != nil { if err != nil {
t.Errorf("Verify() unmodified request failed: %v", err) t.Errorf("Verify() unmodified request failed: %v", err)
} }
@@ -329,10 +323,9 @@ func TestSigner_Sign_ExactHostInData(t *testing.T) {
Size: imagerequest.Size{Width: 800, Height: 600}, Size: imagerequest.Size{Width: 800, Height: 600},
Format: testFormatWebP, Format: testFormatWebP,
}, },
Expires: time.Unix(1704067200, 0),
} }
sig := signer.Sign(req) sig := signer.Sign(req, time.Unix(1704067200, 0))
if existing, ok := sigs[sig]; ok { if existing, ok := sigs[sig]; ok {
t.Errorf("hosts %q and %q produced the same signature", existing, host) t.Errorf("hosts %q and %q produced the same signature", existing, host)
} }
@@ -356,20 +349,20 @@ func TestSigner_DifferentKeys(t *testing.T) {
Size: imagerequest.Size{Width: 800, Height: 600}, Size: imagerequest.Size{Width: 800, Height: 600},
Format: testFormatWebP, Format: testFormatWebP,
}, },
Expires: time.Now().Add(1 * time.Hour),
} }
expires := time.Now().Add(1 * time.Hour)
// Sign with key 1 // Sign with key 1
req.Signature = signer1.Sign(req) sig := signer1.Sign(req, expires)
// Verify with key 1 should succeed // Verify with key 1 should succeed
err := signer1.Verify(req) err := signer1.Verify(req, sig, expires)
if err != nil { if err != nil {
t.Errorf("Verify() with same key failed: %v", err) t.Errorf("Verify() with same key failed: %v", err)
} }
// Verify with key 2 should fail // Verify with key 2 should fail
err = signer2.Verify(req) err = signer2.Verify(req, sig, expires)
if !errors.Is(err, signature.ErrInvalid) { if !errors.Is(err, signature.ErrInvalid) {
t.Errorf("Verify() with different key should fail, got: %v", err) t.Errorf("Verify() with different key should fail, got: %v", err)
} }
@@ -395,12 +388,11 @@ func TestSign_ReadmeQueryExample(t *testing.T) {
Quality: 85, Quality: 85,
FitMode: testFitCover, FitMode: testFitCover,
}, },
Expires: time.Unix(1704067200, 0),
} }
const wantSig = "37aFa9GSVCZM1kRY9SzMOk2fOg_M7Wt5gKNR4xM-FYA=" const wantSig = "37aFa9GSVCZM1kRY9SzMOk2fOg_M7Wt5gKNR4xM-FYA="
gotSig := signer.Sign(&req) gotSig := signer.Sign(&req, time.Unix(1704067200, 0))
if gotSig != wantSig { if gotSig != wantSig {
t.Errorf("Sign() = %q, want %q", gotSig, wantSig) t.Errorf("Sign() = %q, want %q", gotSig, wantSig)
} }