From 147eb058b97faf47b186d105b9adf91cb1306b58 Mon Sep 17 00:00:00 2001
From: Florian Bruhin <me@the-compiler.org>
Date: Fri, 19 Mar 2021 15:55:00 +0100
Subject: Add content.tls.certificate_errors = ask-block-thirdparty

Fixes #3418
---
 doc/help/settings.asciidoc                         |  1 +
 qutebrowser/browser/shared.py                      | 51 ++++++++++++++++++----
 qutebrowser/browser/webengine/webenginetab.py      | 23 +++++++---
 .../browser/webkit/network/networkmanager.py       | 48 ++++++++++++++------
 qutebrowser/config/configdata.yml                  |  2 +
 tests/end2end/data/prompt/script.js                |  5 +++
 tests/end2end/features/prompts.feature             | 38 ++++++++++++++++
 tests/end2end/features/test_prompts_bdd.py         | 11 +++++
 tests/end2end/fixtures/webserver_sub.py            |  6 +++
 tests/end2end/fixtures/webserver_sub_ssl.py        |  5 +++
 tests/end2end/templates/https-script.html          | 11 +++++
 11 files changed, 171 insertions(+), 30 deletions(-)
 create mode 100644 tests/end2end/data/prompt/script.js
 create mode 100644 tests/end2end/templates/https-script.html

diff --git a/doc/help/settings.asciidoc b/doc/help/settings.asciidoc
index d4089c915..3741589e7 100644
--- a/doc/help/settings.asciidoc
+++ b/doc/help/settings.asciidoc
@@ -2638,6 +2638,7 @@ Type: <<types,String>>
 Valid values:
 
  * +ask+: Ask how to proceed for every certificate error (unless non-overridable due to HSTS).
+ * +ask-block-thirdparty+: Ask how to proceed for normal page loads, but silently block resource loads.
  * +block+: Automatically block loading on certificate errors.
  * +load-insecurely+: Force loading pages despite certificate errors. This is *insecure* and should be avoided. Instead of using this, consider fixing the underlying issue or importing a self-signed certificate via `certutil` (or Chromium) instead.
 
diff --git a/qutebrowser/browser/shared.py b/qutebrowser/browser/shared.py
index 532ccb91b..36f85e486 100644
--- a/qutebrowser/browser/shared.py
+++ b/qutebrowser/browser/shared.py
@@ -156,34 +156,61 @@ def javascript_log_message(level, source, line, msg):
 
 
 def ignore_certificate_error(
-        url: QUrl,
+        *,
+        request_url: QUrl,
+        first_party_url: QUrl,
         error: usertypes.AbstractCertificateErrorWrapper,
         abort_on: Iterable[pyqtBoundSignal],
 ) -> bool:
     """Display a certificate error question.
 
     Args:
-        url: The URL the errors happened in
-        errors: A single error.
+        request_url: The URL of the request where the errors happened.
+        first_party_url: The URL of the page we're visiting. Might be an invalid QUrl.
+        error: A single error.
         abort_on: Signals aborting a question.
 
     Return:
         True if the error should be ignored, False otherwise.
     """
-    conf = config.instance.get('content.tls.certificate_errors', url=url)
+    conf = config.instance.get('content.tls.certificate_errors', url=request_url)
     log.network.debug(f"Certificate error {error!r}, config {conf}")
 
     assert error.is_overridable(), repr(error)
 
-    if conf == 'ask':
+    # We get the first party URL with a heuristic - with HTTP -> HTTPS redirects, the
+    # scheme might not match.
+    is_resource = (
+        first_party_url.isValid() and
+        not request_url.matches(first_party_url, QUrl.RemoveScheme))
+
+    if conf == 'ask' or conf == 'ask-block-thirdparty' and not is_resource:
         err_template = jinja.environment.from_string("""
-            <p>Error while loading <b>{{url.toDisplayString()}}</b>:</p>
+            {% if is_resource %}
+            <p>
+                Error while loading resource <b>{{request_url.toDisplayString()}}</b><br/>
+                on page <b>{{first_party_url.toDisplayString()}}</b>:
+            </p>
+            {% else %}
+            <p>Error while loading page <b>{{request_url.toDisplayString()}}</b>:</p>
+            {% endif %}
 
             {{error.html()|safe}}
+
+            {% if is_resource %}
+            <p><i>Consider reporting this to the website operator, or set
+            <tt>content.tls.certificate_errors</tt> to <tt>ask-block-thirdparty</tt> to
+            always block invalid resource loads.</i></p>
+            {% endif %}
         """.strip())
