diff mbox series

[Openvpn-devel,v3] Remove/combine redundant call of EVP_CipherInit before EVP_CipherInit_Ex

Message ID 20240402134909.6340-1-gert@greenie.muc.de
State Accepted
Headers show
Series [Openvpn-devel,v3] Remove/combine redundant call of EVP_CipherInit before EVP_CipherInit_Ex | expand

Commit Message

Gert Doering April 2, 2024, 1:49 p.m. UTC 
From: Arne Schwabe <arne@rfc2549.org>

EVP_CipherInit basically is the same EVP_CipherInit_ex except that it
in some instances it resets/inits the ctx parameter first. We already
call EVP_CIPHER_CTX_reset to reset/init the ctx before. Also ensure that
EVP_CipherInit_Ex gets the cipher to actually be able to initialise the
context.

OpenSSL 1.0.2:

https://github.com/openssl/openssl/blob/OpenSSL_1_0_2-stable/crypto/evp/evp_enc.c#L94

EVP_CipherInit calls first EVP_CIPHER_CTX_init and then EVP_CipherInit_ex

Our openssl_compat.h has

for these older OpenSSL versions

OpenSSL 3.0:

https://github.com/openssl/openssl/blob/openssl-3.2/crypto/evp/evp_enc.c#L450

basically the same as 1.0.2. Just that method names have been changed.

Change-Id: I911e25949a8647b567fd4178683534d4404ab469
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/552
This mail reflects revision 3 of this Change.

Acked-by according to Gerrit (reflected above):
Gert Doering <gert@greenie.muc.de>

Comments

Gert Doering April 2, 2024, 2:56 p.m. UTC | #1 
Looking at the OpenSSL implementation for the EVP_CipherInit*() functions
makes it really clear that this is, basically, dead code.  Of course the
"kt" init needs to go to the second call now, because otherwise it will
bomb with OpenSSL 3.x ("no default cipher").

Tested with various OpenSSL (and LibreSSL) versions across the buildbot
and GHA test bed.

Your patch has been applied to the master branch.

(Not applied to release/2.6 since it's not actually fixing something,
just cleaning up old code - typical "master" material)

commit e81e3eb1a4322148b06f353eaa22b0a803fd74f4
Author: Arne Schwabe
Date:   Tue Apr 2 15:49:09 2024 +0200

     Remove/combine redundant call of EVP_CipherInit before EVP_CipherInit_Ex

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20240402134909.6340-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28523.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index bfc5e37..b2c4eb6 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -846,11 +846,7 @@ 
     evp_cipher_type *kt = cipher_get(ciphername);
 
     EVP_CIPHER_CTX_reset(ctx);
-    if (!EVP_CipherInit(ctx, kt, NULL, NULL, enc))
-    {
-        crypto_msg(M_FATAL, "EVP cipher init #1");
-    }
-    if (!EVP_CipherInit_ex(ctx, NULL, NULL, key, NULL, enc))
+    if (!EVP_CipherInit_ex(ctx, kt, NULL, key, NULL, enc))
     {
         crypto_msg(M_FATAL, "EVP cipher init #2");
     }