395224 - Don't reject certs with critical NetscapeCertType extensions in libPKIX

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Alexei Volkov]

libpkix (in file pkix_buil.c) tracks the list of internally processed extensions for the purpose of identifying certs with unknown/unprocessed critical extensions.

NS cert type extension is not in the list, although it been handled by the code.

Comment 1

[Alexei Volkov]

Attachment: adding NS Cert type to pkix processed extensions.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 279922 [details] [diff] [review]
adding NS Cert type to pkix processed extensions.

I have lots of questions about this code, and the purpose of this list of OIDs.  At a minimum, more comments are needed to explain the real purpose of the list.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 279922 [details] [diff] [review]
adding NS Cert type to pkix processed extensions.

It appears to me that this patch does not actually add any code to process this extension.  It merely ignores this extension, even if critical.  
Is that right?

Comment 4

[Alexei Volkov]

This is correct. I assumed that code written to process KU and EKU extensions already does the work, that is needed for this extension.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Please find the code that processes Netscape cert type extensions 
(perhaps as part of processing KU and/or EKU extensions), and give some
URLs from lxr for that code here.  
If you can't find any, then clearly more work is needed.  :)

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 279922 [details] [diff] [review]
adding NS Cert type to pkix processed extensions.

Alexei confirmed today that the code to handle these extensions is not present (or so I understood him to say).

Comment 7

[Alexei Volkov]

I did say this at the meeting, but I've checked libpkix code today and here is what I've found:
pkix_pl_Cert_CheckExtendedKeyUsage function calls  cert_GetCertType to get cert type and later uses the type to compare with type requested by user.
(See http://lxr.mozilla.org/security/source/security/nss/lib/libpkix/pkix_pl_nss/pki/pkix_pl_cert.c#760)

Now, cert_GetCertType does check if a cert has NS cert type extension by calling CERT_FindNSCertTypeExtension and later process the returned result.
(See http://lxr.mozilla.org/security/source/security/nss/lib/certdb/certdb.c#554)

So, IMO, is valid to say that we process NS cert type extension and we just need to add NS_CERT_TYPE_EXT into the list of processed critical extensions.

Please consider the patch if you are agree with my findings.

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 279922 [details] [diff] [review]
adding NS Cert type to pkix processed extensions.

ok

Comment 9

[Alexei Volkov]

patch v1 is integrated into the trunk.