[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 <victor1984@riseup.net>

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)