Add support for private-network upstreams by exempting hosts from the SSRF dial gate - #51
Add support for private-network upstreams by exempting hosts from the SSRF dial gate#51Tilian wants to merge 2 commits into
Conversation
|
Context: We want to support a multitude of Maven upstreams, so we have a Reposilite instance sitting in between our proxy instance(s) and the actual upstreams. This now needs a public ingress to work. |
There was a problem hiding this comment.
Pull request overview
This PR introduces an explicit per-host allowlist to relax the SSRF dial gate for specific upstreams, enabling proxy deployments where upstream registries are only reachable via private network IPs (e.g., Kubernetes ClusterIP-backed services).
Changes:
- Added
WithAllowPrivateHosts(...)option to whitelist specific upstream hosts for relaxed SSRF dial gating. - Introduced
Fetcher.gateOptions(host)to selectsafehttpgate behavior per dial target. - Added unit test coverage for the new allowlist behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| fetch/fetcher.go | Adds host allowlist storage and per-host selection of safehttp.CheckIP options during dialing. |
| fetch/fetcher_test.go | Adds a unit test validating allowlist normalization and strict-by-default behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // WithAllowPrivateHosts exempts the named hosts from the dial gate's loopback and private-address checks. | ||
| func WithAllowPrivateHosts(hosts ...string) Option { | ||
| return func(f *Fetcher) { | ||
| if f.allowPrivate == nil { | ||
| f.allowPrivate = make(map[string]bool, len(hosts)) | ||
| } | ||
| for _, h := range hosts { | ||
| if h = strings.TrimSpace(h); h != "" { | ||
| f.allowPrivate[strings.ToLower(h)] = true | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // gateOptions returns the safehttp options for a dial to host. | ||
| // Zero-value strict gate unless the host was whitelisted via WithAllowPrivateHosts. | ||
| func (f *Fetcher) gateOptions(host string) safehttp.Options { | ||
| if f.allowPrivate[strings.ToLower(host)] { | ||
| return safehttp.Options{AllowLoopback: true, AllowPrivate: true} | ||
| } | ||
| return safehttp.Options{} | ||
| } |
| func (f *Fetcher) gateOptions(host string) safehttp.Options { | ||
| if f.allowPrivate[strings.ToLower(host)] { | ||
| return safehttp.Options{AllowLoopback: true, AllowPrivate: true} | ||
| } |
andrew
left a comment
There was a problem hiding this comment.
Thanks for this — the per-hostname allowlist is the right shape for the k8s-colocated upstream case, and keying the gate on the dial host keeps redirects to non-allowlisted private hosts blocked.
Two things before merge:
-
gateOptionssetsAllowLoopback: true. A k8s Service resolves to RFC1918 (10.x / 172.16.x / 192.168.x), never 127.0.0.1, soAllowPrivate: truealone covers the use case (plus ULA and CGNAT). With loopback enabled, an allowlisted hostname whose DNS is poisoned or misconfigured to 127.0.0.1 becomes dialable. Please dropAllowLoopbackfrom the exemption. Link-local (169.254.x.x, metadata endpoint) is already unconditionally blocked insafehttp.check, so that stays safe either way. -
safehttp.Options.AllowLoopback/AllowPrivateare documented as "Test-only; never set in production paths" (safehttp/safehttp.go:47-51). This PR now setsAllowPrivatein a production path, so please update those doc comments in the same PR (something like "for tests and explicit operator allowlists") so the next reader doesn't flag this call site as a bug.
Minor: an allowlist entry like registry.internal:8080 or svc.cluster.local. will never match because the dial-side host has already been through SplitHostPort. Stripping a trailing :port and trailing . when populating the map would stop a copy-pasted upstream host from silently failing to match.
Currently the
safehttpdial gate refuses loopback and link-local addresses on every upstream dial.When the proxy runs inside a private network next to its upstream (the Kubernetes use-case of pointing an upstream at a local registry service like
maven-mirror.internal.svc.cluster.local, which resolves to a cluster IP) the gate makes that upstream unreachable by design.Today the only workaround is going through a public ingress, which costs an extra network round-trip and forces an otherwise-internal registry to be externally exposed.