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)

x86
Linux
enhancement

Tracking

()

RESOLVED DUPLICATE of bug 1560538

People

(Reporter: briansmith, Unassigned)

References

Details

(Whiteboard: [psm-backlog])

Attachments

(1 file)

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.
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
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 on attachment 646734 [details] [diff] [review] Pass usage=certUsageSSLServer in nsNSSCertificate::GetChain Maybe Brian should review this patch.
Attachment #646734 - Flags: review?(kaie) → review?
Attachment #646734 - Flags: review? → review?(bsmith)
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).
wtc: is this now a works for me? On successful server verification, the chain returned is tha same as computed.
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)
Whiteboard: [psm-backlog]
Priority: -- → P3

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.

Attachment

General

Creator:
Created:
Updated:
Size: