Repository navigation
sec(httpclient): resolve hostname before IMDS-block check (closes DNS-bypass gap) #1324
Description
Severity / Confidence
P3, medium confidence. Defense-in-depth gap. Not exploitable in current code paths but worth closing.
Affected files
internal/secrets/azure_resolver.go:32-41(blockIMDSDialer.DialContext)providers/azure/internal/httpclient/httpclient.go:26-35(same pattern)
Evidence
Both IMDS-blocking dialers check host from net.SplitHostPort(addr) against a map of literal IPs (169.254.169.254, fd00:ec2::254). They do NOT resolve hostnames before checking.
net.Dialer.DialContext is invoked by http.Transport with addr as host:port, where host can be a hostname or IP literal. DNS resolution happens inside inner.DialContext. So if a request targets a hostname that resolves to an IMDS address (for example, the GCP convention metadata.google.internal -> 169.254.169.254, or an attacker-controlled DNS record), the application-layer check at internal/secrets/azure_resolver.go:37 does not fire.
In current code the practical risk is low:
- The Azure Key Vault URL comes from env var
AZURE_KEY_VAULT_URL; attacker control of that env var is already an operational compromise. - The
azsecrets/azcoreHTTP pipeline does not follow redirects by default, so a malicious redirect response cannot pivot to an arbitrary host.
But this is the SSRF defense the memory feedback_azure_use_httpclient_new.md says is the whole point of httpclient.New(), and it currently only catches the literal-IP form of the attack.
Recommendation
Resolve the host before the IMDS check, e.g.:
func (d *blockIMDSDialer) DialContext(ctx context.Context, network, addr string) (net.Conn, error) {
host, _, err := net.SplitHostPort(addr)
if err != nil {
host = addr
}
ips, err := d.resolver.LookupIPAddr(ctx, host) // d.resolver = net.DefaultResolver if nil
if err != nil {
return nil, err
}
for _, ip := range ips {
if imdsAddresses[ip.IP.String()] {
return nil, fmt.Errorf("connection to metadata endpoint %s (via %s) is blocked", ip.IP, host)
}
}
return d.inner.DialContext(ctx, network, addr)
}Add a regression test that uses a net.Resolver whose Dial returns a stub DNS response mapping metadata.google.internal -> 169.254.169.254, and asserts the block fires.
Best landed together with the shared-package refactor in the duplication follow-up so both internal/secrets/azure_resolver.go and providers/azure/internal/httpclient/httpclient.go get the hardening at once.
Triage labels
type/security, priority/p3, severity/low, urgency/eventually, impact/internal, effort/s, triaged.
Source: PR #1224 adversarial review.
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 mechanism independently. The mechanism described here is correct; the scope and the severity assessment are now out of date and, left as-is, will cause this to be picked up as a small defence-in-depth chore.
Where the scope has moved
This issue lists only:
internal/secrets/azure_resolver.go:32-41providers/azure/internal/httpclient/httpclient.go:26-35The same defect is now in
pkg/httpclient/httpclient.go:41-50, which is the shared implementation that backs the Azure service clients inproviders/azure/services/*/client.go, the Key Vault resolver behind the credential encryption key (internal/credentials/cipher.go:106), and the scheduled-task JWKS fetch. The package header states it is "the single shared implementation".Why the severity assessment no longer holds
The issue currently argues low practical risk on two grounds:
Both are specific to the Key Vault resolver. They do not hold for
pkg/httpclient, whose callers include request-tainted URL paths. And the bypass needs neither an attacker-controlled DNS record nor a redirect:metadata.google.internalis a stable, publicly documented name resolving to169.254.169.254, sopasses the check and returns a service-account token. That is a straightforward credential-exfiltration primitive, not a rebinding race.
Current labels are
priority/p3,severity/low,urgency/eventually,impact/internal. Suggest re-triaging in line with LeanerCloud/cloud-commitments-go#20 (priority/p1,severity/high).One correction to the recommended patch
The snippet in this issue resolves the host, checks the resolved IPs, and then dials the original
addr:That reintroduces a rebinding window: the resolver call and the dial each perform their own lookup, so a short-TTL record can answer benignly for the check and with the metadata address for the connect. Dial the validated IP literal instead, and validate against CIDR ranges rather than the two-entry map.
Also missing from the map in every copy:
169.254.170.2(ECS task metadata, container credentials) and169.254.170.23/fd00:ec2::23(EKS Pod Identity).Related
pkg/httpclient: extend SSRF block-list) is the P1 sibling; its fix sketch already contains the resolve-then-dial-the-literal step.cristim commentedon Sep 2, 2026
Duplicate of LeanerCloud/cloud-commitments-go#48: that issue covers the same hostname-not-resolved-IP gap in the same blockIMDSDialer.DialContext (pkg/httpclient/httpclient.go:41 and the internal/secrets copy), re-frames it at severity/high P1 instead of P3 defence-in-depth, and its fix closes this one.