nsNSS_SSLGetClientAuthData calls nsNSSCertificate::FormatUIStrings for each client cert that it is about to display. FormatUIStrings does certificate path validation via nsNSSCertificate::GetUsagesString. This is wrong for two reasons: 1. It causes the main thread to block on certificate path validation (OCSP request I/O, in particular). 2. We should not be validating client certificates anyway, because we likely do not have the intermediate and/or root certificates needed to validate them, and/or those intermediate and/or root certificates won't be trusted. I propose that we fix this by removing the verified usages display from the client cert picker UI.
Created attachment 563934 [details] [diff] [review] Do not verify client certificates when they are being used for SSL client auth This is a prerequisite for the SSL thread removal. Watch https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=647a0a569156 for your results to come in. Builds and logs will be available at http://firstname.lastname@example.org.
Attachment #563934 - Flags: review?(honzab.moz)
Comment on attachment 563934 [details] [diff] [review] Do not verify client certificates when they are being used for SSL client auth Review of attachment 563934 [details] [diff] [review]: ----------------------------------------------------------------- So, this will just remove the "Purposes:" line from the details? I'm not aware of another way to get this info anywhere in the cert viewer. Maybe we should add one (in a new bug, not a priority, until someone complains about it missing). CertInfoPurposes locale string can be removed with the code.
Attachment #563934 - Flags: review?(honzab.moz) → review+
And: please, do a good testing of client cert auth with and w/o the patch. We don't have an automatic tests for this feature, my IIS is stupid to setup, so it is hard to say if all works or not. Thanks.
Created attachment 565411 [details] client cert that can be imported for testing Thanks for the review. You can import this client cert using Options -> Advanced -> Certificates -> Your certificates -> Import. The password is blank. You can check the functionality via https://www.mikestoolbox.net/. As you can see, there is no effect on the UI before/after the change, because the old UI omitted the usage info when it failed to validate the certificate. Client certificates will usually fail to validate because the root for client certificates is, in practice, usually not installed. I also verified that the Microsoft IE UI does not include the usage information. For the most part, it is useless jargon for the user.
(In reply to Honza Bambas (:mayhemer) from comment #2) > I'm not > aware of another way to get this info anywhere in the cert viewer. Maybe we > should add one (in a new bug, not a priority, until someone complains about > it missing). Originally, there was a "Purposes" column in most of the cert manager tabs, see bug 383969 comment 7. Bug 384611 is about restoring that feature.
Whiteboard: [psm-arch][psm-auth] → [psm-arch][psm-auth][inbound]
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
You need to log in before you can comment on or make changes to this bug.