crypto/x509: usage of IPv6 zone IDs can bypass URI name constraints [CVE-2024-45341] [1.23 backport]

[gopherbot]

@neild requested issue #71156 to be considered for backport to the next 1.23 minor release.

@gopherbot please open backport issues for 1.22, 1.23, and 1.24

[gopherbot]

Change https://go.dev/cl/643103 mentions this issue: [release-branch.go1.23] crypto/x509: properly check for IPv6 hosts in URIs

[gopherbot]

Closed by merging CL 643103 (commit fdb8413) to release-branch.go1.23.

[RachidEddahaPrivate]

Hi everyone,

I believe the new condition:

if _, err := netip.ParseAddr(host); err == nil || (strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]")) {
    return false, fmt.Errorf("URI with IP (%q) cannot be matched against constraints", uri.String())
}

should use err != nil instead of err == nil.

Previously, we checked if net.ParseIP(host) != nil, which explicitly verified that the host was a valid IP. Now, using err == nil with netip.ParseAddr(host) changes the logic. This means that any successfully parsed address triggers the error, which seems incorrect.

Shouldn't the condition be:

if _, err := netip.ParseAddr(host); err != nil || (strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]")) {
    return false, fmt.Errorf("URI with IP (%q) cannot be matched against constraints", uri.String())
}

This would align more closely with the original intent. Let me know if I'm missing something!

[ianlancetaylor]

The earlier code tested net.ParseIP(host) != nil. The net.ParseIP function returns a net.IP value. It returns nil on error. So the test net.ParseIP(host) != nil is equivalent to saying "parsing host as an IP address succeeds".

The new code tests _, err := netip.ParseAddr(host); err == nil. The netip.ParseAddr function returns a nil error value if the parse succeeds. So the test err == nil is equivalent to saying "parsing host as an IP address succeeds".

So the code seems roughly equivalent. Am I missing something?

I don't really know what this code is doing, but the error message suggests that if the URI is an IP address, rather than, say, a domain name, the match of the URI against the constraint will fail. So it is correct to say that if the host can be parsed as an IP address, then the code should return an error.