From 28ad779a6f64b484d57ed92097db5cf884ad1842 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Sun, 11 Dec 2016 13:32:09 +0100 Subject: [PATCH 01/36] 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 641fd08dd..216abc57d 100644 --- a/borg/archive.py +++ b/borg/archive.py @@ -231,7 +231,7 @@ class Archive: 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 @@ Number of files: {0.stats.nfiles}'''.format( '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 2cdfe37f3..037f3680d 100644 --- a/borg/archiver.py +++ b/borg/archiver.py @@ -30,7 +30,7 @@ from .compress import Compressor 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 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), @@ -135,6 +135,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) @@ -161,6 +163,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) @@ -209,6 +212,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) @@ -705,21 +709,43 @@ class Archiver: 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) - 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.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) return self.exit_code def do_debug_info(self, args): @@ -1613,6 +1639,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. @@ -1665,6 +1713,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/borg/crypto.pyx b/borg/crypto.pyx index be692742c..f7beb3433 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 a8106aeef..df5a213af 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 @@ 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 @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 @@ class Manifest: 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 @@ class Manifest: 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 e4fcd03d9..318f8d0ed 100644 --- a/borg/key.py +++ b/borg/key.py @@ -5,15 +5,17 @@ import os 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 @@ class KeyBase: 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 @@ class PlaintextKey(KeyBase): 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 @@ class PlaintextKey(KeyBase): 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 @@ class AESKeyBase(KeyBase): """ 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 @@ class PassphraseKey(AESKeyBase): 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 PassphraseKey(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): @@ -307,6 +428,7 @@ class KeyfileKeyBase(AESKeyBase): 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 @@ class KeyfileKeyBase(AESKeyBase): 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 @@ 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), 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 815c8943a..2daa92198 100644 --- a/borg/testsuite/archiver.py +++ b/borg/testsuite/archiver.py @@ -2,6 +2,8 @@ from binascii import unhexlify, b2a_base64 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 @@ import unittest from unittest.mock import patch from hashlib import sha256 +import msgpack import pytest from .. import xattr @@ -21,13 +24,15 @@ from ..archive import Archive, ChunkBuffer, CHUNK_MAX_EXP, flags_noatime, flags_ 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 @@ 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) @@ -1253,7 +1258,7 @@ class ArchiverTestCase(ArchiverTestCaseBase): 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 @@ class ArchiverCheckTestCase(ArchiverTestCaseBase): 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 c68101942..e80a38b34 100644 --- a/borg/testsuite/crypto.py +++ b/borg/testsuite/crypto.py @@ -2,6 +2,7 @@ from binascii import hexlify 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 @@ class CryptoTestCase(BaseTestCase): 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 c6dadbece..47d49f999 100644 --- a/borg/testsuite/helpers.py +++ b/borg/testsuite/helpers.py @@ -11,7 +11,7 @@ import msgpack.fallback 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 4c57d1f02..2bb9d86fe 100644 --- a/borg/testsuite/key.py +++ b/borg/testsuite/key.py @@ -4,9 +4,14 @@ import shutil 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 KeyTestCase(BaseTestCase): class _Location: orig = '/some/place' + def canonical_path(self): + return self.orig + _location = _Location() id = bytes(32) @@ -101,3 +109,115 @@ class KeyTestCase(BaseTestCase): 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 71677bcb3..c24ea18ef 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 cb034394e..ec1a790f6 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). From f2f50efc2873636dc7acfcd222e21fe40fe667a5 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Thu, 15 Dec 2016 20:02:37 +0100 Subject: [PATCH 02/36] check: handle duplicate archive items neatly Signed-off-by: Thomas Waldmann --- borg/archive.py | 14 ++++++++++++-- borg/testsuite/archiver.py | 27 +++++++++++++++++++++++++++ docs/changes.rst | 3 +++ 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/borg/archive.py b/borg/archive.py index 216abc57d..baf212337 100644 --- a/borg/archive.py +++ b/borg/archive.py @@ -935,8 +935,18 @@ class ArchiveChecker: except (TypeError, ValueError, StopIteration): continue if valid_archive(archive): - logger.info('Found archive %s', archive[b'name'].decode('utf-8')) - manifest.archives[archive[b'name'].decode('utf-8')] = {b'id': chunk_id, b'time': archive[b'time']} + name = archive[b'name'].decode() + 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] = {b'id': chunk_id, b'time': archive[b'time']} logger.info('Manifest rebuild complete.') return manifest diff --git a/borg/testsuite/archiver.py b/borg/testsuite/archiver.py index 2daa92198..5af3e3931 100644 --- a/borg/testsuite/archiver.py +++ b/borg/testsuite/archiver.py @@ -1468,6 +1468,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(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: diff --git a/docs/changes.rst b/docs/changes.rst index c24ea18ef..b99d82edb 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: From 1c55930840a7958ff8e5dd300a92229dcb10dfed Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 17 Dec 2016 15:09:03 +0100 Subject: [PATCH 03/36] ran build_usage --- docs/usage/upgrade.rst.inc | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/docs/usage/upgrade.rst.inc b/docs/usage/upgrade.rst.inc index 6c44edf74..c31be16b0 100644 --- a/docs/usage/upgrade.rst.inc +++ b/docs/usage/upgrade.rst.inc @@ -8,7 +8,7 @@ borg upgrade usage: borg upgrade [-h] [--critical] [--error] [--warning] [--info] [--debug] [--lock-wait N] [--show-rc] [--no-files-cache] [--umask M] - [--remote-path PATH] [-p] [-n] [-i] + [--remote-path PATH] [-p] [-n] [-i] [--force] [--tam] [REPOSITORY] upgrade a repository from a previous version @@ -34,11 +34,36 @@ borg upgrade -n, --dry-run do not change repository -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) 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. + +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 ec4f42c9f85aed182e840f30f9bb01717c7ec592 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Sun, 18 Dec 2016 21:45:19 +0100 Subject: [PATCH 04/36] init: explain manifest auth compatibility --- borg/archiver.py | 18 +++++++++++++++--- borg/key.py | 2 +- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/borg/archiver.py b/borg/archiver.py index 037f3680d..9206a4b9d 100644 --- a/borg/archiver.py +++ b/borg/archiver.py @@ -127,7 +127,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) key = key_creator(repository, args) manifest = Manifest(key, repository) manifest.key = key @@ -135,8 +136,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/borg/key.py b/borg/key.py index 318f8d0ed..3540ea58f 100644 --- a/borg/key.py +++ b/borg/key.py @@ -46,7 +46,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 4d6141a607c875d74460c4f235b6175037bf6989 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Sun, 18 Dec 2016 23:28:01 +0100 Subject: [PATCH 05/36] upgrade: --disable-tam --- borg/archiver.py | 28 +++++++++++++++++++++++++--- borg/helpers.py | 15 +++++++++++---- borg/testsuite/archiver.py | 37 ++++++++++++++++++++++++++++--------- 3 files changed, 64 insertions(+), 16 deletions(-) diff --git a/borg/archiver.py b/borg/archiver.py index 9206a4b9d..8a243f890 100644 --- a/borg/archiver.py +++ b/borg/archiver.py @@ -48,6 +48,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 @@ -721,29 +723,43 @@ class Archiver: DASHES) 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.list_archive_infos(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. @@ -1666,6 +1682,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. @@ -1729,6 +1749,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/borg/helpers.py b/borg/helpers.py index df5a213af..b38ec9455 100644 --- a/borg/helpers.py +++ b/borg/helpers.py @@ -129,10 +129,17 @@ class Manifest: 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() + + 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/borg/testsuite/archiver.py b/borg/testsuite/archiver.py index 5af3e3931..6968ec33b 100644 --- a/borg/testsuite/archiver.py +++ b/borg/testsuite/archiver.py @@ -1534,6 +1534,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(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) @@ -1573,15 +1584,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(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) @@ -1589,6 +1592,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 580599cf32dbddf9eb85bd9da8166ab6ac66e099 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Mon, 19 Dec 2016 04:21:13 +0100 Subject: [PATCH 06/36] ran build_usage --- docs/usage/upgrade.rst.inc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/usage/upgrade.rst.inc b/docs/usage/upgrade.rst.inc index c31be16b0..b4793c2cc 100644 --- a/docs/usage/upgrade.rst.inc +++ b/docs/usage/upgrade.rst.inc @@ -9,6 +9,7 @@ borg upgrade usage: borg upgrade [-h] [--critical] [--error] [--warning] [--info] [--debug] [--lock-wait N] [--show-rc] [--no-files-cache] [--umask M] [--remote-path PATH] [-p] [-n] [-i] [--force] [--tam] + [--disable-tam] [REPOSITORY] upgrade a repository from a previous version @@ -37,6 +38,7 @@ borg upgrade --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) Description ~~~~~~~~~~~ @@ -57,6 +59,10 @@ 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. From 3362ec319e6ebdb423e2e407f8a98bf024c1a8d3 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Mon, 19 Dec 2016 16:06:54 +0100 Subject: [PATCH 07/36] 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 c54a9121ae5ea207123f4ea7631e23234534dddb Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Tue, 20 Dec 2016 00:49:24 +0100 Subject: [PATCH 08/36] 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 b99d82edb..209893bb2 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 0ff76bdc9df99c0d0e2d5ea6d82eb6d99178bb13 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Mon, 26 Dec 2016 15:29:22 +0100 Subject: [PATCH 09/36] dump a trace on SIGUSR2 --- borg/archiver.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/borg/archiver.py b/borg/archiver.py index 8a243f890..6fa6c5756 100644 --- a/borg/archiver.py +++ b/borg/archiver.py @@ -3,6 +3,7 @@ from datetime import datetime from hashlib import sha256 from operator import attrgetter import argparse +import faulthandler import functools import inspect import os @@ -2021,6 +2022,11 @@ def sig_info_handler(sig_no, stack): # pragma: no cover break +def sig_trace_handler(sig_no, stack): # pragma: no cover + print('\nReceived SIGUSR2 at %s, dumping trace...' % datetime.now().replace(microsecond=0), file=sys.stderr) + faulthandler.dump_traceback() + + def main(): # pragma: no cover # Make sure stdout and stderr have errors='replace') to avoid unicode # issues when print()-ing unicode file names @@ -2036,6 +2042,7 @@ def main(): # pragma: no cover signal_handler('SIGHUP', raising_signal_handler(SigHup)), \ signal_handler('SIGTERM', raising_signal_handler(SigTerm)), \ signal_handler('SIGUSR1', sig_info_handler), \ + signal_handler('SIGUSR2', sig_trace_handler), \ signal_handler('SIGINFO', sig_info_handler): archiver = Archiver() msg = None From c2c31aa13a3b7d59bbd319fa24cbb7fcf33daa9a Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Mon, 26 Dec 2016 15:29:30 +0100 Subject: [PATCH 10/36] enable faulthandler --- borg/archiver.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/borg/archiver.py b/borg/archiver.py index 6fa6c5756..7ad2195d8 100644 --- a/borg/archiver.py +++ b/borg/archiver.py @@ -2038,6 +2038,9 @@ def main(): # pragma: no cover # SIGHUP is important especially for systemd systems, where logind # sends it when a session exits, in addition to any traditional use. # Output some info if we receive SIGUSR1 or SIGINFO (ctrl-t). + + # Register fault handler for SIGSEGV, SIGFPE, SIGABRT, SIGBUS and SIGILL. + faulthandler.enable() with signal_handler('SIGINT', raising_signal_handler(KeyboardInterrupt)), \ signal_handler('SIGHUP', raising_signal_handler(SigHup)), \ signal_handler('SIGTERM', raising_signal_handler(SigTerm)), \ From 85e79f96a134c93e14873de8248021eeb5551ce4 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Tue, 3 Jan 2017 12:47:42 +0100 Subject: [PATCH 11/36] xattr: ignore empty names returned by llistxattr(2) et al --- borg/xattr.py | 6 +++--- docs/changes.rst | 7 +++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/borg/xattr.py b/borg/xattr.py index 50d1b0b81..c408268a4 100644 --- a/borg/xattr.py +++ b/borg/xattr.py @@ -205,7 +205,7 @@ if sys.platform.startswith('linux'): # pragma: linux only n, buf = _listxattr_inner(func, path) return [os.fsdecode(name) for name in split_string0(buf[:n]) - if not name.startswith(b'system.posix_acl_')] + if name and not name.startswith(b'system.posix_acl_')] def getxattr(path, name, *, follow_symlinks=True): def func(path, name, buf, size): @@ -261,7 +261,7 @@ elif sys.platform == 'darwin': # pragma: darwin only return libc.listxattr(path, buf, size, XATTR_NOFOLLOW) n, buf = _listxattr_inner(func, path) - return [os.fsdecode(name) for name in split_string0(buf[:n])] + return [os.fsdecode(name) for name in split_string0(buf[:n]) if name] def getxattr(path, name, *, follow_symlinks=True): def func(path, name, buf, size): @@ -320,7 +320,7 @@ elif sys.platform.startswith('freebsd'): # pragma: freebsd only return libc.extattr_list_link(path, ns, buf, size) n, buf = _listxattr_inner(func, path) - return [os.fsdecode(name) for name in split_lstring(buf[:n])] + return [os.fsdecode(name) for name in split_lstring(buf[:n]) if name] def getxattr(path, name, *, follow_symlinks=True): def func(path, name, buf, size): diff --git a/docs/changes.rst b/docs/changes.rst index 209893bb2..2a860f075 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -126,6 +126,13 @@ The best check that everything is ok is to run a dry-run extraction:: Changelog ========= +Version 1.0.10rc1 (not released yet) +------------------------------------ + +Bug fixes: + +- Avoid triggering an ObjectiveFS bug in xattr retrieval, #1992 + Version 1.0.9 (2016-12-20) -------------------------- From 3e04fa972ad499f621474c4a980f9ccf1b66fe3c Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Tue, 3 Jan 2017 14:25:55 +0100 Subject: [PATCH 12/36] xattr: only skip file on BufferTooSmallError redefine __str__ to get a proper error message, not '' --- borg/xattr.py | 9 ++++++--- docs/changes.rst | 1 + 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/borg/xattr.py b/borg/xattr.py index c408268a4..0c1e9f0eb 100644 --- a/borg/xattr.py +++ b/borg/xattr.py @@ -10,7 +10,7 @@ from ctypes import CDLL, create_string_buffer, c_ssize_t, c_size_t, c_char_p, c_ from ctypes.util import find_library from distutils.version import LooseVersion -from .helpers import Buffer +from .helpers import Buffer, Error try: @@ -113,8 +113,11 @@ def split_lstring(buf): return result -class BufferTooSmallError(Exception): - """the buffer given to an xattr function was too small for the result""" +class BufferTooSmallError(OSError): + """insufficient buffer memory for completing a xattr operation.""" + + def __str__(self): + return self.__doc__ def _check(rv, path=None, detect_buffer_too_small=False): diff --git a/docs/changes.rst b/docs/changes.rst index 2a860f075..7e3c876fd 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -132,6 +132,7 @@ Version 1.0.10rc1 (not released yet) Bug fixes: - Avoid triggering an ObjectiveFS bug in xattr retrieval, #1992 +- When running out of buffer memory when reading xattrs, only skip the current file, #1993 Version 1.0.9 (2016-12-20) -------------------------- From 6a5b3018c1c1d3c8db13dd93bb22c7297ce78afd Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Tue, 3 Jan 2017 17:15:32 +0100 Subject: [PATCH 13/36] fix upgrade --tam crashing if repository is not encrypted --- borg/archiver.py | 4 ++++ docs/changes.rst | 2 ++ 2 files changed, 6 insertions(+) diff --git a/borg/archiver.py b/borg/archiver.py index 7ad2195d8..973ea6f7a 100644 --- a/borg/archiver.py +++ b/borg/archiver.py @@ -730,6 +730,10 @@ class Archiver: if args.tam: manifest, key = Manifest.load(repository, force_tam_not_required=args.force) + if not hasattr(key, 'change_passphrase'): + print('This repository is not encrypted, cannot enable TAM.') + return EXIT_ERROR + 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:') diff --git a/docs/changes.rst b/docs/changes.rst index 7e3c876fd..ae471af86 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -133,6 +133,8 @@ Bug fixes: - Avoid triggering an ObjectiveFS bug in xattr retrieval, #1992 - When running out of buffer memory when reading xattrs, only skip the current file, #1993 +- Fixed "borg upgrade --tam" crashing with unencrypted repositories. Since :ref:`the issue ` is + not relevant for unencrypted repositories, it now does nothing and prints an error, #1981. Version 1.0.9 (2016-12-20) -------------------------- From 7519bf8100a3aa9d664d39f2d4f0dd7a1ff7b10e Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Tue, 3 Jan 2017 17:15:59 +0100 Subject: [PATCH 14/36] fix change-passphrase crashing if repository is not encrypted --- borg/archiver.py | 3 +++ docs/changes.rst | 1 + 2 files changed, 4 insertions(+) diff --git a/borg/archiver.py b/borg/archiver.py index 973ea6f7a..1adbba895 100644 --- a/borg/archiver.py +++ b/borg/archiver.py @@ -177,6 +177,9 @@ class Archiver: @with_repository() def do_change_passphrase(self, args, repository, manifest, key): """Change repository key file passphrase""" + if not hasattr(key, 'change_passphrase'): + print('This repository is not encrypted, cannot change the passphrase.') + return EXIT_ERROR key.change_passphrase() logger.info('Key updated') return EXIT_SUCCESS diff --git a/docs/changes.rst b/docs/changes.rst index ae471af86..301246004 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -135,6 +135,7 @@ Bug fixes: - When running out of buffer memory when reading xattrs, only skip the current file, #1993 - Fixed "borg upgrade --tam" crashing with unencrypted repositories. Since :ref:`the issue ` is not relevant for unencrypted repositories, it now does nothing and prints an error, #1981. +- Fixed change-passphrase crashing with unencrypted repositories, #1978 Version 1.0.9 (2016-12-20) -------------------------- From 4b9a9f9b5ec8fd54be78569b2e5e9201a2a9dada Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Tue, 3 Jan 2017 13:00:37 +0100 Subject: [PATCH 15/36] change-passphrase: print key location --- borg/archiver.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/borg/archiver.py b/borg/archiver.py index 1adbba895..c8c80f37c 100644 --- a/borg/archiver.py +++ b/borg/archiver.py @@ -182,6 +182,9 @@ class Archiver: return EXIT_ERROR key.change_passphrase() logger.info('Key updated') + if hasattr(key, 'find_key'): + # print key location to make backing it up easier + logger.info('Key location: %s', key.find_key()) return EXIT_SUCCESS @with_repository(lock=False, exclusive=False, manifest=False, cache=False) From 95334930475b70f7e5cbf21314093bdc088d9d75 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Wed, 4 Jan 2017 00:57:35 +0100 Subject: [PATCH 16/36] tox / travis: also test on Python 3.6 --- .travis.yml | 8 ++++++++ tox.ini | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 36c38d631..e1433a173 100644 --- a/.travis.yml +++ b/.travis.yml @@ -16,6 +16,10 @@ matrix: os: linux dist: trusty env: TOXENV=py35 + - python: 3.6 + os: linux + dist: trusty + env: TOXENV=py36 - python: 3.5 os: linux dist: trusty @@ -28,6 +32,10 @@ matrix: os: osx osx_image: xcode6.4 env: TOXENV=py35 + - language: generic + os: osx + osx_image: xcode6.4 + env: TOXENV=py36 allow_failures: - os: osx diff --git a/tox.ini b/tox.ini index 699ef251a..d44009e46 100644 --- a/tox.ini +++ b/tox.ini @@ -2,7 +2,7 @@ # fakeroot -u tox --recreate [tox] -envlist = py{34,35},flake8 +envlist = py{34,35,36},flake8 [testenv] # Change dir to avoid import problem for cython code. The directory does From 7d4d7e79012ea4798f7281a34ad9579ebf4de16d Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Wed, 4 Jan 2017 01:02:25 +0100 Subject: [PATCH 17/36] setup.py: add Python 3.6 qualifier --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index 00e48f3f3..4f2ca8681 100644 --- a/setup.py +++ b/setup.py @@ -292,6 +292,7 @@ setup( 'Programming Language :: Python :: 3', 'Programming Language :: Python :: 3.4', 'Programming Language :: Python :: 3.5', + 'Programming Language :: Python :: 3.6', 'Topic :: Security :: Cryptography', 'Topic :: System :: Archiving :: Backup', ], From c412b86455ced36a3b9e77c2b91143381f8096b6 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Wed, 4 Jan 2017 01:06:57 +0100 Subject: [PATCH 18/36] vagrant: add Python 3.6.0 --- Vagrantfile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Vagrantfile b/Vagrantfile index df636ce3e..26dcfc044 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -223,6 +223,7 @@ def install_pythons(boxname) . ~/.bash_profile pyenv install 3.4.0 # tests pyenv install 3.5.0 # tests + pyenv install 3.6.0 # tests pyenv install 3.5.2 # binary build, use latest 3.5.x release pyenv rehash EOF @@ -315,7 +316,7 @@ def run_tests(boxname) . ../borg-env/bin/activate if which pyenv 2> /dev/null; then # for testing, use the earliest point releases of the supported python versions: - pyenv global 3.4.0 3.5.0 + pyenv global 3.4.0 3.5.0 3.6.0 fi # otherwise: just use the system python if which fakeroot 2> /dev/null; then From be8e0c89b3d4a59ecf01be09876a58066c8d4c4a Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Wed, 4 Jan 2017 19:25:03 +0100 Subject: [PATCH 19/36] check: fail if single archive does not exist --- borg/archive.py | 4 ++++ docs/changes.rst | 1 + 2 files changed, 5 insertions(+) diff --git a/borg/archive.py b/borg/archive.py index baf212337..afc734cef 100644 --- a/borg/archive.py +++ b/borg/archive.py @@ -1107,6 +1107,10 @@ class ArchiveChecker: archive_items = [item for item in self.manifest.archives.items() if item[0] == archive] num_archives = 1 end = 1 + if not archive_items: + logger.error('Archive %s does not exist', archive) + self.error_found = True + return with cache_if_remote(self.repository) as repository: for i, (name, info) in enumerate(archive_items[:end]): diff --git a/docs/changes.rst b/docs/changes.rst index 301246004..a26e0040a 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -136,6 +136,7 @@ Bug fixes: - Fixed "borg upgrade --tam" crashing with unencrypted repositories. Since :ref:`the issue ` is not relevant for unencrypted repositories, it now does nothing and prints an error, #1981. - Fixed change-passphrase crashing with unencrypted repositories, #1978 +- Fixed "borg check repo::archive" indicating success if "archive" does not exist, #1997 Version 1.0.9 (2016-12-20) -------------------------- From 320a56144fa5228562c2211b29754ce18690c4e2 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Tue, 3 Jan 2017 17:38:18 +0100 Subject: [PATCH 20/36] helpers.Buffer: raise OSError subclass if too much memory is allocd --- borg/helpers.py | 14 ++++++++++++-- borg/testsuite/helpers.py | 4 ++-- borg/xattr.py | 9 +++------ 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/borg/helpers.py b/borg/helpers.py index b38ec9455..7c4a0fa91 100644 --- a/borg/helpers.py +++ b/borg/helpers.py @@ -55,9 +55,15 @@ class Error(Exception): # show a traceback? traceback = False + def __init__(self, *args): + super().__init__(*args) + self.args = args + def get_message(self): return type(self).__doc__.format(*self.args) + __str__ = get_message + class ErrorWithTraceback(Error): """like Error, but show a traceback also""" @@ -699,6 +705,10 @@ class Buffer: """ provide a thread-local buffer """ + + class MemoryLimitExceeded(Error, OSError): + """Requested buffer size {} is above the limit of {}.""" + def __init__(self, allocator, size=4096, limit=None): """ Initialize the buffer: use allocator(size) call to allocate a buffer. @@ -718,11 +728,11 @@ class Buffer: """ resize the buffer - to avoid frequent reallocation, we usually always grow (if needed). giving init=True it is possible to first-time initialize or shrink the buffer. - if a buffer size beyond the limit is requested, raise ValueError. + if a buffer size beyond the limit is requested, raise Buffer.MemoryLimitExceeded (OSError). """ size = int(size) if self.limit is not None and size > self.limit: - raise ValueError('Requested buffer size %d is above the limit of %d.' % (size, self.limit)) + raise Buffer.MemoryLimitExceeded(size, self.limit) if init or len(self) < size: self._thread_local.buffer = self.allocator(size) diff --git a/borg/testsuite/helpers.py b/borg/testsuite/helpers.py index 47d49f999..3f9c70969 100644 --- a/borg/testsuite/helpers.py +++ b/borg/testsuite/helpers.py @@ -793,7 +793,7 @@ class TestBuffer: buffer = Buffer(bytearray, size=100, limit=200) buffer.resize(200) assert len(buffer) == 200 - with pytest.raises(ValueError): + with pytest.raises(Buffer.MemoryLimitExceeded): buffer.resize(201) assert len(buffer) == 200 @@ -807,7 +807,7 @@ class TestBuffer: b3 = buffer.get(200) assert len(b3) == 200 assert b3 is not b2 # new, resized buffer - with pytest.raises(ValueError): + with pytest.raises(Buffer.MemoryLimitExceeded): buffer.get(201) # beyond limit assert len(buffer) == 200 diff --git a/borg/xattr.py b/borg/xattr.py index 0c1e9f0eb..75cb91890 100644 --- a/borg/xattr.py +++ b/borg/xattr.py @@ -10,7 +10,7 @@ from ctypes import CDLL, create_string_buffer, c_ssize_t, c_size_t, c_char_p, c_ from ctypes.util import find_library from distutils.version import LooseVersion -from .helpers import Buffer, Error +from .helpers import Buffer try: @@ -113,11 +113,8 @@ def split_lstring(buf): return result -class BufferTooSmallError(OSError): - """insufficient buffer memory for completing a xattr operation.""" - - def __str__(self): - return self.__doc__ +class BufferTooSmallError(Exception): + """the buffer given to an xattr function was too small for the result.""" def _check(rv, path=None, detect_buffer_too_small=False): From 853cfb703b98400177ebeb01b0df2b29710766ac Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Tue, 3 Jan 2017 04:26:04 +0100 Subject: [PATCH 21/36] parallelizing tests via pytest-xdist --- requirements.d/development.txt | 1 + tox.ini | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/requirements.d/development.txt b/requirements.d/development.txt index a0cb3c2ac..a07f0b02a 100644 --- a/requirements.d/development.txt +++ b/requirements.d/development.txt @@ -1,6 +1,7 @@ virtualenv<14.0 tox pytest +pytest-xdist pytest-cov pytest-benchmark Cython diff --git a/tox.ini b/tox.ini index d44009e46..ce0aaacb0 100644 --- a/tox.ini +++ b/tox.ini @@ -12,7 +12,7 @@ deps = -rrequirements.d/development.txt -rrequirements.d/attic.txt -rrequirements.d/fuse.txt -commands = py.test -rs --cov=borg --cov-config=../.coveragerc --benchmark-skip --pyargs {posargs:borg.testsuite} +commands = py.test -n 8 -rs --cov=borg --cov-config=../.coveragerc --benchmark-skip --pyargs {posargs:borg.testsuite} # fakeroot -u needs some env vars: passenv = * From 5ed6d213028082cb094bb0e8661ac52f677fb571 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Tue, 3 Jan 2017 04:27:51 +0100 Subject: [PATCH 22/36] parallel testing: fix issue related to non-reproducible set / dict order --- borg/testsuite/archive.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/borg/testsuite/archive.py b/borg/testsuite/archive.py index 2dbdd7bc8..65b3f78b2 100644 --- a/borg/testsuite/archive.py +++ b/borg/testsuite/archive.py @@ -1,3 +1,4 @@ +from collections import OrderedDict from datetime import datetime, timezone from unittest.mock import Mock @@ -131,11 +132,15 @@ def test_invalid_msgpacked_item(packed, item_keys_serialized): assert not valid_msgpacked_dict(packed, item_keys_serialized) +# pytest-xdist requires always same order for the keys and dicts: +IK = sorted(list(ITEM_KEYS)) + + @pytest.mark.parametrize('packed', [msgpack.packb(o) for o in [ {b'path': b'/a/b/c'}, # small (different msgpack mapping type!) - dict((k, b'') for k in ITEM_KEYS), # as big (key count) as it gets - dict((k, b'x' * 1000) for k in ITEM_KEYS), # as big (key count and volume) as it gets + OrderedDict((k, b'') for k in IK), # as big (key count) as it gets + OrderedDict((k, b'x' * 1000) for k in IK), # as big (key count and volume) as it gets ]]) def test_valid_msgpacked_items(packed, item_keys_serialized): assert valid_msgpacked_dict(packed, item_keys_serialized) From a1d223cec0839be47f338ce4d13a317016ffbcb8 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Thu, 5 Jan 2017 03:47:13 +0100 Subject: [PATCH 23/36] always setup module level "logger" in the same way this is a cleanup change, found this while trying to find out why borg_cmd spuriously does not have INFO loglevel when testing with pytest-xdist. the cleanup did NOT help with this, but is at least a cleanup. --- borg/repository.py | 7 ++++--- borg/upgrader.py | 5 +++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/borg/repository.py b/borg/repository.py index 690e77707..dd49b368b 100644 --- a/borg/repository.py +++ b/borg/repository.py @@ -3,15 +3,16 @@ from binascii import unhexlify from datetime import datetime from itertools import islice import errno -import logging -logger = logging.getLogger(__name__) - import os import shutil import struct from zlib import crc32 import msgpack + +from .logger import create_logger +logger = create_logger() + from .helpers import Error, ErrorWithTraceback, IntegrityError, Location, ProgressIndicatorPercent, bin_to_hex from .hashindex import NSIndex from .locking import Lock, LockError, LockErrorT diff --git a/borg/upgrader.py b/borg/upgrader.py index c6700262c..208e7f7b4 100644 --- a/borg/upgrader.py +++ b/borg/upgrader.py @@ -1,10 +1,11 @@ import datetime -import logging -logger = logging.getLogger(__name__) import os import shutil import time +from .logger import create_logger +logger = create_logger() + from .helpers import get_keys_dir, get_cache_dir, ProgressIndicatorPercent, bin_to_hex from .locking import Lock from .repository import Repository, MAGIC From 2938a5f6fbb437adced4b88357d30e7ee9f5d29b Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Thu, 5 Jan 2017 03:59:33 +0100 Subject: [PATCH 24/36] work around spurious log level related test fail when using pytest-xdist --- borg/testsuite/repository.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/borg/testsuite/repository.py b/borg/testsuite/repository.py index e217137af..713b03cfd 100644 --- a/borg/testsuite/repository.py +++ b/borg/testsuite/repository.py @@ -1,3 +1,4 @@ +import logging import os import shutil import sys @@ -440,6 +441,8 @@ class RemoteRepositoryTestCase(RepositoryTestCase): assert self.repository.borg_cmd(None, testing=True) == [sys.executable, '-m', 'borg.archiver', 'serve'] args = MockArgs() + # XXX without next line we get spurious test fails when using pytest-xdist, root cause unknown: + logging.getLogger().setLevel(logging.INFO) # note: test logger is on info log level, so --info gets added automagically assert self.repository.borg_cmd(args, testing=False) == ['borg', 'serve', '--umask=077', '--info'] args.remote_path = 'borg-0.28.2' From 370cb1f19a9ba8b5191000e96350482e593749e7 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Fri, 6 Jan 2017 06:19:26 +0100 Subject: [PATCH 25/36] travis: fix osxfuse install --- .travis/install.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis/install.sh b/.travis/install.sh index 92f14e21c..71026d5d1 100755 --- a/.travis/install.sh +++ b/.travis/install.sh @@ -17,7 +17,7 @@ if [[ "$(uname -s)" == 'Darwin' ]]; then brew install lz4 brew outdated pyenv || brew upgrade pyenv brew install pkg-config - brew install Caskroom/versions/osxfuse + brew install Caskroom/cask/osxfuse case "${TOXENV}" in py34) From e119042f4c7693592619a9b26e6424391b1ad25b Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Fri, 6 Jan 2017 06:49:40 +0100 Subject: [PATCH 26/36] travis: install py36 on OS X --- .travis/install.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.travis/install.sh b/.travis/install.sh index 71026d5d1..d90254370 100755 --- a/.travis/install.sh +++ b/.travis/install.sh @@ -28,6 +28,10 @@ if [[ "$(uname -s)" == 'Darwin' ]]; then pyenv install 3.5.1 pyenv global 3.5.1 ;; + py36) + pyenv install 3.6.0 + pyenv global 3.6.0 + ;; esac pyenv rehash python -m pip install --user 'virtualenv<14.0' From 69b816fe763e3303dfca378eb4b9da0abf484db6 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 7 Jan 2017 23:12:27 +0100 Subject: [PATCH 27/36] update CHANGES (1.0-maint) --- docs/changes.rst | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 301246004..23f9174e2 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -132,11 +132,27 @@ Version 1.0.10rc1 (not released yet) Bug fixes: - Avoid triggering an ObjectiveFS bug in xattr retrieval, #1992 -- When running out of buffer memory when reading xattrs, only skip the current file, #1993 -- Fixed "borg upgrade --tam" crashing with unencrypted repositories. Since :ref:`the issue ` is - not relevant for unencrypted repositories, it now does nothing and prints an error, #1981. +- When running out of buffer memory when reading xattrs, only skip the + current file, #1993 +- Fixed "borg upgrade --tam" crashing with unencrypted repositories. Since + :ref:`the issue ` is not relevant for unencrypted repositories, + it now does nothing and prints an error, #1981. - Fixed change-passphrase crashing with unencrypted repositories, #1978 +Other changes: + +- xattr: ignore empty names returned by llistxattr(2) et al +- Enable the fault handler: install handlers for the SIGSEGV, SIGFPE, SIGABRT, + SIGBUS and SIGILL signals to dump the Python traceback. +- Also print a traceback on SIGUSR2. +- borg change-passphrase: print key location (simplify making a backup of it) +- officially support Python 3.6 (setup.py: add Python 3.6 qualifier) +- tests: + + - vagrant / travis / tox: add Python 3.6 based testing + - travis: fix osxfuse install (fixes OS X testing on Travis CI) + + Version 1.0.9 (2016-12-20) -------------------------- From 33be583920b8880da829b512066f64e1131ad769 Mon Sep 17 00:00:00 2001 From: sherbang Date: Sat, 7 Jan 2017 18:34:11 -0500 Subject: [PATCH 28/36] Update faq.rst --- docs/faq.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/faq.rst b/docs/faq.rst index ed4b6f76f..f0ae2fe7f 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -13,7 +13,7 @@ Yes, the `deduplication`_ technique used by Also, we have optional simple sparse file support for extract. If you use non-snapshotting backup tools like Borg to back up virtual machines, -then these should be turned off for doing so. Backing up live VMs this way can (and will) +then the VMs should be turned off for the duration of the backup. Backing up live VMs can (and will) result in corrupted or inconsistent backup contents: a VM image is just a regular file to Borg with the same issues as regular files when it comes to concurrent reading and writing from the same file. From fe6b03a72d9d7db8d1a0a4eace2c3bbd142002c7 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Wed, 4 Jan 2017 19:32:39 +0100 Subject: [PATCH 29/36] check: print non-exit-code warning if --last or --prefix aren't fulfilled --- borg/archive.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/borg/archive.py b/borg/archive.py index afc734cef..26ad16455 100644 --- a/borg/archive.py +++ b/borg/archive.py @@ -1100,8 +1100,12 @@ class ArchiveChecker: key=lambda name_info: name_info[1][b'time']) if prefix is not None: archive_items = [item for item in archive_items if item[0].startswith(prefix)] + if not archive_items: + logger.warning('--prefix %s does not match any archives', prefix) num_archives = len(archive_items) end = None if last is None else min(num_archives, last) + if last is not None and end < last: + logger.warning('--last %d archives: only found %d archives', last, end) else: # we only want one specific archive archive_items = [item for item in self.manifest.archives.items() if item[0] == archive] From 01090d2d40b20f75568740f888227c48d9f115f4 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Thu, 12 Jan 2017 02:25:41 +0100 Subject: [PATCH 30/36] fix typos taken from debian package, thanks to danny edel and lintian for finding these. --- borg/archiver.py | 2 +- docs/usage.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/borg/archiver.py b/borg/archiver.py index c8c80f37c..2d0e792fa 100644 --- a/borg/archiver.py +++ b/borg/archiver.py @@ -541,7 +541,7 @@ class Archiver: @with_repository() def do_mount(self, args, repository, manifest, key): - """Mount archive or an entire repository as a FUSE fileystem""" + """Mount archive or an entire repository as a FUSE filesystem""" try: from .fuse import FuseOperations except ImportError as e: diff --git a/docs/usage.rst b/docs/usage.rst index ec1a790f6..18a7cdbdf 100644 --- a/docs/usage.rst +++ b/docs/usage.rst @@ -160,7 +160,7 @@ General: BORG_FILES_CACHE_TTL When set to a numeric value, this determines the maximum "time to live" for the files cache entries (default: 20). The files cache is used to quickly determine whether a file is unchanged. - The FAQ explains this more detailled in: :ref:`always_chunking` + The FAQ explains this more detailed in: :ref:`always_chunking` TMPDIR where temporary files are stored (might need a lot of temporary space for some operations) From b6fa8629dbc858d997253e25fa48172e92435463 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Thu, 12 Jan 2017 02:39:56 +0100 Subject: [PATCH 31/36] remote: log SSH command line at debug level --- borg/remote.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/borg/remote.py b/borg/remote.py index 00c3114a4..6a6c51f8a 100644 --- a/borg/remote.py +++ b/borg/remote.py @@ -13,9 +13,12 @@ from . import __version__ from .helpers import Error, IntegrityError, sysinfo from .helpers import replace_placeholders from .repository import Repository +from .logger import create_logger import msgpack +logger = create_logger(__name__) + RPC_PROTOCOL_VERSION = 2 BUFSIZE = 10 * 1024 * 1024 @@ -185,6 +188,7 @@ class RemoteRepository: env.pop('LD_LIBRARY_PATH', None) env.pop('BORG_PASSPHRASE', None) # security: do not give secrets to subprocess env['BORG_VERSION'] = __version__ + logger.debug('SSH command line: %s', borg_cmd) self.p = Popen(borg_cmd, bufsize=0, stdin=PIPE, stdout=PIPE, stderr=PIPE, env=env) self.stdin_fd = self.p.stdin.fileno() self.stdout_fd = self.p.stdout.fileno() From 2d2bff9bf6fe03716861f5c621610a88315f8e94 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Thu, 12 Jan 2017 02:41:29 +0100 Subject: [PATCH 32/36] remote: include unknown data in error message this makes it far, far easier to diagnose issues like an account being locked: Got unexpected RPC data format from server: This account is currently not available. --- borg/remote.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/borg/remote.py b/borg/remote.py index 6a6c51f8a..0944997cb 100644 --- a/borg/remote.py +++ b/borg/remote.py @@ -4,14 +4,16 @@ import logging import os import select import shlex -from subprocess import Popen, PIPE import sys import tempfile +import textwrap +from subprocess import Popen, PIPE from . import __version__ from .helpers import Error, IntegrityError, sysinfo from .helpers import replace_placeholders +from .helpers import bin_to_hex from .repository import Repository from .logger import create_logger @@ -47,7 +49,16 @@ class UnexpectedRPCDataFormatFromClient(Error): class UnexpectedRPCDataFormatFromServer(Error): - """Got unexpected RPC data format from server.""" + """Got unexpected RPC data format from server:\n{}""" + + def __init__(self, data): + try: + data = data.decode()[:128] + except UnicodeDecodeError: + data = data[:128] + data = ['%02X' % byte for byte in data] + data = textwrap.fill(' '.join(data), 16 * 3) + super().__init__(data) class RepositoryServer: # pragma: no cover @@ -350,7 +361,7 @@ This problem will go away as soon as the server has been upgraded to 1.0.7+. self.unpacker.feed(data) for unpacked in self.unpacker: if not (isinstance(unpacked, tuple) and len(unpacked) == 4): - raise UnexpectedRPCDataFormatFromServer() + raise UnexpectedRPCDataFormatFromServer(data) type, msgid, error, res = unpacked if msgid in self.ignore_responses: self.ignore_responses.remove(msgid) From 660313334472f22c9accc7c8d012bb8cd1db3685 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Thu, 12 Jan 2017 02:47:51 +0100 Subject: [PATCH 33/36] update CHANGES (1.0-maint) --- docs/changes.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/changes.rst b/docs/changes.rst index 1a9b40d53..f9a2f4fa1 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -139,6 +139,7 @@ Bug fixes: it now does nothing and prints an error, #1981. - Fixed change-passphrase crashing with unencrypted repositories, #1978 - Fixed "borg check repo::archive" indicating success if "archive" does not exist, #1997 +- borg check: print non-exit-code warning if --last or --prefix aren't fulfilled Other changes: @@ -152,6 +153,13 @@ Other changes: - vagrant / travis / tox: add Python 3.6 based testing - travis: fix osxfuse install (fixes OS X testing on Travis CI) + - use pytest-xdist to parallelize testing +- docs: + + - language clarification - VM backup FAQ +- fix typos (taken from Debian package patch) +- remote: include data hexdump in "unexpected RPC data" error message +- remote: log SSH command line at debug level Version 1.0.9 (2016-12-20) From 3c0a903e8a596681615d81b3b853b1a6acc00c72 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Thu, 12 Jan 2017 14:30:09 +0100 Subject: [PATCH 34/36] upgrade: fix incorrect title levels --- borg/archiver.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/borg/archiver.py b/borg/archiver.py index 2d0e792fa..032d600c6 100644 --- a/borg/archiver.py +++ b/borg/archiver.py @@ -1680,7 +1680,7 @@ class Archiver: 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 @@ -1702,7 +1702,7 @@ class Archiver: 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 1d40675ce49d6f7608ab74931c9ad518e820b6f6 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Thu, 12 Jan 2017 15:04:57 +0100 Subject: [PATCH 35/36] merge fixup --- src/borg/testsuite/archiver.py | 76 ---------------------------------- 1 file changed, 76 deletions(-) diff --git a/src/borg/testsuite/archiver.py b/src/borg/testsuite/archiver.py index 6da3073ab..2d97ce06d 100644 --- a/src/borg/testsuite/archiver.py +++ b/src/borg/testsuite/archiver.py @@ -2358,82 +2358,6 @@ class ManifestAuthenticationTest(ArchiverTestCaseBase): assert not self.cmd('list', self.repository_location) -class ManifestAuthenticationTest(ArchiverTestCaseBase): - def spoof_manifest(self, repository): - 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() - - 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 - self.spoof_manifest(repository) - # 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) - - 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): prefix = '__testsuite__:' From 7923088ff9d1236f3a22a8e7b5641e89c155085f Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Thu, 12 Jan 2017 17:03:51 +0100 Subject: [PATCH 36/36] check: pick better insufficent archives matched warning from TW's merge --- src/borg/archive.py | 8 ++++++-- src/borg/repository.py | 5 ++--- src/borg/testsuite/archive.py | 4 ++-- src/borg/upgrader.py | 6 +++--- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/borg/archive.py b/src/borg/archive.py index 25983a3c9..195ae8e1c 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -1360,8 +1360,12 @@ class ArchiveChecker: sort_by = sort_by.split(',') if any((first, last, prefix)): archive_infos = self.manifest.archives.list(sort_by=sort_by, prefix=prefix, first=first, last=last) - if not archive_infos: - logger.warning('--first/--last/--prefix did not match any archives') + if prefix and not archive_infos: + logger.warning('--prefix %s does not match any archives', prefix) + if first and len(archive_infos) < first: + logger.warning('--first %d archives: only found %d archives', first, len(archive_infos)) + if last and len(archive_infos) < last: + logger.warning('--last %d archives: only found %d archives', last, len(archive_infos)) else: archive_infos = self.manifest.archives.list(sort_by=sort_by) else: diff --git a/src/borg/repository.py b/src/borg/repository.py index 20ae274ef..824985da0 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -11,9 +11,6 @@ from itertools import islice import msgpack -import logging -logger = logging.getLogger(__name__) - from .constants import * # NOQA from .hashindex import NSIndex from .helpers import Error, ErrorWithTraceback, IntegrityError, format_file_size, parse_file_size @@ -27,6 +24,8 @@ from .lrucache import LRUCache from .platform import SaveFile, SyncFile, sync_dir from .crc32 import crc32 +logger = create_logger(__name__) + MAX_OBJECT_SIZE = 20 * 1024 * 1024 MAGIC = b'BORG_SEG' MAGIC_LEN = len(MAGIC) diff --git a/src/borg/testsuite/archive.py b/src/borg/testsuite/archive.py index a4f613d18..7bf3f5b63 100644 --- a/src/borg/testsuite/archive.py +++ b/src/borg/testsuite/archive.py @@ -31,8 +31,8 @@ def test_stats_basic(stats): assert stats.usize == 10 -def tests_stats_progress(stats, columns=80): - os.environ['COLUMNS'] = str(columns) +def tests_stats_progress(stats, monkeypatch, columns=80): + monkeypatch.setenv('COLUMNS', str(columns)) out = StringIO() stats.show_progress(stream=out) s = '20 B O 10 B C 10 B D 0 N ' diff --git a/src/borg/upgrader.py b/src/borg/upgrader.py index 78da849a2..28473e074 100644 --- a/src/borg/upgrader.py +++ b/src/borg/upgrader.py @@ -3,15 +3,15 @@ import os import shutil import time -import logging -logger = logging.getLogger(__name__) - from .constants import REPOSITORY_README from .helpers import get_home_dir, get_keys_dir, get_cache_dir from .helpers import ProgressIndicatorPercent from .key import KeyfileKey, KeyfileNotFoundError from .locking import Lock from .repository import Repository, MAGIC +from .logger import create_logger + +logger = create_logger(__name__) ATTIC_MAGIC = b'ATTICSEG'