-        msg = err_template.render(url=url, error=error)
+        msg = err_template.render(
+            request_url=request_url,
+            first_party_url=first_party_url,
+            is_resource=is_resource,
+            error=error,
+        )
 
-        urlstr = url.toString(QUrl.RemovePassword | QUrl.FullyEncoded)
+        urlstr = request_url.toString(QUrl.RemovePassword | QUrl.FullyEncoded)
         ignore = message.ask(title="Certificate error - continue?", text=msg,
                              mode=usertypes.PromptMode.yesno, default=False,
                              abort_on=abort_on, url=urlstr)
@@ -196,7 +223,13 @@ def ignore_certificate_error(
         return True
     elif conf == 'block':
         return False
-    raise utils.Unreachable(conf)
+    elif conf == 'ask-block-thirdparty' and is_resource:
+        log.network.error(
+            f"Certificate error in resource load: {error}\n"
+            f"  request URL:     {request_url.toDisplayString()}\n"
+            f"  first party URL: {first_party_url.toDisplayString()}")
+        return False
+    raise utils.Unreachable(conf, is_resource)
 
 
 def feature_permission(url, option, msg, yes_action, no_action, abort_on,
diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py
index 3c113e55c..0d78324ec 100644
--- a/qutebrowser/browser/webengine/webenginetab.py
+++ b/qutebrowser/browser/webengine/webenginetab.py
@@ -1516,11 +1516,22 @@ class WebEngineTab(browsertab.AbstractTab):
         url = error.url()
         self._insecure_hosts.add(url.host())
 
+        # WORKAROUND for https://bugreports.qt.io/browse/QTBUG-92009
+        # self.url() is not available yet and the requested URL might not match the URL
+        # we get from the error - so we just apply a heuristic here.
+        assert self.data.last_navigation is not None
+        first_party_url = self.data.last_navigation.url
+
         log.network.debug("Certificate error: {}".format(error))
+        log.network.debug("First party URL: {}".format(first_party_url))
 
         if error.is_overridable():
             error.ignore = shared.ignore_certificate_error(
-                url, error, abort_on=[self.abort_questions])
+                request_url=url,
+                first_party_url=first_party_url,
+                error=error,
+                abort_on=[self.abort_questions],
+            )
         else:
             log.network.error("Non-overridable certificate error: "
                               "{}".format(error))
@@ -1544,12 +1555,10 @@ class WebEngineTab(browsertab.AbstractTab):
 
         # We can't really know when to show an error page, as the error might
         # have happened when loading some resource.
-        # However, self.url() is not available yet and the requested URL
-        # might not match the URL we get from the error - so we just apply a
-        # heuristic here.
-        assert self.data.last_navigation is not None
-        if (show_non_overr_cert_error and
-                url.matches(self.data.last_navigation.url, QUrl.RemoveScheme)):
+        is_resource = (
+            first_party_url.isValid() and
+            url.matches(first_party_url, QUrl.RemoveScheme))
+        if show_non_overr_cert_error and is_resource:
             self._show_error_page(url, str(error))
 
     @pyqtSlot()
diff --git a/qutebrowser/browser/webkit/network/networkmanager.py b/qutebrowser/browser/webkit/network/networkmanager.py
index a600667e9..130e151bd 100644
--- a/qutebrowser/browser/webkit/network/networkmanager.py
+++ b/qutebrowser/browser/webkit/network/networkmanager.py
@@ -214,6 +214,25 @@ class NetworkManager(QNetworkAccessManager):
             abort_on.append(tab.load_started)
         return abort_on
 
+    def _get_tab(self):
+        """Get the tab this NAM is associated with.
+
+        Return:
+            The tab if available, None otherwise.
+        """
+        # There are some scenarios where we can't figure out current_url:
+        # - There's a generic NetworkManager, e.g. for downloads
+        # - The download was in a tab which is now closed.
+        if self._tab_id is None:
+            return
+
+        assert self._win_id is not None
+        try:
+            return objreg.get('tab', scope='tab', window=self._win_id, tab=self._tab_id)
+        except KeyError:
+            # https://github.com/qutebrowser/qutebrowser/issues/889
+            return None
+
     def shutdown(self):
         """Abort all running requests."""
         self.setNetworkAccessible(QNetworkAccessManager.NotAccessible)
@@ -254,7 +273,16 @@ class NetworkManager(QNetworkAccessManager):
             return
 
         abort_on = self._get_abort_signals(reply)
-        ignore = shared.ignore_certificate_error(reply.url(), errors, abort_on=abort_on)
+
+        tab = self._get_tab()
+        first_party_url = QUrl() if tab is None else tab.data.last_navigation.url
+
+        ignore = shared.ignore_certificate_error(
+            request_url=reply.url(),
+            first_party_url=first_party_url,
+            error=errors,
+            abort_on=abort_on,
+        )
         if ignore:
             reply.ignoreSslErrors()
             if host_tpl is not None:
@@ -387,22 +415,14 @@ class NetworkManager(QNetworkAccessManager):
         for header, value in shared.custom_headers(url=req.url()):
             req.setRawHeader(header, value)
 
-        # There are some scenarios where we can't figure out current_url:
-        # - There's a generic NetworkManager, e.g. for downloads
-        # - The download was in a tab which is now closed.
+        tab = self._get_tab()
         current_url = QUrl()
-
-        if self._tab_id is not None:
-            assert self._win_id is not None
+        if tab is not None:
             try:
-                tab = objreg.get('tab', scope='tab', window=self._win_id,
-                                 tab=self._tab_id)
                 current_url = tab.url()
-            except (KeyError, RuntimeError):
-                # https://github.com/qutebrowser/qutebrowser/issues/889
-                # Catching RuntimeError because we could be in the middle of
-                # the webpage shutdown here.
-                current_url = QUrl()
+            except RuntimeError:
+                # We could be in the middle of the webpage shutdown here.
+                pass
 
         request = interceptors.Request(first_party_url=current_url,
                                        request_url=req.url())
diff --git a/qutebrowser/config/configdata.yml b/qutebrowser/config/configdata.yml
index 6fa1a1c15..762f42415 100644
--- a/qutebrowser/config/configdata.yml
+++ b/qutebrowser/config/configdata.yml
@@ -960,6 +960,8 @@ content.tls.certificate_errors:
     valid_values:
       - ask: Ask how to proceed for every certificate error (unless non-overridable due
             to HSTS).
+      - ask-block-thirdparty: Ask how to proceed for normal page loads, but silently
+            block resource loads.
       - block: Automatically block loading on certificate errors.
       - load-insecurely: Force loading pages despite certificate errors. This is
             *insecure* and should be avoided. Instead of using this, consider fixing the
diff --git a/tests/end2end/data/prompt/script.js b/tests/end2end/data/prompt/script.js
new file mode 100644
index 000000000..7e2425df5
--- /dev/null
+++ b/tests/end2end/data/prompt/script.js
@@ -0,0 +1,5 @@
+document.addEventListener('DOMContentLoaded', (event) => {
+    const elem = document.getElementById('text');
+    elem.textContent = 'Script loaded';
+    console.log('Script loaded');
+})
diff --git a/tests/end2end/features/prompts.feature b/tests/end2end/features/prompts.feature
index 10e7e4ceb..2bbef829b 100644
--- a/tests/end2end/features/prompts.feature
+++ b/tests/end2end/features/prompts.feature
@@ -202,6 +202,44 @@ Feature: Prompts
         And I run :mode-leave
         Then a SSL error page should be shown
 
+    Scenario: SSL error with content.tls.certificate_errors = ask-block-thirdparty -> yes
+        When I clear SSL errors
+        And I set content.tls.certificate_errors to ask-block-thirdparty
+        And I load an SSL page
+        And I wait for a prompt
+        And I run :prompt-accept yes
+        And I wait until the SSL page finished loading
+        Then the page should contain the plaintext "Hello World via SSL!"
+
+    Scenario: SSL resource error with content.tls.certificate_errors = ask -> yes
+        When I clear SSL errors
+        And I set content.tls.certificate_errors to ask
+        And I load an SSL resource page
+        And I wait for a prompt
+        And I run :prompt-accept yes
+        And I wait until the SSL resource page finished loading
+        Then the javascript message "Script loaded" should be logged
+        And the page should contain the plaintext "Script loaded"
+
+    Scenario: SSL resource error with content.tls.certificate_errors = ask -> no
+        When I clear SSL errors
+        And I set content.tls.certificate_errors to ask
+        And I load an SSL resource page
+        And I wait for a prompt
+        And I run :prompt-accept no
+        And I wait until the SSL resource page finished loading
+        Then the javascript message "Script loaded" should not be logged
+        And the page should contain the plaintext "Script not loaded"
+
+    Scenario: SSL resource error with content.tls.certificate_errors = ask-block-thirdparty
+        When I clear SSL errors
+        And I set content.tls.certificate_errors to ask-block-thirdparty
+        And I load an SSL resource page
+        And I wait until the SSL resource page finished loading
+        Then "Certificate error in resource load: *" should be logged
+        And the javascript message "Script loaded" should not be logged
+        And the page should contain the plaintext "Script not loaded"
+
     # Geolocation
 
     Scenario: Always rejecting geolocation
diff --git a/tests/end2end/features/test_prompts_bdd.py b/tests/end2end/features/test_prompts_bdd.py
index 3b42be4d0..46cc9c698 100644
--- a/tests/end2end/features/test_prompts_bdd.py
+++ b/tests/end2end/features/test_prompts_bdd.py
@@ -36,6 +36,17 @@ def wait_ssl_page_finished_loading(quteproc, ssl_server):
                                     load_status='warn')
 
 
