changeset 2687:e9cd473a2f46

core (xmpp): server certificate validation: XMPP server certificate is now checked, and connection is refused (by default) if it's not valid. Certificate check can be disabled in the new parameter "Configuration/check_certificate". If certificate checking is disabled, a warning note is sent on every new connection. Twisted and Wokkel are temporarly monkey patched in sat.core.tls_patches module, until modifications are merged upstream.
author Goffi <goffi@goffi.org>
date Sat, 10 Nov 2018 10:16:35 +0100
parents ce1e15d59496
children 943e78e18882
files CHANGELOG sat/core/exceptions.py sat/core/patches.py sat/core/sat_main.py sat/core/xmpp.py sat/memory/params.py
diffstat 6 files changed, 131 insertions(+), 28 deletions(-) [+]
line wrap: on
line diff
--- a/CHANGELOG	Fri Nov 09 16:17:45 2018 +0100
+++ b/CHANGELOG	Sat Nov 10 10:16:35 2018 +0100
@@ -1,7 +1,8 @@
 All theses changelogs are not exhaustive, please check the Mercurial repository for more details.
 
 v 0.7.0 (NOT RELEASED YET):
-    This version is a huge gap with previous one, changelog only show a part of novelties
+    This version is a huge gap with previous one, changelog only show a part of novelties.
+    This is also the first "general audience" version.
     - XEP-0070 implementation (HTTP Auth via XMPP)
     - XEP-0184 implementation (Delivery Receipts)
     - XEP-0231 implementation (Bits of Binary)
@@ -29,6 +30,7 @@
     - language detection plugin
     - media remote control through MPRIS standard
     - generic encryption methods handling in core
+    - server certificate validation
     - jp:
         - new debug commands, to monitor stream, call bridge method or send fake signals
         - new info/session command, to get data on current session
--- a/sat/core/exceptions.py	Fri Nov 09 16:17:45 2018 +0100
+++ b/sat/core/exceptions.py	Sat Nov 10 10:16:35 2018 +0100
@@ -121,3 +121,8 @@
 # Something which need to be done is not available yet
 class NotReady(Exception):
     pass
+
+
+class InvalidCertificate(Exception):
+    """A TLS certificate is not valid"""
+    pass
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/sat/core/patches.py	Sat Nov 10 10:16:35 2018 +0100
@@ -0,0 +1,63 @@
+from twisted.words.protocols.jabber import xmlstream
+from twisted.internet import ssl
+from wokkel import client
+
+"""This module apply monkey patches to Twisted and Wokkel to handle certificate validation
+   during XMPP connection"""
+
+
+class TLSInitiatingInitializer(xmlstream.TLSInitiatingInitializer):
+    check_certificate = True
+
+    def onProceed(self, obj):
+        self.xmlstream.removeObserver('/failure', self.onFailure)
+        trustRoot = ssl.platformTrust() if self.check_certificate else None
+        ctx = ssl.CertificateOptions(trustRoot=trustRoot)
+        self.xmlstream.transport.startTLS(ctx)
+        self.xmlstream.reset()
+        self.xmlstream.sendHeader()
+        self._deferred.callback(xmlstream.Reset)
+
+
+class XMPPClient(client.XMPPClient):
+
+    def __init__(self, jid, password, host=None, port=5222, check_certificate=True):
+        self.jid = jid
+        self.domain = jid.host.encode('idna')
+        self.host = host
+        self.port = port
+
+        factory = HybridClientFactory(jid, password, check_certificate)
+
+        client.StreamManager.__init__(self, factory)
+
+
+def HybridClientFactory(jid, password, check_certificate=True):
+    a = HybridAuthenticator(jid, password, check_certificate)
+
+    return xmlstream.XmlStreamFactory(a)
+
+
+class HybridAuthenticator(client.HybridAuthenticator):
+
+    def __init__(self, jid, password, check_certificate):
+        xmlstream.ConnectAuthenticator.__init__(self, jid.host)
+        self.jid = jid
+        self.password = password
+        self.check_certificate = check_certificate
+
+    def associateWithStream(self, xs):
+        xmlstream.ConnectAuthenticator.associateWithStream(self, xs)
+
+        tlsInit = xmlstream.TLSInitiatingInitializer(xs)
+        tlsInit.check_certificate = self.check_certificate
+        xs.initializers = [client.client.CheckVersionInitializer(xs),
+                           tlsInit,
+                           client.CheckAuthInitializer(xs)]
+
+
+def apply():
+    xmlstream.TLSInitiatingInitializer = TLSInitiatingInitializer
+    client.XMPPClient = XMPPClient
+    client.HybridClientFactory = HybridClientFactory
+    client.HybridAuthenticator = HybridAuthenticator
--- a/sat/core/sat_main.py	Fri Nov 09 16:17:45 2018 +0100
+++ b/sat/core/sat_main.py	Sat Nov 10 10:16:35 2018 +0100
@@ -19,6 +19,8 @@
 
 import sat
 from sat.core.i18n import _, languageSwitch
+from sat.core import patches
+patches.apply()
 from twisted.application import service
 from twisted.internet import defer
 from twisted.words.protocols.jabber import jid
--- a/sat/core/xmpp.py	Fri Nov 09 16:17:45 2018 +0100
+++ b/sat/core/xmpp.py	Sat Nov 10 10:16:35 2018 +0100
@@ -35,6 +35,7 @@
 log = getLogger(__name__)
 from sat.core import exceptions
 from sat.memory import encryption
+from sat.tools import xml_tools
 from zope.interface import implements
 import time
 import calendar
@@ -44,6 +45,7 @@
 
 class SatXMPPEntity(object):
     """Common code for Client and Component"""
+    _reason = None  # reason of disconnection
 
     def __init__(self, host_app, profile, max_retries):
 
@@ -214,6 +216,11 @@
             return
         super(SatXMPPEntity, self)._authd(xmlstream)
 
+        if self._reason is not None:
+            # if we have had trouble to connect we can reset
+            # the exception as the connection is now working.
+            del self._reason
+
         # the following Deferred is used to know when we are connected
         # so we need to be set it to None when connection is lost
         self._connected = defer.Deferred()
@@ -267,6 +274,11 @@
 
     ## connection ##
 
+    def _disconnected(self, reason):
+        # we have to save the reason of disconnection, otherwise it would be lost
+        self._reason = reason
+        super(SatXMPPEntity, self)._disconnected(reason)
+
     def connectionLost(self, connector, reason):
         try:
             self.keep_alife.stop()
@@ -288,9 +300,21 @@
         if not self.conn_deferred.called:
             # FIXME: real error is not gotten here (e.g. if jid is not know by Prosody,
             #        we should have the real error)
-            self.conn_deferred.errback(
-                error.StreamError(u"Server unexpectedly closed the connection")
-            )
+            if self._reason is None:
+                err = error.StreamError(u"Server unexpectedly closed the connection")
+            else:
+                err = self._reason
+                try:
+                    if err.value.args[0][0][2] == "certificate verify failed":
+                        err = exceptions.InvalidCertificate(
+                            _(u"Your server certificate is not valid "
+                              u"(its identity can't be checked).\n\n"
+                              u"This should never happen and may indicate that "
+                              u"somebody is trying to spy on you.\n"
+                              u"Please contact your server administrator."))
+                except (IndexError, TypeError):
+                    pass
+            self.conn_deferred.errback(err)
 
     @defer.inlineCallbacks
     def _cleanConnection(self, __):
@@ -544,16 +568,8 @@
     trigger_suffix = ""
     is_component = False
 
-    def __init__(
-        self,
-        host_app,
-        profile,
-        user_jid,
-        password,
-        host=None,
-        port=C.XMPP_C2S_PORT,
-        max_retries=C.XMPP_MAX_RETRIES,
-    ):
+    def __init__(self, host_app, profile, user_jid, password, host=None,
+                 port=C.XMPP_C2S_PORT, max_retries=C.XMPP_MAX_RETRIES):
         # XXX: DNS SRV records are checked when the host is not specified.
         # If no SRV record is found, the host is directly extracted from the JID.
         self.started = time.time()
