651994 - Viewing certificate from the site identity block does not necessarily show the cert chain used to make the trust decision

Status: RESOLVED DUPLICATE of bug 1560538

(Core :: Security: PSM, enhancement, P3)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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.

Comment 1

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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.

Comment 2

[Wan-Teh Chang]

Attachment: Pass usage=certUsageSSLServer in nsNSSCertificate::GetChain

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.

Comment 3

[Kai Engert (:KaiE:)]

Comment on attachment 646734 [details] [diff] [review]
Pass usage=certUsageSSLServer in nsNSSCertificate::GetChain

Maybe Brian should review this patch.

Comment 4

[Camilo Viecco (:cviecco)]

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

[Camilo Viecco (:cviecco)]

wtc: is this now a works for me?

On successful server verification, the chain returned is tha same as computed.

Comment 6

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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.

Comment 7

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Bug 1560538 addressed this with the new certificate viewer.