From 6a68ad5cd6c05ef3493d3f3fd9575aa1047f77eb Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 16 Sep 2023 01:26:10 +0200 Subject: [PATCH] remove archive TAMs --- src/borg/archive.py | 38 +++------------- src/borg/cache.py | 2 +- src/borg/crypto/key.py | 51 ++++------------------ src/borg/helpers/parseformat.py | 3 +- src/borg/testsuite/archiver/check_cmd.py | 2 +- src/borg/testsuite/archiver/checks.py | 55 +----------------------- src/borg/testsuite/key.py | 48 ++------------------- 7 files changed, 22 insertions(+), 177 deletions(-) diff --git a/src/borg/archive.py b/src/borg/archive.py index 8db9e28c9..026186780 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -535,7 +535,7 @@ class Archive: def _load_meta(self, id): cdata = self.repository.get(id) _, data = self.repo_objs.parse(id, cdata, ro_type=ROBJ_ARCHIVE_META) - archive, _ = self.key.unpack_and_verify_archive(data) + archive = self.key.unpack_archive(data) 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") @@ -702,7 +702,7 @@ Duration: {0.duration} metadata.update({"size": stats.osize, "nfiles": stats.nfiles}) metadata.update(additional_metadata or {}) metadata = ArchiveItem(metadata) - data = self.key.pack_and_authenticate_metadata(metadata.as_dict(), context=b"archive") + data = self.key.pack_metadata(metadata.as_dict()) self.id = self.repo_objs.id_hash(data) try: self.cache.add_chunk(self.id, {}, data, stats=self.stats, ro_type=ROBJ_ARCHIVE_META) @@ -1030,7 +1030,7 @@ Duration: {0.duration} setattr(metadata, key, value) if "items" in metadata: del metadata.items - data = self.key.pack_and_authenticate_metadata(metadata.as_dict(), context=b"archive") + data = self.key.pack_metadata(metadata.as_dict()) new_id = self.key.id_hash(data) self.cache.add_chunk(new_id, {}, data, stats=self.stats, ro_type=ROBJ_ARCHIVE_META) self.manifest.archives[self.name] = (new_id, metadata.time) @@ -1998,23 +1998,7 @@ class ArchiveChecker: 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, _ = self.key.unpack_and_verify_archive(data) - except IntegrityError as integrity_error: - # 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. - name = archive.get(b"name", b"").decode("ascii", "replace") - logger.error("Archive TAM authentication issue for archive %s: %s", name, integrity_error) - logger.error("This archive will *not* be added to the rebuilt manifest! It will be deleted.") - self.error_found = True - continue - # note: if we get here and verified is False, a TAM is not required. + archive = self.key.unpack_archive(data) archive = ArchiveItem(internal_dict=archive) name = archive.name logger.info("Found archive %s", name) @@ -2271,17 +2255,7 @@ class ArchiveChecker: self.error_found = True del self.manifest.archives[info.name] continue - try: - archive, salt = self.key.unpack_and_verify_archive(data) - 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) - logger.error("This archive will be *removed* from the manifest! It will be deleted.") - self.error_found = True - del self.manifest.archives[info.name] - continue + archive = self.key.unpack_archive(data) archive = ArchiveItem(internal_dict=archive) if archive.version != 2: raise Exception("Unknown archive metadata version") @@ -2301,7 +2275,7 @@ class ArchiveChecker: 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", salt=salt) + data = self.key.pack_metadata(archive.as_dict()) new_archive_id = self.key.id_hash(data) cdata = self.repo_objs.format(new_archive_id, {}, data, ro_type=ROBJ_ARCHIVE_META) add_reference(new_archive_id, len(data), cdata) diff --git a/src/borg/cache.py b/src/borg/cache.py index fb99309aa..c18f315a2 100644 --- a/src/borg/cache.py +++ b/src/borg/cache.py @@ -755,7 +755,7 @@ class LocalCache(CacheStatsMixin): nonlocal processed_item_metadata_chunks csize, data = decrypted_repository.get(archive_id) chunk_idx.add(archive_id, 1, len(data)) - archive, _ = self.key.unpack_and_verify_archive(data) + archive = self.key.unpack_archive(data) 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 ef9f86efa..42bd3c582 100644 --- a/src/borg/crypto/key.py +++ b/src/borg/crypto/key.py @@ -72,15 +72,6 @@ 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 @@ -90,15 +81,6 @@ class TAMInvalid(IntegrityError): 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.""" @@ -242,6 +224,10 @@ class KeyBase: tam["hmac"] = hmac.digest(tam_key, packed, "sha512") return msgpack.packb(metadata_dict) + def pack_metadata(self, metadata_dict): + metadata_dict = StableDict(metadata_dict) + return msgpack.packb(metadata_dict) + def unpack_and_verify_manifest(self, data): """Unpack msgpacked *data* and return manifest.""" if data.startswith(b"\xc1" * 4): @@ -276,35 +262,14 @@ class KeyBase: logger.debug("TAM-verified manifest") return unpacked - def unpack_and_verify_archive(self, data): - """Unpack msgpacked *data* and return (object, salt).""" + def unpack_archive(self, data): + """Unpack msgpacked *data* and return archive metadata dict.""" data = bytearray(data) unpacker = get_limited_unpacker("archive") unpacker.feed(data) unpacked = unpacker.unpack() - if "tam" not in unpacked: - archive_name = unpacked.get("name", "") - raise ArchiveTAMRequiredError(archive_name) - tam = unpacked.pop("tam", None) - if not isinstance(tam, dict): - raise ArchiveTAMInvalid() - tam_type = tam.get("type", "") - if tam_type != "HKDF_HMAC_SHA512": - raise TAMUnsupportedSuiteError(repr(tam_type)) - 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") - 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, tam_salt + unpacked.pop("tam", None) # legacy + return unpacked class PlaintextKey(KeyBase): diff --git a/src/borg/helpers/parseformat.py b/src/borg/helpers/parseformat.py index 7f8456044..4b0caf6cd 100644 --- a/src/borg/helpers/parseformat.py +++ b/src/borg/helpers/parseformat.py @@ -723,12 +723,11 @@ 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", "tam"), + ("archive", "name", "comment", "id"), ("start", "time", "end", "command_line"), ("hostname", "username"), ("size", "nfiles"), diff --git a/src/borg/testsuite/archiver/check_cmd.py b/src/borg/testsuite/archiver/check_cmd.py index 01c313d56..a932cbae7 100644 --- a/src/borg/testsuite/archiver/check_cmd.py +++ b/src/borg/testsuite/archiver/check_cmd.py @@ -241,7 +241,7 @@ def test_manifest_rebuild_duplicate_archive(archivers, request): "time": "2016-12-15T18:49:51.849711", "version": 2, } - archive = repo_objs.key.pack_and_authenticate_metadata(archive_dict, context=b"archive") + archive = repo_objs.key.pack_metadata(archive_dict) archive_id = repo_objs.id_hash(archive) repository.put(archive_id, repo_objs.format(archive_id, {}, archive, ro_type=ROBJ_ARCHIVE_META)) repository.commit(compact=False) diff --git a/src/borg/testsuite/archiver/checks.py b/src/borg/testsuite/archiver/checks.py index c3ed2720a..719ff2ac8 100644 --- a/src/borg/testsuite/archiver/checks.py +++ b/src/borg/testsuite/archiver/checks.py @@ -8,7 +8,7 @@ import pytest from ...cache import Cache, LocalCache from ...constants import * # NOQA from ...crypto.key import TAMRequiredError -from ...helpers import Location, get_security_dir, bin_to_hex, archive_ts_now +from ...helpers import Location, get_security_dir, bin_to_hex from ...helpers import EXIT_ERROR from ...helpers import msgpack from ...manifest import Manifest, MandatoryFeatureUnsupported @@ -382,59 +382,6 @@ 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, ro_type=ROBJ_ARCHIVE_META) - 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}{NL}'") - assert "archive_tam" 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") - cmd(archiver, "check", "--repair") - output = cmd(archiver, "rlist", "--format='{name}{NL}'") - assert "archive_tam" 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 749c792e4..c862484ed 100644 --- a/src/borg/testsuite/key.py +++ b/src/borg/testsuite/key.py @@ -11,7 +11,7 @@ from ..crypto.key import AEADKeyBase 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, ArchiveTAMInvalid +from ..crypto.key import TAMRequiredError, TAMInvalid, TAMUnsupportedSuiteError from ..crypto.key import UnsupportedManifestError, UnsupportedKeyFormatError from ..crypto.key import identify_key from ..crypto.low_level import IntegrityError as IntegrityErrorBase @@ -276,15 +276,11 @@ class TestTAM: 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_unknown_type(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) @pytest.mark.parametrize( "tam, exc", @@ -300,20 +296,6 @@ class TestTAM: 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)), @@ -329,8 +311,6 @@ class TestTAM: 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_manifest(self, key): data = {"foo": "bar"} @@ -346,15 +326,10 @@ class TestTAM: 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, _ = key.unpack_and_verify_archive(blob) + blob = key.pack_metadata(data) + unpacked = key.unpack_archive(blob) assert unpacked["foo"] == "bar" - assert "tam" not in unpacked + assert "tam" not in unpacked # legacy @pytest.mark.parametrize("which", ("hmac", "salt")) def test_tampered_manifest(self, key, which): @@ -371,21 +346,6 @@ class TestTAM: 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."""