Don't reject certs with critical NetscapeCertType extensions in libPKIX

RESOLVED FIXED in 3.12

Status

NSS
Libraries
P1
major
RESOLVED FIXED
10 years ago
3 years ago

People

(Reporter: Alexei Volkov, Assigned: Alexei Volkov)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: PKIX)

Attachments

(1 attachment)

(Assignee)

Description

10 years ago
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.
(Assignee)

Updated

10 years ago
Whiteboard: PKIX
(Assignee)

Updated

10 years ago
Blocks: 390542
(Assignee)

Comment 1

10 years ago
Created attachment 279922 [details] [diff] [review]
adding NS Cert type to pkix processed extensions.
Attachment #279922 - Flags: review?(nelson)
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 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?
(Assignee)

Comment 4

10 years ago
This is correct. I assumed that code written to process KU and EKU extensions already does the work, that is needed for this extension.
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.  :)
Severity: enhancement → major
Priority: -- → P1
Summary: add NS Cert Type extension to the pkix list of processed extension. → Don't reject certs with critical NetscapeCertType extensions in libPKIX
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).
Attachment #279922 - Flags: review?(nelson) → review-
(Assignee)

Comment 7

10 years ago
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.
(Assignee)

Updated

10 years ago
Blocks: 396598
(Assignee)

Updated

10 years ago
No longer blocks: 390542
Comment on attachment 279922 [details] [diff] [review]
adding NS Cert type to pkix processed extensions.

ok
Attachment #279922 - Flags: review- → review+
(Assignee)

Comment 9

10 years ago
patch v1 is integrated into the trunk.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
See Also: → bug 1009161
You need to log in before you can comment on or make changes to this bug.