Last Comment Bug 395224 - Don't reject certs with critical NetscapeCertType extensions in libPKIX
: Don't reject certs with critical NetscapeCertType extensions in libPKIX
Status: RESOLVED FIXED
PKIX
:
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: 3.12
: All All
: P1 major (vote)
: 3.12
Assigned To: Alexei Volkov
:
Mentors:
Depends on:
Blocks: 396598
  Show dependency treegraph
 
Reported: 2007-09-06 09:14 PDT by Alexei Volkov
Modified: 2014-07-30 10:32 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
adding NS Cert type to pkix processed extensions. (2.06 KB, patch)
2007-09-06 09:23 PDT, Alexei Volkov
nelson: review+
Details | Diff | Review

Description Alexei Volkov 2007-09-06 09:14:19 PDT
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 2007-09-06 09:23:45 PDT
Created attachment 279922 [details] [diff] [review]
adding NS Cert type to pkix processed extensions.
Comment 2 Nelson Bolyard (seldom reads bugmail) 2007-09-09 20:57:10 PDT
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) 2007-09-10 09:37:28 PDT
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 2007-09-12 12:18:26 PDT
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) 2007-09-12 13:30:57 PDT
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) 2007-09-13 23:48:37 PDT
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 2007-09-14 17:13:28 PDT
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) 2007-09-18 16:18:16 PDT
Comment on attachment 279922 [details] [diff] [review]
adding NS Cert type to pkix processed extensions.

ok
Comment 9 Alexei Volkov 2007-09-19 17:19:07 PDT
patch v1 is integrated into the trunk.

Note You need to log in before you can comment on or make changes to this bug.