Merge pull request Electron-Cash#1919 from EchterAgo/unpin_cert

Network: Show problematic pinned certificates and allow unpinning

contrib/fonts/glyphs.txt

@@ -134,3 +134,4 @@ U+2B50
 U+26D4
 U+274C
 U+2705
+U+2757

gui/qt/network_dialog.py

@@ -25,6 +25,7 @@
 
 import queue
 import socket
+from functools import partial
 
 from PyQt5.QtGui import *
 from PyQt5.QtCore import *
@@ -202,11 +203,22 @@ def update(self, network, servers):
 
 class ServerFlag:
     ''' Used by ServerListWidget for Server flags & Symbols '''
+    BadCertificate = 4 # Servers with a bad certificate.
     Banned = 2 # Blacklisting/banning was a hidden mechanism inherited from Electrum. We would blacklist misbehaving servers under the hood. Now that facility is exposed (editable by the user). We never connect to blacklisted servers.
     Preferred = 1 # Preferred servers (white-listed) start off as the servers in servers.json and are "more trusted" and optionally the user can elect to connect to only these servers
     NoFlag = 0
-    Symbol = ("", "⭐", "⛔") # indexed using pseudo-enum above
-    UnSymbol = ("", "❌", "✅") # used for "disable X" context menu
+    Symbol = {
+        NoFlag: "",
+        Preferred: "⭐",
+        Banned: "⛔",
+        BadCertificate: "❗️"
+    }
+    UnSymbol = { # used for "disable X" context menu
+        NoFlag: "",
+        Preferred: "❌",
+        Banned: "✅",
+        BadCertificate: ""
+    }
 
 class ServerListWidget(QTreeWidget):
 
@@ -246,8 +258,16 @@ def create_menu(self, position):
                     optxt_fav = ServerFlag.Symbol[ServerFlag.Preferred] + " " + _("Add to preferred")
                 menu.addAction(optxt_fav, lambda: self.parent.set_whitelisted(server, not iswl))
         menu.addAction(optxt, lambda: self.parent.set_blacklisted(server, not isbl))
+        if flagval & ServerFlag.BadCertificate:
+            optxt = ServerFlag.UnSymbol[ServerFlag.BadCertificate] + " " + _("Remove pinned certificate")
+            menu.addAction(optxt, partial(self.on_remove_pinned_certificate, server))
         menu.exec_(self.viewport().mapToGlobal(position))
 
+    def on_remove_pinned_certificate(self, server):
+        if not self.parent.remove_pinned_certificate(server):
+            QMessageBox.critical(None, _("Remove pinned certificate"),
+                                 _("Failed to remove the pinned certificate. Check the log for errors."))
+
     def set_server(self, s):
         host, port, protocol = deserialize_server(s)
         self.parent.server_host.setText(host)
@@ -288,11 +308,27 @@ def update(self, network, servers, protocol, use_tor):
             port = d.get(protocol)
             if port:
                 server = serialize_server(_host, port, protocol)
-                flag, flagval, tt = (ServerFlag.Symbol[ServerFlag.Banned], ServerFlag.Banned, _("This server is banned")) if network.server_is_blacklisted(server) else ("", 0, "")
-                flag2, flagval2, tt2 = (ServerFlag.Symbol[ServerFlag.Preferred], ServerFlag.Preferred, _("This is a preferred server")) if network.server_is_whitelisted(server) else ("", 0, "")
-                flag = flag or flag2; del flag2
-                tt = tt or tt2; del tt2
-                flagval |= flagval2; del flagval2
+
+                flag = ""
+                flagval = 0
+                tt = ""
+
+                if network.server_is_blacklisted(server):
+                    flagval |= ServerFlag.Banned
+                if network.server_is_whitelisted(server):
+                    flagval |= ServerFlag.Preferred
+                if network.server_is_bad_certificate(server):
+                    flagval |= ServerFlag.BadCertificate
+
+                if flagval & ServerFlag.Banned:
+                    flag = ServerFlag.Symbol[ServerFlag.Banned]
+                    tt = _("This server is banned")
+                elif flagval & ServerFlag.BadCertificate:
+                    flag = ServerFlag.Symbol[ServerFlag.BadCertificate]
+                    tt = _("This server's pinned certificate mismatches its current certificate")
+                elif flagval & ServerFlag.Preferred:
+                    flag = ServerFlag.Symbol[ServerFlag.Preferred]
+                    tt = _("This is a preferred server")
 
                 display_text = _host
                 if is_onion and 'display' in d:
@@ -302,11 +338,12 @@ def update(self, network, servers, protocol, use_tor):
                 if is_onion:
                     x.setIcon(2, QIcon(":icons/tor_logo.svg"))
                 if tt: x.setToolTip(0, tt)
-                if (wl_only and flagval != ServerFlag.Preferred) or flagval & ServerFlag.Banned:
+                if (wl_only and not flagval & ServerFlag.Preferred) or flagval & ServerFlag.Banned:
                     # lighten the text of servers we can't/won't connect to for the given mode
                     self.lightenItemText(x, range(1,4))
                 x.setData(2, Qt.UserRole, server)
                 x.setData(0, Qt.UserRole, flagval)
+                x.setTextAlignment(0, Qt.AlignHCenter)
                 self.addTopLevelItem(x)
 
         h = self.header()
@@ -555,6 +592,8 @@ def hideEvent(slf, e):
 
         self.network.tor_controller.active_port_changed.append_weak(self.on_tor_port_changed)
 
+        self.network.server_list_updated.append_weak(self.on_server_list_updated)
+
         self.fill_in_proxy_settings()
         self.update()
 
@@ -580,6 +619,10 @@ def on_tor_port_changed(self, controller: TorController):
         # set the value in the text box here.
         self.proxy_port.setText(str(controller.active_socks_port))
 
+    @in_main_thread
+    def on_server_list_updated(self):
+        self.update()
+
     def check_disable_proxy(self, b):
         if not self.config.is_modifiable('proxy'):
             b = False
@@ -841,6 +884,9 @@ def on_custom_port_cb_click(self, b):
     def proxy_settings_changed(self):
         self.tor_cb.setChecked(False)
 
+    def remove_pinned_certificate(self, server):
+        return self.network.remove_pinned_certificate(server)
+
     def set_blacklisted(self, server, bl):
         self.network.server_set_blacklisted(server, bl, True)
         self.set_server() # if the blacklisted server is the active server, this will force a reconnect to another server

lib/interface.py

@@ -35,6 +35,7 @@
 from typing import Optional, Tuple
 
 from .util import print_error
+from .utils import Event
 
 ca_path = requests.certs.where()
 
@@ -43,7 +44,7 @@
 from . import pem
 
 
