Skip to content

Commit

Permalink
Security: Remember hosts with ignored cert errors for load status
Browse files Browse the repository at this point in the history
Without this change, we only set a flag when a certificate error occurred.
However, when the same certificate error then happens a second time (e.g.
because of a reload or opening the same URL again), we then colored the URL as
success_https (i.e. green) again.

See #5403

(cherry picked from commit 021ab57)
  • Loading branch information
The-Compiler committed May 2, 2020
1 parent 3508352 commit 19f01bb
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 9 deletions.
16 changes: 12 additions & 4 deletions qutebrowser/browser/browsertab.py
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,13 @@ class AbstractTab(QWidget):
# arg 1: The exit code.
renderer_process_terminated = pyqtSignal(TerminationStatus, int)

# Hosts for which a certificate error happened. Shared between all tabs.
#
# Note that we remember hosts here, without scheme/port:
# QtWebEngine/Chromium also only remembers hostnames, and certificates are
# for a given hostname anyways.
_insecure_hosts = set() # type: typing.Set[str]

def __init__(self, *, win_id: int, private: bool,
parent: QWidget = None) -> None:
self.is_private = private
Expand All @@ -881,7 +888,6 @@ def __init__(self, *, win_id: int, private: bool,
self._layout = miscwidgets.WrapperLayout(self)
self._widget = None # type: typing.Optional[QWidget]
self._progress = 0
self._has_ssl_errors = False
self._load_status = usertypes.LoadStatus.none
self._mouse_event_filter = mouse.MouseEventFilter(
self, parent=self)
Expand Down Expand Up @@ -969,7 +975,6 @@ def _on_url_changed(self, url: QUrl) -> None:
@pyqtSlot()
def _on_load_started(self) -> None:
self._progress = 0
self._has_ssl_errors = False
self.data.viewing_source = False
self._set_load_status(usertypes.LoadStatus.loading)
self.load_started.emit()
Expand Down Expand Up @@ -1029,9 +1034,12 @@ def _update_load_status(self, ok: bool) -> None:
Needs to be called by subclasses to trigger a load status update, e.g.
as a response to a loadFinished signal.
"""
if ok and not self._has_ssl_errors:
if ok:
if self.url().scheme() == 'https':
self._set_load_status(usertypes.LoadStatus.success_https)
if self.url().host() in self._insecure_hosts:
self._set_load_status(usertypes.LoadStatus.warn)
else:
self._set_load_status(usertypes.LoadStatus.success_https)
else:
self._set_load_status(usertypes.LoadStatus.success)
elif ok:
Expand Down
4 changes: 2 additions & 2 deletions qutebrowser/browser/webengine/webenginetab.py
Original file line number Diff line number Diff line change
Expand Up @@ -1395,9 +1395,9 @@ def _on_load_finished(self, ok: bool) -> None:

@pyqtSlot(certificateerror.CertificateErrorWrapper)
def _on_ssl_errors(self, error):
self._has_ssl_errors = True

url = error.url()
self._insecure_hosts.add(url.host())

log.webview.debug("Certificate error: {}".format(error))

if error.is_overridable():
Expand Down
6 changes: 3 additions & 3 deletions qutebrowser/browser/webkit/webkittab.py
Original file line number Diff line number Diff line change
Expand Up @@ -851,9 +851,9 @@ def _on_navigation_request(self, navigation):
if navigation.is_main_frame:
self.settings.update_for_url(navigation.url)

@pyqtSlot()
def _on_ssl_errors(self):
self._has_ssl_errors = True
@pyqtSlot('QNetworkReply*')
def _on_ssl_errors(self, reply):
self._insecure_hosts.add(reply.url().host())

def _connect_signals(self):
view = self._widget
Expand Down

0 comments on commit 19f01bb

Please sign in to comment.