+@bdd.when("I load an SSL resource page")
+def load_ssl_resource_page(quteproc, server, ssl_server):
+    # We don't wait here as we can get an SSL question.
+    quteproc.open_path(f'https-script/{ssl_server.port}', port=server.port, wait=False)
+
+
+@bdd.when("I wait until the SSL resource page finished loading")
+def wait_ssl_resource_page_finished_loading(quteproc, server, ssl_server):
+    quteproc.wait_for_load_finished(f'https-script/{ssl_server.port}', port=server.port)
+
+
 @bdd.when("I wait for a prompt")
 def wait_for_prompt(quteproc):
     quteproc.wait_for(message='Asking question *')
diff --git a/tests/end2end/fixtures/webserver_sub.py b/tests/end2end/fixtures/webserver_sub.py
index a4f54e19c..92f3091d1 100644
--- a/tests/end2end/fixtures/webserver_sub.py
+++ b/tests/end2end/fixtures/webserver_sub.py
@@ -261,6 +261,12 @@ def headers_link(port):
     return flask.render_template('headers-link.html', port=port)
 
 
+@app.route('/https-script/<int:port>')
+def https_script(port):
+    """Get a script loaded via HTTPS."""
+    return flask.render_template('https-script.html', port=port)
+
+
 @app.route('/response-headers')
 def response_headers():
     """Return a set of response headers from the query string."""
diff --git a/tests/end2end/fixtures/webserver_sub_ssl.py b/tests/end2end/fixtures/webserver_sub_ssl.py
index 354c05db0..05c44e85e 100644
--- a/tests/end2end/fixtures/webserver_sub_ssl.py
+++ b/tests/end2end/fixtures/webserver_sub_ssl.py
@@ -40,6 +40,11 @@ def hello_world():
     return "Hello World via SSL!"
 
 
+@app.route('/data/<path:path>')
+def send_data(path):
+    return webserver_sub.send_data(path)
+
+
 @app.route('/favicon.ico')
 def favicon():
     return webserver_sub.favicon()
diff --git a/tests/end2end/templates/https-script.html b/tests/end2end/templates/https-script.html
new file mode 100644
index 000000000..d24d9c77b
--- /dev/null
+++ b/tests/end2end/templates/https-script.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <meta charset="utf-8">
+        <title>HTTPS script</title>
+        <script src="https://localhost:{{ port }}/data/prompt/script.js"></script>
+    </head>
+    <body>
+        <p id="text">Script not loaded.</p>
+    </body>
+</html>
-- 
cgit v1.2.3-54-g00ecf