crypto: misc. improvements to code and docs - #9979
Open
ThomasWaldmann wants to merge 7 commits into
Open
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9979 +/- ##
=======================================
Coverage 85.86% 85.87%
=======================================
Files 95 95
Lines 17064 17078 +14
Branches 2614 2616 +2
=======================================
+ Hits 14652 14665 +13
Misses 1673 1673
- Partials 739 740 +1 ☔ View full report in Codecov by Harness. |
ThomasWaldmann
force-pushed
the
improve-crypto
branch
3 times, most recently
from
July 31, 2026 15:42
f01d7ae to
ed7ba4b
Compare
…gbackup#6501 AES-OCB has a birthday-type security bound in the amount of data encrypted using one key: the attacker's advantage is about 6 * sigma^2 / 2^128, sigma being the number of 128bit cipher blocks (Krovetz/Rogaway OCB3, Theorem 1). RFC 7253 derives its "at most 2^48 blocks (4PiB) per key" rule of thumb from that bound, which corresponds to an advantage of 2^-32. We now aim higher and start a new session after 2^37 blocks (2TiB), giving an advantage of about 2^-51, in line with the target probability used in the examples of draft-irtf-cfrg-aead-limits. Starting a new session is cheap (one sha256 for the new session key) and fully transparent, because the session id is part of every chunk header - so old chunks stay decryptable and nothing changes for reading existing repositories. Rekeying that often also is what helps in the multi-key setting: the advantages of the individual session keys just add up, so the quantity that matters over the lifetime of a borg key is sum(sigma_i^2) and not (sum sigma_i)^2. chacha20-poly1305 does not need such a limit: its confidentiality bound does not depend on the amount of data encrypted at all and its integrity bound only limits the number of forgery attempts, counted over all keys.
The "48 bit IV is way more than needed" paragraph justified the IV size with the amount of data encrypted in one session, but the IV only limits the number of **messages** (2^48, borg refuses to encrypt more). How much **data** we may encrypt with one session key is determined by the security bounds of the ciphers. So now we state all 3 relevant quantities and the target probability we aim for: - number of messages (q): 2^48 per session key, never the binding limit. - data volume: AES-OCB only, 2^37 blocks (2TiB) per session key == p 2^-51 (RFC 7253's 4PiB rule of thumb corresponds to p 2^-32). chacha20-poly1305 does not have such a limit. - forgery attempts (v): chacha20-poly1305, about 2^33 at p 2^-50 for our biggest messages, counted over all session keys (so more sessions do not help there). Also mention the session key change in the in-depth crypto docs (security.rst).
The forgery attempts (v) limit is the only AEAD limit borg does not enforce (chacha20-poly1305: about 2^33 for our biggest messages at p 2^-50, AES-OCB with its 128bit auth tag is far less restrictive). Document that this is intentional: - a failed decryption means tampered or corrupted data, borg refuses it and usually aborts the whole command (borg check and archive listing keep going, but only to report the damage). - reaching the limit would need way more tampered data than a real repository will ever hold. Also note it at the place where the failed decryptions actually happen.
"AES-OCB, CHACHA20 ciphers all add a internal 32bit counter to the 96bit IV we provide" is only true for chacha20-poly1305: the ChaCha20 block function has a 32bit block counter besides the 96bit nonce, limiting a message to 2^32 blocks (256GiB) per (key, nonce). AES-OCB has no such counter, it derives the per-block offsets from the nonce (RFC 7253). The conclusions drawn from that claim were fine for both ciphers, so this is a comment/docs fix only: - the 2^32 blocks per message check stays as it is - it is required for chacha and just conservative for OCB (it can not trigger anyway, our messages are limited to MAX_DATA_SIZE == 20MiB). - incrementing the IV by 1 per message stays correct for both ciphers, because the cipher blocks of a message do not consume IVs. Also reworded the ValueError message, which claimed a counter overflow.
None of the ciphersuite classes in this module has next_iv(len(data)) - both the AEAD and the legacy AES-CTR classes compute the next iv without arguments. Also, header_len and aad_offset are constructor arguments, not encrypt/decrypt arguments, and encrypt takes iv and aad.
A truncated envelope (shorter than header + auth tag, for the legacy AES-CTR
modes: header + mac + iv) previously ended up calling EVP_DecryptUpdate with a
negative length, which OpenSSL luckily rejects, resulting in
CryptoError('EVP_DecryptUpdate failed').
But CryptoError means "malfunction in the crypto module" and no caller catches
it, so a truncated chunk (e.g. 32..47 bytes for the AEAD envelope layout) gave
an ugly crash instead of being handled like what it is: corrupted or tampered
data. Now we check the envelope length first and raise IntegrityError, which
the upper layers already handle properly.
Also: requirements_check() raised the NotImplemented constant (a TypeError at
runtime) instead of NotImplementedError, and the _AEAD_BASE key param docstring
called the AEAD key an "encrypt-then-mac key" - AEAD modes are exactly what
replaced borg 1.x's encrypt-then-MAC construction, they take a single AEAD key.
CryptoTestCase is part of the startup self test, so SELFTEST_COUNT needs to be
updated for the 2 test methods added here (otherwise every borg process aborts
with "Self-test count mismatch").
borg.crypto.low_level has no AES class - the legacy AES wrapper (only used for legacy key file encryption) lives in borg.legacy.crypto.low_level, which had no type stub at all. So move the stub entry where the class actually is.
ThomasWaldmann
force-pushed
the
improve-crypto
branch
from
July 31, 2026 15:47
ed7ba4b to
b890e9c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6501.
Claude says: Implements the assessment items from #6501 (comment), plus some related crypto code/docs fixes found while working on it.
Limit the data encrypted with one aes256-ocb session key (item 2)
AES-OCB has a birthday-type security bound in the amount of data encrypted using one key: the attacker's advantage is about
6 * sigma^2 / 2^128,sigmabeing the number of 128bit cipher blocks (Krovetz/Rogaway OCB3, Theorem 1). RFC 7253 derives its "use a key for at most 2^48 blocks (4PiB)" rule of thumb from that bound, which corresponds to an advantage of 2^-32.We now aim higher and start a new session after 2^37 blocks (2TiB), giving an advantage of about 2^-51, in line with the target probability used in the examples of draft-irtf-cfrg-aead-limits.
AEADKeyBase.encryptcounts the cipher blocks processed with the current session key (payload and AAD rounded up separately, +2 for the per-message cipher setup) and starts a new session before the limit would be exceeded. Starting a new session is just a new random session id, a new derived session key and IV counting from 0 again - cheap (one sha256) and fully transparent, because the session id is part of every chunk header, so old chunks stay decryptable and nothing changes for reading existing repositories. No repository format change.Rekeying that often is also what helps in the multi-key setting: the advantages of the individual session keys just add up, so the quantity that matters over the lifetime of a borg key is
sum(sigma_i^2)and not(sum sigma_i)^2.chacha20-poly1305 keeps
MAX_SESSION_BLOCKS = None(no limit, and no per-chunk counting overhead): its confidentiality bound does not depend on the amount of data encrypted at all (draft-irtf-cfrg-aead-limits 6.3.1) and its integrity bound only limits the number of forgery attempts, which is counted over all keys (7.2.1) and thus can not be improved by using more session keys anyway.Document the AEAD usage limits (items 3 + 4)
New "AEAD usage limits" section in
docs/internals/data-structures.rst: the IV range limits the number of messages per session key (2^48, never binding), the amount of data per session key is limited by the cipher security bounds (see above), and the forgery attempts limit (chacha20-poly1305, about 2^33 for our biggest messages at p 2^-50, counted over all keys) is deliberately not counted/enforced - borg refuses tampered data and usually aborts, and reaching the limit would need more tampered data than a real repository will ever hold. Cross-referenced from the in-depth crypto docs (security.rst) and noted at the decrypt code.Fix the "internal 32bit counter" claim (item 6)
"AES-OCB, CHACHA20 ciphers all add a internal 32bit counter to the 96bit IV we provide" was only true for chacha20-poly1305; AES-OCB has no such counter, it derives the per-block offsets from the nonce (RFC 7253). The conclusions drawn from the claim (2^32 blocks per message check, increment IV by 1 per message) stay valid for both ciphers and are now explained correctly. Also fixed the stale API sketch in the module docstring (
next_iv()takes no argument etc.).Truncated envelope fix (found in review)
A truncated envelope (shorter than header + auth tag; for the legacy AES-CTR modes: header + mac + iv) ended up calling
EVP_DecryptUpdatewith a negative length, which OpenSSL luckily rejects, surfacing asCryptoError('EVP_DecryptUpdate failed')- which no caller catches, so e.g. a 32..47 byte tampered chunk gave an ugly crash instead of clean error handling. Now the envelope length is checked first andIntegrityErroris raised, which the upper layers already handle as corrupted/tampered data.Also:
requirements_check()raised theNotImplementedconstant (aTypeErrorat runtime) instead ofNotImplementedError; the_AEAD_BASEkey docstring called the AEAD key an "encrypt-then-mac key";low_level.pyideclared anAESclass that does not exist in this module - the stub moved to a newborg/legacy/crypto/low_level.pyi, where the legacy AES wrapper actually lives; removed unused cimports/extern declarations.Tests
test_ocb_session_key_rollover: with the limit monkeypatched down, the session id changes, the IV starts from the beginning in each new session, and all chunks still decrypt.test_chpo_no_session_key_rollover: chacha20-poly1305 keeps one session id.test_AEAD_truncated_envelope/test_AE_truncated_envelope: truncated envelopes raiseIntegrityErrorfor both AEAD ciphers and the legacy CTR mode.borg createof 8MiB used 15 session keys, an unpatchedborg checkandborg extractthen read it back byte-identical.