737802 - regression: SSL step-up EKU OID no longer treated as SSL Server type

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Rob Stradling]

User Agent: Mozilla/5.0 (X11; Linux i686) AppleWebKit/536.3 (KHTML, like Gecko) Chrome/19.0.1068.1 Safari/536.3

Steps to reproduce:

Access the following URLs using Google Chrome on Linux, NSS's tstclnt command-line tool and Firefox with "NSS_ENABLE_PKIX_VERIFY=1" (i.e. libpkix certificate chain verification):
https://desk.phi9.com
https://www.dominos.co.uk


Actual results:

Certificate chain rejected with an error along the lines of "unknown issuer" or "wrong certificate type".


Expected results:

No errors, certificate chain accepted.

Comment 1

[Rob Stradling]

Attachment: Treat step-up OID as also having SSL Server type

Patch to fix the problem.  It simply reinstates the code removed from certdb.c by bug 651523.

Comment 2

[Rob Stradling]

In https://bugzilla.mozilla.org/show_bug.cgi?id=651523#c4, rrelyea wrote:
"The only issue here could be the change to cert_ComputeCertType(), which would treat take a cert that only had the setup oid and not recognize it as an SSL cert. This may also affect the client, though my guess is that such a cert is highly unlikely. I would be OK with not including the certdb.c if we need to be more risk adverse."

I'm afraid "highly unlikely" was a bad guess.  :(

