Mercurial > libervia-backend

changeset 3160:330a5f1d9eea

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
core (memory/crypto): replaced `PyCrypto` by `cryptography`: `PyCrypto` is unmaintained for years but was used in SàT for password hashing. This patch fixes that by replacing `PyCrypto` by the reference `cryptography` module which is well maintained. The behaviour stays the same (except that previously async `hash`, `encrypt` and `decrypt` methods are now synchronous, as they are quick and using a deferToThread may actually be more resource intensive than using blocking methods). It is planed to improve `memory.crypto` by using more up-to-date cryptography/hashing algorithms in the future. PyCrypto is no more a dependency of SàT
author Goffi <goffi@goffi.org>
date Sun, 09 Feb 2020 23:50:26 +0100
parents 30e08d904208
children be5fffe34987
files sat/memory/crypto.py sat/memory/memory.py sat/memory/params.py sat/memory/sqlite.py sat/plugins/plugin_sec_otr.py setup.py
diffstat 6 files changed, 103 insertions(+), 136 deletions(-) [+]
line wrap: on
line diff
--- a/sat/memory/crypto.py	Sun Feb 09 23:50:21 2020 +0100
+++ b/sat/memory/crypto.py	Sun Feb 09 23:50:26 2020 +0100
@@ -1,6 +1,5 @@
 #!/usr/bin/env python3
 
-
 # SAT: a jabber client
 # Copyright (C) 2009-2020 Jérôme Poisson (goffi@goffi.org)
 # Copyright (C) 2013-2016 Adrien Cossa (souliane@mailoo.org)
@@ -18,26 +17,25 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-try:
-    from Crypto.Cipher import AES
-    from Crypto.Protocol.KDF import PBKDF2
-except ImportError:
-    raise Exception("PyCrypto is not installed.")
-
 from os import urandom
 from base64 import b64encode, b64decode
-from twisted.internet.threads import deferToThread
-from twisted.internet.defer import succeed
+from cryptography.hazmat.primitives import hashes
+from cryptography.hazmat.primitives.kdf.pbkdf2 import PBKDF2HMAC
+from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes
+from cryptography.hazmat.backends import default_backend
+
+
+crypto_backend = default_backend()
 
 
-class BlockCipher(object):
+class BlockCipher:
 
-    BLOCK_SIZE = AES.block_size  # 16 bits
-    MAX_KEY_SIZE = AES.key_size[-1]  # 32 bits = AES-256
+    BLOCK_SIZE = 16
+    MAX_KEY_SIZE = 32
     IV_SIZE = BLOCK_SIZE  # initialization vector size, 16 bits
 
-    @classmethod
-    def encrypt(cls, key, text, leave_empty=True):
+    @staticmethod
+    def encrypt(key, text, leave_empty=True):
         """Encrypt a message.
 
         Based on http://stackoverflow.com/a/12525165
@@ -48,22 +46,22 @@
         @return (D(str)): base-64 encoded encrypted message
         """
         if leave_empty and text == "":
-            return succeed(text)
+            return ""
         iv = BlockCipher.getRandomKey()
-        key = key.encode("utf-8")
+        key = key.encode()
         key = (
             key[: BlockCipher.MAX_KEY_SIZE]
             if len(key) >= BlockCipher.MAX_KEY_SIZE
             else BlockCipher.pad(key)
         )
-        cipher = AES.new(key, AES.MODE_CFB, iv)
-        d = deferToThread(cipher.encrypt, BlockCipher.pad(text.encode("utf-8")))
-        d.addCallback(lambda ciphertext: b64encode(iv + ciphertext))
-        d.addCallback(lambda bytes_cypher: bytes_cypher.decode('utf-8'))
-        return d
 
