Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
6a753f14c4 | ||
|
|
5f88dc5cfa |
@@ -111,10 +111,6 @@ Configured via YAML file (`--config`). Key settings:
|
|||||||
|
|
||||||
- `access_control_allow_origin` — CORS origin
|
- `access_control_allow_origin` — CORS origin
|
||||||
- `allowlist_hosts` — list of allowed upstream hosts
|
- `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_fetch_timeout` — timeout for origin requests
|
||||||
- `upstream_max_response_size` — max origin response size
|
- `upstream_max_response_size` — max origin response size
|
||||||
- `downstream_timeout` — client response timeout
|
- `downstream_timeout` — client response timeout
|
||||||
|
|||||||
@@ -1,52 +1,28 @@
|
|||||||
# Workflow
|
# Workflow
|
||||||
|
|
||||||
* branch per issue from `next`
|
* branch (from `main`)
|
||||||
* do the work in Next Step
|
* do the work in Next Step
|
||||||
* move Next Step to the top of Completed Steps
|
* move Next Step to the top of Completed Steps
|
||||||
* move the top item of Future Steps into Next Step
|
* move the top item of Future Steps into Next Step
|
||||||
* commit (`TODO.md` changes in the same commit as the work)
|
* commit (`TODO.md` changes in the same commit as the work)
|
||||||
* open a PR based on `next`
|
* merge to `main` if the branch is not protected, otherwise open a PR
|
||||||
* 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
|
* push
|
||||||
|
|
||||||
# Status
|
# Status
|
||||||
|
|
||||||
pre-1.0. No git tags exist. The `1.0.0` milestone is in progress; work
|
pre-1.0. No git tags exist. Recent work extracted the internal/magic,
|
||||||
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
|
internal/allowlist, internal/httpfetcher, and internal/signature
|
||||||
packages. The gosec findings from the 2026-07-06 survey are resolved.
|
packages. The gosec findings from the 2026-07-06 survey are resolved
|
||||||
The disk cache is now size-bounded with LRU eviction
|
and `make check` is green on main. The disk cache is now size-bounded
|
||||||
(`cache_max_bytes`), closing the unbounded disk growth DoS vector.
|
with LRU eviction (`cache_max_bytes`), closing the unbounded disk
|
||||||
|
growth DoS vector.
|
||||||
|
|
||||||
# Next Step
|
# Next Step
|
||||||
|
|
||||||
P1: rate limit global concurrent upstream fetches to prevent resource
|
P1: implement blocked networks configuration to extend SSRF protection
|
||||||
exhaustion
|
|
||||||
|
|
||||||
# Completed Steps
|
# 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
|
- 2026-08-07 update golangci-lint to v2.12.2 with the canonical
|
||||||
`.golangci.yml` (v2 schema, `default: all` minus six disabled
|
`.golangci.yml` (v2 schema, `default: all` minus six disabled
|
||||||
linters, `lll` 88, tests included): bumped the pinned
|
linters, `lll` 88, tests included): bumped the pinned
|
||||||
@@ -140,6 +116,8 @@ exhaustion
|
|||||||
|
|
||||||
# Future Steps
|
# Future Steps
|
||||||
|
|
||||||
|
- P1: rate limit global concurrent upstream fetches to prevent
|
||||||
|
resource exhaustion
|
||||||
- P1: strip EXIF and other metadata from processed images (privacy)
|
- P1: strip EXIF and other metadata from processed images (privacy)
|
||||||
- P2: security
|
- P2: security
|
||||||
- referer blacklist
|
- referer blacklist
|
||||||
|
|||||||
@@ -22,15 +22,6 @@ allowlist_hosts:
|
|||||||
- github.com
|
- github.com
|
||||||
- user-images.githubusercontent.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 upstream (only for testing, always use HTTPS in production)
|
||||||
allow_http: false
|
allow_http: false
|
||||||
|
|
||||||
|
|||||||
@@ -1,90 +0,0 @@
|
|||||||
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},
|
|
||||||
},
|
|
||||||
})
|
|
||||||
}
|
|
||||||
+3
-101
@@ -6,7 +6,6 @@ import (
|
|||||||
"fmt"
|
"fmt"
|
||||||
"log/slog"
|
"log/slog"
|
||||||
"math"
|
"math"
|
||||||
"net/netip"
|
|
||||||
"net/url"
|
"net/url"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
@@ -43,7 +42,6 @@ const (
|
|||||||
keyAllowHTTP = "allow_http"
|
keyAllowHTTP = "allow_http"
|
||||||
keyUpstreamConnectionsPerHost = "upstream_connections_per_host"
|
keyUpstreamConnectionsPerHost = "upstream_connections_per_host"
|
||||||
keyCacheMaxBytes = "cache_max_bytes"
|
keyCacheMaxBytes = "cache_max_bytes"
|
||||||
keyBlockedNetworks = "blocked_networks"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
// Static validation errors. Each use site attaches the offending key
|
// Static validation errors. Each use site attaches the offending key
|
||||||
@@ -56,7 +54,6 @@ var (
|
|||||||
errNotAnInteger = errors.New("not an integer")
|
errNotAnInteger = errors.New("not an integer")
|
||||||
errNotABoolean = errors.New("not a boolean")
|
errNotABoolean = errors.New("not a boolean")
|
||||||
errNotAStringList = errors.New("not a list of strings")
|
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")
|
errNotAMetricsMap = errors.New("not a map of metrics settings")
|
||||||
errEmptyListEntry = errors.New("list contains an empty entry")
|
errEmptyListEntry = errors.New("list contains an empty entry")
|
||||||
errEmptyEntry = errors.New("contains an empty entry")
|
errEmptyEntry = errors.New("contains an empty entry")
|
||||||
@@ -103,11 +100,6 @@ type Config struct {
|
|||||||
AllowHTTP bool // Allow non-TLS upstream (testing only)
|
AllowHTTP bool // Allow non-TLS upstream (testing only)
|
||||||
UpstreamConnectionsPerHost int // Max concurrent connections per upstream host
|
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
|
// CacheMaxBytes is the disk cache size limit in bytes. Zero
|
||||||
// disables the disk cache entirely. When cache_max_bytes is
|
// disables the disk cache entirely. When cache_max_bytes is
|
||||||
// omitted from the configuration, this holds the computed default
|
// omitted from the configuration, this holds the computed default
|
||||||
@@ -176,11 +168,6 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
blockedNetworks, err := getBlockedNetworks(sc)
|
|
||||||
if err != nil {
|
|
||||||
return nil, err
|
|
||||||
}
|
|
||||||
|
|
||||||
loader := &strictLoader{sc: sc}
|
loader := &strictLoader{sc: sc}
|
||||||
|
|
||||||
c := &Config{
|
c := &Config{
|
||||||
@@ -196,8 +183,7 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
|
|||||||
AllowHTTP: loader.boolVal(keyAllowHTTP, false),
|
AllowHTTP: loader.boolVal(keyAllowHTTP, false),
|
||||||
UpstreamConnectionsPerHost: loader.intVal(
|
UpstreamConnectionsPerHost: loader.intVal(
|
||||||
keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost),
|
keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost),
|
||||||
CacheMaxBytes: loader.int64Val(keyCacheMaxBytes, 0),
|
CacheMaxBytes: loader.int64Val(keyCacheMaxBytes, 0),
|
||||||
BlockedNetworks: blockedNetworks,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// The computed default for cache_max_bytes needs a validated
|
// The computed default for cache_max_bytes needs a validated
|
||||||
@@ -229,7 +215,7 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
|
|||||||
return nil, loader.err
|
return nil, loader.err
|
||||||
}
|
}
|
||||||
|
|
||||||
err = c.validate()
|
err := c.validate()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
@@ -313,7 +299,7 @@ func isKnownConfigKey(key string) bool {
|
|||||||
switch key {
|
switch key {
|
||||||
case keyDebug, keyMaintenanceMode, keyPort, keyStateDir, keySentryDSN,
|
case keyDebug, keyMaintenanceMode, keyPort, keyStateDir, keySentryDSN,
|
||||||
keyDBURL, keyMetrics, keySigningKey, keyAllowlistHosts, keyAllowHTTP,
|
keyDBURL, keyMetrics, keySigningKey, keyAllowlistHosts, keyAllowHTTP,
|
||||||
keyUpstreamConnectionsPerHost, keyCacheMaxBytes, keyBlockedNetworks, "env":
|
keyUpstreamConnectionsPerHost, keyCacheMaxBytes, "env":
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -792,87 +778,3 @@ func getStringSlice(sc *smartconfig.Config) []string {
|
|||||||
|
|
||||||
return nil
|
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)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|||||||
@@ -1,45 +0,0 @@
|
|||||||
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)
|
|
||||||
})
|
|
||||||
}
|
|
||||||
}
|
|
||||||
@@ -1,177 +0,0 @@
|
|||||||
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")
|
|
||||||
}
|
|
||||||
}
|
|
||||||
@@ -111,8 +111,6 @@ func (s *Handlers) initImageService() error {
|
|||||||
fetcherCfg.MaxConnectionsPerHost = s.config.UpstreamConnectionsPerHost
|
fetcherCfg.MaxConnectionsPerHost = s.config.UpstreamConnectionsPerHost
|
||||||
}
|
}
|
||||||
|
|
||||||
fetcherCfg.BlockedNetworks = s.config.BlockedNetworks
|
|
||||||
|
|
||||||
// Create the service
|
// Create the service
|
||||||
svc, err := imgcache.NewService(&imgcache.ServiceConfig{
|
svc, err := imgcache.NewService(&imgcache.ServiceConfig{
|
||||||
Cache: cache,
|
Cache: cache,
|
||||||
|
|||||||
@@ -1,111 +0,0 @@
|
|||||||
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)
|
|
||||||
}
|
|
||||||
})
|
|
||||||
}
|
|
||||||
@@ -1,421 +0,0 @@
|
|||||||
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)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
@@ -11,7 +11,6 @@ import (
|
|||||||
"net"
|
"net"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/http/httptrace"
|
"net/http/httptrace"
|
||||||
"net/netip"
|
|
||||||
neturl "net/url"
|
neturl "net/url"
|
||||||
"slices"
|
"slices"
|
||||||
"strings"
|
"strings"
|
||||||
@@ -47,20 +46,6 @@ const (
|
|||||||
localhostIPv6 = "::1"
|
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.
|
// Fetcher errors.
|
||||||
var (
|
var (
|
||||||
ErrSSRFBlocked = errors.New("request blocked: private or internal IP")
|
ErrSSRFBlocked = errors.New("request blocked: private or internal IP")
|
||||||
@@ -122,9 +107,6 @@ type Config struct {
|
|||||||
AllowHTTP bool
|
AllowHTTP bool
|
||||||
// MaxConnectionsPerHost limits concurrent connections to each upstream host.
|
// MaxConnectionsPerHost limits concurrent connections to each upstream host.
|
||||||
MaxConnectionsPerHost int
|
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.
|
// DefaultConfig returns a Config with sensible defaults.
|
||||||
@@ -160,13 +142,9 @@ func New(config *Config) *HTTPFetcher {
|
|||||||
config = DefaultConfig()
|
config = DefaultConfig()
|
||||||
}
|
}
|
||||||
|
|
||||||
// Create transport with SSRF-safe dialer. The dialer re-resolves and
|
// Create transport with SSRF-safe dialer
|
||||||
// re-checks at connect time (closing the DNS-rebinding window) against
|
|
||||||
// both the built-in ranges and the operator-supplied blocklist.
|
|
||||||
transport := &http.Transport{
|
transport := &http.Transport{
|
||||||
DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
|
DialContext: ssrfSafeDialer,
|
||||||
return dialSSRFSafe(ctx, network, addr, config.BlockedNetworks)
|
|
||||||
},
|
|
||||||
TLSHandshakeTimeout: DefaultTLSTimeout,
|
TLSHandshakeTimeout: DefaultTLSTimeout,
|
||||||
MaxIdleConns: DefaultMaxIdleConns,
|
MaxIdleConns: DefaultMaxIdleConns,
|
||||||
IdleConnTimeout: DefaultIdleConnTimeout,
|
IdleConnTimeout: DefaultIdleConnTimeout,
|
||||||
@@ -473,53 +451,11 @@ func isPrivateIP(ip net.IP) bool {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Special-use ranges the net.IP predicates above do not cover.
|
return false
|
||||||
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)
|
|
||||||
})
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// isBlockedIP reports whether ip is refused, either by the built-in
|
// ssrfSafeDialer is a custom dialer that validates IP addresses before connecting.
|
||||||
// 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) {
|
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)
|
host, port, err := net.SplitHostPort(addr)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
@@ -532,10 +468,8 @@ func dialSSRFSafe(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Check all resolved IPs
|
// Check all resolved IPs
|
||||||
for _, ip := range ips {
|
if slices.ContainsFunc(ips, isPrivateIP) {
|
||||||
if isBlockedIP(ip, blocked) {
|
return nil, ErrSSRFBlocked
|
||||||
return nil, ErrSSRFBlocked
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Connect using the first valid IP
|
// Connect using the first valid IP
|
||||||
|
|||||||
@@ -21,33 +21,6 @@ import (
|
|||||||
// CORSMaxAgeSeconds is the max age for CORS preflight cache (24 hours).
|
// CORSMaxAgeSeconds is the max age for CORS preflight cache (24 hours).
|
||||||
const CORSMaxAgeSeconds = 86400
|
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.
|
// Params defines dependencies for Middleware.
|
||||||
type Params struct {
|
type Params struct {
|
||||||
fx.In
|
fx.In
|
||||||
@@ -191,16 +164,6 @@ func (s *Middleware) SecurityHeaders() func(http.Handler) http.Handler {
|
|||||||
// Disable XSS filtering (modern browsers don't need it, can cause issues)
|
// Disable XSS filtering (modern browsers don't need it, can cause issues)
|
||||||
w.Header().Set("X-XSS-Protection", "0")
|
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)
|
next.ServeHTTP(w, r)
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -56,61 +56,6 @@ 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) {
|
func TestSecurityHeaders_PreservesExistingHeaders(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
+11
-27
@@ -9,40 +9,24 @@ import (
|
|||||||
|
|
||||||
// HTTP server configuration constants.
|
// HTTP server configuration constants.
|
||||||
const (
|
const (
|
||||||
HTTPReadTimeout = 30 * time.Second
|
HTTPReadTimeout = 30 * time.Second
|
||||||
// HTTPReadHeaderTimeout bounds the request-header read on its own,
|
HTTPWriteTimeout = 60 * time.Second
|
||||||
// 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
|
HTTPMaxHeaderBytes = 8 << 10 // 8KB
|
||||||
)
|
)
|
||||||
|
|
||||||
// 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() {
|
func (s *Server) serveUntilShutdown() {
|
||||||
s.httpServer = s.newHTTPServer()
|
listenAddr := fmt.Sprintf(":%d", s.config.Port)
|
||||||
|
s.httpServer = &http.Server{
|
||||||
|
Addr: listenAddr,
|
||||||
|
ReadTimeout: HTTPReadTimeout,
|
||||||
|
WriteTimeout: HTTPWriteTimeout,
|
||||||
|
MaxHeaderBytes: HTTPMaxHeaderBytes,
|
||||||
|
Handler: s,
|
||||||
|
}
|
||||||
|
|
||||||
s.SetupRoutes()
|
s.SetupRoutes()
|
||||||
|
|
||||||
s.log.Info("http begin listen", "listenaddr", s.httpServer.Addr)
|
s.log.Info("http begin listen", "listenaddr", listenAddr)
|
||||||
|
|
||||||
err := s.httpServer.ListenAndServe()
|
err := s.httpServer.ListenAndServe()
|
||||||
if err != nil && !errors.Is(err, http.ErrServerClosed) {
|
if err != nil && !errors.Is(err, http.ErrServerClosed) {
|
||||||
|
|||||||
@@ -1,65 +0,0 @@
|
|||||||
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)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
@@ -8,7 +8,6 @@ import (
|
|||||||
"github.com/go-chi/chi/v5/middleware"
|
"github.com/go-chi/chi/v5/middleware"
|
||||||
"github.com/prometheus/client_golang/prometheus/promhttp"
|
"github.com/prometheus/client_golang/prometheus/promhttp"
|
||||||
|
|
||||||
"sneak.berlin/go/pixa/internal/handlers"
|
|
||||||
"sneak.berlin/go/pixa/internal/static"
|
"sneak.berlin/go/pixa/internal/static"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -47,10 +46,8 @@ func (s *Server) SetupRoutes() {
|
|||||||
|
|
||||||
// Login/generator UI. The form routes carry CSRF protection; the
|
// Login/generator UI. The form routes carry CSRF protection; the
|
||||||
// token cookie is independent of the session cookie, so it also
|
// token cookie is independent of the session cookie, so it also
|
||||||
// covers the login POST, where no session exists yet. LimitBody caps
|
// covers the login POST, where no session exists yet.
|
||||||
// the POST body ahead of CSRF, which reads its token from that body.
|
|
||||||
s.router.Group(func(r chi.Router) {
|
s.router.Group(func(r chi.Router) {
|
||||||
r.Use(s.h.LimitBody(handlers.MaxFormBytes))
|
|
||||||
r.Use(s.h.CSRF())
|
r.Use(s.h.CSRF())
|
||||||
r.Get("/", s.h.HandleRoot())
|
r.Get("/", s.h.HandleRoot())
|
||||||
r.Post("/", s.h.HandleRoot())
|
r.Post("/", s.h.HandleRoot())
|
||||||
|
|||||||
+1
-5
@@ -17,11 +17,7 @@ run_with_cgo_deps() {
|
|||||||
main() {
|
main() {
|
||||||
cd "$ROOT"
|
cd "$ROOT"
|
||||||
echo "Running tests..."
|
echo "Running tests..."
|
||||||
# Run without -v first for clean output on success; on failure rerun
|
run_with_cgo_deps "CGO_ENABLED=1 go test -timeout 30s -race -v ./..."
|
||||||
# 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 "$@"
|
main "$@"
|
||||||
|
|||||||
Reference in New Issue
Block a user