diff mbox series

[Openvpn-devel,v3] Do not look up the "none" cipher in OpenSSL

Message ID 20261103192732.13656-1-gert@greenie.muc.de
State New
Headers
Series [Openvpn-devel,v3] Do not look up the "none" cipher in OpenSSL |

Commit Message

Gert Doering Nov. 3, 2026, 7:27 p.m. UTC 
  From: Drew Blokzyl <drew@linuxkids.com>

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. Before
commit 15a1465f ("Make CRL reload EOF detection independent of stale
error queue entries") that stale entry made the first handshake after a
CRL change log "CRL: cannot read CRL from file" for a CRL it loaded fine
(GitHub #1103); since then the reload reports it at D_LOW as "CRL:
OpenSSL error queue not empty on CRL load" and clears it.

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.

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.

With this change the queue is empty when the CRL reload runs for UDP
and TCP clients, and CRL replacements give clean reloads with no
"error queue not empty" report (current master: one per replacement).

Change-Id: I440c4c73865fdbd80f2c607e83c50e345a0e2438
Signed-off-by: Drew Blokzyl <drew@linuxkids.com>
Acked-by: Razvan Cojocaru <razvanc@mailbox.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1950
---

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/+/1950
This mail reflects revision 3 of this Change.

Acked-by according to Gerrit (reflected above):
Razvan Cojocaru <razvanc@mailbox.org>

Comments

Gert Doering Nov. 3, 2026, 7:38 p.m. UTC | #1 
Thanks for this.  Took a bit to polish it to what is there now, but
now we have a proper SSL error stack everywhere, as far as I understand :-)

Haven't tested beyond "does it compile" and "does it look nice".

Your patch has been applied to the master and release/2.7 branch (bugfix).

commit d42fbb1f40fdc9ef20dbe524db108e4920e388ac (master)
commit d028b9e243d7503b5de4ad014cce9fdf30533995 (release/2.7)
Author: Drew Blokzyl
Date:   Tue Nov 3 20:27:24 2026 +0100

     Do not look up the none cipher in OpenSSL

     Signed-off-by: Drew Blokzyl <drew@linuxkids.com>
     Acked-by: Razvan Cojocaru <razvanc@mailbox.org>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1950
     Message-Id: <20261103192732.13656-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg40626.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 3c708c2..b7f2ef3 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -569,6 +569,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);
 }