Bind pack object header into AEAD authentication

docs/internals/packs.rst

@@ -53,14 +53,37 @@ The fixed part of each blob header is 49 bytes (``REPOOBJ_HEADER_SIZE``):
 ``len(OBJ_MAGIC)`` + 1 version + 32 chunk_id + 4 meta_size + 4 data_size.
 ``REPOOBJ_HEADER_SIZE = len(OBJ_MAGIC) + 1 + 32 + 4 + 4 = 49``
 
+Format version ``0x02`` (``OBJ_VERSION_HEADER_AAD``) binds the header's first 41 bytes (``OBJ_MAGIC``
++ version + ``chunk_id`` -- ``REPOOBJ_HEADER_AAD_SIZE``) into the AEAD authentication of
+``encrypted_meta`` and ``encrypted_data`` as additional authenticated data (AAD: data that is
+authenticated together with the ciphertext, but not itself encrypted). This applies to the AEAD
+encryption modes (AES-256-OCB, ChaCha20-Poly1305). ``meta_size`` and ``data_size`` are excluded from
+the AAD, since they are only known after encryption; tampering with either still fails authentication,
+because it changes the length of the ciphertext slice being decrypted. A forged ``chunk_id``, version,
+or magic byte therefore fails AEAD authentication in ``RepoObj.parse()``/``parse_meta()``.
+
+``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
+its slot. This stops an attacker controlling repo storage from swapping the two ciphertexts (adjusting
+``meta_size``/``data_size`` to match): decrypting a ciphertext under the wrong slot's AAD fails
+authentication.
+
+Format version ``0x01`` (``OBJ_VERSION_NO_HEADER_AAD``) authenticates ``encrypted_meta`` and
+``encrypted_data`` with ``aad=chunk_id`` only, without the header bound in. ``RepoObj.format()``
+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.
+
 .. figure:: pack-objheader.png
     :width: 100%
     :figclass: figure-padded
     :alt: The 49-byte RepoObj header: magic, version, chunk_id, meta_size, data_size.
 
     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 AEAD.
+    per-blob AEAD, which authenticates magic/version/chunk_id as additional
+    authenticated data.
 
 A reader locates the next blob by advancing::
 

src/borg/crypto/key.py

@@ -258,10 +258,10 @@ def id_hash(self, data):
         """Return HMAC hash using the "id" HMAC key"""
         raise NotImplementedError
 
-    def encrypt(self, id, data):
+    def encrypt(self, id, data, aad=b""):
         pass
 
-    def decrypt(self, id, data):
+    def decrypt(self, id, data, aad=b""):
         pass
 
     def assert_id(self, id, data):
@@ -340,10 +340,10 @@ def detect(cls, repository, manifest_data, *, other=False):
     def id_hash(self, data):
         return sha256(data).digest()
 
-    def encrypt(self, id, data):
+    def encrypt(self, id, data, aad=b""):
         return b"".join([self.TYPE_STR, data])
 
-    def decrypt(self, id, data):
+    def decrypt(self, id, data, aad=b""):
         self.assert_type(data[0], id)
         return memoryview(data)[1:]
 
@@ -379,16 +379,16 @@ class AESKeyBase(KeyBase):
 
     logically_encrypted = True
 
-    def encrypt(self, id, data):
+    def encrypt(self, id, data, aad=b""):
         # legacy, this is only used by the tests.
         next_iv = self.cipher.next_iv()
         return self.cipher.encrypt(data, header=self.TYPE_STR, iv=next_iv)
 
-    def decrypt(self, id, data):
+    def decrypt(self, id, data, aad=b""):
         self.assert_type(data[0], id)
         try:
             return self.cipher.decrypt(data)
-        except IntegrityError as e:
+        except low_level.IntegrityError as e:
             raise IntegrityError(f"Chunk {bin_to_hex(id)}: Could not decrypt [{str(e)}]")
 
     def init_from_given_data(self, *, crypt_key, id_key, chunk_seed):
@@ -978,10 +978,10 @@ def init_ciphers(self, manifest_data=None):
         if manifest_data is not None:
             self.assert_type(manifest_data[0])
 
-    def encrypt(self, id, data):
+    def encrypt(self, id, data, aad=b""):
         return b"".join([self.TYPE_STR, data])
 
-    def decrypt(self, id, data):
+    def decrypt(self, id, data, aad=b""):
         self.assert_type(data[0], id)
         return memoryview(data)[1:]
 
@@ -1071,26 +1071,29 @@ def assert_id(self, id, data):
             if not hmac.compare_digest(id_computed, id):
                 raise IntegrityError("Chunk %s: id verification failed" % bin_to_hex(id))
 
