Repository navigation
pkg/httpclient: extend SSRF block-list (loopback, RFC 1918, all link-local, hostname/DNS resolution) #20
Description
Context
Spun out of the adversarial review of PR LeanerCloud/cloud-commitments-cli#1231 (SEC-04, hardened HTTP client for JWKS warmup and fetch). The PR moves the existing Azure IMDS-blocking implementation into pkg/httpclient and applies it to the scheduled-task OIDC validator. PR LeanerCloud/cloud-commitments-cli#1231 also picked up a parity fix (canonical/expanded IPv6 form comparison via net.IP.Equal). The gaps below are larger and were carved out as follow-ups.
Gaps in the current block-list
pkg/httpclient.blockIMDSDialer.DialContext only rejects the dial when the literal IP in the URL parses to the AWS IMDS IPv4 or IPv6 endpoint. Several SSRF-adjacent paths are still reachable:
- Loopback (
127.0.0.0/8,::1): a JWKS URL pointed athttp://127.0.0.1:6379/orhttp://localhost:8080/reaches whatever loopback services run on the same host (Redis, Prometheus, sidecar admin endpoints). - RFC 1918 private space (
10/8,172.16/12,192.168/16): intranet pivot. - All link-local (
169.254.0.0/16): only169.254.169.254is blocked. Sibling link-local addresses are also reachable. - Other IPv6 link-local (
fe80::/10): not blocked. - Hostname-based SSRF: a URL whose hostname is
metadata.google.internal,instance-data.ec2.internal, etc., or a hostile DNS record that resolves to a metadata IP, bypasses the block-list. The current dialer only inspects the literal host string offnet.SplitHostPortand does not look at the resolved IP.
(1) through (4) follow the standard SSRF deny-list pattern.
(5) requires resolving the hostname before the connect, validating the resolved IP against the deny-ranges, and dialing the specific IP (so a DNS-rebinding attack cannot swap the IP between resolution and connect).
Fix sketch
In pkg/httpclient:
- Replace the IMDS list with a set of denied
net.IPNetranges covering link-local, loopback, RFC 1918, multicast, unspecified, and IPv6 site-local/ULA-metadata. - Wrap
DialContextso it pre-resolves the host (vianet.DefaultResolver.LookupNetIPwith the dial ctx), rejects any resolved IP that falls in a denied range, and dials the chosen IP literal (preventing DNS rebinding between resolution and connect). - Keep a per-call test seam (resolver injection) so the deny-range behavior can be asserted in unit tests without going through real DNS.
Test coverage
- Each denied range gets its own positive test (
127.0.0.1,127.0.0.2,10.0.0.1,192.168.1.1,169.254.1.1,fe80::1,::1). - Each denied range gets a hostname-shaped variant via a stub resolver that returns the denied IP.
- Negative tests for non-denied public IPs and hostnames.
Closing-issue trailer for the fix PR
Closes #<this issue number>
Findings from the 2026-09-02 codebase audit
Added by an automated audit of 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd (tip of origin/main). Each item below was reported by one reviewer and independently confirmed by a second that did not write it. Full report: docs/audits/codebase-audit-2026-09-02.md.
A08b-002 (high)
Two addresses to add to the deny-set in this issue, from finding A08b-002. The ranges listed here (loopback, RFC 1918, 169.254.0.0/16, fe80::/10) do not cover Azure's WireServer / host-agent endpoint 168.63.129.16, which serves instance configuration and extension settings and is a plain routable address, nor Alibaba's 100.100.100.200. http://168.63.129.16/machine?comp=goalstate is reachable today from every Azure service client, all of which carry a managed-identity token. Worth adding both as explicit single-address entries alongside the CIDR set, since a range-only deny-list will not catch them.
Activity
cristim commentedon Jul 28, 2026
Reviewed commit:
be11bdcb5. Note:origin/mainmoved to3e9660d06during the review; re-verify against currentmainbefore changing code, since a finding may have been fixed or moved.The 2026-07-28 full-repo review re-derived this independently and reached a stronger framing than the issue title carries. Adding it here so the fix is not scoped down to the deny-list.
Where
pkg/httpclient/httpclient.go:41-50(blockIMDSDialer.DialContext)pkg/httpclient/httpclient.go:31-34(imdsAddresses)pkg/httpclient/httpclient.go:32(the GCP claim)What the review adds
Gap (5) in this issue is not the fifth of five; it is the reason gaps (1) through (4) are mostly unreachable.
http.TransporthandsDialContextthehost:porttaken from the URL, and resolution happens insideinner.DialContexton line 49, after the map lookup on line 46. For any URL whose host is a name rather than a literal,imdsAddresses[host]compares a hostname against IP-literal keys and can never match. The guard is therefore inert on the common case, not merely incomplete.The practical consequence for whoever picks this up: landing gaps (1) through (4) alone (replacing the map with
net.IPNetdeny-ranges) does not close this issue. Only the resolve-then-validate-then-dial-the-literal-IP part of the fix sketch actually binds the check to the address dialled. Worth reflecting in the title, which currently reads as a list-extension task.No DNS rebinding is required for the cheapest exploit.
metadata.google.internalis a stable, documented name that resolves to169.254.169.254:Two AWS credential endpoints are missing from the enumeration here as well, and they are the ones that serve credentials rather than instance identity:
169.254.170.2(ECS task metadata, container credentials)169.254.170.23andfd00:ec2::23(EKS Pod Identity)The in-code comment claims coverage the code does not have, and will mislead the next reader:
GCP's documented metadata access is by name, so "GCP" is false for every request GCP's own documentation tells you to make. Please correct or delete that comment as part of the fix.
Failure scenario
Any request-tainted URL reaching a
httpclient.New()-backed caller (the Azure service clients inproviders/azure/services/*/client.go, the Key Vault resolver behindinternal/credentials/cipher.go:106, the scheduled-task JWKS fetch) returns a managed-identity access token or ECS container credentials to the attacker. That is the outcome the package header on lines 1-9 states the package exists to prevent.Related
severity/lowbut scoped tointernal/secrets/azure_resolver.goandproviders/azure/internal/httpclient/httpclient.go, not topkg/httpclient.internal/secretsduplicate) should land with or before this so the hostname fix does not have to be written twice.sec(httpclient): block Azure WireServer and Alibaba metadata addresses (
cristim commentedon Oct 5, 2026
PR #192 merged normally as 831acac: #192
The shared HTTP client now blocks Azure WireServer 168.63.129.16 and Alibaba metadata 100.100.100.200. Regression requests remain contained even when a production prefix is removed, so the tests cannot contact real metadata endpoints on that failure path.
Exact pre-merge head 5ac180b passed fresh independent gpt-6-astra review and macOS verification: focused race tests, all four deliberate failure probes, Azure constructor consumer checks, package race suite with zero skips, build, format, vet and lint. All required Linux CI checks passed before merge. Full scope and limitations: #192 (comment)
This proves native pre-connect rejection, not cloud acceptance or deployed Azure DNS behavior. The broader loopback/private-address policy remains unresolved under this issue; this merge does not close #20. Post-merge main pre-commit has passed; main build/test is being watched separately.