-def Connection(server, queue, config_path):
+def Connection(server, queue, config_path, callback=None):
     """Makes asynchronous connections to a remote electrum server.
     Returns the running thread that is making the connection.
 
@@ -55,11 +56,14 @@ def Connection(server, queue, config_path):
     if not protocol in 'st':
         raise Exception('Unknown protocol: %s' % protocol)
     c = TcpConnection(server, queue, config_path)
+    if callback:
+        callback(c)
     c.start()
     return c
 
 
 class TcpConnection(threading.Thread, util.PrintError):
+    bad_certificate = Event()
 
     def __init__(self, server, queue, config_path):
         threading.Thread.__init__(self)
@@ -232,9 +236,12 @@ def get_socket(self):
                     return
                 if is_new:
                     rej = cert_path + '.rej'
-                    if os.path.exists(rej):
-                        os.unlink(rej)
-                    os.rename(temporary_path, rej)
+                    try:
+                        if os.path.exists(rej):
+                            os.unlink(rej)
+                        os.rename(temporary_path, rej)
+                    except OSError as e:
+                        self.print_error("Could not rename rejected certificate:", rej, repr(e))
                 else:
                     util.assert_datadir_available(self.config_path)
                     with open(cert_path, encoding='utf-8') as f:
@@ -247,14 +254,20 @@ def get_socket(self):
                             self.print_error("Error checking certificate, traceback follows")
                             traceback.print_exc(file=sys.stderr)
                         self.print_error("wrong certificate")
+                        self.bad_certificate(self.server, cert_path)
                         return
                     try:
                         x.check_date()
                     except:
                         self.print_error("certificate has expired:", cert_path)
-                        os.unlink(cert_path)
+                        try:
+                            os.unlink(cert_path)
+                            self.print_error("Removed expired certificate:", cert_path)
+                        except OSError as e:
+                            self.print_error("Could not remove expired certificate:", cert_path, repr(e))
                         return
                     self.print_error("wrong certificate")
+                    self.bad_certificate(self.server, cert_path)
                 if e.errno == 104:
                     return
                 return

lib/network.py

@@ -33,6 +33,7 @@
 import threading
 import socket
 import json
+from typing import Dict
 
 import socks
 from . import util
@@ -44,6 +45,7 @@
 from . import blockchain
 from . import version
 from .tor import TorController
+from .utils import Event
 
 DEFAULT_AUTO_CONNECT = True
 # Versions prior to 4.0.15 had this set to True, but we opted for False to
@@ -227,6 +229,8 @@ def __init__(self, config=None):
         self.whitelisted_servers, self.whitelisted_servers_hostmap = self._compute_whitelist()
         self.print_error("server blacklist: {} server whitelist: {}".format(self.blacklisted_servers, self.whitelisted_servers))
         self.default_server = self.get_config_server()
+        self.bad_certificate_servers: Dict[str, str] = dict()
+        self.server_list_updated = Event()
 
         self.tor_controller = TorController(self.config)
         self.tor_controller.active_port_changed.append(self.on_tor_port_changed)
@@ -578,7 +582,8 @@ def start_interface(self, server_key):
                 self.print_error("connecting to %s as new interface" % server_key)
                 self.set_status('connecting')
             self.connecting.add(server_key)
-            c = Connection(server_key, self.socket_queue, self.config.path)
+            c = Connection(server_key, self.socket_queue, self.config.path,
+                           lambda x: x.bad_certificate.append_weak(self.on_bad_certificate))
 
     def get_unavailable_servers(self):
         exclude_set = set(self.interfaces)
@@ -1023,6 +1028,7 @@ def maintain_sockets(self):
             if server in self.connecting:
                 self.connecting.remove(server)
             if socket:
+                self.remove_bad_certificate(server)
                 self.new_interface(server, socket)
             else:
                 self.connection_down(server)
@@ -1933,6 +1939,37 @@ def get_proxies(self):
             return proxies
         return None
 
+    def on_bad_certificate(self, server, certificate):
+        if server in self.bad_certificate_servers:
+            return
+        self.bad_certificate_servers[server] = certificate
+        self.server_list_updated()
+
+    def remove_bad_certificate(self, server):
+        if server not in self.bad_certificate_servers:
+            return
+        del self.bad_certificate_servers[server]
+        self.server_list_updated()
+
+    def remove_pinned_certificate(self, server):
+        cert_file = self.bad_certificate_servers.get(server)
+        if not cert_file:
+            return False
+
+        try:
+            os.unlink(cert_file)
+            self.print_error("Removed pinned certificate:", cert_file)
+        except OSError as e:
+            self.print_error("Could not remove pinned certificate:", cert_file, repr(e))
+            if os.path.exists(cert_file):
+                # Don't remove from bad certificate list if we failed to unpin
+                return False
+        self.remove_bad_certificate(server)
+        return True
+
+
+    def server_is_bad_certificate(self, server): return server in self.bad_certificate_servers
+
     def server_set_blacklisted(self, server, b, save=True, skip_connection_logic=False):
         assert isinstance(server, str)
         if b: