Escape : and % in the signed and cached path and query (closes #247)
check / check (push) Waiting to run
check / check (push) Waiting to run
The signed data and the cache key joined host, path and query with ":" and no escaping, so path /a:b with no query and path /a with query b: gave the same string: a URL signed for https://h/a:b verified for https://h/a?b:, and the two shared a cached image. Both now write each % in the path and the query as %25 and each : as %3A before joining them. The host never holds / and the path always starts with /, so the joined string reads back one way only. A path and query holding neither character are signed and keyed as before, so the existing known-answer vectors and the README example are unchanged; new vectors cover the escaped cases. Signature Specification in README.md states the escaping. Model: opus-5-5
This commit was merged in pull request #269.
This commit is contained in:
@@ -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`
|
||||||
|
|||||||
@@ -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
|
||||||
|
|||||||
@@ -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) {
|
||||||
|
|||||||
@@ -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)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -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 == "" {
|
||||||
|
|||||||
Reference in New Issue
Block a user