# HG changeset patch # User Goffi # Date 1581288626 -3600 # Node ID 330a5f1d9eea9b79a491fccfc66c0ea09243494c # Parent 30e08d904208fe8fcca55148163c643e0fe86269 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 diff -r 30e08d904208 -r 330a5f1d9eea sat/memory/crypto.py --- 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 . -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 diff -r 30e08d904208 -r 330a5f1d9eea sat/memory/memory.py --- 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( diff -r 30e08d904208 -r 330a5f1d9eea sat/memory/params.py --- 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: diff -r 30e08d904208 -r 330a5f1d9eea sat/memory/sqlite.py --- 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,)) diff -r 30e08d904208 -r 330a5f1d9eea sat/plugins/plugin_sec_otr.py --- 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( diff -r 30e08d904208 -r 330a5f1d9eea setup.py --- 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',