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

docs/internals/packs.rst

@@ -91,10 +91,29 @@ 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. A header that
+fails these checks means a corrupt pack, and ``IntegrityError`` is raised.
+
+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) scans forward for the next blob and
+resumes there, so the blobs after the damaged part of the pack are still found.
+
+``OBJ_MAGIC`` occurs inside the payloads as well, and in the ``none-*`` and
+``authenticated-*`` modes the payloads are user content stored as it is, so a
+backed up file can contain something shaped like a blob. A candidate is
+therefore accepted only when its metadata slot verifies against the header AAD
+described above; the header and that slot, a few hundred bytes, are what the
+scan reads. Verifying needs the key, so a repair that cannot read the manifest
+walks without scanning.
+
+In the ``none-*`` modes the tag is an unkeyed checksum, so the scan accepts any
+well-formed blob, including one a backed up file contains.
+
+``data_size`` is not part of the AAD, so accepting a candidate authenticates
+its chunk id, and its size only as far as the blob fits into the pack. 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

@@ -1875,6 +1875,28 @@ def __next__(self):
             return next(self._unpacker)
 
 
+def resync_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 header's magic, version and chunk id as well (AAD, additional authenticated
+    data: bytes the tag covers without being part of the ciphertext).
+
+    In the "none-*" modes the tag is an unkeyed checksum, so validate accepts any well-formed
+    object, including one that a backed up file contains.
+    """
+
+    def validate(chunk_id, obj):
+        try:
+            repo_objs.parse_meta(chunk_id, obj, ro_type=ROBJ_DONTCARE)
+        except Exception:
+            # arbitrary bytes fail the tag, the msgpack unpacking or the length checks.
+            return False
+        return True
+
+    return validate
+
+
 class ArchiveChecker:
     # Bound how many missing file chunks rebuild_archives buffers for its end-of-run report,
     # so checking a badly damaged repo with very many missing chunks can not exhaust memory.
@@ -1926,7 +1948,18 @@ 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.
+        # 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"{err}. Packs with a corrupt object header can not be repaired.")
+        validate = resync_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)

src/borg/cache.py

@@ -852,10 +852,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: handed to PackReader.iter_headers when rebuilding from the packs, making it resync
+    # past a corrupt object header rather than raise IntegrityError.
     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:
@@ -942,7 +950,7 @@ def build_chunkindex_from_repo(
         # PackReader uses the store directly, so refresh the lock here; a full rebuild can be slow.
         repository._lock_refresh()
         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/repository.py

@@ -30,14 +30,17 @@
 from .storelocking import Lock
 from .logger import create_logger
 from .manifest import NoManifestError
-from .repoobj import RepoObj, OBJ_MAGIC
+from .repoobj import RepoObj, OBJ_MAGIC, SUPPORTED_OBJ_VERSIONS
 from .crypto.key import is_keyfile
 
 logger = create_logger(__name__)
 
 # an object name is its sha256 as 64 lowercase hex digits.
 _valid_object_name = re.compile(r"[0-9a-f]{64}").fullmatch
 
+# how much of a pack PackReader reads at once when searching for the next object header.
+RESYNC_WINDOW_SIZE = 1024 * 1024
+
 
 def repo_lister(repository, *, limit=None):
     marker = None
@@ -362,24 +365,73 @@ def read(self, offset, size):
         return self.store.load(self.key, offset=offset, size=size)
 
     def size(self):
-        """Return the pack size in bytes; for a store-backed pack this is one metadata lookup."""
+        """Return the pack size in bytes (a store metadata lookup, unless the pack is in memory)."""
         if self.pack_contents is not None:
             return len(self.pack_contents)
         return self.store.info(self.key).size
 
-    def iter_headers(self):
+    @staticmethod
+    def _parse_header(hdr_data, offset, pack_size):
+        """Return the ObjHeader in hdr_data if it is a valid header at offset, None otherwise.
+
+        Valid means: OBJ_MAGIC, a supported version, and an object that fits into the pack.
+        """
+        hdr = RepoObj.ObjHeader(*RepoObj.obj_header.unpack(hdr_data))
+        if hdr.magic != OBJ_MAGIC or hdr.version not in SUPPORTED_OBJ_VERSIONS:
+            return None
+        if offset + RepoObj.obj_header.size + hdr.meta_size + hdr.data_size > pack_size:
+            return None
+        return hdr
+
+    def _find_header(self, offset, pack_size, validate):
+        """Scan forward from offset for the next object validate accepts, return its offset or None.
+
+        A pack has no framing besides the object headers, so this searches for OBJ_MAGIC. That byte
+        sequence also occurs inside payloads, so a candidate is accepted only when its header parses
+        and validate(chunk_id, obj) confirms the header and metadata slot at that position.
+        """
+        hdr_size = RepoObj.obj_header.size
+        while offset + hdr_size <= pack_size:
+            # a window at a time, so the scan costs one store request per RESYNC_WINDOW_SIZE bytes.
+            buf = bytes(self.read(offset, min(RESYNC_WINDOW_SIZE, pack_size - offset)))
+            if len(buf) < hdr_size:
+                break
+            pos = 0
+            while True:
+                pos = buf.find(OBJ_MAGIC, pos)
+                if pos < 0 or pos + hdr_size > len(buf):
+                    break  # not in this window, or a header overlapping its end: the next window has it
+                hdr = self._parse_header(buf[pos : pos + hdr_size], offset + pos, pack_size)
+                if hdr is not None:
+                    obj_size = hdr_size + hdr.meta_size + hdr.data_size
+                    # an object is at most MAX_DATA_SIZE bytes, so a bigger one is a false match on
+                    # OBJ_MAGIC inside a payload.
+                    if obj_size <= MAX_DATA_SIZE:
+                        size = hdr_size + hdr.meta_size  # the bytes validate looks at
+                        end = pos + size
+                        # the window holds these bytes, unless the candidate crosses its end.
+                        obj = buf[pos:end] if end <= len(buf) else self.read(offset + pos, size)
+                        if validate(hdr.chunk_id, obj):
+                            return offset + pos
+                pos += 1
+            # step by the window less one header, so a magic straddling the boundary is still found.
+            offset += max(len(buf) - (hdr_size - 1), 1)
+        return None
+
+    def iter_headers(self, validate=None):
         """Yield (chunk_id, offset, size) for each object by walking the fixed object headers.
 
