[Openvpn-devel,v3,2/2] Make CRL reload EOF detection independent of stale error queue entries
Commit Message
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 <drew@linuxkids.com>
---
src/openvpn/ssl_openssl.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
@@ -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;
}