summaryrefslogtreecommitdiff
path: root/client
diff options
context:
space:
mode:
authordrebs <drebs@leap.se>2015-06-03 15:56:40 -0300
committerKali Kaneko <kali@leap.se>2015-07-27 09:58:29 -0400
commitbbfb3bb44915004a70702030aa1d2f9336a60938 (patch)
treef4736717fcf6eb436bfd9ac17f1e32a6c6bbb622 /client
parent3546eff73297945c1519e925c994e28d6ad523f4 (diff)
[bug] remove mac from secrets file
This is how a secret was stored in the secrets json file: * each secret is symmetrically encrypted amd MACed with keys derived from the user's passphrase. * the encrypted secrets dictionary is then MACed with another key derived * from the user's passphrase. * each key is derived using scrypt and a unique random salt. There are disadvantages to this approach: * repeating scrypt many times is a waste of time. * an attacker could crack whichever has weaker parameters, if they get out of sync. * if an attacker can modify the secret in a way it is good to decrypt the database, then she can also modify the MAC. The solution for this is: * completelly eliminate the MAC from the storage secrets file. * attempt to decrypt the database with whatever is got from the decryption of the secret. If that is wrong, report an error. Closes #6980.
Diffstat (limited to 'client')
-rw-r--r--client/changes/feature_6980_remove-mac-from-secrets-file1
-rw-r--r--client/src/leap/soledad/client/secrets.py71
2 files changed, 11 insertions, 61 deletions
diff --git a/client/changes/feature_6980_remove-mac-from-secrets-file b/client/changes/feature_6980_remove-mac-from-secrets-file
new file mode 100644
index 00000000..6a424013
--- /dev/null
+++ b/client/changes/feature_6980_remove-mac-from-secrets-file
@@ -0,0 +1 @@
+ o Remove MAC from secrets file. Closes #6980.
diff --git a/client/src/leap/soledad/client/secrets.py b/client/src/leap/soledad/client/secrets.py
index 57a1169a..97dbbaca 100644
--- a/client/src/leap/soledad/client/secrets.py
+++ b/client/src/leap/soledad/client/secrets.py
@@ -23,7 +23,6 @@ Soledad secrets handling.
import os
import scrypt
-import hmac
import logging
import binascii
import errno
@@ -37,7 +36,6 @@ from leap.soledad.common import soledad_assert
from leap.soledad.common import soledad_assert_type
from leap.soledad.common import document
from leap.soledad.common import errors
-from leap.soledad.common import crypto
from leap.soledad.client import events
@@ -104,8 +102,8 @@ class SoledadSecrets(object):
REMOTE_STORAGE_SECRET_LENGTH = 512
"""
- The length, in bytes, of the secret used to derive an encryption key and a
- MAC auth key for remote storage.
+ The length, in bytes, of the secret used to derive an encryption key for
+ remote storage.
"""
SALT_LENGTH = 64
@@ -275,7 +273,7 @@ class SoledadSecrets(object):
content = None
with open(self._secrets_path, 'r') as f:
content = json.loads(f.read())
- _, mac, active_secret = self._import_recovery_document(content)
+ _, active_secret = self._import_recovery_document(content)
# choose first secret if no secret_id was given
if self._secret_id is None:
if active_secret is None:
@@ -291,7 +289,7 @@ class SoledadSecrets(object):
self._secrets[self._secret_id] += new_piece
enlarged = True
# store and save in shared db if needed
- if not mac or enlarged:
+ if enlarged:
self._store_secrets()
self._put_secrets_in_shared_db()
@@ -311,9 +309,7 @@ class SoledadSecrets(object):
logger.info(
'Found cryptographic secrets in shared recovery '
'database.')
- _, mac, active_secret = self._import_recovery_document(doc.content)
- if mac is False:
- self.put_secrets_in_shared_db()
+ _, active_secret = self._import_recovery_document(doc.content)
self._store_secrets() # save new secrets in local file
if self._secret_id is None:
if active_secret is None:
@@ -371,32 +367,20 @@ class SoledadSecrets(object):
{
'storage_secrets': {
'<storage_secret id>': {
- 'kdf': 'scrypt',
- 'kdf_salt': '<b64 repr of salt>'
- 'kdf_length': <key length>
'cipher': 'aes256',
'length': <secret length>,
'secret': '<encrypted storage_secret>',
},
},
'active_secret': '<secret_id>',
- 'kdf': 'scrypt',
- 'kdf_salt': '<b64 repr of salt>',
- 'kdf_length: <key length>,
- '_mac_method': 'hmac',
- '_mac': '<mac>'
}
Note that multiple storage secrets might be stored in one recovery
- document. This method will also calculate a MAC of a string
- representation of the secrets dictionary.
+ document.
:return: The recovery document.
:rtype: dict
"""
- # create salt and key for calculating MAC
- salt = os.urandom(self.SALT_LENGTH)
- key = scrypt.hash(self._passphrase_as_string(), salt, buflen=32)
# encrypt secrets
encrypted_secrets = {}
for secret_id in self._secrets:
@@ -406,14 +390,6 @@ class SoledadSecrets(object):
data = {
self.STORAGE_SECRETS_KEY: encrypted_secrets,
self.ACTIVE_SECRET_KEY: self._secret_id,
- self.KDF_KEY: self.KDF_SCRYPT,
- self.KDF_SALT_KEY: binascii.b2a_base64(salt),
- self.KDF_LENGTH_KEY: len(key),
- crypto.MAC_METHOD_KEY: crypto.MacMethods.HMAC,
- crypto.MAC_KEY: hmac.new(
- key,
- json.dumps(encrypted_secrets, sort_keys=True),
- sha256).hexdigest(),
}
return data
@@ -428,38 +404,11 @@ class SoledadSecrets(object):
:param data: The recovery document.
:type data: dict
- :return: A tuple containing the number of imported secrets, whether
- there was MAC information available for authenticating, and
- the secret_id of the last active secret.
- :rtype: (int, bool)
+ :return: A tuple containing the number of imported secrets and the
+ secret_id of the last active secret.
+ :rtype: (int, str)
"""
soledad_assert(self.STORAGE_SECRETS_KEY in data)
- # check mac of the recovery document
- mac = None
- if crypto.MAC_KEY in data:
- soledad_assert(data[crypto.MAC_KEY] is not None)
- soledad_assert(crypto.MAC_METHOD_KEY in data)
- soledad_assert(self.KDF_KEY in data)
- soledad_assert(self.KDF_SALT_KEY in data)
- soledad_assert(self.KDF_LENGTH_KEY in data)
- if data[crypto.MAC_METHOD_KEY] == crypto.MacMethods.HMAC:
- key = scrypt.hash(
- self._passphrase_as_string(),
- binascii.a2b_base64(data[self.KDF_SALT_KEY]),
- buflen=32)
- mac = hmac.new(
- key,
- json.dumps(
- data[self.STORAGE_SECRETS_KEY], sort_keys=True),
- sha256).hexdigest()
- else:
- raise crypto.UnknownMacMethodError('Unknown MAC method: %s.' %
- data[crypto.MAC_METHOD_KEY])
- if mac != data[crypto.MAC_KEY]:
- raise crypto.WrongMacError(
- 'Could not authenticate recovery document\'s '
- 'contents.')
-
# include secrets in the secret pool.
secret_count = 0
secrets = data[self.STORAGE_SECRETS_KEY].items()
@@ -477,7 +426,7 @@ class SoledadSecrets(object):
except SecretsException as e:
logger.error("Failed to decrypt storage secret: %s"
% str(e))
- return secret_count, mac, active_secret
+ return secret_count, active_secret
def _get_secrets_from_shared_db(self):
"""