summaryrefslogtreecommitdiff
path: root/1003-CVE-2025-22874-crypto-x509-decouple-key-usage-and-po.patch
blob: 9c01f02f58b2b76b8fc565aa780e386f363c2661 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
From 8cc22cc92b6941aaefe9c18b88662f5088228e92 Mon Sep 17 00:00:00 2001
From: Roland Shoemaker <roland@golang.org>
Date: Tue, 6 May 2025 09:27:10 -0700
Subject: [PATCH] crypto/x509: decouple key usage and policy validation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
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.

Fixes #73612
Fixes CVE-2025-22874

Confict: no
Reference:https://go-review.googlesource.com/c/go/+/670375

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>
Signed-off-by: jichao wu <wujichao1@huawei.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 5fe93c6..7cc0fb2 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 1175e7d..7991f49 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.33.0