Enforce and document exact-match-only for signature verification (#40)
All checks were successful
check / check (push) Successful in 58s
All checks were successful
check / check (push) Successful in 58s
Closes #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](#27), [PR #32](#32), and [PR #35](#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 <user@Mac.lan guest wan> Reviewed-on: #40 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org>
This commit was merged in pull request #40.
This commit is contained in:
@@ -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):
|
||||
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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")
|
||||
|
||||
Reference in New Issue
Block a user