From 6697d692e1580953aa0531c1a13a5f3d496c9af6 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 10 Nov 2016 06:50:00 +0100 Subject: [PATCH] webengine: Initial SSL error support --- doc/help/settings.asciidoc | 2 - qutebrowser/browser/browsertab.py | 17 +++ qutebrowser/browser/shared.py | 47 ++++++++ qutebrowser/browser/webengine/webenginetab.py | 25 ++++- qutebrowser/browser/webengine/webview.py | 29 ++++- .../browser/webkit/network/networkmanager.py | 103 +++++------------- qutebrowser/browser/webkit/webkittab.py | 33 +++++- qutebrowser/config/configdata.py | 3 +- tests/end2end/features/prompts.feature | 7 +- tests/end2end/fixtures/quteprocess.py | 22 ++-- 10 files changed, 191 insertions(+), 97 deletions(-) diff --git a/doc/help/settings.asciidoc b/doc/help/settings.asciidoc index 9341676f5..159d173cb 100644 --- a/doc/help/settings.asciidoc +++ b/doc/help/settings.asciidoc @@ -797,8 +797,6 @@ Valid values: Default: +pass:[ask]+ -This setting is only available with the QtWebKit backend. - [[network-dns-prefetch]] === dns-prefetch Whether to try to pre-fetch DNS entries to speed up browsing. diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index 22f1c7e51..2589154a5 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -75,6 +75,23 @@ class UnsupportedOperationError(WebTabError): """Raised when an operation is not supported with the given backend.""" +class AbstractCertificateErrorWrapper: + + """A wrapper over an SSL/certificate error.""" + + def __init__(self, error): + self._error = error + + def __str__(self): + raise NotImplementedError + + def __repr__(self): + raise NotImplementedError + + def is_overridable(self): + raise NotImplementedError + + class TabData: """A simple namespace with a fixed set of attributes. diff --git a/qutebrowser/browser/shared.py b/qutebrowser/browser/shared.py index fbd53327c..417b741c6 100644 --- a/qutebrowser/browser/shared.py +++ b/qutebrowser/browser/shared.py @@ -21,6 +21,8 @@ import html +import jinja2 + from qutebrowser.config import config from qutebrowser.utils import usertypes, message, log @@ -110,3 +112,48 @@ def javascript_alert(url, js_msg, abort_on): html.escape(js_msg)) message.ask('Javascript alert', msg, mode=usertypes.PromptMode.alert, abort_on=abort_on) + + +def ignore_certificate_errors(url, errors, abort_on): + """Display a certificate error question. + + Args: + url: The URL the errors happened in + errors: A list of QSslErrors or QWebEngineCertificateErrors + + Return: + True if the error should be ignored, False otherwise. + """ + ssl_strict = config.get('network', 'ssl-strict') + log.webview.debug("Certificate errors {!r}, strict {}".format( + errors, ssl_strict)) + + for error in errors: + assert error.is_overridable(), repr(error) + + if ssl_strict == 'ask': + err_template = jinja2.Template(""" + Errors while loading {{url.toDisplayString()}}:
+ + """.strip()) + msg = err_template.render(url=url, errors=errors) + + return message.ask(title="Certificate errors - continue?", text=msg, + mode=usertypes.PromptMode.yesno, default=False, + abort_on=abort_on) + elif ssl_strict is False: + log.webview.debug("ssl-strict is False, only warning about errors") + for err in errors: + # FIXME we might want to use warn here (non-fatal error) + # https://github.com/The-Compiler/qutebrowser/issues/114 + message.error('Certificate error: {}'.format(err)) + return True + elif ssl_strict is True: + return False + else: + raise ValueError("Invalid ssl_strict value {!r}".format(ssl_strict)) + raise AssertionError("Not reached") diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 95b570b38..9ec89836a 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -30,7 +30,8 @@ from PyQt5.QtGui import QKeyEvent, QIcon # pylint: disable=no-name-in-module,import-error,useless-suppression from PyQt5.QtWidgets import QOpenGLWidget, QApplication from PyQt5.QtWebEngineWidgets import (QWebEnginePage, QWebEngineScript, - QWebEngineProfile) + QWebEngineProfile, + QWebEngineCertificateError) # pylint: enable=no-name-in-module,import-error,useless-suppression from qutebrowser.browser import browsertab, mouse, shared @@ -38,7 +39,7 @@ from qutebrowser.browser.webengine import (webview, webengineelem, tabhistory, interceptor, webenginequtescheme, webenginedownloads) from qutebrowser.utils import (usertypes, qtutils, log, javascript, utils, - objreg, message) + objreg, message, debug) _qute_scheme_handler = None @@ -78,6 +79,26 @@ _JS_WORLD_MAP = { } +class CertificateErrorWrapper(browsertab.AbstractCertificateErrorWrapper): + + """A wrapper over a QWebEngineCertificateError.""" + + def __init__(self, error): + self._error = error + + def __str__(self): + return self._error.errorDescription() + + def __repr__(self): + return utils.get_repr( + self, error=debug.qenum_key(QWebEngineCertificateError, + self._error.error()), + string=str(self)) + + def is_overridable(self): + return self._error.isOverridable() + + class WebEnginePrinting(browsertab.AbstractPrinting): """QtWebEngine implementations related to printing.""" diff --git a/qutebrowser/browser/webengine/webview.py b/qutebrowser/browser/webengine/webview.py index 52ca6de97..c8488bb4a 100644 --- a/qutebrowser/browser/webengine/webview.py +++ b/qutebrowser/browser/webengine/webview.py @@ -27,8 +27,9 @@ from PyQt5.QtWebEngineWidgets import QWebEngineView, QWebEnginePage # pylint: enable=no-name-in-module,import-error,useless-suppression from qutebrowser.browser import shared +from qutebrowser.browser.webengine import webenginetab from qutebrowser.config import config -from qutebrowser.utils import log, debug, usertypes, objreg, qtutils +from qutebrowser.utils import log, debug, usertypes, objreg, qtutils, jinja class WebEngineView(QWebEngineView): @@ -107,7 +108,7 @@ class WebEnginePage(QWebEnginePage): _is_shutting_down: Whether the page is currently shutting down. Signals: - certificate_error: FIXME:qtwebengine + certificate_error: Emitted on certificate errors. link_clicked: Emitted when a link was clicked on a page. shutting_down: Emitted when the page is shutting down. """ @@ -127,7 +128,29 @@ class WebEnginePage(QWebEnginePage): def certificateError(self, error): self.certificate_error.emit() - return super().certificateError(error) + url = error.url() + error = webenginetab.CertificateErrorWrapper(error) + + # FIXME + error_page = jinja.render('error.html', + title="Error while loading page", + url=url.toDisplayString(), error=str(error), + icon='', qutescheme=False) + + if not error.is_overridable(): + log.webview.error("Non-overridable certificate error: " + "{}".format(error)) + self.setHtml(error_page) + return False + + ignore = shared.ignore_certificate_errors( + url, [error], abort_on=[self.loadStarted, self.shutting_down]) + + if not ignore: + log.webview.error("Certificate error: {}".format(error)) + self.setHtml(error_page) + + return ignore def javaScriptConfirm(self, url, js_msg): if self._is_shutting_down: diff --git a/qutebrowser/browser/webkit/network/networkmanager.py b/qutebrowser/browser/webkit/network/networkmanager.py index 21f4f48bd..d4655d04a 100644 --- a/qutebrowser/browser/webkit/network/networkmanager.py +++ b/qutebrowser/browser/webkit/network/networkmanager.py @@ -24,7 +24,6 @@ import collections import netrc import html -import jinja2 from PyQt5.QtCore import (pyqtSlot, pyqtSignal, PYQT_VERSION, QCoreApplication, QUrl, QByteArray) from PyQt5.QtNetwork import (QNetworkAccessManager, QNetworkReply, QSslError, @@ -34,8 +33,9 @@ from qutebrowser.config import config from qutebrowser.utils import (message, log, usertypes, utils, objreg, qtutils, urlutils, debug) from qutebrowser.browser import shared -from qutebrowser.browser.webkit.network import webkitqutescheme, networkreply -from qutebrowser.browser.webkit.network import filescheme +from qutebrowser.browser.webkit import webkittab +from qutebrowser.browser.webkit.network import (webkitqutescheme, networkreply, + filescheme) HOSTBLOCK_ERROR_STRING = '%HOSTBLOCK%' @@ -112,24 +112,6 @@ def init(): QSslSocket.setDefaultCiphers(good_ciphers) -class SslError(QSslError): - - """A QSslError subclass which provides __hash__ on Qt < 5.4.""" - - def __hash__(self): - try: - # Qt >= 5.4 - # pylint: disable=not-callable,useless-suppression - return super().__hash__() - except TypeError: - return hash((self.certificate().toDer(), self.error())) - - def __repr__(self): - return utils.get_repr( - self, error=debug.qenum_key(QSslError, self.error()), - string=self.errorString()) - - class NetworkManager(QNetworkAccessManager): """Our own QNetworkAccessManager. @@ -203,6 +185,19 @@ class NetworkManager(QNetworkAccessManager): self.setCache(cache) cache.setParent(app) + def _get_abort_signals(self, owner=None): + """Get a list of signals which should abort a question.""" + abort_on = [self.shutting_down] + if owner is not None: + abort_on.append(owner.destroyed) + # This might be a generic network manager, e.g. one belonging to a + # DownloadManager. In this case, just skip the webview thing. + if self._tab_id is not None: + tab = objreg.get('tab', scope='tab', window=self._win_id, + tab=self._tab_id) + abort_on.append(tab.load_started) + return abort_on + def _ask(self, title, text, mode, owner=None, default=None): """Ask a blocking question in the statusbar. @@ -216,17 +211,7 @@ class NetworkManager(QNetworkAccessManager): Return: The answer the user gave or None if the prompt was cancelled. """ - abort_on = [self.shutting_down] - if owner is not None: - abort_on.append(owner.destroyed) - - # This might be a generic network manager, e.g. one belonging to a - # DownloadManager. In this case, just skip the webview thing. - if self._tab_id is not None: - tab = objreg.get('tab', scope='tab', window=self._win_id, - tab=self._tab_id) - abort_on.append(tab.load_started) - + abort_on = self._get_abort_signals(owner) return message.ask(title=title, text=text, mode=mode, abort_on=abort_on, default=default) @@ -248,11 +233,8 @@ class NetworkManager(QNetworkAccessManager): reply: The QNetworkReply that is encountering the errors. errors: A list of errors. """ - errors = [SslError(e) for e in errors] - ssl_strict = config.get('network', 'ssl-strict') - log.webview.debug("SSL errors {!r}, strict {}".format( - errors, ssl_strict)) - + errors = [webkittab.CertificateErrorWrapper(e) for e in errors] + log.webview.debug("Certificate errors {!r}".format(errors)) try: host_tpl = urlutils.host_tuple(reply.url()) except ValueError: @@ -268,42 +250,22 @@ class NetworkManager(QNetworkAccessManager): log.webview.debug("Already accepted: {} / " "rejected {}".format(is_accepted, is_rejected)) - if (ssl_strict and ssl_strict != 'ask') or is_rejected: + if is_rejected: return elif is_accepted: reply.ignoreSslErrors() return - if ssl_strict == 'ask': - err_template = jinja2.Template(""" - Errors while loading {{url.toDisplayString()}}:
- - """.strip()) - msg = err_template.render(url=reply.url(), errors=errors) - - answer = self._ask('SSL errors - continue?', msg, - mode=usertypes.PromptMode.yesno, owner=reply, - default=False) - log.webview.debug("Asked for SSL errors, answer {}".format(answer)) - if answer: - reply.ignoreSslErrors() - err_dict = self._accepted_ssl_errors - else: - err_dict = self._rejected_ssl_errors - if host_tpl is not None: - err_dict[host_tpl] += errors - else: - log.webview.debug("ssl-strict is False, only warning about errors") - for err in errors: - # FIXME we might want to use warn here (non-fatal error) - # https://github.com/The-Compiler/qutebrowser/issues/114 - message.error('SSL error: {}'.format(err.errorString())) + abort_on = self._get_abort_signals(reply) + ignore = shared.ignore_certificate_errors(reply.url(), errors, + abort_on=abort_on) + if ignore: reply.ignoreSslErrors() - self._accepted_ssl_errors[host_tpl] += errors + err_dict = self._accepted_ssl_errors + else: + err_dict = self._rejected_ssl_errors + if host_tpl is not None: + err_dict[host_tpl] += errors def clear_all_ssl_errors(self): """Clear all remembered SSL errors.""" @@ -347,12 +309,7 @@ class NetworkManager(QNetworkAccessManager): authenticator.setUser(user) authenticator.setPassword(password) else: - abort_on = [self.shutting_down, reply.destroyed] - if self._tab_id is not None: - tab = objreg.get('tab', scope='tab', window=self._win_id, - tab=self._tab_id) - abort_on.append(tab.load_started) - + abort_on = self._get_abort_signals(reply) shared.authentication_required(reply.url(), authenticator, abort_on=abort_on) diff --git a/qutebrowser/browser/webkit/webkittab.py b/qutebrowser/browser/webkit/webkittab.py index 61de1bd54..a2bbb2b9a 100644 --- a/qutebrowser/browser/webkit/webkittab.py +++ b/qutebrowser/browser/webkit/webkittab.py @@ -30,11 +30,12 @@ from PyQt5.QtWidgets import QApplication from PyQt5.QtWebKitWidgets import QWebPage, QWebFrame from PyQt5.QtWebKit import QWebSettings from PyQt5.QtPrintSupport import QPrinter +from PyQt5.QtNetwork import QSslError from qutebrowser.browser import browsertab from qutebrowser.browser.webkit import webview, tabhistory, webkitelem from qutebrowser.browser.webkit.network import proxy, webkitqutescheme -from qutebrowser.utils import qtutils, objreg, usertypes, utils, log +from qutebrowser.utils import qtutils, objreg, usertypes, utils, log, debug def init(): @@ -49,6 +50,36 @@ def init(): objreg.register('js-bridge', js_bridge) +class CertificateErrorWrapper(browsertab.AbstractCertificateErrorWrapper): + + """A wrapper over a QSslError.""" + + def __init__(self, error): + self._error = error + + def __str__(self): + return self._error.errorString() + + def __repr__(self): + return utils.get_repr( + self, error=debug.qenum_key(QSslError, self._error.error()), + string=str(self)) + + def __hash__(self): + try: + # Qt >= 5.4 + return hash(self._error) + except TypeError: + return hash((self._error.certificate().toDer(), + self._error.error())) + + def __eq__(self, other): + return self._error == other._error + + def is_overridable(self): + return True + + class WebKitPrinting(browsertab.AbstractPrinting): """QtWebKit implementations related to printing.""" diff --git a/qutebrowser/config/configdata.py b/qutebrowser/config/configdata.py index d55830380..a841d0567 100644 --- a/qutebrowser/config/configdata.py +++ b/qutebrowser/config/configdata.py @@ -430,8 +430,7 @@ def data(readonly=False): "Whether to send DNS requests over the configured proxy."), ('ssl-strict', - SettingValue(typ.BoolAsk(), 'ask', - backends=[usertypes.Backend.QtWebKit]), + SettingValue(typ.BoolAsk(), 'ask'), "Whether to validate SSL handshakes."), ('dns-prefetch', diff --git a/tests/end2end/features/prompts.feature b/tests/end2end/features/prompts.feature index 12497857b..90e2e6d9c 100644 --- a/tests/end2end/features/prompts.feature +++ b/tests/end2end/features/prompts.feature @@ -166,16 +166,14 @@ Feature: Prompts # SSL - @qtwebengine_todo: SSL errors are not implemented yet Scenario: SSL error with ssl-strict = false When I run :debug-clear-ssl-errors And I set network -> ssl-strict to false And I load an SSL page And I wait until the SSL page finished loading - Then the error "SSL error: *" should be shown + Then the error "Certificate error: *" should be shown And the page should contain the plaintext "Hello World via SSL!" - @qtwebengine_todo: SSL errors are not implemented yet Scenario: SSL error with ssl-strict = true When I run :debug-clear-ssl-errors And I set network -> ssl-strict to true @@ -183,7 +181,6 @@ Feature: Prompts Then "Error while loading *: SSL handshake failed" should be logged And the page should contain the plaintext "Unable to load page" - @qtwebengine_todo: SSL errors are not implemented yet Scenario: SSL error with ssl-strict = ask -> yes When I run :debug-clear-ssl-errors And I set network -> ssl-strict to ask @@ -193,7 +190,6 @@ Feature: Prompts And I wait until the SSL page finished loading Then the page should contain the plaintext "Hello World via SSL!" - @qtwebengine_todo: SSL errors are not implemented yet Scenario: SSL error with ssl-strict = ask -> no When I run :debug-clear-ssl-errors And I set network -> ssl-strict to ask @@ -473,7 +469,6 @@ Feature: Prompts # https://github.com/The-Compiler/qutebrowser/issues/1249#issuecomment-175205531 # https://github.com/The-Compiler/qutebrowser/pull/2054#issuecomment-258285544 - @qtwebengine_todo: SSL errors are not implemented yet Scenario: Interrupting SSL prompt during a notification prompt When I set content -> notifications to ask And I set network -> ssl-strict to ask diff --git a/tests/end2end/fixtures/quteprocess.py b/tests/end2end/fixtures/quteprocess.py index 46a0d2ff7..4db75e55b 100644 --- a/tests/end2end/fixtures/quteprocess.py +++ b/tests/end2end/fixtures/quteprocess.py @@ -53,6 +53,19 @@ def is_ignored_qt_message(message): return False +def is_ignored_lowlevel_message(message): + """Check if we want to ignore a lowlevel process output.""" + if 'Running without the SUID sandbox!' in message: + return True + elif message.startswith('Xlib: sequence lost'): + # https://travis-ci.org/The-Compiler/qutebrowser/jobs/157941720 + # ??? + return True + elif 'CERT_PKIXVerifyCert for localhost failed' in message: + return True + return False + + class LogLine(testprocess.Line): """A parsed line from the qutebrowser log output. @@ -222,14 +235,7 @@ class QuteProc(testprocess.Process): except testprocess.InvalidLine: if not line.strip(): return None - elif 'Running without the SUID sandbox!' in line: - # QtWebEngine error - return None - elif line.startswith('Xlib: sequence lost'): - # https://travis-ci.org/The-Compiler/qutebrowser/jobs/157941720 - # ??? - return None - elif is_ignored_qt_message(line): + elif is_ignored_qt_message(line) or is_ignored_lowlevel_message(line): return None else: raise