-
Notifications
You must be signed in to change notification settings - Fork 32
Description
⚠️ Please ignore this issue before the #658 is reviewed and merged, solving it beforehand will cause file conflicts that might make the 658 unusable after a rebase
In the following file : crate/crypto/src/crypto/symmetric/symmetric_ciphers.rs
Around line 188 :
fn to_openssl_cipher(self) -> Result<Cipher, CryptoError> {
match self {
Self::Aes128Ecb => Ok(Cipher::aes_128_ecb()),
Self::Aes192Ecb => Ok(Cipher::aes_192_ecb()),
Self::Aes256Ecb => Ok(Cipher::aes_256_ecb()),
Self::Aes128Cbc => Ok(Cipher::aes_128_cbc()),
Self::Aes192Cbc => Ok(Cipher::aes_192_cbc()),
Self::Aes256Cbc => Ok(Cipher::aes_256_cbc()),
Self::Aes128Gcm => Ok(Cipher::aes_128_gcm()),
Self::Aes192Gcm => Ok(Cipher::aes_192_gcm()),
Self::Aes256Gcm => Ok(Cipher::aes_256_gcm()),
Self::Aes128Xts => Ok(Cipher::aes_128_xts()),
Self::Aes256Xts => Ok(Cipher::aes_256_xts()),
Self::Rfc3394_16 | Self::Rfc3394_24 | Self::Rfc3394_32 => {
crypto_bail!(CryptoError::NotSupported(
"RFC3394 is not supported in this version of openssl".to_owned()
))
}
Self::Rfc5649_16 | Self::Rfc5649_24 | Self::Rfc5649_32 => {
crypto_bail!(CryptoError::NotSupported(
"RFC5649 is not supported in this version of openssl".to_owned()
))
}
// rest of codeThe Kms will return an error and say openssl does not support those ciphers, however, it actually does ( it's somewhat the main point of the #658 )
Why this isn't easy to fix
to_openssl_cipher returns a Cipher
However, there is nothing under openssl::sym::Ciper called aes_128_wrap and neither aes_128_wrap_pad . This makes the crypto bail above correct at the time it was written, but I raised this issue to make sure it's still valid since now the open ssl library has this module called use openssl::cipher::{Cipher, CipherRef};
This module contains those operations, but it won't return a Cipher like to_openssl_cipher wants, but a CypherRef
Changing the return value of that function breaks a lot of cascading functions, so there is a lot of work to do if we want to take this issue into account