From 28ad779a6f64b484d57ed92097db5cf884ad1842 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Sun, 11 Dec 2016 13:32:09 +0100 Subject: [PATCH] Add tertiary authentication for metadata (TAM) Signed-off-by: Thomas Waldmann --- borg/archive.py | 4 +- borg/archiver.py | 84 +++++++++++++++++---- borg/crypto.pyx | 33 ++++++++- borg/helpers.py | 41 +++++++--- borg/key.py | 148 ++++++++++++++++++++++++++++++++++--- borg/testsuite/archiver.py | 72 ++++++++++++++++-- borg/testsuite/crypto.py | 53 +++++++++++++ borg/testsuite/helpers.py | 14 +++- borg/testsuite/key.py | 122 +++++++++++++++++++++++++++++- docs/changes.rst | 69 ++++++++++++++++- docs/usage.rst | 3 + 11 files changed, 591 insertions(+), 52 deletions(-) diff --git a/borg/archive.py b/borg/archive.py index 641fd08dd8..216abc57d2 100644 --- a/borg/archive.py +++ b/borg/archive.py @@ -231,7 +231,7 @@ def __init__(self, repository, key, manifest, name, cache=None, create=False, def _load_meta(self, id): data = self.key.decrypt(id, self.repository.get(id)) - metadata = msgpack.unpackb(data) + metadata = msgpack.unpackb(data, unicode_errors='surrogateescape') if metadata[b'version'] != 1: raise Exception('Unknown archive metadata version') return metadata @@ -325,7 +325,7 @@ def save(self, name=None, timestamp=None): 'time': start.isoformat(), 'time_end': end.isoformat(), }) - data = msgpack.packb(metadata, unicode_errors='surrogateescape') + data = self.key.pack_and_authenticate_metadata(metadata, context=b'archive') self.id = self.key.id_hash(data) self.cache.add_chunk(self.id, data, self.stats) self.manifest.archives[name] = {'id': self.id, 'time': metadata['time']} diff --git a/borg/archiver.py b/borg/archiver.py index 2cdfe37f3e..037f3680d6 100644 --- a/borg/archiver.py +++ b/borg/archiver.py @@ -30,7 +30,7 @@ from .upgrader import AtticRepositoryUpgrader, BorgRepositoryUpgrader from .repository import Repository from .cache import Cache -from .key import key_creator, RepoKey, PassphraseKey +from .key import key_creator, tam_required_file, tam_required, RepoKey, PassphraseKey from .keymanager import KeyManager from .archive import backup_io, BackupOSError, Archive, ArchiveChecker, CHUNKER_PARAMS, is_special from .remote import RepositoryServer, RemoteRepository, cache_if_remote @@ -51,7 +51,7 @@ def argument(args, str_or_bool): return str_or_bool -def with_repository(fake=False, create=False, lock=True, exclusive=False, manifest=True, cache=False): +def with_repository(fake=False, invert_fake=False, create=False, lock=True, exclusive=False, manifest=True, cache=False): """ Method decorator for subcommand-handling methods: do_XYZ(self, args, repository, …) @@ -68,7 +68,7 @@ def decorator(method): def wrapper(self, args, **kwargs): location = args.location # note: 'location' must be always present in args append_only = getattr(args, 'append_only', False) - if argument(args, fake): + if argument(args, fake) ^ invert_fake: return method(self, args, repository=None, **kwargs) elif location.proto == 'ssh': repository = RemoteRepository(location, create=create, exclusive=argument(args, exclusive), @@ -135,6 +135,8 @@ def do_init(self, args, repository): repository.commit() with Cache(repository, key, manifest, warn_if_unencrypted=False): pass + tam_file = tam_required_file(repository) + open(tam_file, 'w').close() return self.exit_code @with_repository(exclusive=True, manifest=False) @@ -161,6 +163,7 @@ def do_check(self, args, repository): def do_change_passphrase(self, args, repository, manifest, key): """Change repository key file passphrase""" key.change_passphrase() + logger.info('Key updated') return EXIT_SUCCESS @with_repository(lock=False, exclusive=False, manifest=False, cache=False) @@ -209,6 +212,7 @@ def do_migrate_to_repokey(self, args, repository): key_new.id_key = key_old.id_key key_new.chunk_seed = key_old.chunk_seed key_new.change_passphrase() # option to change key protection passphrase, save + logger.info('Key updated') return EXIT_SUCCESS @with_repository(fake='dry_run', exclusive=True) @@ -705,21 +709,43 @@ def do_prune(self, args, repository, manifest, key): DASHES) return self.exit_code - def do_upgrade(self, args): + @with_repository(fake='tam', invert_fake=True, manifest=False, exclusive=True) + def do_upgrade(self, args, repository, manifest=None, key=None): """upgrade a repository from a previous version""" - # mainly for upgrades from Attic repositories, - # but also supports borg 0.xx -> 1.0 upgrade. + if args.tam: + manifest, key = Manifest.load(repository, force_tam_not_required=args.force) + + if not manifest.tam_verified: + # The standard archive listing doesn't include the archive ID like in borg 1.1.x + print('Manifest contents:') + for archive_info in manifest.list_archive_infos(sort_by='ts'): + print(format_archive(archive_info), '[%s]' % bin_to_hex(archive_info.id)) + manifest.write() + repository.commit() + if not key.tam_required: + key.tam_required = True + key.change_passphrase(key._passphrase) + print('Updated key') + if hasattr(key, 'find_key'): + print('Key location:', key.find_key()) + if not tam_required(repository): + tam_file = tam_required_file(repository) + open(tam_file, 'w').close() + print('Updated security database') + else: + # mainly for upgrades from Attic repositories, + # but also supports borg 0.xx -> 1.0 upgrade. - repo = AtticRepositoryUpgrader(args.location.path, create=False) - try: - repo.upgrade(args.dry_run, inplace=args.inplace, progress=args.progress) - except NotImplementedError as e: - print("warning: %s" % e) - repo = BorgRepositoryUpgrader(args.location.path, create=False) - try: - repo.upgrade(args.dry_run, inplace=args.inplace, progress=args.progress) - except NotImplementedError as e: - print("warning: %s" % e) + repo = AtticRepositoryUpgrader(args.location.path, create=False) + try: + repo.upgrade(args.dry_run, inplace=args.inplace, progress=args.progress) + except NotImplementedError as e: + print("warning: %s" % e) + repo = BorgRepositoryUpgrader(args.location.path, create=False) + try: + repo.upgrade(args.dry_run, inplace=args.inplace, progress=args.progress) + except NotImplementedError as e: + print("warning: %s" % e) return self.exit_code def do_debug_info(self, args): @@ -1613,6 +1639,28 @@ def build_parser(self, args=None, prog=None): upgrade_epilog = textwrap.dedent(""" Upgrade an existing Borg repository. + + Borg 1.x.y upgrades + ------------------- + + Use ``borg upgrade --tam REPO`` to require manifest authentication + introduced with Borg 1.0.9 to address security issues. This means + that modifying the repository after doing this with a version prior + to 1.0.9 will raise a validation error, so only perform this upgrade + after updating all clients using the repository to 1.0.9 or newer. + + This upgrade should be done on each client for safety reasons. + + If a repository is accidentally modified with a pre-1.0.9 client after + this upgrade, use ``borg upgrade --tam --force REPO`` to remedy it. + + See + https://borgbackup.readthedocs.io/en/stable/changes.html#pre-1-0-9-manifest-spoofing-vulnerability + for details. + + Attic and Borg 0.xx to Borg 1.x + ------------------------------- + This currently supports converting an Attic repository to Borg and also helps with converting Borg 0.xx to 1.0. @@ -1665,6 +1713,10 @@ def build_parser(self, args=None, prog=None): default=False, action='store_true', help="""rewrite repository in place, with no chance of going back to older versions of the repository.""") + subparser.add_argument('--force', dest='force', action='store_true', + help="""Force upgrade""") + subparser.add_argument('--tam', dest='tam', action='store_true', + help="""Enable manifest authentication (in key and cache) (Borg 1.0.9 and later)""") subparser.add_argument('location', metavar='REPOSITORY', nargs='?', default='', type=location_validator(archive=False), help='path to the repository to be upgraded') diff --git a/borg/crypto.pyx b/borg/crypto.pyx index be692742c1..f7beb34334 100644 --- a/borg/crypto.pyx +++ b/borg/crypto.pyx @@ -2,9 +2,13 @@ This could be replaced by PyCrypto maybe? """ +import hashlib +import hmac +from math import ceil + from libc.stdlib cimport malloc, free -API_VERSION = 2 +API_VERSION = 3 cdef extern from "openssl/rand.h": int RAND_bytes(unsigned char *buf, int num) @@ -171,3 +175,30 @@ cdef class AES: return out[:ptl] finally: free(out) + + +def hkdf_hmac_sha512(ikm, salt, info, output_length): + """ + Compute HKDF-HMAC-SHA512 with input key material *ikm*, *salt* and *info* to produce *output_length* bytes. + + This is the "HMAC-based Extract-and-Expand Key Derivation Function (HKDF)" (RFC 5869) + instantiated with HMAC-SHA512. + + *output_length* must not be greater than 64 * 255 bytes. + """ + digest_length = 64 + assert output_length <= (255 * digest_length), 'output_length must be <= 255 * 64 bytes' + # Step 1. HKDF-Extract (ikm, salt) -> prk + if salt is None: + salt = bytes(64) + prk = hmac.HMAC(salt, ikm, hashlib.sha512).digest() + + # Step 2. HKDF-Expand (prk, info, output_length) -> output key + n = ceil(output_length / digest_length) + t_n = b'' + output = b'' + for i in range(n): + msg = t_n + info + (i + 1).to_bytes(1, 'little') + t_n = hmac.HMAC(prk, msg, hashlib.sha512).digest() + output += t_n + return output[:output_length] diff --git a/borg/helpers.py b/borg/helpers.py index a8106aeef6..df5a213af4 100644 --- a/borg/helpers.py +++ b/borg/helpers.py @@ -86,7 +86,7 @@ def check_extension_modules(): raise ExtensionModuleError if chunker.API_VERSION != 2: raise ExtensionModuleError - if crypto.API_VERSION != 2: + if crypto.API_VERSION != 3: raise ExtensionModuleError if platform.API_VERSION != 3: raise ExtensionModuleError @@ -103,10 +103,11 @@ def __init__(self, key, repository, item_keys=None): self.key = key self.repository = repository self.item_keys = frozenset(item_keys) if item_keys is not None else ITEM_KEYS + self.tam_verified = False @classmethod - def load(cls, repository, key=None): - from .key import key_factory + def load(cls, repository, key=None, force_tam_not_required=False): + from .key import key_factory, tam_required_file, tam_required from .repository import Repository from .archive import ITEM_KEYS try: @@ -117,8 +118,8 @@ def load(cls, repository, key=None): key = key_factory(repository, cdata) manifest = cls(key, repository) data = key.decrypt(None, cdata) + m, manifest.tam_verified = key.unpack_and_verify_manifest(data, force_tam_not_required=force_tam_not_required) manifest.id = key.id_hash(data) - m = msgpack.unpackb(data) if not m.get(b'version') == 1: raise ValueError('Invalid manifest version') manifest.archives = dict((k.decode('utf-8'), v) for k, v in m[b'archives'].items()) @@ -128,19 +129,27 @@ def load(cls, repository, key=None): manifest.config = m[b'config'] # valid item keys are whatever is known in the repo or every key we know manifest.item_keys = frozenset(m.get(b'item_keys', [])) | ITEM_KEYS + if manifest.config.get(b'tam_required', False) and manifest.tam_verified and not tam_required(repository): + logger.debug('Manifest is TAM verified and says TAM is required, updating security database...') + file = tam_required_file(repository) + open(file, 'w').close() return manifest, key def write(self): + if self.key.tam_required: + self.config[b'tam_required'] = True self.timestamp = datetime.utcnow().isoformat() - data = msgpack.packb(StableDict({ + m = { 'version': 1, - 'archives': self.archives, + 'archives': StableDict((name, StableDict(archive)) for name, archive in self.archives.items()), 'timestamp': self.timestamp, - 'config': self.config, - 'item_keys': tuple(self.item_keys), - })) + 'config': StableDict(self.config), + 'item_keys': tuple(sorted(self.item_keys)), + } + self.tam_verified = True + data = self.key.pack_and_authenticate_metadata(m) self.id = self.key.id_hash(data) - self.repository.put(self.MANIFEST_ID, self.key.encrypt(data)) + self.repository.put(self.MANIFEST_ID, self.key.encrypt(data, none_compression=True)) def list_archive_infos(self, sort_by=None, reverse=False): # inexpensive Archive.list_archives replacement if we just need .name, .id, .ts @@ -249,6 +258,18 @@ def get_keys_dir(): return keys_dir +def get_security_dir(repository_id=None): + """Determine where to store local security information.""" + xdg_config = os.environ.get('XDG_CONFIG_HOME', os.path.join(os.path.expanduser('~'), '.config')) + security_dir = os.environ.get('BORG_SECURITY_DIR', os.path.join(xdg_config, 'borg', 'security')) + if repository_id: + security_dir = os.path.join(security_dir, repository_id) + if not os.path.exists(security_dir): + os.makedirs(security_dir) + os.chmod(security_dir, stat.S_IRWXU) + return security_dir + + def get_cache_dir(): """Determine where to repository keys and cache""" xdg_cache = os.environ.get('XDG_CACHE_HOME', os.path.join(os.path.expanduser('~'), '.cache')) diff --git a/borg/key.py b/borg/key.py index e4fcd03d9a..318f8d0ed1 100644 --- a/borg/key.py +++ b/borg/key.py @@ -5,15 +5,17 @@ import sys import textwrap from hmac import HMAC, compare_digest -from hashlib import sha256, pbkdf2_hmac +from hashlib import sha256, sha512, pbkdf2_hmac -from .helpers import IntegrityError, get_keys_dir, Error, yes, bin_to_hex +import msgpack + +from .helpers import StableDict, IntegrityError, get_keys_dir, get_security_dir, Error, yes, bin_to_hex from .logger import create_logger logger = create_logger() from .crypto import AES, bytes_to_long, long_to_bytes, bytes_to_int, num_aes_blocks -from .compress import Compressor -import msgpack +from .crypto import hkdf_hmac_sha512 +from .compress import Compressor, CNONE PREFIX = b'\0' * 8 @@ -30,6 +32,10 @@ class UnsupportedPayloadError(Error): """Unsupported payload type {}. A newer version is required to access this repository.""" +class UnsupportedManifestError(Error): + """Unsupported manifest envelope. A newer version is required to access this repository.""" + + class KeyfileNotFoundError(Error): """No key file for repository {} found in {}.""" @@ -38,6 +44,32 @@ class RepoKeyNotFoundError(Error): """No key entry found in the config of repository {}.""" +class TAMRequiredError(IntegrityError): + __doc__ = textwrap.dedent(""" + Manifest is unauthenticated, but authentication is required for this repository. + + This either means that you are under attack, or that you modified this repository + with a Borg version older than 1.0.9 after TAM authentication was enabled. + + In the latter case, use "borg upgrade --tam --force '{}'" to re-authenticate the manifest. + """).strip() + traceback = False + + +class TAMInvalid(IntegrityError): + __doc__ = IntegrityError.__doc__ + traceback = False + + def __init__(self): + # Error message becomes: "Data integrity error: Manifest authentication did not verify" + super().__init__('Manifest authentication did not verify') + + +class TAMUnsupportedSuiteError(IntegrityError): + """Could not verify manifest: Unsupported suite {!r}; a newer version is needed.""" + traceback = False + + def key_creator(repository, args): if args.encryption == 'keyfile': return KeyfileKey.create(repository, args) @@ -63,6 +95,16 @@ def key_factory(repository, manifest_data): raise UnsupportedPayloadError(key_type) +def tam_required_file(repository): + security_dir = get_security_dir(bin_to_hex(repository.id)) + return os.path.join(security_dir, 'tam_required') + + +def tam_required(repository): + file = tam_required_file(repository) + return os.path.isfile(file) + + class KeyBase: TYPE = None # override in subclasses @@ -71,23 +113,90 @@ def __init__(self, repository): self.repository = repository self.target = None # key location file path / repo obj self.compressor = Compressor('none') + self.tam_required = True def id_hash(self, data): """Return HMAC hash using the "id" HMAC key """ - def encrypt(self, data): + def encrypt(self, data, none_compression=False): pass def decrypt(self, id, data): pass + def _tam_key(self, salt, context): + return hkdf_hmac_sha512( + ikm=self.id_key + self.enc_key + self.enc_hmac_key, + salt=salt, + info=b'borg-metadata-authentication-' + context, + output_length=64 + ) + + def pack_and_authenticate_metadata(self, metadata_dict, context=b'manifest'): + metadata_dict = StableDict(metadata_dict) + tam = metadata_dict['tam'] = StableDict({ + 'type': 'HKDF_HMAC_SHA512', + 'hmac': bytes(64), + 'salt': os.urandom(64), + }) + packed = msgpack.packb(metadata_dict, unicode_errors='surrogateescape') + tam_key = self._tam_key(tam['salt'], context) + tam['hmac'] = HMAC(tam_key, packed, sha512).digest() + return msgpack.packb(metadata_dict, unicode_errors='surrogateescape') + + def unpack_and_verify_manifest(self, data, force_tam_not_required=False): + """Unpack msgpacked *data* and return (object, did_verify).""" + if data.startswith(b'\xc1' * 4): + # This is a manifest from the future, we can't read it. + raise UnsupportedManifestError() + tam_required = self.tam_required + if force_tam_not_required and tam_required: + logger.warning('Manifest authentication DISABLED.') + tam_required = False + data = bytearray(data) + # Since we don't trust these bytes we use the slower Python unpacker, + # which is assumed to have a lower probability of security issues. + unpacked = msgpack.fallback.unpackb(data, object_hook=StableDict, unicode_errors='surrogateescape') + if b'tam' not in unpacked: + if tam_required: + raise TAMRequiredError(self.repository._location.canonical_path()) + else: + logger.debug('TAM not found and not required') + return unpacked, False + tam = unpacked.pop(b'tam', None) + if not isinstance(tam, dict): + raise TAMInvalid() + tam_type = tam.get(b'type', b'').decode('ascii', 'replace') + if tam_type != 'HKDF_HMAC_SHA512': + if tam_required: + raise TAMUnsupportedSuiteError(repr(tam_type)) + else: + logger.debug('Ignoring TAM made with unsupported suite, since TAM is not required: %r', tam_type) + return unpacked, False + tam_hmac = tam.get(b'hmac') + tam_salt = tam.get(b'salt') + if not isinstance(tam_salt, bytes) or not isinstance(tam_hmac, bytes): + raise TAMInvalid() + offset = data.index(tam_hmac) + data[offset:offset + 64] = bytes(64) + tam_key = self._tam_key(tam_salt, context=b'manifest') + calculated_hmac = HMAC(tam_key, data, sha512).digest() + if not compare_digest(calculated_hmac, tam_hmac): + raise TAMInvalid() + logger.debug('TAM-verified manifest') + return unpacked, True + class PlaintextKey(KeyBase): TYPE = 0x02 chunk_seed = 0 + def __init__(self, repository): + super().__init__(repository) + self.tam_required = False + @classmethod def create(cls, repository, args): logger.info('Encryption NOT enabled.\nUse the "--encryption=repokey|keyfile" to enable encryption.') @@ -100,8 +209,12 @@ def detect(cls, repository, manifest_data): def id_hash(self, data): return sha256(data).digest() - def encrypt(self, data): - return b''.join([self.TYPE_STR, self.compressor.compress(data)]) + def encrypt(self, data, none_compression=False): + if none_compression: + compressed = CNONE().compress(data) + else: + compressed = self.compressor.compress(data) + return b''.join([self.TYPE_STR, compressed]) def decrypt(self, id, data): if data[0] != self.TYPE: @@ -112,6 +225,9 @@ def decrypt(self, id, data): raise IntegrityError('Chunk %s: id verification failed' % bin_to_hex(id)) return data + def _tam_key(self, salt, context): + return salt + context + class AESKeyBase(KeyBase): """Common base class shared by KeyfileKey and PassphraseKey @@ -133,8 +249,11 @@ def id_hash(self, data): """ return HMAC(self.id_key, data, sha256).digest() - def encrypt(self, data): - data = self.compressor.compress(data) + def encrypt(self, data, none_compression=False): + if none_compression: + data = CNONE().compress(data) + else: + data = self.compressor.compress(data) self.enc_cipher.reset() data = b''.join((self.enc_cipher.iv[8:], self.enc_cipher.encrypt(data))) hmac = HMAC(self.enc_hmac_key, data, sha256).digest() @@ -269,6 +388,7 @@ def detect(cls, repository, manifest_data): key.decrypt(None, manifest_data) num_blocks = num_aes_blocks(len(manifest_data) - 41) key.init_ciphers(PREFIX + long_to_bytes(key.extract_nonce(manifest_data) + num_blocks)) + key._passphrase = passphrase return key except IntegrityError: passphrase = Passphrase.getpass(prompt) @@ -284,6 +404,7 @@ class ImmutablePassphraseError(Error): def init(self, repository, passphrase): self.init_from_random_data(passphrase.kdf(repository.id, self.iterations, 100)) self.init_ciphers() + self.tam_required = False class KeyfileKeyBase(AESKeyBase): @@ -307,6 +428,7 @@ def detect(cls, repository, manifest_data): raise PassphraseWrong num_blocks = num_aes_blocks(len(manifest_data) - 41) key.init_ciphers(PREFIX + long_to_bytes(key.extract_nonce(manifest_data) + num_blocks)) + key._passphrase = passphrase return key def find_key(self): @@ -327,6 +449,7 @@ def _load(self, key_data, passphrase): self.enc_hmac_key = key[b'enc_hmac_key'] self.id_key = key[b'id_key'] self.chunk_seed = key[b'chunk_seed'] + self.tam_required = key.get(b'tam_required', tam_required(self.repository)) return True return False @@ -363,15 +486,16 @@ def _save(self, passphrase): 'enc_hmac_key': self.enc_hmac_key, 'id_key': self.id_key, 'chunk_seed': self.chunk_seed, + 'tam_required': self.tam_required, } data = self.encrypt_key_file(msgpack.packb(key), passphrase) key_data = '\n'.join(textwrap.wrap(b2a_base64(data).decode('ascii'))) return key_data - def change_passphrase(self): - passphrase = Passphrase.new(allow_empty=True) + def change_passphrase(self, passphrase=None): + if passphrase is None: + passphrase = Passphrase.new(allow_empty=True) self.save(self.target, passphrase) - logger.info('Key updated') @classmethod def create(cls, repository, args): diff --git a/borg/testsuite/archiver.py b/borg/testsuite/archiver.py index 815c8943ab..2daa92198f 100644 --- a/borg/testsuite/archiver.py +++ b/borg/testsuite/archiver.py @@ -2,6 +2,8 @@ from configparser import ConfigParser import errno import os +from datetime import datetime +from datetime import timedelta from io import StringIO import random import stat @@ -14,6 +16,7 @@ from unittest.mock import patch from hashlib import sha256 +import msgpack import pytest from .. import xattr @@ -21,13 +24,15 @@ from ..archiver import Archiver from ..cache import Cache from ..crypto import bytes_to_long, num_aes_blocks -from ..helpers import Manifest, PatternMatcher, parse_pattern, EXIT_SUCCESS, EXIT_WARNING, EXIT_ERROR, bin_to_hex -from ..key import RepoKey, KeyfileKey, Passphrase +from ..helpers import Manifest, PatternMatcher, parse_pattern, EXIT_SUCCESS, EXIT_WARNING, EXIT_ERROR, bin_to_hex, \ + get_security_dir +from ..key import RepoKey, KeyfileKey, Passphrase, TAMRequiredError from ..keymanager import RepoIdMismatch, NotABorgKeyFile from ..remote import RemoteRepository, PathNotAllowed from ..repository import Repository from . import BaseTestCase, changedir, environment_variable, no_selinux from .platform import fakeroot_detected +from . import key try: import llfuse @@ -1143,8 +1148,8 @@ def verify_aes_counter_uniqueness(self, method): def verify_uniqueness(): with Repository(self.repository_path) as repository: - for key, _ in repository.open_index(repository.get_transaction_id()).iteritems(): - data = repository.get(key) + for id, _ in repository.open_index(repository.get_transaction_id()).iteritems(): + data = repository.get(id) hash = sha256(data).digest() if hash not in seen: seen.add(hash) @@ -1253,7 +1258,7 @@ def test_key_export_repokey(self): repo_key = RepoKey(repository) repo_key.load(None, Passphrase.env_passphrase()) - backup_key = KeyfileKey(None) + backup_key = KeyfileKey(key.KeyTestCase.MockRepository()) backup_key.load(export_file, Passphrase.env_passphrase()) assert repo_key.enc_key == backup_key.enc_key @@ -1501,6 +1506,63 @@ def test_attic013_acl_bug(self): self.cmd('check', self.repository_location, exit_code=0) +class ManifestAuthenticationTest(ArchiverTestCaseBase): + def test_fresh_init_tam_required(self): + self.cmd('init', self.repository_location) + repository = Repository(self.repository_path, exclusive=True) + with repository: + manifest, key = Manifest.load(repository) + repository.put(Manifest.MANIFEST_ID, key.encrypt(msgpack.packb({ + 'version': 1, + 'archives': {}, + 'timestamp': (datetime.utcnow() + timedelta(days=1)).isoformat(), + }))) + repository.commit() + + with pytest.raises(TAMRequiredError): + self.cmd('list', self.repository_location) + + def test_not_required(self): + self.cmd('init', self.repository_location) + self.create_src_archive('archive1234') + repository = Repository(self.repository_path, exclusive=True) + with repository: + shutil.rmtree(get_security_dir(bin_to_hex(repository.id))) + _, key = Manifest.load(repository) + key.tam_required = False + key.change_passphrase(key._passphrase) + + manifest = msgpack.unpackb(key.decrypt(None, repository.get(Manifest.MANIFEST_ID))) + del manifest[b'tam'] + repository.put(Manifest.MANIFEST_ID, key.encrypt(msgpack.packb(manifest))) + repository.commit() + output = self.cmd('list', '--debug', self.repository_location) + assert 'archive1234' in output + assert 'TAM not found and not required' in output + # Run upgrade + self.cmd('upgrade', '--tam', self.repository_location) + # Manifest must be authenticated now + output = self.cmd('list', '--debug', self.repository_location) + assert 'archive1234' in output + assert 'TAM-verified manifest' in output + # Try to spoof / modify pre-1.0.9 + with repository: + _, key = Manifest.load(repository) + repository.put(Manifest.MANIFEST_ID, key.encrypt(msgpack.packb({ + 'version': 1, + 'archives': {}, + 'config': {}, + 'timestamp': (datetime.utcnow() + timedelta(days=1)).isoformat(), + }))) + repository.commit() + # Fails + with pytest.raises(TAMRequiredError): + self.cmd('list', self.repository_location) + # Force upgrade + self.cmd('upgrade', '--tam', '--force', self.repository_location) + self.cmd('list', self.repository_location) + + @pytest.mark.skipif(sys.platform == 'cygwin', reason='remote is broken on cygwin and hangs') class RemoteArchiverTestCase(ArchiverTestCase): prefix = '__testsuite__:' diff --git a/borg/testsuite/crypto.py b/borg/testsuite/crypto.py index c68101942d..e80a38b34e 100644 --- a/borg/testsuite/crypto.py +++ b/borg/testsuite/crypto.py @@ -2,6 +2,7 @@ from ..crypto import AES, bytes_to_long, bytes_to_int, long_to_bytes from ..crypto import increment_iv, bytes16_to_int, int_to_bytes16 +from ..crypto import hkdf_hmac_sha512 from . import BaseTestCase @@ -50,3 +51,55 @@ def test_aes(self): pdata = aes.decrypt(cdata) self.assert_equal(data, pdata) self.assert_equal(bytes_to_long(aes.iv, 8), 2) + + # These test vectors come from https://www.kullo.net/blog/hkdf-sha-512-test-vectors/ + # who claims to have verified these against independent Python and C++ implementations. + + def test_hkdf_hmac_sha512(self): + ikm = b'\x0b' * 22 + salt = bytes.fromhex('000102030405060708090a0b0c') + info = bytes.fromhex('f0f1f2f3f4f5f6f7f8f9') + l = 42 + + okm = hkdf_hmac_sha512(ikm, salt, info, l) + assert okm == bytes.fromhex('832390086cda71fb47625bb5ceb168e4c8e26a1a16ed34d9fc7fe92c1481579338da362cb8d9f925d7cb') + + def test_hkdf_hmac_sha512_2(self): + ikm = bytes.fromhex('000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f2021222324252627' + '28292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f404142434445464748494a4b4c4d4e4f') + salt = bytes.fromhex('606162636465666768696a6b6c6d6e6f707172737475767778797a7b7c7d7e7f808182838485868' + '788898a8b8c8d8e8f909192939495969798999a9b9c9d9e9fa0a1a2a3a4a5a6a7a8a9aaabacadaeaf') + info = bytes.fromhex('b0b1b2b3b4b5b6b7b8b9babbbcbdbebfc0c1c2c3c4c5c6c7c8c9cacbcccdcecfd0d1d2d3d4d5d6d7' + 'd8d9dadbdcdddedfe0e1e2e3e4e5e6e7e8e9eaebecedeeeff0f1f2f3f4f5f6f7f8f9fafbfcfdfeff') + l = 82 + + okm = hkdf_hmac_sha512(ikm, salt, info, l) + assert okm == bytes.fromhex('ce6c97192805b346e6161e821ed165673b84f400a2b514b2fe23d84cd189ddf1b695b48cbd1c838844' + '1137b3ce28f16aa64ba33ba466b24df6cfcb021ecff235f6a2056ce3af1de44d572097a8505d9e7a93') + + def test_hkdf_hmac_sha512_3(self): + ikm = bytes.fromhex('0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b') + salt = None + info = b'' + l = 42 + + okm = hkdf_hmac_sha512(ikm, salt, info, l) + assert okm == bytes.fromhex('f5fa02b18298a72a8c23898a8703472c6eb179dc204c03425c970e3b164bf90fff22d04836d0e2343bac') + + def test_hkdf_hmac_sha512_4(self): + ikm = bytes.fromhex('0b0b0b0b0b0b0b0b0b0b0b') + salt = bytes.fromhex('000102030405060708090a0b0c') + info = bytes.fromhex('f0f1f2f3f4f5f6f7f8f9') + l = 42 + + okm = hkdf_hmac_sha512(ikm, salt, info, l) + assert okm == bytes.fromhex('7413e8997e020610fbf6823f2ce14bff01875db1ca55f68cfcf3954dc8aff53559bd5e3028b080f7c068') + + def test_hkdf_hmac_sha512_5(self): + ikm = bytes.fromhex('0c0c0c0c0c0c0c0c0c0c0c0c0c0c0c0c0c0c0c0c0c0c') + salt = None + info = b'' + l = 42 + + okm = hkdf_hmac_sha512(ikm, salt, info, l) + assert okm == bytes.fromhex('1407d46013d98bc6decefcfee55f0f90b0c7f63d68eb1a80eaf07e953cfc0a3a5240a155d6e4daa965bb') diff --git a/borg/testsuite/helpers.py b/borg/testsuite/helpers.py index c6dadbecea..47d49f999e 100644 --- a/borg/testsuite/helpers.py +++ b/borg/testsuite/helpers.py @@ -11,7 +11,7 @@ import time from ..helpers import Location, format_file_size, format_timedelta, format_line, PlaceholderError, make_path_safe, \ - prune_within, prune_split, get_cache_dir, get_keys_dir, Statistics, is_slow_msgpack, \ + prune_within, prune_split, get_cache_dir, get_keys_dir, get_security_dir, Statistics, is_slow_msgpack, \ yes, TRUISH, FALSISH, DEFAULTISH, \ StableDict, int_to_bigint, bigint_to_int, parse_timestamp, CompressionSpec, ChunkerParams, \ ProgressIndicatorPercent, ProgressIndicatorEndless, load_excludes, parse_pattern, \ @@ -654,6 +654,18 @@ def test_get_keys_dir(monkeypatch): assert get_keys_dir() == '/var/tmp' +def test_get_security_dir(monkeypatch): + """test that get_security_dir respects environment""" + monkeypatch.delenv('BORG_SECURITY_DIR', raising=False) + monkeypatch.delenv('XDG_CONFIG_HOME', raising=False) + assert get_security_dir() == os.path.join(os.path.expanduser('~'), '.config', 'borg', 'security') + assert get_security_dir(repository_id='1234') == os.path.join(os.path.expanduser('~'), '.config', 'borg', 'security', '1234') + monkeypatch.setenv('XDG_CONFIG_HOME', '/var/tmp/.config') + assert get_security_dir() == os.path.join('/var/tmp/.config', 'borg', 'security') + monkeypatch.setenv('BORG_SECURITY_DIR', '/var/tmp') + assert get_security_dir() == '/var/tmp' + + @pytest.fixture() def stats(): stats = Statistics() diff --git a/borg/testsuite/key.py b/borg/testsuite/key.py index 4c57d1f025..2bb9d86fe9 100644 --- a/borg/testsuite/key.py +++ b/borg/testsuite/key.py @@ -4,9 +4,14 @@ import tempfile from binascii import hexlify, unhexlify +import msgpack + +import pytest + from ..crypto import bytes_to_long, num_aes_blocks from ..key import PlaintextKey, PassphraseKey, KeyfileKey -from ..helpers import Location +from ..key import UnsupportedManifestError, TAMRequiredError, TAMUnsupportedSuiteError, TAMInvalid +from ..helpers import Location, StableDict from . import BaseTestCase @@ -42,6 +47,9 @@ class MockRepository: class _Location: orig = '/some/place' + def canonical_path(self): + return self.orig + _location = _Location() id = bytes(32) @@ -101,3 +109,115 @@ def test_passphrase(self): data = b'foo' self.assert_equal(hexlify(key.id_hash(data)), b'818217cf07d37efad3860766dcdf1d21e401650fed2d76ed1d797d3aae925990') self.assert_equal(data, key2.decrypt(key2.id_hash(data), key.encrypt(data))) + + +class TestTAM: + @pytest.fixture + def key(self, monkeypatch): + monkeypatch.setenv('BORG_PASSPHRASE', 'test') + return KeyfileKey.create(KeyTestCase.MockRepository(), KeyTestCase.MockArgs()) + + def test_unpack_future(self, key): + blob = b'\xc1\xc1\xc1\xc1foobar' + with pytest.raises(UnsupportedManifestError): + key.unpack_and_verify_manifest(blob) + + blob = b'\xc1\xc1\xc1' + with pytest.raises(msgpack.UnpackException): + key.unpack_and_verify_manifest(blob) + + def test_missing_when_required(self, key): + blob = msgpack.packb({}) + with pytest.raises(TAMRequiredError): + key.unpack_and_verify_manifest(blob) + + def test_missing(self, key): + blob = msgpack.packb({}) + key.tam_required = False + unpacked, verified = key.unpack_and_verify_manifest(blob) + assert unpacked == {} + assert not verified + + def test_unknown_type_when_required(self, key): + blob = msgpack.packb({ + 'tam': { + 'type': 'HMAC_VOLLBIT', + }, + }) + with pytest.raises(TAMUnsupportedSuiteError): + key.unpack_and_verify_manifest(blob) + + def test_unknown_type(self, key): + blob = msgpack.packb({ + 'tam': { + 'type': 'HMAC_VOLLBIT', + }, + }) + key.tam_required = False + unpacked, verified = key.unpack_and_verify_manifest(blob) + assert unpacked == {} + assert not verified + + @pytest.mark.parametrize('tam, exc', ( + ({}, TAMUnsupportedSuiteError), + ({'type': b'\xff'}, TAMUnsupportedSuiteError), + (None, TAMInvalid), + (1234, TAMInvalid), + )) + def test_invalid(self, key, tam, exc): + blob = msgpack.packb({ + 'tam': tam, + }) + with pytest.raises(exc): + key.unpack_and_verify_manifest(blob) + + @pytest.mark.parametrize('hmac, salt', ( + ({}, bytes(64)), + (bytes(64), {}), + (None, bytes(64)), + (bytes(64), None), + )) + def test_wrong_types(self, key, hmac, salt): + data = { + 'tam': { + 'type': 'HKDF_HMAC_SHA512', + 'hmac': hmac, + 'salt': salt + }, + } + tam = data['tam'] + if hmac is None: + del tam['hmac'] + if salt is None: + del tam['salt'] + blob = msgpack.packb(data) + with pytest.raises(TAMInvalid): + key.unpack_and_verify_manifest(blob) + + def test_round_trip(self, key): + data = {'foo': 'bar'} + blob = key.pack_and_authenticate_metadata(data) + assert blob.startswith(b'\x82') + + unpacked = msgpack.unpackb(blob) + assert unpacked[b'tam'][b'type'] == b'HKDF_HMAC_SHA512' + + unpacked, verified = key.unpack_and_verify_manifest(blob) + assert verified + assert unpacked[b'foo'] == b'bar' + assert b'tam' not in unpacked + + @pytest.mark.parametrize('which', (b'hmac', b'salt')) + def test_tampered(self, key, which): + data = {'foo': 'bar'} + blob = key.pack_and_authenticate_metadata(data) + assert blob.startswith(b'\x82') + + unpacked = msgpack.unpackb(blob, object_hook=StableDict) + assert len(unpacked[b'tam'][which]) == 64 + unpacked[b'tam'][which] = unpacked[b'tam'][which][0:32] + bytes(32) + assert len(unpacked[b'tam'][which]) == 64 + blob = msgpack.packb(unpacked) + + with pytest.raises(TAMInvalid): + key.unpack_and_verify_manifest(blob) diff --git a/docs/changes.rst b/docs/changes.rst index 71677bcb3c..c24ea18efc 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -1,7 +1,62 @@ Important notes =============== -This section is used for infos about e.g. security and corruption issues. +This section is used for infos about security and corruption issues. + +.. _tam_vuln: + +Pre-1.0.9 manifest spoofing vulnerability +----------------------------------------- + +A flaw in the cryptographic authentication scheme in Borg allowed an attacker +to spoof the manifest. The attack requires an attacker to be able to + +1. insert files (with no additional headers) into backups +2. gain write access to the repository + +This vulnerability does not disclose plaintext to the attacker, nor does it +affect the authenticity of existing archives. + +The vulnerability allows an attacker to create a spoofed manifest (the list of archives). +Creating plausible fake archives may be feasible for small archives, but is unlikely +for large archives. + +The fix adds a separate authentication tag to the manifest. For compatibility +with prior versions this authentication tag is *not* required by default +for existing repositories. Repositories created with 1.0.9 and later require it. + +Steps you should take: + +1. Upgrade all clients to 1.0.9 or later. +2. Run ``borg upgrade --tam `` *on every client* for *each* repository. +3. This will list all archives, including archive IDs, for easy comparison with your logs. +4. Done. + +Prior versions can access and modify repositories with this measure enabled, however, +to 1.0.9 or later their modifications are indiscernible from an attack and will +raise an error until the below procedure is followed. We are aware that this can +be be annoying in some circumstances, but don't see a way to fix the vulnerability +otherwise. + +In case a version prior to 1.0.9 is used to modify a repository where above procedure +was completed, and now you get an error message from other clients: + +1. ``borg upgrade --tam --force `` once with *any* client suffices. + +This attack is mitigated by: + +- Noting/logging ``borg list``, ``borg info``, or ``borg create --stats``, which + contain the archive IDs. + +We are not aware of others having discovered, disclosed or exploited this vulnerability. + +Vulnerability time line: + +* 2016-11-14: Vulnerability and fix discovered during review of cryptography by Marian Beermann (@enkore) +* 2016-11-20: First patch +* 2016-12-18: Released fixed versions: 1.0.9, 1.1.0b3 + +.. _attic013_check_corruption: Pre-1.0.9 potential data loss ----------------------------- @@ -71,8 +126,14 @@ The best check that everything is ok is to run a dry-run extraction:: Changelog ========= -Version 1.0.9 (not released yet) --------------------------------- +Version 1.0.9 (2016-12-18) +-------------------------- + +Security fixes: + +- A flaw in the cryptographic authentication scheme in Borg allowed an attacker + to spoof the manifest. See :ref:`tam_vuln` above for the steps you should + take. Bug fixes: @@ -96,7 +157,7 @@ Other changes: - markup fixes - tests: - - test_get_(cache|keys)_dir: clean env state, #1897 + - test_get\_(cache|keys)_dir: clean env state, #1897 - get back pytest's pretty assertion failures, #1938 - setup.py build_usage: diff --git a/docs/usage.rst b/docs/usage.rst index cb034394eb..ec1a790f6c 100644 --- a/docs/usage.rst +++ b/docs/usage.rst @@ -181,6 +181,9 @@ Some automatic "answerers" (if set, they automatically answer confirmation quest Directories: BORG_KEYS_DIR Default to '~/.config/borg/keys'. This directory contains keys for encrypted repositories. + BORG_SECURITY_DIR + Default to '~/.config/borg/security'. This directory is used by Borg to track various + pieces of security-related data. BORG_CACHE_DIR Default to '~/.cache/borg'. This directory contains the local cache and might need a lot of space for dealing with big repositories).