From a2ee13fd341dcd004b4a06b17d6f2fc759327861 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Tue, 13 Jun 2023 20:17:01 +0200 Subject: [PATCH 01/10] check: rebuild_manifest must verify archive TAM --- src/borg/archive.py | 13 ++++++ src/borg/crypto/key.py | 58 ++++++++++++++++++++++++ src/borg/helpers/msgpack.py | 4 +- src/borg/testsuite/archiver/check_cmd.py | 22 ++++----- 4 files changed, 83 insertions(+), 14 deletions(-) diff --git a/src/borg/archive.py b/src/borg/archive.py index 808f901fb6..ef4217a9b9 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -1992,6 +1992,19 @@ def valid_archive(obj): except msgpack.UnpackException: continue if valid_archive(archive): + # **after** doing the low-level checks and having a strong indication that we + # are likely looking at an archive item here, also check the TAM authentication: + try: + archive, verified = self.key.unpack_and_verify_archive(data, force_tam_not_required=False) + except IntegrityError: + # TAM issues - do not accept this archive! + # either somebody is trying to attack us with a fake archive data or + # we have an ancient archive made before TAM was a thing (borg < 1.0.9) **and** this repo + # was not correctly upgraded to borg 1.2.5 (see advisory at top of the changelog). + # borg can't tell the difference, so it has to assume this archive might be an attack + # and drops this archive. + continue + # note: if we get here and verified is False, a TAM is not required. archive = ArchiveItem(internal_dict=archive) name = archive.name logger.info("Found archive %s", name) diff --git a/src/borg/crypto/key.py b/src/borg/crypto/key.py index 338888b3af..ebaee58a2b 100644 --- a/src/borg/crypto/key.py +++ b/src/borg/crypto/key.py @@ -72,6 +72,15 @@ class TAMRequiredError(IntegrityError): traceback = False +class ArchiveTAMRequiredError(TAMRequiredError): + __doc__ = textwrap.dedent( + """ + Archive '{}' is unauthenticated, but it is required for this repository. + """ + ).strip() + traceback = False + + class TAMInvalid(IntegrityError): __doc__ = IntegrityError.__doc__ traceback = False @@ -81,6 +90,15 @@ def __init__(self): super().__init__("Manifest authentication did not verify") +class ArchiveTAMInvalid(IntegrityError): + __doc__ = IntegrityError.__doc__ + traceback = False + + def __init__(self): + # Error message becomes: "Data integrity error: Archive authentication did not verify" + super().__init__("Archive authentication did not verify") + + class TAMUnsupportedSuiteError(IntegrityError): """Could not verify manifest: Unsupported suite {!r}; a newer version is needed.""" @@ -279,6 +297,46 @@ def unpack_and_verify_manifest(self, data, force_tam_not_required=False): logger.debug("TAM-verified manifest") return unpacked, True + def unpack_and_verify_archive(self, data, force_tam_not_required=False): + """Unpack msgpacked *data* and return (object, did_verify).""" + tam_required = self.tam_required + if force_tam_not_required and tam_required: + logger.warning("Archive authentication DISABLED.") + tam_required = False + data = bytearray(data) + unpacker = get_limited_unpacker("archive") + unpacker.feed(data) + unpacked = unpacker.unpack() + if b"tam" not in unpacked: + if tam_required: + archive_name = unpacked.get(b"name", b"").decode("ascii", "replace") + raise ArchiveTAMRequiredError(archive_name) + else: + logger.debug("TAM not found and not required") + return unpacked, False + tam = unpacked.pop(b"tam", None) + if not isinstance(tam, dict): + raise ArchiveTAMInvalid() + 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 ArchiveTAMInvalid() + offset = data.index(tam_hmac) + data[offset : offset + 64] = bytes(64) + 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): + raise ArchiveTAMInvalid() + logger.debug("TAM-verified archive") + return unpacked, True + class PlaintextKey(KeyBase): TYPE = KeyType.PLAINTEXT diff --git a/src/borg/helpers/msgpack.py b/src/borg/helpers/msgpack.py index ddbd95944a..dce6eb33fb 100644 --- a/src/borg/helpers/msgpack.py +++ b/src/borg/helpers/msgpack.py @@ -219,10 +219,10 @@ def get_limited_unpacker(kind): args = dict(use_list=False, max_buffer_size=3 * max(BUFSIZE, MAX_OBJECT_SIZE)) # return tuples, not lists if kind in ("server", "client"): pass # nothing special - elif kind in ("manifest", "key"): + elif kind in ("manifest", "archive", "key"): args.update(dict(use_list=True, object_hook=StableDict)) # default value else: - raise ValueError('kind must be "server", "client", "manifest" or "key"') + raise ValueError('kind must be "server", "client", "manifest", "archive" or "key"') return Unpacker(**args) diff --git a/src/borg/testsuite/archiver/check_cmd.py b/src/borg/testsuite/archiver/check_cmd.py index b5db325386..5ddce3437c 100644 --- a/src/borg/testsuite/archiver/check_cmd.py +++ b/src/borg/testsuite/archiver/check_cmd.py @@ -6,7 +6,6 @@ from ...archive import ChunkBuffer from ...constants import * # NOQA from ...helpers import bin_to_hex -from ...helpers import msgpack from ...manifest import Manifest from ...repository import Repository from . import cmd, src_file, create_src_archive, open_archive, generate_archiver_tests, RK_ENCRYPTION @@ -233,17 +232,16 @@ def test_manifest_rebuild_duplicate_archive(archivers, request): manifest = repository.get(Manifest.MANIFEST_ID) corrupted_manifest = manifest + b"corrupted!" repository.put(Manifest.MANIFEST_ID, corrupted_manifest) - archive = msgpack.packb( - { - "command_line": "", - "item_ptrs": [], - "hostname": "foo", - "username": "bar", - "name": "archive1", - "time": "2016-12-15T18:49:51.849711", - "version": 2, - } - ) + archive_dict = { + "command_line": "", + "item_ptrs": [], + "hostname": "foo", + "username": "bar", + "name": "archive1", + "time": "2016-12-15T18:49:51.849711", + "version": 2, + } + archive = repo_objs.key.pack_and_authenticate_metadata(archive_dict, context=b"archive") archive_id = repo_objs.id_hash(archive) repository.put(archive_id, repo_objs.format(archive_id, {}, archive)) repository.commit(compact=False) From bfead4b288833f890523d8881797ff6b345edaf9 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Tue, 29 Aug 2023 22:32:30 +0200 Subject: [PATCH 02/10] fixup with msgpack data types related fixes --- src/borg/crypto/key.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/borg/crypto/key.py b/src/borg/crypto/key.py index ebaee58a2b..aa46b29002 100644 --- a/src/borg/crypto/key.py +++ b/src/borg/crypto/key.py @@ -307,27 +307,29 @@ def unpack_and_verify_archive(self, data, force_tam_not_required=False): unpacker = get_limited_unpacker("archive") unpacker.feed(data) unpacked = unpacker.unpack() - if b"tam" not in unpacked: + if "tam" not in unpacked: if tam_required: - archive_name = unpacked.get(b"name", b"").decode("ascii", "replace") + archive_name = unpacked.get("name", "") raise ArchiveTAMRequiredError(archive_name) else: logger.debug("TAM not found and not required") return unpacked, False - tam = unpacked.pop(b"tam", None) + tam = unpacked.pop("tam", None) if not isinstance(tam, dict): raise ArchiveTAMInvalid() - tam_type = tam.get(b"type", b"").decode("ascii", "replace") + tam_type = tam.get("type", "") 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): + tam_hmac = tam.get("hmac") + tam_salt = tam.get("salt") + if not isinstance(tam_salt, (bytes, str)) or not isinstance(tam_hmac, (bytes, str)): raise ArchiveTAMInvalid() + tam_hmac = want_bytes(tam_hmac) # legacy + tam_salt = want_bytes(tam_salt) # legacy offset = data.index(tam_hmac) data[offset : offset + 64] = bytes(64) tam_key = self._tam_key(tam_salt, context=b"archive") From 462c1bdf2e597bd2e276c8fea82c84fabc0b7244 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Fri, 16 Jun 2023 21:56:06 +0200 Subject: [PATCH 03/10] check: rebuild_refcounts verify and recreate TAM This part of the archive checker recreates the Archive items (always, just in case some missing chunks needed repairing). When loading the Archive item, we now verify the TAM. When saving the (potentially modified) Archive item, we now (re-)generate the TAM. Archives without a valid TAM are dropped rather than TAM-authenticated when saving them. There shouldn't be any archives without a valid TAM: - borg writes an archive TAM since long (1.0.9) - users are expected to TAM-authenticate archives created by older borg when upgrading to borg 1.2.5. Also: Archive.set_meta: TAM-authenticate new archive This is also used by Archive.rename and .recreate. --- src/borg/archive.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/borg/archive.py b/src/borg/archive.py index ef4217a9b9..5c5b8d2b69 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -1024,7 +1024,7 @@ def set_meta(self, key, value): setattr(metadata, key, value) if "items" in metadata: del metadata.items - data = msgpack.packb(metadata.as_dict()) + data = self.key.pack_and_authenticate_metadata(metadata.as_dict(), context=b"archive") new_id = self.key.id_hash(data) self.cache.add_chunk(new_id, {}, data, stats=self.stats) self.manifest.archives[self.name] = (new_id, metadata.time) @@ -2261,7 +2261,17 @@ def valid_item(obj): self.error_found = True del self.manifest.archives[info.name] continue - archive = ArchiveItem(internal_dict=msgpack.unpackb(data)) + try: + archive, verified = self.key.unpack_and_verify_archive(data, force_tam_not_required=False) + except IntegrityError as integrity_error: + # looks like there is a TAM issue with this archive, this might be an attack! + # when upgrading to borg 1.2.5, users are expected to TAM-authenticate all archives they + # trust, so there shouldn't be any without TAM. + logger.error("Archive TAM authentication issue for archive %s: %s", info.name, integrity_error) + self.error_found = True + del self.manifest.archives[info.name] + continue + archive = ArchiveItem(internal_dict=archive) if archive.version != 2: raise Exception("Unknown archive metadata version") items_buffer = ChunkBuffer(self.key) @@ -2280,7 +2290,7 @@ def valid_item(obj): archive.item_ptrs = archive_put_items( items_buffer.chunks, repo_objs=self.repo_objs, add_reference=add_reference ) - data = msgpack.packb(archive.as_dict()) + data = self.key.pack_and_authenticate_metadata(archive.as_dict(), context=b"archive") new_archive_id = self.key.id_hash(data) cdata = self.repo_objs.format(new_archive_id, {}, data) add_reference(new_archive_id, len(data), cdata) From 277b0b81a860f4518d7bf0cc0951e77f9c99336d Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Fri, 16 Jun 2023 22:40:51 +0200 Subject: [PATCH 04/10] cache sync: check archive TAM --- src/borg/cache.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/borg/cache.py b/src/borg/cache.py index 77df3fc594..a6ec061ed1 100644 --- a/src/borg/cache.py +++ b/src/borg/cache.py @@ -755,7 +755,8 @@ def fetch_and_build_idx(archive_id, decrypted_repository, chunk_idx): nonlocal processed_item_metadata_chunks csize, data = decrypted_repository.get(archive_id) chunk_idx.add(archive_id, 1, len(data)) - archive = ArchiveItem(internal_dict=msgpack.unpackb(data)) + archive, verified = self.key.unpack_and_verify_archive(data, force_tam_not_required=True) + archive = ArchiveItem(internal_dict=archive) if archive.version not in (1, 2): # legacy raise Exception("Unknown archive metadata version") if archive.version == 1: From b23e6cb73da01df038f7bd10c34a91c7187817b0 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Wed, 14 Jun 2023 03:45:24 +0200 Subject: [PATCH 05/10] list: support {tam} placeholder. check archive TAM. list: shows either "verified" or "none", depending on whether a TAM auth tag could be verified or was missing (old archives from borg < 1.0.9). when loading an archive, we now try to verify the archive TAM, but we do not require it. people might still have old archives in their repos and we want to be able to list such repos without fatal exceptions. --- src/borg/archive.py | 5 ++++- src/borg/crypto/key.py | 4 +++- src/borg/helpers/parseformat.py | 7 ++++++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/borg/archive.py b/src/borg/archive.py index 5c5b8d2b69..da9d3661fd 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -493,6 +493,7 @@ def __init__( self.name = name # overwritten later with name from archive metadata self.name_in_manifest = name # can differ from .name later (if borg check fixed duplicate archive names) self.comment = None + self.tam_verified = False self.numeric_ids = numeric_ids self.noatime = noatime self.noctime = noctime @@ -532,7 +533,9 @@ def __init__( def _load_meta(self, id): cdata = self.repository.get(id) _, data = self.repo_objs.parse(id, cdata) - metadata = ArchiveItem(internal_dict=msgpack.unpackb(data)) + # we do not require TAM for archives, otherwise we can not even borg list a repo with old archives. + archive, self.tam_verified = self.key.unpack_and_verify_archive(data, force_tam_not_required=True) + metadata = ArchiveItem(internal_dict=archive) if metadata.version not in (1, 2): # legacy: still need to read v1 archives raise Exception("Unknown archive metadata version") # note: metadata.items must not get written to disk! diff --git a/src/borg/crypto/key.py b/src/borg/crypto/key.py index aa46b29002..d42549c03f 100644 --- a/src/borg/crypto/key.py +++ b/src/borg/crypto/key.py @@ -301,7 +301,9 @@ def unpack_and_verify_archive(self, data, force_tam_not_required=False): """Unpack msgpacked *data* and return (object, did_verify).""" tam_required = self.tam_required if force_tam_not_required and tam_required: - logger.warning("Archive authentication DISABLED.") + # for a long time, borg only checked manifest for "tam_required" and + # people might have archives without TAM, so don't be too annoyingly loud here: + logger.debug("Archive authentication DISABLED.") tam_required = False data = bytearray(data) unpacker = get_limited_unpacker("archive") diff --git a/src/borg/helpers/parseformat.py b/src/borg/helpers/parseformat.py index 7297db3769..65d7ac7f37 100644 --- a/src/borg/helpers/parseformat.py +++ b/src/borg/helpers/parseformat.py @@ -723,11 +723,12 @@ class ArchiveFormatter(BaseFormatter): "id": "internal ID of the archive", "hostname": "hostname of host on which this archive was created", "username": "username of user who created this archive", + "tam": "TAM authentication state of this archive", "size": "size of this archive (data plus metadata, not considering compression and deduplication)", "nfiles": "count of files in this archive", } KEY_GROUPS = ( - ("archive", "name", "comment", "id"), + ("archive", "name", "comment", "id", "tam"), ("start", "time", "end", "command_line"), ("hostname", "username"), ("size", "nfiles"), @@ -750,6 +751,7 @@ def __init__(self, format, repository, manifest, key, *, iec=False): "username": partial(self.get_meta, "username", ""), "comment": partial(self.get_meta, "comment", ""), "command_line": partial(self.get_meta, "command_line", ""), + "tam": self.get_tam, "size": partial(self.get_meta, "size", 0), "nfiles": partial(self.get_meta, "nfiles", 0), "end": self.get_ts_end, @@ -795,6 +797,9 @@ def get_meta(self, key, default=None): def get_ts_end(self): return self.format_time(self.archive.ts_end) + def get_tam(self): + return "verified" if self.archive.tam_verified else "none" + def format_time(self, ts): return OutputTimestamp(ts) From 95b560442284eda3ffae403c3086d549f6e121b8 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Tue, 27 Jun 2023 16:01:50 +0200 Subject: [PATCH 06/10] TAM msgs: be more specific: archives vs. manifest --- src/borg/crypto/key.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/borg/crypto/key.py b/src/borg/crypto/key.py index d42549c03f..aedb19f5ef 100644 --- a/src/borg/crypto/key.py +++ b/src/borg/crypto/key.py @@ -270,7 +270,7 @@ def unpack_and_verify_manifest(self, data, force_tam_not_required=False): if tam_required: raise TAMRequiredError(self.repository._location.canonical_path()) else: - logger.debug("TAM not found and not required") + logger.debug("Manifest TAM not found and not required") return unpacked, False tam = unpacked.pop("tam", None) if not isinstance(tam, dict): @@ -280,7 +280,9 @@ def unpack_and_verify_manifest(self, data, force_tam_not_required=False): 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) + logger.debug( + "Ignoring manifest TAM made with unsupported suite, since TAM is not required: %r", tam_type + ) return unpacked, False tam_hmac = tam.get("hmac") tam_salt = tam.get("salt") @@ -314,7 +316,7 @@ def unpack_and_verify_archive(self, data, force_tam_not_required=False): archive_name = unpacked.get("name", "") raise ArchiveTAMRequiredError(archive_name) else: - logger.debug("TAM not found and not required") + logger.debug("Archive TAM not found and not required") return unpacked, False tam = unpacked.pop("tam", None) if not isinstance(tam, dict): @@ -324,7 +326,9 @@ def unpack_and_verify_archive(self, data, force_tam_not_required=False): 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) + logger.debug( + "Ignoring archive TAM made with unsupported suite, since TAM is not required: %r", tam_type + ) return unpacked, False tam_hmac = tam.get("hmac") tam_salt = tam.get("salt") From 5cd2060345f38f2e0324ab178f847c2f45598b12 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Tue, 4 Jul 2023 00:05:10 +0200 Subject: [PATCH 07/10] rebuild_refcounts: keep archive ID, if possible rebuild_refcounts verifies and recreates the TAM. Now it re-uses the salt, so that the archive ID does not change just because of a new salt if the archive has still the same data. --- src/borg/archive.py | 8 ++++---- src/borg/cache.py | 2 +- src/borg/crypto/key.py | 14 ++++++++------ 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/borg/archive.py b/src/borg/archive.py index da9d3661fd..964e32398f 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -534,7 +534,7 @@ def _load_meta(self, id): cdata = self.repository.get(id) _, data = self.repo_objs.parse(id, cdata) # we do not require TAM for archives, otherwise we can not even borg list a repo with old archives. - archive, self.tam_verified = self.key.unpack_and_verify_archive(data, force_tam_not_required=True) + archive, self.tam_verified, _ = self.key.unpack_and_verify_archive(data, force_tam_not_required=True) metadata = ArchiveItem(internal_dict=archive) if metadata.version not in (1, 2): # legacy: still need to read v1 archives raise Exception("Unknown archive metadata version") @@ -1998,7 +1998,7 @@ def valid_archive(obj): # **after** doing the low-level checks and having a strong indication that we # are likely looking at an archive item here, also check the TAM authentication: try: - archive, verified = self.key.unpack_and_verify_archive(data, force_tam_not_required=False) + archive, verified, _ = self.key.unpack_and_verify_archive(data, force_tam_not_required=False) except IntegrityError: # TAM issues - do not accept this archive! # either somebody is trying to attack us with a fake archive data or @@ -2265,7 +2265,7 @@ def valid_item(obj): del self.manifest.archives[info.name] continue try: - archive, verified = self.key.unpack_and_verify_archive(data, force_tam_not_required=False) + archive, verified, salt = self.key.unpack_and_verify_archive(data, force_tam_not_required=False) except IntegrityError as integrity_error: # looks like there is a TAM issue with this archive, this might be an attack! # when upgrading to borg 1.2.5, users are expected to TAM-authenticate all archives they @@ -2293,7 +2293,7 @@ def valid_item(obj): archive.item_ptrs = archive_put_items( items_buffer.chunks, repo_objs=self.repo_objs, add_reference=add_reference ) - data = self.key.pack_and_authenticate_metadata(archive.as_dict(), context=b"archive") + data = self.key.pack_and_authenticate_metadata(archive.as_dict(), context=b"archive", salt=salt) new_archive_id = self.key.id_hash(data) cdata = self.repo_objs.format(new_archive_id, {}, data) add_reference(new_archive_id, len(data), cdata) diff --git a/src/borg/cache.py b/src/borg/cache.py index a6ec061ed1..fc9fd1851e 100644 --- a/src/borg/cache.py +++ b/src/borg/cache.py @@ -755,7 +755,7 @@ def fetch_and_build_idx(archive_id, decrypted_repository, chunk_idx): nonlocal processed_item_metadata_chunks csize, data = decrypted_repository.get(archive_id) chunk_idx.add(archive_id, 1, len(data)) - archive, verified = self.key.unpack_and_verify_archive(data, force_tam_not_required=True) + archive, verified, _ = self.key.unpack_and_verify_archive(data, force_tam_not_required=True) archive = ArchiveItem(internal_dict=archive) if archive.version not in (1, 2): # legacy raise Exception("Unknown archive metadata version") diff --git a/src/borg/crypto/key.py b/src/borg/crypto/key.py index aedb19f5ef..3fb4818aa2 100644 --- a/src/borg/crypto/key.py +++ b/src/borg/crypto/key.py @@ -243,11 +243,13 @@ def _tam_key(self, salt, context): output_length=64, ) - def pack_and_authenticate_metadata(self, metadata_dict, context=b"manifest"): + def pack_and_authenticate_metadata(self, metadata_dict, context=b"manifest", salt=None): + if salt is None: + salt = os.urandom(64) metadata_dict = StableDict(metadata_dict) - tam = metadata_dict["tam"] = StableDict({"type": "HKDF_HMAC_SHA512", "hmac": bytes(64), "salt": os.urandom(64)}) + tam = metadata_dict["tam"] = StableDict({"type": "HKDF_HMAC_SHA512", "hmac": bytes(64), "salt": salt}) packed = msgpack.packb(metadata_dict) - tam_key = self._tam_key(tam["salt"], context) + tam_key = self._tam_key(salt, context) tam["hmac"] = hmac.digest(tam_key, packed, "sha512") return msgpack.packb(metadata_dict) @@ -317,7 +319,7 @@ def unpack_and_verify_archive(self, data, force_tam_not_required=False): raise ArchiveTAMRequiredError(archive_name) else: logger.debug("Archive TAM not found and not required") - return unpacked, False + return unpacked, False, None tam = unpacked.pop("tam", None) if not isinstance(tam, dict): raise ArchiveTAMInvalid() @@ -329,7 +331,7 @@ def unpack_and_verify_archive(self, data, force_tam_not_required=False): logger.debug( "Ignoring archive TAM made with unsupported suite, since TAM is not required: %r", tam_type ) - return unpacked, False + return unpacked, False, None tam_hmac = tam.get("hmac") tam_salt = tam.get("salt") if not isinstance(tam_salt, (bytes, str)) or not isinstance(tam_hmac, (bytes, str)): @@ -343,7 +345,7 @@ def unpack_and_verify_archive(self, data, force_tam_not_required=False): if not hmac.compare_digest(calculated_hmac, tam_hmac): raise ArchiveTAMInvalid() logger.debug("TAM-verified archive") - return unpacked, True + return unpacked, True, tam_salt class PlaintextKey(KeyBase): From 56da3987111eb80b4ca38ac3e6aaa7953c61d2e3 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Fri, 28 Jul 2023 15:58:03 +0200 Subject: [PATCH 08/10] add tests for archive TAMs --- src/borg/testsuite/archiver/checks.py | 60 +++++++++++++++++++++- src/borg/testsuite/key.py | 73 ++++++++++++++++++++++----- 2 files changed, 119 insertions(+), 14 deletions(-) diff --git a/src/borg/testsuite/archiver/checks.py b/src/borg/testsuite/archiver/checks.py index 7f3c977adf..766ac8602e 100644 --- a/src/borg/testsuite/archiver/checks.py +++ b/src/borg/testsuite/archiver/checks.py @@ -8,7 +8,7 @@ from ...cache import Cache, LocalCache from ...constants import * # NOQA from ...crypto.key import TAMRequiredError -from ...helpers import Location, get_security_dir, bin_to_hex +from ...helpers import Location, get_security_dir, bin_to_hex, archive_ts_now from ...helpers import EXIT_ERROR from ...helpers import msgpack from ...manifest import Manifest, MandatoryFeatureUnsupported @@ -322,7 +322,7 @@ def test_check_cache(archivers, request): check_cache(archiver) -# Begin manifest tests +# Begin manifest TAM tests def spoof_manifest(repository): with repository: manifest = Manifest.load(repository, Manifest.NO_OPERATION_CHECK) @@ -380,6 +380,62 @@ def test_not_required(archiver): cmd(archiver, "rlist") +# Begin archive TAM tests +def write_archive_without_tam(repository, archive_name): + manifest = Manifest.load(repository, Manifest.NO_OPERATION_CHECK) + archive_data = msgpack.packb( + { + "version": 2, + "name": archive_name, + "item_ptrs": [], + "command_line": "", + "hostname": "", + "username": "", + "time": archive_ts_now().isoformat(timespec="microseconds"), + "size": 0, + "nfiles": 0, + } + ) + archive_id = manifest.repo_objs.id_hash(archive_data) + cdata = manifest.repo_objs.format(archive_id, {}, archive_data) + repository.put(archive_id, cdata) + manifest.archives[archive_name] = (archive_id, datetime.now()) + manifest.write() + repository.commit(compact=False) + + +def test_check_rebuild_manifest(archiver): + cmd(archiver, "rcreate", RK_ENCRYPTION) + create_src_archive(archiver, "archive_tam") + repository = Repository(archiver.repository_path, exclusive=True) + with repository: + write_archive_without_tam(repository, "archive_no_tam") + repository.delete(Manifest.MANIFEST_ID) # kill manifest, so check has to rebuild it + repository.commit(compact=False) + cmd(archiver, "check", "--repair") + output = cmd(archiver, "rlist", "--format='{name} tam:{tam}{NL}'") + assert "archive_tam tam:verified" in output # TAM-verified archive is in rebuilt manifest + assert "archive_no_tam" not in output # check got rid of untrusted not TAM-verified archive + + +def test_check_rebuild_refcounts(archiver): + cmd(archiver, "rcreate", RK_ENCRYPTION) + create_src_archive(archiver, "archive_tam") + archive_id_pre_check = cmd(archiver, "rlist", "--format='{name} {id}{NL}'") + repository = Repository(archiver.repository_path, exclusive=True) + with repository: + write_archive_without_tam(repository, "archive_no_tam") + output = cmd(archiver, "rlist", "--format='{name} tam:{tam}{NL}'") + assert "archive_tam tam:verified" in output # good + assert "archive_no_tam tam:none" in output # could be borg < 1.0.9 archive or fake + cmd(archiver, "check", "--repair") + output = cmd(archiver, "rlist", "--format='{name} tam:{tam}{NL}'") + assert "archive_tam tam:verified" in output # TAM-verified archive still there + assert "archive_no_tam" not in output # check got rid of untrusted not TAM-verified archive + archive_id_post_check = cmd(archiver, "rlist", "--format='{name} {id}{NL}'") + assert archive_id_post_check == archive_id_pre_check # rebuild_refcounts didn't change archive_tam archive id + + # Begin Remote Tests def test_remote_repo_restrict_to_path(remote_archiver): original_location, repo_path = remote_archiver.repository_location, remote_archiver.repository_path diff --git a/src/borg/testsuite/key.py b/src/borg/testsuite/key.py index 94a3765c12..73d9df35d7 100644 --- a/src/borg/testsuite/key.py +++ b/src/borg/testsuite/key.py @@ -11,13 +11,8 @@ from ..crypto.key import AESOCBRepoKey, AESOCBKeyfileKey, CHPORepoKey, CHPOKeyfileKey from ..crypto.key import Blake2AESOCBRepoKey, Blake2AESOCBKeyfileKey, Blake2CHPORepoKey, Blake2CHPOKeyfileKey from ..crypto.key import ID_HMAC_SHA_256, ID_BLAKE2b_256 -from ..crypto.key import ( - TAMRequiredError, - TAMInvalid, - TAMUnsupportedSuiteError, - UnsupportedManifestError, - UnsupportedKeyFormatError, -) +from ..crypto.key import TAMRequiredError, TAMInvalid, TAMUnsupportedSuiteError, ArchiveTAMInvalid +from ..crypto.key import UnsupportedManifestError, UnsupportedKeyFormatError from ..crypto.key import identify_key from ..crypto.low_level import IntegrityError as IntegrityErrorBase from ..helpers import IntegrityError @@ -281,6 +276,8 @@ def test_missing_when_required(self, key): blob = msgpack.packb({}) with pytest.raises(TAMRequiredError): key.unpack_and_verify_manifest(blob) + with pytest.raises(TAMRequiredError): + key.unpack_and_verify_archive(blob) def test_missing(self, key): blob = msgpack.packb({}) @@ -288,11 +285,16 @@ def test_missing(self, key): unpacked, verified = key.unpack_and_verify_manifest(blob) assert unpacked == {} assert not verified + unpacked, verified, _ = key.unpack_and_verify_archive(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) + with pytest.raises(TAMUnsupportedSuiteError): + key.unpack_and_verify_archive(blob) def test_unknown_type(self, key): blob = msgpack.packb({"tam": {"type": "HMAC_VOLLBIT"}}) @@ -300,6 +302,9 @@ def test_unknown_type(self, key): unpacked, verified = key.unpack_and_verify_manifest(blob) assert unpacked == {} assert not verified + unpacked, verified, _ = key.unpack_and_verify_archive(blob) + assert unpacked == {} + assert not verified @pytest.mark.parametrize( "tam, exc", @@ -310,11 +315,25 @@ def test_unknown_type(self, key): (1234, TAMInvalid), ), ) - def test_invalid(self, key, tam, exc): + def test_invalid_manifest(self, key, tam, exc): blob = msgpack.packb({"tam": tam}) with pytest.raises(exc): key.unpack_and_verify_manifest(blob) + @pytest.mark.parametrize( + "tam, exc", + ( + ({}, TAMUnsupportedSuiteError), + ({"type": b"\xff"}, TAMUnsupportedSuiteError), + (None, ArchiveTAMInvalid), + (1234, ArchiveTAMInvalid), + ), + ) + def test_invalid_archive(self, key, tam, exc): + blob = msgpack.packb({"tam": tam}) + with pytest.raises(exc): + key.unpack_and_verify_archive(blob) + @pytest.mark.parametrize( "hmac, salt", (({}, bytes(64)), (bytes(64), {}), (None, bytes(64)), (bytes(64), None)), @@ -330,10 +349,12 @@ def test_wrong_types(self, key, hmac, salt): blob = msgpack.packb(data) with pytest.raises(TAMInvalid): key.unpack_and_verify_manifest(blob) + with pytest.raises(ArchiveTAMInvalid): + key.unpack_and_verify_archive(blob) - def test_round_trip(self, key): + def test_round_trip_manifest(self, key): data = {"foo": "bar"} - blob = key.pack_and_authenticate_metadata(data) + blob = key.pack_and_authenticate_metadata(data, context=b"manifest") assert blob.startswith(b"\x82") unpacked = msgpack.unpackb(blob) @@ -344,10 +365,23 @@ def test_round_trip(self, key): assert unpacked["foo"] == "bar" assert "tam" not in unpacked + def test_round_trip_archive(self, key): + data = {"foo": "bar"} + blob = key.pack_and_authenticate_metadata(data, context=b"archive") + assert blob.startswith(b"\x82") + + unpacked = msgpack.unpackb(blob) + assert unpacked["tam"]["type"] == "HKDF_HMAC_SHA512" + + unpacked, verified, _ = key.unpack_and_verify_archive(blob) + assert verified + assert unpacked["foo"] == "bar" + assert "tam" not in unpacked + @pytest.mark.parametrize("which", ("hmac", "salt")) - def test_tampered(self, key, which): + def test_tampered_manifest(self, key, which): data = {"foo": "bar"} - blob = key.pack_and_authenticate_metadata(data) + blob = key.pack_and_authenticate_metadata(data, context=b"manifest") assert blob.startswith(b"\x82") unpacked = msgpack.unpackb(blob, object_hook=StableDict) @@ -359,6 +393,21 @@ def test_tampered(self, key, which): with pytest.raises(TAMInvalid): key.unpack_and_verify_manifest(blob) + @pytest.mark.parametrize("which", ("hmac", "salt")) + def test_tampered_archive(self, key, which): + data = {"foo": "bar"} + blob = key.pack_and_authenticate_metadata(data, context=b"archive") + assert blob.startswith(b"\x82") + + unpacked = msgpack.unpackb(blob, object_hook=StableDict) + assert len(unpacked["tam"][which]) == 64 + unpacked["tam"][which] = unpacked["tam"][which][0:32] + bytes(32) + assert len(unpacked["tam"][which]) == 64 + blob = msgpack.packb(unpacked) + + with pytest.raises(ArchiveTAMInvalid): + key.unpack_and_verify_archive(blob) + def test_decrypt_key_file_unsupported_algorithm(): """We will add more algorithms in the future. We should raise a helpful error.""" From 449cd51b73b0710a940af8cefe74793ce81563f4 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Tue, 27 Jun 2023 15:02:54 +0200 Subject: [PATCH 09/10] document vulnerability, repo upgrade procedure --- docs/changes_1.x.rst | 66 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/docs/changes_1.x.rst b/docs/changes_1.x.rst index 3dae2a7bb6..728a93f264 100644 --- a/docs/changes_1.x.rst +++ b/docs/changes_1.x.rst @@ -5,6 +5,72 @@ Important notes 1.x This section provides information about security and corruption issues. +.. _archives_tam_vuln: + +Pre-1.2.5 archives spoofing vulnerability (CVE-2023-36811) +---------------------------------------------------------- + +A flaw in the cryptographic authentication scheme in Borg allowed an attacker to +fake archives and potentially indirectly cause backup data loss in the repository. + +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. + +Creating plausible fake archives may be feasible for empty or small archives, +but is unlikely for large archives. + +The fix enforces checking the TAM authentication tag of archives at critical +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.5, we mean a borg version >= 1.2.5 **or** a +borg version that has the relevant security patches for this vulnerability applied +(could be also an older version in that case). + +Steps you must take to upgrade a repository: + +1. Upgrade all clients using this repository to borg 1.2.5. + 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.5 before completing the upgrade steps. + +2. Run ``borg info --debug 2>&1 | grep TAM | grep -i manifest``. + 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*. + +3. Run ``borg list --format='{name} {time} tam:{tam}{NL}' ``. + "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" could also come from archives created by an attacker. + You should verify that "tam:none" archives 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:non archives left at this point, you can skip this step. + Run ``borg upgrade --archives-tam ``. + This will make sure all archives are TAM authenticated (an archive TAM will be added + for all archives still missing one). + ``borg check`` would consider TAM-less archives as garbage or a potential attack. + Optionally run the same command as in step 3 to see that all archives now are "tam:verified". + + +Vulnerability time line: + +* 2023-06-13: Vulnerability discovered during code review by Thomas Waldmann +* 2023-06-13...: Work on fixing the issue, upgrade procedure, docs. +* 2023-06-30: CVE was assigned via Github CNA +* 2023-07-xx: Released fixed version 1.2.5 + .. _hashindex_set_bug: Pre-1.1.11 potential index corruption / data loss issue From f334ef1b4de2f8a359ededa41ce13358b81e63c1 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Wed, 30 Aug 2023 15:49:09 +0200 Subject: [PATCH 10/10] fix CVE timeline, fix markup --- docs/changes_1.x.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/changes_1.x.rst b/docs/changes_1.x.rst index 728a93f264..0bd2c5d474 100644 --- a/docs/changes_1.x.rst +++ b/docs/changes_1.x.rst @@ -42,6 +42,7 @@ Steps you must take to upgrade a repository: Do **not** run ``borg check`` with borg 1.2.5 before completing the upgrade steps. 2. Run ``borg info --debug 2>&1 | grep TAM | grep -i manifest``. + 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*. @@ -69,7 +70,8 @@ Vulnerability time line: * 2023-06-13: Vulnerability discovered during code review by Thomas Waldmann * 2023-06-13...: Work on fixing the issue, upgrade procedure, docs. * 2023-06-30: CVE was assigned via Github CNA -* 2023-07-xx: Released fixed version 1.2.5 +* 2023-06-30 .. 2023-08-29: Fixed issue, code review, docs, testing. +* 2023-08-30: Released fixed version 1.2.5 .. _hashindex_set_bug: