Always set IV length for AES CCM ciphers #2245
Conversation
|
Yeah it unfortunately looks like we can't just unconditionally do this :( |
No it looks like it ain't that easy 😞 Based on the OpenSSL wiki on AES with CCM, I still think that a change is warranted to make 12 byte IVs work so I'm gonna self review another proposal! |
openssl/src/symm.rs
Outdated
| if let (Some(iv), Some(_iv_len)) = (iv, t.iv_len()) { | ||
| ctx.set_iv_length(iv.len())?; |
There was a problem hiding this comment.
What do we think about this instead? I reintroduced the length check and added specific handling for the AES CCM ciphers, which are the ones with the IV length issue.
| if let (Some(iv), Some(_iv_len)) = (iv, t.iv_len()) { | |
| ctx.set_iv_length(iv.len())?; | |
| if let (Some(iv), Some(iv_len)) = (iv, t.iv_len()) { | |
| if iv.len() != iv_len | |
| || matches!( | |
| t.nid(), | |
| Nid::AES_128_CCM | Nid::AES_192_CCM | Nid::AES_256_CCM | |
| ) | |
| { | |
| ctx.set_iv_length(iv.len())?; | |
| } |
There was a problem hiding this comment.
I think I hate it, but that's just OpenSSL I guess :P
There was a problem hiding this comment.
Looks like boringssl's not happy with it though.
There was a problem hiding this comment.
I waited for you to give your opinion before pushing and running all of the CI. Now I pushed in 20ce889!
There was a problem hiding this comment.
Also, agree that it's not very pretty but it should (hopefully!) get the job done!
This fixes an issue where the IV length would not be set if the length was equal to the recommended length. The issue shows up at least when an IV of length 12 (which is returned by `t.iv_len()`) is used with the AES256 CCM cipher, as OpenSSL defaults the IV length to 7 bytes [^1] and it would not be correctly set to 12. [^1]: https://wiki.openssl.org/index.php/EVP_Authenticated_Encryption_and_Decryption Closes sfackler#2244.
lwestlund
force-pushed
the
fix/always-set-cipher-iv-len
branch
from
20ce889
to
542b783
Compare
lwestlund
changed the title
|
@sfackler please have another look at this! |
This fixes an issue where the IV length would not be set if the length
was equal to the recommended length. The issue shows up at least when an
IV of length 12 (which is returned by
t.iv_len()) is used with theAES256 CCM cipher, as OpenSSL defaults the IV length to 7 bytes 1 and it
would not be correctly set to 12.
Closes #2244.
Footnotes
https://wiki.openssl.org/index.php/EVP_Authenticated_Encryption_and_Decryption ↩