diff mbox series

[Openvpn-devel,v3,1/2] Do not look up the "none" cipher or digest in OpenSSL

Message ID 20260930132707.51452-2-drew@linuxkids.com
State New
Headers
Series Stop failed cipher/digest lookups from polluting the OpenSSL error queue |

Commit Message

Drew Blokzyl Sept. 30, 2026, 1:27 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. 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).

Signed-off-by: Drew Blokzyl <drew@linuxkids.com>
---
 src/openvpn/crypto_openssl.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
diff mbox series

Patch

diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 29c5fa68..b44d0797 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);
 }
@@ -981,6 +990,14 @@  md_get(const char *digest)
 {
     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)
     {