-        Only the headers are read, not the payloads, so locating every object costs one short
-        range read per object (or just a slice, when the pack is already in memory), plus one
-        store metadata lookup for the pack size.
+        The walk reads a header per object: one short range read each (or a slice, for a pack in
+        memory), plus one store metadata lookup for the pack size.
+
+        A header must have OBJ_MAGIC, a supported version and describe an object that fits into
+        the pack, otherwise the pack is corrupt and IntegrityError is raised. A read shorter than
+        a header ends the walk: that is the end of the pack.
 
-        Each full header must have OBJ_MAGIC and describe an object that fits into the pack,
-        otherwise the pack is corrupt and IntegrityError is raised. Ending the walk instead
-        would be worse than raising: the chunks index rebuilt from these headers would just be
-        missing the rest of the pack, and borg check --repair would then "fix" the archives by
-        dropping chunks that are there.
-        A trailing partial header is the clean end of the pack, not corruption.
+        validate(chunk_id, obj) tells whether obj - an object's header and metadata slot - is the
+        repo object with id chunk_id. Given one, a corrupt header makes the walk resync instead:
+        it scans for the next object validate accepts, logs how many bytes that skipped and
+        continues there.
         """
         pack_hex = bin_to_hex(self.pack_id) if self.pack_id is not None else "<no id>"
         pack_size = self.size()
@@ -389,17 +441,26 @@ def iter_headers(self):
             hdr_data = self.read(offset, hdr_size)
             if len(hdr_data) < hdr_size:
                 break  # clean EOF, or trailing partial bytes
-            hdr = RepoObj.ObjHeader(*RepoObj.obj_header.unpack(hdr_data))
-            if hdr.magic != OBJ_MAGIC:
-                raise IntegrityError(
-                    f'pack {pack_hex}: no object header at offset {offset} (pack corruption), run "borg check"'
+            hdr = self._parse_header(hdr_data, offset, pack_size)
+            if hdr is None:
+                if validate is None:
+                    raise IntegrityError(
+                        f'pack {pack_hex}: invalid object header at offset {offset} (pack corruption), run "borg check"'
+                    )
+                next_offset = self._find_header(offset + 1, pack_size, validate)
+                if next_offset is None:
+                    logger.warning(
+                        f"pack {pack_hex}: invalid object header at offset {offset} and none after it, "
+                        f"skipping the remaining {pack_size - offset} bytes."
+                    )
+                    break
+                logger.warning(
+                    f"pack {pack_hex}: invalid object header at offset {offset}, "
+                    f"skipping {next_offset - offset} bytes to the next one."
                 )
+                offset = next_offset
+                continue
             obj_size = hdr_size + hdr.meta_size + hdr.data_size
-            if offset + obj_size > pack_size:
-                raise IntegrityError(
-                    f"pack {pack_hex}: object extends past end of file at offset {offset} "
-                    f'(pack corruption), run "borg check"'
-                )
             yield hdr.chunk_id, offset, obj_size
             offset += obj_size
 

src/borg/testsuite/archiver/check_cmd_test.py

@@ -688,6 +688,33 @@ def test_extra_chunks(archivers, request):
     cmd(archiver, "check", "-v", exit_code=0)  # check does not deal with orphans anymore
 
 
+def test_repair_resyncs_pack_with_corrupt_object_header(archivers, request):
+    """--repair rebuilds the chunks index from a pack whose object header is damaged.
+
+    A damaged header loses the object boundaries, so the rebuild scans for the next object that
+    authenticates and continues there. Authenticating needs the key, which --repair reads first.
+    """
+    archiver = request.getfixturevalue(archivers)
+    if archiver.get_kind() != "local":
+        pytest.skip("inspects the store directly")
+    check_cmd_setup(archiver)
+    cmd(archiver, "check", exit_code=0)
+
+    with Repository(archiver.repository_location, exclusive=True) as repository:
+        # damage the header of the second object of a pack that holds more than two.
+        by_pack = {}
+        for chunk_id, entry in repository.chunks.items():
+            by_pack.setdefault(entry.pack_id, []).append((entry.obj_offset, chunk_id))
+        pack_id, objs = next((p, sorted(o)) for p, o in by_pack.items() if len(o) > 2)
+        damaged_offset, _ = objs[1]
+        key = "packs/" + bin_to_hex(pack_id)
+        repository.store_store(key, corrupt(repository.store_load(key), damaged_offset))
+
+    output = cmd(archiver, "check", "--repair", "--debug", exit_code=0)
+    assert f"invalid object header at offset {damaged_offset}" in output
+    assert "bytes to the next one" in output  # the rebuild resumed at the next object
+
+
 def test_repair_finish_flushes_pack_writer(archivers, request):
     """finish() stores chunks re-added during --repair before it drops the index (#10055).
 

src/borg/testsuite/cache_test.py

@@ -26,7 +26,8 @@
 )
 from ..hashindex import ChunkIndex, ChunkIndexEntry
 from ..crypto.key import AESOCBKey
