Merge pull request #10441 from ThomasWaldmann/repo-defaults-mandatory

repo-create: always write config/defaults, so its removal is detected, refs #346

Author: ThomasWaldmann

docs/internals/data-structures.rst

@@ -41,7 +41,7 @@ config/
     the repository config (see :ref:`repo_config`), a text object
   defaults
     the repository defaults (see :ref:`repo_defaults`), in the key's store object
-    envelope (see below). Only present if ``borg repo-create`` was given a default.
+    envelope (see below). ``borg repo-create`` always writes it.
   space-reserve.N
     purely random binary data to reserve space, e.g. for disk-full emergencies.
     These objects are created and removed by ``borg repo-space``.
@@ -132,8 +132,9 @@ packs; any other command that needs the chunks index aborts, except ``borg compa
 and ``borg repo-compress``, which rebuild it from the packs, as they rewrite the whole
 chunks index anyway (under an exclusive lock). A corrupted cache is ignored and
 rebuilt. A lock object that fails the authentication is treated as a foreign exclusive
-lock, see :ref:`storelocking`. Commands that use ``config/defaults`` abort if it fails
-the authentication. The ``chunkindex-invalid`` marker has no content and is
+lock, see :ref:`storelocking`. The commands that use ``config/defaults`` abort if it is
+missing or fails the authentication, ``borg check --repair`` replaces it by empty defaults
+(see :ref:`repo_defaults`). The ``chunkindex-invalid`` marker has no content and is
 stored as is.
 
 
@@ -339,8 +340,13 @@ for it.
 Unlike the repository config, which borg must read before it knows the key, the
 defaults are stored in the :ref:`store object envelope <store_object_envelope>`,
 so they are authenticated: an attacker with write access to the storage can not
-change them (e.g. remove an ``obfuscate`` compression) without being noticed. A
-repository without the object has no defaults.
+change them (e.g. remove an ``obfuscate`` compression) without being noticed.
+``borg repo-create`` always writes the object, also when no default was given (an
+empty dict), so removing it is noticed as well: the commands that use the defaults
+refuse to run if the object is missing or fails the authentication. ``borg check``
+reports such an object and ``borg check --repair`` replaces it by empty defaults, so
+the repository can be used again (with the built-in defaults). Like the repository
+config, the defaults themselves can not be restored.
 
 .. _archive:
 

docs/internals/frontends.rst

@@ -848,6 +848,8 @@ Errors
         {} has no repository config.
     Repository.CheckNeeded rc: 12 traceback: yes
         Inconsistency detected. Please run "borg check {}".
+    Repository.DefaultsMissing rc: 34 traceback: no
+        Repository {} has no config/defaults object, run "borg check --repair" to store empty defaults.
     Repository.DoesNotExist rc: 13 traceback: no
         Repository {} does not exist.
     Repository.InsufficientFreeSpaceError rc: 14 traceback: no

src/borg/archiver/_common.py

@@ -183,14 +183,15 @@ def wrapper(self, args, **kwargs):
                         ro_cls = RepoObj1
                     manifest_ = Manifest.load(repository, other=False, ro_cls=ro_cls)
                     kwargs["manifest"] = manifest_
+                    if secure:
+                        assert_secure(repository, manifest_)
+                    # the repository defaults are read only after the security checks passed.
                     if "compression" in args:
                         if args.compression is None:  # not given, see default_compression()
                             args.compression = default_compression(repository)
                         manifest_.repo_objs.compressor = args.compression.compressor
                     if "chunker_params" in args and args.chunker_params == DEFAULT_CHUNKER_PARAMS:
                         args.chunker_params = default_chunker_params(repository)
