OpenSSL: remove EVP_CIPHER_CTX_new() from the compat layer

PATCH In review Emmanuel Deloget 3 participants 7 messages Jun 29, 2017

Share & tools

Emmanuel Deloget

author

2017-06-29 14:21 UTC #0 lore raw

PATCH 1/2 OpenSSL: remove EVP_CIPHER_CTX_new() from the compat layer

For unknown reason, the write of the compat layer seemed to think that this function was only present in OpenSSL 1.1. This is not the case at all, since it has been introduced in OpenSSL before version 0.9.8.

Thus, there is no need to add this function to the compat layer, and it can be safely removed.

Signed-off-by Emmanuel Deloget <[email protected]...>

---
 configure.ac                 |  1 -
 src/openvpn/openssl_compat.h | 15 ---------------
 2 files changed, 16 deletions(-)

configure.ac +0 -1

index 22f91cb6..cb121795 100644
@@ -919,7 +919,6 @@ if test "${enable_crypto}" = "yes" -a "${with_crypto_library}" = "openssl"; then
 
 	AC_CHECK_FUNCS(
 		[ \
-			EVP_CIPHER_CTX_new \
 			EVP_CIPHER_CTX_free \
 			HMAC_CTX_new \
 			HMAC_CTX_free \

src/openvpn/openssl_compat.h +0 ~~-15~~

index 617410e0..cd25bd37 100644
@@ -101,21 +101,6 @@ EVP_CIPHER_CTX_free(EVP_CIPHER_CTX *c)
 }
 #endif
 
-#if !defined(HAVE_EVP_CIPHER_CTX_NEW)
-/**
- * Allocate a new cipher context object
- *
- * @return                    A zero'ed cipher context object
- */
-static inline EVP_CIPHER_CTX *
-EVP_CIPHER_CTX_new(void)
-{
-    EVP_CIPHER_CTX *ctx = NULL;
-    ALLOC_OBJ_CLEAR(ctx, EVP_CIPHER_CTX);
-    return ctx;
-}
-#endif
-
 #if !defined(HAVE_HMAC_CTX_RESET)
 /**
  * Reset a HMAC context

Signature · 1 line

2.11.0

Emmanuel Deloget

author

2017-06-29 14:21 UTC #1 lore raw

PATCH 2/2 OpenSSL: remove EVP_CIPHER_CTX_free() from the compat layer

Thus, there is no need to add this function to the compat layer, and it can be safely removed.

Signed-off-by Emmanuel Deloget <[email protected]...>

---
 configure.ac                 |  1 -
 src/openvpn/openssl_compat.h | 13 -------------
 2 files changed, 14 deletions(-)

configure.ac +0 -1

index cb121795..60bb4658 100644
@@ -919,7 +919,6 @@ if test "${enable_crypto}" = "yes" -a "${with_crypto_library}" = "openssl"; then
 
 	AC_CHECK_FUNCS(
 		[ \
-			EVP_CIPHER_CTX_free \
 			HMAC_CTX_new \
 			HMAC_CTX_free \
 			HMAC_CTX_reset \

src/openvpn/openssl_compat.h +0 ~~-13~~

index cd25bd37..36f68b01 100644
@@ -88,19 +88,6 @@ EVP_MD_CTX_new(void)
 }
 #endif
 
-#if !defined(HAVE_EVP_CIPHER_CTX_FREE)
-/**
- * Free an existing cipher context
- *
- * @param ctx                 The cipher context
- */
-static inline void
-EVP_CIPHER_CTX_free(EVP_CIPHER_CTX *c)
-{
-	free(c);
-}
-#endif
-
 #if !defined(HAVE_HMAC_CTX_RESET)
 /**
  * Reset a HMAC context

Signature · 1 line

2.11.0

Steffan Karger

2017-06-29 17:34 UTC #2 lore raw

Re: [Openvpn-devel] [PATCH 2/2] OpenSSL: remove EVP_CIPHER_CTX_free() from the compat layer

Hi,

Quoted 40 lines · Emmanuel Deloget

For unknown reason, the write of the compat layer seemed to think that this function was only present in OpenSSL 1.1. This is not the case at all, since it has been introduced in OpenSSL before version 0.9.8.
Thus, there is no need to add this function to the compat layer, and it can be safely removed.
Signed-off-by Emmanuel Deloget <[email protected]...>
---
 configure.ac                 |  1 -
 src/openvpn/openssl_compat.h | 13 -------------
 2 files changed, 14 deletions(-)
configure.ac +0 -1
index cb121795..60bb4658 100644
@@ -919,7 +919,6 @@ if test "${enable_crypto}" = "yes" -a "${with_crypto_library}" = "openssl"; then
 
 	AC_CHECK_FUNCS(
 		[ \
-			EVP_CIPHER_CTX_free \
 			HMAC_CTX_new \
 			HMAC_CTX_free \
 			HMAC_CTX_reset \
src/openvpn/openssl_compat.h +0 -6
index cd25bd37..36f68b01 100644
@@ -88,19 +88,6 @@ EVP_MD_CTX_new(void)
 }
 #endif
 
-#if !defined(HAVE_EVP_CIPHER_CTX_FREE)
-/**
- * Free an existing cipher context
- *
- * @param ctx                 The cipher context
- */

src/openvpn/openssl_compat.h EVP_MD_CTX_new(void) · line 93

Quoted 10 lines

src/openvpn/openssl_compat.h +0 -7

-static inline void
-EVP_CIPHER_CTX_free(EVP_CIPHER_CTX *c)
-{
-	free(c);
-}
-#endif
-
 #if !defined(HAVE_HMAC_CTX_RESET)
 /**
  * Reset a HMAC context

For some reason, the reviewer seemed to think this author was right about that. He should crearly have looked a bit better...

ACK

-Steffan

Gert Doering

2017-06-29 18:47 UTC #3 lore raw

[Openvpn-devel] [PATCH applied] Re: OpenSSL: remove EVP_CIPHER_CTX_free() from the compat layer

In reply to Emmanuel Deloget

Your patch has been applied to the master and release/2.4 branch.

commit 7ee9a94fcbbde941bfed167229a64df0f7cdae0b (master) commit ebb5c70bf6acede8d941687ae0e40d827003fb2e (release/2.4)

Author: Emmanuel Deloget
Date:   Thu Jun 29 16:21:19 2017 +0200

     OpenSSL: remove EVP_CIPHER_CTX_free() from the compat layer

     Signed-off-by: Emmanuel Deloget <[email protected]...>
     Acked-by: Steffan Karger <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/search?l=mid&[email protected]...
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

Emmanuel Deloget

author

2017-06-29 14:29 UTC #4 lore raw

In reply to Emmanuel Deloget

Hi,

For the two patches in this series, I failed to correctly write "writer", which I spelled "write", sorry for that. And sorry to said writer for the gratuitous attack (but then, I'm me, and I'm pretty sure I'll be able to handle that). And I forgot to mention that the EVP_CIPHER_CTX_free() function is broken (good thing it has never been used) since it leaks memory.

For the record, I found this (kinda stupid?) mistake while I was writing yet another shim to openssl 1.1. I started a discussion on the openssh mailing list and it seems that supporting a shim in openssh is the least prefered idea for a few people, right after "not supporting openssl 1.1 at all". The idea would then to have an independant shim that would cover the needs for various openssl-based projects. I'm doing my tests on openvpn since it's the easy way (for me).

So: sorry for the annoying noise :)

Best regards,

-- Emmanuel Deloget

Steffan Karger

2017-06-29 17:35 UTC #5 lore raw

In reply to Emmanuel Deloget

Hi,

Quoted 42 lines · Emmanuel Deloget

For unknown reason, the write of the compat layer seemed to think that this function was only present in OpenSSL 1.1. This is not the case at all, since it has been introduced in OpenSSL before version 0.9.8.
Thus, there is no need to add this function to the compat layer, and it can be safely removed.
Signed-off-by Emmanuel Deloget <[email protected]...>
---
 configure.ac                 |  1 -
 src/openvpn/openssl_compat.h | 15 ---------------
 2 files changed, 16 deletions(-)
configure.ac +0 -1
index 22f91cb6..cb121795 100644
@@ -919,7 +919,6 @@ if test "${enable_crypto}" = "yes" -a "${with_crypto_library}" = "openssl"; then
 
 	AC_CHECK_FUNCS(
 		[ \
-			EVP_CIPHER_CTX_new \
 			EVP_CIPHER_CTX_free \
 			HMAC_CTX_new \
 			HMAC_CTX_free \
src/openvpn/openssl_compat.h +0 -8
index 617410e0..cd25bd37 100644
@@ -101,21 +101,6 @@ EVP_CIPHER_CTX_free(EVP_CIPHER_CTX *c)
 }
 #endif
 
-#if !defined(HAVE_EVP_CIPHER_CTX_NEW)
-/**
- * Allocate a new cipher context object
- *
- * @return                    A zero'ed cipher context object
- */
-static inline EVP_CIPHER_CTX *
-EVP_CIPHER_CTX_new(void)

src/openvpn/openssl_compat.h EVP_CIPHER_CTX_free(EVP_CIPHER_CTX *c) · line 106

Quoted 10 lines

src/openvpn/openssl_compat.h +0 -7

-{
-    EVP_CIPHER_CTX *ctx = NULL;
-    ALLOC_OBJ_CLEAR(ctx, EVP_CIPHER_CTX);
-    return ctx;
-}
-#endif
-
 #if !defined(HAVE_HMAC_CTX_RESET)
 /**
  * Reset a HMAC context

For some reason, the reviewer seemed to think this author was right about that. He should crearly have looked a bit better...

ACK

-Steffan

Gert Doering

2017-06-29 18:47 UTC #6 lore raw

In reply to Emmanuel Deloget

Your patch has been applied to the master and release/2.4 branch.

commit a72d21a56a0223b8a50d05d88af64abcda0fc5dc (master) commit 0f9575852bc981e75884d1a75b2e7c1f9af091f4 (release/2.4)

Author: Emmanuel Deloget
Date:   Thu Jun 29 16:21:18 2017 +0200

     OpenSSL: remove EVP_CIPHER_CTX_new() from the compat layer

     Signed-off-by: Emmanuel Deloget <[email protected]...>
     Acked-by: Steffan Karger <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg14989.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering