| Message ID | 20210415093454.18324-1-maximilian.fillinger@foxcrypto.com |
|---|---|
| State | Accepted |
| Delegated to: | Antonio Quartulli |
| Headers | show |
| Series | None | expand |
Hi, On 15/04/2021 11:34, Max Fillinger wrote: > Now that the path for the CRL file is handled correctly when using > chroot, there's no good reason for the file to be inaccessible during > ssl_init(). > > This commit ensures that the CRL file is accessed successfully at least > once, which fixes a bug where the mbedtls version of OpenVPN wouldn't > use a reloaded CRL if it initially failed to access the file. > > Signed-off-by: Max Fillinger <maximilian.fillinger@foxcrypto.com> It simply does what it says: when calling init_ssl() upon instance initialization (i.e. upon openvpn startup, and upon USR1 on clients) if the CRL is not accessible, then openvpn will abort. This behaviour is what we need because "starting an instance without having the CRL where it is expected" is equivalent to a configuration error. Being unable to re-load the CRL at runtime is instead acceptable, because some subsystem might be just recreating the CRL, therefore openvpn will skip the reload and continue using what is currently in memory. The CRL will be reloaded at the next occasion. Acked-by: Antonio Quartulli <antonio@openvpn.net>
The actual code change looks very magical to me, and shrunk dramatically
between v1 and v3 of that code :-) - but if Antonio has tested this
and ACKs it, who am I to complain. I have compile-tested it, at least.
Your patch has been applied to the master and release/2.5 branch (bugfix).
commit 940619c88067d95a1c9865795624bc3822a89bd7 (master)
commit 8a06459d0c969b083377b63be16e5da1245ce0ee (release/2.5)
Author: Max Fillinger
Date: Thu Apr 15 11:34:54 2021 +0200
Abort if CRL file can't be stat-ed in ssl_init
Signed-off-by: Max Fillinger <maximilian.fillinger@foxcrypto.com>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210415093454.18324-1-maximilian.fillinger@foxcrypto.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22118.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 1e0e6170..6ce1d079 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -559,7 +559,15 @@ tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, const char *crl_file, } else if (platform_stat(crl_file, &crl_stat) < 0) { - msg(M_WARN, "WARNING: Failed to stat CRL file, not (re)loading CRL."); + /* If crl_last_mtime is zero, the CRL file has not been read before. */ + if (ssl_ctx->crl_last_mtime == 0) + { + msg(M_FATAL, "ERROR: Failed to stat CRL file during initialization, exiting."); + } + else + { + msg(M_WARN, "WARNING: Failed to stat CRL file, not reloading CRL."); + } return; }
Now that the path for the CRL file is handled correctly when using chroot, there's no good reason for the file to be inaccessible during ssl_init(). This commit ensures that the CRL file is accessed successfully at least once, which fixes a bug where the mbedtls version of OpenVPN wouldn't use a reloaded CRL if it initially failed to access the file. Signed-off-by: Max Fillinger <maximilian.fillinger@foxcrypto.com> --- src/openvpn/ssl.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)