mirror of https://github.com/golang/go.git
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>
This commit is contained in:
parent
76f63ee890
commit
9bba799955
|
|
@ -841,31 +841,45 @@ func (c *Certificate) Verify(opts VerifyOptions) (chains [][]*Certificate, err e
|
|||
}
|
||||
}
|
||||
|
||||
if len(opts.KeyUsages) == 0 {
|
||||
opts.KeyUsages = []ExtKeyUsage{ExtKeyUsageServerAuth}
|
||||
}
|
||||
|
||||
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
|
||||
}
|
||||
}
|
||||
|
||||
chains = make([][]*Certificate, 0, len(candidateChains))
|
||||
var incompatibleKeyUsageChains, invalidPoliciesChains int
|
||||
|
||||
var invalidPoliciesChains 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 {
|
||||
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 chains, nil
|
||||
}
|
||||
}
|
||||
|
||||
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
|
||||
}
|
||||
chains = append(chains, candidate)
|
||||
}
|
||||
|
||||
if len(chains) == 0 {
|
||||
var details []string
|
||||
if incompatibleKeyUsageChains > 0 {
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue