From af923e261bacbaf0bfb4be77a57d3604cf7e9291 Mon Sep 17 00:00:00 2001 From: enkore Date: Sat, 17 Dec 2016 15:23:47 +0100 Subject: [PATCH 01/12] conftest: pytest 2 compat --- conftest.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/conftest.py b/conftest.py index 197af6fd1..596a4b21f 100644 --- a/conftest.py +++ b/conftest.py @@ -5,7 +5,8 @@ import sys import pytest # needed to get pretty assertion failures in unit tests: -pytest.register_assert_rewrite('borg.testsuite') +if hasattr(pytest, 'register_assert_rewrite'): + pytest.register_assert_rewrite('borg.testsuite') # This is a hack to fix path problems because "borg" (the package) is in the source root. # When importing the conftest an "import borg" can incorrectly import the borg from the From e28b470cfb72a99528e8f665ca89332248d0918d Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Sat, 17 Dec 2016 18:00:03 +0100 Subject: [PATCH 02/12] Fix subsubparsers for Python <3.4.3 This works around http://bugs.python.org/issue9351 Since Debian and Ubuntu ship 3.4.2 and 3.4.0 respectively. --- borg/archiver.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/borg/archiver.py b/borg/archiver.py index 5ee612972..2cdfe37f3 100644 --- a/borg/archiver.py +++ b/borg/archiver.py @@ -1160,7 +1160,7 @@ class Archiver: help='manage repository key') key_parsers = subparser.add_subparsers(title='required arguments', metavar='') - subparser.set_defaults(func=functools.partial(self.do_subcommand_help, subparser)) + subparser.set_defaults(fallback_func=functools.partial(self.do_subcommand_help, subparser)) key_export_epilog = textwrap.dedent(""" If repository encryption is used, the repository is inaccessible @@ -1694,7 +1694,7 @@ class Archiver: help='debugging command (not intended for normal use)') debug_parsers = subparser.add_subparsers(title='required arguments', metavar='') - subparser.set_defaults(func=functools.partial(self.do_subcommand_help, subparser)) + subparser.set_defaults(fallback_func=functools.partial(self.do_subcommand_help, subparser)) debug_info_epilog = textwrap.dedent(""" This command displays some system information that might be useful for bug @@ -1902,11 +1902,13 @@ class Archiver: def run(self, args): os.umask(args.umask) # early, before opening files self.lock_wait = args.lock_wait - setup_logging(level=args.log_level, is_serve=args.func == self.do_serve) # do not use loggers before this! + # This works around http://bugs.python.org/issue9351 + func = getattr(args, 'func', None) or getattr(args, 'fallback_func') + setup_logging(level=args.log_level, is_serve=func == self.do_serve) # do not use loggers before this! check_extension_modules() if is_slow_msgpack(): logger.warning("Using a pure-python msgpack! This will result in lower performance.") - return args.func(args) + return func(args) def sig_info_handler(sig_no, stack): # pragma: no cover From 5e1cb9d89963325b44f270554fdc982ca87cce8a Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Sat, 17 Dec 2016 00:51:25 +0100 Subject: [PATCH 03/12] Add tertiary authentication for metadata (TAM) --- docs/changes.rst | 69 ++++++++++++++++- setup.py | 2 +- src/borg/archive.py | 4 +- src/borg/archiver.py | 84 +++++++++++++++++---- src/borg/crypto.pyx | 33 ++++++++- src/borg/helpers.py | 30 +++++--- src/borg/item.pyx | 3 +- src/borg/key.py | 132 ++++++++++++++++++++++++++++++--- src/borg/selftest.py | 2 +- src/borg/testsuite/archiver.py | 69 ++++++++++++++++- src/borg/testsuite/crypto.py | 54 +++++++++++++- src/borg/testsuite/helpers.py | 1 + src/borg/testsuite/key.py | 119 ++++++++++++++++++++++++++++- 13 files changed, 550 insertions(+), 52 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index e6ebcb45c..928a9c2e2 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/setup.py b/setup.py index 6ef6d543b..c6e27f4d6 100644 --- a/setup.py +++ b/setup.py @@ -109,7 +109,7 @@ except ImportError: platform_darwin_source = platform_darwin_source.replace('.pyx', '.c') from distutils.command.build_ext import build_ext if not on_rtd and not all(os.path.exists(path) for path in [ - compress_source, crypto_source, chunker_source, hashindex_source, + compress_source, crypto_source, chunker_source, hashindex_source, item_source, platform_posix_source, platform_linux_source, platform_freebsd_source, platform_darwin_source]): raise ImportError('The GIT version of Borg needs Cython. Install Cython or use a released version.') diff --git a/src/borg/archive.py b/src/borg/archive.py index 2c7dfa964..3793e7e7f 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -307,7 +307,7 @@ class Archive: def _load_meta(self, id): _, data = self.key.decrypt(id, self.repository.get(id)) - metadata = ArchiveItem(internal_dict=msgpack.unpackb(data)) + metadata = ArchiveItem(internal_dict=msgpack.unpackb(data, unicode_errors='surrogateescape')) if metadata.version != 1: raise Exception('Unknown archive metadata version') return metadata @@ -409,7 +409,7 @@ Number of files: {0.stats.nfiles}'''.format( } metadata.update(additional_metadata or {}) metadata = ArchiveItem(metadata) - data = msgpack.packb(metadata.as_dict(), unicode_errors='surrogateescape') + data = self.key.pack_and_authenticate_metadata(metadata.as_dict(), context=b'archive') self.id = self.key.id_hash(data) self.cache.add_chunk(self.id, Chunk(data), self.stats) self.manifest.archives[name] = (self.id, metadata.time) diff --git a/src/borg/archiver.py b/src/borg/archiver.py index f6017e130..cfe497b3b 100644 --- a/src/borg/archiver.py +++ b/src/borg/archiver.py @@ -45,7 +45,7 @@ from .helpers import signal_handler, raising_signal_handler, SigHup, SigTerm from .helpers import ErrorIgnoringTextIOWrapper from .helpers import ProgressIndicatorPercent from .item import Item -from .key import key_creator, RepoKey, PassphraseKey +from .key import key_creator, tam_required_file, tam_required, RepoKey, PassphraseKey from .keymanager import KeyManager from .platform import get_flags, umount from .remote import RepositoryServer, RemoteRepository, cache_if_remote @@ -64,7 +64,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, …) @@ -81,7 +81,7 @@ def with_repository(fake=False, create=False, lock=True, exclusive=False, manife 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), @@ -194,6 +194,8 @@ class Archiver: 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) @@ -224,6 +226,7 @@ class Archiver: 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) @@ -272,6 +275,7 @@ class Archiver: 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) @@ -1046,21 +1050,43 @@ class Archiver: DASHES, logger=logging.getLogger('borg.output.stats')) 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) - 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) + 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.archives.list(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) return self.exit_code @with_repository(cache=True, exclusive=True) @@ -2303,6 +2329,28 @@ class Archiver: 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. @@ -2355,6 +2403,10 @@ class Archiver: 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/src/borg/crypto.pyx b/src/borg/crypto.pyx index 7fa8b8913..c3cda4ac3 100644 --- a/src/borg/crypto.pyx +++ b/src/borg/crypto.pyx @@ -1,9 +1,13 @@ """A thin OpenSSL wrapper""" +import hashlib +import hmac +from math import ceil + from libc.stdlib cimport malloc, free from cpython.buffer cimport PyBUF_SIMPLE, PyObject_GetBuffer, PyBuffer_Release -API_VERSION = 3 +API_VERSION = 4 cdef extern from "blake2-libselect.h": @@ -247,3 +251,30 @@ def blake2b_256(key, data): raise Exception('blake2b_final() failed') return md + + +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/src/borg/helpers.py b/src/borg/helpers.py index 0c98f3a38..321202517 100644 --- a/src/borg/helpers.py +++ b/src/borg/helpers.py @@ -93,7 +93,7 @@ def check_extension_modules(): raise ExtensionModuleError if compress.API_VERSION != 2: raise ExtensionModuleError - if crypto.API_VERSION != 3: + if crypto.API_VERSION != 4: raise ExtensionModuleError if platform.API_VERSION != platform.OS_API_VERSION != 5: raise ExtensionModuleError @@ -192,15 +192,16 @@ class Manifest: 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 @property def id_str(self): return bin_to_hex(self.id) @classmethod - def load(cls, repository, key=None): + def load(cls, repository, key=None, force_tam_not_required=False): from .item import ManifestItem - from .key import key_factory + from .key import key_factory, tam_required_file, tam_required from .repository import Repository try: cdata = repository.get(cls.MANIFEST_ID) @@ -209,9 +210,10 @@ class Manifest: if not key: key = key_factory(repository, cdata) manifest = cls(key, repository) - _, data = key.decrypt(None, cdata) + data = key.decrypt(None, cdata).data + manifest_dict, manifest.tam_verified = key.unpack_and_verify_manifest(data, force_tam_not_required=force_tam_not_required) + m = ManifestItem(internal_dict=manifest_dict) manifest.id = key.id_hash(data) - m = ManifestItem(internal_dict=msgpack.unpackb(data)) if m.get('version') != 1: raise ValueError('Invalid manifest version') manifest.archives.set_raw_dict(m.archives) @@ -219,21 +221,28 @@ class Manifest: manifest.config = m.config # valid item keys are whatever is known in the repo or every key we know manifest.item_keys = ITEM_KEYS | frozenset(key.decode() for key in m.get('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): from .item import ManifestItem + if self.key.tam_required: + self.config[b'tam_required'] = True self.timestamp = datetime.utcnow().isoformat() manifest = ManifestItem( version=1, - archives=self.archives.get_raw_dict(), + archives=StableDict(self.archives.get_raw_dict()), timestamp=self.timestamp, - config=self.config, - item_keys=tuple(self.item_keys), + config=StableDict(self.config), + item_keys=tuple(sorted(self.item_keys)), ) - data = msgpack.packb(manifest.as_dict()) + self.tam_verified = True + data = self.key.pack_and_authenticate_metadata(manifest.as_dict()) self.id = self.key.id_hash(data) - self.repository.put(self.MANIFEST_ID, self.key.encrypt(Chunk(data))) + self.repository.put(self.MANIFEST_ID, self.key.encrypt(Chunk(data, compression={'name': 'none'}))) def prune_within(archives, within): @@ -292,7 +301,6 @@ def get_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(get_home_dir(), '.config')) security_dir = os.environ.get('BORG_SECURITY_DIR', os.path.join(xdg_config, 'borg', 'security')) if repository_id: diff --git a/src/borg/item.pyx b/src/borg/item.pyx index 802322a87..bdcb9a539 100644 --- a/src/borg/item.pyx +++ b/src/borg/item.pyx @@ -213,7 +213,7 @@ class Key(PropDict): If a Key shall be serialized, give as_dict() method output to msgpack packer. """ - VALID_KEYS = {'version', 'repository_id', 'enc_key', 'enc_hmac_key', 'id_key', 'chunk_seed'} # str-typed keys + VALID_KEYS = {'version', 'repository_id', 'enc_key', 'enc_hmac_key', 'id_key', 'chunk_seed', 'tam_required'} # str-typed keys __slots__ = ("_dict", ) # avoid setting attributes not supported by properties @@ -223,6 +223,7 @@ class Key(PropDict): enc_hmac_key = PropDict._make_property('enc_hmac_key', bytes) id_key = PropDict._make_property('id_key', bytes) chunk_seed = PropDict._make_property('chunk_seed', int) + tam_required = PropDict._make_property('tam_required', bool) class ArchiveItem(PropDict): diff --git a/src/borg/key.py b/src/borg/key.py index 3a8168db0..a03d21c15 100644 --- a/src/borg/key.py +++ b/src/borg/key.py @@ -4,8 +4,8 @@ import os import sys import textwrap from binascii import a2b_base64, b2a_base64, hexlify, unhexlify -from hashlib import sha256, pbkdf2_hmac -from hmac import compare_digest +from hashlib import sha256, sha512, pbkdf2_hmac +from hmac import HMAC, compare_digest import msgpack @@ -14,11 +14,11 @@ logger = create_logger() from .constants import * # NOQA from .compress import Compressor, get_compressor -from .crypto import AES, bytes_to_long, bytes_to_int, num_aes_blocks, hmac_sha256, blake2b_256 -from .helpers import Chunk +from .crypto import AES, bytes_to_long, bytes_to_int, num_aes_blocks, hmac_sha256, blake2b_256, hkdf_hmac_sha512 +from .helpers import Chunk, StableDict from .helpers import Error, IntegrityError from .helpers import yes -from .helpers import get_keys_dir +from .helpers import get_keys_dir, get_security_dir from .helpers import bin_to_hex from .helpers import CompressionDecider2, CompressionSpec from .item import Key, EncryptedKey @@ -41,6 +41,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 {}.""" @@ -57,6 +61,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) @@ -94,6 +124,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 @@ -103,11 +143,11 @@ class KeyBase: self.target = None # key location file path / repo obj self.compression_decider2 = CompressionDecider2(CompressionSpec('none')) self.compressor = Compressor('none') # for decompression + self.tam_required = True def id_hash(self, data): """Return HMAC hash using the "id" HMAC key """ - def compress(self, chunk): compr_args, chunk = self.compression_decider2.decide(chunk) compressor = Compressor(**compr_args) @@ -127,6 +167,68 @@ class KeyBase: if not compare_digest(id_computed, id): raise IntegrityError('Chunk %s: id verification failed' % bin_to_hex(id)) + 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 @@ -134,6 +236,10 @@ class PlaintextKey(KeyBase): 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.') @@ -161,6 +267,9 @@ class PlaintextKey(KeyBase): self.assert_id(id, data) return Chunk(data) + def _tam_key(self, salt, context): + return salt + context + def random_blake2b_256_key(): # This might look a bit curious, but is the same construction used in the keyed mode of BLAKE2b. @@ -373,6 +482,7 @@ class PassphraseKey(ID_HMAC_SHA_256, AESKeyBase): key.decrypt(None, manifest_data) num_blocks = num_aes_blocks(len(manifest_data) - 41) key.init_ciphers(key.extract_nonce(manifest_data) + num_blocks) + key._passphrase = passphrase return key except IntegrityError: passphrase = Passphrase.getpass(prompt) @@ -388,6 +498,7 @@ class PassphraseKey(ID_HMAC_SHA_256, AESKeyBase): 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): @@ -411,6 +522,7 @@ class KeyfileKeyBase(AESKeyBase): raise PassphraseWrong num_blocks = num_aes_blocks(len(manifest_data) - 41) key.init_ciphers(key.extract_nonce(manifest_data) + num_blocks) + key._passphrase = passphrase return key def find_key(self): @@ -432,6 +544,7 @@ class KeyfileKeyBase(AESKeyBase): self.enc_hmac_key = key.enc_hmac_key self.id_key = key.id_key self.chunk_seed = key.chunk_seed + self.tam_required = key.get('tam_required', tam_required(self.repository)) return True return False @@ -469,15 +582,16 @@ class KeyfileKeyBase(AESKeyBase): 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.as_dict()), 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/src/borg/selftest.py b/src/borg/selftest.py index 40d8d06e9..d2ea9a769 100644 --- a/src/borg/selftest.py +++ b/src/borg/selftest.py @@ -30,7 +30,7 @@ SELFTEST_CASES = [ ChunkerTestCase, ] -SELFTEST_COUNT = 30 +SELFTEST_COUNT = 35 class SelfTestResult(TestResult): diff --git a/src/borg/testsuite/archiver.py b/src/borg/testsuite/archiver.py index 68c8f9e24..a9c2c7ba2 100644 --- a/src/borg/testsuite/archiver.py +++ b/src/borg/testsuite/archiver.py @@ -3,6 +3,8 @@ from configparser import ConfigParser import errno import os import inspect +from datetime import datetime +from datetime import timedelta from io import StringIO import logging import random @@ -17,6 +19,7 @@ import unittest from unittest.mock import patch from hashlib import sha256 +import msgpack import pytest try: import llfuse @@ -34,7 +37,7 @@ from ..helpers import Chunk, Manifest from ..helpers import EXIT_SUCCESS, EXIT_WARNING, EXIT_ERROR from ..helpers import bin_to_hex from ..item import Item -from ..key import KeyfileKeyBase, RepoKey, KeyfileKey, Passphrase +from ..key import KeyfileKeyBase, RepoKey, KeyfileKey, Passphrase, TAMRequiredError from ..keymanager import RepoIdMismatch, NotABorgKeyFile from ..remote import RemoteRepository, PathNotAllowed from ..repository import Repository @@ -42,6 +45,7 @@ from . import has_lchflags, has_llfuse from . import BaseTestCase, changedir, environment_variable, no_selinux from . import are_symlinks_supported, are_hardlinks_supported, are_fifos_supported, is_utime_fully_supported from .platform import fakeroot_detected +from . import key src_dir = os.path.abspath(os.path.join(os.path.dirname(__file__), '..')) @@ -1645,8 +1649,8 @@ class ArchiverTestCase(ArchiverTestCaseBase): 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) @@ -1947,7 +1951,7 @@ class ArchiverTestCase(ArchiverTestCaseBase): repo_key = RepoKey(repository) repo_key.load(None, Passphrase.env_passphrase()) - backup_key = KeyfileKey(None) + backup_key = KeyfileKey(key.TestKey.MockRepository()) backup_key.load(export_file, Passphrase.env_passphrase()) assert repo_key.enc_key == backup_key.enc_key @@ -2251,6 +2255,63 @@ class ArchiverCheckTestCase(ArchiverTestCaseBase): self.cmd('list', self.repository_location + '::0.13', 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(Chunk(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)).data) + del manifest[b'tam'] + repository.put(Manifest.MANIFEST_ID, key.encrypt(Chunk(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(Chunk(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/src/borg/testsuite/crypto.py b/src/borg/testsuite/crypto.py index aa138a763..92cb06a4c 100644 --- a/src/borg/testsuite/crypto.py +++ b/src/borg/testsuite/crypto.py @@ -2,7 +2,7 @@ from binascii import hexlify, unhexlify from ..crypto import AES, bytes_to_long, bytes_to_int, long_to_bytes, hmac_sha256, blake2b_256 from ..crypto import increment_iv, bytes16_to_int, int_to_bytes16 - +from ..crypto import hkdf_hmac_sha512 from . import BaseTestCase # Note: these tests are part of the self test, do not use or import py.test functionality here. @@ -96,3 +96,55 @@ class CryptoTestCase(BaseTestCase): key = unhexlify('e944973af2256d4d670c12dd75304c319f58f4e40df6fb18ef996cb47e063676') data = memoryview(b'1234567890' * 100) assert blake2b_256(key, data) == unhexlify('97ede832378531dd0f4c668685d166e797da27b47d8cd441e885b60abd5e0cb2') + + # 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/src/borg/testsuite/helpers.py b/src/borg/testsuite/helpers.py index 1bc1ad7cc..6277d98d1 100644 --- a/src/borg/testsuite/helpers.py +++ b/src/borg/testsuite/helpers.py @@ -659,6 +659,7 @@ def test_get_keys_dir(monkeypatch): 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') diff --git a/src/borg/testsuite/key.py b/src/borg/testsuite/key.py index 99e3a6c8e..6d916a28e 100644 --- a/src/borg/testsuite/key.py +++ b/src/borg/testsuite/key.py @@ -5,14 +5,16 @@ import os.path from binascii import hexlify, unhexlify import pytest +import msgpack from ..crypto import bytes_to_long, num_aes_blocks from ..helpers import Location -from ..helpers import Chunk +from ..helpers import Chunk, StableDict from ..helpers import IntegrityError from ..helpers import get_security_dir from ..key import PlaintextKey, PassphraseKey, KeyfileKey, RepoKey, Blake2KeyfileKey, Blake2RepoKey, AuthenticatedKey from ..key import Passphrase, PasswordRetriesExceeded, bin_to_hex +from ..key import TAMRequiredError, TAMInvalid, TAMUnsupportedSuiteError, UnsupportedManifestError class TestKey: @@ -74,6 +76,9 @@ class TestKey: class _Location: orig = '/some/place' + def canonical_path(self): + return self.orig + _location = _Location() id = bytes(32) id_str = bin_to_hex(id) @@ -277,3 +282,115 @@ class TestPassphrase: def test_passphrase_repr(self): assert "secret" not in repr(Passphrase("secret")) + + +class TestTAM: + @pytest.fixture + def key(self, monkeypatch): + monkeypatch.setenv('BORG_PASSPHRASE', 'test') + return KeyfileKey.create(TestKey.MockRepository(), TestKey.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) From d15fb241bd2ca701bc52e3a9c2925cff21b6b238 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Sat, 17 Dec 2016 01:48:33 +0100 Subject: [PATCH 04/12] check: handle duplicate archive items neatly # Conflicts: # src/borg/archive.py --- docs/changes.rst | 3 +++ src/borg/archive.py | 14 ++++++++++++-- src/borg/testsuite/archiver.py | 27 +++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 928a9c2e2..7da0c2f1d 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -134,6 +134,9 @@ 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. +- borg check: When rebuilding the manifest (which should only be needed very rarely) + duplicate archive names would be handled on a "first come first serve" basis, allowing + an attacker to apparently replace archives. Bug fixes: diff --git a/src/borg/archive.py b/src/borg/archive.py index 3793e7e7f..90759dae8 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -1197,8 +1197,18 @@ class ArchiveChecker: continue if valid_archive(archive): archive = ArchiveItem(internal_dict=archive) - logger.info('Found archive %s', archive.name) - manifest.archives[archive.name] = (chunk_id, archive.time) + name = archive.name + logger.info('Found archive %s', name) + if name in manifest.archives: + i = 1 + while True: + new_name = '%s.%d' % (name, i) + if new_name not in manifest.archives: + break + i += 1 + logger.warning('Duplicate archive name %s, storing as %s', name, new_name) + name = new_name + manifest.archives[name] = (chunk_id, archive.time) logger.info('Manifest rebuild complete.') return manifest diff --git a/src/borg/testsuite/archiver.py b/src/borg/testsuite/archiver.py index a9c2c7ba2..5637446cc 100644 --- a/src/borg/testsuite/archiver.py +++ b/src/borg/testsuite/archiver.py @@ -2180,6 +2180,33 @@ class ArchiverCheckTestCase(ArchiverTestCaseBase): self.assert_in('archive2', output) self.cmd('check', self.repository_location, exit_code=0) + def test_manifest_rebuild_duplicate_archive(self): + archive, repository = self.open_archive('archive1') + key = archive.key + with repository: + manifest = repository.get(Manifest.MANIFEST_ID) + corrupted_manifest = manifest + b'corrupted!' + repository.put(Manifest.MANIFEST_ID, corrupted_manifest) + + archive = msgpack.packb({ + 'cmdline': [], + 'items': [], + 'hostname': 'foo', + 'username': 'bar', + 'name': 'archive1', + 'time': '2016-12-15T18:49:51.849711', + 'version': 1, + }) + archive_id = key.id_hash(archive) + repository.put(archive_id, key.encrypt(Chunk(archive))) + repository.commit() + self.cmd('check', self.repository_location, exit_code=1) + self.cmd('check', '--repair', self.repository_location, exit_code=0) + output = self.cmd('list', self.repository_location) + self.assert_in('archive1', output) + self.assert_in('archive1.1', output) + self.assert_in('archive2', output) + def test_extra_chunks(self): self.cmd('check', self.repository_location, exit_code=0) with Repository(self.repository_location, exclusive=True) as repository: From c7c8c0fb57b54c0b87ffba5691e89938cdaa6cec Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Sun, 18 Dec 2016 23:30:34 +0100 Subject: [PATCH 05/12] init: explain manifest auth compatibility # Conflicts: # src/borg/archiver.py --- src/borg/archiver.py | 18 +++++++++++++++--- src/borg/key.py | 2 +- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/borg/archiver.py b/src/borg/archiver.py index cfe497b3b..55b20ad31 100644 --- a/src/borg/archiver.py +++ b/src/borg/archiver.py @@ -182,7 +182,8 @@ class Archiver: @with_repository(create=True, exclusive=True, manifest=False) def do_init(self, args, repository): """Initialize an empty repository""" - logger.info('Initializing repository at "%s"' % args.location.canonical_path()) + path = args.location.canonical_path() + logger.info('Initializing repository at "%s"' % path) try: key = key_creator(repository, args) except (EOFError, KeyboardInterrupt): @@ -194,8 +195,19 @@ class Archiver: repository.commit() with Cache(repository, key, manifest, warn_if_unencrypted=False): pass - tam_file = tam_required_file(repository) - open(tam_file, 'w').close() + if key.tam_required: + tam_file = tam_required_file(repository) + open(tam_file, 'w').close() + logger.warning( + '\n' + 'By default repositories initialized with this version will produce security\n' + 'errors if written to with an older version (up to and including Borg 1.0.8).\n' + '\n' + 'If you want to use these older versions, you can disable the check by runnning:\n' + 'borg upgrade --disable-tam \'%s\'\n' + '\n' + 'See https://borgbackup.readthedocs.io/en/stable/changes.html#pre-1-0-9-manifest-spoofing-vulnerability ' + 'for details about the security implications.', path) return self.exit_code @with_repository(exclusive=True, manifest=False) diff --git a/src/borg/key.py b/src/borg/key.py index a03d21c15..a623dd940 100644 --- a/src/borg/key.py +++ b/src/borg/key.py @@ -63,7 +63,7 @@ class RepoKeyNotFoundError(Error): class TAMRequiredError(IntegrityError): __doc__ = textwrap.dedent(""" - Manifest is unauthenticated, but authentication is required for this repository. + Manifest is unauthenticated, but it 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. From df40b3840c56501754d3bf855b3502ff6368da54 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Sun, 18 Dec 2016 23:32:42 +0100 Subject: [PATCH 06/12] upgrade: --disable-tam # Conflicts: # src/borg/helpers.py # src/borg/testsuite/archiver.py --- src/borg/archiver.py | 28 ++++++++++++++++++++++--- src/borg/helpers.py | 15 ++++++++++---- src/borg/testsuite/archiver.py | 37 +++++++++++++++++++++++++--------- 3 files changed, 64 insertions(+), 16 deletions(-) diff --git a/src/borg/archiver.py b/src/borg/archiver.py index 55b20ad31..1c650f05c 100644 --- a/src/borg/archiver.py +++ b/src/borg/archiver.py @@ -61,6 +61,8 @@ def argument(args, str_or_bool): """If bool is passed, return it. If str is passed, retrieve named attribute from args.""" if isinstance(str_or_bool, str): return getattr(args, str_or_bool) + if isinstance(str_or_bool, (list, tuple)): + return any(getattr(args, item) for item in str_or_bool) return str_or_bool @@ -1062,29 +1064,43 @@ class Archiver: DASHES, logger=logging.getLogger('borg.output.stats')) return self.exit_code - @with_repository(fake='tam', invert_fake=True, manifest=False, exclusive=True) + @with_repository(fake=('tam', 'disable_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""" if args.tam: manifest, key = Manifest.load(repository, force_tam_not_required=args.force) - if not manifest.tam_verified: + if not manifest.tam_verified or not manifest.config.get(b'tam_required', False): # The standard archive listing doesn't include the archive ID like in borg 1.1.x print('Manifest contents:') for archive_info in manifest.archives.list(sort_by=['ts']): print(format_archive(archive_info), '[%s]' % bin_to_hex(archive_info.id)) + manifest.config[b'tam_required'] = True manifest.write() repository.commit() if not key.tam_required: key.tam_required = True key.change_passphrase(key._passphrase) - print('Updated key') + print('Key updated') 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') + elif args.disable_tam: + manifest, key = Manifest.load(repository, force_tam_not_required=True) + if tam_required(repository): + os.unlink(tam_required_file(repository)) + if key.tam_required: + key.tam_required = False + key.change_passphrase(key._passphrase) + print('Key updated') + if hasattr(key, 'find_key'): + print('Key location:', key.find_key()) + manifest.config[b'tam_required'] = False + manifest.write() + repository.commit() else: # mainly for upgrades from Attic repositories, # but also supports borg 0.xx -> 1.0 upgrade. @@ -2356,6 +2372,10 @@ class Archiver: 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. + If you routinely do this you might not want to enable this upgrade + (which will leave you exposed to the security issue). You can + reverse the upgrade by issuing ``borg upgrade --disable-tam REPO``. + See https://borgbackup.readthedocs.io/en/stable/changes.html#pre-1-0-9-manifest-spoofing-vulnerability for details. @@ -2419,6 +2439,8 @@ class Archiver: 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('--disable-tam', dest='disable_tam', action='store_true', + help="""Disable manifest authentication (in key and cache)""") subparser.add_argument('location', metavar='REPOSITORY', nargs='?', default='', type=location_validator(archive=False), help='path to the repository to be upgraded') diff --git a/src/borg/helpers.py b/src/borg/helpers.py index 321202517..9b10a984d 100644 --- a/src/borg/helpers.py +++ b/src/borg/helpers.py @@ -221,10 +221,17 @@ class Manifest: manifest.config = m.config # valid item keys are whatever is known in the repo or every key we know manifest.item_keys = ITEM_KEYS | frozenset(key.decode() for key in m.get('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() + + if manifest.tam_verified: + manifest_required = manifest.config.get(b'tam_required', False) + security_required = tam_required(repository) + if manifest_required and not security_required: + logger.debug('Manifest is TAM verified and says TAM is required, updating security database...') + file = tam_required_file(repository) + open(file, 'w').close() + if not manifest_required and security_required: + logger.debug('Manifest is TAM verified and says TAM is *not* required, updating security database...') + os.unlink(tam_required_file(repository)) return manifest, key def write(self): diff --git a/src/borg/testsuite/archiver.py b/src/borg/testsuite/archiver.py index 5637446cc..b3c702d7c 100644 --- a/src/borg/testsuite/archiver.py +++ b/src/borg/testsuite/archiver.py @@ -2283,6 +2283,17 @@ class ArchiverCheckTestCase(ArchiverTestCaseBase): class ManifestAuthenticationTest(ArchiverTestCaseBase): + def spoof_manifest(self, repository): + with repository: + _, key = Manifest.load(repository) + repository.put(Manifest.MANIFEST_ID, key.encrypt(Chunk(msgpack.packb({ + 'version': 1, + 'archives': {}, + 'config': {}, + 'timestamp': (datetime.utcnow() + timedelta(days=1)).isoformat(), + })))) + repository.commit() + def test_fresh_init_tam_required(self): self.cmd('init', self.repository_location) repository = Repository(self.repository_path, exclusive=True) @@ -2322,15 +2333,7 @@ class ManifestAuthenticationTest(ArchiverTestCaseBase): 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(Chunk(msgpack.packb({ - 'version': 1, - 'archives': {}, - 'config': {}, - 'timestamp': (datetime.utcnow() + timedelta(days=1)).isoformat(), - })))) - repository.commit() + self.spoof_manifest(repository) # Fails with pytest.raises(TAMRequiredError): self.cmd('list', self.repository_location) @@ -2338,6 +2341,22 @@ class ManifestAuthenticationTest(ArchiverTestCaseBase): self.cmd('upgrade', '--tam', '--force', self.repository_location) self.cmd('list', self.repository_location) + def test_disable(self): + self.cmd('init', self.repository_location) + self.create_src_archive('archive1234') + self.cmd('upgrade', '--disable-tam', self.repository_location) + repository = Repository(self.repository_path, exclusive=True) + self.spoof_manifest(repository) + assert not self.cmd('list', self.repository_location) + + def test_disable2(self): + self.cmd('init', self.repository_location) + self.create_src_archive('archive1234') + repository = Repository(self.repository_path, exclusive=True) + self.spoof_manifest(repository) + self.cmd('upgrade', '--disable-tam', self.repository_location) + assert not self.cmd('list', self.repository_location) + @pytest.mark.skipif(sys.platform == 'cygwin', reason='remote is broken on cygwin and hangs') class RemoteArchiverTestCase(ArchiverTestCase): From 91991988e10b08fbc5d0517b4c6079a6b0462e74 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Sat, 17 Dec 2016 18:00:03 +0100 Subject: [PATCH 07/12] Fix subsubparsers for Python <3.4.3 This works around http://bugs.python.org/issue9351 Since Debian and Ubuntu ship 3.4.2 and 3.4.0 respectively. --- borg/archiver.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/borg/archiver.py b/borg/archiver.py index 5ee612972..2cdfe37f3 100644 --- a/borg/archiver.py +++ b/borg/archiver.py @@ -1160,7 +1160,7 @@ class Archiver: help='manage repository key') key_parsers = subparser.add_subparsers(title='required arguments', metavar='') - subparser.set_defaults(func=functools.partial(self.do_subcommand_help, subparser)) + subparser.set_defaults(fallback_func=functools.partial(self.do_subcommand_help, subparser)) key_export_epilog = textwrap.dedent(""" If repository encryption is used, the repository is inaccessible @@ -1694,7 +1694,7 @@ class Archiver: help='debugging command (not intended for normal use)') debug_parsers = subparser.add_subparsers(title='required arguments', metavar='') - subparser.set_defaults(func=functools.partial(self.do_subcommand_help, subparser)) + subparser.set_defaults(fallback_func=functools.partial(self.do_subcommand_help, subparser)) debug_info_epilog = textwrap.dedent(""" This command displays some system information that might be useful for bug @@ -1902,11 +1902,13 @@ class Archiver: def run(self, args): os.umask(args.umask) # early, before opening files self.lock_wait = args.lock_wait - setup_logging(level=args.log_level, is_serve=args.func == self.do_serve) # do not use loggers before this! + # This works around http://bugs.python.org/issue9351 + func = getattr(args, 'func', None) or getattr(args, 'fallback_func') + setup_logging(level=args.log_level, is_serve=func == self.do_serve) # do not use loggers before this! check_extension_modules() if is_slow_msgpack(): logger.warning("Using a pure-python msgpack! This will result in lower performance.") - return args.func(args) + return func(args) def sig_info_handler(sig_no, stack): # pragma: no cover From 880578da064f1ddcd53fdee2c356529c75bcbca7 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Mon, 19 Dec 2016 16:06:54 +0100 Subject: [PATCH 08/12] quickstart: use prune with --list so people are better aware of what's happening, avoiding pitfalls with wrong or missing --prefix. --- docs/quickstart.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/quickstart.rst b/docs/quickstart.rst index 78966eb5e..58ce6c96f 100644 --- a/docs/quickstart.rst +++ b/docs/quickstart.rst @@ -126,7 +126,7 @@ certain number of old archives:: # archives of THIS machine. The '{hostname}-' prefix is very important to # limit prune's operation to this machine's archives and not apply to # other machine's archives also. - borg prune -v $REPOSITORY --prefix '{hostname}-' \ + borg prune -v --list $REPOSITORY --prefix '{hostname}-' \ --keep-daily=7 --keep-weekly=4 --keep-monthly=6 Pitfalls with shell variables and environment variables From c9cc97e05be23d2af3decfa89977d2bdbcfec488 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Tue, 20 Dec 2016 00:49:24 +0100 Subject: [PATCH 09/12] CHANGES: fix 1.0.9 release date --- docs/changes.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changes.rst b/docs/changes.rst index 7da0c2f1d..4208dad43 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -126,7 +126,7 @@ The best check that everything is ok is to run a dry-run extraction:: Changelog ========= -Version 1.0.9 (2016-12-18) +Version 1.0.9 (2016-12-20) -------------------------- Security fixes: From e0e5bc4aa448b2cbe400275d960b6dda70a522f2 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Tue, 20 Dec 2016 23:09:28 +0100 Subject: [PATCH 10/12] ran build_usage --- docs/usage/upgrade.rst.inc | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/docs/usage/upgrade.rst.inc b/docs/usage/upgrade.rst.inc index 525c5ebde..6b06847a5 100644 --- a/docs/usage/upgrade.rst.inc +++ b/docs/usage/upgrade.rst.inc @@ -20,6 +20,12 @@ optional arguments ``-i``, ``--inplace`` | rewrite repository in place, with no chance of going back to older | versions of the repository. + ``--force`` + | Force upgrade + ``--tam`` + | Enable manifest authentication (in key and cache) (Borg 1.0.9 and later) + ``--disable-tam`` + | Disable manifest authentication (in key and cache) `Common options`_ | @@ -28,6 +34,32 @@ Description ~~~~~~~~~~~ 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. + +If you routinely do this you might not want to enable this upgrade +(which will leave you exposed to the security issue). You can +reverse the upgrade by issuing ``borg upgrade --disable-tam REPO``. + +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. From a0abc3eb7526d550ce093a5c139550300128f336 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Tue, 20 Dec 2016 23:26:22 +0100 Subject: [PATCH 11/12] CHANGES: move 1.1.0b3 to correct position --- docs/changes.rst | 146 +++++++++++++++++++++++------------------------ 1 file changed, 72 insertions(+), 74 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 4208dad43..053147ffa 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -126,6 +126,78 @@ The best check that everything is ok is to run a dry-run extraction:: Changelog ========= +Version 1.1.0b3 (not released yet) +---------------------------------- + +Bug fixes: + +- borg recreate: don't rechunkify unless explicitly told so +- borg info: fixed bug when called without arguments, #1914 +- borg init: fix free space check crashing if disk is full, #1821 +- borg debug delete/get obj: fix wrong reference to exception + +New features: + +- add blake2b key modes (use blake2b as MAC). This links against system libb2, + if possible, otherwise uses bundled code +- automatically remove stale locks - set BORG_HOSTNAME_IS_UNIQUE env var + to enable stale lock killing. If set, stale locks in both cache and + repository are deleted. #562 +- borg info : print general repo information, #1680 +- borg check --first / --last / --sort / --prefix, #1663 +- borg mount --first / --last / --sort / --prefix, #1542 +- implement "health" item formatter key, #1749 +- BORG_SECURITY_DIR to remember security related infos outside the cache. + Key type, location and manifest timestamp checks now survive cache + deletion. This also means that you can now delete your cache and avoid + previous warnings, since Borg can still tell it's safe. +- implement BORG_NEW_PASSPHRASE, #1768 + +Other changes: + +- borg recreate: + + - remove special-cased --dry-run + - update --help + - remove bloat: interruption blah, autocommit blah, resuming blah + - re-use existing checkpoint functionality + - archiver tests: add check_cache tool - lints refcounts + +- fixed cache sync performance regression from 1.1.0b1 onwards, #1940 +- syncing the cache without chunks.archive.d (see :ref:`disable_archive_chunks`) + now avoids any merges and is thus faster, #1940 +- borg check --verify-data: faster due to linear on-disk-order scan +- borg debug-xxx commands removed, we use "debug xxx" subcommands now, #1627 +- improve metadata handling speed +- shortcut hashindex_set by having hashindex_lookup hint about address +- improve / add progress displays, #1721 +- check for index vs. segment files object count mismatch +- make RPC protocol more extensible: use named parameters. +- RemoteRepository: misc. code cleanups / refactors +- clarify cache/repository README file + +- docs: + + - quickstart: add a comment about other (remote) filesystems + - quickstart: only give one possible ssh url syntax, all others are + documented in usage chapter. + - mention file:// + - document repo URLs / archive location + - clarify borg diff help, #980 + - deployment: synthesize alternative --restrict-to-path example + - improve cache / index docs, esp. files cache docs, #1825 + - document using "git merge 1.0-maint -s recursive -X rename-threshold=20%" + for avoiding troubles when merging the 1.0-maint branch into master. + +- tests: + + - fuse tests: catch ENOTSUP on freebsd + - fuse tests: test troublesome xattrs last + - fix byte range error in test, #1740 + - use monkeypatch to set env vars, but only on pytest based tests. + - point XDG_*_HOME to temp dirs for tests, #1714 + - remove all BORG_* env vars from the outer environment + Version 1.0.9 (2016-12-20) -------------------------- @@ -226,80 +298,6 @@ Other changes: - add windows virtual machine with cygwin - Vagrantfile cleanup / code deduplication - -Version 1.1.0b3 (not released yet) ----------------------------------- - -Bug fixes: - -- borg recreate: don't rechunkify unless explicitly told so -- borg info: fixed bug when called without arguments, #1914 -- borg init: fix free space check crashing if disk is full, #1821 -- borg debug delete/get obj: fix wrong reference to exception - -New features: - -- add blake2b key modes (use blake2b as MAC). This links against system libb2, - if possible, otherwise uses bundled code -- automatically remove stale locks - set BORG_HOSTNAME_IS_UNIQUE env var - to enable stale lock killing. If set, stale locks in both cache and - repository are deleted. #562 -- borg info : print general repo information, #1680 -- borg check --first / --last / --sort / --prefix, #1663 -- borg mount --first / --last / --sort / --prefix, #1542 -- implement "health" item formatter key, #1749 -- BORG_SECURITY_DIR to remember security related infos outside the cache. - Key type, location and manifest timestamp checks now survive cache - deletion. This also means that you can now delete your cache and avoid - previous warnings, since Borg can still tell it's safe. -- implement BORG_NEW_PASSPHRASE, #1768 - -Other changes: - -- borg recreate: - - - remove special-cased --dry-run - - update --help - - remove bloat: interruption blah, autocommit blah, resuming blah - - re-use existing checkpoint functionality - - archiver tests: add check_cache tool - lints refcounts - -- fixed cache sync performance regression from 1.1.0b1 onwards, #1940 -- syncing the cache without chunks.archive.d (see :ref:`disable_archive_chunks`) - now avoids any merges and is thus faster, #1940 -- borg check --verify-data: faster due to linear on-disk-order scan -- borg debug-xxx commands removed, we use "debug xxx" subcommands now, #1627 -- improve metadata handling speed -- shortcut hashindex_set by having hashindex_lookup hint about address -- improve / add progress displays, #1721 -- check for index vs. segment files object count mismatch -- make RPC protocol more extensible: use named parameters. -- RemoteRepository: misc. code cleanups / refactors -- clarify cache/repository README file - -- docs: - - - quickstart: add a comment about other (remote) filesystems - - quickstart: only give one possible ssh url syntax, all others are - documented in usage chapter. - - mention file:// - - document repo URLs / archive location - - clarify borg diff help, #980 - - deployment: synthesize alternative --restrict-to-path example - - improve cache / index docs, esp. files cache docs, #1825 - - document using "git merge 1.0-maint -s recursive -X rename-threshold=20%" - for avoiding troubles when merging the 1.0-maint branch into master. - -- tests: - - - fuse tests: catch ENOTSUP on freebsd - - fuse tests: test troublesome xattrs last - - fix byte range error in test, #1740 - - use monkeypatch to set env vars, but only on pytest based tests. - - point XDG_*_HOME to temp dirs for tests, #1714 - - remove all BORG_* env vars from the outer environment - - Version 1.1.0b2 (2016-10-01) ---------------------------- From 742bfa33c48122e212e369ecdfb44f1219f93e09 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Wed, 21 Dec 2016 00:36:31 +0100 Subject: [PATCH 12/12] flake8 --- src/borg/key.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/borg/key.py b/src/borg/key.py index a623dd940..b1f2b8b17 100644 --- a/src/borg/key.py +++ b/src/borg/key.py @@ -148,6 +148,7 @@ class KeyBase: def id_hash(self, data): """Return HMAC hash using the "id" HMAC key """ + def compress(self, chunk): compr_args, chunk = self.compression_decider2.decide(chunk) compressor = Compressor(**compr_args)