src/crypto/x509/verify.go | 32 +++++++++++++++++++++++--------- src/crypto/x509/verify_test.go | 36 ++++++++++++++++++++++++++++++++++++ diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go index 5fe93c6124a989223708881a894359474e32c637..7cc0fb2e3e0385c65f607e0c42f11a466a77e7a4 100644 --- a/src/crypto/x509/verify.go +++ b/src/crypto/x509/verify.go @@ -841,31 +841,45 @@ return nil, err } } - 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 --- a/src/crypto/x509/verify_test.go +++ b/src/crypto/x509/verify_test.go @@ -3012,3 +3012,39 @@ } }) } } + +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) + } +}