[Openvpn-devel,v1] Workaround issue in LibreSSL crashing when enumerating digests/ciphers

Message ID 20240508220540.12554-1-gert@greenie.muc.de
State Accepted
Headers show
Series [Openvpn-devel,v1] Workaround issue in LibreSSL crashing when enumerating digests/ciphers | expand

Commit Message

Gert Doering May 8, 2024, 10:05 p.m. UTC
From: Arne Schwabe <arne@rfc2549.org>

OpenBSD/LibreSSL reimplemented EVP_get_cipherbyname/EVP_get_digestbyname
and broke calling EVP_get_cipherbynid/EVP_get_digestbyname with an
invalid nid in the process so that it would segfault.

Workaround but doing that NULL check in OpenVPN instead of leaving it
to the library.

Change-Id: Ia08a9697d0ff41721fb0acf17ccb4cfa23cb3934
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/+/586
This mail reflects revision 1 of this Change.

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

Comments

Gert Doering May 13, 2024, 3:23 p.m. UTC | #1
This is one of many not-so-good approaches to the problem, but 
LibreSSL upstream says "can not backport to OpenBSD 7.5" and since we
just got a new and shiny OpenBSD 7.5 buildbot, everything fails without
this patch... we can remove it from master as soon as OpenBSD 7.6 is
there (+ the buildbot upgraded).

Tested on FreeBSD with OpenSSL 1.1 & 3.0, and OpenBSD 7.5, of course :)

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

commit b3a271b11723cbe520ad4ce6b4b0459de57ade06 (master)
commit 8aed156be81a3bdd3098bfed5e8f95662d06633c (release/2.6)
Author: Arne Schwabe
Date:   Thu May 9 00:05:40 2024 +0200

     Workaround issue in LibreSSL crashing when enumerating digests/ciphers

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


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 61c6518..1649ab7 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -387,7 +387,19 @@ 
 #else
     for (int nid = 0; nid < 10000; ++nid)
     {
+#if defined(LIBRESSL_VERSION_NUMBER)
+        /* OpenBSD/LibreSSL reimplemented EVP_get_cipherbyname and broke
+         * calling EVP_get_cipherbynid with an invalid nid in the process
+         * so that it would segfault. */
+        const EVP_CIPHER *cipher = NULL;
+        const char *name = OBJ_nid2sn(nid);
+        if (name)
+        {
+            cipher = EVP_get_cipherbyname(name);
+        }
+#else  /* if defined(LIBRESSL_VERSION_NUMBER) */
         const EVP_CIPHER *cipher = EVP_get_cipherbynid(nid);
+#endif
         /* We cast the const away so we can keep the function prototype
          * compatible with EVP_CIPHER_do_all_provided */
         collect_ciphers((EVP_CIPHER *) cipher, &cipher_list);
@@ -441,7 +453,19 @@ 
 #else
     for (int nid = 0; nid < 10000; ++nid)
     {
+        /* OpenBSD/LibreSSL reimplemented EVP_get_digestbyname and broke
+         * calling EVP_get_digestbynid with an invalid nid in the process
+         * so that it would segfault. */
+#ifdef LIBRESSL_VERSION_NUMBER
+        const EVP_MD *digest = NULL;
+        const char *name = OBJ_nid2sn(nid);
+        if (name)
+        {
+            digest = EVP_get_digestbyname(name);
+        }
+#else  /* ifdef LIBRESSL_VERSION_NUMBER */
         const EVP_MD *digest = EVP_get_digestbynid(nid);
+#endif
         if (digest)
         {
             /* We cast the const away so we can keep the function prototype
@@ -449,7 +473,7 @@ 
             print_digest((EVP_MD *)digest, NULL);
         }
     }
-#endif
+#endif /* if OPENSSL_VERSION_NUMBER >= 0x30000000L */
     printf("\n");
 }