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)
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

index 7c25b59..3eef53c 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: evp_enc.c,v 1.80 2023/12/26 08:39:28 tb Exp $ */
+/* $OpenBSD: evp_enc.c,v 1.81 2023/12/26 09:04:30 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -72,8 +72,6 @@ int
 EVP_CipherInit(EVP_CIPHER_CTX *ctx, const EVP_CIPHER *cipher,
     const unsigned char *key, const unsigned char *iv, int enc)
 {
-       if (cipher != NULL)
-               EVP_CIPHER_CTX_cleanup(ctx);
        return EVP_CipherInit_ex(ctx, cipher, NULL, key, iv, enc);
 }