Lower-case the image URL's host and require a signature for a port (closes #246)
check / check (push) Canceled after 0s
check / check (push) Canceled after 0s
With `cdn.example.com` allowlisted, other letter cases and a host with a port also passed the allowlist; each spelling was fetched and cached on its own with its own per-host connection limit, and a port let an unsigned URL reach any port of the host. The image URL parser now lower-cases the host and refuses one holding `@` with 400. A host followed by `:`, with or without a port (read as Go's URL parser reads it, unbracketed IPv6 included), never counts as allowlisted, so it needs a signature. The host is signed lower-cased, as the URL writes it, so a signature made over an upper-case spelling is refused with 401. Model: opus-5-5
This commit was merged in pull request #266.
This commit is contained in:
@@ -418,7 +418,9 @@ 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 host as the URL writes it up to the first `/`, in lower case,
|
||||||
|
keeping an IPv6 address's brackets and any `:` and port after it (e.g.
|
||||||
|
`cdn.example.com`, `cdn.example.com:8443`, `[2001:db8::1]:8443`)
|
||||||
- `path` — source path (e.g. `/photos/cat.jpg`), with each `%` written as `%25`
|
- `path` — source path (e.g. `/photos/cat.jpg`), with each `%` written as `%25`
|
||||||
and each `:` as `%3A`, so `/photos/a:b.jpg` is signed as `/photos/a%3Ab.jpg`
|
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
|
- `query` — source query string, empty string if none, with `%` and `:` written
|
||||||
@@ -473,6 +475,12 @@ entry that is neither a host name (letters, digits, hyphens, underscores and
|
|||||||
dots, with at most one leading dot) nor an IP address, such as one with a port
|
dots, with at most one leading dot) nor an IP address, such as one with a port
|
||||||
or a `*.` wildcard, aborts startup.
|
or a `*.` wildcard, aborts startup.
|
||||||
|
|
||||||
|
pixa lower-cases the host of a `/v1/image/` URL before anything else uses it, so
|
||||||
|
`CDN.Example.com` is matched, fetched, cached and counted against
|
||||||
|
`upstream_connections_per_host` as `cdn.example.com`. Such a URL whose host has
|
||||||
|
a port, as in `cdn.example.com:8443`, or a `:` with no port, needs a signature
|
||||||
|
even when the host is allowlisted. A host holding `@` is refused with 400.
|
||||||
|
|
||||||
### Configuration
|
### Configuration
|
||||||
|
|
||||||
Every setting can be given as an environment variable, in a YAML config file
|
Every setting can be given as an environment variable, in a YAML config file
|
||||||
|
|||||||
@@ -30,6 +30,17 @@ P2: security: per-origin rate limiting
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-10-15 the host of a `/v1/image/` URL is lower-cased, and one with a port
|
||||||
|
needs a signature (closes #246): with `cdn.example.com` allowlisted,
|
||||||
|
`CDN.Example.com` and `cdn.example.com:8443` passed the allowlist too, and
|
||||||
|
each spelling was fetched and cached on its own and had connections of its own
|
||||||
|
under `upstream_connections_per_host`. The image URL parser now lower-cases
|
||||||
|
the host and refuses one holding `@`, which a URL reads as a user name and
|
||||||
|
password, with 400. A host with a port, or with a `:` and no port, needs a
|
||||||
|
signature even when it is allowlisted. That is checked in the image service,
|
||||||
|
not in `allowlist.IsAllowed`, which also matches `referer_blocklist`, where a
|
||||||
|
`Referer` is blocked whatever its port. A URL is signed over its host as it
|
||||||
|
writes it, in lower case, with any IPv6 brackets, `:` and port kept.
|
||||||
- 2026-10-15 a `:` in the source path or query no longer moves where the path
|
- 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
|
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
|
joined host, path and query with `:` and no escaping, so a URL signed for
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ import (
|
|||||||
"log/slog"
|
"log/slog"
|
||||||
"net/url"
|
"net/url"
|
||||||
"runtime/debug"
|
"runtime/debug"
|
||||||
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/dustin/go-humanize"
|
"github.com/dustin/go-humanize"
|
||||||
@@ -218,7 +219,16 @@ func (s *Service) ValidateRequest(req *ImageRequest) error {
|
|||||||
return fmt.Errorf("invalid source URL: %w", err)
|
return fmt.Errorf("invalid source URL: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
if s.allowlist.IsAllowed(parsedURL) {
|
// An allowlist entry names no port, so a host followed by ":", with or
|
||||||
|
// without a port, needs a signature. The port is read from the parsed
|
||||||
|
// URL, as the fetcher reads it: in an IPv6 address written without
|
||||||
|
// brackets, such as 2001:db8::1:8443, it is what follows the last ":".
|
||||||
|
// This is checked here and not in allowlist.IsAllowed, which also matches
|
||||||
|
// referer_blocklist, where a Referer must be blocked whatever its port.
|
||||||
|
hostHasPort := parsedURL.Port() != "" ||
|
||||||
|
strings.HasSuffix(parsedURL.Host, ":")
|
||||||
|
|
||||||
|
if !hostHasPort && s.allowlist.IsAllowed(parsedURL) {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -250,6 +250,97 @@ func TestService_ValidateRequest_SignatureExactHostMatch(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestService_ValidateRequest_HostWithPortNeedsSignature verifies that a host
|
||||||
|
// followed by a port, or by a colon with no port, matches no allowlist entry,
|
||||||
|
// so that a request for it without a signature is refused, while the same
|
||||||
|
// host with no port is allowlisted.
|
||||||
|
func TestService_ValidateRequest_HostWithPortNeedsSignature(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
svc, _ := SetupTestService(t,
|
||||||
|
WithSigningKey(testSigningKey),
|
||||||
|
WithAllowlist(testHostCDN, "203.0.113.5", "2001:db8::1"),
|
||||||
|
)
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
host string
|
||||||
|
allowlisted bool
|
||||||
|
}{
|
||||||
|
{testHostCDN, true},
|
||||||
|
{"cdn.example.com:8443", false},
|
||||||
|
{"cdn.example.com:443", false},
|
||||||
|
{"cdn.example.com:", false},
|
||||||
|
{"203.0.113.5", true},
|
||||||
|
{"203.0.113.5:8080", false},
|
||||||
|
{"[2001:db8::1]", true},
|
||||||
|
{"[2001:db8::1]:8443", false},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.host, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
req := &ImageRequest{
|
||||||
|
SourceHost: tt.host,
|
||||||
|
SourcePath: testPathCat,
|
||||||
|
Size: Size{Width: 50, Height: 50},
|
||||||
|
Format: FormatJPEG,
|
||||||
|
Quality: 85,
|
||||||
|
FitMode: FitCover,
|
||||||
|
}
|
||||||
|
|
||||||
|
err := svc.ValidateRequest(req)
|
||||||
|
t.Logf("ValidateRequest() for host %q with no signature: %v",
|
||||||
|
tt.host, err)
|
||||||
|
|
||||||
|
if allowlisted := err == nil; allowlisted != tt.allowlisted {
|
||||||
|
t.Errorf("host %q served without a signature = %v, want %v",
|
||||||
|
tt.host, allowlisted, tt.allowlisted)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestService_ValidateRequest_UnbracketedIPv6PortNeedsSignature verifies that
|
||||||
|
// an allowlisted IPv6 address written without brackets and followed by a
|
||||||
|
// port, or by a colon with no port, needs a signature. Go's URL parser, which
|
||||||
|
// the fetcher uses, reads 2001:db8::1:8443 as the host 2001:db8::1 with the
|
||||||
|
// port 8443.
|
||||||
|
func TestService_ValidateRequest_UnbracketedIPv6PortNeedsSignature(
|
||||||
|
t *testing.T,
|
||||||
|
) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
svc, _ := SetupTestService(t,
|
||||||
|
WithSigningKey(testSigningKey),
|
||||||
|
WithAllowlist("2001:db8::1"),
|
||||||
|
)
|
||||||
|
|
||||||
|
for _, host := range []string{"2001:db8::1:8443", "2001:db8::1:"} {
|
||||||
|
t.Run(host, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
req := &ImageRequest{
|
||||||
|
SourceHost: host,
|
||||||
|
SourcePath: testPathCat,
|
||||||
|
Size: Size{Width: 50, Height: 50},
|
||||||
|
Format: FormatJPEG,
|
||||||
|
Quality: 85,
|
||||||
|
FitMode: FitCover,
|
||||||
|
}
|
||||||
|
|
||||||
|
err := svc.ValidateRequest(req)
|
||||||
|
t.Logf("ValidateRequest() for host %q with no signature: %v",
|
||||||
|
host, err)
|
||||||
|
|
||||||
|
if err == nil {
|
||||||
|
t.Errorf("host %q served without a signature, want refused",
|
||||||
|
host)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestService_Get_InvalidFile(t *testing.T) {
|
func TestService_Get_InvalidFile(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ import (
|
|||||||
var (
|
var (
|
||||||
ErrInvalidPath = errors.New("invalid image path")
|
ErrInvalidPath = errors.New("invalid image path")
|
||||||
ErrMissingHost = errors.New("missing source host")
|
ErrMissingHost = errors.New("missing source host")
|
||||||
|
ErrInvalidHost = errors.New("invalid source host")
|
||||||
ErrMissingSize = errors.New("missing size specification")
|
ErrMissingSize = errors.New("missing size specification")
|
||||||
ErrInvalidSize = errors.New("invalid size format")
|
ErrInvalidSize = errors.New("invalid size format")
|
||||||
ErrInvalidFormat = errors.New("invalid or unsupported format")
|
ErrInvalidFormat = errors.New("invalid or unsupported format")
|
||||||
@@ -143,6 +144,17 @@ func parseImageComponents(remainder string) (*ParsedURL, error) {
|
|||||||
return nil, ErrMissingHost
|
return nil, ErrMissingHost
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A URL reads what comes before "@" as a user name and password, and the
|
||||||
|
// rest as the host
|
||||||
|
if strings.Contains(host, "@") {
|
||||||
|
return nil, fmt.Errorf("%w: %q holds @", ErrInvalidHost, host)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Host names do not depend on letter case, so the allowlist, the upstream
|
||||||
|
// connection limits, the cache and the signature see one spelling of each
|
||||||
|
// host
|
||||||
|
host = strings.ToLower(host)
|
||||||
|
|
||||||
// Extract query string if present in path
|
// Extract query string if present in path
|
||||||
if qIndex := strings.Index(path, "?"); qIndex != -1 {
|
if qIndex := strings.Index(path, "?"); qIndex != -1 {
|
||||||
query = path[qIndex+1:]
|
query = path[qIndex+1:]
|
||||||
|
|||||||
@@ -234,6 +234,49 @@ func TestParseImagePath(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestParseImagePath_LowerCasesHost verifies that the host is lower-cased, so
|
||||||
|
// that every spelling of its letter case is the same host to the allowlist,
|
||||||
|
// the upstream connection limits and the cache, and that the path keeps its
|
||||||
|
// case.
|
||||||
|
func TestParseImagePath_LowerCasesHost(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
got, err := ParseImagePath("CDN.Example.COM/Photos/Cat.jpg/800x600.webp")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ParseImagePath() error = %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
t.Logf("parsed host %q, path %q", got.Host, got.Path)
|
||||||
|
|
||||||
|
if got.Host != testHostCDN {
|
||||||
|
t.Errorf("Host = %q, want %q", got.Host, testHostCDN)
|
||||||
|
}
|
||||||
|
|
||||||
|
if got.Path != "/Photos/Cat.jpg" {
|
||||||
|
t.Errorf("Path = %q, want %q", got.Path, "/Photos/Cat.jpg")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestParseImagePath_RefusesUserinfo verifies that a host holding "@" is
|
||||||
|
// refused: a URL reads what comes before the "@" as a user name and password
|
||||||
|
// and the rest as the host.
|
||||||
|
func TestParseImagePath_RefusesUserinfo(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
for _, input := range []string{
|
||||||
|
"user@cdn.example.com/photos/cat.jpg/800x600.webp",
|
||||||
|
"user:password@cdn.example.com/photos/cat.jpg/800x600.webp",
|
||||||
|
} {
|
||||||
|
got, err := ParseImagePath(input)
|
||||||
|
t.Logf("ParseImagePath(%q) = %+v, error = %v", input, got, err)
|
||||||
|
|
||||||
|
if !errors.Is(err, ErrInvalidHost) {
|
||||||
|
t.Errorf("ParseImagePath(%q) error = %v, want %v",
|
||||||
|
input, err, ErrInvalidHost)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestParsedURL_ToImageRequest(t *testing.T) {
|
func TestParsedURL_ToImageRequest(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user