-                    if secure:
-                        assert_secure(repository, manifest_)
                 if cache:
                     with Cache(
                         repository,

src/borg/archiver/check_cmd.py

@@ -4,17 +4,51 @@
 from ..archive import ArchiveChecker
 from ..constants import *  # NOQA
 from ..crypto.key import key_factory
-from ..helpers import set_ec, EXIT_WARNING, CancelledByUser, CommandError, Error
+from ..helpers import set_ec, EXIT_WARNING, CancelledByUser, CommandError, Error, IntegrityError
 from ..helpers import relative_time_marker_validator, yes, ArchiveFormatter, sig_int
 from ..helpers.argparsing import ArgumentParser
 from ..helpers.time import archive_ts_now, calculate_relative_offset
 from ..repoobj import RepoObj, object_validator
+from ..repository import Repository, DEFAULTS_NAME
 
 from ..logger import create_logger
 
 logger = create_logger()
 
 
+def check_repository_defaults(repository, *, repair):
+    """Verify the repository defaults object (config/defaults, see Repository.load_defaults).
+
+    "borg repo-create" always writes it, so a missing object was removed (or lost), like one that fails
+    the authentication of the key's envelope or does not deserialize. Such an object is an error: the
+    commands using the defaults refuse to run. With repair, empty defaults are stored instead, so the
+    repository can be used again (with the built-in defaults).
+
+    Returns False if a problem was found and not repaired.
+    """
+    try:
+        repository.load_defaults()
+    except Repository.DefaultsMissing:
+        problem = "is missing"
+    except IntegrityError:
+        problem = "fails the authentication"
+    except Repository.InvalidRepositoryConfig:
+        problem = "is malformed"
+    else:
+        return True
+    if not repair:
+        logger.error(
+            f'Repository defaults object {DEFAULTS_NAME} {problem}. Run "borg check --repair" to store empty defaults.'
+        )
+        return False
+    logger.warning(
+        f"Repository defaults object {DEFAULTS_NAME} {problem}, storing empty defaults. "
+        "The defaults set by borg repo-create are lost, the built-in defaults are used now."
+    )
+    repository.save_defaults({})
+    return True
+
+
 class CheckMixIn:
     @with_repository(exclusive=True, manifest=False)
     def do_check(self, args, repository):
@@ -91,6 +125,8 @@ def do_check(self, args, repository):
                 set_ec(EXIT_WARNING)
             if sig_int:  # repository check interrupted; skip the archive check
                 raise Error("Got Ctrl-C / SIGINT.")
+            if not check_repository_defaults(repository, repair=args.repair):
+                set_ec(EXIT_WARNING)
         if not args.repo_only and not archive_checker.check(
             repository,
             verify_data=args.verify_data,
@@ -128,7 +164,9 @@ def build_parser_check(self, subparsers, common_parser, mid_common_parser):
            not a MAC, this step does not detect tampering of the packs. The index objects
            are also authenticated with the key when they are loaded for that cross-check.
            A corrupt index ends the check after this step, as the archives check needs it,
-           unless ``--repair`` is given (see below). Running the repository check can
+           unless ``--repair`` is given (see below). This step also verifies the repository
+           defaults object (see ``borg repo-create``): it must be present and authenticate
+           with the key. Running the repository check can
            be split into multiple partial checks using ``--max-duration``.
            For ssh:// repositories, the server computes the hashes, so the pack contents do
            not have to travel over the network. For other remote backends, borg usually has
@@ -269,6 +307,8 @@ def build_parser_check(self, subparsers, common_parser, mid_common_parser):
            index entries of the chunks stored in missing packs (packs the index references,
            but that are absent from the repository). Only a full ``borg check --repair``
            repairs the archives that reference these chunks, ``--repository-only`` does not.
+           A missing or corrupt repository defaults object is replaced by empty defaults, so
+           the repository can be used again; the commands then use the built-in defaults.
 
         2. When checking the consistency and correctness of archives, repair mode might
            remove whole archives from the manifest if their archive metadata chunk is

src/borg/archiver/repo_create_cmd.py

@@ -46,8 +46,8 @@ def do_repo_create(self, args, repository, *, other_repository=None, other_manif
                 defaults["compression"] = str(args.compression)
             if args.chunker_params not in (None, DEFAULT_CHUNKER_PARAMS):
                 defaults["chunker_params"] = ",".join(str(p) for p in args.chunker_params)
-            if defaults:
-                repository.save_defaults(defaults)
+            # always written, also when empty: a missing object means it was removed, see load_defaults().
+            repository.save_defaults(defaults)
             # we know repo/packs/ still does not have any chunks stored in it, but for some stores, there
             # might be a lot of empty directories and listing them all might be rather slow, so we better
             # store an empty ChunkIndex now, so that the first repo operation does not have to build the
@@ -218,8 +218,11 @@ def build_parser_repo_create(self, subparsers, common_parser, mid_common_parser)
         Using the same chunker parameters is important for deduplication.
         ``borg repo-info`` shows the defaults.
 
-        The defaults are stored in the repository and protected by the repository key, so nobody without
-        the key can change them (e.g. remove an ``obfuscate`` compression) without being noticed.
+        The defaults are stored in the repository and protected by the repository key: changing them
+        (e.g. removing an ``obfuscate`` compression) needs the key. The defaults object is always
+        written, also when no default was given, so removing it is noticed, too: the commands refuse
+        to run if it is missing or fails the authentication. ``borg check`` reports such an object,
+        ``borg check --repair`` replaces it by empty defaults (the built-in defaults are used then).
 
         Creating a related repository
         +++++++++++++++++++++++++++++

src/borg/repository.py

@@ -891,6 +891,11 @@ class LegacyRepository(Error):
 
         exit_mcode = 29
 
+    class DefaultsMissing(Error):
+        """Repository {} has no config/defaults object, run "borg check --repair" to store empty defaults."""
+
+        exit_mcode = 34
+
     # Whole packs kept in memory for reads; the least recently used is evicted first.
     # Memory use is this count times the pack size.
     PACK_READER_CACHE_SIZE = 3
@@ -2466,29 +2471,35 @@ def save_defaults(self, defaults):
         """Store the repository defaults (the config/defaults store object).
 
         defaults: a dict mapping option names to their default values as strings, e.g.
-        {"compression": "zstd,3"}. The commands use such a default if the option was not given (see
-        with_repository). Unlike config/config, which is read before the key is known, the object is
-        stored in the key's envelope (see store_encrypt_store), so nobody without the key can change the
-        defaults (e.g. remove an "obfuscate" compression) without being noticed.
+        {"compression": "zstd,3"}, or {} for no defaults. The commands use such a default if the option
+        was not given (see with_repository).
+
+        "borg repo-create" always writes the object, so a repository without it lost it (see
+        load_defaults). Unlike config/config, which is read before the key is known, the object is stored
+        in the key's envelope (see store_encrypt_store), so changing the defaults (e.g. removing an
+        "obfuscate" compression) needs the key: a changed object fails the authentication, a removed one
+        is missing, and the commands refuse to run either way.
         """
         self.store_encrypt_store(DEFAULTS_NAME, msgpack.packb(defaults))
         self._defaults = dict(defaults)
 
     def load_defaults(self):
-        """Return the repository defaults stored by save_defaults(), or {} if there are none.
+        """Return the repository defaults stored by save_defaults(), {} if there are none.
 
         The store object is only read once, later calls return the same defaults.
 
-        Raises IntegrityError if the envelope authentication fails, InvalidRepositoryConfig if the
-        content is not a dict of strings.
+        Raises DefaultsMissing if the object is missing (it was removed or lost, "borg check --repair"
+        stores empty defaults), IntegrityError if the envelope authentication fails,
+        InvalidRepositoryConfig if the content is not a dict of strings.
         """
         if self._defaults is not None:
             return dict(self._defaults)
         try:
             data = self.store_load_decrypt(DEFAULTS_NAME)
         except StoreObjectNotFound:
-            self._defaults = {}
-            return {}
+            raise self.DefaultsMissing(self._location.canonical_path()) from None
+        except IntegrityError as err:
+            raise IntegrityError(f'{err.args[0]}. Run "borg check --repair" to store empty defaults.') from err
         try:
             defaults = msgpack.unpackb(data)
         except msgpack.UnpackException:

src/borg/testsuite/archiver/check_cmd_test.py

@@ -1,4 +1,5 @@
 import gc
+import os
 from pathlib import Path
 import re
 import shutil
@@ -70,6 +71,31 @@ def check_cmd_setup(archiver):
         create_src_archive(archiver, "archive2")
 
 
+@pytest.mark.parametrize("damage", ["missing", "corrupt"])
+def test_check_repository_defaults(archivers, request, damage):
+    # repo-create always writes config/defaults, so a missing object was removed, like one that fails the
+    # authentication. check reports it, check --repair replaces it by empty defaults (the set defaults
+    # are lost), so the repository can be used again.
+    archiver = request.getfixturevalue(archivers)
+    cmd(archiver, "repo-create", RK_ENCRYPTION, "--compression=zstd,5")
+    create_src_archive(archiver, "archive1")
+    assert "Default compression: zstd,5" + os.linesep in cmd(archiver, "repo-info")
+    with open_repository(archiver) as repository:
+        if damage == "missing":
+            repository.store_delete("config/defaults")
+        else:
+            repository.store_store("config/defaults", corrupt(repository.store_load("config/defaults"), -1))
+    problem = "is missing" if damage == "missing" else "fails the authentication"
+    output = cmd(archiver, "check", exit_code=EXIT_WARNING)
+    assert f"config/defaults {problem}" in output
+    assert "borg check --repair" in output
+    output = cmd(archiver, "check", "--repair", exit_code=0)
+    assert f"config/defaults {problem}, storing empty defaults" in output
+    cmd(archiver, "check", exit_code=0)
+    assert "Default compression: lz4 (built-in)" + os.linesep in cmd(archiver, "repo-info")
+    create_src_archive(archiver, "archive2")  # the repository is usable again
+
+
 def test_check_usage(archivers, request):
     archiver = request.getfixturevalue(archivers)
     check_cmd_setup(archiver)

src/borg/testsuite/archiver/repo_create_cmd_test.py

@@ -240,7 +240,7 @@ def test_repo_create_without_default_compression(archivers, request):
     create_regular_file(archiver.input_path, "file1", size=1024 * 80)
     cmd(archiver, "repo-create", RK_ENCRYPTION)
     with open_repository(archiver) as repository:
-        assert repository.load_defaults() == {}
+        assert repository.load_defaults() == {}  # the object exists (else DefaultsMissing), but is empty
     output = cmd(archiver, "repo-info")
     assert "Default compression: lz4 (built-in)" + os.linesep in output
     assert "Default chunker params: %s,%d,%d,%d,%d (built-in)" % CHUNKER_PARAMS + os.linesep in output
@@ -333,3 +333,22 @@ def test_default_chunker_params_invalid(archivers, request):
     else:
         with pytest.raises(Repository.InvalidRepositoryConfig):
             cmd(archiver, "create", "test", "input")
+
+
+def test_defaults_missing(archivers, request):
+    # repo-create always writes config/defaults, so a missing object was removed: the commands that use
+    # the defaults refuse to run, unless every default is given explicitly (see check for the repair).
+    archiver = request.getfixturevalue(archivers)
+    create_regular_file(archiver.input_path, "file1", size=1024 * 80)
+    cmd(archiver, "repo-create", RK_ENCRYPTION, "--compression=zstd,5")
+    with open_repository(archiver) as repository:
+        repository.store_delete("config/defaults")
+    for args in (("create", "test", "input"), ("repo-info",)):
+        if archiver.FORK_DEFAULT:
+            output = cmd(archiver, *args, exit_code=Repository.DefaultsMissing("x").exit_code)
+            assert "borg check --repair" in output
+        else:
+            with pytest.raises(Repository.DefaultsMissing):
+                cmd(archiver, *args)
+    cmd(archiver, "create", "--compression=lz4", "--chunker-params=fixed,4096", "test", "input")
+    assert stored_compression(archiver) == {(LZ4.ID, 255)}