-from ..helpers import safe_ns
+from ..helpers import bin_to_hex, safe_ns
+from ..helpers import IntegrityError
 from ..helpers.msgpack import int_to_timestamp
 from ..manifest import Manifest
 from ..repository import Repository
@@ -505,6 +506,26 @@ def test_close_consolidates_fragments_across_sessions(tmp_path, monkeypatch):
             assert cid in index
 
 
+def test_build_chunkindex_repair_resyncs_after_corrupt_header(tmp_path):
+    """A corrupt object header fails the rebuild; with validate, the objects after it are indexed."""
+    from .repository_test import accept_all, fchunk
+
+    obj1 = bytearray(fchunk(b"first", chunk_id=H(90)))
+    obj2 = fchunk(b"second", chunk_id=H(91))
+    obj1[0] ^= 0xFF  # break the magic of the first object's header
+    pack_id = H(92)
+    with Repository(os.fspath(tmp_path / "repository"), exclusive=True, create=True) as repository:
+        repository.store_store("packs/" + bin_to_hex(pack_id), bytes(obj1) + obj2)
+        with pytest.raises(IntegrityError):
+            build_chunkindex_from_repo(repository, slow_rebuild=True)
+        # accept_all takes any candidate, so this covers the plumbing, not the authentication.
+        index = build_chunkindex_from_repo(repository, slow_rebuild=True, validate=accept_all)
+        assert H(91) in index  # found by resyncing past the damaged header
+        assert H(90) not in index  # its header is damaged, so its id is unknown
+        assert index[H(91)].pack_id == pack_id
+        assert index[H(91)].obj_offset == len(obj1)
+
+
 def test_repack_leaves_sealed_untouched_and_reconstructs(tmp_path, monkeypatch):
     """Sealed (>= MIN) fragments survive a repack; build_chunkindex_from_repo reconstructs the index."""
     monkeypatch.setattr(cache_mod, "CHUNKINDEX_FRAGMENT_ENTRIES_MIN", 1000)