Part 1: core: split BlockCipher into block-aligned BlockCipherEncryptor/Decryptor - #107
Part 1: core: split BlockCipher into block-aligned BlockCipherEncryptor/Decryptor#107dghgit wants to merge 4 commits into
Conversation
…ptor
Rework the block cipher streaming traits ahead of the first mode
implementations:
- Split the single BlockCipher trait into BlockCipherEncryptor and
BlockCipherDecryptor (mirroring KEMEncapsulator/KEMDecapsulator) so
the direction can be encoded in the implementing type. A minimal
BlockCipher supertrait carries the shared MAX_SECURITY_STRENGTH.
The SymmetricCipher one-shot API is no longer a supertrait.
- Replace the single-block do_{en,de}crypt_block[_out] with
do_{en,de}crypt_blocks[_out]<const N>, taking &[[u8; BLOCK_LEN]; N]
so the block count is compile-time and in/out lengths cannot
disagree.
- Add do_encrypt_init_rng(key, &mut dyn RNG) alongside do_encrypt_init,
matching the encaps/encaps_rng pattern.
- Remove the do_{en,de}crypt_final[_out] methods. The traits are now
strictly block-aligned; padding of arbitrary-length data belongs to
a separate PaddedEncryptor/PaddedDecryptor layer to be built on top.
Update the core-test-framework block cipher test to take separate
encryptor/decryptor type parameters and to exercise N = 1 and N = 2,
including mixed single/multi-block encrypt vs decrypt sequences.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Formatting for the previous commit; no semantic change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…traits Provided (default) methods on BlockCipherEncryptor -- encrypt_blocks, encrypt_blocks_rng, encrypt_blocks_out, encrypt_blocks_out_rng -- and on BlockCipherDecryptor -- decrypt_blocks, decrypt_blocks_out -- implemented once in the trait as init + blocks, so every block-aligned mode gets the house-standard take-data-return-result static API at no cost to implementors. Arbitrary-length one-shots remain the padding layer's job. The core-test-framework block cipher test now checks the one-shots agree with the streaming API and round-trip. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
b770f56 to
7f0380f
Compare
|
|
||
| * The core-test-framework block cipher test now takes separate encryptor/decryptor type parameters, exercises N = 1 and | ||
| N = 2 (including mixed single/multi-block encrypt vs decrypt sequences), and checks the one-shots agree with the | ||
| streaming API and round-trip. |
There was a problem hiding this comment.
Cut this down to a single line.
Or maybe just remove the release note for this PR entirely because the entire concept of symmetric ciphers is new in 0.1.3, so the release note will end up being:
- Implemented the following block ciphers, and the core traits to go along with them.
- AES
- AES_lowmemory
- ASCON
- ...
| /// Maximum security strength supported by the algorithm; keys tagged with a lower strength are | ||
| /// rejected by the `_init` constructors. | ||
| const MAX_SECURITY_STRENGTH: SecurityStrength; | ||
| } |
There was a problem hiding this comment.
This is a literal duplicate of Algorithm. Why add a new trait instead of bounding BlockCipherEncryptor on Algorithm instead of on BlockCipher?
There was a problem hiding this comment.
It was in SymmetricCipher. Algorithm would work though.
| ) -> Result<(Self, [u8; INIT_DATA_LEN]), SymmetricCipherError>; | ||
| /// Encrypts `N` consecutive blocks of plaintext. A sequence of calls is equivalent to one call over | ||
| /// the concatenation. | ||
| fn do_encrypt_blocks<const N: usize>( |
There was a problem hiding this comment.
This seems to me like an odd way to define this API.
How often is your application going to know N (ie the size of the plaintext) at compile-time?
I also still don't love forcing the user to break their plaintext into an array-of-arrays to match the block size -- something about that feels like leaking implementation detail through the abstraction layer. This should just take a plaintext as a simple &[u8].
I suggest that the better way to define this API would be:
/// Encrypts multiple consecutive blocks of plaintext. A sequence of calls is equivalent to one call over
/// the concatenation.
/// The provided `plaintext` must be an even number of blocks; ie a multiple of [`Self::BLOCK_LEN`],
/// or else a [`SymmetricCipherError::LengthError`] is returned.
/// Since block ciphers guarantee that ciphertext and plaintext are the same size, the ciphertext
/// is written to the provided `plaintext` buffer.
/// Returns the number of bytes written to `plaintext`.
fn do_encrypt_blocks(&mut self, plaintext: &mut [u8]) -> Result<usize, SymmetricCipherError>;| plaintext: &[[u8; BLOCK_LEN]; N], | ||
| ) -> Result<[[u8; BLOCK_LEN]; N], SymmetricCipherError>; | ||
| /// Encrypts `N` consecutive blocks of plaintext into the provided buffer. Returns `N * BLOCK_LEN`. | ||
| fn do_encrypt_blocks_out<const N: usize>( |
There was a problem hiding this comment.
If you take my suggestion to handle plaintext as a input/output argument, then this version can be deleted.
| ) -> Result<([u8; INIT_DATA_LEN], [[u8; BLOCK_LEN]; N]), SymmetricCipherError> { | ||
| let (mut enc, init_data) = Self::do_encrypt_init(key)?; | ||
| Ok((init_data, enc.do_encrypt_blocks(plaintext)?)) | ||
| } |
There was a problem hiding this comment.
I suggest instead:
/// One-shot: encrypts multiple blocks under a fresh init. Returns the generated init data and the ciphertext.
///
/// The provided `plaintext` must be an even number of blocks; ie a multiple of [`Self::BLOCK_LEN`],
/// or else a [`SymmetricCipherError::LengthError`] is returned.
/// Since block ciphers guarantee that ciphertext and plaintext are the same size, the ciphertext
/// is written to the provided `plaintext` buffer.
/// Returns the number of bytes written to `plaintext`.
fn encrypt(
key: &KeyMaterial<KEY_LEN>,
plaintext: &mut [u8],
) -> Result<([u8; INIT_DATA_LEN], usize), SymmetricCipherError> {
let (mut enc, init_data) = Self::do_encrypt_init(key)?;
Ok((init_data, enc.do_encrypt_blocks(plaintext)?))
}There was a problem hiding this comment.
Also, this is the first inline code in a trait. I honestly didn't know you could do that. I like it.
| ) -> Result<([u8; INIT_DATA_LEN], [[u8; BLOCK_LEN]; N]), SymmetricCipherError> { | ||
| let (mut enc, init_data) = Self::do_encrypt_init_rng(key, rng)?; | ||
| Ok((init_data, enc.do_encrypt_blocks(plaintext)?)) | ||
| } |
There was a problem hiding this comment.
I suggest instead:
/// Same as [`BlockCipherEncryptor::encrypt_blocks`], but sources randomness from the provided RNG.
fn encrypt_blocks_rng<const N: usize>(
key: &KeyMaterial<KEY_LEN>,
rng: &mut dyn RNG,
plaintext: &mut [u8],
) -> Result<([u8; INIT_DATA_LEN], usize), SymmetricCipherError> {
let (mut enc, init_data) = Self::do_encrypt_init_rng(key, rng)?;
Ok((init_data, enc.do_encrypt_blocks(plaintext)?))
}| } | ||
| /// One-shot: encrypts `N` blocks under a fresh init into the provided buffer. | ||
| /// Returns the generated init data and `N * BLOCK_LEN`. | ||
| fn encrypt_blocks_out<const N: usize>( |
There was a problem hiding this comment.
Can delete if we make the above have an input/output argument.
| Ok((init_data, enc.do_encrypt_blocks_out(plaintext, ciphertext)?)) | ||
| } | ||
| /// As [`BlockCipherEncryptor::encrypt_blocks_out`], but sources randomness from the provided RNG. | ||
| fn encrypt_blocks_out_rng<const N: usize>( |
| } | ||
|
|
||
| /// The decryption half of a block cipher's streaming API; see [`BlockCipherEncryptor`]. | ||
| pub trait BlockCipherDecryptor< |
There was a problem hiding this comment.
Ditto the changes to BlockEncryptor
|
Also, it's been bothering be for a while that the traits are not sorted alphabetically. This PR would be a good place to do that since it's just the symmetric cipher ones that are out-of-order. |
BlockCipher declared only MAX_SECURITY_STRENGTH, which Algorithm already has, so every implementor of BlockPermutation (which also implemented Algorithm) had two copies of the same constant and had to qualify every use of it. BlockPermutation, BlockCipherEncryptor and BlockCipherDecryptor are now bounded on Algorithm instead, and Cbc implements Algorithm with its permutation's ALG_NAME and MAX_SECURITY_STRENGTH. Raised in review of PR #107. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A block cipher mode never changes the length of its data, so a separate output buffer was only ever a copy, and a copy of plaintext is one more thing to scrub. The one-shots, the flat streaming methods and the implementor hook now all take a single `&mut [u8; LEN]` / `&mut [[u8; BLOCK_LEN]; N]` and transform it in place; `encrypt` and `encrypt_rng` return just the init data. The `_out` variants and the `usize` byte counts (always LEN) are gone. The compile-time `LEN % BLOCK_LEN == 0` check is unchanged. The data methods keep a `Result` for modes with a per-initialization data limit (counter-based modes); CBC never fails them, and the docs say so. Cbc, PaddedEncryptor/PaddedDecryptor, the test framework, the modes tests and benches, the doc examples and the CLI follow. The padding layer pads and encrypts the final block inside its `Secret`, so only ciphertext is ever copied out of it. Raised in review of PR #107. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A pure reordering (verified: the sorted non-blank lines are identical before and after). Each trait keeps its doc comment and any todo notes attached to it; SecurityStrength keeps its two impl blocks. Sorted case-insensitively by item name. Requested in review of PR #107. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ptor with multi-block and one-shot methods (PR #107)
Part 1 of 3 of the block-cipher stack, re-opened after
release/0.1.3alphawas rolled back toc026339so the three pieces can land as separate, linear PRs. Content is identical to #96 as previously reviewed and merged (commitse537e23..b770f56).BlockCipherintoBlockCipherEncryptor/BlockCipherDecryptor(mirroringKEMEncapsulator/KEMDecapsulator), with a minimalBlockCiphersupertrait carryingMAX_SECURITY_STRENGTH.do_*_block[_out]with multi-blockdo_*_blocks[_out]<const N>over[[u8; BLOCK_LEN]; N]; adddo_encrypt_init_rng; removedo_*_final(padding is a separate layer).Merge order: this PR, then #105 (Part 2, AES low-memory), then #106 (Part 3, CBC mode), then #97 (padding). Note for whoever merges: land it by pushing to
origin-- GitHub is a mirror and a GitHub-side merge gets reverted by the sync bot.