From 7877527fe64eaee1f7f107913a4a3dc78767a338 Mon Sep 17 00:00:00 2001 From: Victor Shyba Date: Wed, 7 Dec 2016 02:03:58 -0300 Subject: [feature] Change CTR to GCM on secrets.py Current implementation can allow tampering and the CTR->GCM exchange can help to avoid it. This commits also alters a behaviour where we moved ahead after failing to decrypt a recovery document. IMHO we can't move ahead as this is a fatal error. Signed-off-by: Victor Shyba --- client/src/leap/soledad/client/secrets.py | 12 +++++++++--- testing/tests/client/test_aux_methods.py | 4 ++-- testing/tests/client/test_crypto.py | 5 +++-- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/client/src/leap/soledad/client/secrets.py b/client/src/leap/soledad/client/secrets.py index 06488f74..3fe98c64 100644 --- a/client/src/leap/soledad/client/secrets.py +++ b/client/src/leap/soledad/client/secrets.py @@ -142,7 +142,7 @@ class SoledadSecrets(object): KDF_SALT_KEY = 'kdf_salt' KDF_LENGTH_KEY = 'kdf_length' KDF_SCRYPT = 'scrypt' - CIPHER_AES256 = 'aes256' # deprecated, AES-GCM + CIPHER_AES256 = 'aes256' # deprecated, AES-CTR CIPHER_AES256_GCM = _crypto.ENC_METHOD.aes_256_gcm RECOVERY_DOC_VERSION_KEY = 'version' RECOVERY_DOC_VERSION = 1 @@ -451,6 +451,7 @@ class SoledadSecrets(object): except SecretsException as e: logger.error("failed to decrypt storage secret: %s" % str(e)) + raise e return secret_count, active_secret def _get_secrets_from_shared_db(self): @@ -549,7 +550,12 @@ class SoledadSecrets(object): iv, ciphertext = encrypted_secret_dict[self.SECRET_KEY].split( self.SEPARATOR, 1) ciphertext = binascii.a2b_base64(ciphertext) - decrypted_secret = _crypto.decrypt_sym(ciphertext, key, iv, doc_cipher) + try: + decrypted_secret = _crypto.decrypt_sym( + ciphertext, key, iv, doc_cipher) + except Exception as e: + logger.error(e) + raise SecretsException("Unable to decrypt secret.") if encrypted_secret_dict[self.LENGTH_KEY] != len(decrypted_secret): raise SecretsException("Wrong length of decrypted secret.") return decrypted_secret @@ -583,7 +589,7 @@ class SoledadSecrets(object): salt = os.urandom(self.SALT_LENGTH) # get a 256-bit key key = scrypt.hash(self._passphrase_as_string(), salt, buflen=32) - doc_cipher = doc_cipher or self.CIPHER_AES256 + doc_cipher = doc_cipher or self.CIPHER_AES256_GCM iv, ciphertext = _crypto.encrypt_sym(decrypted_secret, key, doc_cipher) ciphertext = binascii.b2a_base64(ciphertext) encrypted_secret_dict = { diff --git a/testing/tests/client/test_aux_methods.py b/testing/tests/client/test_aux_methods.py index c25ff8ca..9b4a175f 100644 --- a/testing/tests/client/test_aux_methods.py +++ b/testing/tests/client/test_aux_methods.py @@ -21,10 +21,10 @@ import os from twisted.internet import defer -from leap.soledad.common.errors import DatabaseAccessError from leap.soledad.client import Soledad from leap.soledad.client.adbapi import U1DBConnectionPool from leap.soledad.client.secrets import PassphraseTooShort +from leap.soledad.client.secrets import SecretsException from test_soledad.util import BaseSoledadTest @@ -108,7 +108,7 @@ class AuxMethodsTestCase(BaseSoledadTest): sol.change_passphrase(u'654321') sol.close() - with self.assertRaises(DatabaseAccessError): + with self.assertRaises(SecretsException): self._soledad_instance( 'leap@leap.se', passphrase=u'123', diff --git a/testing/tests/client/test_crypto.py b/testing/tests/client/test_crypto.py index 277d5430..49a61438 100644 --- a/testing/tests/client/test_crypto.py +++ b/testing/tests/client/test_crypto.py @@ -200,8 +200,9 @@ class RecoveryDocumentTestCase(BaseSoledadTest): encrypted_secret = rd[ self._soledad.secrets.STORAGE_SECRETS_KEY][secret_id] self.assertTrue(self._soledad.secrets.CIPHER_KEY in encrypted_secret) - self.assertTrue( - encrypted_secret[self._soledad.secrets.CIPHER_KEY] == 'aes256') + self.assertEquals( + _crypto.ENC_METHOD.aes_256_gcm, + encrypted_secret[self._soledad.secrets.CIPHER_KEY]) self.assertTrue(self._soledad.secrets.LENGTH_KEY in encrypted_secret) self.assertTrue(self._soledad.secrets.SECRET_KEY in encrypted_secret) -- cgit v1.2.3