From 2d637d8ead3028beca0ea4a4ee6d68048943e898 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 27 Jan 2018 21:43:12 +0100 Subject: [PATCH 1/2] borg config: add some validation, fixes #3566 only a few basic checks, it does not prevent all stupidities. --- src/borg/archiver.py | 41 ++++++++++++++++++++++++++++++++++ src/borg/testsuite/archiver.py | 19 +++++++++------- 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/src/borg/archiver.py b/src/borg/archiver.py index 8a3acbdc4..6c84b258b 100644 --- a/src/borg/archiver.py +++ b/src/borg/archiver.py @@ -1523,6 +1523,43 @@ class Archiver: @with_repository(exclusive=True, cache=True, compatibility=(Manifest.Operation.WRITE,)) def do_config(self, args, repository, manifest, key, cache): """get, set, and delete values in a repository or cache config file""" + + def repo_validate(section, name, value=None, check_value=True): + if section not in ['repository', ]: + raise ValueError('Invalid section') + if name in ['segments_per_dir', 'max_segment_size', 'storage_quota', ]: + if check_value: + try: + int(value) + except ValueError: + raise ValueError('Invalid value') from None + elif name in ['additional_free_space', ]: + if check_value: + try: + parse_file_size(value) + except ValueError: + raise ValueError('Invalid value') from None + elif name in ['append_only', ]: + if check_value and value not in ['0', '1']: + raise ValueError('Invalid value') + elif name in ['id', ]: + if check_value: + try: + bin_id = unhexlify(value) + except: + raise ValueError('Invalid value, must be 64 hex digits') from None + if len(bin_id) != 32: + raise ValueError('Invalid value, must be 64 hex digits') + else: + raise ValueError('Invalid name') + + def cache_validate(section, name, value=None, check_value=True): + if section not in ['cache', ]: + raise ValueError('Invalid section') + # I looked at the cache config and did not see anything a user would want to edit, + # so, for now, raise for any key name + raise ValueError('Invalid name') + try: section, name = args.name.split('.') except ValueError: @@ -1533,16 +1570,20 @@ class Archiver: cache.cache_config.load() config = cache.cache_config._config save = cache.cache_config.save + validate = cache_validate else: config = repository.config save = lambda: repository.save_config(repository.path, repository.config) + validate = repo_validate if args.delete: + validate(section, name, check_value=False) config.remove_option(section, name) if len(config.options(section)) == 0: config.remove_section(section) save() elif args.value: + validate(section, name, args.value) if section not in config.sections(): config.add_section(section) config.set(section, name, args.value) diff --git a/src/borg/testsuite/archiver.py b/src/borg/testsuite/archiver.py index 2868b0dd4..12c536344 100644 --- a/src/borg/testsuite/archiver.py +++ b/src/borg/testsuite/archiver.py @@ -2782,14 +2782,17 @@ id: 2 / e29442 3506da 4e1ea7 / 25f62a 5a3d41 - 02 self.create_test_files() os.unlink('input/flagfile') self.cmd('init', '--encryption=repokey', self.repository_location) - for flags in [[], ['--cache']]: - for cfg_key in {'testkey', 'testsection.testkey'}: - self.cmd('config', self.repository_location, *flags, cfg_key, exit_code=1) - self.cmd('config', self.repository_location, *flags, cfg_key, 'testcontents') - output = self.cmd('config', self.repository_location, *flags, cfg_key) - assert output == 'testcontents\n' - self.cmd('config', self.repository_location, *flags, '--delete', cfg_key) - self.cmd('config', self.repository_location, *flags, cfg_key, exit_code=1) + for cfg_key, cfg_value in [ + ('additional_free_space', '2G'), + ('repository.append_only', '1'), + ]: + output = self.cmd('config', self.repository_location, cfg_key) + assert output == '0' + '\n' + self.cmd('config', self.repository_location, cfg_key, cfg_value) + output = self.cmd('config', self.repository_location, cfg_key) + assert output == cfg_value + '\n' + self.cmd('config', self.repository_location, '--delete', cfg_key) + self.cmd('config', self.repository_location, cfg_key, exit_code=1) requires_gnutar = pytest.mark.skipif(not have_gnutar(), reason='GNU tar must be installed for this test.') requires_gzip = pytest.mark.skipif(not shutil.which('gzip'), reason='gzip must be installed for this test.') From 0e0e6da585ef81e6b5a20a187851e17f1a0b64ae Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 3 Mar 2018 00:17:40 +0100 Subject: [PATCH 2/2] make sure all segment file offsets fit into uint32, fixes #3592 C code and the repo index use uint32 type for segment file offsets, so when opening a repo and the config max_segment_size is too big, fail early. Also disallow setting a too big value via "borg config". --- src/borg/archiver.py | 3 +++ src/borg/constants.py | 3 +++ src/borg/repository.py | 6 ++++++ 3 files changed, 12 insertions(+) diff --git a/src/borg/archiver.py b/src/borg/archiver.py index 6c84b258b..b56d49450 100644 --- a/src/borg/archiver.py +++ b/src/borg/archiver.py @@ -1533,6 +1533,9 @@ class Archiver: int(value) except ValueError: raise ValueError('Invalid value') from None + if name == 'max_segment_size': + if int(value) >= MAX_SEGMENT_SIZE_LIMIT: + raise ValueError('Invalid value: max_segment_size >= %d' % MAX_SEGMENT_SIZE_LIMIT) elif name in ['additional_free_space', ]: if check_value: try: diff --git a/src/borg/constants.py b/src/borg/constants.py index 77129ea18..5a67da54a 100644 --- a/src/borg/constants.py +++ b/src/borg/constants.py @@ -36,6 +36,9 @@ MAX_DATA_SIZE = 20971479 MAX_OBJECT_SIZE = MAX_DATA_SIZE + 41 # see LoggedIO.put_header_fmt.size assertion in repository module assert MAX_OBJECT_SIZE == 20 * 1024 * 1024 +# repo config max_segment_size value must be below this limit to stay within uint32 offsets: +MAX_SEGMENT_SIZE_LIMIT = 2 ** 32 - MAX_OBJECT_SIZE + # borg.remote read() buffer size BUFSIZE = 10 * 1024 * 1024 diff --git a/src/borg/repository.py b/src/borg/repository.py index 671415bfc..7fbc00ef6 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -122,6 +122,9 @@ class Repository: class InvalidRepository(Error): """{} is not a valid repository. Check repo config.""" + class InvalidRepositoryConfig(Error): + """{} does not have a valid configuration. Check repo config [{}].""" + class AtticRepository(Error): """Attic repository detected. Please run "borg upgrade {}".""" @@ -383,6 +386,9 @@ class Repository: self.close() raise self.InvalidRepository(path) self.max_segment_size = self.config.getint('repository', 'max_segment_size') + if self.max_segment_size >= MAX_SEGMENT_SIZE_LIMIT: + self.close() + raise self.InvalidRepositoryConfig(path, 'max_segment_size >= %d' % MAX_SEGMENT_SIZE_LIMIT) # issue 3592 self.segments_per_dir = self.config.getint('repository', 'segments_per_dir') self.additional_free_space = parse_file_size(self.config.get('repository', 'additional_free_space', fallback=0)) # append_only can be set in the constructor