5 Commits
Author SHA1 Message Date
clawbot 10eab440e7 Resolve real client IP behind trusted proxies (closes #94)
check / check (push) Successful in 2m31s
RFC1918 ranges are the default trusted proxy set on an omitted key; an explicit list replaces the default; an explicit empty list trusts no one; unparseable values abort startup; forwarded headers honored only from trusted peers. Independent review passed: #127 (comment)

model: claude-opus-4-8 (implementation and review); merged by claude-fable-5
2026-09-22 10:25:41 +02:00
clawbot 3cfcda0730 feat: blocked_networks config and extended SSRF ranges (closes #67)
check / check (push) Failing after 1s
Adds the blocked_networks config key: a list of CIDRs, parsed with net/netip, that is added to the built-in list of address ranges the fetcher refuses to contact and can never remove an entry from it. An invalid CIDR aborts startup naming the key and the value.

The built-in list gains CGNAT 100.64.0.0/10, IETF protocol assignments 192.0.0.0/24, benchmark 198.18.0.0/15 and NAT64 64:ff9b::/96. Resolved addresses are unmapped before matching, so IPv4-mapped IPv6 forms are caught too. Enforcement stays in the dial-time re-resolution, which is what closes the DNS rebinding window.

What a reader would trip over: 192.0.0.0/24 is now blocked but TEST-NET-1 (192.0.2.0/24), which the Fetch tests use as a public upstream, is a different range and stays dialable. The package-level dialer enforces the built-in ranges only; operator entries are applied by the fetcher.

Disclosure: one nolint:gochecknoglobals on the immutable built-in prefix list.

Model: opus-4-8 (implementation, review); fable-5-1 (landing message)
2026-09-22 00:43:27 +02:00
clawbot 1798cba96c Take the image signing key from PIXA_SIGNING_KEY and refuse the example placeholder (closes #110)
check / check (push) Failing after 1s
The Docker image now ships config.docker.yml, which sets only signing_key (read from the PIXA_SIGNING_KEY environment variable), state_dir and port. The placeholder key and the five-host allowlist from config.example.yml are no longer in the image; anything else is configured by mounting a file over /etc/pixa/config.yml. A container started without PIXA_SIGNING_KEY exits naming it.

Startup now refuses the exact placeholder signing_key from config.example.yml. It is 45 characters long and used to pass the length check, so a deployment could sign URLs with a key that is public in this repository. README Getting Started is corrected to match.

What a reader would trip over: the unset-variable error comes from config interpolation, not from validate(); the signing key checks moved into validateSigningKey to stay under the complexity limit.

Disclosure: TODO.md is not updated by this change.

Model: opus-4-8 (implementation, review); fable-5-1 (landing message)
2026-09-21 21:59:24 +02:00
clawbot 37d49ade11 Harden http.Server: slowloris timeouts and form body limit (closes #92)
check / check (push) Failing after 1s
The http.Server now sets ReadHeaderTimeout (10s), which bounds the slow header dribble that ReadTimeout alone does not, and IdleTimeout (120s), which bounds keep-alive reuse. Server construction moved into a small helper so a test can assert the timeouts without binding a listener.

POST / and POST /generate bodies are capped at 1 MiB and an oversized body returns 413.

What a reader would trip over: the CSRF library reads its token from the form and swallows a parse error, so a cap applied only inside it would surface as 403. The body limit therefore parses the form under the cap before the CSRF check; the parsed form is reused afterwards. A test covers an oversized body that carries a valid token.

Judgement call: WriteTimeout stays at 60s; it also bounds how long a large image may take to send over a slow link.

Model: opus-4-8 (implementation, review); fable-5-1 (landing message)
2026-09-21 20:59:24 +02:00
clawbot b4e5300feb feat: add HSTS, CSP, and Permissions-Policy security headers (closes #91)
check / check (push) Failing after 0s
SecurityHeaders() now also sets Strict-Transport-Security (one year, includeSubDomains), a Content-Security-Policy (default-src self, frame-ancestors none) and a Permissions-Policy denying the browser features pixa does not use. X-Frame-Options stays as the legacy fallback.

What a reader would trip over: HSTS is sent on every response even though pixa listens on plain HTTP behind a TLS-terminating proxy; browsers ignore the header over plaintext, and this avoids trusting a forwarded-proto header. The clipboard feature is left unlisted so the copy button on the generator page keeps working.

Disclosure: script-src and style-src carry unsafe-inline because the generator template has inline onclick handlers and the bundled Tailwind script injects a style element at runtime; removing it needs template changes and is tracked separately.

Model: opus-4-8 (implementation, review); fable-5-1 (landing message)
2026-09-21 20:43:13 +02:00
26 changed files with 1554 additions and 152 deletions
+5 -7
View File
@@ -13,12 +13,9 @@ RUN go mod download
# Copy source code
COPY . .
# Run formatting check and linter. The linter is invoked directly, not
# via `make lint`: `make lint` now builds Dockerfile.lint, and there is
# no Docker inside a Docker build. This is the same linter, image, and
# config that Dockerfile.lint and script/lint run.
# Run formatting check and linter
RUN make fmt-check
RUN golangci-lint run --config .golangci.yml ./...
RUN make lint
# Build stage
# golang:1.25.4-alpine, 2026-02-25
@@ -70,8 +67,9 @@ RUN adduser -D -H -s /sbin/nologin pixad && \
mkdir -p /var/lib/pixa /etc/pixa && \
chown pixad:pixad /var/lib/pixa
# Copy default config (edit signing_key before use)
COPY config.example.yml /etc/pixa/config.yml
# Copy the image config; signing_key comes from PIXA_SIGNING_KEY.
# Mount a file over /etc/pixa/config.yml to override anything else.
COPY config.docker.yml /etc/pixa/config.yml
USER pixad
WORKDIR /var/lib/pixa
-41
View File
@@ -1,41 +0,0 @@
# Dockerfile.lint: the one and only path that runs golangci-lint.
#
# golangci-lint is never installed on the host; it runs only inside this
# build. A clean build of this file therefore IS a clean lint over the
# whole tree. It runs the same linter and config as Dockerfile's lint
# stage, pinned to the same image so the two cannot drift to different
# linter versions.
#
# golangci/golangci-lint:v2.12.2-alpine, 2026-08-07
FROM golangci/golangci-lint:v2.12.2-alpine@sha256:91b27804074a0bacea298707f016911e60cf0cdbc6c7bf5ccacb5f0606d18d60
# pixa is CGO/libvips: the type-aware linters compile every package, so
# this image needs the same C libraries the build does.
RUN apk add --no-cache build-base vips-dev libheif-dev pkgconfig
WORKDIR /src
# Modules first for layer caching; go.mod/go.sum settle this layer's
# result, so it may safely be reused between runs.
COPY go.mod go.sum ./
RUN go mod download
COPY . .
# Caching is deliberately waived for the lint step: an unchanged tree
# must still run the linter, not return a cached success in well under a
# second having linted nothing. CACHEBUST carries a value that differs
# on every run (script/lint supplies it and refuses to build without
# one). The lint RUN below references it, so BuildKit cannot serve that
# step from cache. Keep the ${CACHEBUST} reference on that step: dropping
# it lets the linter cache again and report a green that linted nothing.
ARG CACHEBUST
RUN test -n "${CACHEBUST}" || { \
echo "Dockerfile.lint requires the CACHEBUST build-arg; build it via script/lint." >&2; \
exit 1; }
# `golangci-lint config verify` is deliberately not run: it fetches its
# JSON schema over an unpinned live HTTPS call, which REPO_POLICIES.md
# forbids for external references.
RUN echo "pixa-lint: running golangci-lint (${CACHEBUST})" && \
golangci-lint run --config .golangci.yml ./...
+30 -3
View File
@@ -15,14 +15,25 @@ git clone https://git.eeqj.de/sneak/pixa.git
cd pixa
make build
# run with a config file
./bin/pixad --config config.example.yml
# run with a config file: copy the example and set a real signing key
# (the example placeholder is refused at startup), e.g. with
# openssl rand -base64 32
cp config.example.yml config.yml
$EDITOR config.yml # replace the signing_key placeholder
./bin/pixad --config config.yml
# or build and run via Docker
make docker
docker run -p 8080:8080 pixad:latest
docker run -p 8080:8080 -e PIXA_SIGNING_KEY="$(openssl rand -base64 32)" pixa:latest
```
A container is configured two ways. The signing key comes from the
`PIXA_SIGNING_KEY` environment variable, which the baked-in config
reads; if it is unset the container exits at startup naming the
variable. Everything else uses built-in defaults, so to change any
other setting mount your own file over `/etc/pixa/config.yml` (see
`config.example.yml` for the full set of keys).
## Rationale
Image-heavy web applications need a fast, caching reverse proxy that
@@ -111,6 +122,22 @@ Configured via YAML file (`--config`). Key settings:
- `access_control_allow_origin` — CORS origin
- `allowlist_hosts` — list of allowed upstream hosts
- `blocked_networks` — list of CIDR ranges to refuse for SSRF protection,
added to the always-enforced built-in ranges (loopback, private,
link-local, CGNAT, benchmark, NAT64, and the like); an invalid CIDR
aborts startup
- `trusted_proxies` — list of CIDR ranges of the reverse proxies in front
of pixa. `X-Forwarded-For` is believed only when the direct peer falls
inside one of these ranges; the logged and login-recorded client
address is then the rightmost forwarded entry that is not itself a
trusted proxy. Otherwise the direct peer address is used and the header
is ignored, so a client connecting directly cannot spoof its address.
An omitted key defaults to the RFC 1918 private ranges (`10.0.0.0/8`,
`172.16.0.0/12`, `192.168.0.0/16`), since pixa is deployed behind a
proxy on a private network; an explicitly empty list (`[]`) trusts no
one, and an explicit list replaces the default. An invalid CIDR aborts
startup. Set this to your proxy's address range if it is not already
covered by the defaults
- `upstream_fetch_timeout` — timeout for origin requests
- `upstream_max_response_size` — max origin response size
- `downstream_timeout` — client response timeout
+32 -12
View File
@@ -25,19 +25,41 @@ The disk cache is now size-bounded with LRU eviction
# Next Step
P1: implement blocked networks configuration to extend SSRF protection
P1: rate limit global concurrent upstream fetches to prevent resource
exhaustion
# Completed Steps
- 2026-09-21 run all linting in Docker via `Dockerfile.lint` +
`script/lint` (closes #104): `script/lint` builds a hash-pinned root
`Dockerfile.lint`, and no host or nix-shell `golangci-lint` path
remains; a per-run `CACHEBUST` build-arg forces the lint step to
execute every run, so an unchanged tree cannot return a cached green
that linted nothing; `Dockerfile`'s lint stage runs `golangci-lint`
directly, since `make lint` now builds a container and there is no
Docker inside a build; `golangci-lint config verify` stays out, as it
fetches its schema over an unpinned live HTTPS call
- 2026-09-21 trusted-proxy client IP resolution (closes #94): a
`trusted_proxies` config key taking a list of CIDRs, parsed by the same
`net/netip` list parser as `blocked_networks` (an invalid entry aborts
startup naming the key and value; an omitted key defaults to the RFC 1918
private ranges, an explicitly empty list trusts no one, and an explicit
list replaces the default); a new
`internal/clientip` package resolves the client address by honoring
`X-Forwarded-For` only when the direct peer is a trusted proxy, walking
the chain right-to-left to the rightmost non-proxy entry, so a client
connecting directly cannot spoof its address; the resolved address is
stored in the request context by a new middleware and used by the
request-logging middleware and the login-attempt logs in place of the
raw peer address; documented in `README.md` and `config.example.yml`.
- 2026-09-21 blocked networks configuration extending SSRF protection: a
`blocked_networks` config key taking a list of CIDRs (parsed with
`net/netip`, an invalid entry aborts startup naming the key and value),
added to the built-in blocklist rather than replacing it; the built-in
ranges extended to CGNAT `100.64.0.0/10`, IETF protocol assignments
`192.0.0.0/24`, benchmark `198.18.0.0/15`, and NAT64 `64:ff9b::/96`
(IPv4-mapped forms covered); enforcement stays in the dial-time
re-resolution so the DNS-rebinding window remains closed; documented in
`README.md` and `config.example.yml`.
- 2026-09-21 http.Server hardening (closes #92): added
`HTTPReadHeaderTimeout` (10s, bounds the slowloris header dribble) and
`HTTPIdleTimeout` (120s, bounds keep-alive reuse) alongside the
existing timeouts and wired them onto the server; added a `LimitBody`
middleware capping the two form POST bodies (`POST /`, `POST /generate`)
at `MaxFormBytes` (1 MiB) and returning 413, applied ahead of the CSRF
middleware so an oversized body is refused as 413 rather than being read
as a missing CSRF token (403); left `WriteTimeout` at 60s unchanged
- 2026-08-07 update golangci-lint to v2.12.2 with the canonical
`.golangci.yml` (v2 schema, `default: all` minus six disabled
linters, `lll` 88, tests included): bumped the pinned
@@ -131,8 +153,6 @@ P1: implement blocked networks configuration to extend SSRF protection
# Future Steps
- P1: rate limit global concurrent upstream fetches to prevent
resource exhaustion
- P1: strip EXIF and other metadata from processed images (privacy)
- P2: security
- referer blacklist
+11
View File
@@ -0,0 +1,11 @@
# Pixa configuration baked into the Docker image.
#
# The signing key is read from the PIXA_SIGNING_KEY environment
# variable; startup aborts naming it when it is unset. Every other key
# is omitted so its default applies. Operators who need more (an
# allowlist, metrics, and so on) mount their own file over
# /etc/pixa/config.yml.
signing_key: "${ENV:PIXA_SIGNING_KEY}"
state_dir: /var/lib/pixa
port: 8080
+24
View File
@@ -22,6 +22,30 @@ allowlist_hosts:
- github.com
- user-images.githubusercontent.com
# Additional CIDR ranges to refuse when fetching upstream, extending the
# SSRF protection. These are added to the always-enforced built-in ranges
# (loopback, RFC 1918 private, link-local, CGNAT, benchmark, NAT64, and
# similar), never replacing them. Each entry must be a valid CIDR in IPv4
# or IPv6 form; an invalid entry aborts startup.
# blocked_networks:
# - 100.64.0.0/10
# - 2001:db8::/32
# CIDR ranges of the reverse proxies in front of pixa. X-Forwarded-For
# is believed only when the direct peer is inside one of these ranges;
# the client address in the access log and login records is then the
# rightmost forwarded entry that is not itself a trusted proxy. A client
# connecting directly (peer outside these ranges) cannot spoof its
# address: the header is ignored and the peer address is used. When
# omitted, this defaults to the RFC 1918 private ranges (10.0.0.0/8,
# 172.16.0.0/12, 192.168.0.0/16), since pixa is deployed behind a proxy on
# a private network. An explicitly empty list ([]) trusts no one; an
# explicit list replaces the default. An invalid CIDR aborts startup.
# Uncomment to override the defaults with your proxy's address range.
# trusted_proxies:
# - 10.0.0.0/8
# - 2001:db8::/32
# Allow HTTP upstream (only for testing, always use HTTPS in production)
allow_http: false
+119
View File
@@ -0,0 +1,119 @@
// Package clientip resolves the real client IP address of an HTTP request
// when pixa runs behind a reverse proxy. Forwarding headers are believed
// only when the immediate peer is a configured trusted proxy, so an
// untrusted client cannot spoof its address by sending the header.
package clientip
import (
"context"
"net"
"net/netip"
"slices"
"strings"
)
// ForwardedForHeader is the request header carrying the proxy chain. It is
// honored only when the immediate peer is a trusted proxy.
const ForwardedForHeader = "X-Forwarded-For"
// Resolver determines the client IP of a request against a fixed set of
// trusted proxy networks.
type Resolver struct {
trusted []netip.Prefix
}
// NewResolver returns a Resolver that trusts forwarding headers only from
// peers inside the given CIDR ranges. A nil or empty list trusts no one,
// so the peer address is always used.
func NewResolver(trusted []netip.Prefix) *Resolver {
return &Resolver{trusted: trusted}
}
// Resolve returns the client IP for a request whose direct peer is
// remoteAddr (a "host:port" string as in http.Request.RemoteAddr) and
// whose X-Forwarded-For header lines are forwardedFor (as returned by
// http.Header.Values). When the peer is not a trusted proxy, the peer
// address is returned and the header is ignored entirely. When the peer is
// trusted, the header is walked right to left and the first address that is
// not itself a trusted proxy is returned; this is the client the outermost
// trusted proxy observed, and entries an untrusted client may have prepended
// sit to its left and are never reached.
func (r *Resolver) Resolve(remoteAddr string, forwardedFor []string) string {
peer := hostOnly(remoteAddr)
peerAddr, err := netip.ParseAddr(peer)
if err != nil || !r.isTrusted(peerAddr) {
return peer
}
for _, hop := range slices.Backward(forwardedForChain(forwardedFor)) {
hopAddr, err := netip.ParseAddr(hop)
if err != nil || r.isTrusted(hopAddr) {
continue
}
return hopAddr.String()
}
return peerAddr.String()
}
// isTrusted reports whether addr falls inside one of the trusted proxy
// ranges. Addresses are unmapped first so an IPv4-mapped IPv6 form matches
// an IPv4 range, matching the fetcher's blocklist comparison.
func (r *Resolver) isTrusted(addr netip.Addr) bool {
if !addr.IsValid() {
return false
}
unmapped := addr.Unmap()
return slices.ContainsFunc(r.trusted, func(prefix netip.Prefix) bool {
return prefix.Contains(unmapped)
})
}
// hostOnly strips the port from a "host:port" address. A value without a
// port (already a bare host) is returned unchanged.
func hostOnly(remoteAddr string) string {
host, _, err := net.SplitHostPort(remoteAddr)
if err != nil {
return remoteAddr
}
return host
}
// forwardedForChain flattens the comma-separated entries of every
// X-Forwarded-For header line into a single ordered, trimmed list.
func forwardedForChain(values []string) []string {
var chain []string
for _, value := range values {
for part := range strings.SplitSeq(value, ",") {
trimmed := strings.TrimSpace(part)
if trimmed != "" {
chain = append(chain, trimmed)
}
}
}
return chain
}
// contextKey is the private key type under which the resolved client IP is
// stored in a request context.
type contextKey struct{}
// WithClientIP returns a copy of ctx carrying the resolved client IP.
func WithClientIP(ctx context.Context, ip string) context.Context {
return context.WithValue(ctx, contextKey{}, ip)
}
// FromContext returns the resolved client IP stored in ctx, or an empty
// string if none was set.
func FromContext(ctx context.Context) string {
ip, _ := ctx.Value(contextKey{}).(string)
return ip
}
+189
View File
@@ -0,0 +1,189 @@
package clientip_test
import (
"net/netip"
"testing"
"sneak.berlin/go/pixa/internal/clientip"
)
// Addresses reused across the resolver cases.
const (
trustedRangeV4 = "10.0.0.0/8"
forwardedV4 = "203.0.113.7"
untrustedV4 = "198.51.100.9"
trustedPeer = "10.0.0.1:5000"
)
// mustPrefixes parses CIDR strings into prefixes for building a resolver.
func mustPrefixes(t *testing.T, cidrs ...string) []netip.Prefix {
t.Helper()
prefixes := make([]netip.Prefix, 0, len(cidrs))
for _, c := range cidrs {
p, err := netip.ParsePrefix(c)
if err != nil {
t.Fatalf("netip.ParsePrefix(%q) error = %v", c, err)
}
prefixes = append(prefixes, p)
}
return prefixes
}
type resolveCase struct {
name string
trusted []string
remoteAddr string
forwardedFor []string
want string
}
// runResolveCases runs each case against a resolver built from its trusted
// list and checks the resolved address.
func runResolveCases(t *testing.T, cases []resolveCase) {
t.Helper()
for _, tt := range cases {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
r := clientip.NewResolver(mustPrefixes(t, tt.trusted...))
got := r.Resolve(tt.remoteAddr, tt.forwardedFor)
if got != tt.want {
t.Errorf("Resolve(%q, %v) = %q, want %q",
tt.remoteAddr, tt.forwardedFor, got, tt.want)
}
})
}
}
// TestResolvePeerTrust covers the trust decision on the direct peer: a
// forwarded header is believed only from a trusted peer, and a client
// connecting directly cannot spoof its address.
func TestResolvePeerTrust(t *testing.T) {
t.Parallel()
runResolveCases(t, []resolveCase{
{
name: "trusted peer honors forwarded client",
trusted: []string{trustedRangeV4},
remoteAddr: trustedPeer,
forwardedFor: []string{forwardedV4},
want: forwardedV4,
},
{
name: "untrusted peer ignores forwarded header",
trusted: []string{trustedRangeV4},
remoteAddr: untrustedV4 + ":33333",
forwardedFor: []string{forwardedV4},
want: untrustedV4,
},
{
name: "spoofed chain from untrusted peer cannot influence result",
trusted: []string{trustedRangeV4},
remoteAddr: untrustedV4 + ":33333",
forwardedFor: []string{"1.2.3.4, 10.9.9.9, 127.0.0.1"},
want: untrustedV4,
},
{
name: "empty trusted list always uses peer",
trusted: nil,
remoteAddr: forwardedV4 + ":80",
forwardedFor: []string{"10.0.0.5"},
want: forwardedV4,
},
{
name: "trusted peer with no forwarded header uses peer",
trusted: []string{trustedRangeV4},
remoteAddr: trustedPeer,
forwardedFor: nil,
want: "10.0.0.1",
},
{
name: "unparseable peer is returned unchanged",
trusted: []string{trustedRangeV4},
remoteAddr: "garbage",
forwardedFor: []string{forwardedV4},
want: "garbage",
},
})
}
// TestResolveChainWalk covers walking the X-Forwarded-For chain from a
// trusted peer to the rightmost entry that is not itself a trusted proxy.
func TestResolveChainWalk(t *testing.T) {
t.Parallel()
runResolveCases(t, []resolveCase{
{
name: "rightmost untrusted entry across a mixed chain",
trusted: []string{trustedRangeV4, "192.168.0.0/16"},
remoteAddr: trustedPeer,
forwardedFor: []string{forwardedV4 + ", 192.168.1.1, 10.0.0.2"},
want: forwardedV4,
},
{
name: "spoofed client behind a trusted proxy is not believed",
trusted: []string{trustedRangeV4},
remoteAddr: trustedPeer,
forwardedFor: []string{"1.2.3.4, " + untrustedV4},
want: untrustedV4,
},
{
name: "chain split across multiple header lines",
trusted: []string{trustedRangeV4},
remoteAddr: trustedPeer,
forwardedFor: []string{forwardedV4, "10.0.0.2"},
want: forwardedV4,
},
{
name: "garbage entries are skipped",
trusted: []string{trustedRangeV4},
remoteAddr: trustedPeer,
forwardedFor: []string{forwardedV4 + ", not-an-ip"},
want: forwardedV4,
},
{
name: "all-trusted chain falls back to peer",
trusted: []string{trustedRangeV4},
remoteAddr: trustedPeer,
forwardedFor: []string{"10.0.0.9, 10.0.0.2"},
want: "10.0.0.1",
},
{
name: "trusted IPv6 peer honors forwarded client",
trusted: []string{"2001:db8::/32"},
remoteAddr: "[2001:db8::1]:9000",
forwardedFor: []string{forwardedV4},
want: forwardedV4,
},
{
name: "IPv4-mapped peer matches IPv4 trusted range",
trusted: []string{trustedRangeV4},
remoteAddr: "[::ffff:10.0.0.1]:5000",
forwardedFor: []string{forwardedV4},
want: forwardedV4,
},
})
}
func TestContextRoundTrip(t *testing.T) {
t.Parallel()
ctx := clientip.WithClientIP(t.Context(), forwardedV4)
if got := clientip.FromContext(ctx); got != forwardedV4 {
t.Errorf("FromContext = %q, want %q", got, forwardedV4)
}
}
func TestFromContextAbsent(t *testing.T) {
t.Parallel()
if got := clientip.FromContext(t.Context()); got != "" {
t.Errorf("FromContext with no value = %q, want empty", got)
}
}
@@ -0,0 +1,90 @@
package config
import (
"testing"
)
// TestBlockedNetworksParsed loads a valid blocked_networks list and checks
// each CIDR is parsed into the resolved prefixes in order.
func TestBlockedNetworksParsed(t *testing.T) {
t.Parallel()
yamlContent := signingKeyLine + `blocked_networks:
- 203.0.113.0/24
- 2001:db8::/32
`
c, err := configFromYAML(t, yamlContent)
if err != nil {
t.Fatalf("valid blocked_networks should load, got error: %v", err)
}
want := []string{"203.0.113.0/24", "2001:db8::/32"}
if len(c.BlockedNetworks) != len(want) {
t.Fatalf("BlockedNetworks = %v, want %d entries", c.BlockedNetworks, len(want))
}
for i, w := range want {
if got := c.BlockedNetworks[i].String(); got != w {
t.Errorf("BlockedNetworks[%d] = %q, want %q", i, got, w)
}
}
}
// TestBlockedNetworksOmittedIsEmpty confirms an omitted key leaves the
// operator list empty; the built-in defaults still apply in the fetcher.
func TestBlockedNetworksOmittedIsEmpty(t *testing.T) {
t.Parallel()
c, err := configFromYAML(t, signingKeyLine)
if err != nil {
t.Fatalf("minimal config should be valid, got error: %v", err)
}
if len(c.BlockedNetworks) != 0 {
t.Errorf("BlockedNetworks = %v, want empty", c.BlockedNetworks)
}
}
// TestBlockedNetworksInvalidAbortsStartup checks that malformed values abort
// startup with an error naming the key and the offending value.
func TestBlockedNetworksInvalidAbortsStartup(t *testing.T) {
t.Parallel()
runAbortCases(t, []abortCase{
{
name: "not-a-cidr",
yaml: signingKeyLine + `blocked_networks:
- not-a-cidr
`,
wantErrSubstrings: []string{keyBlockedNetworks, "not-a-cidr"},
},
{
name: "bare-address-without-prefix",
yaml: signingKeyLine + `blocked_networks:
- 10.0.0.1
`,
wantErrSubstrings: []string{keyBlockedNetworks, "10.0.0.1"},
},
{
name: "empty-entry",
yaml: signingKeyLine + `blocked_networks:
- ""
`,
wantErrSubstrings: []string{keyBlockedNetworks},
},
{
name: "non-string-entry",
yaml: signingKeyLine + `blocked_networks:
- 42
`,
wantErrSubstrings: []string{keyBlockedNetworks},
},
{
name: "null-value",
yaml: signingKeyLine + `blocked_networks:
`,
wantErrSubstrings: []string{keyBlockedNetworks, nullValueText},
},
})
}
+170 -6
View File
@@ -6,6 +6,7 @@ import (
"fmt"
"log/slog"
"math"
"net/netip"
"net/url"
"os"
"path/filepath"
@@ -42,8 +43,16 @@ const (
keyAllowHTTP = "allow_http"
keyUpstreamConnectionsPerHost = "upstream_connections_per_host"
keyCacheMaxBytes = "cache_max_bytes"
keyBlockedNetworks = "blocked_networks"
keyTrustedProxies = "trusted_proxies"
)
// placeholderSigningKey is the dummy signing_key shipped in
// config.example.yml. It is 45 characters, so it passes the length
// check, but it is public in this repository and must be rejected at
// startup so no deployment ever signs URLs with it.
const placeholderSigningKey = "CHANGE_ME_generate_with_openssl_rand_base64_32"
// Static validation errors. Each use site attaches the offending key
// and value by wrapping these with fmt.Errorf and %w.
var (
@@ -54,6 +63,7 @@ var (
errNotAnInteger = errors.New("not an integer")
errNotABoolean = errors.New("not a boolean")
errNotAStringList = errors.New("not a list of strings")
errNotAValidCIDR = errors.New("not a valid CIDR network")
errNotAMetricsMap = errors.New("not a map of metrics settings")
errEmptyListEntry = errors.New("list contains an empty entry")
errEmptyEntry = errors.New("contains an empty entry")
@@ -61,6 +71,9 @@ var (
errPortOutOfRange = errors.New("outside the valid port range")
errTooFewConnections = errors.New("must be at least 1")
errValueTooShort = errors.New("value too short")
errPlaceholderKey = errors.New(
"is the placeholder from config.example.yml; " +
"generate a real key with: openssl rand -base64 32")
errMustBeSetTogether = errors.New("must be set together")
errMustNotBeNegative = errors.New("must not be negative")
errOverflowsInt64 = errors.New("overflows a 64-bit integer")
@@ -100,6 +113,22 @@ type Config struct {
AllowHTTP bool // Allow non-TLS upstream (testing only)
UpstreamConnectionsPerHost int // Max concurrent connections per upstream host
// BlockedNetworks are operator-supplied CIDR ranges to refuse in
// addition to the built-in SSRF blocklist. Enforced by the upstream
// fetcher's dialer; the built-in ranges always apply.
BlockedNetworks []netip.Prefix
// TrustedProxies are the CIDR ranges of reverse proxies whose
// forwarding headers may be believed. Forwarded headers are honored
// only when the immediate peer falls inside one of these ranges;
// otherwise the peer address is used and the headers are ignored, so
// an untrusted client cannot spoof its address. An omitted key
// defaults to the RFC 1918 private ranges (see defaultTrustedProxies),
// since pixa is deployed behind a proxy on a private network; an
// explicitly empty list trusts nothing and always uses the peer
// address, and an explicit list replaces the default.
TrustedProxies []netip.Prefix
// CacheMaxBytes is the disk cache size limit in bytes. Zero
// disables the disk cache entirely. When cache_max_bytes is
// omitted from the configuration, this holds the computed default
@@ -168,6 +197,24 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
}
}
blockedNetworks, err := parseCIDRList(sc, keyBlockedNetworks)
if err != nil {
return nil, err
}
trustedProxies, err := parseCIDRList(sc, keyTrustedProxies)
if err != nil {
return nil, err
}
// parseCIDRList returns a nil slice only when the key is absent; an
// explicitly empty list ([]) comes back non-nil and empty. An omitted
// key takes the RFC 1918 default, while an explicit empty list is left
// as trust-nothing.
if trustedProxies == nil {
trustedProxies = defaultTrustedProxies()
}
loader := &strictLoader{sc: sc}
c := &Config{
@@ -184,6 +231,8 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
UpstreamConnectionsPerHost: loader.intVal(
keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost),
CacheMaxBytes: loader.int64Val(keyCacheMaxBytes, 0),
BlockedNetworks: blockedNetworks,
TrustedProxies: trustedProxies,
}
// The computed default for cache_max_bytes needs a validated
@@ -215,7 +264,7 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
return nil, loader.err
}
err := c.validate()
err = c.validate()
if err != nil {
return nil, err
}
@@ -299,7 +348,8 @@ func isKnownConfigKey(key string) bool {
switch key {
case keyDebug, keyMaintenanceMode, keyPort, keyStateDir, keySentryDSN,
keyDBURL, keyMetrics, keySigningKey, keyAllowlistHosts, keyAllowHTTP,
keyUpstreamConnectionsPerHost, keyCacheMaxBytes, "env":
keyUpstreamConnectionsPerHost, keyCacheMaxBytes, keyBlockedNetworks,
keyTrustedProxies, "env":
return true
}
@@ -341,10 +391,10 @@ func (c *Config) ensureStateDirWritable() error {
return nil
}
// validate checks that all required configuration values are set and
// that every value is within its valid range.
func (c *Config) validate() error {
// The signing key value is never echoed in error messages.
// validateSigningKey checks that the signing key is present, long
// enough, and not the public placeholder from config.example.yml. The
// key value itself is never echoed in error messages.
func (c *Config) validateSigningKey() error {
if c.SigningKey == "" {
return fmt.Errorf("config key %q: %w", keySigningKey, errValueRequired)
}
@@ -356,6 +406,21 @@ func (c *Config) validate() error {
keySigningKey, errValueTooShort, minKeyLength, len(c.SigningKey))
}
if c.SigningKey == placeholderSigningKey {
return fmt.Errorf("config key %q: %w", keySigningKey, errPlaceholderKey)
}
return nil
}
// validate checks that all required configuration values are set and
// that every value is within its valid range.
func (c *Config) validate() error {
err := c.validateSigningKey()
if err != nil {
return err
}
const maxPort = 65535
if c.Port < 1 || c.Port > maxPort {
return fmt.Errorf("config key %q: value %d is %w 1-%d",
@@ -778,3 +843,102 @@ func getStringSlice(sc *smartconfig.Config) []string {
return nil
}
// defaultTrustedProxies returns the trusted_proxies default: the three RFC
// 1918 private ranges. pixa is always deployed behind a TLS-terminating
// reverse proxy, which in practice sits on a private network, so its
// forwarding headers are believed unless the operator says otherwise.
// Loopback is deliberately excluded: it is not an RFC 1918 range, and no
// deployment reaches pixa over it. A fresh slice is returned on each call so
// callers may hold it without aliasing shared state.
func defaultTrustedProxies() []netip.Prefix {
return []netip.Prefix{
netip.MustParsePrefix("10.0.0.0/8"),
netip.MustParsePrefix("172.16.0.0/12"),
netip.MustParsePrefix("192.168.0.0/16"),
}
}
// parseCIDRList parses the value of the named config key into CIDR
// prefixes, or returns nil if the key is omitted. It accepts a YAML list
// of strings or a comma-separated string. An explicitly null value, a
// wrong type, an empty entry, a non-string entry, or an unparseable CIDR
// aborts startup naming the key and the offending value; the default
// (an empty list) applies only to an omitted key.
func parseCIDRList(sc *smartconfig.Config, key string) ([]netip.Prefix, error) {
if sc == nil {
return nil, nil
}
raw, ok := sc.Get(key)
if !ok {
return nil, nil
}
if raw == nil {
return nil, errNullConfigValue(key)
}
entries, err := cidrListEntries(raw, key)
if err != nil {
return nil, err
}
prefixes := make([]netip.Prefix, 0, len(entries))
for _, entry := range entries {
prefix, err := netip.ParsePrefix(entry)
if err != nil {
return nil, fmt.Errorf("config key %q: value %q is %w",
key, entry, errNotAValidCIDR)
}
prefixes = append(prefixes, prefix)
}
return prefixes, nil
}
// cidrListEntries extracts the raw entries of the named CIDR-list key as
// trimmed, non-empty strings, from either a YAML list of strings or a
// comma-separated string. Any other shape is a configuration error.
func cidrListEntries(raw any, key string) ([]string, error) {
switch val := raw.(type) {
case []any:
entries := make([]string, 0, len(val))
for _, item := range val {
str, ok := item.(string)
if !ok {
return nil, fmt.Errorf("config key %q: list entry %v (%T) is %w",
key, item, item, errNotAString)
}
if strings.TrimSpace(str) == "" {
return nil, fmt.Errorf("config key %q: %w",
key, errEmptyListEntry)
}
entries = append(entries, strings.TrimSpace(str))
}
return entries, nil
case string:
entries := make([]string, 0)
for part := range strings.SplitSeq(val, ",") {
trimmed := strings.TrimSpace(part)
if trimmed == "" {
return nil, fmt.Errorf("config key %q: value %q %w",
key, val, errEmptyEntry)
}
entries = append(entries, trimmed)
}
return entries, nil
default:
return nil, fmt.Errorf("config key %q: value %v (%T) is %w",
key, raw, raw, errNotAStringList)
}
}
@@ -303,6 +303,11 @@ func invalidHostAndCredentialCases() []abortCase {
yaml: "signing_key: short\n",
wantErrSubstrings: []string{keySigningKey},
},
{
name: "signing_key is the documented placeholder",
yaml: "signing_key: " + placeholderSigningKey + "\n",
wantErrSubstrings: []string{keySigningKey},
},
{
name: "signing_key missing",
yaml: "port: 8080\n",
@@ -0,0 +1,85 @@
package config
import (
"strings"
"testing"
)
// TestTrustedProxiesConfig checks the trusted_proxies key wiring: an
// explicit CIDR list lands in TrustedProxies in order and replaces the
// default, an omitted key falls back to the RFC 1918 private ranges, and an
// explicitly empty list trusts no one. The list parser itself is shared with
// blocked_networks and is exercised in depth by that key's tests.
func TestTrustedProxiesConfig(t *testing.T) {
t.Parallel()
t.Run("explicit list replaces the default in order", func(t *testing.T) {
t.Parallel()
c, err := configFromYAML(t,
signingKeyLine+`trusted_proxies: ["10.0.0.0/8", "2001:db8::/32"]`+"\n")
if err != nil {
t.Fatalf("valid trusted_proxies should load: %v", err)
}
got := make([]string, len(c.TrustedProxies))
for i, p := range c.TrustedProxies {
got[i] = p.String()
}
if joined := strings.Join(got, ","); joined != "10.0.0.0/8,2001:db8::/32" {
t.Errorf("TrustedProxies = %v, want the two ranges in order", got)
}
})
t.Run("omitted key defaults to the RFC 1918 ranges", func(t *testing.T) {
t.Parallel()
c, err := configFromYAML(t, signingKeyLine)
if err != nil {
t.Fatalf("minimal config should load: %v", err)
}
got := make([]string, len(c.TrustedProxies))
for i, p := range c.TrustedProxies {
got[i] = p.String()
}
want := "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16"
if joined := strings.Join(got, ","); joined != want {
t.Errorf("TrustedProxies = %v, want the RFC 1918 ranges %q", got, want)
}
})
t.Run("explicitly empty list trusts no one", func(t *testing.T) {
t.Parallel()
c, err := configFromYAML(t, signingKeyLine+"trusted_proxies: []\n")
if err != nil {
t.Fatalf("empty trusted_proxies should load: %v", err)
}
if len(c.TrustedProxies) != 0 {
t.Errorf("TrustedProxies = %v, want empty", c.TrustedProxies)
}
})
}
// TestTrustedProxiesInvalidAbortsStartup checks that an invalid or null
// value aborts startup with an error naming the key and the offending value.
func TestTrustedProxiesInvalidAbortsStartup(t *testing.T) {
t.Parallel()
runAbortCases(t, []abortCase{
{
name: "invalid cidr",
yaml: signingKeyLine + `trusted_proxies: ["999.0.0.0/8"]` + "\n",
wantErrSubstrings: []string{keyTrustedProxies, "999.0.0.0/8"},
},
{
name: "null value",
yaml: signingKeyLine + "trusted_proxies:\n",
wantErrSubstrings: []string{keyTrustedProxies, nullValueText},
},
})
}
+5 -2
View File
@@ -8,6 +8,7 @@ import (
"strconv"
"time"
"sneak.berlin/go/pixa/internal/clientip"
"sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/imgcache"
"sneak.berlin/go/pixa/internal/templates"
@@ -47,7 +48,8 @@ func (s *Handlers) handleLoginPost(w http.ResponseWriter, r *http.Request) {
// Constant-time comparison to prevent timing attacks
if subtle.ConstantTimeCompare([]byte(submittedKey), []byte(s.config.SigningKey)) != 1 {
s.log.Warn("failed login attempt", "remote_addr", r.RemoteAddr)
s.log.Warn("failed login attempt",
"remote_addr", clientip.FromContext(r.Context()))
s.renderLogin(w, r, "Invalid signing key")
return
@@ -62,7 +64,8 @@ func (s *Handlers) handleLoginPost(w http.ResponseWriter, r *http.Request) {
return
}
s.log.Info("successful login", "remote_addr", r.RemoteAddr)
s.log.Info("successful login",
"remote_addr", clientip.FromContext(r.Context()))
// Redirect to generator page
http.Redirect(w, r, "/", http.StatusSeeOther)
@@ -0,0 +1,41 @@
package handlers
import (
"bytes"
"log/slog"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"testing"
"sneak.berlin/go/pixa/internal/clientip"
"sneak.berlin/go/pixa/internal/config"
)
// TestFailedLoginLogsResolvedClientIP verifies the failed-login record
// carries the resolved client IP from the request context, not the raw
// proxy peer address.
func TestFailedLoginLogsResolvedClientIP(t *testing.T) {
t.Parallel()
var buf bytes.Buffer
h := &Handlers{
log: slog.New(slog.NewJSONHandler(&buf, nil)),
config: &config.Config{SigningKey: testSigningKey},
}
form := url.Values{loginKeyField: {"wrong-key"}}
req := httptest.NewRequestWithContext(
t.Context(), http.MethodPost, "/",
strings.NewReader(form.Encode()))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
req = req.WithContext(clientip.WithClientIP(req.Context(), "203.0.113.7"))
h.handleLoginPost(httptest.NewRecorder(), req)
if !strings.Contains(buf.String(), `"remote_addr":"203.0.113.7"`) {
t.Errorf("failed-login log missing resolved client IP; got %q", buf.String())
}
}
+45
View File
@@ -0,0 +1,45 @@
package handlers
import (
"errors"
"net/http"
)
// MaxFormBytes bounds the request body accepted on the HTML form POST
// routes (POST / and POST /generate). The forms carry a handful of short
// fields, so 1 MiB is generous while making the bound explicit rather than
// resting on ParseForm's incidental 10 MB cap.
const MaxFormBytes = 1 << 20 // 1 MiB
// LimitBody returns middleware that caps the request body on POST requests
// at maxBytes and rejects an oversized body with 413 Request Entity Too
// Large.
//
// It parses the form here, before the CSRF middleware reads the token from
// it. The CSRF middleware reads the token with PostFormValue, which
// swallows a parse error, so if the body were only capped there an
// oversized body would read as a missing token and be refused as 403. By
// parsing under the cap first, an oversized body is refused as 413. A
// successful parse is cached on the request, so the CSRF check and the
// handler reuse it rather than reading the body again.
func (s *Handlers) LimitBody(maxBytes int64) func(http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method == http.MethodPost {
r.Body = http.MaxBytesReader(w, r.Body, maxBytes)
err := r.ParseForm()
var tooLarge *http.MaxBytesError
if errors.As(err, &tooLarge) {
http.Error(w, "Request body too large",
http.StatusRequestEntityTooLarge)
return
}
}
next.ServeHTTP(w, r)
})
}
}
@@ -0,0 +1,177 @@
package handlers
import (
"log/slog"
"net/http"
"net/url"
"strings"
"testing"
"github.com/go-chi/chi/v5"
"sneak.berlin/go/pixa/internal/config"
"sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/session"
)
// Form field names and a throwaway source image URL for the body-limit
// tests.
const (
sourceURLField = "url"
testSourceURL = "https://example.com/a.jpg"
)
// newBodyLimitTestRouter mirrors the production wiring for the form POST
// routes (see server.SetupRoutes): LimitBody sits in front of the CSRF
// middleware, which sits in front of the handlers. maxBytes is the body
// cap under test, so a test can trip the limit with a small body.
func newBodyLimitTestRouter(
t *testing.T, maxBytes int64,
) (*Handlers, http.Handler) {
t.Helper()
cfg := &config.Config{SigningKey: testSigningKey, Debug: true}
sessMgr, err := session.NewManager(testSigningKey)
if err != nil {
t.Fatalf("session.NewManager() error = %v", err)
}
encGen, err := encurl.NewGenerator(testSigningKey)
if err != nil {
t.Fatalf("encurl.NewGenerator() error = %v", err)
}
protect, err := newCSRFProtect(testSigningKey, cfg.Debug)
if err != nil {
t.Fatalf("newCSRFProtect() error = %v", err)
}
h := &Handlers{
log: slog.New(slog.DiscardHandler),
config: cfg,
sessMgr: sessMgr,
encGen: encGen,
csrfProtect: protect,
}
r := chi.NewRouter()
r.Group(func(r chi.Router) {
r.Use(h.LimitBody(maxBytes))
r.Use(h.CSRF())
r.Get("/", h.HandleRoot())
r.Post("/", h.HandleRoot())
r.Post("/generate", h.HandleGenerateURL())
})
return h, r
}
// TestOversizedLoginPostRejectedBeforeCSRF is the core regression: an
// oversized POST / carrying an otherwise valid CSRF cookie and token must
// be rejected with 413. If the body limit ran after CSRF, the truncated
// body would read as a missing token and return 403; if it ran after the
// handler, a valid token would return 303. Getting 413 proves the limit
// fires before CSRF parses the form.
func TestOversizedLoginPostRejectedBeforeCSRF(t *testing.T) {
t.Parallel()
_, srv := newBodyLimitTestRouter(t, 16)
cookies, token := csrfCredentials(t, srv, nil)
rec := postForm(srv, "/", cookies, url.Values{
loginKeyField: {testSigningKey},
csrfTokenField: {token},
})
if rec.Code != http.StatusRequestEntityTooLarge {
t.Errorf("oversized POST / status = %d, want %d",
rec.Code, http.StatusRequestEntityTooLarge)
}
}
// TestOversizedGeneratePostRejectedBeforeCSRF is the same regression for
// POST /generate, which also parses a form behind CSRF.
func TestOversizedGeneratePostRejectedBeforeCSRF(t *testing.T) {
t.Parallel()
h, srv := newBodyLimitTestRouter(t, 16)
sessionCookie := newSessionCookie(t, h)
cookies, token := csrfCredentials(t, srv, []*http.Cookie{sessionCookie})
cookies = append(cookies, sessionCookie)
rec := postForm(srv, "/generate", cookies, url.Values{
sourceURLField: {testSourceURL},
csrfTokenField: {token},
})
if rec.Code != http.StatusRequestEntityTooLarge {
t.Errorf("oversized POST /generate status = %d, want %d",
rec.Code, http.StatusRequestEntityTooLarge)
}
}
// TestWithinLimitLoginPostSucceeds verifies the limit does not disturb a
// normal request: under the production cap, a valid login still parses and
// establishes a session (303). This guards against the body limit
// consuming or corrupting the form the CSRF check and handler depend on.
func TestWithinLimitLoginPostSucceeds(t *testing.T) {
t.Parallel()
_, srv := newBodyLimitTestRouter(t, MaxFormBytes)
cookies, token := csrfCredentials(t, srv, nil)
rec := postForm(srv, "/", cookies, url.Values{
loginKeyField: {testSigningKey},
csrfTokenField: {token},
})
if rec.Code != http.StatusSeeOther {
t.Fatalf("within-limit POST / status = %d, want %d",
rec.Code, http.StatusSeeOther)
}
var authed bool
for _, c := range rec.Result().Cookies() {
if c.Name == session.CookieName && c.Value != "" {
authed = true
}
}
if !authed {
t.Error("within-limit valid login did not set a session cookie")
}
}
// TestWithinLimitGeneratePostSucceeds is the same non-regression check for
// POST /generate.
func TestWithinLimitGeneratePostSucceeds(t *testing.T) {
t.Parallel()
h, srv := newBodyLimitTestRouter(t, MaxFormBytes)
sessionCookie := newSessionCookie(t, h)
cookies, token := csrfCredentials(t, srv, []*http.Cookie{sessionCookie})
cookies = append(cookies, sessionCookie)
rec := postForm(srv, "/generate", cookies, url.Values{
sourceURLField: {testSourceURL},
"format": {"jpeg"},
csrfTokenField: {token},
})
if rec.Code != http.StatusOK {
t.Fatalf("within-limit POST /generate status = %d, want %d",
rec.Code, http.StatusOK)
}
if !strings.Contains(rec.Body.String(), "/v1/e/") {
t.Error("within-limit generate response did not contain a generated URL")
}
}
+2
View File
@@ -111,6 +111,8 @@ func (s *Handlers) initImageService() error {
fetcherCfg.MaxConnectionsPerHost = s.config.UpstreamConnectionsPerHost
}
fetcherCfg.BlockedNetworks = s.config.BlockedNetworks
// Create the service
svc, err := imgcache.NewService(&imgcache.ServiceConfig{
Cache: cache,
@@ -0,0 +1,111 @@
package httpfetcher
import (
"context"
"errors"
"net"
"net/http"
"net/netip"
"testing"
)
// TestIsPrivateIPBlocksSpecialRanges covers the internal and special-use
// ranges added to the built-in blocklist, in IPv4, IPv6, and IPv4-mapped
// forms, alongside public controls that must stay reachable.
func TestIsPrivateIPBlocksSpecialRanges(t *testing.T) {
t.Parallel()
tests := []struct {
name string
ip string
want bool
}{
{"cgnat-low", "100.64.0.1", true},
{"cgnat-high", "100.127.255.254", true},
{"ietf-protocol", "192.0.0.1", true},
{"benchmark-low", "198.18.0.1", true},
{"benchmark-high", "198.19.255.254", true},
{"nat64", "64:ff9b::1", true},
{"nat64-embeds-private", "64:ff9b::a00:1", true}, // maps 10.0.0.1
{"ipv4-mapped-private", "::ffff:10.0.0.1", true},
{"cloud-metadata", "169.254.169.254", true},
{"public-v4", "8.8.8.8", false},
{"test-net-1-public", testPublicHost, false}, // TEST-NET-1, stays public
{"public-v6", "2001:4860:4860::8888", false},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
ip := net.ParseIP(tc.ip)
if ip == nil {
t.Fatalf("failed to parse IP %q", tc.ip)
}
got := isPrivateIP(ip)
if got != tc.want {
t.Errorf("isPrivateIP(%q) = %v, want %v", tc.ip, got, tc.want)
}
})
}
}
// transportOf returns the *http.Transport backing a fetcher, so a test can
// exercise the SSRF-safe dialer New installed with the operator blocklist.
func transportOf(t *testing.T, f *HTTPFetcher) *http.Transport {
t.Helper()
transport, ok := f.client.Transport.(*http.Transport)
if !ok {
t.Fatalf("transport is %T, want *http.Transport", f.client.Transport)
}
return transport
}
// TestDialerEnforcesBlockedNetworks proves an operator-supplied
// blocked_networks entry is enforced by the dialer, in addition to the
// built-in ranges, while an address outside both stays dialable.
func TestDialerEnforcesBlockedNetworks(t *testing.T) {
t.Parallel()
cfg := DefaultConfig()
// TEST-NET-2 (198.51.100.0/24) is public to the built-in check, so
// blocking it can only come from the operator-supplied list.
cfg.BlockedNetworks = []netip.Prefix{netip.MustParsePrefix("198.51.100.0/24")}
transport := transportOf(t, New(cfg))
blocked := []string{
"198.51.100.5:80", // operator-supplied range
"10.0.0.5:80", // built-in RFC 1918, still enforced
"100.64.0.1:80", // built-in CGNAT range
}
for _, addr := range blocked {
t.Run("blocked/"+addr, func(t *testing.T) {
t.Parallel()
_, err := transport.DialContext(context.Background(), "tcp", addr)
if !errors.Is(err, ErrSSRFBlocked) {
t.Errorf("DialContext(%q) = %v, want ErrSSRFBlocked", addr, err)
}
})
}
t.Run("public-not-blocked", func(t *testing.T) {
t.Parallel()
// A cancelled context makes the dial fail without touching the
// network; the point is only that a public literal outside every
// blocked range is not SSRF-blocked.
ctx, cancel := context.WithCancel(context.Background())
cancel()
_, err := transport.DialContext(ctx, "tcp", testPublicHost+":80")
if errors.Is(err, ErrSSRFBlocked) {
t.Errorf("public target SSRF-blocked with operator list set: %v", err)
}
})
}
+71 -5
View File
@@ -11,6 +11,7 @@ import (
"net"
"net/http"
"net/http/httptrace"
"net/netip"
neturl "net/url"
"slices"
"strings"
@@ -46,6 +47,20 @@ const (
localhostIPv6 = "::1"
)
// builtinBlockedPrefixes are internal or special-use ranges that Go's
// net.IP predicates (IsPrivate, IsLinkLocalUnicast, and the like) do not
// already cover. They are always blocked, in addition to any
// operator-supplied networks. IPv4-mapped IPv6 addresses are unmapped
// before matching, so these IPv4 ranges are caught in both forms.
//
//nolint:gochecknoglobals // immutable built-in blocklist
var builtinBlockedPrefixes = []netip.Prefix{
netip.MustParsePrefix("100.64.0.0/10"), // RFC 6598 CGNAT / carrier-grade NAT
netip.MustParsePrefix("192.0.0.0/24"), // RFC 6890 IETF protocol assignments
netip.MustParsePrefix("198.18.0.0/15"), // RFC 2544 benchmarking range
netip.MustParsePrefix("64:ff9b::/96"), // RFC 6052 NAT64 (maps onto IPv4)
}
// Fetcher errors.
var (
ErrSSRFBlocked = errors.New("request blocked: private or internal IP")
@@ -107,6 +122,9 @@ type Config struct {
AllowHTTP bool
// MaxConnectionsPerHost limits concurrent connections to each upstream host.
MaxConnectionsPerHost int
// BlockedNetworks are operator-supplied CIDR ranges refused by the
// dialer, in addition to the always-enforced built-in ranges.
BlockedNetworks []netip.Prefix
}
// DefaultConfig returns a Config with sensible defaults.
@@ -142,9 +160,13 @@ func New(config *Config) *HTTPFetcher {
config = DefaultConfig()
}
// Create transport with SSRF-safe dialer
// Create transport with SSRF-safe dialer. The dialer re-resolves and
// re-checks at connect time (closing the DNS-rebinding window) against
// both the built-in ranges and the operator-supplied blocklist.
transport := &http.Transport{
DialContext: ssrfSafeDialer,
DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
return dialSSRFSafe(ctx, network, addr, config.BlockedNetworks)
},
TLSHandshakeTimeout: DefaultTLSTimeout,
MaxIdleConns: DefaultMaxIdleConns,
IdleConnTimeout: DefaultIdleConnTimeout,
@@ -451,11 +473,53 @@ func isPrivateIP(ip net.IP) bool {
}
}
return false
// Special-use ranges the net.IP predicates above do not cover.
addr, ok := netip.AddrFromSlice(ip)
if !ok {
return true
}
addr = addr.Unmap()
return slices.ContainsFunc(builtinBlockedPrefixes, func(prefix netip.Prefix) bool {
return prefix.Contains(addr)
})
}
// ssrfSafeDialer is a custom dialer that validates IP addresses before connecting.
// isBlockedIP reports whether ip is refused, either by the built-in
// internal-range check or by one of the operator-supplied prefixes.
func isBlockedIP(ip net.IP, blocked []netip.Prefix) bool {
if isPrivateIP(ip) {
return true
}
addr, ok := netip.AddrFromSlice(ip)
if !ok {
return true
}
addr = addr.Unmap()
return slices.ContainsFunc(blocked, func(prefix netip.Prefix) bool {
return prefix.Contains(addr)
})
}
// ssrfSafeDialer validates IP addresses against the built-in blocked ranges
// before connecting. New wraps dialSSRFSafe with the operator-supplied
// blocklist; this entry point enforces the built-in ranges alone.
func ssrfSafeDialer(ctx context.Context, network, addr string) (net.Conn, error) {
return dialSSRFSafe(ctx, network, addr, nil)
}
// dialSSRFSafe re-resolves addr and refuses to connect to any built-in
// internal range or operator-supplied blocked prefix, closing the
// DNS-rebinding window at connect time.
func dialSSRFSafe(
ctx context.Context,
network, addr string,
blocked []netip.Prefix,
) (net.Conn, error) {
host, port, err := net.SplitHostPort(addr)
if err != nil {
return nil, err
@@ -468,9 +532,11 @@ func ssrfSafeDialer(ctx context.Context, network, addr string) (net.Conn, error)
}
// Check all resolved IPs
if slices.ContainsFunc(ips, isPrivateIP) {
for _, ip := range ips {
if isBlockedIP(ip, blocked) {
return nil, ErrSSRFBlocked
}
}
// Connect using the first valid IP
var dialer net.Dialer
@@ -0,0 +1,118 @@
package middleware
import (
"bytes"
"log/slog"
"net/http"
"net/http/httptest"
"net/netip"
"strings"
"testing"
"sneak.berlin/go/pixa/internal/clientip"
"sneak.berlin/go/pixa/internal/config"
)
// testForwardedClient is the client address the proxy forwards.
const testForwardedClient = "203.0.113.7"
// newTestMiddleware builds a Middleware whose resolver trusts the given
// CIDRs and whose logger writes JSON to buf.
func newTestMiddleware(t *testing.T, buf *bytes.Buffer, trusted ...string) *Middleware {
t.Helper()
prefixes := make([]netip.Prefix, 0, len(trusted))
for _, c := range trusted {
p, err := netip.ParsePrefix(c)
if err != nil {
t.Fatalf("netip.ParsePrefix(%q) error = %v", c, err)
}
prefixes = append(prefixes, p)
}
return &Middleware{
log: slog.New(slog.NewJSONHandler(buf, nil)),
config: &config.Config{TrustedProxies: prefixes},
clientIP: clientip.NewResolver(prefixes),
}
}
// TestClientIPMiddlewareStoresResolvedIP verifies the ClientIP middleware
// puts the resolved address into the request context for a trusted and an
// untrusted peer.
func TestClientIPMiddlewareStoresResolvedIP(t *testing.T) {
t.Parallel()
tests := []struct {
name string
remoteAddr string
forwarded string
want string
}{
{
name: "trusted peer honors forwarded client",
remoteAddr: "10.0.0.1:5000",
forwarded: testForwardedClient,
want: testForwardedClient,
},
{
name: "untrusted peer ignores forwarded header",
remoteAddr: "198.51.100.9:5000",
forwarded: testForwardedClient,
want: "198.51.100.9",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
mw := newTestMiddleware(t, &bytes.Buffer{}, "10.0.0.0/8")
var got string
handler := mw.ClientIP()(http.HandlerFunc(
func(_ http.ResponseWriter, r *http.Request) {
got = clientip.FromContext(r.Context())
}))
req := httptest.NewRequestWithContext(
t.Context(), http.MethodGet, "/", nil)
req.RemoteAddr = tt.remoteAddr
req.Header.Set("X-Forwarded-For", tt.forwarded)
handler.ServeHTTP(httptest.NewRecorder(), req)
if got != tt.want {
t.Errorf("client IP in context = %q, want %q", got, tt.want)
}
})
}
}
// TestLoggingUsesResolvedClientIP verifies the logging middleware records
// the resolved forwarded client IP rather than the proxy peer address.
func TestLoggingUsesResolvedClientIP(t *testing.T) {
t.Parallel()
var buf bytes.Buffer
mw := newTestMiddleware(t, &buf, "10.0.0.0/8")
handler := mw.ClientIP()(mw.Logging()(http.HandlerFunc(
func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
})))
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
req.RemoteAddr = "10.0.0.1:5000"
req.Header.Set("X-Forwarded-For", testForwardedClient)
handler.ServeHTTP(httptest.NewRecorder(), req)
if !strings.Contains(buf.String(), `"remoteIP":"`+testForwardedClient+`"`) {
t.Errorf("log output missing resolved client IP; got %q", buf.String())
}
}
+52 -12
View File
@@ -3,7 +3,6 @@ package middleware
import (
"log/slog"
"net"
"net/http"
"time"
@@ -14,6 +13,7 @@ import (
ghmm "github.com/slok/go-http-metrics/middleware"
"github.com/slok/go-http-metrics/middleware/std"
"go.uber.org/fx"
"sneak.berlin/go/pixa/internal/clientip"
"sneak.berlin/go/pixa/internal/config"
"sneak.berlin/go/pixa/internal/logger"
)
@@ -21,6 +21,33 @@ import (
// CORSMaxAgeSeconds is the max age for CORS preflight cache (24 hours).
const CORSMaxAgeSeconds = 86400
// HSTSValue is the Strict-Transport-Security header value: one year with
// includeSubDomains. Emitted unconditionally even though pixa listens plain
// HTTP behind a TLS-terminating proxy; browsers ignore an HSTS header received
// over plaintext (RFC 6797 section 8.1), so it never lies about the connection,
// and emitting it here avoids trusting a forwarded-proto header.
const HSTSValue = "max-age=31536000; includeSubDomains"
// ContentSecurityPolicyValue is the Content-Security-Policy header value.
// default-src 'self' is the baseline and frame-ancestors 'none' is the primary
// clickjacking control. 'unsafe-inline' is required in script-src and style-src
// because the served templates carry inline onclick handlers (generator page)
// and the bundled Tailwind asset injects a runtime <style> element; dropping it
// needs template changes outside this issue's scope.
const ContentSecurityPolicyValue = "default-src 'self'; " +
"script-src 'self' 'unsafe-inline'; " +
"style-src 'self' 'unsafe-inline'; " +
"object-src 'none'; " +
"base-uri 'self'; " +
"form-action 'self'; " +
"frame-ancestors 'none'"
// PermissionsPolicyValue is the Permissions-Policy header value. Every listed
// feature is denied because pixa uses none of them.
const PermissionsPolicyValue = "accelerometer=(), autoplay=(), camera=(), " +
"display-capture=(), geolocation=(), gyroscope=(), magnetometer=(), " +
"microphone=(), payment=(), usb=()"
// Params defines dependencies for Middleware.
type Params struct {
fx.In
@@ -33,6 +60,7 @@ type Params struct {
type Middleware struct {
log *slog.Logger
config *config.Config
clientIP *clientip.Resolver
}
// New creates a new Middleware instance.
@@ -40,22 +68,24 @@ func New(_ fx.Lifecycle, params Params) (*Middleware, error) {
s := &Middleware{
log: params.Logger.Get(),
config: params.Config,
clientIP: clientip.NewResolver(params.Config.TrustedProxies),
}
return s, nil
}
func ipFromHostPort(hp string) string {
h, _, err := net.SplitHostPort(hp)
if err != nil {
return ""
// ClientIP returns a middleware that resolves the real client IP,
// honoring X-Forwarded-For only from trusted proxies, and stores it in
// the request context for the logging middleware and handlers to read.
func (s *Middleware) ClientIP() func(http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
ip := s.clientIP.Resolve(
r.RemoteAddr, r.Header.Values(clientip.ForwardedForHeader))
ctx := clientip.WithClientIP(r.Context(), ip)
next.ServeHTTP(w, r.WithContext(ctx))
})
}
if len(h) > 0 && h[0] == '[' {
return h[1 : len(h)-1]
}
return h
}
type loggingResponseWriter struct {
@@ -100,7 +130,7 @@ func (s *Middleware) Logging() func(http.Handler) http.Handler {
"request_id", reqID,
"referer", r.Referer(),
"proto", r.Proto,
"remoteIP", ipFromHostPort(r.RemoteAddr),
"remoteIP", clientip.FromContext(ctx),
"status", lrw.statusCode,
"response_bytes", lrw.bytesWritten,
"latency_ms", latency.Milliseconds(),
@@ -164,6 +194,16 @@ func (s *Middleware) SecurityHeaders() func(http.Handler) http.Handler {
// Disable XSS filtering (modern browsers don't need it, can cause issues)
w.Header().Set("X-XSS-Protection", "0")
// Force HTTPS on future visits (ignored by browsers over plaintext)
w.Header().Set("Strict-Transport-Security", HSTSValue)
// Restrict content sources; frame-ancestors is the primary
// clickjacking control, X-Frame-Options the legacy fallback
w.Header().Set("Content-Security-Policy", ContentSecurityPolicyValue)
// Deny browser features pixa does not use
w.Header().Set("Permissions-Policy", PermissionsPolicyValue)
next.ServeHTTP(w, r)
})
}
@@ -56,6 +56,61 @@ func TestSecurityHeaders(t *testing.T) {
}
}
func TestSecurityHeaders_PolicyHeaders(t *testing.T) {
t.Parallel()
cfg := &config.Config{}
mw := &Middleware{
log: slog.Default(),
config: cfg,
}
testHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
})
handler := mw.SecurityHeaders()(testHandler)
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/test", nil)
rec := httptest.NewRecorder()
handler.ServeHTTP(rec, req)
tests := []struct {
header string
want string
}{
{"Strict-Transport-Security", "max-age=31536000; includeSubDomains"},
{
"Content-Security-Policy",
"default-src 'self'; " +
"script-src 'self' 'unsafe-inline'; " +
"style-src 'self' 'unsafe-inline'; " +
"object-src 'none'; " +
"base-uri 'self'; " +
"form-action 'self'; " +
"frame-ancestors 'none'",
},
{
"Permissions-Policy",
"accelerometer=(), autoplay=(), camera=(), " +
"display-capture=(), geolocation=(), gyroscope=(), " +
"magnetometer=(), microphone=(), payment=(), usb=()",
},
}
for _, tt := range tests {
t.Run(tt.header, func(t *testing.T) {
t.Parallel()
got := rec.Header().Get(tt.header)
if got != tt.want {
t.Errorf("%s = %q, want %q", tt.header, got, tt.want)
}
})
}
}
func TestSecurityHeaders_PreservesExistingHeaders(t *testing.T) {
t.Parallel()
+21 -5
View File
@@ -10,23 +10,39 @@ import (
// HTTP server configuration constants.
const (
HTTPReadTimeout = 30 * time.Second
// HTTPReadHeaderTimeout bounds the request-header read on its own,
// short, so a slowloris client dribbling headers is dropped well
// before it ties up a connection for the whole ReadTimeout window.
HTTPReadHeaderTimeout = 10 * time.Second
HTTPWriteTimeout = 60 * time.Second
// HTTPIdleTimeout bounds how long an idle keep-alive connection is
// held open, so idle connections cannot accumulate without limit on a
// service targeting high concurrency.
HTTPIdleTimeout = 120 * time.Second
HTTPMaxHeaderBytes = 8 << 10 // 8KB
)
func (s *Server) serveUntilShutdown() {
listenAddr := fmt.Sprintf(":%d", s.config.Port)
s.httpServer = &http.Server{
Addr: listenAddr,
// newHTTPServer builds the http.Server with the hardening timeouts and
// limits applied. It is separate from serveUntilShutdown so the
// configuration can be asserted in a test without binding a listener.
func (s *Server) newHTTPServer() *http.Server {
return &http.Server{
Addr: fmt.Sprintf(":%d", s.config.Port),
ReadTimeout: HTTPReadTimeout,
ReadHeaderTimeout: HTTPReadHeaderTimeout,
WriteTimeout: HTTPWriteTimeout,
IdleTimeout: HTTPIdleTimeout,
MaxHeaderBytes: HTTPMaxHeaderBytes,
Handler: s,
}
}
func (s *Server) serveUntilShutdown() {
s.httpServer = s.newHTTPServer()
s.SetupRoutes()
s.log.Info("http begin listen", "listenaddr", listenAddr)
s.log.Info("http begin listen", "listenaddr", s.httpServer.Addr)
err := s.httpServer.ListenAndServe()
if err != nil && !errors.Is(err, http.ErrServerClosed) {
+65
View File
@@ -0,0 +1,65 @@
package server
import (
"testing"
"time"
"sneak.berlin/go/pixa/internal/config"
)
// TestNewHTTPServerTimeouts verifies that the constructed http.Server
// carries every hardening timeout wired onto it, including the slowloris
// defense (ReadHeaderTimeout) and the keep-alive bound (IdleTimeout). This
// guards against a field being defined but never set on the server, so
// each assertion compares the server field to its constant.
func TestNewHTTPServerTimeouts(t *testing.T) {
t.Parallel()
s := &Server{config: &config.Config{Port: 8080}}
srv := s.newHTTPServer()
fields := []struct {
name string
got time.Duration
want time.Duration
}{
{"ReadTimeout", srv.ReadTimeout, HTTPReadTimeout},
{"ReadHeaderTimeout", srv.ReadHeaderTimeout, HTTPReadHeaderTimeout},
{"WriteTimeout", srv.WriteTimeout, HTTPWriteTimeout},
{"IdleTimeout", srv.IdleTimeout, HTTPIdleTimeout},
}
for _, f := range fields {
if f.got != f.want {
t.Errorf("%s = %v, want %v", f.name, f.got, f.want)
}
}
if srv.MaxHeaderBytes != HTTPMaxHeaderBytes {
t.Errorf("MaxHeaderBytes = %d, want %d",
srv.MaxHeaderBytes, HTTPMaxHeaderBytes)
}
if srv.Handler != s {
t.Error("Handler is not the server")
}
}
// TestHardeningTimeoutValues pins the intent behind the two new timeouts
// without hard-coding brittle exact durations: the header-read phase is
// bounded strictly shorter than the whole-request read (the slowloris
// dribble), and idle keep-alive connections are bounded rather than held
// open forever.
func TestHardeningTimeoutValues(t *testing.T) {
t.Parallel()
if HTTPReadHeaderTimeout <= 0 || HTTPReadHeaderTimeout > HTTPReadTimeout {
t.Errorf("ReadHeaderTimeout = %v, want positive and <= ReadTimeout %v",
HTTPReadHeaderTimeout, HTTPReadTimeout)
}
if HTTPIdleTimeout <= 0 {
t.Errorf("IdleTimeout = %v, want positive bound", HTTPIdleTimeout)
}
}
+5 -1
View File
@@ -8,6 +8,7 @@ import (
"github.com/go-chi/chi/v5/middleware"
"github.com/prometheus/client_golang/prometheus/promhttp"
"sneak.berlin/go/pixa/internal/handlers"
"sneak.berlin/go/pixa/internal/static"
)
@@ -17,6 +18,7 @@ func (s *Server) SetupRoutes() {
s.router.Use(middleware.Recoverer)
s.router.Use(middleware.RequestID)
s.router.Use(s.mw.ClientIP())
s.router.Use(s.mw.SecurityHeaders())
s.router.Use(s.mw.Logging())
@@ -46,8 +48,10 @@ func (s *Server) SetupRoutes() {
// Login/generator UI. The form routes carry CSRF protection; the
// token cookie is independent of the session cookie, so it also
// covers the login POST, where no session exists yet.
// covers the login POST, where no session exists yet. LimitBody caps
// the POST body ahead of CSRF, which reads its token from that body.
s.router.Group(func(r chi.Router) {
r.Use(s.h.LimitBody(handlers.MaxFormBytes))
r.Use(s.h.CSRF())
r.Get("/", s.h.HandleRoot())
r.Post("/", s.h.HandleRoot())
+14 -46
View File
@@ -1,55 +1,23 @@
#!/bin/sh
# script/lint: run golangci-lint over the whole tree.
#
# The linter is never installed on the host: it runs only inside the
# Dockerfile.lint build, one way, everywhere. A clean build is a clean
# lint. See Dockerfile.lint for why the lint step cannot be cached.
# script/lint: run the linter. CGO dependencies (pkg-config, vips,
# libheif) come from nix-shell when not already available (e.g. inside
# a Docker build or an existing nix-shell).
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() {
cd "$ROOT"
# A value no other run repeats. Dockerfile.lint folds it into the
# lint step's cache key, so the linter re-executes every run instead
# of an unchanged tree returning a cached success having linted
# nothing.
cachebust="$(date +%s)-$$"
tmp="$(mktemp -d "${TMPDIR:-/tmp}/pixa-lint.XXXXXX")"
trap 'rm -rf "$tmp"' EXIT INT TERM
# --progress=plain so the lint step's own output reaches the log we
# check below; --output=type=cacheonly because we want the linter's
# verdict, not an image left in the local store. The build status
# travels through a file: a pipeline's exit status is tee's, not the
# build's.
(
set +e
docker build \
--progress=plain \
--build-arg CACHEBUST="$cachebust" \
--output=type=cacheonly \
-f Dockerfile.lint . 2>&1
echo "$?" >"$tmp/status"
) | tee "$tmp/build.log"
status="$(cat "$tmp/status" 2>/dev/null || echo 1)"
[ "${status:-1}" -eq 0 ] || exit "${status:-1}"
# The linter's start line must appear as build output, not only in
# the build's echo of the RUN instruction. A step served from cache
# prints the instruction and none of its output; a step that runs
# prints a "#<n> <elapsed> ..." output line. Requiring that output
# line means a future edit dropping the CACHEBUST reference from
# Dockerfile.lint fails here rather than passing having linted
# nothing.
if ! grep -Eq '^#[0-9]+ +[0-9]+\.[0-9]+ +pixa-lint: running golangci-lint' \
"$tmp/build.log"; then
echo "script/lint: golangci-lint did not execute (cached step?)." >&2
exit 1
run_with_cgo_deps() {
if command -v pkg-config >/dev/null 2>&1; then
sh -c "$1"
else
nix-shell -p pkg-config vips libheif golangci-lint git --run "$1"
fi
}
main() {
cd "$ROOT"
echo "Running linter..."
run_with_cgo_deps "golangci-lint run"
}
main "$@"