Skip to content

G
golang
Commit 7d61ad25 authored by Adam Langley's avatar Adam Langley Committed by Filippo Valsorda
Browse files
Options

crypto/x509: check EKUs like 1.9. 

This change brings back the EKU checking from 1.9. In 1.10, we checked
EKU nesting independent of the requested EKUs so that, after verifying a
certifciate, one could inspect the EKUs in the leaf and trust them.

That, however, was too optimistic. I had misunderstood that the PKI was
/currently/ clean enough to require that, rather than it being
desirable. Go generally does not push the envelope on these sorts of
things and lets the browsers clear the path first.

Fixes #24590

Change-Id: I18c070478e3bbb6468800ae461c207af9e954949
Reviewed-on: https://go-review.googlesource.com/113475Reviewed-by: 's avatarFilippo Valsorda <filippo@golang.org>
parent c1a06801
Hide whitespace changes
Inline Side-by-side
Showing with 121 additions and 162 deletions
src/crypto/x509/name_constraints_test.go
View file @ 7d61ad25
......@@ -1222,8 +1222,9 @@ var nameConstraintsTests = []nameConstraintsTest{
},
},
// #63: A specified key usage in an intermediate forbids other usages
// in the leaf.
// #63: An intermediate with enumerated EKUs causes a failure if we
// test for an EKU not in that set. (ServerAuth is required by
// default.)
nameConstraintsTest{
roots: []constraintsSpec{
constraintsSpec{},
......@@ -1239,11 +1240,11 @@ var nameConstraintsTests = []nameConstraintsTest{
sans: []string{"dns:example.com"},
ekus: []string{"serverAuth"},
},
expectedError: "EKU not permitted",
expectedError: "incompatible key usage",
},
// #64: A specified key usage in an intermediate forbids other usages
// in the leaf, even if we don't recognise them.
// #64: an unknown EKU in the leaf doesn't break anything, even if it's not
// correctly nested.
nameConstraintsTest{
roots: []constraintsSpec{
constraintsSpec{},
......@@ -1259,7 +1260,7 @@ var nameConstraintsTests = []nameConstraintsTest{
sans: []string{"dns:example.com"},
ekus: []string{"other"},
},
expectedError: "EKU not permitted",
requestedEKUs: []ExtKeyUsage{ExtKeyUsageAny},
},
// #65: trying to add extra permitted key usages in an intermediate
......@@ -1284,24 +1285,25 @@ var nameConstraintsTests = []nameConstraintsTest{
},
},
// #66: EKUs in roots are ignored.
// #66: EKUs in roots are not ignored.
nameConstraintsTest{
roots: []constraintsSpec{
constraintsSpec{
ekus: []string{"serverAuth"},
ekus: []string{"email"},
},
},
intermediates: [][]constraintsSpec{
[]constraintsSpec{
constraintsSpec{
ekus: []string{"serverAuth", "email"},
ekus: []string{"serverAuth"},
},
},
},
leaf: leafSpec{
sans: []string{"dns:example.com"},
ekus: []string{"serverAuth", "email"},
ekus: []string{"serverAuth"},
},
expectedError: "incompatible key usage",
},
// #67: in order to support COMODO chains, SGC key usages permit
......@@ -1447,8 +1449,7 @@ var nameConstraintsTests = []nameConstraintsTest{
expectedError: "\"https://example.com/test\" is excluded",
},
// #75: While serverAuth in a CA certificate permits clientAuth in a leaf,
// serverAuth in a leaf shouldn't permit clientAuth when requested in
// #75: serverAuth in a leaf shouldn't permit clientAuth when requested in
// VerifyOptions.
nameConstraintsTest{
roots: []constraintsSpec{
......@@ -1558,6 +1559,27 @@ var nameConstraintsTests = []nameConstraintsTest{
},
requestedEKUs: []ExtKeyUsage{ExtKeyUsageClientAuth, ExtKeyUsageEmailProtection},
},
// #81: EKUs that are not asserted in VerifyOpts are not required to be
// nested.
nameConstraintsTest{
roots: []constraintsSpec{
constraintsSpec{},
},
intermediates: [][]constraintsSpec{
[]constraintsSpec{
constraintsSpec{
ekus: []string{"serverAuth"},
},
},
},
leaf: leafSpec{
sans: []string{"dns:example.com"},
// There's no email EKU in the intermediate. This would be rejected if
// full nesting was required.
ekus: []string{"email", "serverAuth"},
},
},
}
func makeConstraintsCACert(constraints constraintsSpec, name string, key *ecdsa.PrivateKey, parent *Certificate, parentKey *ecdsa.PrivateKey) (*Certificate, error) {
......
src/crypto/x509/verify.go
View file @ 7d61ad25
......@@ -56,8 +56,7 @@ const (
// CPU time to verify.
TooManyConstraints
// CANotAuthorizedForExtKeyUsage results when an intermediate or root
// certificate does not permit an extended key usage that is claimed by
// the leaf certificate.
// certificate does not permit a requested extended key usage.
CANotAuthorizedForExtKeyUsage
)
......@@ -82,7 +81,7 @@ func (e CertificateInvalidError) Error() string {
case TooManyIntermediates:
return "x509: too many intermediates for path length constraint"
case IncompatibleUsage:
return "x509: certificate specifies an incompatible key usage: " + e.Detail
return "x509: certificate specifies an incompatible key usage"
case NameMismatch:
return "x509: issuer name does not match subject from issuing certificate"
case NameConstraintsWithoutSANs:
......@@ -185,9 +184,8 @@ type VerifyOptions struct {
// list means ExtKeyUsageServerAuth. To accept any key usage, include
// ExtKeyUsageAny.
//
// Certificate chains are required to nest extended key usage values,
// irrespective of this value. This matches the Windows CryptoAPI behavior,
// but not the spec.
// Certificate chains are required to nest these extended key usage values.
// (This matches the Windows CryptoAPI behavior, but not the spec.)
KeyUsages []ExtKeyUsage
// MaxConstraintComparisions is the maximum number of comparisons to
// perform when checking a given certificate's name constraints. If
......@@ -549,51 +547,6 @@ func (c *Certificate) checkNameConstraints(count *int,
return nil
}
const (
checkingAgainstIssuerCert = iota
checkingAgainstLeafCert
)
// ekuPermittedBy returns true iff the given extended key usage is permitted by
// the given EKU from a certificate. Normally, this would be a simple
// comparison plus a special case for the “any” EKU. But, in order to support
// existing certificates, some exceptions are made.
func ekuPermittedBy(eku, certEKU ExtKeyUsage, context int) bool {
if certEKU == ExtKeyUsageAny || eku == certEKU {
return true
}
// Some exceptions are made to support existing certificates. Firstly,
// the ServerAuth and SGC EKUs are treated as a group.
mapServerAuthEKUs := func(eku ExtKeyUsage) ExtKeyUsage {
if eku == ExtKeyUsageNetscapeServerGatedCrypto || eku == ExtKeyUsageMicrosoftServerGatedCrypto {
return ExtKeyUsageServerAuth
}
return eku
}
eku = mapServerAuthEKUs(eku)
certEKU = mapServerAuthEKUs(certEKU)
if eku == certEKU {
return true
}
// If checking a requested EKU against the list in a leaf certificate there
// are fewer exceptions.
if context == checkingAgainstLeafCert {
return false
}
// ServerAuth in a CA permits ClientAuth in the leaf.
return (eku == ExtKeyUsageClientAuth && certEKU == ExtKeyUsageServerAuth) ||
// Any CA may issue an OCSP responder certificate.
eku == ExtKeyUsageOCSPSigning ||
// Code-signing CAs can use Microsoft's commercial and
// kernel-mode EKUs.
(eku == ExtKeyUsageMicrosoftCommercialCodeSigning || eku == ExtKeyUsageMicrosoftKernelCodeSigning) && certEKU == ExtKeyUsageCodeSigning
}
// isValid performs validity checks on c given that it is a candidate to append
// to the chain in currentChain.
func (c *Certificate) isValid(certType int, currentChain []*Certificate, opts *VerifyOptions) error {
......@@ -708,59 +661,6 @@ func (c *Certificate) isValid(certType int, currentChain []*Certificate, opts *V
}
}
checkEKUs := certType == intermediateCertificate
// If no extended key usages are specified, then all are acceptable.
if checkEKUs && (len(c.ExtKeyUsage) == 0 && len(c.UnknownExtKeyUsage) == 0) {
checkEKUs = false
}
// If the “any” key usage is permitted, then no more checks are needed.
if checkEKUs {
for _, caEKU := range c.ExtKeyUsage {
comparisonCount++
if caEKU == ExtKeyUsageAny {
checkEKUs = false
break
}
}
}
if checkEKUs {
NextEKU:
for _, eku := range leaf.ExtKeyUsage {
if comparisonCount > maxConstraintComparisons {
return CertificateInvalidError{c, TooManyConstraints, ""}
}
for _, caEKU := range c.ExtKeyUsage {
comparisonCount++
if ekuPermittedBy(eku, caEKU, checkingAgainstIssuerCert) {
continue NextEKU
}
}
oid, _ := oidFromExtKeyUsage(eku)
return CertificateInvalidError{c, CANotAuthorizedForExtKeyUsage, fmt.Sprintf("EKU not permitted: %#v", oid)}
}
NextUnknownEKU:
for _, eku := range leaf.UnknownExtKeyUsage {
if comparisonCount > maxConstraintComparisons {
return CertificateInvalidError{c, TooManyConstraints, ""}
}
for _, caEKU := range c.UnknownExtKeyUsage {
comparisonCount++
if caEKU.Equal(eku) {
continue NextUnknownEKU
}
}
return CertificateInvalidError{c, CANotAuthorizedForExtKeyUsage, fmt.Sprintf("EKU not permitted: %#v", eku)}
}
}
// KeyUsage status flags are ignored. From Engineering Security, Peter
// Gutmann: A European government CA marked its signing certificates as
// being valid for encryption only, but no-one noticed. Another
......@@ -861,63 +761,38 @@ func (c *Certificate) Verify(opts VerifyOptions) (chains [][]*Certificate, err e
}
}
requestedKeyUsages := make([]ExtKeyUsage, len(opts.KeyUsages))
copy(requestedKeyUsages, opts.KeyUsages)
if len(requestedKeyUsages) == 0 {
requestedKeyUsages = append(requestedKeyUsages, ExtKeyUsageServerAuth)
var candidateChains [][]*Certificate
if opts.Roots.contains(c) {
candidateChains = append(candidateChains, []*Certificate{c})
} else {
if candidateChains, err = c.buildChains(make(map[int][][]*Certificate), []*Certificate{c}, &opts); err != nil {
return nil, err
}
}
// If no key usages are specified, then any are acceptable.
checkEKU := len(c.ExtKeyUsage) > 0
for _, eku := range requestedKeyUsages {
if eku == ExtKeyUsageAny {
checkEKU = false
break
}
keyUsages := opts.KeyUsages
if len(keyUsages) == 0 {
keyUsages = []ExtKeyUsage{ExtKeyUsageServerAuth}
}
if checkEKU {
foundMatch := false
NextUsage:
for _, eku := range requestedKeyUsages {
for _, leafEKU := range c.ExtKeyUsage {
if ekuPermittedBy(eku, leafEKU, checkingAgainstLeafCert) {
foundMatch = true
break NextUsage
}
}
// If any key usage is acceptable then we're done.
for _, usage := range keyUsages {
if usage == ExtKeyUsageAny {
return candidateChains, nil
}
}
if !foundMatch {
msg := "leaf contains the following, recognized EKUs: "
for i, leafEKU := range c.ExtKeyUsage {
oid, ok := oidFromExtKeyUsage(leafEKU)
if !ok {
continue
}
if i > 0 {
msg += ", "
}
msg += formatOID(oid)
}
return nil, CertificateInvalidError{c, IncompatibleUsage, msg}
for _, candidate := range candidateChains {
if checkChainForKeyUsage(candidate, keyUsages) {
chains = append(chains, candidate)
}
}
var candidateChains [][]*Certificate
if opts.Roots.contains(c) {
candidateChains = append(candidateChains, []*Certificate{c})
} else {
if candidateChains, err = c.buildChains(make(map[int][][]*Certificate), []*Certificate{c}, &opts); err != nil {
return nil, err
}
if len(chains) == 0 {
return nil, CertificateInvalidError{c, IncompatibleUsage, ""}
}
return candidateChains, nil
return chains, nil
}
func appendToFreshChain(chain []*Certificate, cert *Certificate) []*Certificate {
......@@ -1078,3 +953,65 @@ func (c *Certificate) VerifyHostname(h string) error {
return HostnameError{c, h}
}
func checkChainForKeyUsage(chain []*Certificate, keyUsages []ExtKeyUsage) bool {
usages := make([]ExtKeyUsage, len(keyUsages))
copy(usages, keyUsages)
if len(chain) == 0 {
return false
}
usagesRemaining := len(usages)
// We walk down the list and cross out any usages that aren't supported
// by each certificate. If we cross out all the usages, then the chain
// is unacceptable.
NextCert:
for i := len(chain) - 1; i >= 0; i-- {
cert := chain[i]
if len(cert.ExtKeyUsage) == 0 && len(cert.UnknownExtKeyUsage) == 0 {
// The certificate doesn't have any extended key usage specified.
continue
}
for _, usage := range cert.ExtKeyUsage {
if usage == ExtKeyUsageAny {
// The certificate is explicitly good for any usage.
continue NextCert
}
}
const invalidUsage ExtKeyUsage = -1
NextRequestedUsage:
for i, requestedUsage := range usages {
if requestedUsage == invalidUsage {
continue
}
for _, usage := range cert.ExtKeyUsage {
if requestedUsage == usage {
continue NextRequestedUsage
} else if requestedUsage == ExtKeyUsageServerAuth &&
(usage == ExtKeyUsageNetscapeServerGatedCrypto ||
usage == ExtKeyUsageMicrosoftServerGatedCrypto) {
// In order to support COMODO
// certificate chains, we have to
// accept Netscape or Microsoft SGC
// usages as equal to ServerAuth.
continue NextRequestedUsage
}
}
usages[i] = invalidUsage
usagesRemaining--
if usagesRemaining == 0 {
return false
}
}
}
return true
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment