check --repair: resync past corrupt object headers when rebuilding the chunks index

docs/internals/packs.rst

@@ -62,10 +62,12 @@ Format version ``0x02`` (``OBJ_VERSION_HEADER_AAD``) binds the header's first 41
 authenticated together with the ciphertext, but not itself encrypted). This applies to all borg 2
 modes: the AEAD encryption modes (AES-256-OCB, ChaCha20-Poly1305) authenticate it with their AEAD
 tag, the ``authenticated-*`` modes with their MAC and the ``none-*`` modes with their (unkeyed)
-checksum, see :ref:`tagged_envelope`. ``meta_size`` and ``data_size`` are excluded from
-the AAD; tampering with either still fails the check, because it changes the length of the
-slice being read. A forged ``chunk_id``, version, or magic byte therefore fails
-authentication in ``RepoObj.parse()``/``parse_meta()``.
+checksum, see :ref:`tagged_envelope`. ``meta_size`` and ``data_size`` are excluded from the AAD.
+``RepoObj.parse()`` reads both slots, so tampering with either size still fails the check, by
+changing the length of the slice being read. ``parse_meta()`` reads the metadata slot alone: it
+catches a changed ``meta_size`` the same way, but not a changed ``data_size``, which the repair
+walk described below pins separately. A forged ``chunk_id``, version, or magic byte fails
+authentication in both.
 
 ``encrypted_meta`` and ``encrypted_data`` each add a one-byte slot tag on top of the shared header
 AAD -- ``b"M"`` for ``encrypted_meta``, ``b"D"`` for ``encrypted_data`` -- binding each ciphertext to
@@ -78,7 +80,8 @@ Format version ``0x01`` (``OBJ_VERSION_NO_HEADER_AAD``) authenticates ``encrypte
 writes version ``0x02``; ``parse()``/``parse_meta()`` accept both versions.
 
 ``iter_headers()`` (used for pack recovery/compaction, see below) reads the header without
-decrypting, so it does not check header AAD authentication.
+decrypting, so it does not check header AAD authentication. The repair walk described below is the
+exception: given a validator it reads and decrypts each metadata slot, and thus does check it.
 
 .. figure:: pack-objheader.png
     :width: 100%
@@ -87,17 +90,55 @@ decrypting, so it does not check header AAD authentication.
 
     The fixed 49-byte blob header. ``meta_size`` and ``data_size`` drive
     traversal; integrity comes from the content-addressed pack name and the
-    per-blob tag, which authenticates magic/version/chunk_id as additional
-    authenticated data.
+    per-blob tag, which at version ``0x02`` authenticates magic/version/chunk_id
+    as additional authenticated data.
 
 A reader locates the next blob by advancing::
 
     next_blob_offset = current_blob_offset + REPOOBJ_HEADER_SIZE + meta_size + data_size
 
-The per-blob magic limits the blast radius of corrupted length fields: if
-``meta_size`` or ``data_size`` is damaged, the scanner loses at most one blob.
-Once it finds the next ``OBJ_MAGIC`` sequence it resumes. Other corruption
-(payload bit flips) is caught by AEAD on that blob without losing position.
+``iter_headers()`` checks every header it walks: it must have ``OBJ_MAGIC``, a
+supported version, and sizes that keep the blob inside the pack and within
+``MAX_DATA_SIZE``. A header that fails these checks means a corrupt pack, and
+``IntegrityError`` is raised, naming which check it failed.
+
+The per-blob magic limits the blast radius of corrupted length fields. The
+repair walk (``iter_headers(validate=...)``, used when ``borg check --repair``
+rebuilds the chunks index from the packs) validates every header it walks,
+reading the metadata slot along with it: the slot's tag covers the slot itself
+and the chunk id, and at version ``0x02`` the header AAD described above, so a
+corrupted chunk id or ``meta_size`` fails it; a corrupted magic fails the magic
+check, and a corrupted version fails because the version decides which AAD the
+slot is parsed with. ``data_size`` - the one header field outside the tag at
+either version - must equal ``csize`` (the data payload size recorded in the
+tagged metadata) plus the key's fixed envelope overhead. A header that fails
+makes the walk scan for the next blob that validates and resume there, so the
+blobs after the damaged one are still found; the damaged blob itself is dropped,
+it can not be read back.
+
+The walk rebuilds the index. The damaged bytes stay where they are, as a gap no
+index entry covers, and a pack is named by the
+sha256 of its content, so a pack damaged in the store keeps failing the
+store-level check that ``borg check`` runs over ``packs/``: that check keeps
+reporting the pack after ``borg check --repair`` has rebuilt the index from it.
+Rewriting such a pack is repository-level repair, see :issue:`10026`.
+
+``OBJ_MAGIC`` occurs inside the payloads as well, so the scan accepts a candidate
+only when it validates like any walked header. Validating needs the key, so a
+repair that cannot read the manifest walks without it.
+
+In the ``none-*`` modes the tag is an unkeyed checksum, and in the
+``authenticated-*`` modes it binds a blob to its chunk id and nothing else (see
+:ref:`security_structural_auth`), so validating does not establish there that
+this repository wrote the blob. Both modes also store payloads as they are, so a
+backed up file can contain something that validates - the blobs of a repository
+sharing the key, for instance. Such a blob reads back as itself, adding a chunk
+nothing references, but the extent its ``data_size`` claims covers whatever
+follows it at that offset, which the walk then skips. The scan reaches a payload
+only after the blob owning it failed to validate, so a corrupt header is what
+makes this reachable.
+
+Bit flips in the data are caught when the blob is read, on that blob alone.
 
 Blobs follow one another contiguously with no padding::
 

