From: Adam Langley <agl@golang.org>
Date: Thu, 22 Feb 2018 20:30:44 +0000 (-0800)
Subject: crypto/x509: tighten EKU checking for requested EKUs.
X-Git-Tag: go1.11beta1~1478
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=0681c7c31b5922f08f31404023e6b295f35812fe;p=gostls13.git

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>
---

diff --git a/src/crypto/x509/name_constraints_test.go b/src/crypto/x509/name_constraints_test.go
index 10cc348357..1474159203 100644
--- 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 f0df386122..8b0c41ddbc 100644
--- 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
 				}
 			}