7 Commits
Author SHA1 Message Date
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
clawbot 4f95cb6a37 docs: update TODO.md Workflow and Status for the next branching model (closes #106)
check / check (push) Failing after 0s
The Workflow section of TODO.md still told contributors to branch from main and merge there. It now describes the current model: one branch per issue cut from next, a PR based on next, an independent reviewer, a squash-merge into next by the manager, and only the owner merging next into main through the milestone PR. The Status paragraph no longer claims work is green on main.

Disclosure: only the wrong lines are touched; reflowing the whole file is left to #100.

Model: opus-4-8 (implementation, review); fable-5-1 (landing message)
2026-09-21 19:59:51 +02:00
clawbot 6f416eac31 test: cover redirect SSRF and semaphore release in httpfetcher (closes #78)
check / check (push) Failing after 0s
internal/httpfetcher had only helper-level tests. This adds tests of the full Fetch path, with no non-test code changed: a redirect to a private address is refused and never dialed while public redirects and a two-hop chain still work; the per-host semaphore is released on error, after a full read and after a partial read; an oversized body yields ErrResponseTooLarge; non-2xx and disallowed content types are rejected; the dialer blocks private, link-local and loopback targets.

What a reader would trip over: the upstream host in the tests is the TEST-NET-1 literal 192.0.2.10, which the private-IP check treats as public; a recording dialer routes it to the local test server and records every dial.

Disclosure: DNS rebinding is not simulated end to end (it would mean changing the global resolver under parallel race tests); the dial-time re-resolution is tested directly instead.

Model: opus-4-8 (implementation, review); fable-5-1 (landing message)
2026-09-21 19:59:26 +02:00
clawbot b95ef1eb69 fix: script/test conditional-verbose-rerun with -cover (closes #59)
check / check (push) Failing after 2s
script/test now runs the suite quietly first (with -race and -cover, 30s timeout) and re-runs it with -v only when that run fails, then exits non-zero. This is the pattern REPO_POLICIES.md mandates; before, every green run printed full per-test output.

What a reader would trip over: the whole compound command is passed as one string to run_with_cgo_deps, so it behaves the same on the host path and under the nix-shell fallback. -cover is on the first run only; the verbose rerun exists for diagnostics.

Disclosure: no test was written for the wrapper script itself; the failure path was exercised by hand by author and reviewer.
Disclosure: the nix-shell fallback is kept; moving tests into Docker belongs to #101 and #104.

Model: opus-4-8 (implementation, review); fable-5-1 (landing message)
2026-09-21 19:43:21 +02:00
20 changed files with 1318 additions and 41 deletions
+3 -2
View File
@@ -67,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
+18 -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,10 @@ 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
- `upstream_fetch_timeout` — timeout for origin requests
- `upstream_max_response_size` — max origin response size
- `downstream_timeout` — client response timeout
+32 -10
View File
@@ -1,28 +1,52 @@
# Workflow
* branch (from `main`)
* branch per issue from `next`
* do the work in Next Step
* move Next Step to the top of Completed Steps
* move the top item of Future Steps into Next Step
* commit (`TODO.md` changes in the same commit as the work)
* merge to `main` if the branch is not protected, otherwise open a PR
* open a PR based on `next`
* an independent reviewer who did not write the change gates it
* the manager squash-merges the PR into `next` once review passes
* `next` stays green and mergeable to `main` at any time; only the owner
merges `next` into `main`, via the single milestone PR
* push
# Status
pre-1.0. No git tags exist. Recent work extracted the internal/magic,
pre-1.0. No git tags exist. The `1.0.0` milestone is in progress; work
lands on `next`, and `main` receives only the milestone PR that the
owner merges. `next` is at the canonical `golangci-lint` v2.12.2 config
and is green. Recent work extracted the internal/magic,
internal/allowlist, internal/httpfetcher, and internal/signature
packages. The gosec findings from the 2026-07-06 survey are resolved
and `make check` is green on main. The disk cache is now size-bounded
with LRU eviction (`cache_max_bytes`), closing the unbounded disk
growth DoS vector.
packages. The gosec findings from the 2026-07-06 survey are resolved.
The disk cache is now size-bounded with LRU eviction
(`cache_max_bytes`), closing the unbounded disk growth DoS vector.
# 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 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
@@ -116,8 +140,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
+9
View File
@@ -22,6 +22,15 @@ 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
# Allow HTTP upstream (only for testing, always use HTTPS in production)
allow_http: false
@@ -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},
},
})
}
+129 -7
View File
@@ -6,6 +6,7 @@ import (
"fmt"
"log/slog"
"math"
"net/netip"
"net/url"
"os"
"path/filepath"
@@ -42,8 +43,15 @@ const (
keyAllowHTTP = "allow_http"
keyUpstreamConnectionsPerHost = "upstream_connections_per_host"
keyCacheMaxBytes = "cache_max_bytes"
keyBlockedNetworks = "blocked_networks"
)
// 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 +62,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 +70,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 +112,11 @@ 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
// 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 +185,11 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
}
}
blockedNetworks, err := getBlockedNetworks(sc)
if err != nil {
return nil, err
}
loader := &strictLoader{sc: sc}
c := &Config{
@@ -183,7 +205,8 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
AllowHTTP: loader.boolVal(keyAllowHTTP, false),
UpstreamConnectionsPerHost: loader.intVal(
keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost),
CacheMaxBytes: loader.int64Val(keyCacheMaxBytes, 0),
CacheMaxBytes: loader.int64Val(keyCacheMaxBytes, 0),
BlockedNetworks: blockedNetworks,
}
// The computed default for cache_max_bytes needs a validated
@@ -215,7 +238,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 +322,7 @@ func isKnownConfigKey(key string) bool {
switch key {
case keyDebug, keyMaintenanceMode, keyPort, keyStateDir, keySentryDSN,
keyDBURL, keyMetrics, keySigningKey, keyAllowlistHosts, keyAllowHTTP,
keyUpstreamConnectionsPerHost, keyCacheMaxBytes, "env":
keyUpstreamConnectionsPerHost, keyCacheMaxBytes, keyBlockedNetworks, "env":
return true
}
@@ -341,10 +364,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 +379,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 +816,87 @@ func getStringSlice(sc *smartconfig.Config) []string {
return nil
}
// getBlockedNetworks parses the blocked_networks value 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; a default (the built-in
// blocklist alone) applies only to an omitted key.
func getBlockedNetworks(sc *smartconfig.Config) ([]netip.Prefix, error) {
if sc == nil {
return nil, nil
}
raw, ok := sc.Get(keyBlockedNetworks)
if !ok {
return nil, nil
}
if raw == nil {
return nil, errNullConfigValue(keyBlockedNetworks)
}
entries, err := blockedNetworkEntries(raw)
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",
keyBlockedNetworks, entry, errNotAValidCIDR)
}
prefixes = append(prefixes, prefix)
}
return prefixes, nil
}
// blockedNetworkEntries extracts the raw blocked_networks entries 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 blockedNetworkEntries(raw any) ([]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",
keyBlockedNetworks, item, item, errNotAString)
}
if strings.TrimSpace(str) == "" {
return nil, fmt.Errorf("config key %q: %w",
keyBlockedNetworks, 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",
keyBlockedNetworks, val, errEmptyEntry)
}
entries = append(entries, trimmed)
}
return entries, nil
default:
return nil, fmt.Errorf("config key %q: value %v (%T) is %w",
keyBlockedNetworks, 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",
+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)
}
})
}
+421
View File
@@ -0,0 +1,421 @@
package httpfetcher
import (
"context"
"errors"
"fmt"
"io"
"net"
"net/http"
"net/http/httptest"
"slices"
"strings"
"sync"
"testing"
"time"
)
// testPublicHost is a TEST-NET-1 (RFC 5737) literal. isPrivateIP treats it as
// public, so validateURL and the redirect check accept it with no DNS lookup,
// while the recording dialer routes it to the local httptest server. The
// address is reserved for documentation and is never routed on the network.
const testPublicHost = "192.0.2.10"
// imagePayload is the body served by the fake upstream's image route.
const imagePayload = "fake-jpeg-bytes"
// errUnexpectedDial reports a dial to any host other than testPublicHost, which
// would mean SSRF protection let a forbidden target reach the transport.
var errUnexpectedDial = errors.New("unexpected dial target")
// upstreamURL builds a fetch URL on the fake public host for the given path.
func upstreamURL(path string) string {
return "http://" + testPublicHost + path
}
// recordingDialer records every address the transport asks it to dial and
// routes connections for testPublicHost to a real local server, so the SSRF
// checks run against a public-looking host while bytes go to httptest.
type recordingDialer struct {
target string
mu sync.Mutex
dialed []string
}
func (d *recordingDialer) dialContext(
ctx context.Context,
network, addr string,
) (net.Conn, error) {
d.mu.Lock()
d.dialed = append(d.dialed, addr)
d.mu.Unlock()
host, _, err := net.SplitHostPort(addr)
if err != nil {
return nil, err
}
if host != testPublicHost {
return nil, fmt.Errorf("%w: %s", errUnexpectedDial, addr)
}
var dialer net.Dialer
return dialer.DialContext(ctx, network, d.target)
}
// dialedAddrs returns a copy of the addresses the dialer was asked to reach.
func (d *recordingDialer) dialedAddrs() []string {
d.mu.Lock()
defer d.mu.Unlock()
return slices.Clone(d.dialed)
}
// startUpstream launches a fake upstream with the routes the fetch tests
// exercise and stops it when the test finishes.
func startUpstream(t *testing.T) *httptest.Server {
t.Helper()
mux := http.NewServeMux()
mux.HandleFunc("/image", func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Content-Type", contentTypeJPEG)
_, _ = io.WriteString(w, imagePayload)
})
mux.HandleFunc("/status/500", func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusInternalServerError)
})
mux.HandleFunc("/html", func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Content-Type", "text/html; charset=utf-8")
_, _ = io.WriteString(w, "<html></html>")
})
mux.HandleFunc("/redirect/private", func(w http.ResponseWriter, r *http.Request) {
http.Redirect(w, r, "http://169.254.169.254/latest/meta-data/", http.StatusFound)
})
mux.HandleFunc("/redirect/public", func(w http.ResponseWriter, r *http.Request) {
http.Redirect(w, r, "/image", http.StatusFound)
})
mux.HandleFunc("/redirect/chain", func(w http.ResponseWriter, r *http.Request) {
http.Redirect(w, r, "/redirect/hop", http.StatusFound)
})
mux.HandleFunc("/redirect/hop", func(w http.ResponseWriter, r *http.Request) {
http.Redirect(w, r, "/image", http.StatusFound)
})
srv := httptest.NewServer(mux)
t.Cleanup(srv.Close)
return srv
}
// newServerFetcher builds a fetcher whose transport routes testPublicHost to
// srv, leaving the real SSRF validation and redirect checks in place.
func newServerFetcher(
t *testing.T,
srv *httptest.Server,
cfg *Config,
) (*HTTPFetcher, *recordingDialer) {
t.Helper()
if cfg == nil {
cfg = DefaultConfig()
}
cfg.AllowHTTP = true
f := New(cfg)
transport, ok := f.client.Transport.(*http.Transport)
if !ok {
t.Fatalf("transport is %T, want *http.Transport", f.client.Transport)
}
dialer := &recordingDialer{target: srv.Listener.Addr().String()}
transport.DialContext = dialer.dialContext
return f, dialer
}
// testContext returns a context cancelled when the test ends, bounding any
// fetch that would otherwise block on a leaked semaphore slot.
func testContext(t *testing.T) context.Context {
t.Helper()
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
t.Cleanup(cancel)
return ctx
}
// fetchImage fetches path from the fake upstream and fails on error.
func fetchImage(t *testing.T, f *HTTPFetcher, path string) *FetchResult {
t.Helper()
res, err := f.Fetch(testContext(t), upstreamURL(path))
if err != nil {
t.Fatalf("Fetch(%s) error = %v", path, err)
}
return res
}
// fetchExpectError fetches path and fails unless Fetch returns an error.
func fetchExpectError(t *testing.T, f *HTTPFetcher, path string) error {
t.Helper()
res, err := f.Fetch(testContext(t), upstreamURL(path))
if err == nil {
_ = res.Content.Close()
t.Fatalf("Fetch(%s) = nil error, want an error", path)
}
return err
}
// fetchBody fetches path and returns the fully read, closed response body.
func fetchBody(t *testing.T, f *HTTPFetcher, path string) string {
t.Helper()
res := fetchImage(t, f, path)
defer func() { _ = res.Content.Close() }()
data, err := io.ReadAll(res.Content)
if err != nil {
t.Fatalf("read body: %v", err)
}
return string(data)
}
// semLen reports how many per-host semaphore slots are currently held.
func semLen(f *HTTPFetcher, host string) int {
return len(f.getHostSemaphore(host))
}
func TestFetchRedirectToPrivateIPBlocked(t *testing.T) {
t.Parallel()
srv := startUpstream(t)
f, dialer := newServerFetcher(t, srv, nil)
_, err := f.Fetch(testContext(t), upstreamURL("/redirect/private"))
if !errors.Is(err, ErrSSRFBlocked) {
t.Fatalf("Fetch() error = %v, want ErrSSRFBlocked", err)
}
for _, addr := range dialer.dialedAddrs() {
if strings.Contains(addr, "169.254.169.254") {
t.Errorf("dialer connected to the private redirect target: %s", addr)
}
}
}
func TestFetchRedirectToPublicSucceeds(t *testing.T) {
t.Parallel()
srv := startUpstream(t)
f, _ := newServerFetcher(t, srv, nil)
if body := fetchBody(t, f, "/redirect/public"); body != imagePayload {
t.Errorf("body = %q, want %q", body, imagePayload)
}
}
func TestFetchRedirectChainSucceeds(t *testing.T) {
t.Parallel()
srv := startUpstream(t)
f, _ := newServerFetcher(t, srv, nil)
if body := fetchBody(t, f, "/redirect/chain"); body != imagePayload {
t.Errorf("body = %q, want %q", body, imagePayload)
}
}
func TestFetchRejectsNon2xx(t *testing.T) {
t.Parallel()
srv := startUpstream(t)
f, _ := newServerFetcher(t, srv, nil)
err := fetchExpectError(t, f, "/status/500")
if !errors.Is(err, ErrUpstreamError) {
t.Fatalf("Fetch() error = %v, want ErrUpstreamError", err)
}
}
func TestFetchRejectsDisallowedContentType(t *testing.T) {
t.Parallel()
srv := startUpstream(t)
f, _ := newServerFetcher(t, srv, nil)
err := fetchExpectError(t, f, "/html")
if !errors.Is(err, ErrInvalidContentType) {
t.Fatalf("Fetch() error = %v, want ErrInvalidContentType", err)
}
}
func TestFetchMaxResponseSizeEnforced(t *testing.T) {
t.Parallel()
srv := startUpstream(t)
cfg := DefaultConfig()
cfg.MaxResponseSize = 8
f, _ := newServerFetcher(t, srv, cfg)
res := fetchImage(t, f, "/image")
defer func() { _ = res.Content.Close() }()
data, err := io.ReadAll(res.Content)
if !errors.Is(err, ErrResponseTooLarge) {
t.Fatalf("read error = %v, want ErrResponseTooLarge", err)
}
if int64(len(data)) > cfg.MaxResponseSize {
t.Errorf("read %d bytes, exceeds limit %d", len(data), cfg.MaxResponseSize)
}
}
func TestFetchSemaphoreReleasedOnError(t *testing.T) {
t.Parallel()
srv := startUpstream(t)
cfg := DefaultConfig()
cfg.MaxConnectionsPerHost = 1
f, _ := newServerFetcher(t, srv, cfg)
err := fetchExpectError(t, f, "/status/500")
if !errors.Is(err, ErrUpstreamError) {
t.Fatalf("Fetch() error = %v, want ErrUpstreamError", err)
}
if held := semLen(f, testPublicHost); held != 0 {
t.Fatalf("semaphore slot leaked after error: %d held", held)
}
// One slot per host: this fetch proceeds only if the slot was released.
res := fetchImage(t, f, "/image")
_ = res.Content.Close()
}
// assertSlotReleasedByClose fetches an image over a one-slot host, hands the
// open result to consume, and asserts the slot is held before and freed after,
// then that a follow-up fetch can still acquire it.
func assertSlotReleasedByClose(
t *testing.T,
consume func(*testing.T, *FetchResult),
) {
t.Helper()
srv := startUpstream(t)
cfg := DefaultConfig()
cfg.MaxConnectionsPerHost = 1
f, _ := newServerFetcher(t, srv, cfg)
res := fetchImage(t, f, "/image")
if held := semLen(f, testPublicHost); held != 1 {
t.Fatalf("slot not held while body is open: %d held", held)
}
consume(t, res)
if held := semLen(f, testPublicHost); held != 0 {
t.Fatalf("slot not released after close: %d held", held)
}
next := fetchImage(t, f, "/image")
_ = next.Content.Close()
}
func TestFetchSemaphoreReleasedOnBodyClose(t *testing.T) {
t.Parallel()
assertSlotReleasedByClose(t, func(t *testing.T, res *FetchResult) {
t.Helper()
_, err := io.ReadAll(res.Content)
if err != nil {
t.Fatalf("read body: %v", err)
}
err = res.Content.Close()
if err != nil {
t.Fatalf("close body: %v", err)
}
})
}
func TestFetchSemaphoreReleasedOnPartialReadClose(t *testing.T) {
t.Parallel()
assertSlotReleasedByClose(t, func(t *testing.T, res *FetchResult) {
t.Helper()
buf := make([]byte, 1)
_, err := res.Content.Read(buf)
if err != nil {
t.Fatalf("partial read: %v", err)
}
err = res.Content.Close()
if err != nil {
t.Fatalf("close body: %v", err)
}
})
}
// The dial-time re-resolution in ssrfSafeDialer is what closes the DNS
// rebinding window: even if validateURL saw a public answer earlier, the
// dialer independently re-checks the address it is about to connect to. A full
// rebinding simulation (a resolver returning public, then private) would mean
// replacing the global net.DefaultResolver with a fake DNS server, which is
// heavyweight and unsafe to mutate under parallel -race tests. The property is
// proven directly here instead: the dialer rejects a private target outright,
// which is exactly the check that fires when a validated host later resolves
// to a private address.
func TestSSRFSafeDialerBlocksPrivateTarget(t *testing.T) {
t.Parallel()
for _, addr := range []string{
"169.254.169.254:80", // link-local (cloud metadata)
"127.0.0.1:80", // loopback
"10.0.0.5:80", // RFC 1918 private
} {
t.Run(addr, func(t *testing.T) {
t.Parallel()
_, err := ssrfSafeDialer(context.Background(), "tcp", addr)
if !errors.Is(err, ErrSSRFBlocked) {
t.Errorf("ssrfSafeDialer(%q) = %v, want ErrSSRFBlocked", addr, err)
}
})
}
}
func TestSSRFSafeDialerAllowsPublicTarget(t *testing.T) {
t.Parallel()
// A cancelled context makes the dial fail immediately without touching the
// network; the point is only that a public literal is not SSRF-blocked.
ctx, cancel := context.WithCancel(context.Background())
cancel()
_, err := ssrfSafeDialer(ctx, "tcp", testPublicHost+":80")
if err == nil {
t.Fatal("expected a dial error for an unreachable public target")
}
if errors.Is(err, ErrSSRFBlocked) {
t.Errorf("public target was SSRF-blocked: %v", err)
}
}
+72 -6
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,8 +532,10 @@ func ssrfSafeDialer(ctx context.Context, network, addr string) (net.Conn, error)
}
// Check all resolved IPs
if slices.ContainsFunc(ips, isPrivateIP) {
return nil, ErrSSRFBlocked
for _, ip := range ips {
if isBlockedIP(ip, blocked) {
return nil, ErrSSRFBlocked
}
}
// Connect using the first valid IP
+37
View File
@@ -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
@@ -164,6 +191,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()
+27 -11
View File
@@ -9,24 +9,40 @@ import (
// HTTP server configuration constants.
const (
HTTPReadTimeout = 30 * time.Second
HTTPWriteTimeout = 60 * time.Second
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,
ReadTimeout: HTTPReadTimeout,
WriteTimeout: HTTPWriteTimeout,
MaxHeaderBytes: HTTPMaxHeaderBytes,
Handler: s,
// 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)
}
}
+4 -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"
)
@@ -46,8 +47,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())
+5 -1
View File
@@ -17,7 +17,11 @@ run_with_cgo_deps() {
main() {
cd "$ROOT"
echo "Running tests..."
run_with_cgo_deps "CGO_ENABLED=1 go test -timeout 30s -race -v ./..."
# Run without -v first for clean output on success; on failure rerun
# with -v for full diagnostics, then exit non-zero (REPO_POLICIES.md
# conditional-verbose-rerun pattern). The first run already proved the
# tests broken, so the build fails even if the rerun happens to pass.
run_with_cgo_deps "CGO_ENABLED=1 go test -timeout 30s -race -cover ./... || { echo '--- Rerunning with -v for details ---'; CGO_ENABLED=1 go test -timeout 30s -race -v ./...; exit 1; }"
}
main "$@"