From 2b2314e9f6103de322b2e247387c8b01fd0cd5a4 Mon Sep 17 00:00:00 2001
From: Roland Shoemaker <bracewell@google.com>
Date: Mon, 9 Dec 2024 11:31:22 -0800
Subject: [PATCH] crypto/x509: properly check for IPv6 hosts in URIs
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

When checking URI constraints, use netip.ParseAddr, which understands
zones, unlike net.ParseIP which chokes on them. This prevents zone IDs
from mistakenly satisfying URI constraints.

Thanks to Juho Forsén of Mattermost for reporting this issue.

Fixes #71156
Fixes CVE-2024-45341

Change-Id: Iecac2529f3605382d257996e0fb6d6983547e400
Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/1700
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/643096
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
---
 src/crypto/x509/name_constraints_test.go | 17 +++++++++++++++++
 src/crypto/x509/verify.go                |  7 +++++--
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/src/crypto/x509/name_constraints_test.go b/src/crypto/x509/name_constraints_test.go
index 008c7028f4..a585184516 100644
--- a/src/crypto/x509/name_constraints_test.go
+++ b/src/crypto/x509/name_constraints_test.go
@@ -1607,6 +1607,23 @@ var nameConstraintsTests = []nameConstraintsTest{
 		leaf:          leafSpec{sans: []string{"dns:.example.com"}},
 		expectedError: "cannot parse dnsName \".example.com\"",
 	},
+	// #86: URIs with IPv6 addresses with zones and ports are rejected
+	{
+		roots: []constraintsSpec{
+			{
+				ok: []string{"uri:example.com"},
+			},
+		},
+		intermediates: [][]constraintsSpec{
+			{
+				{},
+			},
+		},
+		leaf: leafSpec{
+			sans: []string{"uri:http://[2006:abcd::1%25.example.com]:16/"},
+		},
+		expectedError: "URI with IP",
+	},
 }
 
 func makeConstraintsCACert(constraints constraintsSpec, name string, key *ecdsa.PrivateKey, parent *Certificate, parentKey *ecdsa.PrivateKey) (*Certificate, error) {
diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go
index d2384f5665..5fe93c6124 100644
--- a/src/crypto/x509/verify.go
+++ b/src/crypto/x509/verify.go
@@ -13,6 +13,7 @@ import (
 	"iter"
 	"maps"
 	"net"
+	"net/netip"
 	"net/url"
 	"reflect"
 	"runtime"
@@ -465,8 +466,10 @@ func matchURIConstraint(uri *url.URL, constraint string) (bool, error) {
 		}
 	}
 
-	if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") ||
-		net.ParseIP(host) != nil {
+	// netip.ParseAddr will reject the URI IPv6 literal form "[...]", so we
+	// check if _either_ the string parses as an IP, or if it is enclosed in
+	// square brackets.
+	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())
 	}
 
-- 
2.48.1