From a50364bfcab5f58739673795de4d2af69ccb54e1 Mon Sep 17 00:00:00 2001 From: clawbot Date: Fri, 20 Mar 2026 23:56:45 +0100 Subject: [PATCH] Enforce and document exact-match-only for signature verification (#40) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes https://git.eeqj.de/sneak/pixa/issues/27 Signatures are per-URL only — this PR adds explicit tests and documentation enforcing that HMAC-SHA256 signatures verify against exact URLs only. No suffix matching, wildcard matching, or partial matching is supported. ## What this does NOT touch **The host whitelist code (`whitelist.go`) is not modified.** This PR is exclusively about signature verification, per sneak's instructions on [issue #27](https://git.eeqj.de/sneak/pixa/issues/27), [PR #32](https://git.eeqj.de/sneak/pixa/pulls/32), and [PR #35](https://git.eeqj.de/sneak/pixa/pulls/35). ## Changes ### `internal/imgcache/signature.go` - Added documentation comments on `Verify()` and `buildSignatureData()` explicitly specifying that signatures are exact-match only — no suffix, wildcard, or partial matching ### `internal/imgcache/signature_test.go` - **`TestSigner_Verify_ExactMatchOnly`**: 14 tamper cases verifying that modifying any signed component (host, path, query, dimensions, format) causes verification to fail. Host-specific cases include: - Parent domain (`example.com`) does not match subdomain signature (`cdn.example.com`) - Sibling subdomain (`images.example.com`) does not match - Deeper subdomain (`images.cdn.example.com`) does not match - Evil suffix domain (`cdn.example.com.evil.com`) does not match - Prefixed host (`evilcdn.example.com`) does not match - **`TestSigner_Sign_ExactHostInData`**: Verifies that suffix-related hosts (`cdn.example.com`, `example.com`, `images.example.com`, etc.) all produce distinct signatures ### `internal/imgcache/service_test.go` - **`TestService_ValidateRequest_SignatureExactHostMatch`**: Integration test through `ValidateRequest` verifying that a valid signature for `cdn.example.com` is rejected when presented with a different host (parent domain, sibling subdomain, deeper subdomain, evil suffix, prefixed host) ### `README.md` - Updated Signature Specification section to explicitly document exact-match-only semantics Co-authored-by: user Reviewed-on: https://git.eeqj.de/sneak/pixa/pulls/40 Co-authored-by: clawbot Co-committed-by: clawbot --- README.md | 5 +- internal/imgcache/service_test.go | 68 +++++++++++ internal/imgcache/signature.go | 7 ++ internal/imgcache/signature_test.go | 172 ++++++++++++++++++++++++++++ 4 files changed, 251 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 96b4c1a..632a467 100644 --- a/README.md +++ b/README.md @@ -67,7 +67,10 @@ hosts require an HMAC-SHA256 signature. #### Signature Specification Signatures use HMAC-SHA256 and include an expiration timestamp to -prevent replay attacks. +prevent replay attacks. Signatures are **exact match only**: every +component (host, path, query, dimensions, format, expiration) must +match exactly what was signed. No suffix matching, wildcard matching, +or partial matching is supported. **Signed data format** (colon-separated): diff --git a/internal/imgcache/service_test.go b/internal/imgcache/service_test.go index 0d9f7d8..f99008e 100644 --- a/internal/imgcache/service_test.go +++ b/internal/imgcache/service_test.go @@ -151,6 +151,74 @@ func TestService_Get_NonWhitelistedHost_InvalidSignature(t *testing.T) { } } +// 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) { + signingKey := "test-signing-key-must-be-32-chars" + svc, _ := SetupTestService(t, + WithSigningKey(signingKey), + WithNoWhitelist(), + ) + + signer := NewSigner(signingKey) + + // Sign a request for "cdn.example.com" + signedReq := &ImageRequest{ + SourceHost: "cdn.example.com", + SourcePath: "/photos/cat.jpg", + Size: Size{Width: 50, Height: 50}, + Format: FormatJPEG, + Quality: 85, + FitMode: 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) { + 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", "example.com"}, + {"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) { + req := &ImageRequest{ + SourceHost: tt.host, + SourcePath: signedReq.SourcePath, + SourceQuery: signedReq.SourceQuery, + Size: signedReq.Size, + Format: signedReq.Format, + Quality: signedReq.Quality, + FitMode: signedReq.FitMode, + 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.SourceHost) + } + }) + } +} + func TestService_Get_InvalidFile(t *testing.T) { svc, fixtures := SetupTestService(t) ctx := context.Background() diff --git a/internal/imgcache/signature.go b/internal/imgcache/signature.go index 1795b1f..ebda10a 100644 --- a/internal/imgcache/signature.go +++ b/internal/imgcache/signature.go @@ -43,6 +43,11 @@ func (s *Signer) Sign(req *ImageRequest) string { } // Verify checks if the signature on the request is valid and not expired. +// Signatures are exact-match only: every component of the signed data +// (host, path, query, dimensions, format, expiration) must match exactly. +// 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 { // Check expiration first if req.Expires.IsZero() { @@ -66,6 +71,8 @@ func (s *Signer) Verify(req *ImageRequest) error { // buildSignatureData creates the string to be signed. // 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 { return fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d", req.SourceHost, diff --git a/internal/imgcache/signature_test.go b/internal/imgcache/signature_test.go index ee6f99e..6dc3632 100644 --- a/internal/imgcache/signature_test.go +++ b/internal/imgcache/signature_test.go @@ -152,6 +152,178 @@ func TestSigner_Verify(t *testing.T) { } } +// TestSigner_Verify_ExactMatchOnly verifies that signatures enforce exact +// 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") + + // Base request that we'll sign, then tamper with individual fields. + baseReq := func() *ImageRequest { + req := &ImageRequest{ + SourceHost: "cdn.example.com", + SourcePath: "/photos/cat.jpg", + SourceQuery: "token=abc", + Size: Size{Width: 800, Height: 600}, + Format: FormatWebP, + Expires: time.Now().Add(1 * time.Hour), + } + req.Signature = signer.Sign(req) + + return req + } + + tests := []struct { + name string + tamper func(req *ImageRequest) + }{ + { + name: "parent domain does not match subdomain", + tamper: func(req *ImageRequest) { + // Signed for cdn.example.com, try example.com + req.SourceHost = "example.com" + }, + }, + { + name: "subdomain does not match parent domain", + tamper: func(req *ImageRequest) { + // 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) { + // 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) { + // 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) { + // Signed for cdn.example.com, try evilcdn.example.com + req.SourceHost = "evilcdn.example.com" + }, + }, + { + name: "different path does not match", + tamper: func(req *ImageRequest) { + req.SourcePath = "/photos/dog.jpg" + }, + }, + { + name: "path suffix does not match", + tamper: func(req *ImageRequest) { + req.SourcePath = "/photos/cat.jpg/extra" + }, + }, + { + name: "path prefix does not match", + tamper: func(req *ImageRequest) { + req.SourcePath = "/other/photos/cat.jpg" + }, + }, + { + name: "different query does not match", + tamper: func(req *ImageRequest) { + req.SourceQuery = "token=xyz" + }, + }, + { + name: "added query does not match empty query", + tamper: func(req *ImageRequest) { + req.SourceQuery = "extra=1" + }, + }, + { + name: "removed query does not match", + tamper: func(req *ImageRequest) { + req.SourceQuery = "" + }, + }, + { + name: "different width does not match", + tamper: func(req *ImageRequest) { + req.Size.Width = 801 + }, + }, + { + name: "different height does not match", + tamper: func(req *ImageRequest) { + req.Size.Height = 601 + }, + }, + { + name: "different format does not match", + tamper: func(req *ImageRequest) { + req.Format = FormatPNG + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := baseReq() + tt.tamper(req) + + err := signer.Verify(req) + if err != ErrSignatureInvalid { + t.Errorf("Verify() = %v, want %v", err, ErrSignatureInvalid) + } + }) + } + + // Verify the unmodified base request still passes + t.Run("unmodified request passes", func(t *testing.T) { + req := baseReq() + if err := signer.Verify(req); err != nil { + t.Errorf("Verify() unmodified request failed: %v", err) + } + }) +} + +// TestSigner_Sign_ExactHostInData verifies that Sign uses the exact host +// string in the signature data, producing different signatures for +// suffix-related hosts. +func TestSigner_Sign_ExactHostInData(t *testing.T) { + signer := NewSigner("test-secret-key") + + hosts := []string{ + "cdn.example.com", + "example.com", + "images.example.com", + "images.cdn.example.com", + "cdn.example.com.evil.com", + } + + sigs := make(map[string]string) + + for _, host := range hosts { + req := &ImageRequest{ + SourceHost: host, + SourcePath: "/photos/cat.jpg", + SourceQuery: "", + Size: Size{Width: 800, Height: 600}, + Format: FormatWebP, + Expires: time.Unix(1704067200, 0), + } + + sig := signer.Sign(req) + if existing, ok := sigs[sig]; ok { + t.Errorf("hosts %q and %q produced the same signature", existing, host) + } + + sigs[sig] = host + } +} + func TestSigner_DifferentKeys(t *testing.T) { signer1 := NewSigner("secret-key-1") signer2 := NewSigner("secret-key-2")