From 03811ab1b31525e8d779997db169c6fedab7c505 Mon Sep 17 00:00:00 2001
From: Roland Shoemaker <roland@golang.org>
Date: Tue, 6 May 2025 09:27:10 -0700
Subject: [PATCH] [release-branch.go1.24] crypto/x509: decouple key usage and
 policy validation
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

Disabling key usage validation (by passing ExtKeyUsageAny)
unintentionally disabled policy validation. This change decouples these
two checks, preventing the user from unintentionally disabling policy
validation.

Thanks to Krzysztof Skrzętnicki (@Tener) of Teleport for reporting this
issue.

Updates #73612
Fixes #73700
Fixes CVE-2025-22874

Change-Id: Iec8f080a8879a3dd44cb3da30352fa3e7f539d40
Reviewed-on: https://go-review.googlesource.com/c/go/+/670375
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Ian Stapleton Cordasco <graffatcolmingov@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 9bba799955e68972041c4f340ee4ea2d267e5c0e)
Reviewed-on: https://go-review.googlesource.com/c/go/+/672316
Reviewed-by: Michael Knyszek <mknyszek@google.com>
---
 src/crypto/x509/verify.go      | 32 +++++++++++++++++++++---------
 src/crypto/x509/verify_test.go | 36 ++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go
index 5fe93c6124..7cc0fb2e3e 100644
--- a/src/crypto/x509/verify.go
+++ b/src/crypto/x509/verify.go
@@ -841,31 +841,45 @@ func (c *Certificate) Verify(opts VerifyOptions) (chains [][]*Certificate, err e
 		}
 	}
 
-	if len(opts.KeyUsages) == 0 {
-		opts.KeyUsages = []ExtKeyUsage{ExtKeyUsageServerAuth}
+	chains = make([][]*Certificate, 0, len(candidateChains))
+
+	var invalidPoliciesChains int
+	for _, candidate := range candidateChains {
+		if !policiesValid(candidate, opts) {
+			invalidPoliciesChains++
+			continue
+		}
+		chains = append(chains, candidate)
+	}
+
+	if len(chains) == 0 {
+		return nil, CertificateInvalidError{c, NoValidChains, "all candidate chains have invalid policies"}
 	}
 
 	for _, eku := range opts.KeyUsages {
 		if eku == ExtKeyUsageAny {
 			// If any key usage is acceptable, no need to check the chain for
 			// key usages.
-			return candidateChains, nil
+			return chains, nil
 		}
 	}
 
-	chains = make([][]*Certificate, 0, len(candidateChains))
-	var incompatibleKeyUsageChains, invalidPoliciesChains int
+	if len(opts.KeyUsages) == 0 {
+		opts.KeyUsages = []ExtKeyUsage{ExtKeyUsageServerAuth}
+	}
+
+	candidateChains = chains
+	chains = chains[:0]
+
+	var incompatibleKeyUsageChains int
 	for _, candidate := range candidateChains {
 		if !checkChainForKeyUsage(candidate, opts.KeyUsages) {
 			incompatibleKeyUsageChains++
 			continue
 		}
-		if !policiesValid(candidate, opts) {
-			invalidPoliciesChains++
-			continue
-		}
 		chains = append(chains, candidate)
 	}
+
 	if len(chains) == 0 {
 		var details []string
 		if incompatibleKeyUsageChains > 0 {
diff --git a/src/crypto/x509/verify_test.go b/src/crypto/x509/verify_test.go
index 1175e7d808..7991f49946 100644
--- a/src/crypto/x509/verify_test.go
+++ b/src/crypto/x509/verify_test.go
@@ -3012,3 +3012,39 @@ func TestPoliciesValid(t *testing.T) {
 		})
 	}
 }
+
+func TestInvalidPolicyWithAnyKeyUsage(t *testing.T) {
+	loadTestCert := func(t *testing.T, path string) *Certificate {
+		b, err := os.ReadFile(path)
+		if err != nil {
+			t.Fatal(err)
+		}
+		p, _ := pem.Decode(b)
+		c, err := ParseCertificate(p.Bytes)
+		if err != nil {
+			t.Fatal(err)
+		}
+		return c
+	}
+
+	testOID3 := mustNewOIDFromInts([]uint64{1, 2, 840, 113554, 4, 1, 72585, 2, 3})
+	root, intermediate, leaf := loadTestCert(t, "testdata/policy_root.pem"), loadTestCert(t, "testdata/policy_intermediate_require.pem"), loadTestCert(t, "testdata/policy_leaf.pem")
+
+	expectedErr := "x509: no valid chains built: all candidate chains have invalid policies"
+
+	roots, intermediates := NewCertPool(), NewCertPool()
+	roots.AddCert(root)
+	intermediates.AddCert(intermediate)
+
+	_, err := leaf.Verify(VerifyOptions{
+		Roots:               roots,
+		Intermediates:       intermediates,
+		KeyUsages:           []ExtKeyUsage{ExtKeyUsageAny},
+		CertificatePolicies: []OID{testOID3},
+	})
+	if err == nil {
+		t.Fatal("unexpected success, invalid policy shouldn't be bypassed by passing VerifyOptions.KeyUsages with ExtKeyUsageAny")
+	} else if err.Error() != expectedErr {
+		t.Fatalf("unexpected error, got %q, want %q", err, expectedErr)
+	}
+}
-- 
2.50.0