-    @classmethod
-    def decrypt(cls, key, ciphertext, leave_empty=True):
+        cipher = Cipher(algorithms.AES(key), modes.CFB8(iv), backend=crypto_backend)
+        encryptor = cipher.encryptor()
+        encrypted = encryptor.update(BlockCipher.pad(text.encode())) + encryptor.finalize()
+        return b64encode(iv + encrypted).decode()
+
+    @staticmethod
+    def decrypt(key, ciphertext, leave_empty=True):
         """Decrypt a message.
 
         Based on http://stackoverflow.com/a/12525165
@@ -74,30 +72,26 @@
         @return: Deferred: str or None if the password could not be decrypted
         """
         if leave_empty and ciphertext == "":
-            return succeed("")
+            return ""
         ciphertext = b64decode(ciphertext)
         iv, ciphertext = (
             ciphertext[: BlockCipher.IV_SIZE],
             ciphertext[BlockCipher.IV_SIZE :],
         )
-        key = key.encode("utf-8")
+        key = key.encode()
         key = (
             key[: BlockCipher.MAX_KEY_SIZE]
             if len(key) >= BlockCipher.MAX_KEY_SIZE
             else BlockCipher.pad(key)
         )
-        cipher = AES.new(key, AES.MODE_CFB, iv)
-        d = deferToThread(cipher.decrypt, ciphertext)
-        d.addCallback(lambda text: BlockCipher.unpad(text))
-        # XXX: cipher.decrypt gives no way to make the distinction between
-        # a decrypted empty value and a decryption failure... both return
-        # the empty value. Fortunately, we detect empty passwords beforehand
-        # thanks to the "leave_empty" parameter which is used by default.
-        d.addCallback(lambda text: text if text else None)
-        return d
 
-    @classmethod
-    def getRandomKey(cls, size=None, base64=False):
+        cipher = Cipher(algorithms.AES(key), modes.CFB8(iv), backend=crypto_backend)
+        decryptor = cipher.decryptor()
+        decrypted = decryptor.update(ciphertext) + decryptor.finalize()
+        return BlockCipher.unpad(decrypted)
+
+    @staticmethod
+    def getRandomKey(size=None, base64=False):
         """Return a random key suitable for block cipher encryption.
 
         Note: a good value for the key length is to make it as long as the block size.
@@ -111,25 +105,25 @@
         key = urandom(size)
         return b64encode(key) if base64 else key
 
-    @classmethod
-    def pad(self, s):
+    @staticmethod
+    def pad(s):
         """Method from http://stackoverflow.com/a/12525165"""
         bs = BlockCipher.BLOCK_SIZE
-        return s + (bs - len(s) % bs) * (chr(bs - len(s) % bs)).encode('utf-8')
+        return s + (bs - len(s) % bs) * (chr(bs - len(s) % bs)).encode()
 
-    @classmethod
-    def unpad(self, s):
+    @staticmethod
+    def unpad(s):
         """Method from http://stackoverflow.com/a/12525165"""
-        s = s.decode('utf-8')
+        s = s.decode()
         return s[0 : -ord(s[-1])]
 
 
-class PasswordHasher(object):
+class PasswordHasher:
 
     SALT_LEN = 16  # 128 bits
 
-    @classmethod
-    def hash(cls, password, salt=None, leave_empty=True):
+    @staticmethod
+    def hash(password, salt=None, leave_empty=True):
         """Hash a password.
 
         @param password (str): the password to hash
@@ -138,33 +132,39 @@
         @return: Deferred: base-64 encoded str
         """
         if leave_empty and password == "":
-            return succeed("")
+            return ""
         salt = (
             b64decode(salt)[: PasswordHasher.SALT_LEN]
             if salt
             else urandom(PasswordHasher.SALT_LEN)
         )
-        d = deferToThread(PBKDF2, password, salt)
-        d.addCallback(lambda hashed: b64encode(salt + hashed))
-        d.addCallback(lambda hashed_bytes: hashed_bytes.decode('utf-8'))
-        return d
 