@@ -594,11 +610,24 @@
                     .format(host_ori=user_jid.host, host=host, port=port)
                 )
 
+        self.check_certificate = host_app.memory.getParamA(
+            "check_certificate", "Connection", profile_key=profile)
+
         wokkel_client.XMPPClient.__init__(
-            self, user_jid, password, host or None, port or C.XMPP_C2S_PORT
+            self, user_jid, password, host or None, port or C.XMPP_C2S_PORT,
+            check_certificate = self.check_certificate
         )
         SatXMPPEntity.__init__(self, host_app, profile, max_retries)
 
+        if not self.check_certificate:
+            msg = (_(u"Certificate validation is deactivated, this is unsecure and "
+                u"somebody may be spying on you. If you have no good reason to disable "
+                u"certificate validation, please activate \"Check certificate\" in your "
+                u"settings in \"Connection\" tab."))
+            xml_tools.quickNote(host_app, self, msg, _(u"Security notice"),
+                level = C.XMLUI_DATA_LVL_WARNING)
+
+
     def _getPluginsList(self):
         for p in self.host_app.plugins.itervalues():
             if C.PLUG_MODE_CLIENT in p._info[u"modes"]:
--- a/sat/memory/params.py	Fri Nov 09 16:17:45 2018 +0100
+++ b/sat/memory/params.py	Sat Nov 10 10:16:35 2018 +0100
@@ -75,23 +75,25 @@
             <param name="%(force_port_param)s" value="" type="int" constraint="1;65535" security="50" />
             <param name="autoconnect" label="%(autoconnect_label)s" value="true" type="bool" security="50" />
             <param name="autodisconnect" label="%(autodisconnect_label)s" value="false"  type="bool" security="50" />
+            <param name="check_certificate" label="%(check_certificate_label)s" value="true"  type="bool" security="4" />
         </category>
     </individual>
     </params>
     """ % {
-        "category_general": D_("General"),
-        "category_connection": D_("Connection"),
-        "history_param": C.HISTORY_LIMIT,
-        "history_label": D_("Chat history limit"),
-        "show_offline_contacts": C.SHOW_OFFLINE_CONTACTS,
-        "show_offline_contacts_label": D_("Show offline contacts"),
-        "show_empty_groups": C.SHOW_EMPTY_GROUPS,
-        "show_empty_groups_label": D_("Show empty groups"),
-        "force_server_param": C.FORCE_SERVER_PARAM,
-        "force_port_param": C.FORCE_PORT_PARAM,
-        "new_account_label": D_("Register new account"),
-        "autoconnect_label": D_("Connect on frontend startup"),
-        "autodisconnect_label": D_("Disconnect on frontend closure"),
+        u"category_general": D_(u"General"),
+        u"category_connection": D_(u"Connection"),
+        u"history_param": C.HISTORY_LIMIT,
+        u"history_label": D_(u"Chat history limit"),
+        u"show_offline_contacts": C.SHOW_OFFLINE_CONTACTS,
+        u"show_offline_contacts_label": D_(u"Show offline contacts"),
+        u"show_empty_groups": C.SHOW_EMPTY_GROUPS,
+        u"show_empty_groups_label": D_(u"Show empty groups"),
+        u"force_server_param": C.FORCE_SERVER_PARAM,
+        u"force_port_param": C.FORCE_PORT_PARAM,
+        u"new_account_label": D_(u"Register new account"),
+        u"autoconnect_label": D_(u"Connect on frontend startup"),
+        u"autodisconnect_label": D_(u"Disconnect on frontend closure"),
+        u"check_certificate_label": D_(u"Check certificate (don't uncheck if unsure)"),
     }
 
     def load_default_params(self):