Message ID | 20210617061226.244-1-lstipakov@gmail.com |
---|---|
State | Accepted |
Headers | show |
Series | Disable OpenSSL config autoload in Windows | expand |
Acked-by: Gert Doering <gert@greenie.muc.de> As discussed in the meeting yesterday - this might break people's use of OpenSSL engines on windows, but we assume that the amount of users of that will be "close to zero". For these cases we can find a better approach after 2.5.3 release. Your patch has been applied to the master and release/2.5 branch. commit abd5ee9b7dc4ba85438da5d16bb7dfb31714dac7 (master) commit 447cfb4f30fd96126f7d2945cd14ef39cc13a08a (release/2.5) Author: Lev Stipakov Date: Thu Jun 17 09:12:26 2021 +0300 crypto_openssl.c: disable explicit initialization on Windows (CVE-2121-3606) Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de> Message-Id: <20210617061226.244-1-lstipakov@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22568.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
Hi On Thu, Jun 17, 2021 at 2:13 AM Lev Stipakov <lstipakov@gmail.com> wrote: > From: Lev Stipakov <lev@openvpn.net> > > Commit a4071b ("crypto_openssl: add initialization to pick up local > configuration") > added openssl initialization to load configuration file. However on Windows > this file is loaded from user-writable directory, such as c.\etc\ssl for > mingw builds > and (for example) c:\vcpkg\packages\openssl_x64-windows\openvpn.cnf for > vcpkg > builds. This could be a security risk. > > Since aforementioned commit implements a niche feature which anyway > should use CryptoAPI on Windows, make this code conditional. > > Signed-off-by: Lev Stipakov <lev@openvpn.net> > --- > src/openvpn/crypto_openssl.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c > index c571030b..603c67b0 100644 > --- a/src/openvpn/crypto_openssl.c > +++ b/src/openvpn/crypto_openssl.c > @@ -154,11 +154,13 @@ crypto_init_lib_engine(const char *engine_name) > void > crypto_init_lib(void) > { > +#ifndef _WIN32 > #if (OPENSSL_VERSION_NUMBER >= 0x10100000L) > OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CONFIG, NULL); > #else > OPENSSL_config(NULL); > #endif > +#endif /* _WIN32 */ > /* > * If you build the OpenSSL library and OpenVPN with > * CRYPTO_MDEBUG, you will get a listing of OpenSSL > -- > 2.23.0.windows.1 > This would make it impossible to use openssl.cnf on Windows, wouldn't it? I use configuraton file to restrict signature algorithms, for example. There are other uses like configuring engines. Instead of disabling, why not make the default path a restricted location within, say, C:\Windows. The user can then override it using env variables. What path does OpenSSL Windows binaries described in OpensSL wiki use? ( https://wiki.openssl.org/index.php/Binaries) Selva <div dir="ltr"><div>Hi</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jun 17, 2021 at 2:13 AM Lev Stipakov <<a href="mailto:lstipakov@gmail.com">lstipakov@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">From: Lev Stipakov <<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>><br> <br> Commit a4071b ("crypto_openssl: add initialization to pick up local configuration")<br> added openssl initialization to load configuration file. However on Windows<br> this file is loaded from user-writable directory, such as c.\etc\ssl for mingw builds<br> and (for example) c:\vcpkg\packages\openssl_x64-windows\openvpn.cnf for vcpkg<br> builds. This could be a security risk.<br> <br> Since aforementioned commit implements a niche feature which anyway<br> should use CryptoAPI on Windows, make this code conditional.<br> <br> Signed-off-by: Lev Stipakov <<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>><br> ---<br> src/openvpn/crypto_openssl.c | 2 ++<br> 1 file changed, 2 insertions(+)<br> <br> diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c<br> index c571030b..603c67b0 100644<br> --- a/src/openvpn/crypto_openssl.c<br> +++ b/src/openvpn/crypto_openssl.c<br> @@ -154,11 +154,13 @@ crypto_init_lib_engine(const char *engine_name)<br> void<br> crypto_init_lib(void)<br> {<br> +#ifndef _WIN32<br> #if (OPENSSL_VERSION_NUMBER >= 0x10100000L)<br> OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CONFIG, NULL);<br> #else<br> OPENSSL_config(NULL);<br> #endif<br> +#endif /* _WIN32 */<br> /*<br> * If you build the OpenSSL library and OpenVPN with<br> * CRYPTO_MDEBUG, you will get a listing of OpenSSL<br> -- <br> 2.23.0.windows.1<br></blockquote><div><br></div><div>This would make it impossible to use openssl.cnf on Windows, wouldn't it? I use configuraton file to restrict signature algorithms, for example. There are other uses like configuring engines.</div><div><br></div><div>Instead of disabling, why not make the default path a restricted location within, say, C:\Windows. The user can then override it using env variables.</div><div><br></div><div>What path does OpenSSL Windows binaries described in OpensSL wiki use? (<a href="https://wiki.openssl.org/index.php/Binaries">https://wiki.openssl.org/index.php/Binaries</a>)</div><div><br></div><div>Selva</div></div></div>
Hi, On Thu, Jun 17, 2021 at 11:25:02AM -0400, Selva Nair wrote: > This would make it impossible to use openssl.cnf on Windows, wouldn't it? I > use configuraton file to restrict signature algorithms, for example. There > are other uses like configuring engines. Right. > Instead of disabling, why not make the default path a restricted location > within, say, C:\Windows. The user can then override it using env variables. This is what we tried to achieve, but Lev could not find a way to make the MSVC build behave wrt "new ETCDIR". I have not personally investigated. So we decided to release 2.5.3 soonish, with "disable openssl.cnf" to cover the problem reported, and then see if we can get to a better fix incrementally (while a "secured" binary is out for users to run). gert
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c index c571030b..603c67b0 100644 --- a/src/openvpn/crypto_openssl.c +++ b/src/openvpn/crypto_openssl.c @@ -154,11 +154,13 @@ crypto_init_lib_engine(const char *engine_name) void crypto_init_lib(void) { +#ifndef _WIN32 #if (OPENSSL_VERSION_NUMBER >= 0x10100000L) OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CONFIG, NULL); #else OPENSSL_config(NULL); #endif +#endif /* _WIN32 */ /* * If you build the OpenSSL library and OpenVPN with * CRYPTO_MDEBUG, you will get a listing of OpenSSL