-    def encrypt(self, id, data):
+    def encrypt(self, id, data, aad=b""):
         # to encrypt new data in this session we use always self.cipher and self.sessionid
+        # aad is additional authenticated data: authenticated together with data, but not encrypted
+        # or returned.
         reserved = b"\0"
         iv = self.cipher.next_iv()
         if iv > self.MAX_IV:  # see the data-structures docs about why the IV range is enough
             raise IntegrityError("IV overflow, should never happen.")
         iv_48bit = iv.to_bytes(6, "big")
-        header = self.TYPE_STR + reserved + iv_48bit + self.sessionid
-        return self.cipher.encrypt(data, header=header, iv=iv, aad=id)
+        envelope_header = self.TYPE_STR + reserved + iv_48bit + self.sessionid
+        return self.cipher.encrypt(data, header=envelope_header, iv=iv, aad=aad + id)
 
-    def decrypt(self, id, data):
+    def decrypt(self, id, data, aad=b""):
         # to decrypt existing data, we need to get a cipher configured for the sessionid and iv from header
         self.assert_type(data[0], id)
         iv_48bit = data[2:8]
         sessionid = bytes(data[8:32])
         iv = int.from_bytes(iv_48bit, "big")
         cipher = self._get_cipher(sessionid, iv)
         try:
-            return cipher.decrypt(data, aad=id)
-        except IntegrityError as e:
+            return cipher.decrypt(data, aad=aad + id)
+        except low_level.IntegrityError as e:
+            # cipher.decrypt() raises low_level.IntegrityError, the base class of the IntegrityError raised below.
             raise IntegrityError(f"Chunk {bin_to_hex(id)}: Could not decrypt [{str(e)}]")
 
     def init_from_given_data(self, *, crypt_key, id_key, chunk_seed):

src/borg/repoobj.py

@@ -11,11 +11,29 @@
 
 
 OBJ_MAGIC = b"BORG_OBJ"
-OBJ_VERSION = 0x01
+
+# meta_encrypted/data_encrypted are AEAD-authenticated with aad=chunk_id.
+OBJ_VERSION_NO_HEADER_AAD = 0x01
+# meta_encrypted/data_encrypted are AEAD-authenticated with aad=header_aad+slot_tag+chunk_id. header_aad
+# is the header prefix (magic, version, chunk_id; REPOOBJ_HEADER_AAD_SIZE bytes). slot_tag is b"M" for
+# meta_encrypted, b"D" for data_encrypted, binding each ciphertext to its slot. format() writes this version.
+OBJ_VERSION_HEADER_AAD = 0x02
+OBJ_VERSION = OBJ_VERSION_HEADER_AAD
+# Versions accepted by parse() and parse_meta().
+SUPPORTED_OBJ_VERSIONS = (OBJ_VERSION_NO_HEADER_AAD, OBJ_VERSION_HEADER_AAD)
 
 # Fixed header size per blob: OBJ_MAGIC(8) + version(1) + chunk_id(32) + meta_size(4) + data_size(4)
 REPOOBJ_HEADER_SIZE = 49
 
+# 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.
+REPOOBJ_HEADER_AAD_SIZE = len(OBJ_MAGIC) + 1 + 32
+
+META_AAD_TAG = b"M"
+DATA_AAD_TAG = b"D"
+
 
 class RepoObj:
     # Object header: magic (8b), format version (1b), chunk_id (32b), meta size (4b), data size (4b).
@@ -31,7 +49,7 @@ def extract_crypted_data(cls, data: bytes) -> bytes:
         hdr = cls.ObjHeader(*cls.obj_header.unpack(data[:hdr_size]))
         if hdr.magic != OBJ_MAGIC:
             raise IntegrityError("invalid object magic")
-        if hdr.version != OBJ_VERSION:
+        if hdr.version not in SUPPORTED_OBJ_VERSIONS:
             raise IntegrityError(f"unsupported object version: {hdr.version}")
         overall_expected_size = hdr_size + hdr.meta_size + hdr.data_size
         if overall_expected_size != len(data):
