Escape : and % in the signed and cached path and query (closes #247) #269

Merged
clawbot merged 2 commits from issue-247-unambiguous-signed-data into next 2026-10-15 09:37:02 +02:00
6 changed files with 212 additions and 10 deletions
+5 -2
View File
@@ -419,8 +419,11 @@ HMAC-SHA256(secret, "host:path:query:width:height:format:expiration:quality:fit"
Where: Where:
- `host` — source origin hostname (e.g. `cdn.example.com`) - `host` — source origin hostname (e.g. `cdn.example.com`)
- `path` — source path (e.g. `/photos/cat.jpg`) - `path` — source path (e.g. `/photos/cat.jpg`), with each `%` written as `%25`
- `query` — source query string, empty string if none and each `:` as `%3A`, so `/photos/a:b.jpg` is signed as `/photos/a%3Ab.jpg`
- `query` — source query string, empty string if none, with `%` and `:` written
as in `path`; neither then holds a `:`, so path `/a:b` with no query and path
`/a` with query `b:` are signed differently
- `width` — requested width in pixels, `0` for original - `width` — requested width in pixels, `0` for original
- `height` — requested height in pixels, `0` for original - `height` — requested height in pixels, `0` for original
- `format` — output format, one of those listed under Routes, with `original` - `format` — output format, one of those listed under Routes, with `original`
+8
View File
@@ -30,6 +30,14 @@ P2: security: per-origin rate limiting
# Completed Steps # Completed Steps
- 2026-10-15 a `:` in the source path or query no longer moves where the path
ends and the query begins (closes #247): the signed data and the cache key
joined host, path and query with `:` and no escaping, so a URL signed for
`https://h/a:b` verified for `https://h/a?b:`; the two also shared a cache
key, and whichever was fetched first was served for both. In both, the path
and the query now have each `%` written as `%25` and each `:` as `%3A`; a path
and query holding neither are signed and keyed as before. Signature
Specification in `README.md` says so.
- 2026-10-15 the example config allowlists no host (closes #249): it allowlisted - 2026-10-15 the example config allowlists no host (closes #249): it allowlisted
`github.com` and `user-images.githubusercontent.com`, where anyone can put an `github.com` and `user-images.githubusercontent.com`, where anyone can put an
image, so a deployment set up as Getting Started says served those images to image, so a deployment set up as Getting Started says served those images to
+13 -2
View File
@@ -10,6 +10,7 @@ import (
"log/slog" "log/slog"
"os" "os"
"path/filepath" "path/filepath"
"strings"
"time" "time"
) )
@@ -371,11 +372,13 @@ func HashPath(path string) PathHash {
// CacheKey generates a unique key for a request variant. // CacheKey generates a unique key for a request variant.
// Format: sha256(host:path:query:width:height:format:quality:fit_mode) // Format: sha256(host:path:query:width:height:format:quality:fit_mode)
// The path and query go through escapePercentAndColon, so that a ":" in
// either cannot be taken for the one between them.
func CacheKey(req *ImageRequest) VariantKey { func CacheKey(req *ImageRequest) VariantKey {
data := fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%s", data := fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%s",
req.SourceHost, req.SourceHost,
req.SourcePath, escapePercentAndColon(req.SourcePath),
req.SourceQuery, escapePercentAndColon(req.SourceQuery),
req.Size.Width, req.Size.Width,
req.Size.Height, req.Size.Height,
req.Format, req.Format,
@@ -387,6 +390,14 @@ func CacheKey(req *ImageRequest) VariantKey {
return VariantKey(hex.EncodeToString(h[:])) return VariantKey(hex.EncodeToString(h[:]))
} }
// escapePercentAndColon returns s with each "%" written as "%25" and each
// ":" as "%3A", so that the result holds no ":".
func escapePercentAndColon(s string) string {
s = strings.ReplaceAll(s, "%", "%25")
return strings.ReplaceAll(s, ":", "%3A")
}
// VariantStorage handles key-based file storage for processed image variants. // VariantStorage handles key-based file storage for processed image variants.
// Files are stored at: <basedir>/<ab>/<cd>/<cache_key> // Files are stored at: <basedir>/<ab>/<cd>/<cache_key>
// Metadata is stored at: <basedir>/<ab>/<cd>/<cache_key>.meta // Metadata is stored at: <basedir>/<ab>/<cd>/<cache_key>.meta
@@ -407,6 +407,62 @@ func TestCacheKey(t *testing.T) {
} }
} }
// TestCacheKey_ColonOrPercentInPathOrQuery checks that two different source
// URLs whose path or query holds ":" or "%" get different keys.
func TestCacheKey_ColonOrPercentInPathOrQuery(t *testing.T) {
t.Parallel()
cases := []struct {
name string
path1, query1 string
path2, query2 string
}{
{
name: "colon moved from path to query",
path1: "/a:b", query1: "",
path2: "/a", query2: "b:",
},
{
name: "colon in path and its percent-encoding",
path1: "/a:b", query1: "",
path2: "/a%3Ab", query2: "",
},
}
for _, tt := range cases {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
key1 := CacheKey(&ImageRequest{
SourceHost: testHostCDN,
SourcePath: tt.path1,
SourceQuery: tt.query1,
Size: Size{Width: 800, Height: 600},
Format: FormatWebP,
Quality: 85,
FitMode: FitCover,
})
key2 := CacheKey(&ImageRequest{
SourceHost: testHostCDN,
SourcePath: tt.path2,
SourceQuery: tt.query2,
Size: Size{Width: 800, Height: 600},
Format: FormatWebP,
Quality: 85,
FitMode: FitCover,
})
t.Logf("CacheKey() of path %q query %q = %s", tt.path1, tt.query1, key1)
t.Logf("CacheKey() of path %q query %q = %s", tt.path2, tt.query2, key2)
if key1 == key2 {
t.Errorf("CacheKey() gave path %q query %q and path %q query %q "+
"the same key", tt.path1, tt.query1, tt.path2, tt.query2)
}
})
}
}
// TestVariantStorage_StoreLogsFailedMetaWrite verifies that a .meta write // TestVariantStorage_StoreLogsFailedMetaWrite verifies that a .meta write
// that fails is logged at warn and the store still succeeds. // that fails is logged at warn and the store still succeeds.
func TestVariantStorage_StoreLogsFailedMetaWrite(t *testing.T) { func TestVariantStorage_StoreLogsFailedMetaWrite(t *testing.T) {
+113
View File
@@ -0,0 +1,113 @@
package signature_test
import (
"errors"
"testing"
"time"
"sneak.berlin/go/pixa/internal/signature"
)
// TestSigner_Verify_ColonCannotMovePathQueryBoundary checks that a URL
// signed for path "/a:b" with no query does not verify for path "/a" with
// query "b:", a different resource on the same host.
func TestSigner_Verify_ColonCannotMovePathQueryBoundary(t *testing.T) {
t.Parallel()
signer := signature.New(goldenSigningKey)
signed := &signature.Request{
SourceHost: testHost,
SourcePath: "/a:b",
Width: 800,
Height: 600,
Format: testFormatWebP,
Quality: 85,
FitMode: testFitCover,
Expires: time.Now().Add(1 * time.Hour),
}
signed.Signature = signer.Sign(signed)
replayed := *signed
replayed.SourcePath = "/a"
replayed.SourceQuery = "b:"
t.Logf("signature for path %q query %q: %s",
signed.SourcePath, signed.SourceQuery, signed.Signature)
t.Logf("signature for path %q query %q: %s",
replayed.SourcePath, replayed.SourceQuery, signer.Sign(&replayed))
err := signer.Verify(&replayed)
if !errors.Is(err, signature.ErrInvalid) {
t.Errorf("Verify() of path %q query %q with the signature for "+
"path %q = %v, want %v",
replayed.SourcePath, replayed.SourceQuery, signed.SourcePath,
err, signature.ErrInvalid)
}
}
// TestSigner_EscapedGoldenVectors pins the signature of requests whose path
// or query holds ":" or "%", which are signed with "%" written as "%25" and
// ":" as "%3A". The expected values were computed once, outside pixa, from
// the signed data in each comment.
func TestSigner_EscapedGoldenVectors(t *testing.T) {
t.Parallel()
signer := signature.New(goldenSigningKey)
vectors := []struct {
name string
path string
query string
wantSignature string
}{
{
name: "colon in path",
path: "/photos/a:b.jpg",
// Signed data:
// "cdn.example.com:/photos/a%3Ab.jpg::800:600:webp:1704067200:85:cover"
wantSignature: "WnEpZ7bQp1Zvw8WDatlj2vu-4wYKWE3BFBizV8m__6k=",
},
{
name: "colon in query",
path: testPath,
query: "t=1:2",
// Signed data:
// "cdn.example.com:/photos/cat.jpg:t=1%3A2:800:600:webp:1704067200:85:cover"
wantSignature: "85kRoN8hhr4cCUkEUjNvUsvVYKjYYfWdeffx5TjJjqA=",
},
{
name: "percent in path and query",
path: "/photos/a%20b.jpg",
query: "q=50%",
// Signed data:
// "cdn.example.com:/photos/a%2520b.jpg:q=50%25:800:600:webp:1704067200:85:cover"
wantSignature: "MjDiiRdgXMrsjqPMsENtD4aG1hp1e-a6zy67vKTX4CQ=",
},
}
for _, tt := range vectors {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
req := &signature.Request{
SourceHost: testHost,
SourcePath: tt.path,
SourceQuery: tt.query,
Width: 800,
Height: 600,
Format: testFormatWebP,
Quality: 85,
FitMode: testFitCover,
Expires: time.Unix(goldenExpiresUnix, 0),
}
gotSignature := signer.Sign(req)
t.Logf("Sign() of path %q query %q = %s", tt.path, tt.query, gotSignature)
if gotSignature != tt.wantSignature {
t.Errorf("Sign() = %q, want %q", gotSignature, tt.wantSignature)
}
})
}
}
+17 -6
View File
@@ -10,6 +10,7 @@ import (
"fmt" "fmt"
"net/url" "net/url"
"strconv" "strconv"
"strings"
"time" "time"
) )
@@ -154,15 +155,17 @@ func (s *Signer) GenerateSignedURL(
// buildSignatureData creates the string to be signed. // buildSignatureData creates the string to be signed.
// Format: "host:path:query:width:height:format:expiration:quality:fit" // Format: "host:path:query:width:height:format:expiration:quality:fit"
// All components are used verbatim (exact match). No normalization, // The path and query go through escapePercentAndColon, so that a ":" in
// suffix matching, or wildcard expansion is performed. Quality and fit // either cannot be taken for the one between them; every other component
// are the effective transform values, so replaying a signed URL with a // is used verbatim. No normalization, suffix matching, or wildcard
// different quality or fit mode fails verification. // expansion is performed. Quality and fit are the effective transform
// values, so replaying a signed URL with a different quality or fit mode
// fails verification.
func (s *Signer) buildSignatureData(req *Request) string { func (s *Signer) buildSignatureData(req *Request) 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.SourceHost, req.SourceHost,
req.SourcePath, escapePercentAndColon(req.SourcePath),
req.SourceQuery, escapePercentAndColon(req.SourceQuery),
req.Width, req.Width,
req.Height, req.Height,
req.Format, req.Format,
@@ -172,6 +175,14 @@ func (s *Signer) buildSignatureData(req *Request) string {
) )
} }
// escapePercentAndColon returns s with each "%" written as "%25" and each
// ":" as "%3A", so that the result holds no ":".
func escapePercentAndColon(s string) string {
s = strings.ReplaceAll(s, "%", "%25")
return strings.ReplaceAll(s, ":", "%3A")
}
// ParseParams extracts signature and expiration from query parameters. // ParseParams extracts signature and expiration from query parameters.
func ParseParams(sig, expStr string) (string, time.Time, error) { func ParseParams(sig, expStr string) (string, time.Time, error) {
if expStr == "" { if expStr == "" {