aboutsummaryrefslogtreecommitdiff
path: root/src/crypto
diff options
context:
space:
mode:
authorRoland Shoemaker <roland@golang.org>2025-05-06 09:27:10 -0700
committerRoland Shoemaker <roland@golang.org>2025-05-13 12:09:49 -0700
commit9bba799955e68972041c4f340ee4ea2d267e5c0e (patch)
tree383010f5b1a370f5922d96351875b64f980cd65a /src/crypto
parent76f63ee890170f4884f4d213e8150d39d6758ad3 (diff)
downloadgo-9bba799955e68972041c4f340ee4ea2d267e5c0e.tar.xz
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. Fixes #73612 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>
Diffstat (limited to 'src/crypto')
-rw-r--r--src/crypto/x509/verify.go32
-rw-r--r--src/crypto/x509/verify_test.go36
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)
+ }
+}