-    @classmethod
-    def compare_hash(cls, hashed_attempt, hashed):
-        assert isinstance(hashed, str)
-        return hashed_attempt == hashed
+        # we use PyCrypto's PBKDF2 arguments while porting to crytography, to stay
+        # compatible with existing installations. But this is temporary and we need
+        # to update them to more secure values.
+        kdf = PBKDF2HMAC(
+            # FIXME: SHA1() is not secure, it is used here for historical reasons
+            #   and must be changed as soon as possible
+            algorithm=hashes.SHA1(),
+            length=16,
+            salt=salt,
+            iterations=1000,
+            backend=crypto_backend
+        )
+        key = kdf.derive(password.encode())
+        return b64encode(salt + key).decode()
 
-    @classmethod
-    def verify(cls, attempt, hashed):
+    @staticmethod
+    def verify(attempt, pwd_hash):
         """Verify a password attempt.
 
         @param attempt (str): the attempt to check
-        @param hashed (str): the hash of the password
+        @param pwd_hash (str): the hash of the password
         @return: Deferred: boolean
         """
         assert isinstance(attempt, str)
-        assert isinstance(hashed, str)
-        leave_empty = hashed == ""
-        d = PasswordHasher.hash(attempt, hashed, leave_empty)
-        d.addCallback(cls.compare_hash, hashed=hashed)
-        return d
+        assert isinstance(pwd_hash, str)
+        leave_empty = pwd_hash == ""
+        attempt_hash = PasswordHasher.hash(attempt, pwd_hash, leave_empty)
+        assert isinstance(attempt_hash, str)
+        return attempt_hash == pwd_hash
--- a/sat/memory/memory.py	Sun Feb 09 23:50:21 2020 +0100
+++ b/sat/memory/memory.py	Sun Feb 09 23:50:26 2020 +0100
@@ -347,7 +347,7 @@
                 finally:
                     return session_d
 
-            auth_d = self.profileAuthenticate(password, profile)
+            auth_d = defer.ensureDeferred(self.profileAuthenticate(password, profile))
             auth_d.addCallback(doStartSession)
             return auth_d
 
@@ -381,37 +381,30 @@
         except KeyError:
             return False
 
-    def profileAuthenticate(self, password, profile):
+    async def profileAuthenticate(self, password, profile):
         """Authenticate the profile.
 
         @param password (unicode): the SàT profile password
-        @param profile: %(doc_profile)s
-        @return (D): a deferred None in case of success, a failure otherwise.
+        @return: None in case of success (an exception is raised otherwise)
         @raise exceptions.PasswordError: the password does not match
         """
         if not password and self.auth_sessions.profileGetUnique(profile):
             # XXX: this allows any frontend to connect with the empty password as soon as
             # the profile has been authenticated at least once before. It is OK as long as
             # submitting a form with empty passwords is restricted to local frontends.
-            return defer.succeed(None)
+            return
 
