Conversation
Author
|
Patch sent to openvpn-devel on 2026-09-10 as |
Member
|
AFAICT that patch did not make it to the list. |
cipher_get() looks a cipher up with EVP_CIPHER_fetch() and hands the
result, possibly NULL, to callers that only care whether it exists:
cipher_valid_reason(), cipher_kt_mode_cbc/ofb_cfb/aead(),
cipher_kt_block_size(), cipher_kt_insecure(). A failed fetch is a normal
outcome for them, but under OpenSSL 3 it also pushes an
EVP_R_UNSUPPORTED error ("digital envelope routines::unsupported,
Algorithm (none : 0)") onto the thread's error queue, and nobody pops it.
The common way to get there is not exotic. A server that does not set
--cipher gets the legacy default BF-CBC, which is not in --data-ciphers,
so do_init_crypto_tls() initialises the pre-negotiation key_type with
cipher "none". Every new client instance then runs init_instance() ->
do_init_crypto_tls() -> cipher_kt_mode_ofb_cfb("none"), and the frame
and OCC calculations (calculate_crypto_overhead(), frame_calculate_*())
walk the same key_type, each fetching "none" and failing.
cipher_kt_block_size() adds a second case for AEAD ciphers whose CBC
sibling does not exist (CHACHA20-POLY1305 -> "CHACHA20-CBC").
md_valid() has the same shape for digests.
The stale entry then misleads code that classifies an unrelated failure
with ERR_peek_error(), which returns the OLDEST queued entry. The visible
symptom is backend_tls_ctx_reload_crl() logging "CRL: cannot read CRL
from file" on the first handshake after the CRL file changes although
the CRL loaded fine (GitHub OpenVPN#1103). Traced with gdb on OpenVPN 2.7.0 and
master with OpenSSL 3.5.5: the single entry on the queue at reload entry
is the cipher_kt_mode_ofb_cfb("none") fetch from do_init_crypto_tls()
of that same client instance.
Bracket the probing fetches with ERR_set_mark()/ERR_pop_to_mark() so a
failed lookup leaves the queue as it found it; the return value already
carries the answer these callers want. wolfSSL's compatibility layer has
no error marks, so openssl_compat.h maps them to ERR_clear_error() there.
With this change the error queue is empty at multi_create_instance() and
at backend_tls_ctx_reload_crl() entry for UDP, TCP and CHACHA20-POLY1305
clients, and the spurious warning is gone: three CRL replacements, three
handshakes, zero warnings (unpatched: three of three).
Signed-off-by: Drew Blokzyl <drew@linuxkids.com>
backend_tls_ctx_reload_crl() treats a NULL from PEM_read_bio_X509_CRL() as EOF when ERR_peek_error() shows PEM_R_NO_START_LINE. ERR_peek_error() returns the OLDEST queued error, so any entry left behind earlier in the thread turns a clean EOF into a "CRL: cannot read CRL from file" warning, prints the unrelated errors as if they came from the CRL file, and still installs the CRLs already parsed. The previous commit removes the leftover that triggered this in practice; this one stops the loop from depending on the queue being clean at all. Start the loop from an empty queue so only errors raised by PEM_read_bio_X509_CRL() are visible, test the error it raised last rather than the oldest one, and clear the queue on the EOF path instead of popping a single entry. Signed-off-by: Drew Blokzyl <drew@linuxkids.com>
linuxkd
force-pushed
the
crl-reload-error-queue
branch
from
September 22, 2026 14:05
47ddf1f to
d0a54d3
Compare
linuxkd
changed the title
Author
|
Right, the list rejected the first send (non-member). Subscribed now; the reworked two-patch series went out today as |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root-cause follow-up to #1103. Review copy of the series sent to openvpn-devel on 2026-09-22 as
[PATCH 0/2] Stop failed cipher/digest lookups from polluting the OpenSSL error queue(Message-ID<20260922140520.71500-1-drew@linuxkids.com>).The stale entry that misled the CRL reload is left by
cipher_get()being asked for the ciphernone: the pre-negotiation key_type every server without--ciphergets (BF-CBC default, not in--data-ciphers), walked bydo_init_crypto_tls()and the frame/OCC helpers for every new client instance. Details and the gdb trace in #1103.cipher_get(),cipher_kt_block_size()andmd_valid()withERR_set_mark()/ERR_pop_to_mark(), with a fallback toERR_clear_error()for wolfSSL inopenssl_compat.h.PEM_read_bio_X509_CRL()raised last, starting from an empty queue.Validated on aarch64 Ubuntu 26.04 (OpenSSL 3.5.5, DCO) with UDP, TCP and CHACHA20-POLY1305 clients: queue empty at
multi_create_instance()and atbackend_tls_ctx_reload_crl()entry, three CRL replacements give three clean reloads (unpatched: three warnings), a garbage CRL still fails withloaded 0 CRLs/VERIFY ERROR: CRL not loaded. Not compile-tested against wolfSSL.