[release-branch.go1.24] crypto/x509: decouple key usage and policy validation

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>

diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go
index 5fe93c6124a989223708881a894359474e32c637..7cc0fb2e3e0385c65f607e0c42f11a466a77e7a4 100644 (file)
--- 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 1175e7d80850d2104e0163f233afeb50e5859846..7991f49946d587b45360d86112b7291e3c8c31e9 100644 (file)
--- 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)
+       }
+}