@@ -62,6 +80,7 @@ def format(
         assert ro_type != ROBJ_DONTCARE
         meta["type"] = ro_type
         assert isinstance(id, bytes)
+        assert len(id) == 32  # struct format "32s" silently pads/truncates a wrong-length id
         assert isinstance(meta, dict)
         assert isinstance(data, (bytes, memoryview))
         assert compress or size is not None and ctype is not None and clevel is not None
@@ -77,9 +96,10 @@ def format(
             meta["clevel"] = clevel
             data_compressed = data  # is already compressed, is NOT prefixed by type/level bytes
             meta["csize"] = len(data_compressed)
-        data_encrypted = self.key.encrypt(id, data_compressed)
+        header_aad = OBJ_MAGIC + bytes([OBJ_VERSION]) + id
+        data_encrypted = self.key.encrypt(id, data_compressed, aad=header_aad + DATA_AAD_TAG)
         meta_packed = msgpack.packb(meta)
-        meta_encrypted = self.key.encrypt(id, meta_packed)
+        meta_encrypted = self.key.encrypt(id, meta_packed, aad=header_aad + META_AAD_TAG)
         hdr = self.ObjHeader(OBJ_MAGIC, OBJ_VERSION, id, len(meta_encrypted), len(data_encrypted))
         hdr_packed = self.obj_header.pack(*hdr)
         return hdr_packed + meta_encrypted + data_encrypted
@@ -97,14 +117,17 @@ def parse_meta(self, id: bytes, cdata: bytes, ro_type: str) -> dict:
         hdr = self.ObjHeader(*self.obj_header.unpack(obj[:hdr_size]))
         if hdr.magic != OBJ_MAGIC:
             raise IntegrityError("invalid object magic")
-        if hdr.version != OBJ_VERSION:
+        if hdr.version not in SUPPORTED_OBJ_VERSIONS:
             raise IntegrityError(f"unsupported object version: {hdr.version}")
         if hdr_size + hdr.meta_size > len(obj):
             raise IntegrityError(
                 f"object too small: expected at least {hdr_size + hdr.meta_size} bytes, got {len(obj)}"
             )
+        # header_aad, meta_aad: see OBJ_VERSION_HEADER_AAD above. b"" for OBJ_VERSION_NO_HEADER_AAD.
+        header_aad = bytes(obj[:REPOOBJ_HEADER_AAD_SIZE]) if hdr.version == OBJ_VERSION_HEADER_AAD else b""
+        meta_aad = header_aad + META_AAD_TAG if hdr.version == OBJ_VERSION_HEADER_AAD else header_aad
         meta_encrypted = obj[hdr_size : hdr_size + hdr.meta_size]
-        meta_packed = self.key.decrypt(id, meta_encrypted)
+        meta_packed = self.key.decrypt(id, meta_encrypted, aad=meta_aad)
         meta = msgpack.unpackb(meta_packed)
         if ro_type != ROBJ_DONTCARE and meta["type"] != ro_type:
             raise IntegrityError(f"ro_type expected: {ro_type} got: {meta['type']}")
@@ -134,18 +157,22 @@ def parse(
         hdr = self.ObjHeader(*self.obj_header.unpack(obj[:hdr_size]))
         if hdr.magic != OBJ_MAGIC:
             raise IntegrityError("invalid object magic")
-        if hdr.version != OBJ_VERSION:
+        if hdr.version not in SUPPORTED_OBJ_VERSIONS:
             raise IntegrityError(f"unsupported object version: {hdr.version}")
         overall_expected_size = hdr_size + hdr.meta_size + hdr.data_size
         if overall_expected_size != len(obj):
             raise IntegrityError(f"object size inconsistent: expected {overall_expected_size} bytes, got {len(obj)}")
+        # header_aad, meta_aad: see parse_meta().
+        header_aad = bytes(obj[:REPOOBJ_HEADER_AAD_SIZE]) if hdr.version == OBJ_VERSION_HEADER_AAD else b""
+        meta_aad = header_aad + META_AAD_TAG if hdr.version == OBJ_VERSION_HEADER_AAD else header_aad
+        data_aad = header_aad + DATA_AAD_TAG if hdr.version == OBJ_VERSION_HEADER_AAD else header_aad
         meta_encrypted = obj[hdr_size : hdr_size + hdr.meta_size]
-        meta_packed = self.key.decrypt(id, meta_encrypted)
+        meta_packed = self.key.decrypt(id, meta_encrypted, aad=meta_aad)
         meta_compressed = msgpack.unpackb(meta_packed)  # means: before adding more metadata in decompress block
         if ro_type != ROBJ_DONTCARE and meta_compressed["type"] != ro_type:
             raise IntegrityError(f"ro_type expected: {ro_type} got: {meta_compressed['type']}")
         data_encrypted = obj[hdr_size + hdr.meta_size : hdr_size + hdr.meta_size + hdr.data_size]
-        data_compressed = self.key.decrypt(id, data_encrypted)  # does not include the type/level bytes
+        data_compressed = self.key.decrypt(id, data_encrypted, aad=data_aad)  # does not include type/level
         if decompress:
             ctype = meta_compressed["ctype"]
             clevel = meta_compressed["clevel"]