Closed
Bug 651994
Opened 14 years ago
Closed 5 years ago
Viewing certificate from the site identity block does not necessarily show the cert chain used to make the trust decision
Categories
(Core :: Security: PSM, enhancement, P3)
Tracking
()
RESOLVED
DUPLICATE
of bug 1560538
People
(Reporter: briansmith, Unassigned)
References
Details
(Whiteboard: [psm-backlog])
Attachments
(1 file)
931 bytes,
patch
|
Details | Diff | Splinter Review |
This is very similar to to bug 650296 and has the same cause: when we open the certificate viewer, it does its own separate chain building and validation. That means the chain and validation results that were used to decide whether to trust the connection may be different, based on various factors (e.g. OCSP reachability, user changes to cert trust, intermediate certificate caching).
This makes debugging of SSL failures more difficult.
Reporter | ||
Comment 1•14 years ago
|
||
We can turn on libpkix by default (bug 651246) without fixing this bug, as it wouldn't be a regression, the necessity isn't clear, and it might be tricky to fix the bugs it depends on.
No longer blocks: pkix-default
No longer depends on: 481656
Comment 2•12 years ago
|
||
I originally created this patch in bug 764393 comment 24.
I am not sure if this patch is suitable for checkin.
nsNSSCertificate::GetChain could be used for other purposes,
for example, to get the chain of a client certificate.
So the hardcoded usage=certUsageSSLServer may not be
suitable for all callers of nsNSSCertificate::GetChain.
Attachment #646734 -
Flags: review?(kaie)
Comment 3•12 years ago
|
||
Comment on attachment 646734 [details] [diff] [review]
Pass usage=certUsageSSLServer in nsNSSCertificate::GetChain
Maybe Brian should review this patch.
Attachment #646734 -
Flags: review?(kaie) → review?
Updated•12 years ago
|
Attachment #646734 -
Flags: review? → review?(bsmith)
Comment 4•12 years ago
|
||
Comment on attachment 646734 [details] [diff] [review]
Pass usage=certUsageSSLServer in nsNSSCertificate::GetChain
Review of attachment 646734 [details] [diff] [review]:
-----------------------------------------------------------------
::: security/manager/ssl/src/nsNSSCertificate.cpp
@@ +823,5 @@
> nsresult rv;
> /* Get the cert chain from NSS */
> CERTCertList *nssChain = NULL;
> PR_LOG(gPIPNSSLog, PR_LOG_DEBUG, ("Getting chain for \"%s\"\n", mCert->nickname));
> + nssChain = CERT_GetCertChainFromCert(mCert, PR_Now(), certUsageSSLServer);
I am almost certain this will break the viewer for viewing client certs, and would not necesarily resolve the issue. Think of cross signing and EV validation (pkix chain can be different, have seen it ).
This is however very close to part of bug 813418 (see https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=813418&attachment=706570). However as brian said, probably we want to remove this call in it current incantation because the usage is unspecified and it might block the main thread (if strict verification is needed).
Comment 5•11 years ago
|
||
wtc: is this now a works for me?
On successful server verification, the chain returned is tha same as computed.
Reporter | ||
Comment 6•11 years ago
|
||
Comment on attachment 646734 [details] [diff] [review]
Pass usage=certUsageSSLServer in nsNSSCertificate::GetChain
Review of attachment 646734 [details] [diff] [review]:
-----------------------------------------------------------------
The bug that this patch fixes is mostly fixed by bug 813418, but I found some bugs in the patch for that bug, so I filed bug 938046 to correct them. Anyway, this patch isn't necessary any more. This bug should stay open since neither bug 813418 nor bug 938046 actually fix the issue described in the summary of this bug.
Attachment #646734 -
Flags: review?(brian)
Updated•9 years ago
|
Whiteboard: [psm-backlog]
Updated•7 years ago
|
Priority: -- → P3
Comment 7•5 years ago
|
||
Bug 1560538 addressed this with the new certificate viewer.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•