After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 744063 - Unacceptable certificate interstitial is confusing and probably ineffective
Unacceptable certificate interstitial is confusing and probably ineffective
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: Interface
3.14.x (obsolete)
Other Linux
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-02-05 18:37 UTC by Michael Catanzaro
Modified: 2016-03-29 18:26 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Our interstitial (60.69 KB, image/png)
2015-02-05 18:38 UTC, Michael Catanzaro
  Details
Chrome's new interstitial (46.05 KB, image/png)
2015-02-05 18:38 UTC, Michael Catanzaro
  Details
Adopt "opinionated design" for unacceptable certificate interstitial (6.53 KB, patch)
2015-02-05 19:53 UTC, Michael Catanzaro
none Details | Review
Adopt "opinionated design" for unacceptable certificate interstitial (6.55 KB, patch)
2015-02-06 04:32 UTC, Michael Catanzaro
reviewed Details | Review
Rework error pages (22.34 KB, patch)
2016-03-14 22:55 UTC, Gabriel Ivașcu
none Details | Review
Screenshots of patch 323935 (350.36 KB, image/png)
2016-03-14 23:02 UTC, Gabriel Ivașcu
  Details
Using channel-insecure-symbolic (35.60 KB, image/png)
2016-03-15 15:46 UTC, Gabriel Ivașcu
  Details
New design for error pages (25.98 KB, patch)
2016-03-15 20:55 UTC, Gabriel Ivașcu
none Details | Review
Updated patch (26.52 KB, patch)
2016-03-16 21:31 UTC, Gabriel Ivașcu
committed Details | Review

Description Michael Catanzaro 2015-02-05 18:37:31 UTC
There's a lot of research [1] [2] that shows that interstitial warnings like the one I implemented for Epiphany 3.14 are not effective at discouraging the user from proceeding to the site. Chrome has recently started using a more effective warning. I propose we copy the Chrome warning almost unchanged, and drop all technical details from our interstitial. In particular, we should not use any technical words like "certificate" or "encryption."

There is some value to keeping the list of vague reasons the certificate was rejected. These can still be accessed from the certificate info dialog.

[1] https://adrifelt.github.io/sslinterstitial-chi.pdf
[2] https://static.googleusercontent.com/external_content/untrusted_dlcp/research.google.com/en/us/pubs/archive/41323.pdf
Comment 1 Michael Catanzaro 2015-02-05 18:38:19 UTC
Created attachment 296232 [details]
Our interstitial
Comment 2 Michael Catanzaro 2015-02-05 18:38:46 UTC
Created attachment 296233 [details]
Chrome's new interstitial
Comment 3 Michael Catanzaro 2015-02-05 19:53:18 UTC
Created attachment 296235 [details] [review]
Adopt "opinionated design" for unacceptable certificate interstitial

If we try harder to discourage users from continuing, we might actually
succeed. Leave the button where it is for now, so as not to alienate
users.
Comment 4 Michael Catanzaro 2015-02-06 04:32:03 UTC
Created attachment 296254 [details] [review]
Adopt "opinionated design" for unacceptable certificate interstitial

