[release-branch.go1.10] 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/96379
Reviewed-by: Jonathan Rudenberg <jonathan@titanous.com>
Reviewed-by: Filippo Valsorda <hi@filippo.io>
Run-TryBot: Filippo Valsorda <hi@filippo.io>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/102780
Run-TryBot: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>

diff --git a/src/crypto/x509/name_constraints_test.go b/src/crypto/x509/name_constraints_test.go
index 10cc3483576c830a5b5d98e9abc9f5ed0af404e4..14741592030e8d8a7102aa03a06825fd517927eb 100644 (file)
--- a/src/crypto/x509/name_constraints_test.go
+++ b/src/crypto/x509/name_constraints_test.go
@@ -42,6 +42,7 @@ type nameConstraintsTest struct {
        roots         []constraintsSpec
        intermediates [][]constraintsSpec
        leaf          leafSpec
+       requestedEKUs []ExtKeyUsage
        expectedError string
        noOpenSSL     bool
 }
@@ -1444,6 +1445,43 @@ 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
+       // 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) {
@@ -1781,6 +1819,7 @@ func TestConstraintCases(t *testing.T) {
                        Roots:         rootPool,
                        Intermediates: intermediatePool,
                        CurrentTime:   time.Unix(1500, 0),
+                       KeyUsages:     test.requestedEKUs,
                }
                _, err = leafCert.Verify(verifyOpts)
 

diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go
index 9477e85b95167a8b9d2d1a7b3bd9797f023860cd..cd6156c69fbaaf565803c55ae4b8b9b75141ba59 100644 (file)
--- a/src/crypto/x509/verify.go
+++ b/src/crypto/x509/verify.go
@@ -543,11 +543,16 @@ 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) bool {
+func ekuPermittedBy(eku, certEKU ExtKeyUsage, context int) bool {
        if certEKU == ExtKeyUsageAny || eku == certEKU {
                return true
        }
@@ -564,18 +569,23 @@ func ekuPermittedBy(eku, certEKU ExtKeyUsage) bool {
        eku = mapServerAuthEKUs(eku)
        certEKU = mapServerAuthEKUs(certEKU)
 
-       if eku == certEKU ||
-               // ServerAuth in a CA permits ClientAuth in the leaf.
-               (eku == ExtKeyUsageClientAuth && certEKU == ExtKeyUsageServerAuth) ||
+       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) {
-               return true
-       }
-
-       return false
+               (eku == ExtKeyUsageMicrosoftCommercialCodeSigning || eku == ExtKeyUsageMicrosoftKernelCodeSigning) && certEKU == ExtKeyUsageCodeSigning
 }
 
 // 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
 
                        for _, caEKU := range c.ExtKeyUsage {
                                comparisonCount++
-                               if ekuPermittedBy(eku, caEKU) {
+                               if ekuPermittedBy(eku, caEKU, checkingAgainstIssuerCert) {
                                        continue NextEKU
                                }
                        }
@@ -850,7 +860,7 @@ func (c *Certificate) Verify(opts VerifyOptions) (chains [][]*Certificate, err e
        NextUsage:
                for _, eku := range requestedKeyUsages {
                        for _, leafEKU := range c.ExtKeyUsage {
-                               if ekuPermittedBy(eku, leafEKU) {
+                               if ekuPermittedBy(eku, leafEKU, checkingAgainstLeafCert) {
                                        continue NextUsage
                                }
                        }