Arne Schwabe has uploaded a new patch set (#2). ( 
https://gerrit.openvpn.net/c/openvpn/+/1950?usp=email )


Change subject: Do not look up the "none" cipher or digest in OpenSSL
......................................................................

Do not look up the "none" cipher or digest in OpenSSL

cipher_get() hands EVP_CIPHER_fetch() whatever name it is given, and the
callers that only ask whether a cipher exists or which mode it has
(cipher_kt_mode_cbc/ofb_cfb/aead(), cipher_kt_block_size(),
cipher_kt_insecure()) treat NULL as "not that". For the "none" cipher
that is the expected answer, but under OpenSSL 3 the failed fetch also
pushes EVP_R_UNSUPPORTED ("digital envelope routines::unsupported,
Algorithm (none : 0)") onto the thread's error queue, and nothing pops
it.

"none" is what every server without --cipher carries in its
pre-negotiation key_type: the legacy BF-CBC default is not in
--data-ciphers, so do_init_crypto_tls() initialises the key_type with
cipher "none". Each new client instance walks it in init_instance() ->
do_init_crypto_tls() -> cipher_kt_mode_ofb_cfb("none") and in the frame
and OCC calculations, and tls_ctx_reload_crl() runs right after. Its
EOF test reads ERR_peek_error(), the OLDEST queued entry, so on the
first handshake after the CRL file changed it finds the stale
"unsupported" error and logs "CRL: cannot read CRL from file" for a CRL
it loaded fine (GitHub #1103). Traced with gdb on 2.7.0 and master
against OpenSSL 3.5.5.

Return NULL for "none" before touching OpenSSL, as cipher_kt_name()
already does. Real cipher names behave as before, and
cipher_valid_reason() still finds the OpenSSL reason on the queue when
it reports an unknown cipher. md_get() gets the same guard: no caller
passes "none" today (md_kt_name(), md_kt_size() and md_defined() check
first), but it has the same shape and would leave the same entry.

Left alone on purpose: cipher_kt_block_size()'s probe for the CBC
sibling of an AEAD cipher (CHACHA20-POLY1305 -> "CHACHA20-CBC") and
md_valid() leave the same kind of entry, but neither runs between
client instance creation and the CRL reload. The next commit makes that
reload robust against any leftover and reports one when it sees it.

With this change the queue is empty at multi_create_instance() and at
backend_tls_ctx_reload_crl() entry for UDP, TCP and CHACHA20-POLY1305
clients; CRL replacements give clean reloads (unpatched: a warning
every time).

Change-Id: I440c4c73865fdbd80f2c607e83c50e345a0e2438
Signed-off-by: Drew Blokzyl <[email protected]>
Signed-off-by: Arne Schwabe <[email protected]>
---
M src/openvpn/crypto_openssl.c
1 file changed, 17 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/50/1950/2

diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 29c5fa6..b44d079 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -568,6 +568,15 @@
 {
     ASSERT(ciphername);

+    /* "none" is a valid OpenVPN cipher name that OpenSSL does not know.
+     * Return NULL without asking OpenSSL: a failed EVP_CIPHER_fetch() would
+     * leave an "unsupported" entry on the error queue that the cipher_kt_*()
+     * callers never clear. */
+    if (strcmp("none", ciphername) == 0)
+    {
+        return NULL;
+    }
+
     ciphername = translate_cipher_name_from_openvpn(ciphername);
     return EVP_CIPHER_fetch(NULL, ciphername, NULL);
 }
@@ -981,6 +990,14 @@
 {
     evp_md_type *md = NULL;
     ASSERT(digest);
+
+    /* "none" is a valid OpenVPN digest name that OpenSSL does not know.
+     * Return NULL without asking OpenSSL, see cipher_get(). */
+    if (strcmp("none", digest) == 0)
+    {
+        return NULL;
+    }
+
     md = EVP_MD_fetch(NULL, digest, NULL);
     if (!md)
     {

--
To view, visit https://gerrit.openvpn.net/c/openvpn/+/1950?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I440c4c73865fdbd80f2c607e83c50e345a0e2438
Gerrit-Change-Number: 1950
Gerrit-PatchSet: 2
Gerrit-Owner: Arne Schwabe <[email protected]>
Gerrit-CC: Răzvan Cojocaru <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to