GNOME Bugzilla – Bug 708847
Integrate WebKit2 permission requests for TLS errors
Last modified: 2014-09-29 17:11:03 UTC
Integrate the new WK2 API for TLS errors into Ephy. See https://bugs.webkit.org/show_bug.cgi?id=120160 for details of the implementation.
Created attachment 255846 [details] [review] WIP patch
Review of attachment 255846 [details] [review]: This is a work in progress which I wanted to upload while I still have a chance to work on this. ::: embed/ephy-web-view.c @@ +1352,3 @@ + int response, + WebKitPermissionRequest *request) +{ The info_bar widget here is taken from the geolocation request and is likely to be replaced with the form approach similar to delete_web_app. @@ +1421,3 @@ + else if (WEBKIT_IS_TLS_PERMISSION_REQUEST (decision)) + { + printf("\n*** [%s:%i] %s ***\n", __FILE__, __LINE__, __FUNCTION__); Please forgive the printf (test code)
I think we don't want to give the user an option to bypass TLS errors, because he will use it [1], which defeats the purpose of TLS. I'm not sure if there's any designs for what to actually do. I imagine displaying a variant of the "Oh no something has gone wrong" page with an evil face instead of a sad face. [1] http://www.superlectures.com/guadec2013/more-secure-with-less-security
(In reply to comment #3) > I think we don't want to give the user an option to bypass TLS errors, because > he will use it [1], which defeats the purpose of TLS. > > I'm not sure if there's any designs for what to actually do. I imagine > displaying a variant of the "Oh no something has gone wrong" page with an evil > face instead of a sad face. > > [1] http://www.superlectures.com/guadec2013/more-secure-with-less-security Great talk, thanks for the link. Have you tried loading a page like https://bugs.gentoo.org in Ephy? The security certificate is invalid, so the question you have now is what should Ephy do? Should it (a) fail always (remove any option to bypass security for the user), (b) ignore the errors or (c) load an error page with a button saying something like "Load anyway" I think you'll find that most other browsers do (c), which is what the patch provided does as well.
I'm actually not aware of any browser that does not do (c). But I also don't think that's an acceptable option for GNOME. Very few people are going to say "no don't load that site," and they will lose their passwords and credit card numbers to the man in the middle. By presenting that option, browsers become complicit in the attack. GNOME should aspire to be better than that. (Of course, (c) is undoubtedly far, far superior to the current behavior, (b), so I do think this patch is very important.) The "load anyway" button made sense back when self-signed or expired certs were relatively common, but legitimate sites don't (or should not) have those issues anymore. Regarding bugs.gentoo.org... well I suppose there is an exception to every rule. I don't have an answer to or an opinion on the CAcert dilemma in general, but I believe that websites that use CAcert ought to be intelligent enough not to redirect HTTP requests to HTTPS. If your website insists on a secure connection, and relies on a root authority that is not generally trusted, then I think Epiphany would be absolutely right to block access to that site. Everyone visiting it in Chrome and Firefox must be getting the "this site is evil" warning the first time, until they click through it... and the Gentoo developers are OK with that? Well, we should not be.
I think we need to involve Stef and the design team in this if we want to make a good decision.
I agree with Michael, that GNOME with our focus on security and privacy should aspire to do better that browsers have typically done. That said, the browser is the hardest place to get this right, due to the myriad of security use cases that a browser in used for, so I don't think we should feel bad if it takes one or two iterations to get done right. There is no question that the user sometimes needs to load a site whose certificate is not signed by the traditional pre-installed anchor certificate authorities. This breaks down into one of two broad use cases: 1. Non-standard CA use case: * A corporate or enterprise site for which a non-standard CA anchor is available. * We should be directing the user to instructions to install that non-standard CA, or ask for help from their admin. * This covers the majority of legitimate cases where SSL verification fails. * This also covers the CACert use case. 2. Self-signed certificate, on a security sensitive site * An admin has configured his site with an SSL self-signed certificate, and made the site available to others. * This is an advanced use case, and does not need to be made super simple for users. * This could be analogous to using a non-standard port for HTTP. The admin of such a site needs to provide its users with the somewhat complex information to make this work. In the first use case, for enterprise use cases, the ideal solution would have been to have an administrator aware of Linux/GNOME and have preinstalled the necessary certificate anchor. But obviously if the user got to this point, then the above didn't apply or happen. So in both cases we want the user to be able to solve the problem (ideally with help from the admin). But what we don't want is a prompt-ignore-and-continue situation. In my opinion, for use case 1 we want to build a UI in seahorse for installing a CA anchor, and link people to a document which helps them accomplish that, once and for all. This is really a much better approach for Enterpise or CACert users as they never see another prompt for another host using a similarly signed certificate. For use case 2 I think we want some UI in epiphany, perhaps a bookmark UI, which lets the user create a bookmark to a site, and associate the expected certificate with that bookmark. When clicking on such a bookmark epiphany would expect to validate the host using that certificate, and only that certificate. This allows advanced users to have the additional benefit of being discovering when the certificate changes. I gave a talk about this at GUADEC, but as noted above I'm completely open to iterations on designs to solve these use cases. What I really do want to avoid is interrupting the user to prompt them about a security question. http://www.superlectures.com/guadec2013/more-secure-with-less-security What do you think?
As a relative newcomer to GNOME I don't have a strong opinion on this topic, except to say that I would be quite frustrated with a browser that didn't allow me to load a page even if I knew that the risk was minimal (e.g. https://bugs.gentoo.org).
(In reply to comment #8) > As a relative newcomer to GNOME I don't have a strong opinion on this topic, > except to say that I would be quite frustrated with a browser that didn't allow > me to load a page even if I knew that the risk was minimal (e.g. > https://bugs.gentoo.org). I agree, it's sad but it's quite common to find web sites with invalid certificates that I really want/need to access (under my own responsibility, of course) I agree we should not load those pages by default like we currently do.
http://www2013.wwwconference.org/proceedings/p59.pdf might be interesting reading relevant to this topic...
I'm having trouble imagining how UI for use case 2 would be integrated into Epiphany. On the bright side, anything at all would be better than the current behavior (ignore the error). Brian, that paper is definitely interesting and relevant. My TL;DR book report: (1) it concludes that "unknown issuer" errors like bugs.gentoo.org are high-risk situations, and unfortunately represent the majority of TLS errors on the web. (2) The paper recommends some changes to certificate validation to ignore common, low-risk errors, like www.example.com != example.com, *.*.example.com != *.example.com, etc. These recommendations seem relevant to GnuTLS. (3) It presents incomplete certificate chains as a serious issue, and shows that caching intermediate certificates significantly reduces TLS errors. It claims all major browsers do this, but Epiphany does not. It also recommends supporting AIA during certificate validation.
(In reply to comment #8) > As a relative newcomer to GNOME I don't have a strong opinion on this topic, > except to say that I would be quite frustrated with a browser that didn't allow > me to load a page even if I knew that the risk was minimal (e.g. > https://bugs.gentoo.org). First of all, bugs.gentoo.org uses CACert. Just install it using your distro's package (most have one) or using '$ trust anchor /path/to/cacert.crt'. This is "case 1" above. To stop ignoring security, and install the CA authority. If your distro does not yet have p11-kit integrated (ie: the 'trust' tool above, then that's the way forward). But in a general case, where actual self-signed certificates are in use (ie: not bugs.gentoo.org) it should be possible to allow this, but not just with a "Load Anyway" button. The reason goverments and hackers can work around encryption on the internet is not (usually) because of broken protocols, but because of broken surrounding implementation issues: such as "Load Anyway" buttons, which cause users to ignore security *precisely* when they're being targeted.
(In reply to comment #11) > I'm having trouble imagining how UI for use case 2 would be integrated into > Epiphany. For 'case 2' What I would suggest is the following. In the bookmark 'Properties' and 'Add bookmark' dialogs, have an "Advanced Security" button, which provides the functionality to pin a certificate (always expect that certificate) to that bookmark. Would you like me to mock-up a design?
(In reply to comment #13) > (In reply to comment #11) > > I'm having trouble imagining how UI for use case 2 would be integrated into > > Epiphany. > > For 'case 2' What I would suggest is the following. In the bookmark > 'Properties' and 'Add bookmark' dialogs, have an "Advanced Security" button, > which provides the functionality to pin a certificate (always expect that > certificate) to that bookmark. > > Would you like me to mock-up a design? Sounds good to me. FYI, after a long discussion we at WebKitGtk+ have decided not to use the permission-request model for tls errors, and instead to emit a signal load-failed-with-tls-errors with a WebKitCertificateInfo struct containing the cert, flags and host when the policy is WEBKIT_TLS_ERRORS_POLICY_ASK. When its in and reviewed I hope to help to integrate it here.
Somewhat related: when a cert check fails, it would be really nice to know more about why; currently in Epiphany you see a simple reason like "The signing certificate authority is not known," but that's not much much compared to e.g. Rekonq which shows you information about every cert in the chain so you can see exactly where the chain of trust is broken. I know we don't want to make Epiphany UI too complicated, but the certificate info viewer is of limited usefulness currently.
(In reply to comment #14) > FYI, after a long discussion we at WebKitGtk+ have decided not to use the > permission-request model for tls errors, and instead to emit a signal > load-failed-with-tls-errors with a WebKitCertificateInfo struct containing the > cert, flags and host when the policy is WEBKIT_TLS_ERRORS_POLICY_ASK. When its > in and reviewed I hope to help to integrate it here. Hey Brian, thanks for your work on this. Any recent progress? This would be great to have before 3.12, either with or without the Load Anyway button.
(In reply to comment #16) > Hey Brian, thanks for your work on this. Any recent progress? This would be > great to have before 3.12, either with or without the Load Anyway button. The API has landed in WebKitGTK, so its just a question of what policy should be implemented. Reading through all the comments above again, I'm left with the sense that policy should be to fail on TLS errors webkit_web_context_set_tls_errors_policy(context, WEBKIT_TLS_ERRORS_POLICY_FAIL) , unless there is a known certificate for the site (e.g. stored in the bookmarks) in which case we add the exception with webkit_web_context_allow_tls_certificate_for_host() before loading the page. If we did this the user will never be prompted to "Load anyway" but has a clear way to allow a site to load with a given certificate if needed, which I think addresses both of Stef's use cases. I don't have much capacity to work on this given downstream requirements, so please feel free to continue where my WIP patch left off. Did anything come of the UI mockup for the bookmarks?
*** Bug 728300 has been marked as a duplicate of this bug. ***
*** Bug 542454 has been marked as a duplicate of this bug. ***
Michael, I just looked now one the code in the tls-errors branch, and I have one comment (before I run the code): Remove markup in translatable messages, as on of the GNOME Goals: https://wiki.gnome.org/Initiatives/GnomeGoals/RemoveMarkupInMessages
Fixed that, thanks Yosef
(In reply to comment #21) > Fixed that, thanks Yosef Thanks!
Created attachment 280439 [details] [review] Do not ignore TLS errors Currently, Epiphany loads web pages even though it realizes the connection may be insecure, displaying a broken lock in the address bar. By this point, it's too late: the attacker already has your session cookies. Display an error page instead. Based on groundwork by Brian Holt. This is the minimal reasonable implementation. We might also want: * A user-friendly description * A button to open the certificate viewer * A mechanism by which to add the certificate to the trust database
Created attachment 280440 [details] [review] Add custom certificate authentication error page Explain to the user why loading an untrusted page might be dangerous. It might be desirable to additionally explain the particular type of failure (lack of trust, mismatched URI, expired or revoked cert, etc.) This patch is based on groundwork by Brian Holt.
Created attachment 280441 [details] [review] TLS error page: Add button to bypass error If the user experiences a TLS error, he needs some way to override it. We can either give the user a button that allows him to click straight through to the website, like Chromium does, or send him to a new dialog designed to explain the implications of adding a security exception, like Firefox does. Another possibility is to disallow exceptions altogether and require the user to install the certificate. See bug 708847 for discussion. We're going to take the Chromium approach for now. This can be revisited in the future.
Created attachment 280442 [details] [review] TLS error page: Add button to bypass error Fix formatting errors
Created attachment 280452 [details] [review] TLS error page: Add buttons to bypass error or open cert viewer If the user experiences a TLS error, he needs some way to override it. We can either give the user a button that allows him to click straight through to the website, like Chromium does, or send him to a new dialog designed to explain the implications of adding a security exception, like Firefox does. Another possibility is to disallow exceptions altogether and require the user to install the certificate. See bug 708847 for discussion. We're going to take the Chromium approach for now. This can be revisited in the future. Also, add the ability to open the cert viewer to get technical details on why the cert did not validate.
Review of attachment 280439 [details] [review]: This should probably squashed in the next commit that adds the error page.
Review of attachment 280440 [details] [review]: ::: embed/ephy-web-view.c @@ +1869,3 @@ +static gboolean +tls_error_cb (WebKitWebView *web_view, I would call this load_failed_with_tls_error_cb @@ +1933,3 @@ + uri = webkit_web_view_get_uri (web_view); + webkit_web_view_load_alternate_html (WEBKIT_WEB_VIEW (view), html->str, uri, 0); + g_string_free (html, TRUE); There's too much code duplicated here. I think we could somehoe add a new kind of error page for TLS errors and adapt ephy_web_view_load_error_page if needed to support all kind of error pages.
Review of attachment 280452 [details] [review]: Instead of adding another button to open the cert viewer, we could set the security level also when loading the TLS error page, so the you can open the cert dialog from the URL icon. I think it's more consistent and the code is a lot simpler. We can add further technical details in an expander like chromium does. ::: embed/ephy-embed-shell.c @@ +283,3 @@ + EphyEmbedShell *shell) +{ + g_signal_emit (shell, signals[ALLOW_TLS_CERTIFICATE], 0); This should be emitted by EphyWebExtensionProxy, here you get the page_id from the signal args, and looks for the web extension calling ephy_embed_shell_find_web_extension. See how web_extension_form_auth_save_requested works. @@ +866,3 @@ + 0, NULL, NULL, + g_cclosure_marshal_VOID__VOID, + G_TYPE_NONE, 0); This signal should be in EphyWebExtensionProxy. ::: embed/ephy-web-view.c @@ +111,3 @@ + gboolean loading_tls_error_page; + /* Valid only if we are currently viewing the TLS error page */ + WebKitCertificateInfo *tls_error_page_info; I would call this certificate_info to make it clear this is the webkit certificate info we save only to be able to add an exception for it. I wonder if it would be simpler if we add webkit_certificate_info_new() to webkit that receives certificate and tls_errors, and we could simply reuse certificate and tls_errors members. Then we could simply create a new WebKitCertificate right before calling webkit_web_context_allow_tls_certificate_for_host(). @@ +770,3 @@ + } else { + g_warn_if_fail (priv->tls_error_page_host == NULL); + } Here we can simply g_clear_pointer(&priv->tls_error_page_info, webkit_certificate_info_free); @@ +773,3 @@ + + if (priv->tls_error_page_host != NULL) + g_free (priv->tls_error_page_host); g_free is null safe and finalize is called once, so you can simply call g_free here. @@ +1629,3 @@ + priv->tls_error_page_host = NULL; + } + } And we can simply all this too, by reusing the certificate and tls_errors and setting the security level also for the tls errors page. We could do something like: /* Security status. */ g_clear_object (&priv->certificate); priv->tls_errors = 0; if (priv->loading_tls_error_page) { priv->loading_tls_error_page = FALSE; priv->tls_errors = webkit_certificate_info_get_tls_errors (priv->tls_error_page_info); priv->certificate = g_object_ref (webkit_certificate_info_get_tls_certificate (priv->tls_error_page_info)); } else if (webkit_web_view_get_tls_info (web_view, &priv->certificate, &priv->tls_errors)) { g_object_ref (priv->certificate); } security_level = priv->tls_errors == 0 ? EPHY_WEB_VIEW_STATE_IS_SECURE_HIGH : EPHY_WEB_VIEW_STATE_IS_BROKEN; ephy_web_view_set_security_level (EPHY_WEB_VIEW (web_view), security_level); If we could reuse certificate and tls_errors instead of saving the WebKitCertificateInfo, this would be even simpler @@ +1925,3 @@ + if (priv->tls_error_page_info != NULL) + webkit_certificate_info_free (priv->tls_error_page_info); g_clear_pointer. @@ +1927,3 @@ + webkit_certificate_info_free (priv->tls_error_page_info); + if (priv->tls_error_page_host != NULL) + g_free (priv->tls_error_page_host); g_clear_pointer @@ +1929,3 @@ + g_free (priv->tls_error_page_host); + priv->loading_tls_error_page = TRUE; + priv->tls_error_page_info = webkit_certificate_info_copy (info); If we could reuse certificate and tls_errors instead of saving the WebKitCertificateInfo, we would save certificate and tls_errors here diredctly. @@ +2951,3 @@ + **/ +void +ephy_web_view_allow_certificate_exception (EphyWebView *view) Instead of a public method, this should be the callback of the allow-tls-certificate signal emitted by EphyWebPageProxy. The signal should receive the page_id, so you can check this is actually for this particular web view (in single web process model, the same extension proxy si shared by all web views). ::: embed/web-extension/ephy-web-extension.c @@ +1269,3 @@ + EPHY_WEB_EXTENSION_INTERFACE, + "AllowTLSCertificate", + NULL, We should pass the page id as parameter @@ +1340,3 @@ + + class = JSClassCreate (&class_def); + class_object = JSObjectMake (context, class, extension); Instead of passing only the extension as private data of the object, we could pass a custom struct containing the extention and the page id, and use that page id as parameter of the signal. Then you would also need to define JSClassDefinition::finalize to free the private struct ::: src/ephy-window.c @@ +3088,2 @@ static void +allow_tls_certificate_cb (EphyEmbedShell *embed_shell, This could be done by the web view internally if the signal is emitted for a particular page
Review of attachment 280452 [details] [review]: ::: embed/web-extension/ephy-web-extension.c @@ +1377,3 @@ + + /* EphyWebView ensures this JS can only take effect when used from our TLS error page. */ + prepare_certificate_exception_js (world, web_page, frame, extension); I'm thinking that it would be much better if we detect here if this is the TLS error page or not, to avoid injecting this custom JS for every page we load. We might use DOM API for this, a very simple approach could be to check the document url and title. If webkit_web_page_get_uri is not about:blank and webkit_dom_document_get_url is about:blank, is becase we are likely loading alternate content. Then we could check the title, or we can add a page identifier somewhere in the html, for example in the body, and here we get the body with webkit_dom_document_get_body() and then check its ID. There might be better ways, though.
See https://bugs.webkit.org/show_bug.cgi?id=134830
Created attachment 280889 [details] [review] Do not ignore TLS errors Thanks for the detailed review! This patch handles the web view security status on the error page a bit differently than you suggested, leaving it unchanged from the failed load. The trick with comparing webkit_web_page_get_uri() with webkit_dom_document_get_url() to decide whether to create the custom js worked great, but calling webkit_dom_document_get_url() inside window_object_cleared_cb() returns NULL (why?) and checking the title would not work well because the second half of the title varies based on the page we're visiting, and the first half is translated, so I currently only have the first check in there. That's probably enough to restrict its use to Epiphany's own error pages, though, right?
(In reply to comment #33) > but calling webkit_dom_document_get_url() inside > window_object_cleared_cb() returns NULL (why?) I meant webkit_dom_document_get_body()
Created attachment 280891 [details] [review] Do not ignore TLS errors
Created attachment 280893 [details] [review] Do not ignore TLS errors
Created attachment 282351 [details] [review] Do not ignore TLS errors Updated for WebKitCertificateInfo API removal
Created attachment 282352 [details] [review] Do not ignore TLS errors Fix leak in previous patch (oops)
Review of attachment 282352 [details] [review]: Ok, this looks much better, I have only a few more comments ::: embed/ephy-web-view.c @@ +1622,3 @@ + } + + ephy_web_view_set_security_level (EPHY_WEB_VIEW (web_view), security_level); So, we don't set the security_level for the tls error page? As I said I think we should set the security level so that you can check the certificate from the url bar icon. At this point we should already have the certificate and errors saved. @@ +1747,3 @@ + /* Good advice from Firefox; displays when a site's TLS certificate is invalid. */ + _("Legitimate banks, stores, and other public sites will " + "not ask you to do this.")); Is all this information true/accurate for all possible certificate errors? I think we should probably show a different error message depending on the error. @@ +1854,3 @@ + + if (load_anyway_js == NULL) + load_anyway_js = g_strconcat ("window.location = '", uri, "';", NULL); maybe g_strdup_printf ("window.location='%s';", uri); is easier to read than the concat @@ +1963,3 @@ + priv->tls_errors = errors; + priv->tls_error_page_host = g_strdup (host); + ephy_web_view_set_security_level (EPHY_WEB_VIEW (web_view), EPHY_WEB_VIEW_STATE_IS_BROKEN); Ah, you are doing it here, ok. ::: embed/web-extension/ephy-web-extension.c @@ +1252,3 @@ + EphyWebExtension *extension; + guint64 page_id; +} allow_tls_certificate_cb_data_t; allow_tls_certificate_cb_data_t -> AllowTLSCertificateData @@ +1297,3 @@ +tls_certificate_error_page_finalize (JSObjectRef object) +{ + g_free (JSObjectGetPrivate (object)); Use g_slice_free @@ +1312,3 @@ + JSObjectRef class_object; + JSStringRef str; + allow_tls_certificate_cb_data_t *cb_data; cb_data -> data This is private data of the class, not only for the allow TLS method. @@ +1322,3 @@ + class_def.finalize = tls_certificate_error_page_finalize; + + cb_data = g_new (allow_tls_certificate_cb_data_t, 1); And g_slice_new here @@ +1371,3 @@ + + if (g_strcmp0 (webkit_web_page_get_uri (web_page), "about:blank") != 0 && + g_strcmp0 (dom_url, "about:blank") == 0) { We should probably add a comment here explaining this hack :-)
Is there some rule for when to use g_slice_alloc instead of g_new? > @@ +1747,3 @@ > + /* Good advice from Firefox; displays when a site's > TLS certificate is invalid. */ > + _("Legitimate banks, stores, and other public sites > will " > + "not ask you to do this.")); > > Is all this information true/accurate for all possible certificate errors? I > think we should probably show a different error message depending on the error. We should definitely display at least a generic reason why the cert was blocked ("The site's identification has expired" or "The site's identification is only valid for a different site"); I just forgot to do this. :( At least the certificate dialog should provide a more detailed explanation (e.g. "This certificate expired on May 1, 2014") but I don't think the main error page should need to mention what a certificate is, since that's a confusing technical detail. Anyway, I picked this text since it's what Firefox uses to get the point across that continuing is dangerous. It's usually true but there are exceptions: if (a) the site is misconfigured (nowadays only common if a site is offered via HTTPS but not by default, so the user had to manually type https into the address bar) or (b) the site is not public, or not intended for use without prior configuration (see comment #7). Practically, there are a couple types of misconfigurations that are less likely to represent security threats: * The site sends an illegal unordered certificate chain. All major browsers reorder these chains, but we don't. Major problem, bug #683266. * If a cert has expired within the past week, probably the site admin has just not updated it yet by mistake. All major browsers block these sites, so it's OK that we do too. * If a cert is for a subdomain of the current site, e.g. you are on example.com but the cert presented is for www.example.com or *.example.com. Or if you are visiting one.two.example.com but the cert is for *.example.com instead of *.*.example.com. All major browsers block these subdomains, so it's OK that we do too, but I'm really unconvinced that it's the right thing to do. Anything besides those two errors is probably either (a) an attacker, or else (b) a chain that's hopelessly broken and the site admin just never noticed because the missing cert is often cached by users' browsers. Most major browsers cache intermediate certs when they're validated, so an intermediate cert effectively becomes a CA root. It'd be independently valuable for us to cache intermediate certificates -- I think glib-networking's WIP NSS backend addresses this issue -- but this is just a fallback for broken sites. I'm pretty sure we need to fail expired certs, since allowing certs one week past expiration accomplishes nothing besides moving back the effective expiration date a bit. I'm imagining some sort of future expiration date war between browsers, each browser trying to allow certs just a bit longer than its competitors to ensure the certs fail in some other browser first. Epiphany might be small enough to get away with it, but I think it'd be bad to change this any lower in the stack. I think allowing certs that are valid for a parent domain but not the current subdomain would be safe, since it's not typical that users would actually want to protect information on subdomain.example.com from example.com. I don't think it's a priority compared to some of our other bugs I'd rather work. Another reason to hesitate is that I'm not sure it's a great idea for our behavior to differ from every major browser (conceivably some sites are actually structured so that subdomains want to protect information from the parent domain; still, this seems unlikely). But if other browsers begin to allow such certs in the future, then it would definitely be a priority for us to do so as well.
Created attachment 282729 [details] [review] Add helper functions for printing TLS errors Previously this was only needed by the certificate dialog. Now we need these error messages to be available on the TLS errors page as well.
Created attachment 282730 [details] [review] Do not ignore TLS errors Currently, Epiphany loads web pages even though it realizes the connection may be insecure, displaying a broken lock in the address bar. By this point, it's too late: the attacker already has your session cookies. Display an error page instead. Based on groundwork by Brian Holt.
Review of attachment 282729 [details] [review]: I'm not sure about this, because in the end this is not actually shared, the dialog uses one method and the error page the other.
Review of attachment 282730 [details] [review]: This looks great, there's only one detail. ::: embed/ephy-web-view.c @@ +1736,3 @@ + + bold_hostname = g_strconcat ("<strong>", hostname, "</strong>", NULL); + details = ephy_certificate_get_simple_error_messages_from_tls_errors (priv->tls_errors); Instead of a text string with the bullets, I think we could use actual HTML here, if we get the array of errors we could add a list in HTML. We could add a function that returns the HTML directly from the GTlsErrorFlags, and we don't need the previous patch at all.
Created attachment 282844 [details] [review] Do not ignore TLS errors Currently, Epiphany loads web pages even though it realizes the connection may be insecure, displaying a broken lock in the address bar. By this point, it's too late: the attacker already has your session cookies. Display an error page instead. Based on groundwork by Brian Holt.
Using HTML does make more sense. Is this what you had in mind? Also: (In reply to comment #40) > Is there some rule for when to use g_slice_alloc instead of g_new?
Review of attachment 282844 [details] [review]: Yes!
(In reply to comment #46) > Using HTML does make more sense. Is this what you had in mind? > > Also: > > (In reply to comment #40) > > Is there some rule for when to use g_slice_alloc instead of g_new? slice allocator is normally used when you allocate small structs and more than one, like in a linked list, but I think it's always safe to use it anyway.
Attachment 282844 [details] pushed as f0e7ab8 - Do not ignore TLS errors
*** Bug 737589 has been marked as a duplicate of this bug. ***