diff --git a/README.md b/README.md index 2286084..cc1a877 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,7 @@ Image-heavy web applications need a fast, caching reverse proxy that can resize and transcode images on the fly. pixa fills that role as a single, self-contained binary with no external runtime dependencies beyond libvips. It supports HMAC-SHA256 signed URLs with expiration to -prevent abuse, and whitelisted source hosts for open access. +prevent abuse, and allowlisted source hosts for open access. ## Design @@ -61,7 +61,7 @@ Images are only fetched from origins using TLS with valid certificates. ### Source Hosts -Source hosts may be whitelisted in the configuration. Non-whitelisted +Source hosts may be allowlisted in the configuration. Non-allowlisted hosts require an HMAC-SHA256 signature. #### Signature Specification @@ -99,7 +99,7 @@ expiration 1704067200: 4. URL: `/v1/image/cdn.example.com/photos/cat.jpg/800x600.webp?sig=&exp=1704067200` -**Whitelist patterns:** +**Allowlist patterns:** - **Exact match**: `cdn.example.com` — matches only that host - **Suffix match**: `.example.com` — matches `cdn.example.com`, @@ -110,7 +110,7 @@ expiration 1704067200: Configured via YAML file (`--config`). Key settings: - `access_control_allow_origin` — CORS origin -- `source_host_whitelist` — list of allowed upstream hosts +- `allowlist_hosts` — list of allowed upstream hosts - `upstream_fetch_timeout` — timeout for origin requests - `upstream_max_response_size` — max origin response size - `downstream_timeout` — client response timeout diff --git a/config.example.yml b/config.example.yml index 1660cb4..d4295f6 100644 --- a/config.example.yml +++ b/config.example.yml @@ -9,13 +9,13 @@ maintenance_mode: false state_dir: ./data # Image proxy settings -# HMAC signing key for URL signatures (leave empty to require whitelist for all requests) +# HMAC signing key for URL signatures (leave empty to require allowlist for all requests) # Generate with: openssl rand -base64 32 signing_key: "CHANGE_ME_generate_with_openssl_rand_base64_32" # Hosts that don't require signatures # Use "." prefix for wildcard subdomain matching (e.g., ".example.com" matches "cdn.example.com") -whitelist_hosts: +allowlist_hosts: - s3.sneak.cloud - static.sneak.cloud - sneak.berlin diff --git a/internal/config/config.go b/internal/config/config.go index 3eb1bc2..a2c3778 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -41,7 +41,7 @@ type Config struct { // Image proxy settings SigningKey string // HMAC signing key for URL signatures - WhitelistHosts []string // Hosts that don't require signatures + AllowlistHosts []string // Hosts that don't require signatures AllowHTTP bool // Allow non-TLS upstream (testing only) UpstreamConnectionsPerHost int // Max concurrent connections per upstream host } @@ -69,7 +69,7 @@ func New(_ fx.Lifecycle, params Params) (*Config, error) { MetricsUsername: getString(sc, "metrics.username", ""), MetricsPassword: getString(sc, "metrics.password", ""), SigningKey: getString(sc, "signing_key", ""), - WhitelistHosts: getStringSlice(sc, "whitelist_hosts"), + AllowlistHosts: getStringSlice(sc, "allowlist_hosts"), AllowHTTP: getBool(sc, "allow_http", false), UpstreamConnectionsPerHost: getInt(sc, "upstream_connections_per_host", DefaultUpstreamConnectionsPerHost), } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index c3c7f7c..f0d927e 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -14,7 +14,7 @@ func TestGetStringSlice_YAMLList(t *testing.T) { configPath := filepath.Join(tmpDir, "config.yml") yamlContent := ` -whitelist_hosts: +allowlist_hosts: - static.sneak.cloud - sneak.berlin - s3.sneak.cloud @@ -31,7 +31,7 @@ whitelist_hosts: } // Test that getStringSlice correctly parses YAML list - hosts := getStringSlice(sc, "whitelist_hosts") + hosts := getStringSlice(sc, "allowlist_hosts") if len(hosts) != 3 { t.Errorf("expected 3 hosts, got %d: %v", len(hosts), hosts) @@ -54,7 +54,7 @@ func TestGetStringSlice_CommaSeparated(t *testing.T) { tmpDir := t.TempDir() configPath := filepath.Join(tmpDir, "config.yml") - yamlContent := `whitelist_hosts: "static.sneak.cloud, sneak.berlin, s3.sneak.cloud"` + yamlContent := `allowlist_hosts: "static.sneak.cloud, sneak.berlin, s3.sneak.cloud"` err := os.WriteFile(configPath, []byte(yamlContent), 0644) if err != nil { @@ -66,7 +66,7 @@ func TestGetStringSlice_CommaSeparated(t *testing.T) { t.Fatalf("failed to load config: %v", err) } - hosts := getStringSlice(sc, "whitelist_hosts") + hosts := getStringSlice(sc, "allowlist_hosts") if len(hosts) != 3 { t.Errorf("expected 3 hosts, got %d: %v", len(hosts), hosts) @@ -100,7 +100,7 @@ func TestGetStringSlice_Empty(t *testing.T) { t.Fatalf("failed to load config: %v", err) } - hosts := getStringSlice(sc, "whitelist_hosts") + hosts := getStringSlice(sc, "allowlist_hosts") if hosts != nil && len(hosts) != 0 { t.Errorf("expected nil or empty slice, got %v", hosts) diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index 31f07d5..89dafbe 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -84,7 +84,7 @@ func (s *Handlers) initImageService() error { Cache: cache, FetcherConfig: fetcherCfg, SigningKey: s.config.SigningKey, - Whitelist: s.config.WhitelistHosts, + Allowlist: s.config.AllowlistHosts, Logger: s.log, }) if err != nil { diff --git a/internal/handlers/handlers_test.go b/internal/handlers/handlers_test.go index 4994646..6ee5ac6 100644 --- a/internal/handlers/handlers_test.go +++ b/internal/handlers/handlers_test.go @@ -57,7 +57,7 @@ func setupTestHandler(t *testing.T) *testFixtures { Cache: cache, Fetcher: newMockFetcher(mockFS), SigningKey: "test-signing-key-must-be-32-chars", - Whitelist: []string{goodHost}, + Allowlist: []string{goodHost}, }) if err != nil { t.Fatalf("failed to create service: %v", err) diff --git a/internal/imgcache/imgcache.go b/internal/imgcache/imgcache.go index 105afb4..22a4f2d 100644 --- a/internal/imgcache/imgcache.go +++ b/internal/imgcache/imgcache.go @@ -75,7 +75,7 @@ type ImageRequest struct { Quality int // FitMode is how to fit the image into requested dimensions FitMode FitMode - // Signature is the HMAC signature for non-whitelisted hosts + // Signature is the HMAC signature for non-allowlisted hosts Signature string // Expires is the signature expiration timestamp Expires time.Time @@ -163,10 +163,10 @@ type SignatureValidator interface { Generate(req *ImageRequest) string } -// Whitelist checks if a URL is whitelisted (no signature required) -type Whitelist interface { - // IsWhitelisted returns true if the URL doesn't require a signature - IsWhitelisted(u *url.URL) bool +// Allowlist checks if a URL is allowlisted (no signature required) +type Allowlist interface { + // IsAllowlisted returns true if the URL doesn't require a signature + IsAllowlisted(u *url.URL) bool } // Storage handles persistent storage of cached content diff --git a/internal/imgcache/service.go b/internal/imgcache/service.go index 4229911..9674fc9 100644 --- a/internal/imgcache/service.go +++ b/internal/imgcache/service.go @@ -15,6 +15,7 @@ import ( "sneak.berlin/go/pixa/internal/httpfetcher" "sneak.berlin/go/pixa/internal/imageprocessor" "sneak.berlin/go/pixa/internal/magic" + "sneak.berlin/go/pixa/internal/signature" ) // Service implements the ImageCache interface, orchestrating cache, fetcher, and processor. @@ -22,7 +23,7 @@ type Service struct { cache *Cache fetcher httpfetcher.Fetcher processor *imageprocessor.ImageProcessor - signer *Signer + signer *signature.Signer allowlist *allowlist.HostAllowList log *slog.Logger allowHTTP bool @@ -39,8 +40,8 @@ type ServiceConfig struct { Fetcher httpfetcher.Fetcher // SigningKey is the HMAC signing key (empty disables signing) SigningKey string - // Whitelist is the list of hosts that don't require signatures - Whitelist []string + // Allowlist is the list of hosts that don't require signatures + Allowlist []string // Logger for logging Logger *slog.Logger } @@ -69,7 +70,7 @@ func NewService(cfg *ServiceConfig) (*Service, error) { fetcher = httpfetcher.New(fetcherCfg) } - signer := NewSigner(cfg.SigningKey) + signer := signature.New(cfg.SigningKey) log := cfg.Logger if log == nil { @@ -88,7 +89,7 @@ func NewService(cfg *ServiceConfig) (*Service, error) { fetcher: fetcher, processor: imageprocessor.New(imageprocessor.Params{MaxInputBytes: maxResponseSize}), signer: signer, - allowlist: allowlist.New(cfg.Whitelist), + allowlist: allowlist.New(cfg.Allowlist), log: log, allowHTTP: allowHTTP, maxResponseSize: maxResponseSize, @@ -397,7 +398,7 @@ func (s *Service) ValidateRequest(req *ImageRequest) error { } // Signature required for non-allowed hosts - return s.signer.Verify(req) + return s.signer.Verify(signatureRequest(req)) } // GenerateSignedURL generates a signed URL for the given request. @@ -406,11 +407,32 @@ func (s *Service) GenerateSignedURL( req *ImageRequest, ttl time.Duration, ) (string, error) { - path, sig, exp := s.signer.GenerateSignedURL(req, ttl) + sigReq := signatureRequest(req) + path, sig, exp := s.signer.GenerateSignedURL(sigReq, ttl) + + // Propagate the generated signature and expiration back onto the request. + req.Expires = sigReq.Expires + req.Signature = sigReq.Signature return fmt.Sprintf("%s%s?sig=%s&exp=%d", baseURL, path, sig, exp), nil } +// signatureRequest projects an ImageRequest onto the standalone +// signature.Request type used by the signature package. This keeps the +// import edge one-way: imgcache depends on signature, never the reverse. +func signatureRequest(req *ImageRequest) *signature.Request { + return &signature.Request{ + SourceHost: req.SourceHost, + SourcePath: req.SourcePath, + SourceQuery: req.SourceQuery, + Width: req.Size.Width, + Height: req.Size.Height, + Format: string(req.Format), + Signature: req.Signature, + Expires: req.Expires, + } +} + // HTTP status codes for error responses. const ( httpStatusBadGateway = 502 diff --git a/internal/imgcache/service_test.go b/internal/imgcache/service_test.go index 5e0b7be..e9f2567 100644 --- a/internal/imgcache/service_test.go +++ b/internal/imgcache/service_test.go @@ -7,9 +7,10 @@ import ( "time" "sneak.berlin/go/pixa/internal/magic" + "sneak.berlin/go/pixa/internal/signature" ) -func TestService_Get_WhitelistedHost(t *testing.T) { +func TestService_Get_AllowlistedHost(t *testing.T) { svc, fixtures := SetupTestService(t) ctx := context.Background() @@ -43,7 +44,7 @@ func TestService_Get_WhitelistedHost(t *testing.T) { } } -func TestService_Get_NonWhitelistedHost_NoSignature(t *testing.T) { +func TestService_Get_NonAllowlistedHost_NoSignature(t *testing.T) { svc, fixtures := SetupTestService(t, WithSigningKey("test-key")) req := &ImageRequest{ @@ -55,14 +56,14 @@ func TestService_Get_NonWhitelistedHost_NoSignature(t *testing.T) { FitMode: FitCover, } - // Should fail validation - not whitelisted and no signature + // Should fail validation - not allowlisted and no signature err := svc.ValidateRequest(req) if err == nil { - t.Error("ValidateRequest() expected error for non-whitelisted host without signature") + t.Error("ValidateRequest() expected error for non-allowlisted host without signature") } } -func TestService_Get_NonWhitelistedHost_ValidSignature(t *testing.T) { +func TestService_Get_NonAllowlistedHost_ValidSignature(t *testing.T) { signingKey := "test-signing-key-12345" svc, fixtures := SetupTestService(t, WithSigningKey(signingKey)) ctx := context.Background() @@ -77,9 +78,9 @@ func TestService_Get_NonWhitelistedHost_ValidSignature(t *testing.T) { } // Generate a valid signature - signer := NewSigner(signingKey) + signer := signature.New(signingKey) req.Expires = time.Now().Add(time.Hour) - req.Signature = signer.Sign(req) + req.Signature = signer.Sign(signatureRequest(req)) // Should pass validation err := svc.ValidateRequest(req) @@ -104,7 +105,7 @@ func TestService_Get_NonWhitelistedHost_ValidSignature(t *testing.T) { } } -func TestService_Get_NonWhitelistedHost_ExpiredSignature(t *testing.T) { +func TestService_Get_NonAllowlistedHost_ExpiredSignature(t *testing.T) { signingKey := "test-signing-key-12345" svc, fixtures := SetupTestService(t, WithSigningKey(signingKey)) @@ -118,9 +119,9 @@ func TestService_Get_NonWhitelistedHost_ExpiredSignature(t *testing.T) { } // Generate an expired signature - signer := NewSigner(signingKey) + signer := signature.New(signingKey) req.Expires = time.Now().Add(-time.Hour) // Already expired - req.Signature = signer.Sign(req) + req.Signature = signer.Sign(signatureRequest(req)) // Should fail validation err := svc.ValidateRequest(req) @@ -129,7 +130,7 @@ func TestService_Get_NonWhitelistedHost_ExpiredSignature(t *testing.T) { } } -func TestService_Get_NonWhitelistedHost_InvalidSignature(t *testing.T) { +func TestService_Get_NonAllowlistedHost_InvalidSignature(t *testing.T) { signingKey := "test-signing-key-12345" svc, fixtures := SetupTestService(t, WithSigningKey(signingKey)) @@ -161,10 +162,10 @@ func TestService_ValidateRequest_SignatureExactHostMatch(t *testing.T) { signingKey := "test-signing-key-must-be-32-chars" svc, _ := SetupTestService(t, WithSigningKey(signingKey), - WithNoWhitelist(), + WithNoAllowlist(), ) - signer := NewSigner(signingKey) + signer := signature.New(signingKey) // Sign a request for "cdn.example.com" signedReq := &ImageRequest{ @@ -176,7 +177,7 @@ func TestService_ValidateRequest_SignatureExactHostMatch(t *testing.T) { FitMode: FitCover, Expires: time.Now().Add(time.Hour), } - signedReq.Signature = signer.Sign(signedReq) + signedReq.Signature = signer.Sign(signatureRequest(signedReq)) // The original request should pass validation t.Run("exact host passes", func(t *testing.T) { @@ -437,8 +438,8 @@ func TestService_Get_DifferentSizes(t *testing.T) { } func TestService_ValidateRequest_NoSigningKey(t *testing.T) { - // Service with no signing key - all non-whitelisted requests should fail - svc, fixtures := SetupTestService(t, WithNoWhitelist()) + // Service with no signing key - all non-allowlisted requests should fail + svc, fixtures := SetupTestService(t, WithNoAllowlist()) req := &ImageRequest{ SourceHost: fixtures.OtherHost, @@ -451,7 +452,7 @@ func TestService_ValidateRequest_NoSigningKey(t *testing.T) { err := svc.ValidateRequest(req) if err == nil { - t.Error("ValidateRequest() expected error when no signing key and host not whitelisted") + t.Error("ValidateRequest() expected error when no signing key and host not allowlisted") } } diff --git a/internal/imgcache/signature_query_test.go b/internal/imgcache/signature_query_test.go deleted file mode 100644 index da05d6f..0000000 --- a/internal/imgcache/signature_query_test.go +++ /dev/null @@ -1,55 +0,0 @@ -package imgcache - -import ( - "strings" - "testing" - "time" -) - -func TestGenerateSignedURL_WithQueryString(t *testing.T) { - signer := NewSigner("test-secret-key-for-testing!") - - req := &ImageRequest{ - SourceHost: "cdn.example.com", - SourcePath: "/photos/cat.jpg", - SourceQuery: "token=abc&v=2", - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, - } - - path, _, _ := signer.GenerateSignedURL(req, time.Hour) - - // The path must NOT contain a bare "?" that would be interpreted as a query string delimiter. - // The size segment must appear as the last path component. - if strings.Contains(path, "?token=abc") { - t.Errorf("GenerateSignedURL() produced bare query string in path: %q", path) - } - - // The size segment must be present in the path - if !strings.Contains(path, "/800x600.webp") { - t.Errorf("GenerateSignedURL() missing size segment in path: %q", path) - } - - // Path should end with the size.format, not with query params - if !strings.HasSuffix(path, "/800x600.webp") { - t.Errorf("GenerateSignedURL() path should end with size.format: %q", path) - } -} - -func TestGenerateSignedURL_WithoutQueryString(t *testing.T) { - signer := NewSigner("test-secret-key-for-testing!") - - req := &ImageRequest{ - SourceHost: "cdn.example.com", - SourcePath: "/photos/cat.jpg", - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, - } - - path, _, _ := signer.GenerateSignedURL(req, time.Hour) - - expected := "/v1/image/cdn.example.com/photos/cat.jpg/800x600.webp" - if path != expected { - t.Errorf("GenerateSignedURL() path = %q, want %q", path, expected) - } -} diff --git a/internal/imgcache/testutil_test.go b/internal/imgcache/testutil_test.go index 2e23df5..181f691 100644 --- a/internal/imgcache/testutil_test.go +++ b/internal/imgcache/testutil_test.go @@ -21,11 +21,11 @@ import ( // TestFixtures contains paths to test files in the mock filesystem. type TestFixtures struct { // Valid image files - GoodHostJPEG string // whitelisted host, valid JPEG - GoodHostPNG string // whitelisted host, valid PNG - GoodHostGIF string // whitelisted host, valid GIF - OtherHostJPEG string // non-whitelisted host, valid JPEG - OtherHostPNG string // non-whitelisted host, valid PNG + GoodHostJPEG string // allowlisted host, valid JPEG + GoodHostPNG string // allowlisted host, valid PNG + GoodHostGIF string // allowlisted host, valid GIF + OtherHostJPEG string // non-allowlisted host, valid JPEG + OtherHostPNG string // non-allowlisted host, valid PNG // Invalid/edge case files InvalidFile string // file with wrong magic bytes @@ -33,8 +33,8 @@ type TestFixtures struct { TextFile string // text file masquerading as image // Hostnames - GoodHost string // whitelisted hostname - OtherHost string // non-whitelisted hostname + GoodHost string // allowlisted hostname + OtherHost string // non-allowlisted hostname } // DefaultFixtures returns the standard test fixture paths. @@ -148,7 +148,7 @@ func SetupTestService(t *testing.T, opts ...TestServiceOption) (*Service, *TestF mockFS, fixtures := NewTestFS(t) cfg := &testServiceConfig{ - whitelist: []string{fixtures.GoodHost}, + allowlist: []string{fixtures.GoodHost}, signingKey: "test-signing-key-must-be-32-chars", } @@ -175,7 +175,7 @@ func SetupTestService(t *testing.T, opts ...TestServiceOption) (*Service, *TestF Cache: cache, Fetcher: httpfetcher.NewMock(mockFS), SigningKey: cfg.signingKey, - Whitelist: cfg.whitelist, + Allowlist: cfg.allowlist, }) if err != nil { t.Fatalf("failed to create service: %v", err) @@ -203,17 +203,17 @@ func setupServiceTestDB(t *testing.T) *sql.DB { } type testServiceConfig struct { - whitelist []string + allowlist []string signingKey string } // TestServiceOption configures the test service. type TestServiceOption func(*testServiceConfig) -// WithWhitelist sets the whitelist for the test service. -func WithWhitelist(hosts ...string) TestServiceOption { +// WithAllowlist sets the allowlist for the test service. +func WithAllowlist(hosts ...string) TestServiceOption { return func(c *testServiceConfig) { - c.whitelist = hosts + c.allowlist = hosts } } @@ -224,9 +224,9 @@ func WithSigningKey(key string) TestServiceOption { } } -// WithNoWhitelist removes all whitelisted hosts. -func WithNoWhitelist() TestServiceOption { +// WithNoAllowlist removes all allowlisted hosts. +func WithNoAllowlist() TestServiceOption { return func(c *testServiceConfig) { - c.whitelist = nil + c.allowlist = nil } } diff --git a/internal/signature/golden_test.go b/internal/signature/golden_test.go new file mode 100644 index 0000000..409b992 --- /dev/null +++ b/internal/signature/golden_test.go @@ -0,0 +1,105 @@ +package signature + +import ( + "testing" + "time" +) + +// goldenExpiresUnix is the fixed expiration timestamp used by all golden +// vectors: 2024-01-01T00:00:00Z. +const goldenExpiresUnix int64 = 1704067200 + +// goldenSigningKey is the fixed signing key used by all golden vectors. +const goldenSigningKey = "golden-test-key" + +// TestSigner_GoldenVectors pins the exact HMAC-SHA256 signature output and +// the exact generated signed URL path for fully-specified requests with a +// hardcoded signing key. The expected values were computed once and are +// hardcoded here as known answers. +// +// If any of these assertions fail, the signed byte format +// ("host:path:query:width:height:format:expiration"), the base64url +// encoding, or the signed URL layout has changed. Such a change breaks +// every signature already issued to clients, so it must be made +// deliberately: update these constants only as part of an intentional, +// documented signature format migration. +func TestSigner_GoldenVectors(t *testing.T) { + signer := New(goldenSigningKey) + + vectors := []struct { + name string + req Request + // wantSignature is the exact base64url (RFC 4648 URL-safe, + // padded) HMAC-SHA256 signature for the request with Expires + // set to goldenExpiresUnix. + wantSignature string + // wantSignedPath is the exact path returned by + // GenerateSignedURL for the request. The signature and + // expiration are returned separately by GenerateSignedURL and + // are not embedded in the path. + wantSignedPath string + }{ + { + name: "resized without query", + req: Request{ + SourceHost: "cdn.example.com", + SourcePath: "/photos/cat.jpg", + SourceQuery: "", + Width: 800, + Height: 600, + Format: "webp", + }, + // Signed data: "cdn.example.com:/photos/cat.jpg::800:600:webp:1704067200" + wantSignature: "x5PfPp8QSDo0cJT96od-AEgrQyOVLfqifH5sst61_-w=", + wantSignedPath: "/v1/image/cdn.example.com/photos/cat.jpg/800x600.webp", + }, + { + name: "resized with query string", + req: Request{ + SourceHost: "cdn.example.com", + SourcePath: "/photos/cat.jpg", + SourceQuery: "token=abc&v=2", + Width: 800, + Height: 600, + Format: "webp", + }, + // Signed data: "cdn.example.com:/photos/cat.jpg:token=abc&v=2:800:600:webp:1704067200" + wantSignature: "394_Vf9TdQFkpQ3XKFDQSyxgqKq8N7mApf2S4QaHqyo=", + wantSignedPath: "/v1/image/cdn.example.com/photos/cat.jpg%3Ftoken=abc&v=2/800x600.webp", + }, + { + name: "original size without query", + req: Request{ + SourceHost: "cdn.example.com", + SourcePath: "/photos/cat.jpg", + SourceQuery: "", + Width: 0, + Height: 0, + Format: "png", + }, + // Signed data: "cdn.example.com:/photos/cat.jpg::0:0:png:1704067200" + wantSignature: "7Be7oteeQwvnSPU4bchyQ4ZGYGsAGBKpeEtuQ02ox60=", + wantSignedPath: "/v1/image/cdn.example.com/photos/cat.jpg/orig.png", + }, + } + + for _, tt := range vectors { + t.Run(tt.name, func(t *testing.T) { + signReq := tt.req + signReq.Expires = time.Unix(goldenExpiresUnix, 0) + + gotSignature := signer.Sign(&signReq) + if gotSignature != tt.wantSignature { + t.Errorf("Sign() = %q, want %q (signed byte format changed?)", + gotSignature, tt.wantSignature) + } + + urlReq := tt.req + gotPath, _, _ := signer.GenerateSignedURL(&urlReq, time.Hour) + if gotPath != tt.wantSignedPath { + t.Errorf("GenerateSignedURL() path = %q, want %q (signed URL layout changed?)", + gotPath, tt.wantSignedPath) + } + }) + } +} diff --git a/internal/imgcache/signature.go b/internal/signature/signature.go similarity index 59% rename from internal/imgcache/signature.go rename to internal/signature/signature.go index ebda10a..092f067 100644 --- a/internal/imgcache/signature.go +++ b/internal/signature/signature.go @@ -1,4 +1,6 @@ -package imgcache +// Package signature provides HMAC-SHA256 signing and verification of image +// requests. +package signature import ( "crypto/hmac" @@ -13,27 +15,49 @@ import ( // Signature errors. var ( - ErrSignatureRequired = errors.New("signature required for non-whitelisted host") - ErrSignatureInvalid = errors.New("invalid signature") - ErrSignatureExpired = errors.New("signature has expired") + ErrRequired = errors.New("signature required for non-allowlisted host") + ErrInvalid = errors.New("invalid signature") + ErrExpired = errors.New("signature has expired") ErrMissingExpiration = errors.New("signature expiration is required") ) +// Request carries the components an image request signature covers. It is a +// standalone type so that this package does not depend on imgcache, keeping +// the import edge one-way (imgcache depends on signature, never the reverse). +type Request struct { + // SourceHost is the origin host (e.g. "cdn.example.com"). + SourceHost string + // SourcePath is the path on the origin (e.g. "/photos/cat.jpg"). + SourcePath string + // SourceQuery is the optional query string for the origin URL. + SourceQuery string + // Width is the requested output width in pixels. + Width int + // Height is the requested output height in pixels. + Height int + // Format is the requested output format (e.g. "webp"). + Format string + // Signature is the HMAC signature to verify. + Signature string + // Expires is the signature expiration timestamp. + Expires time.Time +} + // Signer handles HMAC-SHA256 signature generation and verification. type Signer struct { secretKey []byte } -// NewSigner creates a new Signer with the given secret key. -func NewSigner(secretKey string) *Signer { +// New creates a new Signer with the given secret key. +func New(secretKey string) *Signer { return &Signer{ secretKey: []byte(secretKey), } } -// Sign generates an HMAC-SHA256 signature for the given image request. +// Sign generates an HMAC-SHA256 signature for the given request. // The signature covers: host + path + query + width + height + format + expiration. -func (s *Signer) Sign(req *ImageRequest) string { +func (s *Signer) Sign(req *Request) string { data := s.buildSignatureData(req) mac := hmac.New(sha256.New, s.secretKey) mac.Write([]byte(data)) @@ -48,14 +72,14 @@ func (s *Signer) Sign(req *ImageRequest) string { // No suffix matching, wildcard matching, or partial matching is supported. // A signature for "cdn.example.com" will NOT verify for "example.com" or // "other.cdn.example.com", and vice versa. -func (s *Signer) Verify(req *ImageRequest) error { +func (s *Signer) Verify(req *Request) error { // Check expiration first if req.Expires.IsZero() { return ErrMissingExpiration } if time.Now().After(req.Expires) { - return ErrSignatureExpired + return ErrExpired } // Compute expected signature @@ -63,7 +87,7 @@ func (s *Signer) Verify(req *ImageRequest) error { // Constant-time comparison to prevent timing attacks if !hmac.Equal([]byte(req.Signature), []byte(expected)) { - return ErrSignatureInvalid + return ErrInvalid } return nil @@ -73,13 +97,13 @@ func (s *Signer) Verify(req *ImageRequest) error { // Format: "host:path:query:width:height:format:expiration" // All components are used verbatim (exact match). No normalization, // suffix matching, or wildcard expansion is performed. -func (s *Signer) buildSignatureData(req *ImageRequest) string { +func (s *Signer) buildSignatureData(req *Request) string { return fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d", req.SourceHost, req.SourcePath, req.SourceQuery, - req.Size.Width, - req.Size.Height, + req.Width, + req.Height, req.Format, req.Expires.Unix(), ) @@ -87,7 +111,7 @@ func (s *Signer) buildSignatureData(req *ImageRequest) string { // GenerateSignedURL creates a complete URL with signature and expiration. // Returns the path portion that should be appended to the base URL. -func (s *Signer) GenerateSignedURL(req *ImageRequest, ttl time.Duration) (path string, sig string, exp int64) { +func (s *Signer) GenerateSignedURL(req *Request, ttl time.Duration) (path string, sig string, exp int64) { // Set expiration req.Expires = time.Now().Add(ttl) exp = req.Expires.Unix() @@ -98,15 +122,15 @@ func (s *Signer) GenerateSignedURL(req *ImageRequest, ttl time.Duration) (path s // Build the size component var sizeStr string - if req.Size.OriginalSize() { + if req.Width == 0 && req.Height == 0 { sizeStr = "orig" } else { - sizeStr = fmt.Sprintf("%dx%d", req.Size.Width, req.Size.Height) + sizeStr = fmt.Sprintf("%dx%d", req.Width, req.Height) } // Build the path. // When a source query is present, it is embedded as a path segment - // (e.g. /host/path?query/size.fmt) so that ParseImagePath can extract + // (e.g. /host/path?query/size.fmt) so that the URL parser can extract // it from the last-slash split. The "?" inside a path segment is // percent-encoded by clients but chi delivers it decoded, which is // exactly what the URL parser expects. @@ -130,12 +154,12 @@ func (s *Signer) GenerateSignedURL(req *ImageRequest, ttl time.Duration) (path s return path, sig, exp } -// ParseSignatureParams extracts signature and expiration from query parameters. -func ParseSignatureParams(sig, expStr string) (signature string, expires time.Time, err error) { - signature = sig +// ParseParams extracts signature and expiration from query parameters. +func ParseParams(sig, expStr string) (parsed string, expires time.Time, err error) { + parsed = sig if expStr == "" { - return signature, time.Time{}, nil + return parsed, time.Time{}, nil } expUnix, err := strconv.ParseInt(expStr, 10, 64) @@ -145,5 +169,5 @@ func ParseSignatureParams(sig, expStr string) (signature string, expires time.Ti expires = time.Unix(expUnix, 0) - return signature, expires, nil + return parsed, expires, nil } diff --git a/internal/imgcache/signature_test.go b/internal/signature/signature_test.go similarity index 68% rename from internal/imgcache/signature_test.go rename to internal/signature/signature_test.go index 6dc3632..0f2f223 100644 --- a/internal/imgcache/signature_test.go +++ b/internal/signature/signature_test.go @@ -1,19 +1,21 @@ -package imgcache +package signature import ( + "strings" "testing" "time" ) func TestSigner_Sign(t *testing.T) { - signer := NewSigner("test-secret-key") + signer := New("test-secret-key") - req := &ImageRequest{ + req := &Request{ SourceHost: "cdn.example.com", SourcePath: "/photos/cat.jpg", SourceQuery: "", - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, + Width: 800, + Height: 600, + Format: "webp", Expires: time.Unix(1704067200, 0), // Fixed timestamp for reproducibility } @@ -31,12 +33,13 @@ func TestSigner_Sign(t *testing.T) { } // Different input should produce different signature - req2 := &ImageRequest{ + req2 := &Request{ SourceHost: "cdn.example.com", SourcePath: "/photos/dog.jpg", // Different path SourceQuery: "", - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, + Width: 800, + Height: 600, + Format: "webp", Expires: time.Unix(1704067200, 0), } @@ -47,21 +50,22 @@ func TestSigner_Sign(t *testing.T) { } func TestSigner_Verify(t *testing.T) { - signer := NewSigner("test-secret-key") + signer := New("test-secret-key") tests := []struct { name string - setup func() *ImageRequest + setup func() *Request wantErr error }{ { name: "valid signature", - setup: func() *ImageRequest { - req := &ImageRequest{ + setup: func() *Request { + req := &Request{ SourceHost: "cdn.example.com", SourcePath: "/photos/cat.jpg", - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, + Width: 800, + Height: 600, + Format: "webp", Expires: time.Now().Add(1 * time.Hour), } req.Signature = signer.Sign(req) @@ -72,42 +76,45 @@ func TestSigner_Verify(t *testing.T) { }, { name: "expired signature", - setup: func() *ImageRequest { - req := &ImageRequest{ + setup: func() *Request { + req := &Request{ SourceHost: "cdn.example.com", SourcePath: "/photos/cat.jpg", - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, + Width: 800, + Height: 600, + Format: "webp", Expires: time.Now().Add(-1 * time.Hour), // Expired } req.Signature = signer.Sign(req) return req }, - wantErr: ErrSignatureExpired, + wantErr: ErrExpired, }, { name: "invalid signature", - setup: func() *ImageRequest { - return &ImageRequest{ + setup: func() *Request { + return &Request{ SourceHost: "cdn.example.com", SourcePath: "/photos/cat.jpg", - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, + Width: 800, + Height: 600, + Format: "webp", Expires: time.Now().Add(1 * time.Hour), Signature: "invalid-signature", } }, - wantErr: ErrSignatureInvalid, + wantErr: ErrInvalid, }, { name: "missing expiration", - setup: func() *ImageRequest { - return &ImageRequest{ + setup: func() *Request { + return &Request{ SourceHost: "cdn.example.com", SourcePath: "/photos/cat.jpg", - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, + Width: 800, + Height: 600, + Format: "webp", Signature: "some-signature", // Expires is zero } @@ -116,12 +123,13 @@ func TestSigner_Verify(t *testing.T) { }, { name: "tampered request", - setup: func() *ImageRequest { - req := &ImageRequest{ + setup: func() *Request { + req := &Request{ SourceHost: "cdn.example.com", SourcePath: "/photos/cat.jpg", - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, + Width: 800, + Height: 600, + Format: "webp", Expires: time.Now().Add(1 * time.Hour), } req.Signature = signer.Sign(req) @@ -130,7 +138,7 @@ func TestSigner_Verify(t *testing.T) { return req }, - wantErr: ErrSignatureInvalid, + wantErr: ErrInvalid, }, } @@ -156,16 +164,17 @@ func TestSigner_Verify(t *testing.T) { // matching on every URL component. No suffix matching, wildcard matching, // or partial matching is supported. func TestSigner_Verify_ExactMatchOnly(t *testing.T) { - signer := NewSigner("test-secret-key") + signer := New("test-secret-key") // Base request that we'll sign, then tamper with individual fields. - baseReq := func() *ImageRequest { - req := &ImageRequest{ + baseReq := func() *Request { + req := &Request{ SourceHost: "cdn.example.com", SourcePath: "/photos/cat.jpg", SourceQuery: "token=abc", - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, + Width: 800, + Height: 600, + Format: "webp", Expires: time.Now().Add(1 * time.Hour), } req.Signature = signer.Sign(req) @@ -175,95 +184,95 @@ func TestSigner_Verify_ExactMatchOnly(t *testing.T) { tests := []struct { name string - tamper func(req *ImageRequest) + tamper func(req *Request) }{ { name: "parent domain does not match subdomain", - tamper: func(req *ImageRequest) { + tamper: func(req *Request) { // Signed for cdn.example.com, try example.com req.SourceHost = "example.com" }, }, { name: "subdomain does not match parent domain", - tamper: func(req *ImageRequest) { + tamper: func(req *Request) { // Signed for cdn.example.com, try images.cdn.example.com req.SourceHost = "images.cdn.example.com" }, }, { name: "sibling subdomain does not match", - tamper: func(req *ImageRequest) { + tamper: func(req *Request) { // Signed for cdn.example.com, try images.example.com req.SourceHost = "images.example.com" }, }, { name: "host with suffix appended does not match", - tamper: func(req *ImageRequest) { + tamper: func(req *Request) { // Signed for cdn.example.com, try cdn.example.com.evil.com req.SourceHost = "cdn.example.com.evil.com" }, }, { name: "host with prefix does not match", - tamper: func(req *ImageRequest) { + tamper: func(req *Request) { // Signed for cdn.example.com, try evilcdn.example.com req.SourceHost = "evilcdn.example.com" }, }, { name: "different path does not match", - tamper: func(req *ImageRequest) { + tamper: func(req *Request) { req.SourcePath = "/photos/dog.jpg" }, }, { name: "path suffix does not match", - tamper: func(req *ImageRequest) { + tamper: func(req *Request) { req.SourcePath = "/photos/cat.jpg/extra" }, }, { name: "path prefix does not match", - tamper: func(req *ImageRequest) { + tamper: func(req *Request) { req.SourcePath = "/other/photos/cat.jpg" }, }, { name: "different query does not match", - tamper: func(req *ImageRequest) { + tamper: func(req *Request) { req.SourceQuery = "token=xyz" }, }, { name: "added query does not match empty query", - tamper: func(req *ImageRequest) { + tamper: func(req *Request) { req.SourceQuery = "extra=1" }, }, { name: "removed query does not match", - tamper: func(req *ImageRequest) { + tamper: func(req *Request) { req.SourceQuery = "" }, }, { name: "different width does not match", - tamper: func(req *ImageRequest) { - req.Size.Width = 801 + tamper: func(req *Request) { + req.Width = 801 }, }, { name: "different height does not match", - tamper: func(req *ImageRequest) { - req.Size.Height = 601 + tamper: func(req *Request) { + req.Height = 601 }, }, { name: "different format does not match", - tamper: func(req *ImageRequest) { - req.Format = FormatPNG + tamper: func(req *Request) { + req.Format = "png" }, }, } @@ -274,8 +283,8 @@ func TestSigner_Verify_ExactMatchOnly(t *testing.T) { tt.tamper(req) err := signer.Verify(req) - if err != ErrSignatureInvalid { - t.Errorf("Verify() = %v, want %v", err, ErrSignatureInvalid) + if err != ErrInvalid { + t.Errorf("Verify() = %v, want %v", err, ErrInvalid) } }) } @@ -293,7 +302,7 @@ func TestSigner_Verify_ExactMatchOnly(t *testing.T) { // string in the signature data, producing different signatures for // suffix-related hosts. func TestSigner_Sign_ExactHostInData(t *testing.T) { - signer := NewSigner("test-secret-key") + signer := New("test-secret-key") hosts := []string{ "cdn.example.com", @@ -306,12 +315,13 @@ func TestSigner_Sign_ExactHostInData(t *testing.T) { sigs := make(map[string]string) for _, host := range hosts { - req := &ImageRequest{ + req := &Request{ SourceHost: host, SourcePath: "/photos/cat.jpg", SourceQuery: "", - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, + Width: 800, + Height: 600, + Format: "webp", Expires: time.Unix(1704067200, 0), } @@ -325,14 +335,15 @@ func TestSigner_Sign_ExactHostInData(t *testing.T) { } func TestSigner_DifferentKeys(t *testing.T) { - signer1 := NewSigner("secret-key-1") - signer2 := NewSigner("secret-key-2") + signer1 := New("secret-key-1") + signer2 := New("secret-key-2") - req := &ImageRequest{ + req := &Request{ SourceHost: "cdn.example.com", SourcePath: "/photos/cat.jpg", - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, + Width: 800, + Height: 600, + Format: "webp", Expires: time.Now().Add(1 * time.Hour), } @@ -345,20 +356,21 @@ func TestSigner_DifferentKeys(t *testing.T) { } // Verify with key 2 should fail - if err := signer2.Verify(req); err != ErrSignatureInvalid { + if err := signer2.Verify(req); err != ErrInvalid { t.Errorf("Verify() with different key should fail, got: %v", err) } } func TestGenerateSignedURL(t *testing.T) { - signer := NewSigner("test-secret-key") + signer := New("test-secret-key") - req := &ImageRequest{ + req := &Request{ SourceHost: "cdn.example.com", SourcePath: "/photos/cat.jpg", SourceQuery: "", - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, + Width: 800, + Height: 600, + Format: "webp", } ttl := 1 * time.Hour @@ -389,13 +401,14 @@ func TestGenerateSignedURL(t *testing.T) { } func TestGenerateSignedURL_OrigSize(t *testing.T) { - signer := NewSigner("test-secret-key") + signer := New("test-secret-key") - req := &ImageRequest{ + req := &Request{ SourceHost: "cdn.example.com", SourcePath: "/photos/cat.jpg", - Size: Size{Width: 0, Height: 0}, // Original size - Format: FormatPNG, + Width: 0, // Original size + Height: 0, + Format: "png", } path, _, _ := signer.GenerateSignedURL(req, time.Hour) @@ -406,7 +419,57 @@ func TestGenerateSignedURL_OrigSize(t *testing.T) { } } -func TestParseSignatureParams(t *testing.T) { +func TestGenerateSignedURL_WithQueryString(t *testing.T) { + signer := New("test-secret-key-for-testing!") + + req := &Request{ + SourceHost: "cdn.example.com", + SourcePath: "/photos/cat.jpg", + SourceQuery: "token=abc&v=2", + Width: 800, + Height: 600, + Format: "webp", + } + + path, _, _ := signer.GenerateSignedURL(req, time.Hour) + + // The path must NOT contain a bare "?" that would be interpreted as a query string delimiter. + // The size segment must appear as the last path component. + if strings.Contains(path, "?token=abc") { + t.Errorf("GenerateSignedURL() produced bare query string in path: %q", path) + } + + // The size segment must be present in the path + if !strings.Contains(path, "/800x600.webp") { + t.Errorf("GenerateSignedURL() missing size segment in path: %q", path) + } + + // Path should end with the size.format, not with query params + if !strings.HasSuffix(path, "/800x600.webp") { + t.Errorf("GenerateSignedURL() path should end with size.format: %q", path) + } +} + +func TestGenerateSignedURL_WithoutQueryString(t *testing.T) { + signer := New("test-secret-key-for-testing!") + + req := &Request{ + SourceHost: "cdn.example.com", + SourcePath: "/photos/cat.jpg", + Width: 800, + Height: 600, + Format: "webp", + } + + path, _, _ := signer.GenerateSignedURL(req, time.Hour) + + expected := "/v1/image/cdn.example.com/photos/cat.jpg/800x600.webp" + if path != expected { + t.Errorf("GenerateSignedURL() path = %q, want %q", path, expected) + } +} + +func TestParseParams(t *testing.T) { tests := []struct { name string sig string @@ -439,18 +502,18 @@ func TestParseSignatureParams(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - sig, exp, err := ParseSignatureParams(tt.sig, tt.expStr) + sig, exp, err := ParseParams(tt.sig, tt.expStr) if tt.wantErr { if err == nil { - t.Error("ParseSignatureParams() expected error, got nil") + t.Error("ParseParams() expected error, got nil") } return } if err != nil { - t.Errorf("ParseSignatureParams() unexpected error = %v", err) + t.Errorf("ParseParams() unexpected error = %v", err) return } diff --git a/scripts/manual-test.sh b/scripts/manual-test.sh index 514f378..b0118a3 100755 --- a/scripts/manual-test.sh +++ b/scripts/manual-test.sh @@ -97,8 +97,8 @@ else fail "No encrypted URL to test" fi -# Test 7: Fetch image via whitelisted host (direct proxy) -echo "--- Test 7: Fetch image via direct proxy (whitelisted host) ---" +# Test 7: Fetch image via allowlisted host (direct proxy) +echo "--- Test 7: Fetch image via direct proxy (allowlisted host) ---" # URL format: /v1/image///. PROXY_PATH="/v1/image/s3.sneak.cloud/sneak-public/2021/2021-04-18.untitled.a7r4.07723.jpg/400x300.jpeg" HTTP_CODE=$(curl -sf -o /dev/null -w "%{http_code}" "$BASE_URL$PROXY_PATH")