If we try harder to discourage users from continuing, we might actually
succeed. Leave the button where it is for now, so as not to alienate
users.
Comment 5 Carlos Garcia Campos 2015-02-06 08:02:23 UTC
I'm not sure about this, we could add an advanced expander with the information you are removing, collapsed by default.
Comment 6 Michael Catanzaro 2015-02-22 00:39:13 UTC
(In reply to Carlos Garcia Campos from comment #5)
> I'm not sure about this, we could add an advanced expander with the
> information you are removing, collapsed by default.

I'm fine with this, and will try to implement it sometime next cycle. Obviously we've missed the 3.16 train on this one.
Comment 8 Michael Catanzaro 2015-10-08 14:57:08 UTC
Thanks Jakub, that looks much better. Actually, all of our error pages could have a more similar design to that.

I think we don't want to have the "I know what I'm doing, proceed" link to be so visible, but hidden under the Technical Information slider; even if that doesn't seem like quite the right place, it's reduced clickthrough rates in Chrome, and I think we should copy them on that. Also, very, very few users realize what they're doing when clicking through that, so I would rather phrase it as "accept risk and continue" rather than "I know what I'm doing."

It would also be good to clarify that clicking through this warning is not just a privacy issue -- it allows any arbitrary Internet user to have total control over your current and future (remember the session cookie will be leaked) interactions with the web site. That's not easy to do in a short and understandable warning message, but I think the warning could be worded a bit more strongly. "This is probably not the real www.whatever.com" is what we have now, and it seems right to keep that.
Comment 9 Michael Catanzaro 2015-10-08 14:58:57 UTC
(In reply to Michael Catanzaro from comment #8)
> "This is probably not the real www.whatever.com" is what
> we have now, and it seems right to keep that.

Well we should invert this to be more clear: "This site seems to be impersonating www.whatever.com"
Comment 10 Allan Day 2015-11-17 17:24:01 UTC
Maybe use channel-insecure-symbolic, rather than changes-allow-symbolic? The former looks more like a warning.
Comment 11 Michael Catanzaro 2015-11-17 19:30:48 UTC
Other than the question of the icon (I'm fine with either), the updated design looks great to me. I only wonder if channel-insecure-symbolic will look good in red. It's normally yellow-on-black; would it become yellow-on-red (possibly not good)? I'm not sure, since normally symbolic icons are monochrome.
Comment 12 Gabriel Ivașcu 2016-03-14 22:55:35 UTC
Created attachment 323935 [details] [review]
Rework error pages

I've followed Jakub's mockup and did my best to implement the new interstitial look.

In fact, as Michael suggested, all error pages share the new template now which is:
* error icon
* message title
* message body
* message details (optional)
* hidden action button (optional)
* visible action button

'Technical information' expander is displayed only if any message details string was provided. Clicking it will reveal the extra error information and the hidden button (if there is one). If no message details string was provided then the expander will not be displayed.

I believe this approach is better because it allows you to easily customize the error messages along with some proper action button(s) and also provides a more consistent template.

Let me know what you think of it. (I'll attach some screenshots too)
Comment 13 Gabriel Ivașcu 2016-03-14 23:02:29 UTC
Created attachment 323937 [details]
Screenshots of patch 323935

Used computer-fail-symbolic as the default icon and changes-allow-symbolic for interstitial (can be easily changed though).
Comment 14 Michael Catanzaro 2016-03-15 06:03:40 UTC
Jakub, any comments on these screenshots?

Screenshots look great, good work; this will be a great new feature for our 3.21.1 release. (I still need to review your patch, though.)

A couple thoughts from me:

 * Thinking this through more, we really should use channel-insecure-symbolic rather than changes-allow-symbolic. Misusing icons risks the page breaking if the icon changes in the future, or if Epiphany is used with any icon theme that decides not to use a lock for that.
 * Try to add more horizontal padding between the channel-insecure/computer-fail icon and the heading.
 * Try to add some more vertical padding between the Proceed Anyway button and the text above it.
Comment 15 Gabriel Ivașcu 2016-03-15 15:46:46 UTC
Created attachment 324016 [details]
Using channel-insecure-symbolic

(In reply to Michael Catanzaro from comment #14)
> A couple thoughts from me:
> 
>  * Thinking this through more, we really should use
> channel-insecure-symbolic rather than changes-allow-symbolic. Misusing icons
> risks the page breaking if the icon changes in the future, or if Epiphany is
> used with any icon theme that decides not to use a lock for that.
>  * Try to add more horizontal padding between the
> channel-insecure/computer-fail icon and the heading.
>  * Try to add some more vertical padding between the Proceed Anyway button
> and the text above it.

Okay like this?
Comment 16 Gabriel Ivașcu 2016-03-15 15:54:18 UTC
(In reply to Michael Catanzaro from comment #14)
> Misusing icons risks the page breaking if the icon changes in the future,
> or if Epiphany is used with any icon theme that decides not to use a lock
> for that.

Correct me if I'm wrong.. can't we prevent this by adding the icons in src/resources?
Comment 17 Michael Catanzaro 2016-03-15 18:25:57 UTC
(In reply to Gabriel Ivascu from comment #16)
> Correct me if I'm wrong.. can't we prevent this by adding the icons in
> src/resources?

Yup, it's harder though... give it a shot. :)
Comment 18 Michael Catanzaro 2016-03-15 18:26:54 UTC
(In reply to Gabriel Ivascu from comment #15)
> Okay like this?

That screenshot is nice :)

This is a great contribution, I'm excited to land this after freeze.
Comment 19 Gabriel Ivașcu 2016-03-15 20:55:24 UTC
Created attachment 324046 [details] [review]
New design for error pages

Final form. Please review.
Comment 20 Michael Catanzaro 2016-03-16 01:13:27 UTC
Review of attachment 324046 [details] [review]:

Well done, this looks very good.

::: embed/ephy-web-view.c
@@ +1702,3 @@
+  char *hidden_button_label = NULL;
+  char *hidden_button_action = NULL;
+  const char *hidden_button_accesskey = NULL;

This is maybe one of those functions that is doomed to be messy.

Can we try to rearrange the variables so that the const ones are all listed either above or below the non-const ones?

Also, since we have so many string variables here, I think it would be good to NULL-initialize them all even if not necessary, so you don't have to scroll up to the top of the function to check if a variable is NULL-initialized or not.

I might try splitting this up into separate functions (in a separate patch, after this lands).

@@ +1738,3 @@
+      msg_body = g_strdup_printf (_("<p>The site at <strong>%s</strong> seems "
+                                    "to be unavailable.</p>"
+                                    "<p>It may be temporarily unaccessible or "

Did you intentionally change the word from "unavailable" to "unaccessible?" I think the change is good, since using the same word unavailable to explain why the page is unavailable is not great.

"Unaccessible" is not really idiomatic, though, I would say "inaccessible" instead; that's more common in the US.

@@ +1792,3 @@
     case EPHY_WEB_VIEW_ERROR_INVALID_TLS_CERTIFICATE:
+      /* Page title when a site is not loaded due to an invalid TLS certificate. */
+      page_title = g_strdup_printf (_("Privacy Error"));

I am going to nitpick the text on this page. Our mild-mannered graphic designer slash drone-racing action hero's design is otherwise great, but I prefer to change some of the text.

An unacceptable TLS certificate is not just a privacy issue; the attacker has total control over your interaction with the web site. Extreme examples: if you're visiting Amazon, she could buy stuff with your credit card; if you're visiting your bank, she could drain your bank account. In general, the attacker gets your session cookie, then the attacker can interact with the site as if she were you.

Instead, I would say "Security Violation" (a bit stronger word than "error"). It is a violation because either the user or the server explicitly requested a secure connection (otherwise we wouldn't be using TLS).

@@ +1795,3 @@
+
+      /* Message title when a site is not loaded due to an invalid TLS certificate. */
+      msg_title = g_strdup (_("Your Connection is Not Private"));

"This Connection is Not Secure"

@@ +1798,3 @@
+
+      /* Message body when a site is not loaded due to an invalid TLS certificate. */
+      msg_body = g_strdup_printf (_("<p>Attackers might be able to snoop your "

Let's say this instead:

"This does not look like the real <strong>%s</strong>. Attackers might be trying to steal or alter information going to or from this site (for example, private messages, credit card information, or passwords)."

@@ +1804,3 @@
+                                  hostname);
+      /* Message details when a site is not loaded due to an invalid TLS certificate. */
+      msg_details = detailed_message_from_tls_errors(view->tls_errors);

Missing space before (

@@ +1807,3 @@
+
+      /* The button on the invalid TLS certificate error page. */
+      button_label = g_strdup (_("Back to Safety"));

"Go Back"

Let's take that from Firefox, so as not to imply the page we're going back to is safe. (It probably is an http:// URL and has no security.)

@@ +1812,3 @@
+
+      /* The hidden button on the invalid TLS certificate error page. */
+      hidden_button_label = g_strdup (_("Proceed Anyway"));

"Accept Risk and Proceed"

::: src/resources/error.html
@@ +30,3 @@
+    }
+    function toggleDetailsBox() {
+      var div = document.getElementById('reveal');

Minor nit: does it work if you use 'let' instead of 'var'? I'm not sure if we support 'let' by default yet or not. It does not matter at all in this case, but in general it's better to use 'let' or 'const' instead of 'var', because 'var' does not follow the scoping rules you would except from other languages.

I'm pretty sure we support 'let' now, but not positive (it's new).

@@ +55,3 @@
+      </div>
+      <div id="msg-details" class="%s">
+        <span id="pointer">&#9658;</span>

Does it need to be in its own span?

This patch would work almost perfectly, except for this. I got confused trying to click the arrow to open the expander and seeing nothing happen; I thought it was broken, but it turns out the arrow just isn't part of the link. Would be good to investigate a solution for that.
Comment 21 Gabriel Ivașcu 2016-03-16 21:31:08 UTC
Created attachment 324142 [details] [review]
Updated patch

(In reply to Michael Catanzaro from comment #20)
> Review of attachment 324046 [details] [review] [review]:
>
> Did you intentionally change the word from "unavailable" to "unaccessible?"
> I think the change is good, since using the same word unavailable to explain
> why the page is unavailable is not great.

Yes, I changed it so we don't have to repeat the same word twice.

> @@ +55,3 @@
> +      </div>
> +      <div id="msg-details" class="%s">
> +        <span id="pointer">&#9658;</span>
> 
> Does it need to be in its own span?

I've placed it in its own span so I can easily toggle between arrow right and arrow down.

> I got confused trying to click the arrow to open the expander and seeing
> nothing happen; I thought it was broken, but it turns out the arrow just
> isn't part of the link. Would be good to investigate a solution for that.

I've made the arrow part of the link now.
Comment 22 Michael Catanzaro 2016-03-16 22:05:47 UTC
Review of attachment 324142 [details] [review]:

Excellent