From 46243e18e99e4dc3273f6bef65632eaa9b4c752e Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Sat, 19 Mar 2022 11:57:58 +0300 Subject: [PATCH 01/28] Argon2 the second part: implement decryption of v2 keys 1. Note: I have rebased this on top of 78f041440cd6122e15ba3d1041b84cb56c0be2af and fixed the code to work with the new Passphrase.argon2 interface 2. Refactor: s/enc_key/encrypted_key/ - I intend to use the name enc_key for something else 3. Dispatch on algorithm instead of version https://github.com/borgbackup/borg/issues/747#issuecomment-1076160401 New: rebased on 28731c56d179896a76106529d9fdd9aa84e883d5 (current master) --- src/borg/crypto/key.py | 43 ++++++++++++++--- src/borg/item.pyx | 8 +++- src/borg/testsuite/crypto.py | 89 ++++++++++++++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 7 deletions(-) diff --git a/src/borg/crypto/key.py b/src/borg/crypto/key.py index 0a93e856c5..d947c8390b 100644 --- a/src/borg/crypto/key.py +++ b/src/borg/crypto/key.py @@ -51,6 +51,10 @@ class RepoKeyNotFoundError(Error): """No key entry found in the config of repository {}.""" +class UnsupportedKeyFormatError(Error): + """Your borg key is stored in an unsupported format. Try using a newer version of borg.""" + + class TAMRequiredError(IntegrityError): __doc__ = textwrap.dedent(""" Manifest is unauthenticated, but it is required for this repository. @@ -430,13 +434,40 @@ def decrypt_key_file(self, data, passphrase): unpacker = get_limited_unpacker('key') unpacker.feed(data) data = unpacker.unpack() - enc_key = EncryptedKey(internal_dict=data) - assert enc_key.version == 1 - assert enc_key.algorithm == 'sha256' - key = passphrase.kdf(enc_key.salt, enc_key.iterations, 32) - data = AES(key, b'\0'*16).decrypt(enc_key.data) - if hmac.compare_digest(hmac_sha256(key, data), enc_key.hash): + encrypted_key = EncryptedKey(internal_dict=data) + if encrypted_key.version != 1: + raise UnsupportedKeyFormatError() + else: + if encrypted_key.algorithm == 'sha256': + return self.decrypt_key_file_pbkdf2(encrypted_key, passphrase) + elif encrypted_key.algorithm == 'argon2 aes256-ctr hmac-sha256': + return self.decrypt_key_file_argon2(encrypted_key, passphrase) + else: + raise UnsupportedKeyFormatError() + + def decrypt_key_file_pbkdf2(self, encrypted_key, passphrase): + key = passphrase.kdf(encrypted_key.salt, encrypted_key.iterations, 32) + data = AES(key, b'\0'*16).decrypt(encrypted_key.data) + if hmac.compare_digest(hmac_sha256(key, data), encrypted_key.hash): return data + return None + + def decrypt_key_file_argon2(self, encrypted_key, passphrase): + key = passphrase.argon2( + output_len_in_bytes=64, + salt=encrypted_key.salt, + time_cost=encrypted_key.argon2_time_cost, + memory_cost=encrypted_key.argon2_memory_cost, + parallelism=encrypted_key.argon2_parallelism, + type=encrypted_key.argon2_type, + ) + enc_key, mac_key = key[:32], key[32:] + ae_cipher = AES256_CTR_HMAC_SHA256( + iv=0, header_len=0, aad_offset=0, + enc_key=enc_key, + mac_key=mac_key, + ) + return ae_cipher.decrypt(encrypted_key.data) def encrypt_key_file(self, data, passphrase): salt = os.urandom(32) diff --git a/src/borg/item.pyx b/src/borg/item.pyx index 1f91f52308..85cfc95ebb 100644 --- a/src/borg/item.pyx +++ b/src/borg/item.pyx @@ -7,6 +7,7 @@ from .helpers import bigint_to_int, int_to_bigint from .helpers import StableDict from .helpers import format_file_size + cdef extern from "_item.c": object _object_to_optr(object obj) object _optr_to_object(object bytes) @@ -294,7 +295,8 @@ class EncryptedKey(PropDict): If a EncryptedKey shall be serialized, give as_dict() method output to msgpack packer. """ - VALID_KEYS = {'version', 'algorithm', 'iterations', 'salt', 'hash', 'data'} # str-typed keys + VALID_KEYS = { 'version', 'algorithm', 'iterations', 'salt', 'hash', 'data', + 'argon2_time_cost', 'argon2_memory_cost', 'argon2_parallelism', 'argon2_type' } __slots__ = ("_dict", ) # avoid setting attributes not supported by properties @@ -304,6 +306,10 @@ class EncryptedKey(PropDict): salt = PropDict._make_property('salt', bytes) hash = PropDict._make_property('hash', bytes) data = PropDict._make_property('data', bytes) + argon2_time_cost = PropDict._make_property('argon2_time_cost', int) + argon2_memory_cost = PropDict._make_property('argon2_memory_cost', int) + argon2_parallelism = PropDict._make_property('argon2_parallelism', int) + argon2_type = PropDict._make_property('argon2_type', str, encode=str.encode, decode=bytes.decode) class Key(PropDict): diff --git a/src/borg/testsuite/crypto.py b/src/borg/testsuite/crypto.py index baf8cba2cd..ae2c7b7a23 100644 --- a/src/borg/testsuite/crypto.py +++ b/src/borg/testsuite/crypto.py @@ -1,9 +1,15 @@ from binascii import hexlify +import pytest + from ..crypto.low_level import AES256_CTR_HMAC_SHA256, AES256_OCB, CHACHA20_POLY1305, UNENCRYPTED, \ IntegrityError, is_libressl from ..crypto.low_level import bytes_to_long, bytes_to_int, long_to_bytes from ..crypto.low_level import hkdf_hmac_sha512 +from ..crypto.low_level import AES, hmac_sha256 +from ..crypto.key import KeyfileKey, UnsupportedKeyFormatError +from ..helpers.passphrase import Passphrase +from ..helpers import msgpack from . import BaseTestCase @@ -248,3 +254,86 @@ def test_hkdf_hmac_sha512_5(self): okm = hkdf_hmac_sha512(ikm, salt, info, l) assert okm == bytes.fromhex('1407d46013d98bc6decefcfee55f0f90b0c7f63d68eb1a80eaf07e953cfc0a3a5240a155d6e4daa965bb') + + +def test_decrypt_key_file_argon2_aes256_ctr_hmac_sha256(monkeypatch): + plain = b'hello' + # echo -n "hello, pass phrase" | argon2 saltsaltsaltsalt -id -t 3 -m 16 -p 4 -l 64 -r + enc_key = bytes.fromhex('3dd855b778ba292eda7bf708a9ea111ee99c5c45d2e9a2773d126de46d344410') + mac_key = bytes.fromhex('0b0b65fdccaea7cf5b9a6214cd867983e2326abeccedf1dceb1feee0ae74075b') + ae_cipher = AES256_CTR_HMAC_SHA256( + iv=0, header_len=0, aad_offset=0, + enc_key=enc_key, + mac_key=mac_key, + ) + + envelope = ae_cipher.encrypt(plain) + + encrypted = msgpack.packb({ + 'version': 1, + 'salt': b'salt'*4, + 'argon2_time_cost': 3, + 'argon2_memory_cost': 2**16, + 'argon2_parallelism': 4, + 'argon2_type': b'id', + 'algorithm': 'argon2 aes256-ctr hmac-sha256', + 'data': envelope, + }) + monkeypatch.setenv('BORG_PASSPHRASE', "hello, pass phrase") + passphrase = Passphrase.new() + key = KeyfileKey(None) + + decrypted = key.decrypt_key_file(encrypted, passphrase) + + assert decrypted == plain + + +def test_decrypt_key_file_pbkdf2_sha256_aes256_ctr_hmac_sha256(monkeypatch): + plain = b'hello' + salt = b'salt'*4 + iterations = 100000 + monkeypatch.setenv('BORG_PASSPHRASE', "hello, pass phrase") + passphrase = Passphrase.new() + key = passphrase.kdf(salt, iterations, 32) + hash = hmac_sha256(key, plain) + data = AES(key, b'\0'*16).encrypt(plain) + encrypted = msgpack.packb({ + 'version': 1, + 'algorithm': 'sha256', + 'iterations': iterations, + 'salt': salt, + 'data': data, + 'hash': hash, + }) + key = KeyfileKey(None) + + decrypted = key.decrypt_key_file(encrypted, passphrase) + + assert decrypted == plain + + +def test_decrypt_key_file_unsupported_algorithm(monkeypatch): + '''We will add more algorithms in the future. We should raise a helpful error.''' + monkeypatch.setenv('BORG_PASSPHRASE', "hello, pass phrase") + passphrase = Passphrase.new() + key = KeyfileKey(None) + encrypted = msgpack.packb({ + 'algorithm': 'THIS ALGORITHM IS NOT SUPPORTED', + 'version': 1, + }) + + with pytest.raises(UnsupportedKeyFormatError): + key.decrypt_key_file(encrypted, passphrase) + + +def test_decrypt_key_file_v2_is_unsupported(monkeypatch): + '''There may eventually be a version 2 of the format. For now we should raise a helpful error.''' + monkeypatch.setenv('BORG_PASSPHRASE', "hello, pass phrase") + passphrase = Passphrase.new() + key = KeyfileKey(None) + encrypted = msgpack.packb({ + 'version': 2, + }) + + with pytest.raises(UnsupportedKeyFormatError): + key.decrypt_key_file(encrypted, passphrase) From 5dd38c06abbdbad2b4dfd9398eb04429ca7c4198 Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Sun, 27 Mar 2022 12:13:27 +0300 Subject: [PATCH 02/28] add a roundtrip test --- src/borg/crypto/key.py | 2 +- src/borg/testsuite/crypto.py | 31 ++++++++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/borg/crypto/key.py b/src/borg/crypto/key.py index d947c8390b..05685244b9 100644 --- a/src/borg/crypto/key.py +++ b/src/borg/crypto/key.py @@ -612,7 +612,7 @@ def load(self, target, passphrase): self.target = target return success - def save(self, target, passphrase, create=False): + def save(self, target, passphrase, algorithm='TODO', create=False): key_data = self._save(passphrase) if self.STORAGE == KeyBlobStorage.KEYFILE: if create and os.path.isfile(target): diff --git a/src/borg/testsuite/crypto.py b/src/borg/testsuite/crypto.py index ae2c7b7a23..4cd40d8255 100644 --- a/src/borg/testsuite/crypto.py +++ b/src/borg/testsuite/crypto.py @@ -1,4 +1,6 @@ from binascii import hexlify +from unittest.mock import MagicMock +from binascii import a2b_base64 import pytest @@ -7,7 +9,7 @@ from ..crypto.low_level import bytes_to_long, bytes_to_int, long_to_bytes from ..crypto.low_level import hkdf_hmac_sha512 from ..crypto.low_level import AES, hmac_sha256 -from ..crypto.key import KeyfileKey, UnsupportedKeyFormatError +from ..crypto.key import KeyfileKey, UnsupportedKeyFormatError, RepoKey from ..helpers.passphrase import Passphrase from ..helpers import msgpack @@ -337,3 +339,30 @@ def test_decrypt_key_file_v2_is_unsupported(monkeypatch): with pytest.raises(UnsupportedKeyFormatError): key.decrypt_key_file(encrypted, passphrase) + + +def test_key_file_save_argon2_aes256_ctr_hmac_sha256(): + def to_dict(key): + extract = 'repository_id', 'enc_key', 'enc_hmac_key', 'id_key', 'chunk_seed' + return {a: getattr(key, a) for a in extract} + + repository = MagicMock() + repository.id = b'repository_id' + save_me = RepoKey(repository) + save_me.repository_id = b'repository_id' + save_me.enc_key = b'enc_key' + save_me.enc_hmac_key = b'enc_hmac_key' + save_me.id_key = b'id_key' + save_me.chunk_seed = 42 + passphrase = MagicMock() + passphrase.kdf.return_value = b'derived key'.rjust(32) + + save_me.save(algorithm='argon2 aes256_ctr hmac_sha256', target=repository, passphrase=passphrase) + saved = repository.save_key.call_args.args[0] + repository.load_key.return_value = saved + load_me = RepoKey(repository) + load_me.load(target=repository, passphrase=passphrase) + + assert to_dict(load_me) == to_dict(save_me) + with pytest.raises(AssertionError): + assert msgpack.unpackb(a2b_base64(saved))[b'algorithm'] == b'argon2 aes256_ctr hmac_sha256' From c092bf26b757a8faa763d1acd3fff0d0fbf59e40 Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Sun, 27 Mar 2022 12:21:59 +0300 Subject: [PATCH 03/28] Make algorithm mandatory --- src/borg/archiver.py | 3 ++- src/borg/crypto/key.py | 12 +++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/borg/archiver.py b/src/borg/archiver.py index 2b21f5ba8a..73117e2a23 100644 --- a/src/borg/archiver.py +++ b/src/borg/archiver.py @@ -398,7 +398,8 @@ def do_change_location(self, args, repository, manifest, key, cache): setattr(key_new, name, value) key_new.target = key_new.get_new_target(args) - key_new.save(key_new.target, key._passphrase, create=True) # save with same passphrase + algorithm = 'sha256' # TODO: copy from `key` + key_new.save(key_new.target, key._passphrase, create=True, algorithm=algorithm) # save with same passphrase # rewrite the manifest with the new key, so that the key-type byte of the manifest changes manifest.key = key_new diff --git a/src/borg/crypto/key.py b/src/borg/crypto/key.py index 05685244b9..7c007d8f37 100644 --- a/src/borg/crypto/key.py +++ b/src/borg/crypto/key.py @@ -502,7 +502,8 @@ def _save(self, passphrase): def change_passphrase(self, passphrase=None): if passphrase is None: passphrase = Passphrase.new(allow_empty=True) - self.save(self.target, passphrase) + algorithm = 'sha256' # TODO: copy from `self` + self.save(self.target, passphrase, algorithm=algorithm) @classmethod def create(cls, repository, args): @@ -512,7 +513,8 @@ def create(cls, repository, args): key.init_from_random_data() key.init_ciphers() target = key.get_new_target(args) - key.save(target, passphrase, create=True) + algorithm = 'sha256' # TODO: initialize based on `args` + key.save(target, passphrase, create=True, algorithm=algorithm) logger.info('Key in "%s" created.' % target) logger.info('Keep this key safe. Your data will be inaccessible without it.') return key @@ -612,7 +614,7 @@ def load(self, target, passphrase): self.target = target return success - def save(self, target, passphrase, algorithm='TODO', create=False): + def save(self, target, passphrase, algorithm, create=False): key_data = self._save(passphrase) if self.STORAGE == KeyBlobStorage.KEYFILE: if create and os.path.isfile(target): @@ -688,8 +690,8 @@ def load(self, target, passphrase): self.logically_encrypted = False return success - def save(self, target, passphrase, create=False): - super().save(target, passphrase, create=create) + def save(self, target, passphrase, algorithm, create=False): + super().save(target, passphrase, algorithm, create=create) self.logically_encrypted = False def init_ciphers(self, manifest_data=None): From 40dc9be8c72e88d10011601d42933499b90fe90b Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Sun, 27 Mar 2022 12:53:02 +0300 Subject: [PATCH 04/28] Implement encryption --- src/borg/crypto/key.py | 50 +++++++++++++++++++++++++++++++++--- src/borg/testsuite/crypto.py | 5 ++-- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/src/borg/crypto/key.py b/src/borg/crypto/key.py index 7c007d8f37..42e3505578 100644 --- a/src/borg/crypto/key.py +++ b/src/borg/crypto/key.py @@ -469,7 +469,15 @@ def decrypt_key_file_argon2(self, encrypted_key, passphrase): ) return ae_cipher.decrypt(encrypted_key.data) - def encrypt_key_file(self, data, passphrase): + def encrypt_key_file(self, data, passphrase, algorithm): + if algorithm == 'sha256': + return self.encrypt_key_file_pbkdf2(data, passphrase) + elif algorithm == 'argon2 aes256_ctr hmac_sha256': + return self.encrypt_key_file_argon2(data, passphrase) + else: + raise ValueError(f'Unexpected algorithm: {algorithm}') + + def encrypt_key_file_pbkdf2(self, data, passphrase): salt = os.urandom(32) iterations = PBKDF2_ITERATIONS key = passphrase.kdf(salt, iterations, 32) @@ -485,7 +493,41 @@ def encrypt_key_file(self, data, passphrase): ) return msgpack.packb(enc_key.as_dict()) - def _save(self, passphrase): + def encrypt_key_file_argon2(self, data, passphrase): + # https://www.rfc-editor.org/rfc/rfc9106.html#section-4-6.2 + time_cost = 3 + memory_cost = 2**16 + parallelism = 4 + type_ = 'id' + + salt = os.urandom(16) + key = passphrase.argon2( + output_len_in_bytes=64, + salt=salt, + time_cost=time_cost, + memory_cost=memory_cost, + parallelism=parallelism, + type=type_, + ) + enc_key, mac_key = key[:32], key[32:] + ae_cipher = AES256_CTR_HMAC_SHA256( + iv=0, header_len=0, aad_offset=0, + enc_key=enc_key, + mac_key=mac_key, + ) + encrypted_key = EncryptedKey( + version=1, + algorithm='argon2 aes256-ctr hmac-sha256', + salt=salt, + argon2_time_cost=time_cost, + argon2_memory_cost=memory_cost, + argon2_parallelism=parallelism, + argon2_type=type_, + data=ae_cipher.encrypt(data), + ) + return msgpack.packb(encrypted_key.as_dict()) + + def _save(self, passphrase, algorithm): key = Key( version=1, repository_id=self.repository_id, @@ -495,7 +537,7 @@ def _save(self, passphrase): chunk_seed=self.chunk_seed, tam_required=self.tam_required, ) - data = self.encrypt_key_file(msgpack.packb(key.as_dict()), passphrase) + data = self.encrypt_key_file(msgpack.packb(key.as_dict()), passphrase, algorithm) key_data = '\n'.join(textwrap.wrap(b2a_base64(data).decode('ascii'))) return key_data @@ -615,7 +657,7 @@ def load(self, target, passphrase): return success def save(self, target, passphrase, algorithm, create=False): - key_data = self._save(passphrase) + key_data = self._save(passphrase, algorithm) if self.STORAGE == KeyBlobStorage.KEYFILE: if create and os.path.isfile(target): # if a new keyfile key repository is created, ensure that an existing keyfile of another diff --git a/src/borg/testsuite/crypto.py b/src/borg/testsuite/crypto.py index 4cd40d8255..a643d97e9e 100644 --- a/src/borg/testsuite/crypto.py +++ b/src/borg/testsuite/crypto.py @@ -355,7 +355,7 @@ def to_dict(key): save_me.id_key = b'id_key' save_me.chunk_seed = 42 passphrase = MagicMock() - passphrase.kdf.return_value = b'derived key'.rjust(32) + passphrase.argon2.return_value = b'derived key'.rjust(64) save_me.save(algorithm='argon2 aes256_ctr hmac_sha256', target=repository, passphrase=passphrase) saved = repository.save_key.call_args.args[0] @@ -364,5 +364,4 @@ def to_dict(key): load_me.load(target=repository, passphrase=passphrase) assert to_dict(load_me) == to_dict(save_me) - with pytest.raises(AssertionError): - assert msgpack.unpackb(a2b_base64(saved))[b'algorithm'] == b'argon2 aes256_ctr hmac_sha256' + assert msgpack.unpackb(a2b_base64(saved))[b'algorithm'] == b'argon2 aes256-ctr hmac-sha256' From 32796feae08fa810b5d2bfe8b2c9c656d9892c18 Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Mon, 28 Mar 2022 09:09:11 +0300 Subject: [PATCH 05/28] borg init - defalt to argon2 --- src/borg/archiver.py | 2 ++ src/borg/crypto/key.py | 4 ++-- src/borg/testsuite/archiver.py | 16 +++++++++++++++- src/borg/testsuite/crypto.py | 2 +- 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/borg/archiver.py b/src/borg/archiver.py index 73117e2a23..a10e8b1a89 100644 --- a/src/borg/archiver.py +++ b/src/borg/archiver.py @@ -4311,6 +4311,8 @@ def define_borg_mount(parser): help='Set storage quota of the new repository (e.g. 5G, 1.5T). Default: no quota.') subparser.add_argument('--make-parent-dirs', dest='make_parent_dirs', action='store_true', help='create the parent directories of the repository directory, if they are missing.') + subparser.add_argument('--key-algorithm', dest='key_algorithm', default='argon2 aes256-ctr hmac-sha256', + choices=['pbkdf2-sha256 aes256-ctr hmac-sha256', 'argon2 aes256-ctr hmac-sha256']) # borg key subparser = subparsers.add_parser('key', parents=[mid_common_parser], add_help=False, diff --git a/src/borg/crypto/key.py b/src/borg/crypto/key.py index 42e3505578..8a964f2e14 100644 --- a/src/borg/crypto/key.py +++ b/src/borg/crypto/key.py @@ -472,7 +472,7 @@ def decrypt_key_file_argon2(self, encrypted_key, passphrase): def encrypt_key_file(self, data, passphrase, algorithm): if algorithm == 'sha256': return self.encrypt_key_file_pbkdf2(data, passphrase) - elif algorithm == 'argon2 aes256_ctr hmac_sha256': + elif algorithm == 'argon2 aes256-ctr hmac-sha256': return self.encrypt_key_file_argon2(data, passphrase) else: raise ValueError(f'Unexpected algorithm: {algorithm}') @@ -555,7 +555,7 @@ def create(cls, repository, args): key.init_from_random_data() key.init_ciphers() target = key.get_new_target(args) - algorithm = 'sha256' # TODO: initialize based on `args` + algorithm = 'sha256' if args.key_algorithm == 'pbkdf2-sha256 aes256-ctr hmac-sha256' else args.key_algorithm key.save(target, passphrase, create=True, algorithm=algorithm) logger.info('Key in "%s" created.' % target) logger.info('Keep this key safe. Your data will be inaccessible without it.') diff --git a/src/borg/testsuite/archiver.py b/src/borg/testsuite/archiver.py index 62a33fb898..cb1ec2dd55 100644 --- a/src/borg/testsuite/archiver.py +++ b/src/borg/testsuite/archiver.py @@ -16,7 +16,7 @@ import tempfile import time import unittest -from binascii import unhexlify, b2a_base64 +from binascii import unhexlify, b2a_base64, a2b_base64 from configparser import ConfigParser from datetime import datetime from datetime import timezone @@ -3603,6 +3603,20 @@ def test_recovery_from_deleted_repo_nonce(self): self.cmd('create', self.repository_location + '::test2', 'input') assert os.path.exists(nonce) + def test_init_defaults_to_argon2(self): + """https://github.com/borgbackup/borg/issues/747#issuecomment-1076160401""" + self.cmd('init', '--encryption=repokey', self.repository_location) + with Repository(self.repository_path) as repository: + key = msgpack.unpackb(a2b_base64(repository.load_key())) + assert key[b'algorithm'] == b'argon2 aes256-ctr hmac-sha256' + + def test_init_with_explicit_key_algorithm(self): + """https://github.com/borgbackup/borg/issues/747#issuecomment-1076160401""" + self.cmd('init', '--encryption=repokey', '--key-algorithm', 'pbkdf2-sha256 aes256-ctr hmac-sha256', self.repository_location) + with Repository(self.repository_path) as repository: + key = msgpack.unpackb(a2b_base64(repository.load_key())) + assert key[b'algorithm'] == b'sha256' + @unittest.skipUnless('binary' in BORG_EXES, 'no borg.exe available') class ArchiverTestCaseBinary(ArchiverTestCase): diff --git a/src/borg/testsuite/crypto.py b/src/borg/testsuite/crypto.py index a643d97e9e..8ecb2d9285 100644 --- a/src/borg/testsuite/crypto.py +++ b/src/borg/testsuite/crypto.py @@ -357,7 +357,7 @@ def to_dict(key): passphrase = MagicMock() passphrase.argon2.return_value = b'derived key'.rjust(64) - save_me.save(algorithm='argon2 aes256_ctr hmac_sha256', target=repository, passphrase=passphrase) + save_me.save(algorithm='argon2 aes256-ctr hmac-sha256', target=repository, passphrase=passphrase) saved = repository.save_key.call_args.args[0] repository.load_key.return_value = saved load_me = RepoKey(repository) From 86cf05af70b4922c7d220ab90cdb542bc787cfe6 Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Mon, 28 Mar 2022 09:47:43 +0300 Subject: [PATCH 06/28] add test_passphrase_retry --- src/borg/testsuite/crypto.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/borg/testsuite/crypto.py b/src/borg/testsuite/crypto.py index 8ecb2d9285..767a85acc1 100644 --- a/src/borg/testsuite/crypto.py +++ b/src/borg/testsuite/crypto.py @@ -1,5 +1,6 @@ from binascii import hexlify from unittest.mock import MagicMock +import unittest from binascii import a2b_base64 import pytest @@ -365,3 +366,17 @@ def to_dict(key): assert to_dict(load_me) == to_dict(save_me) assert msgpack.unpackb(a2b_base64(saved))[b'algorithm'] == b'argon2 aes256-ctr hmac-sha256' + + +@unittest.mock.patch('getpass.getpass') +def test_passphrase_retry(getpass, monkeypatch): + repository = MagicMock(id=b'repository_id') + getpass.return_value = "hello, pass phrase" + monkeypatch.setenv('BORG_DISPLAY_PASSPHRASE', 'no') + RepoKey.create(repository, args=MagicMock(key_algorithm='argon2 aes256-ctr hmac-sha256')) + repository.load_key.return_value = repository.save_key.call_args.args[0] + + # BUG: raises borg.crypto.low_level.IntegrityError + import borg.crypto.low_level + with pytest.raises(borg.crypto.low_level.IntegrityError): + RepoKey.detect(repository, manifest_data=MagicMock()) From 3cdbc5d5f08319a8213bcbd6ba787428d5dc0038 Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Mon, 28 Mar 2022 09:48:42 +0300 Subject: [PATCH 07/28] add a docstring --- src/borg/testsuite/crypto.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/borg/testsuite/crypto.py b/src/borg/testsuite/crypto.py index 767a85acc1..035931b188 100644 --- a/src/borg/testsuite/crypto.py +++ b/src/borg/testsuite/crypto.py @@ -370,6 +370,7 @@ def to_dict(key): @unittest.mock.patch('getpass.getpass') def test_passphrase_retry(getpass, monkeypatch): + """https://github.com/borgbackup/borg/pull/6469#discussion_r832670411""" repository = MagicMock(id=b'repository_id') getpass.return_value = "hello, pass phrase" monkeypatch.setenv('BORG_DISPLAY_PASSPHRASE', 'no') From 339d50ea8096421cde60798ed4e57d228f16ebfb Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Mon, 28 Mar 2022 10:17:35 +0300 Subject: [PATCH 08/28] Fix integrity errors in key.detect() --- src/borg/crypto/key.py | 6 +++++- src/borg/testsuite/crypto.py | 8 +++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/borg/crypto/key.py b/src/borg/crypto/key.py index 8a964f2e14..1cb9a4f607 100644 --- a/src/borg/crypto/key.py +++ b/src/borg/crypto/key.py @@ -25,6 +25,7 @@ from .nonces import NonceManager from .low_level import AES, bytes_to_long, long_to_bytes, bytes_to_int, num_cipher_blocks, hmac_sha256, blake2b_256, hkdf_hmac_sha512 from .low_level import AES256_CTR_HMAC_SHA256, AES256_CTR_BLAKE2b, AES256_OCB, CHACHA20_POLY1305 +from . import low_level class UnsupportedPayloadError(Error): @@ -467,7 +468,10 @@ def decrypt_key_file_argon2(self, encrypted_key, passphrase): enc_key=enc_key, mac_key=mac_key, ) - return ae_cipher.decrypt(encrypted_key.data) + try: + return ae_cipher.decrypt(encrypted_key.data) + except low_level.IntegrityError: + return None def encrypt_key_file(self, data, passphrase, algorithm): if algorithm == 'sha256': diff --git a/src/borg/testsuite/crypto.py b/src/borg/testsuite/crypto.py index 035931b188..10cb7446da 100644 --- a/src/borg/testsuite/crypto.py +++ b/src/borg/testsuite/crypto.py @@ -369,7 +369,7 @@ def to_dict(key): @unittest.mock.patch('getpass.getpass') -def test_passphrase_retry(getpass, monkeypatch): +def test_repo_key_detect_does_not_raise_integrity_error(getpass, monkeypatch): """https://github.com/borgbackup/borg/pull/6469#discussion_r832670411""" repository = MagicMock(id=b'repository_id') getpass.return_value = "hello, pass phrase" @@ -377,7 +377,5 @@ def test_passphrase_retry(getpass, monkeypatch): RepoKey.create(repository, args=MagicMock(key_algorithm='argon2 aes256-ctr hmac-sha256')) repository.load_key.return_value = repository.save_key.call_args.args[0] - # BUG: raises borg.crypto.low_level.IntegrityError - import borg.crypto.low_level - with pytest.raises(borg.crypto.low_level.IntegrityError): - RepoKey.detect(repository, manifest_data=MagicMock()) + # This used to raise integrity errors due to a bug + RepoKey.detect(repository, manifest_data=None) From 40d64728330c3cbb6cc7b444334d6fbb41a447bb Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Mon, 28 Mar 2022 10:23:57 +0300 Subject: [PATCH 09/28] Fix AttributeError: 'MockArgs' object has no attribute 'key_algorithm' --- src/borg/testsuite/key.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/borg/testsuite/key.py b/src/borg/testsuite/key.py index 14297d480b..08205a6b59 100644 --- a/src/borg/testsuite/key.py +++ b/src/borg/testsuite/key.py @@ -25,6 +25,7 @@ class TestKey: class MockArgs: location = Location(tempfile.mkstemp()[1]) + key_algorithm = "argon2 aes256-ctr hmac-sha256" keyfile2_key_file = """ BORG_KEY 0000000000000000000000000000000000000000000000000000000000000000 From 6f6e0032e83504381243b0350aef55c63742fdd6 Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Mon, 28 Mar 2022 10:36:09 +0300 Subject: [PATCH 10/28] key change-passphrase: keep key algorithm the same --- src/borg/crypto/key.py | 4 ++-- src/borg/testsuite/archiver.py | 10 ++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/borg/crypto/key.py b/src/borg/crypto/key.py index 1cb9a4f607..72e566c4c1 100644 --- a/src/borg/crypto/key.py +++ b/src/borg/crypto/key.py @@ -439,6 +439,7 @@ def decrypt_key_file(self, data, passphrase): if encrypted_key.version != 1: raise UnsupportedKeyFormatError() else: + self._encrypted_key_algorithm = encrypted_key.algorithm if encrypted_key.algorithm == 'sha256': return self.decrypt_key_file_pbkdf2(encrypted_key, passphrase) elif encrypted_key.algorithm == 'argon2 aes256-ctr hmac-sha256': @@ -548,8 +549,7 @@ def _save(self, passphrase, algorithm): def change_passphrase(self, passphrase=None): if passphrase is None: passphrase = Passphrase.new(allow_empty=True) - algorithm = 'sha256' # TODO: copy from `self` - self.save(self.target, passphrase, algorithm=algorithm) + self.save(self.target, passphrase, algorithm=self._encrypted_key_algorithm) @classmethod def create(cls, repository, args): diff --git a/src/borg/testsuite/archiver.py b/src/borg/testsuite/archiver.py index cb1ec2dd55..a4def2ad8f 100644 --- a/src/borg/testsuite/archiver.py +++ b/src/borg/testsuite/archiver.py @@ -3617,6 +3617,16 @@ def test_init_with_explicit_key_algorithm(self): key = msgpack.unpackb(a2b_base64(repository.load_key())) assert key[b'algorithm'] == b'sha256' + def test_change_passphrase_does_not_change_algorithm(self): + self.cmd('init', '--encryption=repokey', '--key-algorithm', 'argon2 aes256-ctr hmac-sha256', self.repository_location) + os.environ['BORG_NEW_PASSPHRASE'] = 'newpassphrase' + + self.cmd('key', 'change-passphrase', self.repository_location) + + with Repository(self.repository_path) as repository: + key = msgpack.unpackb(a2b_base64(repository.load_key())) + assert key[b'algorithm'] == b'argon2 aes256-ctr hmac-sha256' + @unittest.skipUnless('binary' in BORG_EXES, 'no borg.exe available') class ArchiverTestCaseBinary(ArchiverTestCase): From 3c698230ec51fa2dff111f0bb598ea0d5e4a73b7 Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Mon, 28 Mar 2022 10:47:41 +0300 Subject: [PATCH 11/28] key change-location: keep key algorithm the same --- src/borg/archiver.py | 4 ++-- src/borg/testsuite/archiver.py | 9 +++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/borg/archiver.py b/src/borg/archiver.py index a10e8b1a89..36c3923a59 100644 --- a/src/borg/archiver.py +++ b/src/borg/archiver.py @@ -398,8 +398,8 @@ def do_change_location(self, args, repository, manifest, key, cache): setattr(key_new, name, value) key_new.target = key_new.get_new_target(args) - algorithm = 'sha256' # TODO: copy from `key` - key_new.save(key_new.target, key._passphrase, create=True, algorithm=algorithm) # save with same passphrase + # save with same passphrase and algorithm + key_new.save(key_new.target, key._passphrase, create=True, algorithm=key._encrypted_key_algorithm) # rewrite the manifest with the new key, so that the key-type byte of the manifest changes manifest.key = key_new diff --git a/src/borg/testsuite/archiver.py b/src/borg/testsuite/archiver.py index a4def2ad8f..b3609d242c 100644 --- a/src/borg/testsuite/archiver.py +++ b/src/borg/testsuite/archiver.py @@ -3627,6 +3627,15 @@ def test_change_passphrase_does_not_change_algorithm(self): key = msgpack.unpackb(a2b_base64(repository.load_key())) assert key[b'algorithm'] == b'argon2 aes256-ctr hmac-sha256' + def test_change_location_does_not_change_algorithm(self): + self.cmd('init', '--encryption=keyfile', '--key-algorithm', 'argon2 aes256-ctr hmac-sha256', self.repository_location) + + self.cmd('key', 'change-location', self.repository_location, 'repokey') + + with Repository(self.repository_path) as repository: + key = msgpack.unpackb(a2b_base64(repository.load_key())) + assert key[b'algorithm'] == b'argon2 aes256-ctr hmac-sha256' + @unittest.skipUnless('binary' in BORG_EXES, 'no borg.exe available') class ArchiverTestCaseBinary(ArchiverTestCase): From 02eb0f90a56a9ba3177c6d939a6d0af8cc317c83 Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Mon, 28 Mar 2022 11:03:47 +0300 Subject: [PATCH 12/28] simplify a test --- src/borg/testsuite/crypto.py | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/src/borg/testsuite/crypto.py b/src/borg/testsuite/crypto.py index 10cb7446da..267f98dde1 100644 --- a/src/borg/testsuite/crypto.py +++ b/src/borg/testsuite/crypto.py @@ -342,30 +342,22 @@ def test_decrypt_key_file_v2_is_unsupported(monkeypatch): key.decrypt_key_file(encrypted, passphrase) -def test_key_file_save_argon2_aes256_ctr_hmac_sha256(): +@pytest.mark.parametrize('key_algorithm', ('argon2 aes256-ctr hmac-sha256', 'sha256')) +def test_key_file_roundtrip(monkeypatch, key_algorithm): def to_dict(key): extract = 'repository_id', 'enc_key', 'enc_hmac_key', 'id_key', 'chunk_seed' return {a: getattr(key, a) for a in extract} - repository = MagicMock() - repository.id = b'repository_id' - save_me = RepoKey(repository) - save_me.repository_id = b'repository_id' - save_me.enc_key = b'enc_key' - save_me.enc_hmac_key = b'enc_hmac_key' - save_me.id_key = b'id_key' - save_me.chunk_seed = 42 - passphrase = MagicMock() - passphrase.argon2.return_value = b'derived key'.rjust(64) - - save_me.save(algorithm='argon2 aes256-ctr hmac-sha256', target=repository, passphrase=passphrase) + repository = MagicMock(id=b'repository_id') + monkeypatch.setenv('BORG_PASSPHRASE', "hello, pass phrase") + + save_me = RepoKey.create(repository, args=MagicMock(key_algorithm=key_algorithm)) saved = repository.save_key.call_args.args[0] repository.load_key.return_value = saved - load_me = RepoKey(repository) - load_me.load(target=repository, passphrase=passphrase) + load_me = RepoKey.detect(repository, manifest_data=None) assert to_dict(load_me) == to_dict(save_me) - assert msgpack.unpackb(a2b_base64(saved))[b'algorithm'] == b'argon2 aes256-ctr hmac-sha256' + assert msgpack.unpackb(a2b_base64(saved))[b'algorithm'] == key_algorithm.encode() @unittest.mock.patch('getpass.getpass') @@ -377,5 +369,6 @@ def test_repo_key_detect_does_not_raise_integrity_error(getpass, monkeypatch): RepoKey.create(repository, args=MagicMock(key_algorithm='argon2 aes256-ctr hmac-sha256')) repository.load_key.return_value = repository.save_key.call_args.args[0] - # This used to raise integrity errors due to a bug + # 1. detect() tries an empty passphrase first before prompting the user + # 2. load() was thorwing integrity errors instead of returning None due to a bug RepoKey.detect(repository, manifest_data=None) From 699d063b71159ab420411970ec745f291339c91a Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Wed, 30 Mar 2022 15:05:50 +0300 Subject: [PATCH 13/28] typo --- src/borg/testsuite/crypto.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/borg/testsuite/crypto.py b/src/borg/testsuite/crypto.py index 267f98dde1..809d5c4d94 100644 --- a/src/borg/testsuite/crypto.py +++ b/src/borg/testsuite/crypto.py @@ -370,5 +370,5 @@ def test_repo_key_detect_does_not_raise_integrity_error(getpass, monkeypatch): repository.load_key.return_value = repository.save_key.call_args.args[0] # 1. detect() tries an empty passphrase first before prompting the user - # 2. load() was thorwing integrity errors instead of returning None due to a bug + # 2. load() was throwing integrity errors instead of returning None due to a bug RepoKey.detect(repository, manifest_data=None) From bf7c1966ae4e0e8213c1298162397aa13880a76a Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Wed, 30 Mar 2022 15:15:43 +0300 Subject: [PATCH 14/28] triple-doublequotes --- src/borg/testsuite/crypto.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/borg/testsuite/crypto.py b/src/borg/testsuite/crypto.py index 809d5c4d94..2034caef40 100644 --- a/src/borg/testsuite/crypto.py +++ b/src/borg/testsuite/crypto.py @@ -316,7 +316,7 @@ def test_decrypt_key_file_pbkdf2_sha256_aes256_ctr_hmac_sha256(monkeypatch): def test_decrypt_key_file_unsupported_algorithm(monkeypatch): - '''We will add more algorithms in the future. We should raise a helpful error.''' + """We will add more algorithms in the future. We should raise a helpful error.""" monkeypatch.setenv('BORG_PASSPHRASE', "hello, pass phrase") passphrase = Passphrase.new() key = KeyfileKey(None) @@ -330,7 +330,7 @@ def test_decrypt_key_file_unsupported_algorithm(monkeypatch): def test_decrypt_key_file_v2_is_unsupported(monkeypatch): - '''There may eventually be a version 2 of the format. For now we should raise a helpful error.''' + """There may eventually be a version 2 of the format. For now we should raise a helpful error.""" monkeypatch.setenv('BORG_PASSPHRASE', "hello, pass phrase") passphrase = Passphrase.new() key = KeyfileKey(None) From a23843f414c83dc13ec2d7010fa1ccf6800fc304 Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Wed, 30 Mar 2022 15:33:08 +0300 Subject: [PATCH 15/28] Extract ARGON2_ARGS Now we can use the same values in `borg cpu benchmark` --- src/borg/constants.py | 4 ++++ src/borg/crypto/key.py | 18 +++--------------- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/borg/constants.py b/src/borg/constants.py index 5fb6b9a65a..1e80ae4f8d 100644 --- a/src/borg/constants.py +++ b/src/borg/constants.py @@ -103,6 +103,10 @@ PBKDF2_ITERATIONS = 100000 +# https://www.rfc-editor.org/rfc/rfc9106.html#section-4-6.2 +ARGON2_ARGS = {'time_cost': 3, 'memory_cost': 2**16, 'parallelism': 4, 'type': 'id'} +ARGON2_SALT_BYTES = 16 + class KeyBlobStorage: NO_STORAGE = 'no_storage' diff --git a/src/borg/crypto/key.py b/src/borg/crypto/key.py index 72e566c4c1..1da8b49929 100644 --- a/src/borg/crypto/key.py +++ b/src/borg/crypto/key.py @@ -499,20 +499,11 @@ def encrypt_key_file_pbkdf2(self, data, passphrase): return msgpack.packb(enc_key.as_dict()) def encrypt_key_file_argon2(self, data, passphrase): - # https://www.rfc-editor.org/rfc/rfc9106.html#section-4-6.2 - time_cost = 3 - memory_cost = 2**16 - parallelism = 4 - type_ = 'id' - - salt = os.urandom(16) + salt = os.urandom(ARGON2_SALT_BYTES) key = passphrase.argon2( output_len_in_bytes=64, salt=salt, - time_cost=time_cost, - memory_cost=memory_cost, - parallelism=parallelism, - type=type_, + **ARGON2_ARGS, ) enc_key, mac_key = key[:32], key[32:] ae_cipher = AES256_CTR_HMAC_SHA256( @@ -524,11 +515,8 @@ def encrypt_key_file_argon2(self, data, passphrase): version=1, algorithm='argon2 aes256-ctr hmac-sha256', salt=salt, - argon2_time_cost=time_cost, - argon2_memory_cost=memory_cost, - argon2_parallelism=parallelism, - argon2_type=type_, data=ae_cipher.encrypt(data), + **{'argon2_' + k: v for k, v in ARGON2_ARGS.items()}, ) return msgpack.packb(encrypted_key.as_dict()) From 58c6de25966efc413fb8644d89123f478669f6df Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Wed, 30 Mar 2022 15:58:40 +0300 Subject: [PATCH 16/28] --key-algorithm: use shorter names Long names with spaces are inconvinient to type in CLI --- src/borg/archiver.py | 3 +-- src/borg/constants.py | 8 ++++++++ src/borg/crypto/key.py | 3 +-- src/borg/testsuite/archiver.py | 6 +++--- src/borg/testsuite/crypto.py | 13 ++++++++----- 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/borg/archiver.py b/src/borg/archiver.py index 36c3923a59..519a918025 100644 --- a/src/borg/archiver.py +++ b/src/borg/archiver.py @@ -4311,8 +4311,7 @@ def define_borg_mount(parser): help='Set storage quota of the new repository (e.g. 5G, 1.5T). Default: no quota.') subparser.add_argument('--make-parent-dirs', dest='make_parent_dirs', action='store_true', help='create the parent directories of the repository directory, if they are missing.') - subparser.add_argument('--key-algorithm', dest='key_algorithm', default='argon2 aes256-ctr hmac-sha256', - choices=['pbkdf2-sha256 aes256-ctr hmac-sha256', 'argon2 aes256-ctr hmac-sha256']) + subparser.add_argument('--key-algorithm', dest='key_algorithm', default='argon2', choices=list(KEY_ALGORITHMS)) # borg key subparser = subparsers.add_parser('key', parents=[mid_common_parser], add_help=False, diff --git a/src/borg/constants.py b/src/borg/constants.py index 1e80ae4f8d..4c0f438706 100644 --- a/src/borg/constants.py +++ b/src/borg/constants.py @@ -107,6 +107,14 @@ ARGON2_ARGS = {'time_cost': 3, 'memory_cost': 2**16, 'parallelism': 4, 'type': 'id'} ARGON2_SALT_BYTES = 16 +# Maps the CLI argument to our internal identifier for the format +KEY_ALGORITHMS = { + # encrypt-and-MAC, kdf: PBKDF2(HMAC−SHA256), encryption: AES256-CTR, authentication: HMAC-SHA256 + 'pbkdf2': 'sha256', + # encrypt-then-MAC, kdf: argon2, encryption: AES256-CTR, authentication: HMAC-SHA256 + 'argon2': 'argon2 aes256-ctr hmac-sha256', +} + class KeyBlobStorage: NO_STORAGE = 'no_storage' diff --git a/src/borg/crypto/key.py b/src/borg/crypto/key.py index 1da8b49929..0430ae9aa0 100644 --- a/src/borg/crypto/key.py +++ b/src/borg/crypto/key.py @@ -547,8 +547,7 @@ def create(cls, repository, args): key.init_from_random_data() key.init_ciphers() target = key.get_new_target(args) - algorithm = 'sha256' if args.key_algorithm == 'pbkdf2-sha256 aes256-ctr hmac-sha256' else args.key_algorithm - key.save(target, passphrase, create=True, algorithm=algorithm) + key.save(target, passphrase, create=True, algorithm=KEY_ALGORITHMS[args.key_algorithm]) logger.info('Key in "%s" created.' % target) logger.info('Keep this key safe. Your data will be inaccessible without it.') return key diff --git a/src/borg/testsuite/archiver.py b/src/borg/testsuite/archiver.py index b3609d242c..96ad1b2123 100644 --- a/src/borg/testsuite/archiver.py +++ b/src/borg/testsuite/archiver.py @@ -3612,13 +3612,13 @@ def test_init_defaults_to_argon2(self): def test_init_with_explicit_key_algorithm(self): """https://github.com/borgbackup/borg/issues/747#issuecomment-1076160401""" - self.cmd('init', '--encryption=repokey', '--key-algorithm', 'pbkdf2-sha256 aes256-ctr hmac-sha256', self.repository_location) + self.cmd('init', '--encryption=repokey', '--key-algorithm', 'pbkdf2', self.repository_location) with Repository(self.repository_path) as repository: key = msgpack.unpackb(a2b_base64(repository.load_key())) assert key[b'algorithm'] == b'sha256' def test_change_passphrase_does_not_change_algorithm(self): - self.cmd('init', '--encryption=repokey', '--key-algorithm', 'argon2 aes256-ctr hmac-sha256', self.repository_location) + self.cmd('init', '--encryption=repokey', '--key-algorithm', 'argon2', self.repository_location) os.environ['BORG_NEW_PASSPHRASE'] = 'newpassphrase' self.cmd('key', 'change-passphrase', self.repository_location) @@ -3628,7 +3628,7 @@ def test_change_passphrase_does_not_change_algorithm(self): assert key[b'algorithm'] == b'argon2 aes256-ctr hmac-sha256' def test_change_location_does_not_change_algorithm(self): - self.cmd('init', '--encryption=keyfile', '--key-algorithm', 'argon2 aes256-ctr hmac-sha256', self.repository_location) + self.cmd('init', '--encryption=keyfile', '--key-algorithm', 'argon2', self.repository_location) self.cmd('key', 'change-location', self.repository_location, 'repokey') diff --git a/src/borg/testsuite/crypto.py b/src/borg/testsuite/crypto.py index 2034caef40..7f804c1447 100644 --- a/src/borg/testsuite/crypto.py +++ b/src/borg/testsuite/crypto.py @@ -342,8 +342,11 @@ def test_decrypt_key_file_v2_is_unsupported(monkeypatch): key.decrypt_key_file(encrypted, passphrase) -@pytest.mark.parametrize('key_algorithm', ('argon2 aes256-ctr hmac-sha256', 'sha256')) -def test_key_file_roundtrip(monkeypatch, key_algorithm): +@pytest.mark.parametrize('CLI_argument, expect_algorithm', ( + ('argon2', b'argon2 aes256-ctr hmac-sha256'), + ('pbkdf2', b'sha256'), +)) +def test_key_file_roundtrip(monkeypatch, CLI_argument, expect_algorithm): def to_dict(key): extract = 'repository_id', 'enc_key', 'enc_hmac_key', 'id_key', 'chunk_seed' return {a: getattr(key, a) for a in extract} @@ -351,13 +354,13 @@ def to_dict(key): repository = MagicMock(id=b'repository_id') monkeypatch.setenv('BORG_PASSPHRASE', "hello, pass phrase") - save_me = RepoKey.create(repository, args=MagicMock(key_algorithm=key_algorithm)) + save_me = RepoKey.create(repository, args=MagicMock(key_algorithm=CLI_argument)) saved = repository.save_key.call_args.args[0] repository.load_key.return_value = saved load_me = RepoKey.detect(repository, manifest_data=None) assert to_dict(load_me) == to_dict(save_me) - assert msgpack.unpackb(a2b_base64(saved))[b'algorithm'] == key_algorithm.encode() + assert msgpack.unpackb(a2b_base64(saved))[b'algorithm'] == expect_algorithm @unittest.mock.patch('getpass.getpass') @@ -366,7 +369,7 @@ def test_repo_key_detect_does_not_raise_integrity_error(getpass, monkeypatch): repository = MagicMock(id=b'repository_id') getpass.return_value = "hello, pass phrase" monkeypatch.setenv('BORG_DISPLAY_PASSPHRASE', 'no') - RepoKey.create(repository, args=MagicMock(key_algorithm='argon2 aes256-ctr hmac-sha256')) + RepoKey.create(repository, args=MagicMock(key_algorithm='argon2')) repository.load_key.return_value = repository.save_key.call_args.args[0] # 1. detect() tries an empty passphrase first before prompting the user From 28ae76d59084a54a74442236339f3ec93cb82090 Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Thu, 31 Mar 2022 08:51:31 +0300 Subject: [PATCH 17/28] Fix the testsuite/key.py again --- src/borg/testsuite/key.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/borg/testsuite/key.py b/src/borg/testsuite/key.py index 08205a6b59..8328b1e803 100644 --- a/src/borg/testsuite/key.py +++ b/src/borg/testsuite/key.py @@ -25,7 +25,7 @@ class TestKey: class MockArgs: location = Location(tempfile.mkstemp()[1]) - key_algorithm = "argon2 aes256-ctr hmac-sha256" + key_algorithm = "argon2" keyfile2_key_file = """ BORG_KEY 0000000000000000000000000000000000000000000000000000000000000000 From 01cd10ce691157161a6d7fe6ea7e08de267efb72 Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Thu, 31 Mar 2022 08:55:21 +0300 Subject: [PATCH 18/28] Change names per review comment https://github.com/borgbackup/borg/pull/6469#discussion_r838562804 https://github.com/borgbackup/borg/pull/6469#discussion_r838563666 --- src/borg/testsuite/crypto.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/borg/testsuite/crypto.py b/src/borg/testsuite/crypto.py index 7f804c1447..4de1e7eb00 100644 --- a/src/borg/testsuite/crypto.py +++ b/src/borg/testsuite/crypto.py @@ -342,11 +342,11 @@ def test_decrypt_key_file_v2_is_unsupported(monkeypatch): key.decrypt_key_file(encrypted, passphrase) -@pytest.mark.parametrize('CLI_argument, expect_algorithm', ( +@pytest.mark.parametrize('cli_argument, expected_algorithm', ( ('argon2', b'argon2 aes256-ctr hmac-sha256'), ('pbkdf2', b'sha256'), )) -def test_key_file_roundtrip(monkeypatch, CLI_argument, expect_algorithm): +def test_key_file_roundtrip(monkeypatch, cli_argument, expected_algorithm): def to_dict(key): extract = 'repository_id', 'enc_key', 'enc_hmac_key', 'id_key', 'chunk_seed' return {a: getattr(key, a) for a in extract} @@ -354,13 +354,13 @@ def to_dict(key): repository = MagicMock(id=b'repository_id') monkeypatch.setenv('BORG_PASSPHRASE', "hello, pass phrase") - save_me = RepoKey.create(repository, args=MagicMock(key_algorithm=CLI_argument)) + save_me = RepoKey.create(repository, args=MagicMock(key_algorithm=cli_argument)) saved = repository.save_key.call_args.args[0] repository.load_key.return_value = saved load_me = RepoKey.detect(repository, manifest_data=None) assert to_dict(load_me) == to_dict(save_me) - assert msgpack.unpackb(a2b_base64(saved))[b'algorithm'] == expect_algorithm + assert msgpack.unpackb(a2b_base64(saved))[b'algorithm'] == expected_algorithm @unittest.mock.patch('getpass.getpass') From 1769290734cbbf519dc7c640f29e2cdcb187a1f6 Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Thu, 31 Mar 2022 09:02:15 +0300 Subject: [PATCH 19/28] Use KEY_ALGORITHMS.items() --- src/borg/testsuite/crypto.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/borg/testsuite/crypto.py b/src/borg/testsuite/crypto.py index 4de1e7eb00..5633852f93 100644 --- a/src/borg/testsuite/crypto.py +++ b/src/borg/testsuite/crypto.py @@ -13,6 +13,7 @@ from ..crypto.key import KeyfileKey, UnsupportedKeyFormatError, RepoKey from ..helpers.passphrase import Passphrase from ..helpers import msgpack +from ..constants import KEY_ALGORITHMS from . import BaseTestCase @@ -342,10 +343,7 @@ def test_decrypt_key_file_v2_is_unsupported(monkeypatch): key.decrypt_key_file(encrypted, passphrase) -@pytest.mark.parametrize('cli_argument, expected_algorithm', ( - ('argon2', b'argon2 aes256-ctr hmac-sha256'), - ('pbkdf2', b'sha256'), -)) +@pytest.mark.parametrize('cli_argument, expected_algorithm', KEY_ALGORITHMS.items()) def test_key_file_roundtrip(monkeypatch, cli_argument, expected_algorithm): def to_dict(key): extract = 'repository_id', 'enc_key', 'enc_hmac_key', 'id_key', 'chunk_seed' @@ -360,7 +358,7 @@ def to_dict(key): load_me = RepoKey.detect(repository, manifest_data=None) assert to_dict(load_me) == to_dict(save_me) - assert msgpack.unpackb(a2b_base64(saved))[b'algorithm'] == expected_algorithm + assert msgpack.unpackb(a2b_base64(saved))[b'algorithm'] == expected_algorithm.encode() @unittest.mock.patch('getpass.getpass') From db93243bd7ce64bca51edeccde16fabcd60af8cf Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Sun, 3 Apr 2022 18:11:05 +0300 Subject: [PATCH 20/28] Speed up tests: skip key derivation --- conftest.py | 2 ++ src/borg/helpers/passphrase.py | 4 ++++ src/borg/testsuite/crypto.py | 2 ++ src/borg/testsuite/key.py | 4 ++++ 4 files changed, 12 insertions(+) diff --git a/conftest.py b/conftest.py index 3a7c278af2..49e26dd323 100644 --- a/conftest.py +++ b/conftest.py @@ -36,6 +36,8 @@ def clean_env(tmpdir_factory, monkeypatch): if key.startswith('BORG_') and key not in ('BORG_FUSE_IMPL', )] for key in keys: monkeypatch.delenv(key, raising=False) + # Speed up tests: skip key derivation + monkeypatch.setenv("BORG_TESTONLY_MOCK_KDF", "1") def pytest_report_header(config, startdir): diff --git a/src/borg/helpers/passphrase.py b/src/borg/helpers/passphrase.py index 5a0acd12d1..fad57918d2 100644 --- a/src/borg/helpers/passphrase.py +++ b/src/borg/helpers/passphrase.py @@ -141,6 +141,8 @@ def __repr__(self): return '' def kdf(self, salt, iterations, length): + if os.environ.get("BORG_TESTONLY_MOCK_KDF") is not None: + return b'X' * length return pbkdf2_hmac('sha256', self.encode('utf-8'), salt, iterations, length) def argon2( @@ -152,6 +154,8 @@ def argon2( parallelism, type: Literal['i', 'd', 'id'] ) -> bytes: + if os.environ.get("BORG_TESTONLY_MOCK_KDF") is not None: + return b'X' * output_len_in_bytes type_map = { 'i': argon2.low_level.Type.I, 'd': argon2.low_level.Type.D, diff --git a/src/borg/testsuite/crypto.py b/src/borg/testsuite/crypto.py index 5633852f93..a6ba97cfa3 100644 --- a/src/borg/testsuite/crypto.py +++ b/src/borg/testsuite/crypto.py @@ -261,6 +261,7 @@ def test_hkdf_hmac_sha512_5(self): def test_decrypt_key_file_argon2_aes256_ctr_hmac_sha256(monkeypatch): + monkeypatch.delenv('BORG_TESTONLY_MOCK_KDF') plain = b'hello' # echo -n "hello, pass phrase" | argon2 saltsaltsaltsalt -id -t 3 -m 16 -p 4 -l 64 -r enc_key = bytes.fromhex('3dd855b778ba292eda7bf708a9ea111ee99c5c45d2e9a2773d126de46d344410') @@ -293,6 +294,7 @@ def test_decrypt_key_file_argon2_aes256_ctr_hmac_sha256(monkeypatch): def test_decrypt_key_file_pbkdf2_sha256_aes256_ctr_hmac_sha256(monkeypatch): + monkeypatch.delenv('BORG_TESTONLY_MOCK_KDF') plain = b'hello' salt = b'salt'*4 iterations = 100000 diff --git a/src/borg/testsuite/key.py b/src/borg/testsuite/key.py index 8328b1e803..3e6a7429fa 100644 --- a/src/borg/testsuite/key.py +++ b/src/borg/testsuite/key.py @@ -170,6 +170,7 @@ def test_keyfile_kfenv(self, tmpdir, monkeypatch): KeyfileKey.detect(self.MockRepository(), chunk_cdata) def test_keyfile2(self, monkeypatch, keys_dir): + monkeypatch.delenv('BORG_TESTONLY_MOCK_KDF') with keys_dir.join('keyfile').open('w') as fd: fd.write(self.keyfile2_key_file) monkeypatch.setenv('BORG_PASSPHRASE', 'passphrase') @@ -177,6 +178,7 @@ def test_keyfile2(self, monkeypatch, keys_dir): assert key.decrypt(self.keyfile2_id, self.keyfile2_cdata) == b'payload' def test_keyfile2_kfenv(self, tmpdir, monkeypatch): + monkeypatch.delenv('BORG_TESTONLY_MOCK_KDF') keyfile = tmpdir.join('keyfile') with keyfile.open('w') as fd: fd.write(self.keyfile2_key_file) @@ -186,6 +188,7 @@ def test_keyfile2_kfenv(self, tmpdir, monkeypatch): assert key.decrypt(self.keyfile2_id, self.keyfile2_cdata) == b'payload' def test_keyfile_blake2(self, monkeypatch, keys_dir): + monkeypatch.delenv('BORG_TESTONLY_MOCK_KDF') with keys_dir.join('keyfile').open('w') as fd: fd.write(self.keyfile_blake2_key_file) monkeypatch.setenv('BORG_PASSPHRASE', 'passphrase') @@ -201,6 +204,7 @@ def _corrupt_byte(self, key, data, offset): key.decrypt(b'', data) def test_decrypt_integrity(self, monkeypatch, keys_dir): + monkeypatch.delenv('BORG_TESTONLY_MOCK_KDF') with keys_dir.join('keyfile').open('w') as fd: fd.write(self.keyfile2_key_file) monkeypatch.setenv('BORG_PASSPHRASE', 'passphrase') From 40c5f4d6783f953be6292f97b68204e9c3447ef0 Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Mon, 4 Apr 2022 17:07:25 +0300 Subject: [PATCH 21/28] Remove redundant PBKDF2_ITERATIONS monkeypatch --- conftest.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/conftest.py b/conftest.py index 49e26dd323..7e493ee087 100644 --- a/conftest.py +++ b/conftest.py @@ -6,9 +6,6 @@ # for `from borg.constants import PBKDF2_ITERATIONS` (or star import) usages before # this is executed from borg import constants -# no fixture-based monkey-patching since star-imports are used for the constants module -constants.PBKDF2_ITERATIONS = 1 - # needed to get pretty assertion failures in unit tests: if hasattr(pytest, 'register_assert_rewrite'): From 24010a6623fccaf8932400bfe640b8db8cc15c66 Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Tue, 5 Apr 2022 06:43:18 +0300 Subject: [PATCH 22/28] Tests: weaken the KDF instead of skipping it --- conftest.py | 9 ++------ src/borg/helpers/passphrase.py | 11 ++++++---- src/borg/testsuite/crypto.py | 16 +++++--------- src/borg/testsuite/key.py | 40 +++++++++++++++------------------- 4 files changed, 33 insertions(+), 43 deletions(-) diff --git a/conftest.py b/conftest.py index 7e493ee087..6166b4759d 100644 --- a/conftest.py +++ b/conftest.py @@ -2,11 +2,6 @@ import pytest -# IMPORTANT keep this above all other borg imports to avoid inconsistent values -# for `from borg.constants import PBKDF2_ITERATIONS` (or star import) usages before -# this is executed -from borg import constants - # needed to get pretty assertion failures in unit tests: if hasattr(pytest, 'register_assert_rewrite'): pytest.register_assert_rewrite('borg.testsuite') @@ -33,8 +28,8 @@ def clean_env(tmpdir_factory, monkeypatch): if key.startswith('BORG_') and key not in ('BORG_FUSE_IMPL', )] for key in keys: monkeypatch.delenv(key, raising=False) - # Speed up tests: skip key derivation - monkeypatch.setenv("BORG_TESTONLY_MOCK_KDF", "1") + # Speed up tests + monkeypatch.setenv("BORG_TESTONLY_WEAKEN_KDF", "1") def pytest_report_header(config, startdir): diff --git a/src/borg/helpers/passphrase.py b/src/borg/helpers/passphrase.py index fad57918d2..26382e2b93 100644 --- a/src/borg/helpers/passphrase.py +++ b/src/borg/helpers/passphrase.py @@ -141,8 +141,8 @@ def __repr__(self): return '' def kdf(self, salt, iterations, length): - if os.environ.get("BORG_TESTONLY_MOCK_KDF") is not None: - return b'X' * length + if os.environ.get("BORG_TESTONLY_WEAKEN_KDF") is not None: + iterations = 1 return pbkdf2_hmac('sha256', self.encode('utf-8'), salt, iterations, length) def argon2( @@ -154,8 +154,11 @@ def argon2( parallelism, type: Literal['i', 'd', 'id'] ) -> bytes: - if os.environ.get("BORG_TESTONLY_MOCK_KDF") is not None: - return b'X' * output_len_in_bytes + if os.environ.get("BORG_TESTONLY_WEAKEN_KDF") is not None: + time_cost = 1 + parallelism = 1 + # 8 is the smallest value that aviods the "Memory cost is too small" exception + memory_cost = 8 type_map = { 'i': argon2.low_level.Type.I, 'd': argon2.low_level.Type.D, diff --git a/src/borg/testsuite/crypto.py b/src/borg/testsuite/crypto.py index a6ba97cfa3..11d81c3740 100644 --- a/src/borg/testsuite/crypto.py +++ b/src/borg/testsuite/crypto.py @@ -261,15 +261,13 @@ def test_hkdf_hmac_sha512_5(self): def test_decrypt_key_file_argon2_aes256_ctr_hmac_sha256(monkeypatch): - monkeypatch.delenv('BORG_TESTONLY_MOCK_KDF') plain = b'hello' - # echo -n "hello, pass phrase" | argon2 saltsaltsaltsalt -id -t 3 -m 16 -p 4 -l 64 -r - enc_key = bytes.fromhex('3dd855b778ba292eda7bf708a9ea111ee99c5c45d2e9a2773d126de46d344410') - mac_key = bytes.fromhex('0b0b65fdccaea7cf5b9a6214cd867983e2326abeccedf1dceb1feee0ae74075b') + # echo -n "hello, pass phrase" | argon2 saltsaltsaltsalt -id -t 1 -k 8 -p 1 -l 64 -r + key = bytes.fromhex('d07cc7f9cfb483303e0b9fec176b2a9c559bb70c3a9fb0d5f9c0c23527cd09570212449f09f8cd28c1a41b73fa0098e889c3f2642e87c392e51f95d70d248d9d') ae_cipher = AES256_CTR_HMAC_SHA256( iv=0, header_len=0, aad_offset=0, - enc_key=enc_key, - mac_key=mac_key, + enc_key=key[:32], + mac_key=key[32:], ) envelope = ae_cipher.encrypt(plain) @@ -294,19 +292,17 @@ def test_decrypt_key_file_argon2_aes256_ctr_hmac_sha256(monkeypatch): def test_decrypt_key_file_pbkdf2_sha256_aes256_ctr_hmac_sha256(monkeypatch): - monkeypatch.delenv('BORG_TESTONLY_MOCK_KDF') plain = b'hello' salt = b'salt'*4 - iterations = 100000 monkeypatch.setenv('BORG_PASSPHRASE', "hello, pass phrase") passphrase = Passphrase.new() - key = passphrase.kdf(salt, iterations, 32) + key = passphrase.kdf(salt, iterations=1, length=32) hash = hmac_sha256(key, plain) data = AES(key, b'\0'*16).encrypt(plain) encrypted = msgpack.packb({ 'version': 1, 'algorithm': 'sha256', - 'iterations': iterations, + 'iterations': 1, 'salt': salt, 'data': data, 'hash': hash, diff --git a/src/borg/testsuite/key.py b/src/borg/testsuite/key.py index 3e6a7429fa..4c84756a61 100644 --- a/src/borg/testsuite/key.py +++ b/src/borg/testsuite/key.py @@ -29,13 +29,13 @@ class MockArgs: keyfile2_key_file = """ BORG_KEY 0000000000000000000000000000000000000000000000000000000000000000 - hqppdGVyYXRpb25zzgABhqCkaGFzaNoAIMyonNI+7Cjv0qHi0AOBM6bLGxACJhfgzVD2oq - bIS9SFqWFsZ29yaXRobaZzaGEyNTakc2FsdNoAINNK5qqJc1JWSUjACwFEWGTdM7Nd0a5l - 1uBGPEb+9XM9p3ZlcnNpb24BpGRhdGHaANAYDT5yfPpU099oBJwMomsxouKyx/OG4QIXK2 - hQCG2L2L/9PUu4WIuKvGrsXoP7syemujNfcZws5jLp2UPva4PkQhQsrF1RYDEMLh2eF9Ol - rwtkThq1tnh7KjWMG9Ijt7/aoQtq0zDYP/xaFF8XXSJxiyP5zjH5+spB6RL0oQHvbsliSh - /cXJq7jrqmrJ1phd6dg4SHAM/i+hubadZoS6m25OQzYAW09wZD/phG8OVa698Z5ed3HTaT - SmrtgJL3EoOKgUI9d6BLE4dJdBqntifo""".strip() + hqlhbGdvcml0aG2mc2hhMjU2pGRhdGHaAN4u2SiN7hqISe3OA8raBWNuvHn1R50ZU7HVCn + 11vTJNEaj9soxUaIGcW+pAB2N5yYoKMg/sGCMuZa286iJ008DvN99rf/ORfcKrK2GmzslO + N3uv9Tk9HtqV/Sq5zgM9xuY9rEeQGDQVQ+AOsFamJqSUrAemGJbJqw9IerXC/jN4XPnX6J + pi1cXCFxHfDaEhmWrkdPNoZdirCv/eP/dOVOLmwU58YsS+MvkZNfEa16el/fSb/ENdrwJ/ + 2aYMQrDdk1d5MYzkjotv/KpofNwPXZchu2EwH7OIHWQjEVL1DZWkaGFzaNoAIO/7qn1hr3 + F84MsMMiqpbz4KVICeBZhfAaTPs4W7BC63qml0ZXJhdGlvbnPOAAGGoKRzYWx02gAgLENQ + 2uVCoR7EnAoiRzn8J+orbojKtJlNCnQ31SSC8rendmVyc2lvbgE=""".strip() keyfile2_cdata = unhexlify(re.sub(r'\W', '', """ 0055f161493fcfc16276e8c31493c4641e1eb19a79d0326fad0291e5a9c98e5933 @@ -45,17 +45,17 @@ class MockArgs: keyfile_blake2_key_file = """ BORG_KEY 0000000000000000000000000000000000000000000000000000000000000000 - hqlhbGdvcml0aG2mc2hhMjU2pGRhdGHaAZBu680Do3CmfWzeMCwe48KJi3Vps9mEDy7MKF - TastsEhiAd1RQMuxfZpklkLeddMMWk+aPtFiURRFb02JLXV5cKRC1o2ZDdiNa0nao+o6+i - gUjjsea9TAu25t3vxh8uQWs5BuKRLBRr0nUgrSd0IYMUgn+iVbLJRzCCssvxsklkwQxN3F - Y+MvBnn8kUXSeoSoQ2l0fBHzq94Y7LMOm/owMam5URnE8/UEc6ZXBrbyX4EXxDtUqJcs+D - i451thtlGdigDLpvf9nyK66mjiCpPCTCgtlzq0Pe1jcdhnsUYLg+qWzXZ7e2opEZoC6XxS - 3DIuBOxG3Odqj9IKB+6/kl94vz98awPWFSpYcLZVWu7sIP38ZkUK+ad5MHTo/LvTuZdFnd - iqKzZIDUJl3Zl1WGmP/0xVOmfIlznkCZy4d3SMuujwIcqQ5kDvwDRPpdhBBk+UWQY5vFXk - kR1NBNLSTyhAzu3fiUmFl0qZ+UWPRkGAEBy/NuoEibrWwab8BX97cATyvnmOqYkU9PT0C6 - l2l9E4bPpGhhc2jaACDnIa8KgKv84/b5sjaMgSZeIVkuKSLJy2NN8zoH8lnd36ppdGVyYX - Rpb25zzgABhqCkc2FsdNoAIEJLlLh7q74j3q53856H5GgzA1HH+aW5bA/as544+PGkp3Zl - cnNpb24B""".strip() + hqlhbGdvcml0aG2mc2hhMjU2pGRhdGHaAZ7VCsTjbLhC1ipXOyhcGn7YnROEhP24UQvOCi + Oar1G+JpwgO9BIYaiCODUpzPuDQEm6WxyTwEneJ3wsuyeqyh7ru2xo9FAUKRf6jcqqZnan + ycTfktkUC+CPhKR7W6MTu5fPvy99chyL09/RGdD15aswR5PjNoFu4626sfMrBReyPdlxqt + F80m+fbNE/vln2Trqoz9EMHQ3IxjIK4q0m4Aj7TwCu7ZankFtwt898+tYsWE7lb2Ps/gXB + F8PM/5wHpYps2AKhDCpwKp5HyqIqlF5IzR2ydL9QP20QBjp/rSi6b+xwrfxNJZfw78f8ef + A2Yj7xIsxNQ0kmVmTL/UF6d7+Mw1JfurWrySiDU7QQ+RiZpWUZ0DdReB+e4zn6/KNKC884 + 34SGywADuLIQe2FKU+5jBCbutEyEGILQbAR/cgeLy5+V2XwXMJh4ytwXVIeT6Lk+qhYAdz + Klx4ub7XijKcOxJyBE+4k33DAhcfIT2r4/sxgMhXrIOEQPKsMAixzdcqVYkpou+6c4PZeL + nr+UjfJwOqK1BlWk1NgwE4GXYIKkaGFzaNoAIAzjUtpBPPh6kItZtHQZvnQG6FpucZNfBC + UTHFJg343jqml0ZXJhdGlvbnPOAAGGoKRzYWx02gAgz3YaUZZ/s+UWywj97EY5b4KhtJYi + qkPqtDDxs2j/T7+ndmVyc2lvbgE=""".strip() keyfile_blake2_cdata = bytes.fromhex('04fdf9475cf2323c0ba7a99ddc011064f2e7d039f539f2e448' '0e6f5fc6ff9993d604040404040404098c8cee1c6db8c28947') @@ -170,7 +170,6 @@ def test_keyfile_kfenv(self, tmpdir, monkeypatch): KeyfileKey.detect(self.MockRepository(), chunk_cdata) def test_keyfile2(self, monkeypatch, keys_dir): - monkeypatch.delenv('BORG_TESTONLY_MOCK_KDF') with keys_dir.join('keyfile').open('w') as fd: fd.write(self.keyfile2_key_file) monkeypatch.setenv('BORG_PASSPHRASE', 'passphrase') @@ -178,7 +177,6 @@ def test_keyfile2(self, monkeypatch, keys_dir): assert key.decrypt(self.keyfile2_id, self.keyfile2_cdata) == b'payload' def test_keyfile2_kfenv(self, tmpdir, monkeypatch): - monkeypatch.delenv('BORG_TESTONLY_MOCK_KDF') keyfile = tmpdir.join('keyfile') with keyfile.open('w') as fd: fd.write(self.keyfile2_key_file) @@ -188,7 +186,6 @@ def test_keyfile2_kfenv(self, tmpdir, monkeypatch): assert key.decrypt(self.keyfile2_id, self.keyfile2_cdata) == b'payload' def test_keyfile_blake2(self, monkeypatch, keys_dir): - monkeypatch.delenv('BORG_TESTONLY_MOCK_KDF') with keys_dir.join('keyfile').open('w') as fd: fd.write(self.keyfile_blake2_key_file) monkeypatch.setenv('BORG_PASSPHRASE', 'passphrase') @@ -204,7 +201,6 @@ def _corrupt_byte(self, key, data, offset): key.decrypt(b'', data) def test_decrypt_integrity(self, monkeypatch, keys_dir): - monkeypatch.delenv('BORG_TESTONLY_MOCK_KDF') with keys_dir.join('keyfile').open('w') as fd: fd.write(self.keyfile2_key_file) monkeypatch.setenv('BORG_PASSPHRASE', 'passphrase') From b5b3d43fd316396421aad632b98c60795caa5c81 Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Wed, 6 Apr 2022 05:00:48 +0300 Subject: [PATCH 23/28] typo --- src/borg/helpers/passphrase.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/borg/helpers/passphrase.py b/src/borg/helpers/passphrase.py index 26382e2b93..6a60930ba1 100644 --- a/src/borg/helpers/passphrase.py +++ b/src/borg/helpers/passphrase.py @@ -157,7 +157,7 @@ def argon2( if os.environ.get("BORG_TESTONLY_WEAKEN_KDF") is not None: time_cost = 1 parallelism = 1 - # 8 is the smallest value that aviods the "Memory cost is too small" exception + # 8 is the smallest value that avoids the "Memory cost is too small" exception memory_cost = 8 type_map = { 'i': argon2.low_level.Type.I, From d6474e96842cdf7d450ec25d6f3c7a5473beda6c Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Wed, 6 Apr 2022 05:08:49 +0300 Subject: [PATCH 24/28] Only weaken KDF if env var == "1" --- src/borg/helpers/passphrase.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/borg/helpers/passphrase.py b/src/borg/helpers/passphrase.py index 6a60930ba1..52ecc11bb5 100644 --- a/src/borg/helpers/passphrase.py +++ b/src/borg/helpers/passphrase.py @@ -141,7 +141,7 @@ def __repr__(self): return '' def kdf(self, salt, iterations, length): - if os.environ.get("BORG_TESTONLY_WEAKEN_KDF") is not None: + if os.environ.get("BORG_TESTONLY_WEAKEN_KDF") == "1": iterations = 1 return pbkdf2_hmac('sha256', self.encode('utf-8'), salt, iterations, length) @@ -154,7 +154,7 @@ def argon2( parallelism, type: Literal['i', 'd', 'id'] ) -> bytes: - if os.environ.get("BORG_TESTONLY_WEAKEN_KDF") is not None: + if os.environ.get("BORG_TESTONLY_WEAKEN_KDF") == "1": time_cost = 1 parallelism = 1 # 8 is the smallest value that avoids the "Memory cost is too small" exception From 3c5c1975a9007715f48dddacd85e1b7f57fc2555 Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Thu, 7 Apr 2022 07:09:43 +0300 Subject: [PATCH 25/28] Clarify the purpose of the test --- src/borg/testsuite/crypto.py | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/src/borg/testsuite/crypto.py b/src/borg/testsuite/crypto.py index 11d81c3740..c478e7e994 100644 --- a/src/borg/testsuite/crypto.py +++ b/src/borg/testsuite/crypto.py @@ -361,13 +361,37 @@ def to_dict(key): @unittest.mock.patch('getpass.getpass') def test_repo_key_detect_does_not_raise_integrity_error(getpass, monkeypatch): - """https://github.com/borgbackup/borg/pull/6469#discussion_r832670411""" + """https://github.com/borgbackup/borg/pull/6469#discussion_r832670411 + + This is a regression test for a bug I introduced and fixed: + + Traceback (most recent call last): + File "/home/user/borg-master/src/borg/testsuite/crypto.py", line 384, in test_repo_key_detect_does_not_raise_integrity_error + RepoKey.detect(repository, manifest_data=None) + File "/home/user/borg-master/src/borg/crypto/key.py", line 402, in detect + if not key.load(target, passphrase): + File "/home/user/borg-master/src/borg/crypto/key.py", line 654, in load + success = self._load(key_data, passphrase) + File "/home/user/borg-master/src/borg/crypto/key.py", line 418, in _load + data = self.decrypt_key_file(cdata, passphrase) + File "/home/user/borg-master/src/borg/crypto/key.py", line 444, in decrypt_key_file + return self.decrypt_key_file_argon2(encrypted_key, passphrase) + File "/home/user/borg-master/src/borg/crypto/key.py", line 470, in decrypt_key_file_argon2 + return ae_cipher.decrypt(encrypted_key.data) + File "src/borg/crypto/low_level.pyx", line 302, in borg.crypto.low_level.AES256_CTR_BASE.decrypt + self.mac_verify( idata.buf+aoffset, alen, + File "src/borg/crypto/low_level.pyx", line 382, in borg.crypto.low_level.AES256_CTR_HMAC_SHA256.mac_verify + raise IntegrityError('MAC Authentication failed') + borg.crypto.low_level.IntegrityError: MAC Authentication failed + + 1. FlexiKey.decrypt_key_file() is supposed to signal the decryption failure by returning None + 2. FlexiKey.detect() relies on that interface - it tries an empty passphrase before prompting the user + 3. my initial implementation of decrypt_key_file_argon2() was simply passing through the IntegrityError() from AES256_CTR_BASE.decrypt() + """ repository = MagicMock(id=b'repository_id') getpass.return_value = "hello, pass phrase" monkeypatch.setenv('BORG_DISPLAY_PASSPHRASE', 'no') RepoKey.create(repository, args=MagicMock(key_algorithm='argon2')) repository.load_key.return_value = repository.save_key.call_args.args[0] - # 1. detect() tries an empty passphrase first before prompting the user - # 2. load() was throwing integrity errors instead of returning None due to a bug RepoKey.detect(repository, manifest_data=None) From 2e1de162be7256db2ecb003c8be4053fe5bdf48c Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Thu, 7 Apr 2022 07:16:26 +0300 Subject: [PATCH 26/28] Use consistent argon2 paramaters in a test --- src/borg/testsuite/crypto.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/borg/testsuite/crypto.py b/src/borg/testsuite/crypto.py index c478e7e994..3465bb8ff8 100644 --- a/src/borg/testsuite/crypto.py +++ b/src/borg/testsuite/crypto.py @@ -275,9 +275,9 @@ def test_decrypt_key_file_argon2_aes256_ctr_hmac_sha256(monkeypatch): encrypted = msgpack.packb({ 'version': 1, 'salt': b'salt'*4, - 'argon2_time_cost': 3, - 'argon2_memory_cost': 2**16, - 'argon2_parallelism': 4, + 'argon2_time_cost': 1, + 'argon2_memory_cost': 8, + 'argon2_parallelism': 1, 'argon2_type': b'id', 'algorithm': 'argon2 aes256-ctr hmac-sha256', 'data': envelope, From 721edc957d28020b7b7202339644d8af35b77ad7 Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Thu, 7 Apr 2022 07:35:29 +0300 Subject: [PATCH 27/28] Test that pbkdf2 is not autoupgraded --- src/borg/testsuite/archiver.py | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/borg/testsuite/archiver.py b/src/borg/testsuite/archiver.py index 96ad1b2123..1a4b791cca 100644 --- a/src/borg/testsuite/archiver.py +++ b/src/borg/testsuite/archiver.py @@ -3617,24 +3617,36 @@ def test_init_with_explicit_key_algorithm(self): key = msgpack.unpackb(a2b_base64(repository.load_key())) assert key[b'algorithm'] == b'sha256' - def test_change_passphrase_does_not_change_algorithm(self): - self.cmd('init', '--encryption=repokey', '--key-algorithm', 'argon2', self.repository_location) + def verify_change_passphrase_does_not_change_algorithm(self, given_algorithm, expected_algorithm): + self.cmd('init', '--encryption=repokey', '--key-algorithm', given_algorithm, self.repository_location) os.environ['BORG_NEW_PASSPHRASE'] = 'newpassphrase' self.cmd('key', 'change-passphrase', self.repository_location) with Repository(self.repository_path) as repository: key = msgpack.unpackb(a2b_base64(repository.load_key())) - assert key[b'algorithm'] == b'argon2 aes256-ctr hmac-sha256' + assert key[b'algorithm'] == expected_algorithm - def test_change_location_does_not_change_algorithm(self): - self.cmd('init', '--encryption=keyfile', '--key-algorithm', 'argon2', self.repository_location) + def test_change_passphrase_does_not_change_algorithm_argon2(self): + self.verify_change_passphrase_does_not_change_algorithm('argon2', b'argon2 aes256-ctr hmac-sha256') + + def test_change_passphrase_does_not_change_algorithm_pbkdf2(self): + self.verify_change_passphrase_does_not_change_algorithm('pbkdf2', b'sha256') + + def verify_change_location_does_not_change_algorithm(self, given_algorithm, expected_algorithm): + self.cmd('init', '--encryption=keyfile', '--key-algorithm', given_algorithm, self.repository_location) self.cmd('key', 'change-location', self.repository_location, 'repokey') with Repository(self.repository_path) as repository: key = msgpack.unpackb(a2b_base64(repository.load_key())) - assert key[b'algorithm'] == b'argon2 aes256-ctr hmac-sha256' + assert key[b'algorithm'] == expected_algorithm + + def test_change_location_does_not_change_algorithm_argon2(self): + self.verify_change_location_does_not_change_algorithm('argon2', b'argon2 aes256-ctr hmac-sha256') + + def test_change_location_does_not_change_algorithm_pbkdf2(self): + self.verify_change_location_does_not_change_algorithm('pbkdf2', b'sha256') @unittest.skipUnless('binary' in BORG_EXES, 'no borg.exe available') From 1aa652eda893d7add0e69087a069aba44b1921c4 Mon Sep 17 00:00:00 2001 From: Andrey Bienkowski Date: Thu, 7 Apr 2022 07:42:19 +0300 Subject: [PATCH 28/28] Pass arguments consistently in tests --- src/borg/testsuite/archiver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/borg/testsuite/archiver.py b/src/borg/testsuite/archiver.py index 1a4b791cca..da12b8e64e 100644 --- a/src/borg/testsuite/archiver.py +++ b/src/borg/testsuite/archiver.py @@ -3612,7 +3612,7 @@ def test_init_defaults_to_argon2(self): def test_init_with_explicit_key_algorithm(self): """https://github.com/borgbackup/borg/issues/747#issuecomment-1076160401""" - self.cmd('init', '--encryption=repokey', '--key-algorithm', 'pbkdf2', self.repository_location) + self.cmd('init', '--encryption=repokey', '--key-algorithm=pbkdf2', self.repository_location) with Repository(self.repository_path) as repository: key = msgpack.unpackb(a2b_base64(repository.load_key())) assert key[b'algorithm'] == b'sha256'