ssh-cipher: consolidate aes feature; extract Aes type#530
Conversation
|
Note: I think this approach can be further continued by making |
| /// Advanced Encryption Standard (AES) low-level block cipher. | ||
| /// | ||
| /// Supports 128-bit, 192-bit, and 256-bit key sizes. | ||
| pub(crate) enum Aes { | ||
| Aes128(Aes128), | ||
| Aes192(Aes192), | ||
| Aes256(Aes256), | ||
| } |
There was a problem hiding this comment.
@newpavlov I wonder if something like this would be useful in the aes crate itself (or perhaps a single type that supports all three key sizes and can dynamically select which one at runtime without having to monomorphize the full code three times)
There was a problem hiding this comment.
We could try it, but on the first glance it's likely to be a pretty annoying addition implementation-wise.
There was a problem hiding this comment.
It could use this enum-based implementation for now. It's simple enough.
The version on master hides it inside a wrapper struct, which would futureproof a possible conversion to support dynamic key sizes in the future while making it possible to support them today:
https://github.com/RustCrypto/SSH/blob/master/ssh-cipher/src/block_cipher/aes.rs
One drawback is we don't really have a trait like KeyInit that supports dynamic key sizes like this. It'd basically be KeyInit::new_from_slice split out into its own trait so there's no bound on KeySizeUser. Though I suppose TryFrom<&[u8], Error = InvalidLength> could probably get the job done for now.
There was a problem hiding this comment.
We could just use 256 bits as the default key size, since we usually use the largest supported key size for ciphers which support variable key sizes.
But looking at the code, I am not sure that the Aes enum is a good solution, personally I prefer the old enum with separate Aes128/192/256Cbc variants. It branches only ones, while with Aes you can potentially have multiple unnecessary branches while dealing with the mode.
Combines the `aes-cbc`, `aes-ctr`, and `aes-gcm` features into a single
`aes` feature which provides them all.
Additionally refactors the internals of `Encryptor` and `Decryptor`
around a new `Aes` type which provides an enum over the 128-bit,
192-bit, and 256-bit key sizes, and dynamically selects which one based
on the provided key size.
This type impls the `BlockCipherDecrypt`, `BlockCipherEncrypt`, and
`BlockSizeUser` traits which delegate to the inner cipher, since once
initialized with a key the API provided by the cipher is the same. These
delegate to the respective AES implementations for various key sizes.
Because it impls these traits, we're able to use it directly with types
like `cbc::{Encryptor, Decryptor}` and `Ctr128BE`, collapsing the
combinatorial explosion of key sizes and block modes down to just one
enum variant per block mode.
tarcieri
force-pushed
the
ssh-cipher/dynamic-aes-key-size
branch
from
408527f to
ba022d6
Compare
2 participants
Combines the
aes-cbc,aes-ctr, andaes-gcmfeatures into a singleaesfeature which provides them all.Additionally refactors the internals of
EncryptorandDecryptoraround a newAestype which provides an enum over the 128-bit, 192-bit, and 256-bit key sizes, and dynamically selects which one based on the provided key size.This type impls the
BlockCipherDecrypt,BlockCipherEncrypt, andBlockSizeUsertraits which delegate to the inner cipher, since once initialized with a key the API provided by the cipher is the same. These delegate to the respective AES implementations for various key sizes.Because it impls these traits, we're able to use it directly with types like
cbc::{Encryptor, Decryptor}andCtr128BE, collapsing the combinatorial explosion of key sizes and block modes down to just one enum variant per block mode.