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.
If the queue is not empty when the CRL is loaded, say so at D_LOW with the queued errors, since that is a bug somewhere else worth seeing, then 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 <[email protected]> --- src/openvpn/ssl_openssl.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index 7cfe9f4a..b14cbbe9 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -1360,6 +1360,19 @@ backend_tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, const char *crl_file, b } int num_crls_loaded = 0; + /* + * The EOF test below must only see errors raised by + * PEM_read_bio_X509_CRL(). Anything already queued was left by an + * earlier operation in this thread and would otherwise be what + * ERR_peek_error() returns, turning a clean EOF into "cannot read CRL". + * Report it, since that is a bug elsewhere, then start from an empty + * queue (crypto_msg() drains the queue while printing it). + */ + if (ERR_peek_error() != 0) + { + crypto_msg(D_LOW, "CRL: OpenSSL error queue not empty on CRL load"); + } + ERR_clear_error(); while (true) { X509_CRL *crl = PEM_read_bio_X509_CRL(in, NULL, NULL, NULL); @@ -1367,13 +1380,15 @@ backend_tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, const char *crl_file, b { /* * PEM_R_NO_START_LINE can be considered equivalent to EOF. + * ERR_peek_last_error() is the error PEM_read_bio_X509_CRL() + * raised last; ERR_peek_error() would be the oldest queued one. */ - bool eof = ERR_GET_REASON(ERR_peek_error()) == PEM_R_NO_START_LINE; + bool eof = ERR_GET_REASON(ERR_peek_last_error()) == PEM_R_NO_START_LINE; /* but warn if no CRLs have been loaded */ if (num_crls_loaded > 0 && eof) { /* remove that error from error stack */ - (void)ERR_get_error(); + ERR_clear_error(); break; } -- 2.53.0 _______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
