sec(httpclient): resolve hostname before IMDS-block check (closes DNS-bypass gap)

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/azcore HTTP 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.

Reviewed commit: be11bdcb5. Note: origin/main moved to 3e9660d06 during the review; re-verify against current main before 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-41
providers/azure/internal/httpclient/httpclient.go:26-35

The same defect is now in pkg/httpclient/httpclient.go:41-50, which is the shared implementation that backs the Azure service clients in providers/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:

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/azcore HTTP pipeline does not follow redirects by default...

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.internal is a stable, publicly documented name resolving to 169.254.169.254, so

http://metadata.google.internal/computeMetadata/v1/instance/service-accounts/default/token

passes 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:

return d.inner.DialContext(ctx, network, 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) and 169.254.170.23 / fd00:ec2::23 (EKS Pod Identity).

pkg/httpclient: extend SSRF block-list (loopback, RFC 1918, all link-local, hostname/DNS resolution) cloud-commitments-go#20 (pkg/httpclient: extend SSRF block-list) is the P1 sibling; its fix sketch already contains the resolve-then-dial-the-literal step.
chore(secrets): extract IMDS-blocking transport into shared pkg/httpclient cloud-commitments-go#25 (extract the IMDS transport into the shared package) removes the need to apply this fix in three places.

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.

Navigation Menu

Repository navigation

sec(httpclient): resolve hostname before IMDS-block check (closes DNS-bypass gap) #1324

Severity / Confidence

Affected files

Evidence

Recommendation

Triage labels

Where the scope has moved

Why the severity assessment no longer holds

One correction to the recommended patch

Related

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

sec(httpclient): resolve hostname before IMDS-block check (closes DNS-bypass gap) #1324

Description

Severity / Confidence

Affected files

Evidence

Recommendation

Triage labels

Activity

cristim commented on Jul 28, 2026

Where the scope has moved

Why the severity assessment no longer holds

One correction to the recommended patch

Related

cristim commented on Sep 2, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions