237877 - CERT_KeyUsageAndTypeForCertUsage returns wrong key usage for SSL server certificates with DHE ciphersuites

Status: NEW

(NSS :: Libraries, defect, P2)

Description

[Luciano Medina]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4) Gecko/20030624 Netscape/7.1 (ax)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4) Gecko/20030624 Netscape/7.1 (ax)

I couldn't connect from the Netscape browser to a SSL server with the
TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA cipher suite. The server was sending a
certificate with RSA public key, key usage extension equal to digitalSignature,
but the browser replied with a -8102 error (SEC_ERROR_INADEQUATE_KEY_USAGE).
After looking at the NSS source code, I think I found the bug:

The CERT_KeyUsageAndTypeForCertUsage function returs a
KU_KEY_AGREEMENT_OR_ENCIPHERMENT key usage code for every SSL server certificate
(cert usage code = certUsageSSLServer).
This is ok for all RSA and DH key exchange ciphersuites (TLS_RSA_xxx and
TLS_DH_xxx), but it is wrong for DHE ciphersuites (TLS_DHE_xxx) where the key
usage code should be KU_DIGITAL_SIGNATURE.
From RFC 2246, 7.4.2:
 
      Key Exchange Algorithm  Certificate Key Type

       RSA                     RSA public key; the certificate must
                               allow the key to be used for encryption.

       RSA_EXPORT              RSA public key of length greater than
                               512 bits which can be used for signing,
                               or a key of 512 bits or shorter which
                               can be used for either encryption or
                               signing.

       DHE_DSS                 DSS public key.

       DHE_DSS_EXPORT          DSS public key.

       DHE_RSA                 RSA public key which can be used for
                               signing.

       DHE_RSA_EXPORT          RSA public key which can be used for
                               signing.

       DH_DSS                  Diffie-Hellman key. The algorithm used
                               to sign the certificate should be DSS.

       DH_RSA                  Diffie-Hellman key. The algorithm used
                               to sign the certificate should be RSA.

Reproducible: Always
Steps to Reproduce:
1. Connect from a Netscape browser to a SSL server which accepts only a 
2.
3.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

This bug is strongly related to bug 51466.  The problem described here 
for DHE_RSA is also true for RSA_EXPORT.  bug 51466 also describes
another related issue, which is that NSS doesn't (or didn't) accept 
Server Key Exchange messages in all cases where it should have.  But
nobody cares about the _EXPORT key exchange methods any more.

Function CERT_KeyUsageAndTypeForCertUsage assumes that we can tell the
required usage based on the NSS-defined set of cert usage types 
(e.g. certUsageSSLServer, certUsageSSLServerWithStepUp, and the knowledge
of whether the cert is expected to be a CA cert or not, and that just
isn't enough.  

The key usage KU_KEY_AGREEMENT_OR_ENCIPHERMENT (processed in function
CERT_CheckKeyUsage) amounts to a prior admission that the arguments to 
CERT_KeyUsageAndTypeForCertUsage don't provide enough info.  That hybrid 
key usage allows the real key usage to be determined based on the type 
of key in the cert.  But even that's not enough, as seen by DHE_RSA and
RSA_EXPORT.  Only the application really knows whether the usage needs 
to be signing or encryption or agreement or some _specific_ combination.

Here are some ideas for addressing this issue (with pros and cons):

Idea a) change CERT_CheckKeyUsage's case rsaKey to match case ecKey, and 
allow either signing or encryption certs.  Pro: fixes this bug. 
Con: allows signing only certs to work with the RSA (only) KEA, which 
is wrong.

Idea b) add new SECCertUsage values certUsageSSLServerSignature (to be used 
with DHE_RSA and DHE_RSA_EXPORT) and certUsageSSLServerSignOrEncrypt (to be 
used with RSA_EXPORT).  The code that now calls CERT_VerifyCert with 
certUsageSSLServer would have to be able to select one of those 3 instead.

Pros: doesn't allow signing only certs to be used in appropriately. 
Cons: 

Con 1) Applications and libSSL must also change or they will continue to 
experience the old behavior (the subject of this bug).  This is because for
authentication of incoming SSL peers' certs, SSL calls an "application 
supplied callback" function, for which NSS has its own default 
implementation, but some applications (including mozilla, IINM) also have 
their own.  So, even if we change NSS's callback to use these new 
values, not all applications will get the benefit immediately.  *AND*

Con 2) The interface for the application-supplied callback funciton doesn't
tell that function whether we're using SSL2 or SSL3/TLS, nor which 
ciphersuites is "pending" (being negotiated).  libSSL has accessor functions
for asking for the "current" (already negotiated) ciphersuites, but not for
the "pending" ones (although that could be remedied with more accessors).

Con 3) we also have to change or supplement certUsageSSLServerWithStepUp.  
The options there include:
3a)  change CERT_KeyUsageAndTypeForCertUsage to require ONLY 
KU_NS_GOVT_APPROVED usage, and none of signature/encryption/agreement.
This is reasonable since the step-up usage is only checked after the 
cert has passed the check for certUsageSSLServer), or 
3b) also add new SECCertUsage values certUsageSSLServerSignatureWithStepUp 
and certUsageSSLServerSignOrEncryptWithStepUp.  

Con 4) This solution addresses SSL, but not SMIME.  (Maybe SMIME has no
similar considerations).

Idea c) Stan, NSS 4.0, change the cert verification API big time.  

Comment 2

[Julien Pierre]

I like idea c) . The cert verification API really is out of hand. We can't keep
doing band-aid additions here and there and keep supporting all the old code
forever. At some point, we need to change. I'm not sure that it really has to be
done in parallel with all that was Stan, but I think we could certain justify
replacing the existing certificate verification API. I don't know how the idea
will fly with product groups, but I think it's worth having a meeting about what
we want to change/replace and spec it .

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

nominating for 3.10

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

This is a real flaw and I wish it could be fixed asap,
but I can't prioritize this work before 3.12.
Bob or Wan-Teh, do you want to look at it before then?

Comment 5

[jura]

error seen :
Mozilla/5.0 (Windows; U; Windows NT 5.1; cs; rv:1.8) Gecko/20051111 Firefox/1.5

M$ iexp works ...

Protocol : HyperText Transfer Protocol with Privacy

Type: Not Available

Connection: SSL 3.0, RC4 with 128 bit encryption (High); RSA with 1024 bit exchange

URL : intranet :-(

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Nominating for 3.12
Julien, should this be P1 for 3.12 ?  Would we hold 3.12 for it?

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

The problems described above are now compounded by the addition of the ECC 
cipher suites.  The same problems that previously beset the DH(E) cipher 
suites now also plague the ECDH(E) cipher suites.  

We were going to simply wait and let this problem be fixed with the 
introduction of an entirely new Cert verification API for libPKIX in NSS 3.12.  
We were going to require apps that want this fixed to switch to the new API.

But now we expect that Sun's servers will not be moving to a new CERT 
verification API (function) for 3.12.  But the problems with being unable to 
ensure that certs used for signing have signing capability needs to be fixed
before the servers switch to the new API. 

So, I have decided to go forward with a variant of "idea b" (from comment 1 above).  Adding a few new cert usages to the enumerated type used by the 
existing cert verification API allows us to fix this in a way that is 
compatible both with the old cert verification algorithms and with the new
libPKIX code.  I am working on a patch now.

Comment 10

[Nelson Bolyard (seldom reads bugmail)]

Unsetting target milestone in unresolved bugs whose targets have passed.