Take the signature, the expiry and the URL scheme off the image request (closes #314) #331

Merged
clawbot merged 1 commits from issue-314-request-names-the-object into next 2026-10-26 13:49:13 +01:00
46 changed files with 770 additions and 925 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
@@ -75,9 +75,9 @@ func TestFormatForAccept(t *testing.T) {
// autoPhotoURLs returns a signed /v1/image/ URL and an encrypted /v1/e/ URL, // autoPhotoURLs returns a signed /v1/image/ URL and an encrypted /v1/e/ URL,
// both valid for a minute, for the JPEG at photoPath on signedHost at 50x50 in // both valid for a minute, for the JPEG at photoPath on signedHost at 50x50 in
// the format auto. The first is signed with testSigningKey, the key of the // the format auto. The first is signed with testSigningKey, the key the
// image service newSignedHostServer builds, and the second is made with h's // handlers that newSignedHostServer builds check signatures with. The second
// generator. // is made with h's generator.
func autoPhotoURLs(t *testing.T, h *Handlers) (string, string) { func autoPhotoURLs(t *testing.T, h *Handlers) (string, string) {
t.Helper() t.Helper()
@@ -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)
} }
+27 -21
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())
@@ -63,7 +67,7 @@ func newImageRoute(t *testing.T, fetcher httpfetcher.Fetcher) http.Handler {
} }
// newPhotoFetcher returns a mock fetcher that serves a JPEG at photoPath on // newPhotoFetcher returns a mock fetcher that serves a JPEG at photoPath on
// each of hosts, and answers any other URL with an upstream error. // each of hosts, and answers any other source with an upstream error.
func newPhotoFetcher(t *testing.T, hosts ...string) *httpfetcher.MockFetcher { func newPhotoFetcher(t *testing.T, hosts ...string) *httpfetcher.MockFetcher {
t.Helper() t.Helper()
@@ -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"}),
} }
@@ -18,8 +18,8 @@ import (
// signedPhotoURL returns a signed /v1/image/ URL, valid for a minute, for the // signedPhotoURL returns a signed /v1/image/ URL, valid for a minute, for the
// JPEG at photoPath on signedHost at 50x50. It is signed with testSigningKey, // JPEG at photoPath on signedHost at 50x50. It is signed with testSigningKey,
// the key of the image service newSignedHostServer builds. It takes a Handlers // the key the handlers that newSignedHostServer builds check signatures with.
// it does not use, so that it has the type of encPhotoURL. // It takes a Handlers it does not use, so that it has the type of encPhotoURL.
func signedPhotoURL(t *testing.T, _ *Handlers) string { func signedPhotoURL(t *testing.T, _ *Handlers) string {
t.Helper() t.Helper()
@@ -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)
} }
} }
+6 -7
View File
@@ -126,15 +126,15 @@ 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)
} }
// Go's URL parser refuses brackets around anything but an IPv6 address // The allowlist check and the fetcher drop the brackets, so brackets
// around anything but an IPv6 address would give a host a second spelling
if inBrackets := hostMatch[1]; inBrackets != "" { if inBrackets := hostMatch[1]; inBrackets != "" {
addr, err := netip.ParseAddr(inBrackets) addr, err := netip.ParseAddr(inBrackets)
if err != nil || !addr.Is6() { if err != nil || !addr.Is6() {
@@ -147,9 +147,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.
@@ -79,8 +79,8 @@ func setupHeldFetchService(t *testing.T) (*Service, *TestFixtures, *heldFetcher)
} }
// photoVariant asks for the test photo, 100x100, at 50x25 as a JPEG of the // photoVariant asks for the test photo, 100x100, at 50x25 as a JPEG of the
// given quality and fit mode. Each call returns a new request, as Get writes // given quality and fit mode. Each call returns a new request, which a test
// to the request it is given. // may change.
func photoVariant( func photoVariant(
fixtures *TestFixtures, quality int, fit imagerequest.Fit, fixtures *TestFixtures, quality int, fit imagerequest.Fit,
) *imagerequest.Request { ) *imagerequest.Request {
@@ -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 -307
View File
@@ -9,17 +9,15 @@ 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_GoodHost(t *testing.T) {
t.Parallel() t.Parallel()
svc, fixtures := SetupTestService(t) svc, fixtures := SetupTestService(t)
@@ -60,36 +58,12 @@ func TestService_Get_AllowlistedHost(t *testing.T) {
} }
} }
func TestService_Get_NonAllowlistedHost_NoSignature(t *testing.T) { // TestService_Get_OtherHost checks that the image service serves a second host
// as it serves GoodHost.
func TestService_Get_OtherHost(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 +79,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 +97,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 +417,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),
+8 -50
View File
@@ -31,11 +31,11 @@ const (
// TestFixtures contains paths to test files in the mock filesystem. // TestFixtures contains paths to test files in the mock filesystem.
type TestFixtures struct { type TestFixtures struct {
// Valid image files // Valid image files
GoodHostJPEG string // allowlisted host, valid JPEG GoodHostJPEG string // valid JPEG on GoodHost
GoodHostPNG string // allowlisted host, valid PNG GoodHostPNG string // valid PNG on GoodHost
GoodHostGIF string // allowlisted host, valid GIF GoodHostGIF string // valid GIF on GoodHost
OtherHostJPEG string // non-allowlisted host, valid JPEG OtherHostJPEG string // valid JPEG on OtherHost
OtherHostPNG string // non-allowlisted host, valid PNG OtherHostPNG string // valid PNG on OtherHost
// Invalid/edge case files // Invalid/edge case files
InvalidFile string // file with wrong magic bytes InvalidFile string // file with wrong magic bytes
@@ -43,8 +43,8 @@ type TestFixtures struct {
TextFile string // text file masquerading as image TextFile string // text file masquerading as image
// Hostnames // Hostnames
GoodHost string // allowlisted hostname GoodHost string // a host the mock fetcher serves
OtherHost string // non-allowlisted hostname OtherHost string // a second host the mock fetcher serves
} }
// DefaultFixtures returns the standard test fixture paths. // DefaultFixtures returns the standard test fixture paths.
@@ -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)
} }