link to the site's certificate on error pages where it makes sense to do so

VERIFIED FIXED in Firefox 64

Status

()

P1
normal
VERIFIED FIXED
7 months ago
2 months ago

People

(Reporter: keeler, Assigned: johannh)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 64
Points:
---

Firefox Tracking Flags

(firefox64 verified)

Details

User Story

https://mozilla.invisionapp.com/share/57NZ8GWJCEZ

Attachments

(4 attachments)

In our error pages, we can (and should) link directly to the site's certificate (where "link" means open the certificate viewer for now). The errors where it makes sense to do so are:

SEC_ERROR_UNKNOWN_ISSUER
SSL_ERROR_BAD_CERT_DOMAIN
SEC_ERROR_EXPIRED_CERTIFICATE
MOZILLA_PKIX_ERROR_ADDITIONAL_POLICY_CONSTRAINT_FAILED

and potentially others, but basically none of the SEC_ERROR_OCSP_... ones or errors that indicate that the handshake failed entirely (in which case we won't have a certificate anyway).
(Assignee)

Updated

7 months ago
Priority: -- → P2

Comment 1

7 months ago
The string for those links is: View the website’s certificate.
I've written a preliminary patch for this bug. I've manually tested it and it works well. That being said, it opens the "Page Info" dialog instead of going directly to the "View Certificate" window and I'm not sure if that's right; it also adds a button instead of a link. It's super late but I can upload a screenshot in the morning.

Also, it lacks tests. I assume they're needed?
Also here's a patch for a random typo I found - wasn't really sure where else to put it since I can't check things into the tree myself.
Posted image screenshot
Screenshot after I clicked the "More..." button. "Show Certificate" opens the Page Info dialog.
Attachment #9007483 - Flags: feedback?(ux-review)
Comment on attachment 9007483 [details]
screenshot

Hi AJ,

I believe that we will have a link – instead of a button – called “View the website’s certificate”, located underneath the error details.

https://drive.google.com/file/d/1-aiqUi9hOzr5RfY4yk02tDCr7GiXEN_6/view?usp=sharing


This link also replaces the error code on these errors:

* UNKNOWN_ISSUER
* BAD_CERT_DOMAIN
* EXPIRED_CERTIFICATE
* Symantec TLS Certificate errors


However, on these errors, the error code remains:

* OCSP_INVALID_SIGNING_CERT
* OCSP_FUTURE_RESPONSE


Source: https://docs.google.com/document/d/1PGZFR5fpXAjwJ8B3zg7wmZENYrDL3gWj9qmetJC2XFk/edit?ts=5b913ab2#heading=h.lbq403gyejq6


Meridel can help clarify it further. Thanks!
Flags: needinfo?(mwalkington)

Updated

7 months ago
Attachment #9007483 - Flags: feedback?(ux-review)
(In reply to Bram Pitoyo [:bram] from comment #5)

> I believe that we will have a link – instead of a button – called “View the
> website’s certificate”, located underneath the error details.

Great, thanks for getting back to me! Much appreciated.

> https://drive.google.com/file/d/1-aiqUi9hOzr5RfY4yk02tDCr7GiXEN_6/
> view?usp=sharing

Is it possible for this be shared publicly? I don't have access since I'm not MoCo. (Ditto for the Google Docs link.)

> This link also replaces the error code on these errors:
> 
> * UNKNOWN_ISSUER
> * BAD_CERT_DOMAIN
> * EXPIRED_CERTIFICATE
> * Symantec TLS Certificate errors

The error codes here are already links that open up a pane with technical info and a "Copy to clipboard" button. If they get replaced, how is the user supposed to get to this technical information?

Side note, can someone assign this bug to me? I don't have editbugs on BMO.
Sorry for(In reply to AJ Jordan [:strugee] (he/they) from comment #6)
> Is it possible for this be shared publicly? I don't have access since I'm
> not MoCo. (Ditto for the Google Docs link.)

I’ve updated the “User Story” field with the link to the publicly accessible InVision mockups.

Updated

7 months ago
User Story: (updated)

Comment 8

6 months ago
Re: "The error codes here are already links that open up a pane with technical info and a "Copy to clipboard" button. If they get replaced, how is the user supposed to get to this technical information?"

It was my understanding that the current error codes link to the certificate, so we are only changing the text for the link. I thought the "technical info" WAS the certificate. Dana, is this not true?
Flags: needinfo?(mwalkington) → needinfo?(dkeeler)
The technical info includes the certificate and other information that may be helpful in debugging, so I think we want to keep all that. The problem is that the certificate in that information is encoded, so it's not immediately useful to anyone. Eventually we probably want that pane to be more informative (e.g. actually decode and maybe offer hints as to why the certificate didn't verify), but we don't have that implemented, much less a design for how it would look/work. In the meantime, I think what people are generally looking for is a way to view the contents of the certificate. Right now the only way we have to do that in product is with the certificate viewer, which is a separate dialog (right now you can see it in action by clicking the site identity drop-down, then the right arrow, then "More Information", then "View Certificate"). So, my understanding is we would be adding a distinct button that opens the certificate viewer.
Flags: needinfo?(dkeeler)

Updated

6 months ago
Assignee: nobody → alex
(Assignee)

Comment 10

6 months ago
Hi AJ,

sorry for getting back to you so late, I'm probably the person who should have been mentoring this bug from the start. Your patch looks like a great start, thanks for digging into this. There are a few things I'd like to get done before this is ready to land:

- As noted above, it should be a text link

- IMO, we should really try to open the certificate viewer directly, you can do this in the following way:
https://searchfox.org/mozilla-central/rev/bdc89dfd7869e418d788b28eb60ab8d94e708a15/browser/base/content/pageinfo/security.js#336
. Simply use the securityInfo.serverCert as the cert in that dialog.

- Is there are reason for only showing the link for these specific error codes? I think you can always show it.

- I would really like to have at least basic tests for this, which might be a bit intimidating for a new Firefox developer, but I'm happy to help. Most of the tests for this page currently live here: https://searchfox.org/mozilla-central/source/browser/base/content/test/about/browser_aboutCertError.js

Are you still interested in this bug? Please let me know if you need any help!
Mentor: jhofmann
Status: NEW → ASSIGNED
Flags: needinfo?(alex)
(In reply to Johann Hofmann [:johannh] from comment #10)

> sorry for getting back to you so late, I'm probably the person who should
> have been mentoring this bug from the start. Your patch looks like a great
> start, thanks for digging into this. There are a few things I'd like to get
> done before this is ready to land:

Hi, thanks! My apologies too for responding so late; I kept meaning to come back to this bug but before I could my laptop ended up needing repair.

> - As noted above, it should be a text link
> 
> - IMO, we should really try to open the certificate viewer directly, you can
> do this in the following way:
> https://searchfox.org/mozilla-central/rev/
> bdc89dfd7869e418d788b28eb60ab8d94e708a15/browser/base/content/pageinfo/
> security.js#336
> . Simply use the securityInfo.serverCert as the cert in that dialog.

Thanks for the pointer, that's *super* helpful. It was on my list but I would've taken forever to find the right way.

> - Is there are reason for only showing the link for these specific error
> codes? I think you can always show it.

I was just following comment #0. Happy to amend.

> - I would really like to have at least basic tests for this, which might be
> a bit intimidating for a new Firefox developer, but I'm happy to help. Most
> of the tests for this page currently live here:
> https://searchfox.org/mozilla-central/source/browser/base/content/test/about/
> browser_aboutCertError.js

Great. Will needinfo you or ask #introduction if I get stuck.

> Are you still interested in this bug? Please let me know if you need any
> help!

I'm definitely still interested! However as I said above my laptop's still getting repaired. I would love to finish this up but I don't want to block any work so if someone wants to take my patch and run with it, that's fine too.
Flags: needinfo?(alex)
(Assignee)

Comment 13

5 months ago
Hi Alex, thank you for the efforts! Since this bug is supposed to go into 64 still I'll take it from here. Please let me know when you're ready to contribute again, since I would be happy to forward other patches to you.
Assignee: alex → jhofmann
Mentor: jhofmann
Priority: P2 → P1

Comment 14

5 months ago
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/868c99ece921
Add a "View Certificate" link to all cert error pages. r=nhnt11
(Assignee)

Comment 16

5 months ago
Already relanded!
Flags: needinfo?(jhofmann)

Comment 17

5 months ago
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/55c7172f12a4
Add a "View Certificate" link to all cert error pages. r=nhnt11

Comment 18

5 months ago
Backout by ebalazs@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3c175c006e65
Backed out changeset 868c99ece921 for ES lint failure /builds/worker/checkouts/gecko/browser/base/content/browser.js CLOSED TREE

Comment 19

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/55c7172f12a4
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
status-firefox64: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Tried to verify this fix with 64.0b4 but the changes don't seem to have been introduced in this version.
With 64.0a1(20181018123730) the changes are in effect. 

Is it something expected for the current beta build?
Flags: needinfo?(jhofmann)
(Assignee)

Comment 21

5 months ago
(In reply to Cristi Fogel [:cfogel] from comment #20)
> Tried to verify this fix with 64.0b4 but the changes don't seem to have been
> introduced in this version.
> With 64.0a1(20181018123730) the changes are in effect. 
> 
> Is it something expected for the current beta build?

Yes, you need to flip the browser.security.newcerterrorpage.enabled pref to see it for now. We're still figuring out the best release process for this.
Flags: needinfo?(jhofmann)
Thank you.
Verified with 64.0b5 on Win10, macOS 10.12, Ubuntu16.04.

Tested example pages for the errors mentioned in comment 0

SSL_ERROR_BAD_CERT_DOMAIN
SEC_ERROR_EXPIRED_CERTIFICATE
MOZILLA_PKIX_ERROR_ADDITIONAL_POLICY_CONSTRAINT_FAILED
SEC_ERROR_UNKNOWN_ISSUER

Just as a heads-up, the last 2 do not have the yellow border as the rest. Is that intended or tracked in another issue?
Status: RESOLVED → VERIFIED
status-firefox64: fixed → verified
Flags: qe-verify+ → needinfo?(jhofmann)
(Assignee)

Comment 23

5 months ago
(In reply to Cristi Fogel [:cfogel] from comment #22)
> Thank you.
> Verified with 64.0b5 on Win10, macOS 10.12, Ubuntu16.04.
> 
> Tested example pages for the errors mentioned in comment 0
> 
> SSL_ERROR_BAD_CERT_DOMAIN
> SEC_ERROR_EXPIRED_CERTIFICATE
> MOZILLA_PKIX_ERROR_ADDITIONAL_POLICY_CONSTRAINT_FAILED
> SEC_ERROR_UNKNOWN_ISSUER
> 
> Just as a heads-up, the last 2 do not have the yellow border as the rest. Is
> that intended or tracked in another issue?

I think that's intended, assuming that the SEC_ERROR_UNKNOWN_ISSUER was not overridable (HSTS enabled)
Flags: needinfo?(jhofmann)
(Assignee)

Updated

2 months ago
Duplicate of this bug: 1441906
You need to log in before you can comment on or make changes to this bug.