diff --git a/src/crypto/x509/name_constraints_test.go b/src/crypto/x509/name_constraints_test.go index 0172ccf08c..e89e70d874 100644 --- a/src/crypto/x509/name_constraints_test.go +++ b/src/crypto/x509/name_constraints_test.go @@ -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) { diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go index 0ea214bd2e..60e415b7ec 100644 --- a/src/crypto/x509/verify.go +++ b/src/crypto/x509/verify.go @@ -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,53 +761,6 @@ 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) - } - - // 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 - } - } - - if checkEKU { - foundMatch := false - NextUsage: - for _, eku := range requestedKeyUsages { - for _, leafEKU := range c.ExtKeyUsage { - if ekuPermittedBy(eku, leafEKU, checkingAgainstLeafCert) { - foundMatch = true - break NextUsage - } - } - } - - 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} - } - } - var candidateChains [][]*Certificate if opts.Roots.contains(c) { candidateChains = append(candidateChains, []*Certificate{c}) @@ -917,7 +770,29 @@ func (c *Certificate) Verify(opts VerifyOptions) (chains [][]*Certificate, err e } } - return candidateChains, nil + keyUsages := opts.KeyUsages + if len(keyUsages) == 0 { + keyUsages = []ExtKeyUsage{ExtKeyUsageServerAuth} + } + + // If any key usage is acceptable then we're done. + for _, usage := range keyUsages { + if usage == ExtKeyUsageAny { + return candidateChains, nil + } + } + + for _, candidate := range candidateChains { + if checkChainForKeyUsage(candidate, keyUsages) { + chains = append(chains, candidate) + } + } + + if len(chains) == 0 { + return nil, CertificateInvalidError{c, IncompatibleUsage, ""} + } + + 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 +}