EVP_CipherInit(): remove cleanup call
authortb <tb@openbsd.org>
Tue, 26 Dec 2023 09:04:30 +0000 (09:04 +0000)
committertb <tb@openbsd.org>
Tue, 26 Dec 2023 09:04:30 +0000 (09:04 +0000)
commitfa679d4f7b6530cdb111c9b11479de3ab1da10cc
tree35ac97487363135c68e5a7832ca68b9e480dfb22
parent522874ac1bcf47333cf371da2055e3bdbd8dbd14
EVP_CipherInit(): remove cleanup call

There is a bizarre EVP_CIPHER_CTX_cleanup() call in EVP_CipherInit()
leading to a subtle behavior difference with EVP_CipherInit_ex().

The history is that before EVP_CIPHER_CTX was made opaque, a context would
often live on the stack (hello, MariaDB) and the EVP_CIPHER_CTX_cleanup()
call was in fact an EVP_CIPHER_CTX_init() which just zeroes out the struct.
The problem with doing this is that on context reuse there could be data
hanging off it, causing leaks. Attempts were made to clean up things in
EVP_CipherFinal*(), but that broke applications reaching into the context
afterward, so they were removed again. Later on, opacity allowed changing
the _init() to a _cleanup() since EVP_CIPHER_CTX could no longer live on
the stack, so it would no longer contain garbage. I have to correct myself:
it would no longer contain stack garbage.

Now: EVP_CipherInit_ex() does some extra dances to preserve the AES key
wrap flag, which is cleared unconditionally in EVP_CipherInit(). That's
annoying to document and very likely never going to be an issue in the
wild: you'd need to do key wrap and then use the same context for use
with a cipher that does not allow key wrap for this to make a difference.

This way, all our EVP_{Cipher,Decrypt,Encrypt}*_ex() functions are now
trivially wrapped by their non-_ex() versions.

ok jsing
lib/libcrypto/evp/evp_enc.c