From 3cfcda0730cbf8ebb69b3a32bacb5b2b2c852eaa Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 22 Sep 2026 00:43:27 +0200 Subject: [PATCH] feat: blocked_networks config and extended SSRF ranges (closes #67) 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) --- README.md | 4 + TODO.md | 14 ++- config.example.yml | 9 ++ .../config/blocked_networks_internal_test.go | 90 ++++++++++++++ internal/config/config.go | 104 +++++++++++++++- internal/handlers/handlers.go | 2 + .../blocked_networks_internal_test.go | 111 ++++++++++++++++++ internal/httpfetcher/httpfetcher.go | 78 +++++++++++- 8 files changed, 400 insertions(+), 12 deletions(-) create mode 100644 internal/config/blocked_networks_internal_test.go create mode 100644 internal/httpfetcher/blocked_networks_internal_test.go diff --git a/README.md b/README.md index 32fed7f..ac27ec4 100644 --- a/README.md +++ b/README.md @@ -122,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 diff --git a/TODO.md b/TODO.md index dab3310..c345355 100644 --- a/TODO.md +++ b/TODO.md @@ -25,10 +25,20 @@ The disk cache is now size-bounded with LRU eviction # Next Step -P1: implement blocked networks configuration to extend SSRF protection +P1: rate limit global concurrent upstream fetches to prevent resource +exhaustion # Completed Steps +- 2026-09-21 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 @@ -130,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 diff --git a/config.example.yml b/config.example.yml index 122189e..e3fbd4a 100644 --- a/config.example.yml +++ b/config.example.yml @@ -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 diff --git a/internal/config/blocked_networks_internal_test.go b/internal/config/blocked_networks_internal_test.go new file mode 100644 index 0000000..25373be --- /dev/null +++ b/internal/config/blocked_networks_internal_test.go @@ -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}, + }, + }) +} diff --git a/internal/config/config.go b/internal/config/config.go index e3ab570..c8c1d60 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -6,6 +6,7 @@ import ( "fmt" "log/slog" "math" + "net/netip" "net/url" "os" "path/filepath" @@ -42,6 +43,7 @@ const ( keyAllowHTTP = "allow_http" keyUpstreamConnectionsPerHost = "upstream_connections_per_host" keyCacheMaxBytes = "cache_max_bytes" + keyBlockedNetworks = "blocked_networks" ) // placeholderSigningKey is the dummy signing_key shipped in @@ -60,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") @@ -109,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 @@ -177,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{ @@ -192,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 @@ -224,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 } @@ -308,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 } @@ -802,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) + } +} diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index 5323167..09f3ffc 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -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, diff --git a/internal/httpfetcher/blocked_networks_internal_test.go b/internal/httpfetcher/blocked_networks_internal_test.go new file mode 100644 index 0000000..dfbf8f5 --- /dev/null +++ b/internal/httpfetcher/blocked_networks_internal_test.go @@ -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) + } + }) +} diff --git a/internal/httpfetcher/httpfetcher.go b/internal/httpfetcher/httpfetcher.go index 0abf1fa..64e27eb 100644 --- a/internal/httpfetcher/httpfetcher.go +++ b/internal/httpfetcher/httpfetcher.go @@ -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