parse_meta() does not authenticate anything in authenticated / authenticated-blake2 / none mode

[mr-raj12]

RepoObj.parse_meta() decrypts the metadata and unpacks it, but it never calls assert_id():

https://github.com/borgbackup/borg/blob/master/src/borg/repoobj.py#L163-L190

For the AEAD keys that is fine: the metadata ciphertext is authenticated, and the header prefix (magic, version, chunk_id) is its AAD, so a parse_meta() that returns means the metadata belongs to that chunk id.

For authenticated, authenticated-blake2 and none there is no such tag. KeyBase.id_check_is_authentication says it directly:

True (the default, e.g. for the "authenticated" and "none" modes): decrypt() verifies nothing, so the keyed id hash (resp. the plain sha256 for "none") is the only integrity/authenticity check a read has and thus must never be skipped

parse() honors that (repoobj.py:262-268), parse_meta() has no equivalent. In those modes parse_meta() is therefore just "msgpack.unpackb of whatever bytes were at that offset", with no check that they are this chunk's metadata, or metadata at all.

Effect today

Both callers use parse_meta() as a cheap filter and then run a full parse() on the objects they actually act on, so nothing gets written or extracted on unauthenticated metadata:

• ArchiveChecker.rebuild_archives_directory (archive.py:2162-2178) reads metadata for every chunk to find ROBJ_ARCHIVE_META, then re-reads and parse()s the full object.
• repo-compress transform (archiver/repo_compress_cmd.py:139) reads ctype/clevel/olevel to decide whether recompression is needed, then parse()s if it is.

The consequence is that in authenticated* mode the decision both make can be steered by metadata that was not authenticated: a modified meta["type"] makes rebuild_archives_directory skip an archive metadata chunk (the archive is then not rebuilt, silently), and a modified ctype/clevel makes repo-compress consider an object already recompressed and leave it alone. Neither corrupts data, but "the repository can decide what borg skips" is not what authenticated mode is supposed to allow: the whole point of that mode is that the keyed id hash detects repository-side modification.

Possible fix

parse_meta() cannot verify the id, it does not have the plaintext. So either:

• parse_meta() refuses for keys with id_check_is_authentication and the callers use parse() there, or
• callers check id_check_is_authentication themselves and use parse() in that case, or
• it stays as it is and the docstring says clearly that the result is unauthenticated for those key modes, so future callers do not assume otherwise.

I have no strong preference among those, but right now nothing in the code or the docstring warns about it.

Why now

This also decides part of #10083. The suggestion there is that the repair scan authenticates a candidate object by its metadata only, instead of reading the whole object. That works for the AEAD keys, but with parse_meta() as it is, authenticated* would accept any candidate whose metadata happens to unpack, which is exactly the case (payload bytes that look like an object) the check is there to reject.

[ThomasWaldmann]

Maybe the simplest solution is to ignore that problem now and remove "none" and "authenticated*" modes later, as already suggested there for "none": #9104

[mr-raj12]

Fine ! The current callers need no change either way: both parse_meta() uses re-parse() before acting (ArchiveChecker.rebuild_archives_directory archive.py:2183, Recompressor.transform repo_compress_cmd.py:144), so unauthenticated meta can only make them skip an object, not accept a bad one.

#10094 is where it costs something: its resync scan uses parse_meta() to decide whether some offset really holds an object. For none/authenticated* there is nothing to decrypt, so it reads the whole object and uses parse() for the id check. I asked over there whether that should go too.

[ThomasWaldmann]

#10095 new authenticated-* modes always do a MAC authentication, new none-* modes always do an unkeyed hash check.

So, guess that solves this?