From a09085c4e2dca0be16b549bcd75cf80f786c7cd6 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 24 Mar 2024 14:53:28 +0100 Subject: [PATCH 1/6] upgrade --check-archives-tam: check archives tam status This is a read-only variation of --archives-tam: - it only checks / displays the current status - it does not add / fix the archive TAMs --- .pre-commit-config.yaml | 2 +- src/borg/archiver.py | 36 ++++++++++++++++++++++-------------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f7a03542..0c1a27a7 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,6 +1,6 @@ repos: - repo: https://github.com/pycqa/flake8 - rev: 6.0.0 + rev: 6.1.0 hooks: - id: flake8 files: '(src|scripts|conftest.py)' diff --git a/src/borg/archiver.py b/src/borg/archiver.py index b5edc6cb..80ee5166 100644 --- a/src/borg/archiver.py +++ b/src/borg/archiver.py @@ -1635,10 +1635,11 @@ class Archiver: DASHES, logger=logging.getLogger('borg.output.stats')) return self.exit_code - @with_repository(fake=('tam', 'disable_tam', 'archives_tam'), invert_fake=True, manifest=False, exclusive=True) + @with_repository(fake=('tam', 'disable_tam', 'archives_tam', 'check_archives_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.archives_tam: + if args.archives_tam or args.check_archives_tam: + read_only = args.check_archives_tam manifest, key = Manifest.load(repository, (Manifest.Operation.CHECK,), force_tam_not_required=args.force) with Cache(repository, key, manifest) as cache: stats = Statistics() @@ -1648,20 +1649,25 @@ class Archiver: cdata = repository.get(archive_id) data = key.decrypt(archive_id, cdata) archive, verified, _ = key.unpack_and_verify_archive(data, force_tam_not_required=True) - if not verified: # we do not have an archive TAM yet -> add TAM now! - archive = ArchiveItem(internal_dict=archive) - archive.cmdline = [safe_decode(arg) for arg in archive.cmdline] - data = key.pack_and_authenticate_metadata(archive.as_dict(), context=b'archive') - new_archive_id = key.id_hash(data) - cache.add_chunk(new_archive_id, data, stats) - cache.chunk_decref(archive_id, stats) - manifest.archives[info.name] = (new_archive_id, info.ts) - print(f"Added archive TAM: {archive_formatted} -> [{bin_to_hex(new_archive_id)}]") + if not verified: + if not read_only: + # we do not have an archive TAM yet -> add TAM now! + archive = ArchiveItem(internal_dict=archive) + archive.cmdline = [safe_decode(arg) for arg in archive.cmdline] + data = key.pack_and_authenticate_metadata(archive.as_dict(), context=b'archive') + new_archive_id = key.id_hash(data) + cache.add_chunk(new_archive_id, data, stats) + cache.chunk_decref(archive_id, stats) + manifest.archives[info.name] = (new_archive_id, info.ts) + print(f"Added archive TAM: {archive_formatted} -> [{bin_to_hex(new_archive_id)}]") + else: + print(f"Archive TAM missing: {archive_formatted}") else: print(f"Archive TAM present: {archive_formatted}") - manifest.write() - repository.commit(compact=False) - cache.commit() + if not read_only: + manifest.write() + repository.commit(compact=False) + cache.commit() elif args.tam: manifest, key = Manifest.load(repository, (Manifest.Operation.CHECK,), force_tam_not_required=args.force) if not manifest.tam_verified or not manifest.config.get(b'tam_required', False): @@ -4998,6 +5004,8 @@ class Archiver: 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('--check-archives-tam', dest='check_archives_tam', action='store_true', + help='check TAM authentication for all archives.') subparser.add_argument('--archives-tam', dest='archives_tam', action='store_true', help='add TAM authentication for all archives.') subparser.add_argument('location', metavar='REPOSITORY', nargs='?', default='', From 6154a83355b4b6c110f7595d6730d2a6485905cd Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 24 Mar 2024 15:14:14 +0100 Subject: [PATCH 2/6] upgrade --check-archives-tam: rc=1 for archive TAM issues also: better messages about global status. --- src/borg/archiver.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/borg/archiver.py b/src/borg/archiver.py index 80ee5166..bc1497d2 100644 --- a/src/borg/archiver.py +++ b/src/borg/archiver.py @@ -1639,6 +1639,7 @@ class Archiver: def do_upgrade(self, args, repository, manifest=None, key=None): """upgrade a repository from a previous version""" if args.archives_tam or args.check_archives_tam: + archive_tam_issues = 0 read_only = args.check_archives_tam manifest, key = Manifest.load(repository, (Manifest.Operation.CHECK,), force_tam_not_required=args.force) with Cache(repository, key, manifest) as cache: @@ -1662,12 +1663,23 @@ class Archiver: print(f"Added archive TAM: {archive_formatted} -> [{bin_to_hex(new_archive_id)}]") else: print(f"Archive TAM missing: {archive_formatted}") + archive_tam_issues += 1 else: print(f"Archive TAM present: {archive_formatted}") if not read_only: manifest.write() repository.commit(compact=False) cache.commit() + if archive_tam_issues > 0: + print(f"Fixed {archive_tam_issues} archives with TAM issues!") + print("All archives are TAM authenticated now.") + else: + print("All archives are TAM authenticated.") + else: + if archive_tam_issues > 0: + self.print_warning(f"Found {archive_tam_issues} archives with TAM issues!") + else: + print("All archives are TAM authenticated.") elif args.tam: manifest, key = Manifest.load(repository, (Manifest.Operation.CHECK,), force_tam_not_required=args.force) if not manifest.tam_verified or not manifest.config.get(b'tam_required', False): From 367b38e3d477cc1100042bc7bfdcf675270e0e27 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 24 Mar 2024 17:47:51 +0100 Subject: [PATCH 3/6] do a late lookup on helpers.workarounds so we can modify it within borg code elsewhere and it will get the updated value when it accesses the attribute. --- src/borg/crypto/key.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/borg/crypto/key.py b/src/borg/crypto/key.py index 57c528ae..2f6f1ed7 100644 --- a/src/borg/crypto/key.py +++ b/src/borg/crypto/key.py @@ -14,6 +14,7 @@ from ..logger import create_logger logger = create_logger() +from .. import helpers from ..constants import * # NOQA from ..compress import Compressor from ..helpers import StableDict @@ -24,7 +25,6 @@ from ..helpers import get_limited_unpacker from ..helpers import bin_to_hex from ..helpers import prepare_subprocess_env from ..helpers import msgpack -from ..helpers import workarounds from ..item import Key, EncryptedKey from ..platform import SaveFile @@ -34,7 +34,7 @@ from .low_level import AES256_CTR_HMAC_SHA256, AES256_CTR_BLAKE2b # workaround for lost passphrase or key in "authenticated" or "authenticated-blake2" mode -AUTHENTICATED_NO_KEY = 'authenticated_no_key' in workarounds +AUTHENTICATED_NO_KEY = 'authenticated_no_key' in helpers.workarounds class NoPassphraseFailure(Error): @@ -322,7 +322,7 @@ class KeyBase: tam_key = self._tam_key(tam_salt, context=b'archive') calculated_hmac = hmac.digest(tam_key, data, 'sha512') if not hmac.compare_digest(calculated_hmac, tam_hmac): - if 'ignore_invalid_archive_tam' in workarounds: + if 'ignore_invalid_archive_tam' in helpers.workarounds: logger.debug('ignoring invalid archive TAM due to BORG_WORKAROUNDS') return unpacked, False, None # same as if no TAM is present else: From 6cf4799bb2ca379e7e1fa514ef84ee68676d0afe Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 24 Mar 2024 18:17:21 +0100 Subject: [PATCH 4/6] upgrade: auto-use ignore_invalid_archive_tam --- src/borg/archiver.py | 80 ++++++++++++++++++------------------ src/borg/helpers/__init__.py | 13 ++++++ 2 files changed, 54 insertions(+), 39 deletions(-) diff --git a/src/borg/archiver.py b/src/borg/archiver.py index bc1497d2..59ee4f92 100644 --- a/src/borg/archiver.py +++ b/src/borg/archiver.py @@ -75,6 +75,7 @@ try: from .helpers import sig_int, ignore_sigint from .helpers import iter_separated from .helpers import get_tar_filter + from .helpers import ignore_invalid_archive_tam from .helpers.parseformat import BorgJsonEncoder, safe_decode from .nanorst import rst_to_terminal from .patterns import ArgparsePatternAction, ArgparseExcludeFileAction, ArgparsePatternFileAction, parse_exclude_pattern @@ -1639,47 +1640,48 @@ class Archiver: def do_upgrade(self, args, repository, manifest=None, key=None): """upgrade a repository from a previous version""" if args.archives_tam or args.check_archives_tam: - archive_tam_issues = 0 - read_only = args.check_archives_tam - manifest, key = Manifest.load(repository, (Manifest.Operation.CHECK,), force_tam_not_required=args.force) - with Cache(repository, key, manifest) as cache: - stats = Statistics() - for info in manifest.archives.list(sort_by=['ts']): - archive_id = info.id - archive_formatted = format_archive(info) - cdata = repository.get(archive_id) - data = key.decrypt(archive_id, cdata) - archive, verified, _ = key.unpack_and_verify_archive(data, force_tam_not_required=True) - if not verified: - if not read_only: - # we do not have an archive TAM yet -> add TAM now! - archive = ArchiveItem(internal_dict=archive) - archive.cmdline = [safe_decode(arg) for arg in archive.cmdline] - data = key.pack_and_authenticate_metadata(archive.as_dict(), context=b'archive') - new_archive_id = key.id_hash(data) - cache.add_chunk(new_archive_id, data, stats) - cache.chunk_decref(archive_id, stats) - manifest.archives[info.name] = (new_archive_id, info.ts) - print(f"Added archive TAM: {archive_formatted} -> [{bin_to_hex(new_archive_id)}]") + with ignore_invalid_archive_tam(): + archive_tam_issues = 0 + read_only = args.check_archives_tam + manifest, key = Manifest.load(repository, (Manifest.Operation.CHECK,), force_tam_not_required=args.force) + with Cache(repository, key, manifest) as cache: + stats = Statistics() + for info in manifest.archives.list(sort_by=['ts']): + archive_id = info.id + archive_formatted = format_archive(info) + cdata = repository.get(archive_id) + data = key.decrypt(archive_id, cdata) + archive, verified, _ = key.unpack_and_verify_archive(data, force_tam_not_required=True) + if not verified: + if not read_only: + # we do not have an archive TAM yet -> add TAM now! + archive = ArchiveItem(internal_dict=archive) + archive.cmdline = [safe_decode(arg) for arg in archive.cmdline] + data = key.pack_and_authenticate_metadata(archive.as_dict(), context=b'archive') + new_archive_id = key.id_hash(data) + cache.add_chunk(new_archive_id, data, stats) + cache.chunk_decref(archive_id, stats) + manifest.archives[info.name] = (new_archive_id, info.ts) + print(f"Added archive TAM: {archive_formatted} -> [{bin_to_hex(new_archive_id)}]") + else: + print(f"Archive TAM missing: {archive_formatted}") + archive_tam_issues += 1 else: - print(f"Archive TAM missing: {archive_formatted}") - archive_tam_issues += 1 + print(f"Archive TAM present: {archive_formatted}") + if not read_only: + manifest.write() + repository.commit(compact=False) + cache.commit() + if archive_tam_issues > 0: + print(f"Fixed {archive_tam_issues} archives with TAM issues!") + print("All archives are TAM authenticated now.") + else: + print("All archives are TAM authenticated.") else: - print(f"Archive TAM present: {archive_formatted}") - if not read_only: - manifest.write() - repository.commit(compact=False) - cache.commit() - if archive_tam_issues > 0: - print(f"Fixed {archive_tam_issues} archives with TAM issues!") - print("All archives are TAM authenticated now.") - else: - print("All archives are TAM authenticated.") - else: - if archive_tam_issues > 0: - self.print_warning(f"Found {archive_tam_issues} archives with TAM issues!") - else: - print("All archives are TAM authenticated.") + if archive_tam_issues > 0: + self.print_warning(f"Found {archive_tam_issues} archives with TAM issues!") + else: + print("All archives are TAM authenticated.") elif args.tam: manifest, key = Manifest.load(repository, (Manifest.Operation.CHECK,), force_tam_not_required=args.force) if not manifest.tam_verified or not manifest.config.get(b'tam_required', False): diff --git a/src/borg/helpers/__init__.py b/src/borg/helpers/__init__.py index 34a59b90..b24d61a6 100644 --- a/src/borg/helpers/__init__.py +++ b/src/borg/helpers/__init__.py @@ -5,6 +5,7 @@ that did not fit better elsewhere. Code used to be in borg/helpers.py but was split into the modules in this package, which are imported into here for compatibility. """ +from contextlib import contextmanager from .checks import * # NOQA from .datastruct import * # NOQA @@ -26,6 +27,18 @@ from . import msgpack # see the docs for a list of known workaround strings. workarounds = tuple(os.environ.get('BORG_WORKAROUNDS', '').split(',')) + +@contextmanager +def ignore_invalid_archive_tam(): + global workarounds + saved = workarounds + if 'ignore_invalid_archive_tam' not in workarounds: + # we really need this workaround here or borg will likely raise an exception. + workarounds += ('ignore_invalid_archive_tam',) + yield + workarounds = saved + + """ The global exit_code variable is used so that modules other than archiver can increase the program exit code if a warning or error occurred during their operation. This is different from archiver.exit_code, which is only accessible From 3f1a02fa746514df42c8f30de20090f3e71550d8 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 24 Mar 2024 19:08:14 +0100 Subject: [PATCH 5/6] upgrade --check-tam: check manifest TAM auth issues exit with rc=1 if there are issues. --- src/borg/archiver.py | 59 +++++++++++++++++++++++++++++--------------- 1 file changed, 39 insertions(+), 20 deletions(-) diff --git a/src/borg/archiver.py b/src/borg/archiver.py index 59ee4f92..e46fe110 100644 --- a/src/borg/archiver.py +++ b/src/borg/archiver.py @@ -1636,7 +1636,7 @@ class Archiver: DASHES, logger=logging.getLogger('borg.output.stats')) return self.exit_code - @with_repository(fake=('tam', 'disable_tam', 'archives_tam', 'check_archives_tam'), invert_fake=True, manifest=False, exclusive=True) + @with_repository(fake=('tam', 'check_tam', 'disable_tam', 'archives_tam', 'check_archives_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.archives_tam or args.check_archives_tam: @@ -1682,25 +1682,42 @@ class Archiver: self.print_warning(f"Found {archive_tam_issues} archives with TAM issues!") else: print("All archives are TAM authenticated.") - elif args.tam: - manifest, key = Manifest.load(repository, (Manifest.Operation.CHECK,), force_tam_not_required=args.force) - if not manifest.tam_verified or not manifest.config.get(b'tam_required', False): - print('Manifest contents:') - for archive_info in manifest.archives.list(sort_by=['ts']): - print(format_archive(archive_info)) - manifest.config[b'tam_required'] = True - manifest.write() - repository.commit(compact=False) - if not key.tam_required and hasattr(key, 'change_passphrase'): - key.tam_required = True - key.change_passphrase(key._passphrase) - 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.tam or args.check_tam: + with ignore_invalid_archive_tam(): + manifest_tam_issues = 0 + read_only = args.check_tam + manifest, key = Manifest.load(repository, (Manifest.Operation.CHECK,), force_tam_not_required=args.force) + if not manifest.tam_verified or not manifest.config.get(b'tam_required', False): + if not read_only: + print('Manifest contents:') + for archive_info in manifest.archives.list(sort_by=['ts']): + print(format_archive(archive_info)) + manifest.config[b'tam_required'] = True + manifest.write() + repository.commit(compact=False) + else: + manifest_tam_issues += 1 + self.print_warning("Repository Manifest is not TAM verified or a TAM is not required!") + if not key.tam_required and hasattr(key, 'change_passphrase'): + if not read_only: + key.tam_required = True + key.change_passphrase(key._passphrase) + print('Key updated') + if hasattr(key, 'find_key'): + print('Key location:', key.find_key()) + else: + manifest_tam_issues += 1 + self.print_warning("Key does not require TAM authentication!") + if not tam_required(repository): + if not read_only: + tam_file = tam_required_file(repository) + open(tam_file, 'w').close() + print('Updated security database') + else: + manifest_tam_issues += 1 + self.print_warning("Client-side security database does not require a TAM!") + if read_only and manifest_tam_issues == 0: + print("Manifest authentication setup OK for this client and this repository.") elif args.disable_tam: manifest, key = Manifest.load(repository, Manifest.NO_OPERATION_CHECK, force_tam_not_required=True) if tam_required(repository): @@ -5016,6 +5033,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('--check-tam', dest='check_tam', action='store_true', + help='check manifest authentication (in key and cache).') subparser.add_argument('--disable-tam', dest='disable_tam', action='store_true', help='Disable manifest authentication (in key and cache).') subparser.add_argument('--check-archives-tam', dest='check_archives_tam', action='store_true', From 04c8bc634251861365ddf0a7135f534af3711b52 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 24 Mar 2024 17:46:22 +0100 Subject: [PATCH 6/6] docs: simplify TAM-related upgrade docs using the new commands --- docs/changes.rst | 56 ++++++++++++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 21 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 50c29ee9..96da705b 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -29,49 +29,63 @@ places. Borg now considers archives without TAM as garbage or an attack. We are not aware of others having discovered, disclosed or exploited this vulnerability. -Below, if we speak of borg 1.2.6, we mean a borg version >= 1.2.6 **or** a -borg version that has the relevant security patches for this vulnerability applied +Below, if we speak of borg 1.2.8, we mean a borg version >= 1.2.8 **or** a +borg version that has the relevant patches for this vulnerability applied (could be also an older version in that case). Steps you must take to upgrade a repository (this applies to all kinds of repos no matter what encryption mode they use, including "none"): -1. Upgrade all clients using this repository to borg 1.2.6. +1. Upgrade all clients using this repository to borg 1.2.8. Note: it is not required to upgrade a server, except if the server-side borg is also used as a client (and not just for "borg serve"). - Do **not** run ``borg check`` with borg 1.2.6 before completing the upgrade steps: + Do **not** run ``borg check`` with borg > 1.2.4 before completing the upgrade steps: - ``borg check`` would complain about archives without a valid archive TAM. - ``borg check --repair`` would remove such archives! -2. Run: ``BORG_WORKAROUNDS=ignore_invalid_archive_tam borg info --debug 2>&1 | grep TAM | grep -i manifest`` +2. Do this step on every client using this repo: ``borg upgrade --show-rc --check-tam `` - a) If you get "TAM-verified manifest", continue with 3. - b) If you get "Manifest TAM not found and not required", run - ``borg upgrade --tam --force `` *on every client*. + This will check the manifest TAM authentication setup in the repo and on this client. + The command will exit with rc=0 if all is OK, otherwise with rc=1. -3. Run: ``BORG_WORKAROUNDS=ignore_invalid_archive_tam borg list --consider-checkpoints --format='{name} {time} tam:{tam}{NL}' `` + a) If you get "Manifest authentication setup OK for this client and this repository." + and rc=0, continue with 3. + b) If you get some warnings and rc=1, run: + ``borg upgrade --tam --force `` - "tam:verified" means that the archive has a valid TAM authentication. - "tam:none" is expected as output for archives created by borg <1.0.9. - "tam:none" is also expected for archives resulting from a borg rename - or borg recreate operation (see #7791). - "tam:none" could also come from archives created by an attacker. - You should verify that "tam:none" archives are authentic and not malicious +3. Run: ``borg upgrade --show-rc --check-archives-tam `` + + This will create a report about the TAM status for all archives. + In the last line(s) of the report, it will also report the overall status. + The command will exit with rc=0 if all archives are TAM authenticated or with rc=1 + if there are some archives with TAM issues. + + If there are no issues and all archives are TAM authenticated, continue with 5. + + Archive TAM issues are expected for: + + - archives created by borg <1.0.9. + - archives resulting from a borg rename or borg recreate operation (see #7791) + + But, important, archive TAM issues could also come from archives created by an attacker. + You should verify that archives with TAM issues are authentic and not malicious (== have good content, have correct timestamp, can be extracted successfully). In case you find crappy/malicious archives, you must delete them before proceeding. + In low-risk, trusted environments, you may decide on your own risk to skip step 3 and just trust in everything being OK. -4. If there are no tam:none archives left at this point, you can skip this step. - Run ``BORG_WORKAROUNDS=ignore_invalid_archive_tam borg upgrade --archives-tam ``. +4. If there are no archives with TAM issues left at this point, you can skip this step. + + Run ``borg upgrade --archives-tam ``. + This will unconditionally add a correct archive TAM to all archives not having one. ``borg check`` would consider TAM-less or invalid-TAM archives as garbage or a potential attack. - To see that all archives now are "tam:verified" run: ``borg list --consider-checkpoints --format='{name} {time} tam:{tam}{NL}' `` -5. Please note that you should never use BORG_WORKAROUNDS=ignore_invalid_archive_tam - for normal production operations - it is only needed once to get the archives in a - repository into a good state. All archives have a valid TAM now. + To see that all archives are OK now, you can optionally repeat the command from step 3. + +5. Done. Manifest and archives are TAM authenticated now. Vulnerability time line: