From 15ada4490209444f73532d1d3f47e5a9243789e3 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 20 Aug 2026 04:13:06 +0000 Subject: [PATCH] Add an egress CIDR allowlist to the SSRF guard (closes #204) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SSRF blocklist had no escape hatch, so the thing webhooker is mostly for — taking a public webhook and forwarding it to something on your own network — could not be configured at all. Every private address, Docker sibling and loopback service was permanently unreachable as a delivery destination. ALLOWED_EGRESS_CIDRS (default empty) names blocks that delivery targets may reach despite the default blocklist. It is an allowlist and only ever adds destinations: there is no boolean, and no value disables SSRF protection wholesale. Empty, the guard behaves exactly as before. A fixed set of addresses is refused before the allowlist is consulted, so no supplied CIDR opens one — not the exact address, not a supernet, not 0.0.0.0/0 or ::/0. It is the two link-local blocks (169.254.0.0/16, fe80::/10) plus host routes for the cloud metadata endpoints that sit outside them: AWS's IPv6 IMDS at fd00:ec2::254, which lives in ordinary ULA space, and Alibaba's 100.100.100.200, which lives in CGNAT. Allowlisting fd00::/8 or 100.64.0.0/10 (Tailscale's range) is an ordinary thing for an operator to do and must not reopen instance-credential theft. The IPv4-compatible (::a9fe:a9fe) and NAT64 (64:ff9b::a9fe:a9fe) spellings of 169.254.169.254 are listed too, because To4() does not normalise them into the link-local block the way it does the IPv4-mapped form. Reaching any of these is credential theft rather than delivery to an internal service. The policy now lives in one function, Guard.checkIP, which both target-creation validation and the delivery dialer call. The two paths previously decided separately, which is how they came to disagree about a destination. The guard is built once from config and injected via fx into both the handlers and the delivery engine, so there is a single instance and a single answer. A set-but-unparseable value aborts startup naming the variable, reusing the existing envPrefixList parser. A non-empty list is logged at startup with the blocks spelled out, not counted, so the hole is visible in the log of any deployment that has one. Tests: an allowlisted loopback CIDR both validates and delivers to a live server (and the same URL still fails without the allowlist); a private address outside the listed block stays refused on both paths; every unconditionally blocked address stays refused on both paths under an allowlist that covers it, and the set itself is pinned entry by entry; public addresses are unaffected either way; and config coverage for parsing, startup abort, and the warning's contents. --- README.md | 125 ++++- cmd/webhooker/main.go | 4 + internal/config/config.go | 63 +++ internal/config/config_test.go | 181 +++++++ internal/config/export_test.go | 15 + internal/delivery/client_ssrf_test.go | 9 +- internal/delivery/engine.go | 3 +- internal/delivery/export_test.go | 21 + internal/delivery/ssrf.go | 288 ++++++++++-- internal/delivery/ssrf_allowlist_test.go | 576 +++++++++++++++++++++++ internal/delivery/ssrf_test.go | 18 +- internal/delivery/url_mask_test.go | 2 +- internal/handlers/handlers.go | 7 + internal/handlers/handlers_test.go | 1 + internal/handlers/source_management.go | 2 +- internal/resetpw/resetpw_test.go | 1 + internal/server/routes_test.go | 1 + 17 files changed, 1256 insertions(+), 61 deletions(-) create mode 100644 internal/delivery/ssrf_allowlist_test.go diff --git a/README.md b/README.md index 844e410..18e2007 100644 --- a/README.md +++ b/README.md @@ -114,6 +114,116 @@ TTY detection, and security headers are always applied. | `SESSION_IDLE_TIMEOUT` | Idle session timeout (Go duration) | `24h` | | `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint (10x that per IP across the route) | `120` | | `TRUSTED_PROXIES` | CIDRs whose forwarded headers are trusted (unset: all clients behind a proxy share one rate-limit bucket; a correct login password is never throttled either way) | `""` (none) | +| `ALLOWED_EGRESS_CIDRS` | CIDRs that delivery targets may reach despite the SSRF blocklist. Read [Allowing egress to your own network](#allowing-egress-to-your-own-network) before setting it | `""` (none) | + +#### Allowing egress to your own network + +By default every delivery target must resolve to a public address, and +a handful of public ones are refused too. The private and reserved +ranges — RFC 1918, loopback, CGNAT, link-local and the rest — are +refused, which stops a target from being used to make webhooker probe +the network it sits in; so are the cloud metadata endpoints listed +below that happen to live on public addresses. + +That default is also inconvenient for the thing webhooker is mostly +for: taking a public webhook and forwarding it to something on your own +network. A container on the same Docker network, a box on `10.x`, a +service on `127.0.0.1` — all refused, until you name them. + +`ALLOWED_EGRESS_CIDRS` is a comma-separated list of CIDR blocks (a bare +address such as `10.0.0.7` is accepted and treated as a single host), +for example `10.0.0.0/8, 172.17.0.0/16`. Addresses inside those blocks +become valid delivery destinations. Everything outside them keeps the +default answer, so this only ever adds destinations — it never removes +any, and it cannot narrow what was already reachable. + +**The risk, plainly.** Each block you list is a network that anyone who +can create a delivery target can now make this process issue requests +into, and read the response body back out of via the delivery log. That +is server-side request forgery, deliberately enabled and scoped by you. +A webhooker admin account is therefore as trusted as the narrowest +thing on those networks: an unauthenticated admin panel, a database +listening without a password, or an internal API that trusts its +network position is reachable through it. List the smallest blocks that +cover the destinations you actually deliver to — prefer +`10.1.2.3/32` over `10.0.0.0/8` — and never list a block wider than the +network you are willing to expose. + +Listing `0.0.0.0/0` or `::/0` opens **every** other private and +reserved range at once — loopback, RFC 1918, CGNAT, ULA, the lot. It is +a functional off switch for everything except the addresses listed as +unconditionally blocked below, and it makes any delivery target a probe +into your entire network and this host's own loopback services. Do not +list it. + +Two things this setting cannot do: + +- **It cannot turn the guard off.** There is no boolean, and no value + that disables SSRF protection wholesale. The guard is always on and + the list is always an allowlist; an empty list (the default) means + every private and reserved range stays refused. Note that + `0.0.0.0/0` gets you most of the way there anyway, per above. +- **It cannot open link-local, or a cloud metadata endpoint that + discloses credentials or user data.** An address is on the list below + when both of these hold: the provider fixes it, so it cannot collide + with anything you run; and reaching it hands out credentials, user + data or bootstrap material. Those stay blocked no matter what you + list, including when you list them outright or list a supernet such + as `0.0.0.0/0`, `::/0`, `fd00::/8` or `100.64.0.0/10`. Treat this as + best effort rather than a guarantee — it is a hand-maintained list + and the caveat below the table applies: + + | Blocked unconditionally | What it is | + | ----------------------- | ---------- | + | `169.254.0.0/16` | IPv4 link-local, carrying `169.254.169.254` (AWS, Azure, DigitalOcean, Hetzner, OpenStack and others — not Alibaba, which uses `100.100.100.200` below) | + | `fe80::/10` | IPv6 link-local | + | `fd00:ec2::254/128` | AWS IPv6 IMDS | + | `fd00:ec2::23/128` | AWS EKS Pod Identity Agent | + | `fd20:ce::254/128` | GCP metadata for IPv6-only instances | + | `fd00:c1::a9fe:a9fe/128` | Oracle OCI IMDS over IPv6 | + | `fd00:42::42/128` | Scaleway metadata over IPv6 | + | `fd00:a9fe:a9fe::1/128` | Linode/Akamai metadata over IPv6 | + | `100.100.100.200/32` | Alibaba Cloud metadata, inside CGNAT | + | `192.0.0.192/32` | Oracle Cloud Classic metadata | + | `168.63.129.16/32` | Azure WireServer | + | `147.75.207.243/32` | Equinix Metal metadata | + | `::a9fe:a9fe/128` | `169.254.169.254` as an IPv4-compatible IPv6 address | + | `64:ff9b::a9fe:a9fe/128` | `169.254.169.254` behind the NAT64 well-known prefix | + + The IPv4-mapped form `::ffff:169.254.169.254` is covered by the + `169.254.0.0/16` entry. Reaching any of these is credential or + user-data theft rather than delivery to an internal service. Every + entry outside the two link-local blocks is a single address, so + blocking it costs you nothing else on the network around it. + + The six ULA entries, all inside `fd00::/8`, are why this matters in + practice: `fd00::/8` is an ordinary block to allowlist for your own + IPv6 network, and without those host routes that one line would hand + out cloud credentials on five providers at once. There is only one + `/8` involved — `fd20:ce::254` masks into `fd00::/8` as well — and + the six endpoints are five providers because AWS appears twice, IMDS + and EKS Pod Identity. Several of them are described as "link-local" — + or even "localhost" — in their own vendor's documentation, but they + are ULAs and `fe80::/10` does not cover them. + + `168.63.129.16` and `147.75.207.243` are ordinary public addresses + rather than reserved ones, so they are the only two entries here that + are blocked without an allowlist being involved at all. + + This list is not exhaustive of every cloud's metadata address — if + yours is not here, do not allowlist the block that contains it. + +The list is applied at one place in the code, which both target +creation and delivery consult, so a URL that the target form accepts is +one that delivery will actually attempt — the two cannot disagree. +Delivery re-resolves and re-checks the destination at dial time, so a +hostname that resolves to an allowed address during validation and a +different one later (DNS rebinding) is still refused unless the new +address is also allowed. + +A set but unparseable value aborts startup. When the list is non-empty +webhooker logs it at startup, blocks and all, so the hole is visible in +the log of any deployment that has one. #### Metrics credentials @@ -272,8 +382,9 @@ additionally be a number in the range 1–65535, `RECEIVER_RATE_LIMIT` must be at least 1, `RETENTION_SWEEP_INTERVAL` must be greater than zero (it is a ticker period, so `0s` or a negative value would crash the reaper after -startup), and every entry in `TRUSTED_PROXIES` must be a CIDR block or -a bare IP address. `SESSION_IDLE_TIMEOUT` is the exception: a +startup), and every entry in `TRUSTED_PROXIES` and +`ALLOWED_EGRESS_CIDRS` must be a CIDR block or a bare IP address. +`SESSION_IDLE_TIMEOUT` is the exception: a non-positive value there means idle expiry is disabled, not invalid. Boolean variables (`DEBUG`, `MAINTENANCE_MODE`) accept exactly the @@ -2332,7 +2443,15 @@ check, see [The login endpoint](#the-login-endpoint). ranges (RFC 1918, loopback, link-local, cloud metadata) are blocked both at target creation time (URL validation) and at delivery time (custom HTTP transport with SSRF-safe dialer that validates resolved - IPs before connecting, preventing DNS rebinding attacks) + IPs before connecting, preventing DNS rebinding attacks). Both paths + route through a single decision function, so they cannot disagree + about a destination. An operator can permit specific blocks with + [`ALLOWED_EGRESS_CIDRS`](#allowing-egress-to-your-own-network); the + guard cannot be switched off, and link-local plus a + [pinned set](#allowing-egress-to-your-own-network) of known cloud + metadata endpoints — several of which are ULAs or public addresses + outside link-local — stay blocked whatever is listed, though listing + `0.0.0.0/0` or `::/0` does open every other private range - **Login limiting is inverted, deliberately.** The login `POST` has no pre-emptive rate limiter in front of it. Credentials are verified first and only a _failed_ attempt spends budget, so a diff --git a/cmd/webhooker/main.go b/cmd/webhooker/main.go index 1631256..7bea37a 100644 --- a/cmd/webhooker/main.go +++ b/cmd/webhooker/main.go @@ -157,6 +157,10 @@ func newApp() *fx.App { session.New, handlers.New, middleware.New, + // The one SSRF guard both target-creation validation + // and the delivery dialer consult, so they cannot + // disagree about a destination. + delivery.NewGuard, delivery.New, delivery.NewArchiveSweeper, // Wire *delivery.Engine as delivery.Notifier so the diff --git a/internal/config/config.go b/internal/config/config.go index 5d46109..9c886a1 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -128,6 +128,22 @@ type Config struct { // clients. TrustedProxies []netip.Prefix + // AllowedEgressCIDRs is the set of networks a delivery target + // may reach even though the SSRF guard's default blocklist + // covers them. It is empty unless ALLOWED_EGRESS_CIDRS is set, + // and empty means every private/reserved range stays refused. + // + // This only ever adds destinations to what the guard would + // otherwise refuse. The guard itself is always on: there is no + // setting that disables SSRF protection, and delivery's + // alwaysBlockedNetworks stays blocked no matter what is listed + // here. That set is link-local plus the cloud metadata + // endpoints outside it that disclose credentials or user data + // at a provider-fixed address; it is not exhaustive of every + // cloud's metadata address. See alwaysBlockedNetworks for the + // authoritative list and the criterion it is built from. + AllowedEgressCIDRs []netip.Prefix + params *ConfigParams log *slog.Logger } @@ -472,6 +488,11 @@ func loadFromEnv() (*Config, error) { return nil, err } + allowedEgressCIDRs, err := envPrefixList("ALLOWED_EGRESS_CIDRS") + if err != nil { + return nil, err + } + metricsUsername, metricsPassword, err := resolveMetricsAuth() if err != nil { return nil, err @@ -490,9 +511,49 @@ func loadFromEnv() (*Config, error) { SessionIdleTimeout: sessionIdleTimeout, ReceiverRateLimit: receiverRateLimit, TrustedProxies: trustedProxies, + AllowedEgressCIDRs: allowedEgressCIDRs, }, nil } +// PrefixStrings renders a prefix list as its CIDR strings, for +// logging a list an operator has to be able to read back. +func PrefixStrings(prefixes []netip.Prefix) []string { + out := make([]string, 0, len(prefixes)) + + for _, prefix := range prefixes { + out = append(out, prefix.String()) + } + + return out +} + +// warnEgressAllowlist logs the effective ALLOWED_EGRESS_CIDRS +// whenever it is non-empty. +// +// It prints the blocks themselves rather than a count, because +// this is the one setting that lets a delivery target reach the +// host's own network: an operator reading the startup log has to +// be able to see exactly which hole is open. Silence means the +// list is empty and the SSRF guard is refusing every +// private/reserved range, which is the default. +func (c *Config) warnEgressAllowlist(log *slog.Logger) { + if len(c.AllowedEgressCIDRs) == 0 { + return + } + + log.Warn( + "ALLOWED_EGRESS_CIDRS lets delivery targets reach these "+ + "otherwise-blocked private/reserved networks. Anyone "+ + "who can create a delivery target can now make this "+ + "process issue requests into them, and read back the "+ + "response. Link-local and the known cloud instance "+ + "metadata endpoints outside it stay blocked "+ + "regardless of what is listed here.", + "allowedEgressCIDRs", + strings.Join(PrefixStrings(c.AllowedEgressCIDRs), ","), + ) +} + // warnSharedRateLimitBucket logs a startup warning whenever // TRUSTED_PROXIES is empty, in any environment. // @@ -574,11 +635,13 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) { "sessionIdleTimeout", s.SessionIdleTimeout.String(), "receiverRateLimit", s.ReceiverRateLimit, "trustedProxies", len(s.TrustedProxies), + "allowedEgressCIDRs", len(s.AllowedEgressCIDRs), "hasSentryDSN", s.SentryDSN != "", "hasMetricsAuth", s.MetricsAuthEnabled(), ) s.warnSharedRateLimitBucket(log) + s.warnEgressAllowlist(log) return s, nil } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index cb0ebb9..f38f7fd 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -663,6 +663,187 @@ func testTrustedProxiesSuccess( assert.Equal(t, expected, got) } +// TestAllowedEgressCIDRs covers ALLOWED_EGRESS_CIDRS, the escape +// hatch that lets a self-hosted deployment forward to its own +// network. Unset it must stay empty, so the SSRF guard keeps +// refusing every private/reserved range; a set-but-unparseable +// value must abort startup naming the variable rather than +// silently running with a list the operator did not write. +func TestAllowedEgressCIDRs(t *testing.T) { + tests := []struct { + name string + set bool + value string + expected []string + expectError bool + }{ + { + name: caseUnsetUsesDefault, + set: false, + expected: []string{}, + }, + { + name: "empty value yields empty list", + set: true, + value: "", + expected: []string{}, + }, + { + name: caseValidValueParsed, + set: true, + value: cidrPrivateV4, + expected: []string{cidrPrivateV4}, + }, + { + name: "multiple blocks with whitespace", + set: true, + value: " 10.0.0.0/8 , 127.0.0.0/8 ", + expected: []string{cidrPrivateV4, "127.0.0.0/8"}, + }, + { + name: "bare address becomes a single host", + set: true, + value: "172.17.0.5", + expected: []string{"172.17.0.5/32"}, + }, + { + name: caseUnparseableFails, + set: true, + value: cidrPrivateV4 + ",not-an-address", + expectError: true, + }, + { + name: "out-of-range prefix length fails startup", + set: true, + value: "10.0.0.0/33", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Cannot use t.Parallel() here because t.Setenv + // is incompatible with parallel subtests. + t.Setenv("WEBHOOKER_ENVIRONMENT", "dev") + + if tt.set { + t.Setenv("ALLOWED_EGRESS_CIDRS", tt.value) + } else { + require.NoError( + t, os.Unsetenv("ALLOWED_EGRESS_CIDRS"), + ) + } + + if tt.expectError { + expectStartupErrorFor( + t, "ALLOWED_EGRESS_CIDRS", config.ErrInvalidCIDR, + ) + } else { + testAllowedEgressCIDRsSuccess(t, tt.expected) + } + }) + } +} + +func testAllowedEgressCIDRsSuccess( + t *testing.T, + expected []string, +) { + t.Helper() + + var cfg *config.Config + + app := fxtest.New( + t, + fx.Provide( + globals.New, + logger.New, + config.New, + ), + fx.Populate(&cfg), + ) + require.NoError(t, app.Err()) + + app.RequireStart() + + defer app.RequireStop() + + assert.Equal( + t, expected, config.PrefixStrings(cfg.AllowedEgressCIDRs), + ) +} + +// TestEgressAllowlistWarning covers the startup log that shows an +// operator the hole ALLOWED_EGRESS_CIDRS opened. It must stay +// silent on the default (empty) list and, when set, print the +// blocks themselves rather than a count. +func TestEgressAllowlistWarning(t *testing.T) { + tests := []struct { + name string + allowed string + expectWarning bool + }{ + { + name: "empty allowlist is quiet", + expectWarning: false, + }, + { + name: "non-empty allowlist warns", + allowed: "10.0.0.0/8,127.0.0.0/8", + expectWarning: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Cannot use t.Parallel() here because t.Setenv + // is incompatible with parallel subtests. + t.Setenv("WEBHOOKER_ENVIRONMENT", config.EnvironmentDev) + + if tt.allowed == "" { + require.NoError( + t, os.Unsetenv("ALLOWED_EGRESS_CIDRS"), + ) + } else { + t.Setenv("ALLOWED_EGRESS_CIDRS", tt.allowed) + } + + var buf bytes.Buffer + + log := slog.New(slog.NewJSONHandler( + &buf, &slog.HandlerOptions{ + Level: slog.LevelDebug, + }, + )) + + require.NoError( + t, config.WarnEgressAllowlistForTest(log), + ) + + if !tt.expectWarning { + assert.Empty(t, buf.String()) + + return + } + + logged := buf.String() + + assert.Contains(t, logged, `"level":"WARN"`) + assert.Contains(t, logged, "ALLOWED_EGRESS_CIDRS") + // The blocks themselves, not a count: the operator has + // to be able to read back which networks are open. + assert.Contains(t, logged, "10.0.0.0/8") + assert.Contains(t, logged, "127.0.0.0/8") + // What stays shut. Asserted on the clause naming the + // wider set rather than on "Link-local" alone, so the + // string cannot narrow back to link-local only while + // the always-blocked set covers ULA, CGNAT and two + // public metadata addresses as well. + assert.Contains(t, logged, "metadata endpoints outside it") + }) + } +} + // TestSharedRateLimitBucketWarning covers the startup warning that // tells an operator a deployment behind a reverse proxy shares one // rate-limit bucket between every client, which turns the receiver diff --git a/internal/config/export_test.go b/internal/config/export_test.go index 1d137a6..710e680 100644 --- a/internal/config/export_test.go +++ b/internal/config/export_test.go @@ -21,6 +21,21 @@ func WarnSharedRateLimitBucketForTest(log *slog.Logger) error { return nil } +// WarnEgressAllowlistForTest loads a Config from the current +// environment and emits its egress-allowlist startup warning to +// log, so a test can assert both that the warning fires only when +// the list is non-empty and that it names the blocks it opened. +func WarnEgressAllowlistForTest(log *slog.Logger) error { + c, err := loadFromEnv() + if err != nil { + return err + } + + c.warnEgressAllowlist(log) + + return nil +} + // EnvBoolForTest exposes envBool. func EnvBoolForTest(key string, defaultValue bool) (bool, error) { return envBool(key, defaultValue) diff --git a/internal/delivery/client_ssrf_test.go b/internal/delivery/client_ssrf_test.go index d31164a..44a676c 100644 --- a/internal/delivery/client_ssrf_test.go +++ b/internal/delivery/client_ssrf_test.go @@ -18,8 +18,9 @@ func newSSRFTestEngine() *delivery.Engine { log := slog.New(slog.DiscardHandler) client := &http.Client{ - Timeout: 30 * time.Second, - Transport: delivery.NewSSRFSafeTransport(), + Timeout: 30 * time.Second, + Transport: delivery.NewTestGuard(). + NewSSRFSafeTransport(), } return delivery.NewTestEngine(log, client, 1) @@ -36,8 +37,8 @@ func TestClientForConfig_TimeoutKeepsSSRFGuard(t *testing.T) { engine := newSSRFTestEngine() blocked := []string{ - "http://127.0.0.1/hook", - "http://169.254.169.254/latest/meta-data/", + loopbackHookURL, + metadataURL, "http://[fe80::1]/hook", } diff --git a/internal/delivery/engine.go b/internal/delivery/engine.go index 12f238d..81bdc1f 100644 --- a/internal/delivery/engine.go +++ b/internal/delivery/engine.go @@ -121,6 +121,7 @@ type EngineParams struct { DB *database.Database DBManager *database.WebhookDBManager Logger *logger.Logger + SSRFGuard *Guard } // Engine processes queued deliveries in the background @@ -176,7 +177,7 @@ func New( e.initTargets(&http.Client{ Timeout: httpClientTimeout, - Transport: NewSSRFSafeTransport(), + Transport: params.SSRFGuard.NewSSRFSafeTransport(), }) e.registerHooks(lc) diff --git a/internal/delivery/export_test.go b/internal/delivery/export_test.go index fb509fa..a0ab563 100644 --- a/internal/delivery/export_test.go +++ b/internal/delivery/export_test.go @@ -5,6 +5,7 @@ import ( "log/slog" "net" "net/http" + "net/netip" "time" "go.uber.org/fx" @@ -32,6 +33,26 @@ func ExportIsBlockedIP(ip net.IP) bool { return isBlockedIP(ip) } +// NewTestGuard builds an SSRF Guard from an explicit egress +// allowlist, without going through config. Passing no prefixes +// yields the default guard, which blocks every private/reserved +// range. +func NewTestGuard(allowed ...netip.Prefix) *Guard { + return &Guard{allowed: allowed} +} + +// ExportCheckIP exposes the guard's single decision point, so a +// test can assert the policy both the validator and the dialer +// inherit without needing a live destination. +func (g *Guard) ExportCheckIP(ip net.IP) error { + return g.checkIP(ip) +} + +// ExportAlwaysBlockedNetworks exposes alwaysBlockedNetworks. +func ExportAlwaysBlockedNetworks() []*net.IPNet { + return alwaysBlockedNetworks +} + // ExportBlockedNetworks exposes blockedNetworks. func ExportBlockedNetworks() []*net.IPNet { return blockedNetworks diff --git a/internal/delivery/ssrf.go b/internal/delivery/ssrf.go index fb6bacc..6ed3593 100644 --- a/internal/delivery/ssrf.go +++ b/internal/delivery/ssrf.go @@ -6,8 +6,11 @@ import ( "fmt" "net" "net/http" + "net/netip" "net/url" "time" + + "sneak.berlin/go/webhooker/internal/config" ) const ( @@ -25,20 +28,75 @@ var ( errBlockedIP = errors.New( "blocked private/reserved IP range", ) + errBlockedMetadata = errors.New( + "blocked link-local or cloud instance metadata " + + "address: ALLOWED_EGRESS_CIDRS cannot open it", + ) errInvalidScheme = errors.New( "only http and https are allowed", ) ) // blockedNetworks contains all private/reserved IP ranges -// that should be blocked to prevent SSRF attacks. +// that should be blocked to prevent SSRF attacks. An operator +// can permit specific blocks out of this set with +// ALLOWED_EGRESS_CIDRS; see Guard. // //nolint:gochecknoglobals // package-level network list is appropriate here var blockedNetworks []*net.IPNet +// alwaysBlockedNetworks are the ranges no configuration can +// open: the link-local blocks and the cloud instance metadata +// endpoints that live outside them. Reaching one is credential +// or user-data theft rather than delivery to an internal +// service, so a supplied CIDR that covers such an address still +// leaves it blocked. +// +// Inclusion criterion — an address belongs here only if BOTH +// hold, and every entry below satisfies both: +// +// 1. It is a fixed address assigned by the provider, or a +// range reserved by IANA — never one the operator chose. +// That is what makes a host route free: it cannot collide +// with anything the operator runs. +// 2. Reaching it discloses credentials, or user data or +// bootstrap material — something granting onward access, or +// not cheaply rotated. +// +// Both halves are load-bearing, so use them to refuse a +// candidate and say why. An endpoint disclosing only the +// operator's own inventory (instance id, region, disks, NICs) +// fails (2): letting a delivery target reach the operator's own +// infrastructure is the feature ALLOWED_EGRESS_CIDRS exists to +// provide. But (2) is not "IAM credentials only" either — +// fd00:42::42 serves /user_data and /conf rather than tokens, +// and user data routinely carries bootstrap secrets. An address +// stays out if it fails (1) however well it clears (2): a host +// route inside a block operators really assign from, such as +// 10.0.0.0/8, can collide with a real internal service and +// forfeits the justification in (1). +// +// This is a criterion, not an enumeration of every metadata +// address in existence. +// +// Some of these are also in blockedNetworks and this list is +// what makes them unconditional; two are public unicast and are +// reachable by default without it. Every entry outside the +// link-local blocks is a /32 or /128 host route, so blocking it +// costs an operator nothing else on the surrounding network. +// +// Derive membership from the address, never from the vendor's +// prose. Several providers call these endpoints "link-local" or +// even "localhost" in their own documentation while the address +// is a ULA outside fe80::/10, so a set derived from the docs +// comes out wrong. +// +//nolint:gochecknoglobals // package-level network list is appropriate here +var alwaysBlockedNetworks []*net.IPNet + //nolint:gochecknoinits // init is the idiomatic way to parse CIDRs once at startup func init() { - cidrs := []string{ + blockedNetworks = mustParseCIDRs([]string{ "127.0.0.0/8", "10.0.0.0/8", "172.16.0.0/12", @@ -56,7 +114,78 @@ func init() { "::1/128", "fc00::/7", "fe80::/10", - } + }) + + // Every entry is named. The set must not grow or shrink + // without a matching change to + // TestAlwaysBlockedNetworks_PinnedSet. + // + // The IPv4-mapped form ::ffff:169.254.169.254 needs no + // entry: net.IPNet.Contains normalises it via To4() before + // comparing, so 169.254.0.0/16 already matches it. To4() + // does not normalise the IPv4-compatible or NAT64 forms, + // which is why those are listed separately. + alwaysBlockedNetworks = mustParseCIDRs([]string{ + // IPv4 link-local, carrying the 169.254.169.254 + // metadata service used by AWS, Azure, DigitalOcean, + // Hetzner, OpenStack and others. Not Alibaba, which uses + // 100.100.100.200 below exclusively. + "169.254.0.0/16", + // IPv6 link-local, its IPv6 counterpart. + "fe80::/10", + + // IPv6 metadata endpoints in ULA space. Each is a host + // route, and fd00::/8 is an ordinary block for an + // operator to allowlist, so without these entries that + // one allowlist line hands out cloud credentials on + // every provider below. + // + // AWS IPv6 IMDS. + "fd00:ec2::254/128", + // AWS EKS Pod Identity Agent, which issues pod identity + // credentials. A second AWS endpoint, distinct from + // IMDS above. AWS's own docs call it "localhost". + "fd00:ec2::23/128", + // GCP metadata server for IPv6-only instances. + "fd20:ce::254/128", + // Oracle OCI IMDS, serving /opc/v2 instance principals. + "fd00:c1::a9fe:a9fe/128", + // Scaleway metadata, serving /user_data and /conf. + "fd00:42::42/128", + // Linode/Akamai metadata. Akamai's docs call it + // "link-local"; it is not. + "fd00:a9fe:a9fe::1/128", + + // IPv4 metadata endpoints outside link-local. + // + // Alibaba Cloud metadata. It sits in CGNAT + // 100.64.0.0/10, which Tailscale also uses, so an + // operator allowlisting a Tailscale peer's range would + // otherwise reopen it. + "100.100.100.200/32", + // Oracle Cloud Classic metadata. Inside the blocked + // 192.0.0.0/24, so this entry is what stops an + // allowlist from opening it. + "192.0.0.192/32", + // Azure WireServer, carrying goalstate and extension + // settings on ports 80 and 32526. Public unicast, so + // this entry is what blocks it at all. + "168.63.129.16/32", + // Equinix Metal metadata. Public unicast, likewise. + "147.75.207.243/32", + + // 169.254.169.254 as an IPv4-compatible IPv6 address. + "::a9fe:a9fe/128", + // 169.254.169.254 behind the NAT64 well-known prefix. + "64:ff9b::a9fe:a9fe/128", + }) +} + +// mustParseCIDRs parses a list of CIDR literals, panicking on a +// bad one. The inputs are compile-time constants, so a failure +// is a programming error rather than a runtime condition. +func mustParseCIDRs(cidrs []string) []*net.IPNet { + networks := make([]*net.IPNet, 0, len(cidrs)) for _, cidr := range cidrs { _, network, err := net.ParseCIDR(cidr) @@ -67,16 +196,15 @@ func init() { )) } - blockedNetworks = append( - blockedNetworks, network, - ) + networks = append(networks, network) } + + return networks } -// isBlockedIP checks whether an IP address falls within -// any blocked private/reserved network range. -func isBlockedIP(ip net.IP) bool { - for _, network := range blockedNetworks { +// matchesAny reports whether ip falls inside any of networks. +func matchesAny(networks []*net.IPNet, ip net.IP) bool { + for _, network := range networks { if network.Contains(ip) { return true } @@ -85,9 +213,40 @@ func isBlockedIP(ip net.IP) bool { return false } +// isBlockedIP checks whether an IP address falls within +// any blocked private/reserved network range, before any +// operator allowlist is considered. +func isBlockedIP(ip net.IP) bool { + return matchesAny(blockedNetworks, ip) +} + +// Guard makes every SSRF decision in the process. +// +// It holds the operator's ALLOWED_EGRESS_CIDRS allowlist and +// applies it in exactly one place, checkIP, which both the +// target-creation validator (ValidateTargetURL) and the delivery +// dialer call. Routing both through the same function is the +// point: when the two paths decided separately they drifted and +// disagreed, which is what made a target creatable but +// undeliverable. +// +// The guard is always on. The allowlist only ever adds specific +// networks to what the default blocklist refuses, and no +// configuration turns the guard off wholesale. +type Guard struct { + // allowed is the operator's ALLOWED_EGRESS_CIDRS. Empty + // (the default) means the default blocklist stands as-is. + allowed []netip.Prefix +} + +// NewGuard builds the process-wide SSRF guard from configuration. +func NewGuard(cfg *config.Config) *Guard { + return &Guard{allowed: cfg.AllowedEgressCIDRs} +} + // ValidateTargetURL checks that an HTTP delivery target // URL is safe from SSRF attacks. -func ValidateTargetURL( +func (g *Guard) ValidateTargetURL( ctx context.Context, targetURL string, ) error { parsed, err := url.Parse(targetURL) @@ -111,36 +270,79 @@ func ValidateTargetURL( } if ip := net.ParseIP(host); ip != nil { - return checkBlockedIP(ip) + return g.checkIP(ip) } - return validateHostname(ctx, host) + return g.validateHostname(ctx, host) } -func validateScheme(scheme string) error { - if scheme != "http" && scheme != "https" { +// NewSSRFSafeTransport creates an http.Transport with a +// custom DialContext that refuses connections to any address +// this guard blocks. It resolves and checks at dial time, so a +// name that passed validation but now answers with a blocked +// address (DNS rebinding) is still refused. +func (g *Guard) NewSSRFSafeTransport() *http.Transport { + return &http.Transport{ + DialContext: g.ssrfDialContext, + } +} + +// allows reports whether ip falls inside the operator's +// configured egress allowlist. +func (g *Guard) allows(ip net.IP) bool { + if len(g.allowed) == 0 { + return false + } + + addr, ok := netip.AddrFromSlice(ip) + if !ok { + return false + } + + // Config unmaps every parsed prefix, so an IPv4-mapped + // address has to be unmapped too or it would never match. + addr = addr.Unmap() + + for _, prefix := range g.allowed { + if prefix.Contains(addr) { + return true + } + } + + return false +} + +// checkIP is the single point at which SSRF policy is decided. +// +// The order is the policy: +// +// 1. alwaysBlockedNetworks is refused before the allowlist is +// consulted, so no configured CIDR reaches link-local or a +// cloud instance metadata endpoint. +// 2. The allowlist is consulted next, so a listed private +// network becomes reachable. +// 3. Everything else keeps the default blocklist's answer. +func (g *Guard) checkIP(ip net.IP) error { + if matchesAny(alwaysBlockedNetworks, ip) { return fmt.Errorf( - "unsupported URL scheme %q: %w", - scheme, errInvalidScheme, + "target IP %s: %w", ip, errBlockedMetadata, ) } - return nil -} + if g.allows(ip) { + return nil + } -func checkBlockedIP(ip net.IP) error { if isBlockedIP(ip) { return fmt.Errorf( - "target IP %s is in a blocked "+ - "private/reserved range: %w", - ip, errBlockedIP, + "target IP %s: %w", ip, errBlockedIP, ) } return nil } -func validateHostname( +func (g *Guard) validateHostname( ctx context.Context, host string, ) error { dnsCtx, cancel := context.WithTimeout( @@ -165,11 +367,11 @@ func validateHostname( } for _, ipAddr := range ips { - if isBlockedIP(ipAddr.IP) { + err = g.checkIP(ipAddr.IP) + if err != nil { return fmt.Errorf( - "hostname %q resolves to blocked "+ - "IP %s: %w", - host, ipAddr.IP, errBlockedIP, + "hostname %q resolves to a blocked address: %w", + host, err, ) } } @@ -177,16 +379,7 @@ func validateHostname( return nil } -// NewSSRFSafeTransport creates an http.Transport with a -// custom DialContext that blocks connections to -// private/reserved IP addresses. -func NewSSRFSafeTransport() *http.Transport { - return &http.Transport{ - DialContext: ssrfDialContext, - } -} - -func ssrfDialContext( +func (g *Guard) ssrfDialContext( ctx context.Context, network, addr string, ) (net.Conn, error) { @@ -209,11 +402,11 @@ func ssrfDialContext( } for _, ipAddr := range ips { - if isBlockedIP(ipAddr.IP) { + err = g.checkIP(ipAddr.IP) + if err != nil { return nil, fmt.Errorf( - "ssrf: connection to %s (%s) "+ - "blocked: %w", - host, ipAddr.IP, errBlockedIP, + "ssrf: connection to %s blocked: %w", + host, err, ) } } @@ -225,3 +418,14 @@ func ssrfDialContext( net.JoinHostPort(ips[0].IP.String(), port), ) } + +func validateScheme(scheme string) error { + if scheme != "http" && scheme != "https" { + return fmt.Errorf( + "unsupported URL scheme %q: %w", + scheme, errInvalidScheme, + ) + } + + return nil +} diff --git a/internal/delivery/ssrf_allowlist_test.go b/internal/delivery/ssrf_allowlist_test.go new file mode 100644 index 0000000..864dba1 --- /dev/null +++ b/internal/delivery/ssrf_allowlist_test.go @@ -0,0 +1,576 @@ +package delivery_test + +import ( + "context" + "net" + "net/http" + "net/http/httptest" + "net/netip" + "net/url" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sneak.berlin/go/webhooker/internal/delivery" +) + +// Addresses the SSRF tests in this package share. +const ( + // metadataIP is the cloud instance metadata address, and + // metadataURL an endpoint on it. The guard must never reach + // either, whatever an operator lists. + metadataIP = "169.254.169.254" + metadataURL = "http://" + metadataIP + "/latest/meta-data/" + + // loopbackHookURL is a target on this host: blocked by + // default, reachable only once an operator allowlists + // loopback. + loopbackHookURL = "http://127.0.0.1/hook" + + // publicIP is an ordinary public address, which the guard + // permits with or without an allowlist. + publicIP = "93.184.216.34" + + // allowAllIPv4 and allowAllIPv6 are the widest allowlist + // entries expressible: the whole internet, in each family. + // Nothing unconditionally blocked may be reachable under + // them. + allowAllIPv4 = "0.0.0.0/0" + allowAllIPv6 = "::/0" + + // allowAllULA is the ordinary ULA block an operator lists to + // reach their own IPv6 network. Several providers park a + // metadata endpoint inside it. + allowAllULA = "fd00::/8" + + // metadataRefusalClause is the part of the refusal that only + // alwaysBlockedNetworks produces. Asserting it, rather than + // the bare word "blocked", is what proves the unconditional + // set did the refusing and not the default blocklist. + metadataRefusalClause = "ALLOWED_EGRESS_CIDRS cannot open it" +) + +// TestGuardAllowlist_PermittedCIDRDelivers proves the escape +// hatch actually works end to end: with 127.0.0.0/8 allowed, the +// guard's own transport connects to a loopback server and gets a +// response back. The default guard, given the identical URL, +// refuses it — so the delivery succeeds because of the allowlist +// and nothing else. +func TestGuardAllowlist_PermittedCIDRDelivers(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNoContent) + }, + )) + t.Cleanup(srv.Close) + + // httptest listens on loopback, which the default blocklist + // covers: exactly the "forward to a service on this host" + // case the allowlist exists for. + requireLoopback(t, srv.URL) + + guard := delivery.NewTestGuard( + netip.MustParsePrefix("127.0.0.0/8"), + ) + + require.NoError(t, + guard.ValidateTargetURL(context.Background(), srv.URL), + "an allowlisted loopback target must pass validation", + ) + + client := &http.Client{ + Timeout: 5 * time.Second, + Transport: guard.NewSSRFSafeTransport(), + } + + req, err := http.NewRequestWithContext( + context.Background(), http.MethodPost, srv.URL, nil, + ) + require.NoError(t, err) + + resp, err := client.Do(req) + require.NoError(t, + err, "an allowlisted loopback target must be deliverable", + ) + + defer func() { _ = resp.Body.Close() }() + + assert.Equal(t, http.StatusNoContent, resp.StatusCode) + + // The same URL through the default guard must still fail, or + // this test would pass without the allowlist doing anything. + assert.Error(t, + delivery.NewTestGuard().ValidateTargetURL( + context.Background(), srv.URL, + ), + "without the allowlist the same target must be refused", + ) +} + +// TestGuardAllowlist_UnlistedPrivateStillRefused proves the +// allowlist grants only what it names. A guard that opens one +// private block must keep refusing every other one, at both the +// validation and the delivery entry point. +func TestGuardAllowlist_UnlistedPrivateStillRefused(t *testing.T) { + t.Parallel() + + // Only 10.1.0.0/16 is open — a narrow block inside a much + // wider private range, so the test can tell "permits the + // listed block" from "permits anything private". + guard := delivery.NewTestGuard( + netip.MustParsePrefix("10.1.0.0/16"), + ) + + refused := []string{ + "http://192.168.1.10/hook", + "http://172.16.0.1/hook", + loopbackHookURL, + "http://[fc00::1]/hook", + "http://100.64.0.1/hook", + // Private, adjacent to the allowed block, outside it. + "http://10.2.0.1/hook", + } + + for _, target := range refused { + t.Run(target, func(t *testing.T) { + t.Parallel() + + err := guard.ValidateTargetURL( + context.Background(), target, + ) + require.Error(t, + err, "%s is not allowlisted and must be refused", + target, + ) + assert.Contains(t, err.Error(), "blocked") + + assertDialRefused(t, guard, target) + }) + } + + // The block that is listed must in fact be permitted, so the + // refusals above are selective rather than a guard that + // ignores its allowlist entirely. + assert.NoError(t, + guard.ValidateTargetURL( + context.Background(), "http://10.1.2.3/hook", + ), + "the allowlisted block must be permitted", + ) +} + +// TestGuardAllowlist_MetadataAlwaysRefused is the load-bearing +// case: cloud instance metadata endpoints are credential theft +// rather than delivery to an internal service, so no allowlist +// reaches one. Every guard below names a CIDR that covers its +// target — including 0.0.0.0/0, ::/0, and the ordinary ULA and +// CGNAT blocks an operator would really list — and the address +// must stay refused anyway, on both the validation and the +// delivery path. +func TestGuardAllowlist_MetadataAlwaysRefused(t *testing.T) { + t.Parallel() + + for _, tt := range metadataAlwaysRefusedCases() { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + guard := delivery.NewTestGuard( + netip.MustParsePrefix(tt.allow), + ) + + err := guard.ValidateTargetURL( + context.Background(), tt.target, + ) + require.Error(t, + err, + "%s must stay blocked even though %s covers it", + tt.target, tt.allow, + ) + assert.Contains(t, + err.Error(), + metadataRefusalClause, + "the refusal must say why it cannot be opened", + ) + + // The metadata clause, not just "blocked": that is + // what distinguishes the unconditional set from the + // ordinary blocklist. + assertDialRefusedWith( + t, guard, tt.target, metadataRefusalClause, + ) + }) + } +} + +// metadataAlwaysRefusedCase is one (allowlist, target) pair that +// must be refused: allow covers target, and target must stay +// blocked regardless. +type metadataAlwaysRefusedCase struct { + name string + allow string + target string +} + +// metadataAlwaysRefusedCases enumerates every unconditionally +// blocked address together with an allowlist entry that would +// otherwise reach it. Split by family of address only to stay +// under the function-length limit. +func metadataAlwaysRefusedCases() []metadataAlwaysRefusedCase { + cases := linkLocalRefusedCases() + cases = append(cases, ulaMetadataRefusedCases()...) + cases = append(cases, ipv4MetadataRefusedCases()...) + + return append(cases, encodedMetadataRefusedCases()...) +} + +// linkLocalRefusedCases covers the link-local blocks, including +// an operator naming the metadata address outright. +func linkLocalRefusedCases() []metadataAlwaysRefusedCase { + return []metadataAlwaysRefusedCase{ + { + name: "exact metadata host", + allow: "169.254.169.254/32", + target: metadataURL, + }, + { + name: "whole link-local block", + allow: "169.254.0.0/16", + target: metadataURL, + }, + { + name: "supernet covering link-local", + allow: "169.0.0.0/8", + target: metadataURL, + }, + { + name: "the entire IPv4 internet", + allow: allowAllIPv4, + target: metadataURL, + }, + { + name: "other link-local address", + allow: allowAllIPv4, + target: "http://169.254.1.1/", + }, + { + name: "IPv6 link-local", + allow: allowAllIPv6, + target: "http://[fe80::1]/", + }, + } +} + +// ulaMetadataRefusedCases covers the metadata endpoints parked +// in ULA space. Every one is opened by the single ordinary +// allowlist entry fd00::/8, which is the whole reason they need +// their own /128 host routes: fe80::/10 does not cover a ULA, +// whatever the vendor's documentation calls the address. +func ulaMetadataRefusedCases() []metadataAlwaysRefusedCase { + return []metadataAlwaysRefusedCase{ + { + name: "AWS IPv6 IMDS under an allowlisted ULA block", + allow: allowAllULA, + target: "http://[fd00:ec2::254]/latest/meta-data/", + }, + { + // A second AWS credential endpoint, distinct from + // IMDS. AWS's own docs call this one "localhost". + name: "AWS EKS Pod Identity under an allowlisted ULA block", + allow: allowAllULA, + target: "http://[fd00:ec2::23]/v1/credentials", + }, + { + name: "GCP IPv6 metadata under an allowlisted ULA block", + allow: allowAllULA, + target: "http://[fd20:ce::254]/computeMetadata/v1/", + }, + { + name: "Oracle OCI IPv6 IMDS under an allowlisted ULA block", + allow: allowAllULA, + target: "http://[fd00:c1::a9fe:a9fe]/opc/v2/instance/", + }, + { + name: "Scaleway IPv6 metadata under an allowlisted ULA block", + allow: allowAllULA, + target: "http://[fd00:42::42]/conf", + }, + { + // Akamai's docs call this "link-local"; it is a ULA, + // so fe80::/10 does not cover it. + name: "Linode IPv6 metadata under an allowlisted ULA block", + allow: allowAllULA, + target: "http://[fd00:a9fe:a9fe::1]/v1/instance", + }, + } +} + +// ipv4MetadataRefusedCases covers the IPv4 metadata endpoints +// that sit outside link-local. The last two are public unicast, +// so nothing in the default blocklist covers them and this set +// is the only thing refusing them. +func ipv4MetadataRefusedCases() []metadataAlwaysRefusedCase { + return []metadataAlwaysRefusedCase{ + { + // Tailscale uses 100.64.0.0/10, so an operator + // forwarding to a Tailscale peer lists exactly this. + name: "Alibaba metadata under allowlisted CGNAT", + allow: "100.64.0.0/10", + target: "http://100.100.100.200/latest/meta-data/", + }, + { + // Inside the already-blocked 192.0.0.0/24, so only + // an allowlist can reach it — and must not. + name: "Oracle Cloud Classic metadata under 0.0.0.0/0", + allow: allowAllIPv4, + target: "http://192.0.0.192/latest/meta-data/", + }, + { + name: "Azure WireServer under 0.0.0.0/0", + allow: allowAllIPv4, + target: "http://168.63.129.16/machine/", + }, + { + name: "Equinix Metal metadata under 0.0.0.0/0", + allow: allowAllIPv4, + target: "http://147.75.207.243/metadata", + }, + } +} + +// encodedMetadataRefusedCases covers the alternate IPv6 +// encodings of 169.254.169.254. +func encodedMetadataRefusedCases() []metadataAlwaysRefusedCase { + return []metadataAlwaysRefusedCase{ + { + // To4() does not normalise the IPv4-compatible form, + // so this needs its own always-blocked entry. + name: "IPv4-compatible IPv6 form of the metadata IP", + allow: allowAllIPv6, + target: "http://[::a9fe:a9fe]/latest/meta-data/", + }, + { + // Nor the NAT64 well-known prefix form. + name: "NAT64 form of the metadata IP", + allow: allowAllIPv6, + target: "http://[64:ff9b::a9fe:a9fe]/latest/meta-data/", + }, + { + // Already refused before this change: IPNet.Contains + // calls To4() first, so the mapped form matches + // 169.254.0.0/16. Pinned so it cannot regress. + // + // Allowed under 0.0.0.0/0 rather than ::/0: allows() + // unmaps before matching, so ::/0 would not cover the + // unmapped v4 address and the case would not prove + // the allowlist was overridden. + name: "IPv4-mapped IPv6 form of the metadata IP", + allow: allowAllIPv4, + target: "http://[::ffff:169.254.169.254]/latest/meta-data/", + }, + } +} + +// TestGuardAllowlist_PublicUnaffected asserts the allowlist does +// not narrow anything: public addresses were reachable before it +// existed and stay reachable, whether or not a list is set. +func TestGuardAllowlist_PublicUnaffected(t *testing.T) { + t.Parallel() + + guards := map[string]*delivery.Guard{ + "default": delivery.NewTestGuard(), + "with allowlist": delivery.NewTestGuard( + netip.MustParsePrefix("10.0.0.0/8"), + ), + } + + for name, guard := range guards { + t.Run(name, func(t *testing.T) { + t.Parallel() + + assert.NoError(t, + guard.ValidateTargetURL( + context.Background(), + "http://"+publicIP+"/webhook", + ), + ) + }) + } +} + +// TestGuardCheckIP_BothPathsShareOneDecision asserts that the +// validator and the dialer are not two policies that happen to +// agree: both are defined in terms of checkIP, so the exported +// decision function is the whole answer for a given address. +func TestGuardCheckIP_BothPathsShareOneDecision(t *testing.T) { + t.Parallel() + + guard := delivery.NewTestGuard( + netip.MustParsePrefix("10.0.0.0/8"), + ) + + tests := []struct { + ip string + allowed bool + }{ + {"10.1.2.3", true}, + {publicIP, true}, + {"192.168.1.1", false}, + {"127.0.0.1", false}, + {metadataIP, false}, + } + + for _, tt := range tests { + t.Run(tt.ip, func(t *testing.T) { + t.Parallel() + + ip := net.ParseIP(tt.ip) + require.NotNil(t, ip) + + decision := guard.ExportCheckIP(ip) + + validation := guard.ValidateTargetURL( + context.Background(), "http://"+hostFor(tt.ip)+"/x", + ) + + if tt.allowed { + require.NoError(t, decision) + require.NoError(t, validation) + + return + } + + require.Error(t, decision) + require.Error(t, validation, + "validation must refuse what checkIP refuses", + ) + }) + } +} + +// TestAlwaysBlockedNetworks_PinnedSet pins the unconditional set +// exactly, so it cannot quietly grow or shrink. +// +// It stays deliberately small. Everything else in the default +// blocklist is an operator's own network and must remain +// openable, or the escape hatch would not work — which is why +// the metadata endpoints outside the link-local range are host +// routes rather than the blocks that contain them. +func TestAlwaysBlockedNetworks_PinnedSet(t *testing.T) { + t.Parallel() + + nets := delivery.ExportAlwaysBlockedNetworks() + + got := make([]string, 0, len(nets)) + for _, n := range nets { + got = append(got, n.String()) + } + + want := []string{ + // IPv4 link-local: the 169.254.169.254 metadata + // service on AWS, Azure and others. + "169.254.0.0/16", + // IPv6 link-local. + "fe80::/10", + // AWS IPv6 IMDS, inside the ULA space an operator may + // legitimately allowlist. + "fd00:ec2::254/128", + // AWS EKS Pod Identity Agent, likewise ULA. + "fd00:ec2::23/128", + // GCP metadata for IPv6-only instances, likewise ULA. + "fd20:ce::254/128", + // Oracle OCI IMDS over IPv6, likewise ULA. + "fd00:c1::a9fe:a9fe/128", + // Scaleway metadata over IPv6, likewise ULA. + "fd00:42::42/128", + // Linode/Akamai metadata over IPv6, likewise ULA. + "fd00:a9fe:a9fe::1/128", + // Alibaba Cloud metadata, inside CGNAT. + "100.100.100.200/32", + // Oracle Cloud Classic metadata, inside the blocked + // 192.0.0.0/24. + "192.0.0.192/32", + // Azure WireServer, public unicast. + "168.63.129.16/32", + // Equinix Metal metadata, public unicast. + "147.75.207.243/32", + // 169.254.169.254 as an IPv4-compatible IPv6 address. + "::a9fe:a9fe/128", + // 169.254.169.254 behind the NAT64 well-known prefix. + "64:ff9b::a9fe:a9fe/128", + } + + assert.Equal(t, want, got) +} + +// requireLoopback fails the test unless rawURL's host is a +// loopback address, so the allowlist test cannot silently stop +// exercising a blocked range. +func requireLoopback(t *testing.T, rawURL string) { + t.Helper() + + parsed, err := url.Parse(rawURL) + require.NoError(t, err) + + ip := net.ParseIP(parsed.Hostname()) + require.NotNil(t, ip, "test server host must be an IP literal") + require.True(t, ip.IsLoopback(), + "test server must listen on loopback, got %s", ip, + ) +} + +// assertDialRefused asserts the guard's transport refuses to +// connect to target, which is the delivery-time half of the +// policy. It never reaches the network: the guard checks the +// resolved address before dialling. +func assertDialRefused( + t *testing.T, guard *delivery.Guard, target string, +) { + t.Helper() + + assertDialRefusedWith(t, guard, target, "blocked") +} + +// assertDialRefusedWith is assertDialRefused with the refusal +// text pinned. Callers testing the unconditional set pass +// metadataRefusalClause so the subtest cannot pass on an +// ordinary blocklist refusal instead. +func assertDialRefusedWith( + t *testing.T, guard *delivery.Guard, target, clause string, +) { + t.Helper() + + client := &http.Client{ + Timeout: 5 * time.Second, + Transport: guard.NewSSRFSafeTransport(), + } + + req, err := http.NewRequestWithContext( + context.Background(), http.MethodPost, target, nil, + ) + require.NoError(t, err) + + resp, err := client.Do(req) + if resp != nil { + _ = resp.Body.Close() + } + + require.Error(t, err, + "delivery to %s must be refused by the dialer", target, + ) + assert.Contains(t, err.Error(), clause, + "the refusal must come from the SSRF guard", + ) +} + +// hostFor renders an IP as it appears in a URL host, bracketing +// IPv6 literals. +func hostFor(ip string) string { + if net.ParseIP(ip).To4() == nil { + return "[" + ip + "]" + } + + return ip +} diff --git a/internal/delivery/ssrf_test.go b/internal/delivery/ssrf_test.go index d919d16..14454e9 100644 --- a/internal/delivery/ssrf_test.go +++ b/internal/delivery/ssrf_test.go @@ -31,10 +31,10 @@ func TestIsBlockedIP_PrivateRanges(t *testing.T) { {"192.168.0.1", "192.168.0.1", true}, {"192.168.255.255", "192.168.255.255", true}, {"169.254.0.1", "169.254.0.1", true}, - {"169.254.169.254", "169.254.169.254", true}, + {metadataIP, metadataIP, true}, {"8.8.8.8", "8.8.8.8", false}, {"1.1.1.1", "1.1.1.1", false}, - {"93.184.216.34", "93.184.216.34", false}, + {publicIP, publicIP, false}, {"::1", "::1", true}, {"fd00::1", "fd00::1", true}, {"fc00::1", "fc00::1", true}, @@ -72,12 +72,12 @@ func TestValidateTargetURL_Blocked(t *testing.T) { t.Parallel() blockedURLs := []string{ - "http://127.0.0.1/hook", + loopbackHookURL, "http://127.0.0.1:8080/hook", "https://10.0.0.1/hook", "http://192.168.1.1/webhook", "http://172.16.0.1/api", - "http://169.254.169.254/latest/meta-data/", + metadataURL, "http://[::1]/hook", "http://[fc00::1]/hook", "http://[fe80::1]/hook", @@ -88,7 +88,7 @@ func TestValidateTargetURL_Blocked(t *testing.T) { t.Run(u, func(t *testing.T) { t.Parallel() - err := delivery.ValidateTargetURL( + err := delivery.NewTestGuard().ValidateTargetURL( context.Background(), u, ) @@ -112,7 +112,7 @@ func TestValidateTargetURL_Allowed(t *testing.T) { t.Run(u, func(t *testing.T) { t.Parallel() - err := delivery.ValidateTargetURL( + err := delivery.NewTestGuard().ValidateTargetURL( context.Background(), u, ) @@ -126,7 +126,7 @@ func TestValidateTargetURL_Allowed(t *testing.T) { func TestValidateTargetURL_InvalidScheme(t *testing.T) { t.Parallel() - err := delivery.ValidateTargetURL( + err := delivery.NewTestGuard().ValidateTargetURL( context.Background(), "ftp://example.com/hook", ) @@ -140,7 +140,7 @@ func TestValidateTargetURL_InvalidScheme(t *testing.T) { func TestValidateTargetURL_EmptyHost(t *testing.T) { t.Parallel() - err := delivery.ValidateTargetURL( + err := delivery.NewTestGuard().ValidateTargetURL( context.Background(), "http:///path", ) @@ -150,7 +150,7 @@ func TestValidateTargetURL_EmptyHost(t *testing.T) { func TestValidateTargetURL_InvalidURL(t *testing.T) { t.Parallel() - err := delivery.ValidateTargetURL( + err := delivery.NewTestGuard().ValidateTargetURL( context.Background(), "://invalid", ) diff --git a/internal/delivery/url_mask_test.go b/internal/delivery/url_mask_test.go index e6cc152..f699b36 100644 --- a/internal/delivery/url_mask_test.go +++ b/internal/delivery/url_mask_test.go @@ -185,7 +185,7 @@ func TestDoHTTPRequest_TransportErrorMasksURL(t *testing.T) { func TestValidateTargetURL_UnparsableURLIsMasked(t *testing.T) { t.Parallel() - err := delivery.ValidateTargetURL( + err := delivery.NewTestGuard().ValidateTargetURL( context.TODO(), "https://hooks.slack.com"+maskSecretPath+"\n", ) diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index 35efb1d..80aed4b 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -60,6 +60,7 @@ type HandlersParams struct { Middleware *middleware.Middleware Notifier delivery.Notifier Evictor delivery.WebhookEvictor + SSRFGuard *delivery.Guard } // Handlers provides HTTP handler methods for all application @@ -77,6 +78,11 @@ type Handlers struct { mtr *metrics.Set templates map[string]*template.Template + // ssrf validates submitted target URLs. It is the same guard + // the delivery engine dials through, so a URL accepted here + // is one delivery will actually attempt. + ssrf *delivery.Guard + // dummyVerifications counts the equivalent-cost verifications // charged for usernames that do not exist. It exists so a test // can prove that path runs without measuring wall-clock time. @@ -117,6 +123,7 @@ func New( s.notifier = params.Notifier s.evictor = params.Evictor s.mtr = metrics.Default() + s.ssrf = params.SSRFGuard // Parse all page templates once at startup s.templates = map[string]*template.Template{ diff --git a/internal/handlers/handlers_test.go b/internal/handlers/handlers_test.go index 2c1ab0b..3e9874a 100644 --- a/internal/handlers/handlers_test.go +++ b/internal/handlers/handlers_test.go @@ -110,6 +110,7 @@ func newTestApp( return r }, middleware.New, + delivery.NewGuard, handlers.New, ), fx.Populate(targets...), diff --git a/internal/handlers/source_management.go b/internal/handlers/source_management.go index 581e218..1cccbc2 100644 --- a/internal/handlers/source_management.go +++ b/internal/handlers/source_management.go @@ -1411,7 +1411,7 @@ func (h *Handlers) validateTargetURL( return errMissingURL } - err := delivery.ValidateTargetURL( + err := h.ssrf.ValidateTargetURL( r.Context(), targetURL, ) if err != nil { diff --git a/internal/resetpw/resetpw_test.go b/internal/resetpw/resetpw_test.go index 448ff1d..cca9107 100644 --- a/internal/resetpw/resetpw_test.go +++ b/internal/resetpw/resetpw_test.go @@ -164,6 +164,7 @@ func newServerApp( func() delivery.Notifier { return &noopNotifier{} }, func() delivery.WebhookEvictor { return &noopEvictor{} }, middleware.New, + delivery.NewGuard, handlers.New, ), fx.Populate(&h), diff --git a/internal/server/routes_test.go b/internal/server/routes_test.go index e8212ec..97937ee 100644 --- a/internal/server/routes_test.go +++ b/internal/server/routes_test.go @@ -113,6 +113,7 @@ func newTestEnvWithConfig( func() delivery.Notifier { return &noopNotifier{} }, func() delivery.WebhookEvictor { return &noopEvictor{} }, middleware.New, + delivery.NewGuard, handlers.New, ), fx.Populate(&log, &mw, &hnd, &sess, &db, &dbMgr),