Several cross- and intermediate- CA certificates issued by Comodo some years ago have the EKU extension with the step-up/SGC OIDs but _not_ the Server Authentication OID.  We believed (perhaps wrongly, I don't know) that this was necessary in order activate step-up/SGC in the browsers of that time.

Other CAs apparently had the same understanding.  I've found 1 VeriSign and several GlobalSign intermediates that contain step-up/SGC but not Server Authentication.  However, since these intermediates contain the Netscape Certificate Type extension, they're not affected by this problem.
(But if the NSS team ever decide to remove support for the Netscape Certificate Type extension because it is considered to be no longer needed...)

Comment 3

[Rob Stradling]

IIRC, https://bugzilla.mozilla.org/show_bug.cgi?id=485155#c12 was a response to the same underlying problem (i.e. step-up/SGC OIDs present, Server Authentication OID absent).

Comment 4

[Rob Stradling]

I should also mention that the affected Comodo cross- and intermediate- CA certificates are deployed on many tens of thousands of sites, so replacing them is simply not an option for us.

Comment 5

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Are you still issuing certificates that chain to these certs with only SGC EKUs? I only looked at one of the EE certs, which was issued just 6 months ago.

Comment 6

[Rob Stradling]

Only 1 of the affected Intermediate CA Certificates is still issuing EE certs.  We can retire this Intermediate easily enough.

However, several of the affected certificates are cross-certificates (issued by/to our AddTrust, UTN and COMODO Roots) which don't expire for another 7 or 8 years.  We still issue many certs that chain to these cross-certs.  These cross-certs are so widely deployed across the Internet and in browser CA certificate caches that there is no way we can stop large numbers of clients from using them in chain building.

Comment 7

[Wan-Teh Chang]

Comment on attachment 607900 [details] [diff] [review]
Treat step-up OID as also having SSL Server type

r=wtc.  Thanks for the patch.

Could you confirm that this is the only thing that needs to be
salvaged from attachment 570437 [details] [diff] [review]?  You don't need the Netscape
international step-up support; you just need it to imply SSL
server type, right?

Comment 8

[Rob Stradling]

Confirmed.  We just need the step-up OID to imply SSL server type.

Thanks for the review, Wan-Teh.

Comment 9

[Rob Stradling]

Before you commit the patch, please extend the comment to something like...

/* Treat certs with step-up OID as also having SSL server type.
  COMODO need this behaviour until June 2020!  See Bug 737802 */

Thanks.

Comment 10

[Wan-Teh Chang]

Rob: I will include that comment.

Does this bug only affect libpkix?  Do you know why it doesn't affect the "classic" CERT_VerifyCert*
functions?

Comment 11

[Rob Stradling]

AFAICT, this bug only affects libpkix.  It always occurs with Chrome/Linux (which only uses libpkix, AFAIK), but I can only reproduce it with Firefox when I set "NSS_ENABLE_PKIX_VERIFY=1".

I'm afraid I have no idea why the "classic" certificate processing functions don't trigger this problem as well.  My knowledge of the NSS code base is fairly limited.

Thanks for giving this bug P1 priority.  Do you have an ETA for the release of 3.13.4, by any chance?

Comment 12

[Wan-Teh Chang]

Attachment: Treat step-up OID as also having SSL Server type, by Rob Stradling (v2)

Patch checked in on the NSS trunk (NSS 3.13.4) with the
comment Rob suggested.

Checking in certdb.c;
/cvsroot/mozilla/security/nss/lib/certdb/certdb.c,v  <--  certdb.c
new revision: 1.121; previous revision: 1.120
done

Comment 13

[Wan-Teh Chang]

Rob: the NSS trunk is in the middle of big checkins (TLS 1.1 and
DTLS 1.0), so we will need to create a branch and cherry-pick fixes
if we need to release NSS 3.13.4 soon.  I will look into that.

As for why this bug only affects libpkix, I didn't debug this in
a debugger, but by code inspection I found one difference between
libpkix and the "classic" cert verify code.

Classic: if a CA certificate has a Basic Constraints extension with
isCA=true, then it does not need to have the SSL CA cert type.  If
it has any CA cert type, then it must have the SSL CA cert type:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/certhigh/certvfy.c&rev=1.75&mark=637-639,641-642#631

libpkix: the CA certificate must have the SSL CA cert type.  (Note
that this is part of the code that Rob mentioned in comment 3.)

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/libpkix/pkix_pl_nss/pki/pkix_pl_cert.c&rev=1.28&mark=3019,3021-3023,3036#3019

So libpkix is more strict.

Bob, do you know which behavior is correct?

Comment 14

[Wan-Teh Chang]

Attachment: Make libpkix match "classic" behavior

Rob,

Could you revert your change to lib/certdb/certdb.c and
apply this patch?  Please let me know if this patch also
fixes the problem with libpkix.  Thanks.

Comment 15

[Rob Stradling]

Comment on attachment 608596 [details] [diff] [review]
Make libpkix match "classic" behavior

Review of attachment 608596 [details] [diff] [review]:
-----------------------------------------------------------------

r=rob.  This patch (with my patch reverted) also fixes the problem.

Comment 16

[Wan-Teh Chang]

Rob: thank you for testing my patch.

Bob: the logic in "classic" certvfy.c code has been there
since rev. 1.1 (it has only be rewritten to be more compact):

    /*
     * Make sure that if this is an intermediate CA in the chain that
     * it was given permission by its signer to be a CA.
     */
    /*
     * if basicConstraints says it is a ca, then we check the
     * nsCertType.  If the nsCertType has any CA bits set, then
     * it must have the right one.
     */
    if (!isca || (issuerCert->nsCertType & NS_CERT_TYPE_CA)) {
        isca = (issuerCert->nsCertType & caCertType) ? PR_TRUE : PR_FALSE;
    }

    if (  !isca  ) {
        PORT_SetError(SEC_ERROR_CA_CERT_INVALID);
        LOG_ERROR_OR_EXIT(log,issuerCert,count+1,0);
    }

I am not sure about the part that allows a certificate that does
not bave BasicConstraints cA=TRUE to become a CA certificate by
something in the EKU.  But I think the rest of the logic is good:
if a certificate has BasicConstraints cA=TRUE, it does not need
to have something in the EKU to restrict the type of certificate
(SSL, email, code signing) it can issue.

Comment 17

[Yuhong Bao]

Just found bug 231686. What is fun is this bug was not fixed until after Firefox 1.0 was released, and it gained marketshare despite this problem.

Comment 18

[Yuhong Bao]

Original bug is actually bug 231775, the bug number referred above is just a duplicate.