diff mbox series

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

Message ID 20261029184205.76422-1-drew@linuxkids.com
State New
Headers
Series [Openvpn-devel,v4] Do not look up the "none" cipher in OpenSSL |

Commit Message

Drew Blokzyl Oct. 29, 2026, 6:42 p.m. UTC 
  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>
---
v4: drop the md_get() guard (Razvan, Gerrit 1950 PS2). No caller can
    reach md_get() with "none", so it protected nothing and only changed
    how md_get() reports a failure. Arne asked for it in v3; I agree with
    Razvan that it does not belong here. Subject and commit message
    amended to match. Patch 2/2 of v3 is in master as 15a1465f, so the
    message now refers to it instead of "the next commit".

    Retested on master 54a66f35 + this patch, OpenSSL 3, UDP and TCP
    clients, CRL replaced before each connect: master logs the stale
    "unsupported" entry and "CRL: OpenSSL error queue not empty on CRL
    load" on every reload; with this patch only "CRL: loaded 1 CRLs".
    make check: 23 pass, 0 fail.
 src/openvpn/crypto_openssl.c | 9 +++++++++
 1 file changed, 9 insertions(+)
diff mbox series

Patch

diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index e6abd8f7..6ba9526b 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -568,6 +568,15 @@  cipher_get(const char *ciphername)
 {
     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);
 }