From 8783460fc3a50b8271b2dcf2a3ef3b1eb740f3d8 Mon Sep 17 00:00:00 2001 From: Martin Hostettler Date: Mon, 6 Feb 2017 23:19:02 +0100 Subject: [PATCH 1/5] Add minimal version of in repository mandatory feature flags. This should allow us to make sure older borg versions can be cleanly prevented from doing operations that are no longer safe because of repository format evolution. This allows more fine grained control than just incrementing the manifest version. So for example a change that still allows new archives to be created but would corrupt the repository when an old version tries to delete an archive or check the repository would add the new feature to the check and delete set but leave it out of the write set. This is somewhat inspired by ext{2,3,4} which uses sets for compat (everything except fsck), ro-compat (may only be accessed read-only by older versions) and features (refuse all access). --- borg/archive.py | 2 +- borg/archiver.py | 48 +++++++++++++++++++++++------------- borg/helpers.py | 50 +++++++++++++++++++++++++++++++++++++- borg/testsuite/archiver.py | 18 +++++++------- 4 files changed, 90 insertions(+), 28 deletions(-) diff --git a/borg/archive.py b/borg/archive.py index 0c57680f4..743ec06a1 100644 --- a/borg/archive.py +++ b/borg/archive.py @@ -870,7 +870,7 @@ class ArchiveChecker: self.manifest = self.rebuild_manifest() else: try: - self.manifest, _ = Manifest.load(repository, key=self.key) + self.manifest, _ = Manifest.load(repository, (Manifest.Operation.CHECK,), key=self.key) except IntegrityError as exc: logger.error('Repository manifest is corrupted: %s', exc) self.error_found = True diff --git a/borg/archiver.py b/borg/archiver.py index 9aa8bde06..248d82c59 100644 --- a/borg/archiver.py +++ b/borg/archiver.py @@ -56,7 +56,8 @@ def argument(args, str_or_bool): return str_or_bool -def with_repository(fake=False, invert_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, compatibility=None): """ Method decorator for subcommand-handling methods: do_XYZ(self, args, repository, …) @@ -67,7 +68,20 @@ def with_repository(fake=False, invert_fake=False, create=False, lock=True, excl :param exclusive: (str or bool) lock repository exclusively (for writing) :param manifest: load manifest and key, pass them as keyword arguments :param cache: open cache, pass it as keyword argument (implies manifest) + :param compatibility: mandatory if not create and (manifest or cache), specifies mandatory feature categories to check """ + + if not create and (manifest or cache): + if compatibility is None: + raise AssertionError("with_repository decorator used without compatibility argument") + if type(compatibility) is not tuple: + raise AssertionError("with_repository decorator compatibility argument must be of type tuple") + else: + if compatibility is not None: + raise AssertionError("with_repository called with compatibility argument but would not check" + repr(compatibility)) + if create: + compatibility = Manifest.NO_OPERATION_CHECK + def decorator(method): @functools.wraps(method) def wrapper(self, args, **kwargs): @@ -84,7 +98,7 @@ def with_repository(fake=False, invert_fake=False, create=False, lock=True, excl append_only=append_only) with repository: if manifest or cache: - kwargs['manifest'], kwargs['key'] = Manifest.load(repository) + kwargs['manifest'], kwargs['key'] = Manifest.load(repository, compatibility) if cache: with Cache(repository, kwargs['key'], kwargs['manifest'], do_files=getattr(args, 'cache_files', False), lock_wait=self.lock_wait) as cache_: @@ -176,7 +190,7 @@ class Archiver: return EXIT_WARNING return EXIT_SUCCESS - @with_repository() + @with_repository(compatibility=(Manifest.Operation.CHECK,)) def do_change_passphrase(self, args, repository, manifest, key): """Change repository key file passphrase""" if not hasattr(key, 'change_passphrase'): @@ -241,7 +255,7 @@ class Archiver: logger.info('Key updated') return EXIT_SUCCESS - @with_repository(fake='dry_run', exclusive=True) + @with_repository(fake='dry_run', exclusive=True, compatibility=(Manifest.Operation.WRITE,)) def do_create(self, args, repository, manifest=None, key=None): """Create new archive""" matcher = PatternMatcher(fallback=True) @@ -424,7 +438,7 @@ class Archiver: return matcher.match(item[b'path']) return item_filter - @with_repository() + @with_repository(compatibility=(Manifest.Operation.READ,)) @with_archive def do_extract(self, args, repository, manifest, key, archive): """Extract archive contents""" @@ -490,7 +504,7 @@ class Archiver: self.print_warning("Include pattern '%s' never matched.", pattern) return self.exit_code - @with_repository(exclusive=True, cache=True) + @with_repository(exclusive=True, cache=True, compatibility=(Manifest.Operation.CHECK,)) @with_archive def do_rename(self, args, repository, manifest, key, cache, archive): """Rename an existing archive""" @@ -506,7 +520,7 @@ class Archiver: """Delete an existing repository or archive""" if args.location.archive: archive_name = args.location.archive - manifest, key = Manifest.load(repository) + manifest, key = Manifest.load(repository, (Manifest.Operation.DELETE,)) if args.forced == 2: try: @@ -537,7 +551,7 @@ class Archiver: if not args.cache_only: msg = [] try: - manifest, key = Manifest.load(repository) + manifest, key = Manifest.load(repository, Manifest.NO_OPERATION_CHECK) except NoManifestError: msg.append("You requested to completely DELETE the repository *including* all archives it may contain.") msg.append("This repository seems to have no manifest, so we can't tell anything about its contents.") @@ -573,7 +587,7 @@ class Archiver: return self._do_mount(args) - @with_repository() + @with_repository(compatibility=(Manifest.Operation.READ,)) def _do_mount(self, args, repository, manifest, key): from .fuse import FuseOperations @@ -595,7 +609,7 @@ class Archiver: """un-mount the FUSE filesystem""" return umount(args.mountpoint) - @with_repository() + @with_repository(compatibility=(Manifest.Operation.READ,)) def do_list(self, args, repository, manifest, key): """List archive or repository contents""" if args.location.archive: @@ -681,7 +695,7 @@ class Archiver: print(format_archive(archive_info)) return self.exit_code - @with_repository(cache=True) + @with_repository(cache=True, compatibility=(Manifest.Operation.READ,)) @with_archive def do_info(self, args, repository, manifest, key, archive, cache): """Show archive details such as disk space used""" @@ -699,7 +713,7 @@ class Archiver: print(str(cache)) return self.exit_code - @with_repository(exclusive=True) + @with_repository(exclusive=True, compatibility=(Manifest.Operation.DELETE,)) def do_prune(self, args, repository, manifest, key): """Prune repository archives according to specified rules""" if not any((args.hourly, args.daily, @@ -759,7 +773,7 @@ class Archiver: 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) + manifest, key = Manifest.load(repository, (Manifest.Operation.CHECK,), force_tam_not_required=args.force) if not hasattr(key, 'change_passphrase'): print('This repository is not encrypted, cannot enable TAM.') @@ -784,7 +798,7 @@ class Archiver: open(tam_file, 'w').close() print('Updated security database') elif args.disable_tam: - manifest, key = Manifest.load(repository, force_tam_not_required=True) + manifest, key = Manifest.load(repository, Manifest.NO_OPERATION_CHECK, force_tam_not_required=True) if tam_required(repository): os.unlink(tam_required_file(repository)) if key.tam_required: @@ -817,7 +831,7 @@ class Archiver: print(sysinfo()) return EXIT_SUCCESS - @with_repository() + @with_repository(compatibility=Manifest.NO_OPERATION_CHECK) def do_debug_dump_archive_items(self, args, repository, manifest, key): """dump (decrypted, decompressed) archive items metadata (not: data)""" archive = Archive(repository, key, manifest, args.location.archive) @@ -830,7 +844,7 @@ class Archiver: print('Done.') return EXIT_SUCCESS - @with_repository() + @with_repository(compatibility=Manifest.NO_OPERATION_CHECK) def do_debug_dump_repo_objs(self, args, repository, manifest, key): """dump (decrypted, decompressed) repo objects""" marker = None @@ -904,7 +918,7 @@ class Archiver: print('Done.') return EXIT_SUCCESS - @with_repository(manifest=False, exclusive=True, cache=True) + @with_repository(manifest=False, exclusive=True, cache=True, compatibility=Manifest.NO_OPERATION_CHECK) def do_debug_refcount_obj(self, args, repository, manifest, key, cache): """display refcounts for the objects with the given IDs""" for hex_id in args.ids: diff --git a/borg/helpers.py b/borg/helpers.py index a067b6af8..ee0476061 100644 --- a/borg/helpers.py +++ b/borg/helpers.py @@ -2,6 +2,7 @@ import argparse from binascii import hexlify from collections import namedtuple import contextlib +import enum from functools import wraps import grp import os @@ -110,6 +111,10 @@ class PlaceholderError(Error): """Formatting Error: "{}".format({}): {}({})""" +class MandatoryFeatureUnsupported(Error): + """Unsupported repository feature(s) {}. A newer version of borg is required to access this repository.""" + + def check_extension_modules(): from . import platform, compress if hashindex.API_VERSION != '1.0_01': @@ -126,6 +131,34 @@ def check_extension_modules(): class Manifest: + @enum.unique + class Operation(enum.Enum): + # The comments here only roughly describe the scope of each feature. In the end, additions need to be + # based on potential problems older clients could produce when accessing newer repositories and the + # tradeofs of locking version out or still allowing access. As all older versions and their exact + # behaviours are known when introducing new features sometimes this might not match the general descriptions + # below. + + # The READ operation describes which features are needed to safely list and extract the archives in the + # repository. + READ = 'read' + # The CHECK operation is for all operations that need either to understand every detail + # of the repository (for consistency checks and repairs) or are seldom used functions that just + # should use the most restrictive feature set because more fine grained compatibility tracking is + # not needed. + CHECK = 'check' + # The WRITE operation is for adding archives. Features here ensure that older clients don't add archives + # in an old format, or is used to lock out clients that for other reasons can no longer safely add new + # archives. + WRITE = 'write' + # The DELETE operation is for all operations (like archive deletion) that need a 100% correct reference + # count and the need to be able to find all (directly and indirectly) referenced chunks of a given archive. + DELETE = 'delete' + + NO_OPERATION_CHECK = tuple() + + SUPPORTED_REPO_FEATURES = frozenset([]) + MANIFEST_ID = b'\0' * 32 def __init__(self, key, repository, item_keys=None): @@ -139,7 +172,7 @@ class Manifest: self.timestamp = None @classmethod - def load(cls, repository, key=None, force_tam_not_required=False): + def load(cls, repository, operations, 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 @@ -173,8 +206,23 @@ class Manifest: 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)) + manifest.check_repository_compatibility(operations) return manifest, key + def check_repository_compatibility(self, operations): + for operation in operations: + assert isinstance(operation, self.Operation) + feature_flags = self.config.get(b'feature_flags', None) + if feature_flags is None: + return + if operation.value.encode() not in feature_flags: + continue + requirements = feature_flags[operation.value.encode()] + if b'mandatory' in requirements: + unsupported = set(requirements[b'mandatory']) - self.SUPPORTED_REPO_FEATURES + if unsupported: + raise MandatoryFeatureUnsupported([f.decode() for f in unsupported]) + def write(self): if self.key.tam_required: self.config[b'tam_required'] = True diff --git a/borg/testsuite/archiver.py b/borg/testsuite/archiver.py index aa2689953..0476a6096 100644 --- a/borg/testsuite/archiver.py +++ b/borg/testsuite/archiver.py @@ -248,7 +248,7 @@ class ArchiverTestCaseBase(BaseTestCase): def open_archive(self, name): repository = Repository(self.repository_path, exclusive=True) with repository: - manifest, key = Manifest.load(repository) + manifest, key = Manifest.load(repository, Manifest.NO_OPERATION_CHECK) archive = Archive(repository, key, manifest, name) return archive, repository @@ -815,7 +815,7 @@ class ArchiverTestCase(ArchiverTestCaseBase): self.cmd('extract', '--dry-run', self.repository_location + '::test.4') # Make sure both archives have been renamed with Repository(self.repository_path) as repository: - manifest, key = Manifest.load(repository) + manifest, key = Manifest.load(repository, Manifest.NO_OPERATION_CHECK) self.assert_equal(len(manifest.archives), 2) self.assert_in('test.3', manifest.archives) self.assert_in('test.4', manifest.archives) @@ -853,7 +853,7 @@ class ArchiverTestCase(ArchiverTestCaseBase): self.cmd('init', '--encryption=none', self.repository_location) self.create_src_archive('test') with Repository(self.repository_path, exclusive=True) as repository: - manifest, key = Manifest.load(repository) + manifest, key = Manifest.load(repository, Manifest.NO_OPERATION_CHECK) archive = Archive(repository, key, manifest, 'test') for item in archive.iter_items(): if 'chunks' in item: @@ -870,7 +870,7 @@ class ArchiverTestCase(ArchiverTestCaseBase): self.cmd('init', '--encryption=none', self.repository_location) self.create_src_archive('test') with Repository(self.repository_path, exclusive=True) as repository: - manifest, key = Manifest.load(repository) + manifest, key = Manifest.load(repository, Manifest.NO_OPERATION_CHECK) archive = Archive(repository, key, manifest, 'test') id = archive.metadata[b'items'][0] repository.put(id, b'corrupted items metadata stream chunk') @@ -915,7 +915,7 @@ class ArchiverTestCase(ArchiverTestCaseBase): self.cmd('create', '--dry-run', self.repository_location + '::test', 'input') # Make sure no archive has been created with Repository(self.repository_path) as repository: - manifest, key = Manifest.load(repository) + manifest, key = Manifest.load(repository, Manifest.NO_OPERATION_CHECK) self.assert_equal(len(manifest.archives), 0) def test_progress(self): @@ -1594,7 +1594,7 @@ class ArchiverCheckTestCase(ArchiverTestCaseBase): # This bug can still live on in Borg repositories (through borg upgrade). archive, repository = self.open_archive('archive1') with repository: - manifest, key = Manifest.load(repository) + manifest, key = Manifest.load(repository, Manifest.NO_OPERATION_CHECK) with Cache(repository, key, manifest) as cache: archive = Archive(repository, key, manifest, '0.13', cache=cache, create=True) archive.items_buffer.add({ @@ -1611,7 +1611,7 @@ class ArchiverCheckTestCase(ArchiverTestCaseBase): class ManifestAuthenticationTest(ArchiverTestCaseBase): def spoof_manifest(self, repository): with repository: - _, key = Manifest.load(repository) + _, key = Manifest.load(repository, Manifest.NO_OPERATION_CHECK) repository.put(Manifest.MANIFEST_ID, key.encrypt(msgpack.packb({ 'version': 1, 'archives': {}, @@ -1624,7 +1624,7 @@ class ManifestAuthenticationTest(ArchiverTestCaseBase): self.cmd('init', self.repository_location) repository = Repository(self.repository_path, exclusive=True) with repository: - manifest, key = Manifest.load(repository) + manifest, key = Manifest.load(repository, Manifest.NO_OPERATION_CHECK) repository.put(Manifest.MANIFEST_ID, key.encrypt(msgpack.packb({ 'version': 1, 'archives': {}, @@ -1641,7 +1641,7 @@ class ManifestAuthenticationTest(ArchiverTestCaseBase): repository = Repository(self.repository_path, exclusive=True) with repository: shutil.rmtree(get_security_dir(bin_to_hex(repository.id))) - _, key = Manifest.load(repository) + _, key = Manifest.load(repository, Manifest.NO_OPERATION_CHECK) key.tam_required = False key.change_passphrase(key._passphrase) From ca5ea4f53d6f2a829a1ce9a738f1cc741bba13ea Mon Sep 17 00:00:00 2001 From: Martin Hostettler Date: Sun, 12 Mar 2017 14:12:04 +0100 Subject: [PATCH 2/5] Add tests for mandatory repository feature flags. --- borg/testsuite/archiver.py | 63 +++++++++++++++++++++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/borg/testsuite/archiver.py b/borg/testsuite/archiver.py index 0476a6096..671607fa8 100644 --- a/borg/testsuite/archiver.py +++ b/borg/testsuite/archiver.py @@ -25,7 +25,7 @@ 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, \ - get_security_dir, MAX_S + get_security_dir, MAX_S, MandatoryFeatureUnsupported from ..key import RepoKey, KeyfileKey, Passphrase, TAMRequiredError from ..keymanager import RepoIdMismatch, NotABorgKeyFile from ..remote import RemoteRepository, PathNotAllowed @@ -918,6 +918,67 @@ class ArchiverTestCase(ArchiverTestCaseBase): manifest, key = Manifest.load(repository, Manifest.NO_OPERATION_CHECK) self.assert_equal(len(manifest.archives), 0) + def add_unknown_feature(self, operation): + with Repository(self.repository_path, exclusive=True) as repository: + manifest, key = Manifest.load(repository, Manifest.NO_OPERATION_CHECK) + manifest.config[b'feature_flags'] = {operation.value.encode(): {b'mandatory': [b'unknown-feature']}} + manifest.write() + repository.commit() + + def cmd_raises_unknown_feature(self, args): + if self.FORK_DEFAULT: + self.cmd(*args, exit_code=EXIT_ERROR) + else: + with pytest.raises(MandatoryFeatureUnsupported) as excinfo: + self.cmd(*args) + assert excinfo.value.args == (['unknown-feature'],) + + def test_unknown_feature_on_create(self): + print(self.cmd('init', self.repository_location)) + self.add_unknown_feature(Manifest.Operation.WRITE) + self.cmd_raises_unknown_feature(['create', self.repository_location + '::test', 'input']) + + def test_unknown_feature_on_change_passphrase(self): + print(self.cmd('init', self.repository_location)) + self.add_unknown_feature(Manifest.Operation.CHECK) + self.cmd_raises_unknown_feature(['change-passphrase', self.repository_location]) + + def test_unknown_feature_on_read(self): + print(self.cmd('init', self.repository_location)) + self.cmd('create', self.repository_location + '::test', 'input') + self.add_unknown_feature(Manifest.Operation.READ) + with changedir('output'): + self.cmd_raises_unknown_feature(['extract', self.repository_location + '::test']) + + self.cmd_raises_unknown_feature(['list', self.repository_location]) + self.cmd_raises_unknown_feature(['info', self.repository_location + '::test']) + + def test_unknown_feature_on_rename(self): + print(self.cmd('init', self.repository_location)) + self.cmd('create', self.repository_location + '::test', 'input') + self.add_unknown_feature(Manifest.Operation.CHECK) + self.cmd_raises_unknown_feature(['rename', self.repository_location + '::test', 'other']) + + def test_unknown_feature_on_delete(self): + print(self.cmd('init', self.repository_location)) + self.cmd('create', self.repository_location + '::test', 'input') + self.add_unknown_feature(Manifest.Operation.DELETE) + # delete of an archive raises + self.cmd_raises_unknown_feature(['delete', self.repository_location + '::test']) + self.cmd_raises_unknown_feature(['prune', '--keep-daily=3', self.repository_location]) + # delete of the whole repository ignores features + self.cmd('delete', self.repository_location) + + @unittest.skipUnless(has_llfuse, 'llfuse not installed') + def test_unknown_feature_on_mount(self): + self.cmd('init', self.repository_location) + self.cmd('create', self.repository_location + '::test', 'input') + self.add_unknown_feature(Manifest.Operation.READ) + mountpoint = os.path.join(self.tmpdir, 'mountpoint') + os.mkdir(mountpoint) + # XXX this might hang if it doesn't raise an error + self.cmd_raises_unknown_feature(['mount', self.repository_location + '::test', mountpoint]) + def test_progress(self): self.create_regular_file('file1', size=1024 * 80) self.cmd('init', self.repository_location) From 7f57086223d6634b91f2fb9dc43eccbed54e0a36 Mon Sep 17 00:00:00 2001 From: Martin Hostettler Date: Sun, 12 Mar 2017 11:04:26 +0100 Subject: [PATCH 3/5] permit manifest version 2 as well 1 one. --- borg/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/borg/helpers.py b/borg/helpers.py index ee0476061..2dfdbeb74 100644 --- a/borg/helpers.py +++ b/borg/helpers.py @@ -186,7 +186,7 @@ class Manifest: 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) - if not m.get(b'version') == 1: + if m.get(b'version') not in (1, 2): raise ValueError('Invalid manifest version') manifest.archives = dict((k.decode('utf-8'), v) for k, v in m[b'archives'].items()) manifest.timestamp = m.get(b'timestamp') From fb128fbf8429baeb1ecfcbc1fb3d71b4239dbf26 Mon Sep 17 00:00:00 2001 From: Martin Hostettler Date: Sun, 28 May 2017 18:04:33 +0200 Subject: [PATCH 4/5] Cache: Wipe cache if compatibility is not sure Add detection of possibly incompatible combinations of the borg versions maintaining the cache and the featues used. --- borg/cache.py | 51 +++++++++++++++++++++++++++++++++++++++++++++++-- borg/helpers.py | 16 ++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/borg/cache.py b/borg/cache.py index ef87984dc..51e13ab1e 100644 --- a/borg/cache.py +++ b/borg/cache.py @@ -9,8 +9,8 @@ import shutil from .key import PlaintextKey from .logger import create_logger logger = create_logger() -from .helpers import Error, get_cache_dir, decode_dict, int_to_bigint, \ - bigint_to_int, format_file_size, yes, bin_to_hex, Location, safe_ns +from .helpers import Error, Manifest, get_cache_dir, decode_dict, int_to_bigint, \ + bigint_to_int, format_file_size, yes, bin_to_hex, Location, safe_ns, parse_stringified_list from .locking import Lock from .hashindex import ChunkIndex @@ -84,6 +84,11 @@ class Cache: self.begin_txn() self.commit() + if not self.check_cache_compatibility(): + self.wipe_cache() + + self.update_compatibility() + if sync and self.manifest.id != self.manifest_id: # If repository is older than the cache something fishy is going on if self.timestamp and self.timestamp > manifest.timestamp: @@ -94,6 +99,7 @@ class Cache: # Make sure an encrypted repository has not been swapped for an unencrypted repository if self.key_type is not None and self.key_type != str(key.TYPE): raise self.EncryptionMethodMismatch() + self.sync() self.commit() except: @@ -175,6 +181,8 @@ Chunk index: {0.total_unique_chunks:20d} {0.total_chunks:20d}""" self.timestamp = self.config.get('cache', 'timestamp', fallback=None) self.key_type = self.config.get('cache', 'key_type', fallback=None) self.previous_location = self.config.get('cache', 'previous_location', fallback=None) + self.ignored_features = set(parse_stringified_list(self.config.get('cache', 'ignored_features', fallback=''))) + self.mandatory_features = set(parse_stringified_list(self.config.get('cache', 'mandatory_features', fallback=''))) self.chunks = ChunkIndex.read(os.path.join(self.path, 'chunks').encode('utf-8')) self.files = None @@ -240,6 +248,8 @@ Chunk index: {0.total_unique_chunks:20d} {0.total_chunks:20d}""" self.config.set('cache', 'timestamp', self.manifest.timestamp) self.config.set('cache', 'key_type', str(self.key.TYPE)) self.config.set('cache', 'previous_location', self.repository._location.canonical_path()) + self.config.set('cache', 'ignored_features', ','.join(self.ignored_features)) + self.config.set('cache', 'mandatory_features', ','.join(self.mandatory_features)) with open(os.path.join(self.path, 'config'), 'w') as fd: self.config.write(fd) self.chunks.write(os.path.join(self.path, 'chunks').encode('utf-8')) @@ -390,6 +400,43 @@ Chunk index: {0.total_unique_chunks:20d} {0.total_chunks:20d}""" self.do_cache = os.path.isdir(archive_path) self.chunks = create_master_idx(self.chunks) + def check_cache_compatibility(self): + my_features = Manifest.SUPPORTED_REPO_FEATURES + if self.ignored_features & my_features: + # The cache might not contain references of chunks that need a feature that is mandatory for some operation + # and which this version supports. To avoid corruption while executing that operation force rebuild. + return False + if not self.mandatory_features <= my_features: + # The cache was build with consideration to at least one feature that this version does not understand. + # This client might misinterpret the cache. Thus force a rebuild. + return False + return True + + def wipe_cache(self): + logger.warning("Discarding incompatible cache and forcing a cache rebuild") + archive_path = os.path.join(self.path, 'chunks.archive.d') + if os.path.isdir(archive_path): + shutil.rmtree(os.path.join(self.path, 'chunks.archive.d')) + os.makedirs(os.path.join(self.path, 'chunks.archive.d')) + self.chunks = ChunkIndex() + with open(os.path.join(self.path, 'files'), 'wb'): + pass # empty file + self.manifest_id = '' + self.config.set('cache', 'manifest', '') + + self.ignored_features = set() + self.mandatory_features = set() + + def update_compatibility(self): + operation_to_features_map = self.manifest.get_all_mandatory_features() + my_features = Manifest.SUPPORTED_REPO_FEATURES + repo_features = set() + for operation, features in operation_to_features_map.items(): + repo_features.update(features) + + self.ignored_features.update(repo_features - my_features) + self.mandatory_features.update(repo_features & my_features) + def add_chunk(self, id, data, stats): if not self.txn_active: self.begin_txn() diff --git a/borg/helpers.py b/borg/helpers.py index 2dfdbeb74..9ae81f6c5 100644 --- a/borg/helpers.py +++ b/borg/helpers.py @@ -223,6 +223,17 @@ class Manifest: if unsupported: raise MandatoryFeatureUnsupported([f.decode() for f in unsupported]) + def get_all_mandatory_features(self): + result = {} + feature_flags = self.config.get(b'feature_flags', None) + if feature_flags is None: + return result + + for operation, requirements in feature_flags.items(): + if b'mandatory' in requirements: + result[operation.decode()] = set([feature.decode() for feature in requirements[b'mandatory']]) + return result + def write(self): if self.key.tam_required: self.config[b'tam_required'] = True @@ -929,6 +940,11 @@ def bin_to_hex(binary): return hexlify(binary).decode('ascii') +def parse_stringified_list(s): + l = re.split(" *, *", s) + return [item for item in l if item != ''] + + class Location: """Object representing a repository / archive location """ From 6ce476a42b7a6044fdaf8e0c97f8c8b1eb52b518 Mon Sep 17 00:00:00 2001 From: Martin Hostettler Date: Tue, 6 Jun 2017 22:37:53 +0200 Subject: [PATCH 5/5] Add tests for cache compatibility code. --- borg/testsuite/archiver.py | 43 +++++++++++++++++++++++++++++++++++++- conftest.py | 21 +++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/borg/testsuite/archiver.py b/borg/testsuite/archiver.py index 671607fa8..211b25efd 100644 --- a/borg/testsuite/archiver.py +++ b/borg/testsuite/archiver.py @@ -25,7 +25,7 @@ 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, \ - get_security_dir, MAX_S, MandatoryFeatureUnsupported + get_security_dir, MAX_S, MandatoryFeatureUnsupported, Location from ..key import RepoKey, KeyfileKey, Passphrase, TAMRequiredError from ..keymanager import RepoIdMismatch, NotABorgKeyFile from ..remote import RemoteRepository, PathNotAllowed @@ -979,6 +979,47 @@ class ArchiverTestCase(ArchiverTestCaseBase): # XXX this might hang if it doesn't raise an error self.cmd_raises_unknown_feature(['mount', self.repository_location + '::test', mountpoint]) + @pytest.mark.allow_cache_wipe + def test_unknown_mandatory_feature_in_cache(self): + if self.prefix: + path_prefix = 'ssh://__testsuite__' + else: + path_prefix = '' + + print(self.cmd('init', self.repository_location)) + + with Repository(self.repository_path, exclusive=True) as repository: + if path_prefix: + repository._location = Location(self.repository_location) + manifest, key = Manifest.load(repository, Manifest.NO_OPERATION_CHECK) + with Cache(repository, key, manifest) as cache: + cache.begin_txn() + cache.mandatory_features = set(['unknown-feature']) + cache.commit() + + if self.FORK_DEFAULT: + self.cmd('create', self.repository_location + '::test', 'input') + else: + called = False + wipe_cache_safe = Cache.wipe_cache + + def wipe_wrapper(*args): + nonlocal called + called = True + wipe_cache_safe(*args) + + with patch.object(Cache, 'wipe_cache', wipe_wrapper): + self.cmd('create', self.repository_location + '::test', 'input') + + assert called + + with Repository(self.repository_path, exclusive=True) as repository: + if path_prefix: + repository._location = Location(self.repository_location) + manifest, key = Manifest.load(repository, Manifest.NO_OPERATION_CHECK) + with Cache(repository, key, manifest) as cache: + assert cache.mandatory_features == set([]) + def test_progress(self): self.create_regular_file('file1', size=1024 * 80) self.cmd('init', self.repository_location) diff --git a/conftest.py b/conftest.py index 596a4b21f..808094066 100644 --- a/conftest.py +++ b/conftest.py @@ -4,6 +4,8 @@ import sys import pytest +import borg.cache + # needed to get pretty assertion failures in unit tests: if hasattr(pytest, 'register_assert_rewrite'): pytest.register_assert_rewrite('borg.testsuite') @@ -35,3 +37,22 @@ def clean_env(tmpdir_factory, monkeypatch): keys = [key for key in os.environ if key.startswith('BORG_')] for key in keys: monkeypatch.delenv(key, raising=False) + + +class DefaultPatches: + def __init__(self, request): + self.org_cache_wipe_cache = borg.cache.Cache.wipe_cache + + def wipe_should_not_be_called(*a, **kw): + raise AssertionError("Cache wipe was triggered, if this is part of the test add @pytest.mark.allow_cache_wipe") + if 'allow_cache_wipe' not in request.keywords: + borg.cache.Cache.wipe_cache = wipe_should_not_be_called + request.addfinalizer(self.undo) + + def undo(self): + borg.cache.Cache.wipe_cache = self.org_cache_wipe_cache + + +@pytest.fixture(autouse=True) +def default_patches(request): + return DefaultPatches(request)