diff --git a/README.md b/README.md index 5fb6a49..687e737 100644 --- a/README.md +++ b/README.md @@ -418,7 +418,8 @@ HMAC-SHA256(secret, "host:path:query:width:height:format:expiration:quality:fit" Where: -- `host` — source origin hostname (e.g. `cdn.example.com`) +- `host` — source origin hostname in lower case, whatever its case in the URL, + followed by `:` and the port when the URL has one (e.g. `cdn.example.com`) - `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` - `query` — source query string, empty string if none, with `%` and `:` written @@ -473,6 +474,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 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 Every setting can be given as an environment variable, in a YAML config file diff --git a/TODO.md b/TODO.md index 7793ef4..3285f97 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,17 @@ P2: security: per-origin rate limiting # 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 the lower-cased + host. - 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 diff --git a/internal/imgcache/service.go b/internal/imgcache/service.go index 52684a9..c7706c5 100644 --- a/internal/imgcache/service.go +++ b/internal/imgcache/service.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "log/slog" + "net" "net/url" "runtime/debug" "time" @@ -218,7 +219,14 @@ func (s *Service) ValidateRequest(req *ImageRequest) error { 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. This is checked here and not in + // allowlist.IsAllowed, which also matches referer_blocklist, where a + // Referer must be blocked whatever its port. + _, _, splitErr := net.SplitHostPort(parsedURL.Host) + hostHasPort := splitErr == nil + + if !hostHasPort && s.allowlist.IsAllowed(parsedURL) { return nil } diff --git a/internal/imgcache/urlparser.go b/internal/imgcache/urlparser.go index d836930..ea5b6fd 100644 --- a/internal/imgcache/urlparser.go +++ b/internal/imgcache/urlparser.go @@ -144,6 +144,16 @@ func parseImageComponents(remainder string) (*ParsedURL, error) { 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 and the cache see one spelling of each host + host = strings.ToLower(host) + // Extract query string if present in path if qIndex := strings.Index(path, "?"); qIndex != -1 { query = path[qIndex+1:]