-        def check_result(result):
-            if not result:
-                log.warning("Authentication failure of profile {}".format(profile))
-                raise failure.Failure(
-                    exceptions.PasswordError(
-                        "The provided profile password doesn't match."
-                    )
-                )
-            return self.newAuthSession(password, profile)
-
-        d = self.asyncGetParamA(
+        sat_cipher = await self.asyncGetParamA(
             C.PROFILE_PASS_PATH[1], C.PROFILE_PASS_PATH[0], profile_key=profile
         )
-        d.addCallback(lambda sat_cipher: PasswordHasher.verify(password, sat_cipher))
-        return d.addCallback(check_result)
+        valid = PasswordHasher.verify(password, sat_cipher)
+        if not valid:
+            log.warning(_("Authentication failure of profile {profile}").format(
+                profile=profile))
+            raise exceptions.PasswordError("The provided profile password doesn't match.")
+        return await self.newAuthSession(password, profile)
 
-    def newAuthSession(self, key, profile):
+    async def newAuthSession(self, key, profile):
         """Start a new session for the authenticated profile.
 
         If there is already an existing session, no new one is created
@@ -419,21 +412,16 @@
 
         @param key: the key to decrypt the personal key
         @param profile: %(doc_profile)s
-        @return: a deferred None value
         """
-
-        def gotPersonalKey(personal_key):
-            """Create the session for this profile and store the personal key"""
-            session_data = self.auth_sessions.profileGetUnique(profile)
-            if not session_data:
-                self.auth_sessions.newSession(
-                    {C.MEMORY_CRYPTO_KEY: personal_key}, profile=profile
-                )
-                log.debug("auth session created for profile %s" % profile)
-
-        d = PersistentDict(C.MEMORY_CRYPTO_NAMESPACE, profile).load()
-        d.addCallback(lambda data: BlockCipher.decrypt(key, data[C.MEMORY_CRYPTO_KEY]))
-        return d.addCallback(gotPersonalKey)
+        data = await PersistentDict(C.MEMORY_CRYPTO_NAMESPACE, profile).load()
+        personal_key = BlockCipher.decrypt(key, data[C.MEMORY_CRYPTO_KEY])
+        # Create the session for this profile and store the personal key
+        session_data = self.auth_sessions.profileGetUnique(profile)
+        if not session_data:
+            self.auth_sessions.newSession(
+                {C.MEMORY_CRYPTO_KEY: personal_key}, profile=profile
+            )
+            log.debug("auth session created for profile %s" % profile)
 
     def purgeProfileSession(self, profile):
         """Delete cache of data of profile
@@ -1064,13 +1052,8 @@
         """
 
         def gotIndMemory(data):
-            d = BlockCipher.encrypt(crypto_key, data_value)
-
-            def cb(cipher):
-                data[data_key] = cipher
-                return data.force(data_key)
-
-            return d.addCallback(cb)
+            data[data_key] = BlockCipher.encrypt(crypto_key, data_value)
+            return data.force(data_key)
 
         def done(__):
             log.debug(
--- a/sat/memory/params.py	Sun Feb 09 23:50:21 2020 +0100
+++ b/sat/memory/params.py	Sun Feb 09 23:50:26 2020 +0100
@@ -1,7 +1,6 @@
 #!/usr/bin/env python3
 
-
-# SAT: a jabber client
+# SàT: a XMPP client
 # Copyright (C) 2009-2020 Jérôme Poisson (goffi@goffi.org)
 
 # This program is free software: you can redistribute it and/or modify
@@ -521,18 +520,11 @@
             raise exceptions.ProfileNotSetError(
                 "The profile is needed to decrypt a password"
             )
-        d = self.host.memory.decryptValue(value, profile)
+        password = self.host.memory.decryptValue(value, profile)
 
-        def gotPlainPassword(password):
-            if (
-                password is None
-            ):  # empty value means empty password, None means decryption failure
-                raise exceptions.InternalError(
-                    _("The stored password could not be decrypted!")
-                )
-            return password
-
-        return d.addCallback(gotPlainPassword)
+        if password is None:
+            raise exceptions.InternalError("password should never be None")
+        return defer.succeed(password)
 
     def _type_to_str(self, result):
         """Convert result to string, according to its type """
@@ -1056,7 +1048,7 @@
                     lambda __: PasswordHasher.hash(value)
                 )  # profile password is hashed (empty value stays empty)
             elif value:  # other non empty passwords are encrypted with the personal key
-                d = BlockCipher.encrypt(personal_key, value)
+                d = defer.succeed(BlockCipher.encrypt(personal_key, value))
             else:
                 d = defer.succeed(value)
         else:
--- a/sat/memory/sqlite.py	Sun Feb 09 23:50:21 2020 +0100
+++ b/sat/memory/sqlite.py	Sun Feb 09 23:50:26 2020 +0100
@@ -1633,23 +1633,20 @@
                     return defer.succeed(None)
 
                 sat_password = xmpp_password
-                d1 = PasswordHasher.hash(sat_password)
+                sat_cipher = PasswordHasher.hash(sat_password)
                 personal_key = BlockCipher.getRandomKey(base64=True)
-                d2 = BlockCipher.encrypt(sat_password, personal_key)
-                d3 = BlockCipher.encrypt(personal_key, xmpp_password)
+                personal_cipher = BlockCipher.encrypt(sat_password, personal_key)
+                xmpp_cipher = BlockCipher.encrypt(personal_key, xmpp_password)
 
-                def gotValues(res):
-                    sat_cipher, personal_cipher, xmpp_cipher = res[0][1], res[1][1], res[2][1]
-                    ret.append("INSERT INTO param_ind(category,name,profile_id,value) VALUES ('%s','%s',%s,'%s')" %
-                               (C.PROFILE_PASS_PATH[0], C.PROFILE_PASS_PATH[1], id_, sat_cipher))
+                ret.append("INSERT INTO param_ind(category,name,profile_id,value) VALUES ('%s','%s',%s,'%s')" %
+                           (C.PROFILE_PASS_PATH[0], C.PROFILE_PASS_PATH[1], id_, sat_cipher))
 
-                    ret.append("INSERT INTO private_ind(namespace,key,profile_id,value) VALUES ('%s','%s',%s,'%s')" %
-                               (C.MEMORY_CRYPTO_NAMESPACE, C.MEMORY_CRYPTO_KEY, id_, personal_cipher))
+                ret.append("INSERT INTO private_ind(namespace,key,profile_id,value) VALUES ('%s','%s',%s,'%s')" %
+                           (C.MEMORY_CRYPTO_NAMESPACE, C.MEMORY_CRYPTO_KEY, id_, personal_cipher))
 
-                    ret.append("REPLACE INTO param_ind(category,name,profile_id,value) VALUES ('%s','%s',%s,'%s')" %
-                               (xmpp_pass_path[0], xmpp_pass_path[1], id_, xmpp_cipher))
+                ret.append("REPLACE INTO param_ind(category,name,profile_id,value) VALUES ('%s','%s',%s,'%s')" %
+                           (xmpp_pass_path[0], xmpp_pass_path[1], id_, xmpp_cipher))
 
-                return defer.DeferredList([d1, d2, d3]).addCallback(gotValues)
 
             for profile_id, xmpp_password in values:
                 d = self.dbpool.runQuery("SELECT id FROM profiles WHERE id=?", (profile_id,))
--- a/sat/plugins/plugin_sec_otr.py	Sun Feb 09 23:50:21 2020 +0100
+++ b/sat/plugins/plugin_sec_otr.py	Sun Feb 09 23:50:26 2020 +0100
@@ -248,12 +248,8 @@
         if self.privkey is None:
             raise exceptions.InternalError(_("Save is called but privkey is None !"))
         priv_key = hexlify(self.privkey.serializePrivateKey())
-        d = self.host.memory.encryptValue(priv_key, self.client.profile)
-
-        def save_encrypted_key(encrypted_priv_key):
-            self.client._otr_data[PRIVATE_KEY] = encrypted_priv_key
-
-        d.addCallback(save_encrypted_key)
+        encrypted_priv_key = self.host.memory.encryptValue(priv_key, self.client.profile)
+        self.client._otr_data[PRIVATE_KEY] = encrypted_priv_key
 
     def loadTrusts(self):
         trust_data = self.client._otr_data.get("trust", {})
@@ -377,7 +373,7 @@
         yield client._otr_data.load()
         encrypted_priv_key = client._otr_data.get(PRIVATE_KEY, None)
         if encrypted_priv_key is not None:
-            priv_key = yield self.host.memory.decryptValue(
+            priv_key = self.host.memory.decryptValue(
                 encrypted_priv_key, client.profile
             )
             ctxMng.account.privkey = potr.crypt.PK.parsePrivateKey(
--- a/setup.py	Sun Feb 09 23:50:21 2020 +0100
+++ b/setup.py	Sun Feb 09 23:50:26 2020 +0100
@@ -35,7 +35,6 @@
     'netifaces',
     'pillow',
     'progressbar2',
-    'pycrypto >= 2.6.1',
     'cryptography',
     'pygments',
     'pygobject',