src/borg/archive.py

@@ -54,7 +54,7 @@
 from . import platform
 from .platform import acl_get, acl_set, set_flags, get_flags, set_times, swidth
 from .repository import Repository, NoManifestError
-from .repoobj import RepoObj
+from .repoobj import RepoObj, object_validator
 
 # macOS: SF_DATALESS marks dataless placeholder files (e.g. cloud files not materialized locally).
 # Reading such files triggers downloading their content. stat.SF_DATALESS is only available
@@ -2181,7 +2181,22 @@ def check(
         # so we do not rebuild it from the packs (reading every pack is far too slow for a routine check).
         # --repair does rebuild from the packs (slow_rebuild=repair), working from the real packs so it
         # can detect and fix archives that reference chunks whose pack has gone missing.
-        self.chunks = build_chunkindex_from_repo(self.repository, slow_rebuild=repair, write_immediately=False)
+        # --repair also passes validate, which makes the rebuild resync past a corrupt object header.
+        # Every object header is validated, since a corrupt data_size parses and points the walk into
+        # the middle of the pack. Cost: one metadata slot read and one decryption per object.
+        # Validating needs the key, so read it here. manifest_only=True, because the other source
+        # make_key reads keys from is self.chunks, which is only built below.
+        if repair and self.key is None:
+            try:
+                self.key = self.make_key(repository, manifest_only=True)
+            except IntegrityError as err:
+                logger.warning(
+                    f"Could not read the key ({err}), so a pack with a corrupt object header stays as it is."
+                )
+        validate = object_validator(RepoObj(self.key)) if repair and self.key is not None else None
+        self.chunks = build_chunkindex_from_repo(
+            self.repository, slow_rebuild=repair, validate=validate, write_immediately=False
+        )
         if self.key is None:
             self.key = self.make_key(repository)
         self.repo_objs = RepoObj(self.key)
@@ -2712,9 +2727,15 @@ def finish(self):
             self.repository.flush()
             if self.chunks_modified:
                 # the packs changed, so the index no longer matches them: rebuild it from the packs
-                # and persist it.
+                # and persist it: deleting a defect chunk rewrites its pack and repoints that
+                # pack's other objects in the repository's index, so our offsets for them are stale.
                 logger.info("Rebuilding and writing the repository chunks index.")
-                build_chunkindex_from_repo(self.repository, slow_rebuild=True, write_immediately=True)
+                build_chunkindex_from_repo(
+                    self.repository,
+                    slow_rebuild=True,
+                    validate=object_validator(self.repo_objs),
+                    write_immediately=True,
+                )
             else:
                 # the packs are unchanged, so the index still matches them: persist it as is.
                 logger.info("Writing the rebuilt repository chunks index.")

src/borg/cache.py

@@ -857,10 +857,18 @@ def repack_chunkindex(repository):
 
 
 def build_chunkindex_from_repo(
-    repository, *, slow_rebuild=False, fragments_only=False, write_immediately=False, init_flags=ChunkIndex.F_USED
+    repository,
+    *,
+    slow_rebuild=False,
+    fragments_only=False,
+    validate=None,
+    write_immediately=False,
+    init_flags=ChunkIndex.F_USED,
 ):
     # fragments_only: build the index from the index/ fragments only, returning None if they cannot be
     # read completely, and never write to the repo.
+    # validate: a repo object validator, handed to PackReader.iter_headers so the rebuild skips the
+    # objects that fail it.
     assert not (slow_rebuild and fragments_only)
     assert not (fragments_only and write_immediately)  # fragments_only never writes to the repo
     # first, try to build a fresh, mostly complete chunk index from centrally stored index fragments:
@@ -952,7 +960,7 @@ def build_chunkindex_from_repo(
         repository._lock_refresh()
         pi.show(increase=1)
         pack_id = hex_to_bin(info.name)
-        for chunk_id, obj_offset, obj_size in PackReader(repository.store, pack_id).iter_headers():
+        for chunk_id, obj_offset, obj_size in PackReader(repository.store, pack_id).iter_headers(validate=validate):
             num_chunks += 1
             chunks[chunk_id] = ChunkIndexEntry(
                 flags=init_flags, size=0, pack_id=pack_id, obj_offset=obj_offset, obj_size=obj_size

src/borg/repoobj.py

@@ -6,6 +6,7 @@
 from .helpers import msgpack, workarounds
 from .helpers.errors import Error, IntegrityError
 from .compress import Compressor, LZ4_COMPRESSOR
+from .crypto.low_level import IntegrityError as IntegrityErrorBase
 
 # Workaround for lost passphrase or key in the "authenticated-*" modes
 AUTHENTICATED_NO_KEY = "authenticated_no_key" in workarounds
@@ -75,7 +76,9 @@ def get_assert_id_places():
 # Size of the header prefix used as AEAD AAD (additional authenticated data: authenticated together
 # with the ciphertext, but not itself encrypted) for OBJ_VERSION_HEADER_AAD objects: magic(8) +
 # version(1) + chunk_id(32). meta_size and data_size are excluded, since they are only known after
-# encryption; a change to either still fails authentication, by changing the ciphertext slice length.
+# encryption. A change to either changes the ciphertext slice length, so parse(), which reads both
+# slots, fails authentication; parse_meta() reads the metadata slot alone and thus does not see a
+# changed data_size.
 REPOOBJ_HEADER_AAD_SIZE = len(OBJ_MAGIC) + 1 + 32
 
 META_AAD_TAG = b"M"
@@ -277,5 +280,37 @@ def parse(
         return meta_compressed if want_compressed else meta, data_compressed if want_compressed else data
 
 
+def object_validator(repo_objs):
+    """Return validate(chunk_id, obj): True if obj is the repo object with id chunk_id.
+
+    obj is an object's header plus its metadata slot. Parsing that slot verifies its tag, which is
+    computed over the slot itself and over the chunk id, so a wrong meta_size or chunk id fails it.
+    At object version OBJ_VERSION_HEADER_AAD the magic and the version are covered as well (as AAD,
+    additional authenticated data: bytes the tag covers without being part of the ciphertext); at
+    OBJ_VERSION_NO_HEADER_AAD they are not: a wrong magic fails the explicit magic check, and a
+    wrong version fails because the version selects the AAD the slot is parsed with. data_size, the
+    one header field outside the tag at either version, must match csize - the data slot's payload
+    size, recorded in the tagged metadata - plus the key's fixed envelope overhead.
+
+    In the "none-*" modes the tag is an unkeyed checksum, and in the "authenticated-*" modes it is
+    deterministic and binds an object to its chunk id alone. Both therefore accept an object that a
+    backed up file contains, at any offset in any pack.
+    """
+    hdr_size = RepoObj.obj_header.size
+    overhead = repo_objs.key.PAYLOAD_OVERHEAD  # the envelope adds a fixed number of bytes to the payload
+
+    def validate(chunk_id, obj):
+        try:
+            meta = repo_objs.parse_meta(chunk_id, obj, ro_type=ROBJ_DONTCARE)
+            data_size = RepoObj.ObjHeader(*RepoObj.obj_header.unpack(obj[:hdr_size])).data_size
+            return data_size == meta["csize"] + overhead
+        except (IntegrityErrorBase, msgpack.UnpackException, KeyError, TypeError, IndexError):
+            # arbitrary bytes fail the tag, the msgpack unpacking, the length checks or the csize lookup.
+            # Anything else is a bug and propagates: dropping an object over it would lose data.
+            return False
+
+    return validate
+
+
 # Backward compatibility: RepoObj1 has moved to borg.legacy.repoobj
 from .legacy.repoobj import RepoObj1  # noqa: F401