Commit 0681c7c3 authored by Adam Langley's avatar Adam Langley Committed by Filippo Valsorda

crypto/x509: tighten EKU checking for requested EKUs.

There are, sadly, many exceptions to EKU checking to reflect mistakes
that CAs have made in practice. However, the requirements for checking
requested EKUs against the leaf should be tighter than for checking leaf
EKUs against a CA.

Fixes #23884

Change-Id: I05ea874c4ada0696d8bb18cac4377c0b398fcb5e
Reviewed-on: https://go-review.googlesource.com/96379Reviewed-by: default avatarJonathan Rudenberg <jonathan@titanous.com>
Reviewed-by: default avatarFilippo Valsorda <hi@filippo.io>
Run-TryBot: Filippo Valsorda <hi@filippo.io>
TryBot-Result: Gobot Gobot <gobot@golang.org>
parent 72635401
...@@ -42,6 +42,7 @@ type nameConstraintsTest struct { ...@@ -42,6 +42,7 @@ type nameConstraintsTest struct {
roots []constraintsSpec roots []constraintsSpec
intermediates [][]constraintsSpec intermediates [][]constraintsSpec
leaf leafSpec leaf leafSpec
requestedEKUs []ExtKeyUsage
expectedError string expectedError string
noOpenSSL bool noOpenSSL bool
} }
...@@ -1444,6 +1445,43 @@ var nameConstraintsTests = []nameConstraintsTest{ ...@@ -1444,6 +1445,43 @@ var nameConstraintsTests = []nameConstraintsTest{
}, },
expectedError: "\"https://example.com/test\" is excluded", 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
// VerifyOptions.
nameConstraintsTest{
roots: []constraintsSpec{
constraintsSpec{},
},
intermediates: [][]constraintsSpec{
[]constraintsSpec{
constraintsSpec{},
},
},
leaf: leafSpec{
sans: []string{"dns:example.com"},
ekus: []string{"serverAuth"},
},
requestedEKUs: []ExtKeyUsage{ExtKeyUsageClientAuth},
expectedError: "incompatible key usage",
},
// #76: However, MSSGC in a leaf should match a request for serverAuth.
nameConstraintsTest{
roots: []constraintsSpec{
constraintsSpec{},
},
intermediates: [][]constraintsSpec{
[]constraintsSpec{
constraintsSpec{},
},
},
leaf: leafSpec{
sans: []string{"dns:example.com"},
ekus: []string{"msSGC"},
},
requestedEKUs: []ExtKeyUsage{ExtKeyUsageServerAuth},
},
} }
func makeConstraintsCACert(constraints constraintsSpec, name string, key *ecdsa.PrivateKey, parent *Certificate, parentKey *ecdsa.PrivateKey) (*Certificate, error) { func makeConstraintsCACert(constraints constraintsSpec, name string, key *ecdsa.PrivateKey, parent *Certificate, parentKey *ecdsa.PrivateKey) (*Certificate, error) {
...@@ -1781,6 +1819,7 @@ func TestConstraintCases(t *testing.T) { ...@@ -1781,6 +1819,7 @@ func TestConstraintCases(t *testing.T) {
Roots: rootPool, Roots: rootPool,
Intermediates: intermediatePool, Intermediates: intermediatePool,
CurrentTime: time.Unix(1500, 0), CurrentTime: time.Unix(1500, 0),
KeyUsages: test.requestedEKUs,
} }
_, err = leafCert.Verify(verifyOpts) _, err = leafCert.Verify(verifyOpts)
......
...@@ -543,11 +543,16 @@ func (c *Certificate) checkNameConstraints(count *int, ...@@ -543,11 +543,16 @@ func (c *Certificate) checkNameConstraints(count *int,
return nil return nil
} }
const (
checkingAgainstIssuerCert = iota
checkingAgainstLeafCert
)
// ekuPermittedBy returns true iff the given extended key usage is permitted by // ekuPermittedBy returns true iff the given extended key usage is permitted by
// the given EKU from a certificate. Normally, this would be a simple // 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 // comparison plus a special case for the “any” EKU. But, in order to support
// existing certificates, some exceptions are made. // existing certificates, some exceptions are made.
func ekuPermittedBy(eku, certEKU ExtKeyUsage) bool { func ekuPermittedBy(eku, certEKU ExtKeyUsage, context int) bool {
if certEKU == ExtKeyUsageAny || eku == certEKU { if certEKU == ExtKeyUsageAny || eku == certEKU {
return true return true
} }
...@@ -564,18 +569,23 @@ func ekuPermittedBy(eku, certEKU ExtKeyUsage) bool { ...@@ -564,18 +569,23 @@ func ekuPermittedBy(eku, certEKU ExtKeyUsage) bool {
eku = mapServerAuthEKUs(eku) eku = mapServerAuthEKUs(eku)
certEKU = mapServerAuthEKUs(certEKU) certEKU = mapServerAuthEKUs(certEKU)
if eku == 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. // ServerAuth in a CA permits ClientAuth in the leaf.
(eku == ExtKeyUsageClientAuth && certEKU == ExtKeyUsageServerAuth) || return (eku == ExtKeyUsageClientAuth && certEKU == ExtKeyUsageServerAuth) ||
// Any CA may issue an OCSP responder certificate. // Any CA may issue an OCSP responder certificate.
eku == ExtKeyUsageOCSPSigning || eku == ExtKeyUsageOCSPSigning ||
// Code-signing CAs can use Microsoft's commercial and // Code-signing CAs can use Microsoft's commercial and
// kernel-mode EKUs. // kernel-mode EKUs.
((eku == ExtKeyUsageMicrosoftCommercialCodeSigning || eku == ExtKeyUsageMicrosoftKernelCodeSigning) && certEKU == ExtKeyUsageCodeSigning) { (eku == ExtKeyUsageMicrosoftCommercialCodeSigning || eku == ExtKeyUsageMicrosoftKernelCodeSigning) && certEKU == ExtKeyUsageCodeSigning
return true
}
return false
} }
// isValid performs validity checks on c given that it is a candidate to append // isValid performs validity checks on c given that it is a candidate to append
...@@ -716,7 +726,7 @@ func (c *Certificate) isValid(certType int, currentChain []*Certificate, opts *V ...@@ -716,7 +726,7 @@ func (c *Certificate) isValid(certType int, currentChain []*Certificate, opts *V
for _, caEKU := range c.ExtKeyUsage { for _, caEKU := range c.ExtKeyUsage {
comparisonCount++ comparisonCount++
if ekuPermittedBy(eku, caEKU) { if ekuPermittedBy(eku, caEKU, checkingAgainstIssuerCert) {
continue NextEKU continue NextEKU
} }
} }
...@@ -850,7 +860,7 @@ func (c *Certificate) Verify(opts VerifyOptions) (chains [][]*Certificate, err e ...@@ -850,7 +860,7 @@ func (c *Certificate) Verify(opts VerifyOptions) (chains [][]*Certificate, err e
NextUsage: NextUsage:
for _, eku := range requestedKeyUsages { for _, eku := range requestedKeyUsages {
for _, leafEKU := range c.ExtKeyUsage { for _, leafEKU := range c.ExtKeyUsage {
if ekuPermittedBy(eku, leafEKU) { if ekuPermittedBy(eku, leafEKU, checkingAgainstLeafCert) {
continue NextUsage continue NextUsage
} }
} }
......
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