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

NEW
Unassigned

Status

()

Core
Security: PSM
P3
enhancement
7 years ago
3 months ago

People

(Reporter: briansmith, Unassigned)

Tracking

(Depends on: 1 bug)

Trunk
x86
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [psm-backlog])

Attachments

(1 attachment)

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.
Depends on: 651996
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: 651246
No longer depends on: 481656

Comment 2

6 years ago
Created attachment 646734 [details] [diff] [review]
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.
Attachment #646734 - Flags: review?(kaie)

Comment 3

5 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

5 years ago
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
You need to log in before you can comment on or make changes to this bug.