| Message ID | 20211118212945.478-1-lstipakov@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [Openvpn-devel,v2] Load OpenSSL config on Windows from trusted location | expand |
Hi, +1 for setting these env vars. I will test this but some quick comments On Tue, Nov 23, 2021 at 10:08 AM Lev Stipakov <lstipakov@gmail.com> wrote: > From: Lev Stipakov <lev@openvpn.net> > > Commit 7e33127d5 ("contrib/vcpkg-ports: remove openssl port") > disabled OpenSSL config loading to prevent loading config > from untrusted locations. > > Config loading feature might be useful for some users. This > brings it back, and sets OpenSSL enviroment variables > > OPENSSL_CONF, OPENSSL_ENGINES, OPENSSL_MODULES > > which are used to load config, engines and modules, to a trusted location. > The location is built based on installation path, read from registry on > startup. > If installation path cannot be read, Windows\System32 is used as a > fallback. > > While on it, remove unused "bool impersonate_as_system();" declaration. > > Signed-off-by: Lev Stipakov <lev@openvpn.net> > --- > v2: add missing "static" modifier to set_openssl_env_vars() > declaration noted by gcc > > .../vcpkg-triplets/arm64-windows-ovpn.cmake | 2 - > contrib/vcpkg-triplets/x64-windows-ovpn.cmake | 2 - > contrib/vcpkg-triplets/x86-windows-ovpn.cmake | 2 - > src/openvpn/buffer.c | 23 ------ > src/openvpn/win32.c | 76 +++++++++++++++++++ > src/openvpn/win32.h | 8 +- > 6 files changed, 83 insertions(+), 30 deletions(-) > > diff --git a/contrib/vcpkg-triplets/arm64-windows-ovpn.cmake > b/contrib/vcpkg-triplets/arm64-windows-ovpn.cmake > index 89f6a279..dd3c6c0a 100644 > --- a/contrib/vcpkg-triplets/arm64-windows-ovpn.cmake > +++ b/contrib/vcpkg-triplets/arm64-windows-ovpn.cmake > @@ -5,5 +5,3 @@ set(VCPKG_LIBRARY_LINKAGE dynamic) > if(PORT STREQUAL "lz4") > set(VCPKG_LIBRARY_LINKAGE static) > endif() > - > -set(OPENSSL_NO_AUTOLOAD_CONFIG ON) > diff --git a/contrib/vcpkg-triplets/x64-windows-ovpn.cmake > b/contrib/vcpkg-triplets/x64-windows-ovpn.cmake > index d860eed6..7036ed2d 100644 > --- a/contrib/vcpkg-triplets/x64-windows-ovpn.cmake > +++ b/contrib/vcpkg-triplets/x64-windows-ovpn.cmake > @@ -5,5 +5,3 @@ set(VCPKG_LIBRARY_LINKAGE dynamic) > if(PORT STREQUAL "lz4") > set(VCPKG_LIBRARY_LINKAGE static) > endif() > - > -set(OPENSSL_NO_AUTOLOAD_CONFIG ON) > diff --git a/contrib/vcpkg-triplets/x86-windows-ovpn.cmake > b/contrib/vcpkg-triplets/x86-windows-ovpn.cmake > index c1ea6ef3..7d3bf340 100644 > --- a/contrib/vcpkg-triplets/x86-windows-ovpn.cmake > +++ b/contrib/vcpkg-triplets/x86-windows-ovpn.cmake > @@ -5,5 +5,3 @@ set(VCPKG_LIBRARY_LINKAGE dynamic) > if(PORT STREQUAL "lz4") > set(VCPKG_LIBRARY_LINKAGE static) > endif() > - > -set(OPENSSL_NO_AUTOLOAD_CONFIG ON) > diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c > index c82d3d4d..54e758af 100644 > --- a/src/openvpn/buffer.c > +++ b/src/openvpn/buffer.c > @@ -310,29 +310,6 @@ openvpn_snprintf(char *str, size_t size, const char > *format, ...) > return (len >= 0 && len < size); > } > > -/* > - * openvpn_swprintf() is currently only used by Windows code paths > - * and when enabled for all platforms it will currently break older > - * OpenBSD versions lacking vswprintf(3) support in their libc. > - */ > - > -#ifdef _WIN32 > -bool > -openvpn_swprintf(wchar_t *const str, const size_t size, const wchar_t > *const format, ...) > -{ > - va_list arglist; > - int len = -1; > - if (size > 0) > - { > - va_start(arglist, format); > - len = vswprintf(str, size, format, arglist); > - va_end(arglist); > - str[size - 1] = L'\0'; > - } > - return (len >= 0 && len < size); > -} > -#endif > - > /* > * write a string to the end of a buffer that was > * truncated by buf_printf > diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c > index 6cff17b2..ebc7c833 100644 > --- a/src/openvpn/win32.c > +++ b/src/openvpn/win32.c > @@ -101,6 +101,12 @@ struct semaphore netcmd_semaphore; /* GLOBAL */ > */ > static char *win_sys_path = NULL; /* GLOBAL */ > > +/** > + * Set OpenSSL environment variables to a safe directory > + */ > +static void > +set_openssl_env_vars(); > + > void > init_win32(void) > { > @@ -110,6 +116,8 @@ init_win32(void) > } > window_title_clear(&window_title); > win32_signal_clear(&win32_signal); > + > + set_openssl_env_vars(); > } > > void > @@ -1509,4 +1517,72 @@ send_msg_iservice(HANDLE pipe, const void *data, > size_t size, > return ret; > } > > +bool > +openvpn_swprintf(wchar_t* const str, const size_t size, const wchar_t* > const format, ...) > +{ > + va_list arglist; > + int len = -1; > + if (size > 0) > + { > + va_start(arglist, format); > + len = vswprintf(str, size, format, arglist); > + va_end(arglist); > + str[size - 1] = L'\0'; > + } > + return (len >= 0 && len < size); > +} > + > +static BOOL > +get_install_path(WCHAR *path, DWORD size) > +{ > + WCHAR reg_path[256]; > + HKEY key; > + BOOL res = FALSE; > + openvpn_swprintf(reg_path, _countof(reg_path), L"SOFTWARE\\" > PACKAGE_NAME); > Does this string concatenation like L"foo" "bar" work correctly on MSVC? I know it works on mingw, but in the past with the GUI resources we had run into issues with such usage -- iirc, MSVC wanted L"foo" L"bar". > + > + LONG status = RegOpenKeyExW(HKEY_LOCAL_MACHINE, reg_path, 0, > KEY_READ, &key); > + if (status != ERROR_SUCCESS) > + { > + return res; > + } > + > + /* The default value of REG_KEY is the install path */ > + status = RegGetValueW(key, NULL, NULL, RRF_RT_REG_SZ, NULL, > (LPBYTE)path, &size); > + res = status == ERROR_SUCCESS; > + > + RegCloseKey(key); > + > + return res; > +} > + > +static void > +set_openssl_env_vars() > +{ > + const WCHAR* ssl_fallback_dir = L"C:\\Windows\\System32\\"; > + > + WCHAR install_path[MAX_PATH] = { 0 }; > + if (!get_install_path(install_path, _countof(install_path))) > + { > + /* if we cannot find installation path from the registry, > + * use Windows directory as a fallback > + */ > + openvpn_swprintf(install_path, _countof(install_path), L"%ls", > ssl_fallback_dir); > + } > Is this registry value guaranteed to end with the directory separator "\\"? Should we check it? If so, it may be easier to strip it here and add in the format with no ending "\\" in the default above. > + > + WCHAR openssl_cnf[MAX_PATH] = {0}; > + WCHAR openssl_engines[MAX_PATH] = {0}; > + WCHAR openssl_modules[MAX_PATH] = {0}; > On linux one needs strings on the heap or const strings for putenv() as it does not make a copy unlike setenv (with some exceptions). Does _wputenv on Windows behave differently? Do local variables work? > + > + openvpn_swprintf(openssl_cnf, _countof(install_path), > + L"OPENSSL_CONF=%lsssl\\openssl.cnf", install_path); > + openvpn_swprintf(openssl_engines, _countof(openssl_engines), > + L"OPENSSL_ENGINES=%lsssl\\engines", install_path); > + openvpn_swprintf(openssl_modules, _countof(openssl_modules), > + L"OPENSSL_MODULES=%lsssl\\modules", install_path); > + > + _wputenv(openssl_cnf); > + _wputenv(openssl_engines); > + _wputenv(openssl_modules); > I think we should set these only if not already set. Otherwise the user has no way of overriding these locations which is the original purpose of env vars. > +} > + > #endif /* ifdef _WIN32 */ > diff --git a/src/openvpn/win32.h b/src/openvpn/win32.h > index 5d3371a0..4a992d91 100644 > --- a/src/openvpn/win32.h > +++ b/src/openvpn/win32.h > @@ -327,7 +327,13 @@ bool send_msg_iservice(HANDLE pipe, const void *data, > size_t size, > int > openvpn_execve(const struct argv *a, const struct env_set *es, const > unsigned int flags); > > -bool impersonate_as_system(); > +/* > + * openvpn_swprintf() is currently only used by Windows code paths > + * and when enabled for all platforms it will currently break older > + * OpenBSD versions lacking vswprintf(3) support in their libc. > + */ > +bool > +openvpn_swprintf(wchar_t* const str, const size_t size, const wchar_t* > const format, ...); > > #endif /* ifndef OPENVPN_WIN32_H */ > #endif /* ifdef _WIN32 */ > -- Commit message says this brings back config loading but the config loading in crypto_openssl.c stays disabled for Windows. An oversight? Selva <div dir="ltr"><div dir="ltr">Hi,</div><div><br></div>+1 for setting these env vars. I will test this but some quick comments<div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Nov 23, 2021 at 10:08 AM Lev Stipakov <<a href="mailto:lstipakov@gmail.com" target="_blank">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 7e33127d5 ("contrib/vcpkg-ports: remove openssl port")<br> disabled OpenSSL config loading to prevent loading config<br> from untrusted locations.<br> <br> Config loading feature might be useful for some users. This<br> brings it back, and sets OpenSSL enviroment variables<br> <br> OPENSSL_CONF, OPENSSL_ENGINES, OPENSSL_MODULES<br> <br> which are used to load config, engines and modules, to a trusted location.<br> The location is built based on installation path, read from registry on startup.<br> If installation path cannot be read, Windows\System32 is used as a fallback.<br> <br> While on it, remove unused "bool impersonate_as_system();" declaration.<br> <br> Signed-off-by: Lev Stipakov <<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>><br> ---<br> v2: add missing "static" modifier to set_openssl_env_vars()<br> declaration noted by gcc<br> <br> .../vcpkg-triplets/arm64-windows-ovpn.cmake | 2 -<br> contrib/vcpkg-triplets/x64-windows-ovpn.cmake | 2 -<br> contrib/vcpkg-triplets/x86-windows-ovpn.cmake | 2 -<br> src/openvpn/buffer.c | 23 ------<br> src/openvpn/win32.c | 76 +++++++++++++++++++<br> src/openvpn/win32.h | 8 +-<br> 6 files changed, 83 insertions(+), 30 deletions(-)<br> <br> diff --git a/contrib/vcpkg-triplets/arm64-windows-ovpn.cmake b/contrib/vcpkg-triplets/arm64-windows-ovpn.cmake<br> index 89f6a279..dd3c6c0a 100644<br> --- a/contrib/vcpkg-triplets/arm64-windows-ovpn.cmake<br> +++ b/contrib/vcpkg-triplets/arm64-windows-ovpn.cmake<br> @@ -5,5 +5,3 @@ set(VCPKG_LIBRARY_LINKAGE dynamic)<br> if(PORT STREQUAL "lz4")<br> set(VCPKG_LIBRARY_LINKAGE static)<br> endif()<br> -<br> -set(OPENSSL_NO_AUTOLOAD_CONFIG ON)<br> diff --git a/contrib/vcpkg-triplets/x64-windows-ovpn.cmake b/contrib/vcpkg-triplets/x64-windows-ovpn.cmake<br> index d860eed6..7036ed2d 100644<br> --- a/contrib/vcpkg-triplets/x64-windows-ovpn.cmake<br> +++ b/contrib/vcpkg-triplets/x64-windows-ovpn.cmake<br> @@ -5,5 +5,3 @@ set(VCPKG_LIBRARY_LINKAGE dynamic)<br> if(PORT STREQUAL "lz4")<br> set(VCPKG_LIBRARY_LINKAGE static)<br> endif()<br> -<br> -set(OPENSSL_NO_AUTOLOAD_CONFIG ON)<br> diff --git a/contrib/vcpkg-triplets/x86-windows-ovpn.cmake b/contrib/vcpkg-triplets/x86-windows-ovpn.cmake<br> index c1ea6ef3..7d3bf340 100644<br> --- a/contrib/vcpkg-triplets/x86-windows-ovpn.cmake<br> +++ b/contrib/vcpkg-triplets/x86-windows-ovpn.cmake<br> @@ -5,5 +5,3 @@ set(VCPKG_LIBRARY_LINKAGE dynamic)<br> if(PORT STREQUAL "lz4")<br> set(VCPKG_LIBRARY_LINKAGE static)<br> endif()<br> -<br> -set(OPENSSL_NO_AUTOLOAD_CONFIG ON)<br> diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c<br> index c82d3d4d..54e758af 100644<br> --- a/src/openvpn/buffer.c<br> +++ b/src/openvpn/buffer.c<br> @@ -310,29 +310,6 @@ openvpn_snprintf(char *str, size_t size, const char *format, ...)<br> return (len >= 0 && len < size);<br> }<br> <br> -/*<br> - * openvpn_swprintf() is currently only used by Windows code paths<br> - * and when enabled for all platforms it will currently break older<br> - * OpenBSD versions lacking vswprintf(3) support in their libc.<br> - */<br> -<br> -#ifdef _WIN32<br> -bool<br> -openvpn_swprintf(wchar_t *const str, const size_t size, const wchar_t *const format, ...)<br> -{<br> - va_list arglist;<br> - int len = -1;<br> - if (size > 0)<br> - {<br> - va_start(arglist, format);<br> - len = vswprintf(str, size, format, arglist);<br> - va_end(arglist);<br> - str[size - 1] = L'\0';<br> - }<br> - return (len >= 0 && len < size);<br> -}<br> -#endif<br> -<br> /*<br> * write a string to the end of a buffer that was<br> * truncated by buf_printf<br> diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c<br> index 6cff17b2..ebc7c833 100644<br> --- a/src/openvpn/win32.c<br> +++ b/src/openvpn/win32.c<br> @@ -101,6 +101,12 @@ struct semaphore netcmd_semaphore; /* GLOBAL */<br> */<br> static char *win_sys_path = NULL; /* GLOBAL */<br> <br> +/**<br> + * Set OpenSSL environment variables to a safe directory<br> + */<br> +static void<br> +set_openssl_env_vars();<br> +<br> void<br> init_win32(void)<br> {<br> @@ -110,6 +116,8 @@ init_win32(void)<br> }<br> window_title_clear(&window_title);<br> win32_signal_clear(&win32_signal);<br> +<br> + set_openssl_env_vars();<br> }<br> <br> void<br> @@ -1509,4 +1517,72 @@ send_msg_iservice(HANDLE pipe, const void *data, size_t size,<br> return ret;<br> }<br> <br> +bool<br> +openvpn_swprintf(wchar_t* const str, const size_t size, const wchar_t* const format, ...)<br> +{<br> + va_list arglist;<br> + int len = -1;<br> + if (size > 0)<br> + {<br> + va_start(arglist, format);<br> + len = vswprintf(str, size, format, arglist);<br> + va_end(arglist);<br> + str[size - 1] = L'\0';<br> + }<br> + return (len >= 0 && len < size);<br> +}<br> +<br> +static BOOL<br> +get_install_path(WCHAR *path, DWORD size)<br> +{<br> + WCHAR reg_path[256];<br> + HKEY key;<br> + BOOL res = FALSE;<br> + openvpn_swprintf(reg_path, _countof(reg_path), L"SOFTWARE\\" PACKAGE_NAME);<br></blockquote><div><br></div><div>Does this string concatenation like L"foo" "bar" work correctly on MSVC? I know it works on mingw, but in the past with the GUI resources we had run into issues with such usage -- iirc, MSVC wanted L"foo" L"bar".</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> +<br> + LONG status = RegOpenKeyExW(HKEY_LOCAL_MACHINE, reg_path, 0, KEY_READ, &key);<br> + if (status != ERROR_SUCCESS)<br> + {<br> + return res;<br> + }<br> +<br> + /* The default value of REG_KEY is the install path */<br> + status = RegGetValueW(key, NULL, NULL, RRF_RT_REG_SZ, NULL, (LPBYTE)path, &size);<br> + res = status == ERROR_SUCCESS;<br> +<br> + RegCloseKey(key);<br> +<br> + return res;<br> +}<br> +<br> +static void<br> +set_openssl_env_vars()<br> +{<br> + const WCHAR* ssl_fallback_dir = L"C:\\Windows\\System32\\";<br> +<br> + WCHAR install_path[MAX_PATH] = { 0 };<br> + if (!get_install_path(install_path, _countof(install_path)))<br> + {<br> + /* if we cannot find installation path from the registry,<br> + * use Windows directory as a fallback<br> + */<br> + openvpn_swprintf(install_path, _countof(install_path), L"%ls", ssl_fallback_dir);<br> + }<br> </blockquote><div><br></div><div>Is this registry value guaranteed to end with the directory separator "\\"? Should we check it? If so, it may be easier to strip it here and add in the format with no ending "\\" in the default above.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">+<br> + WCHAR openssl_cnf[MAX_PATH] = {0};<br> + WCHAR openssl_engines[MAX_PATH] = {0};<br> + WCHAR openssl_modules[MAX_PATH] = {0};<br></blockquote><div><br></div><div>On linux one needs strings on the heap or const strings for putenv() as it does not make a copy unlike setenv (with some exceptions). Does _wputenv on Windows behave differently? Do local variables work?<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> +<br> + openvpn_swprintf(openssl_cnf, _countof(install_path),<br> + L"OPENSSL_CONF=%lsssl\\openssl.cnf", install_path);<br> + openvpn_swprintf(openssl_engines, _countof(openssl_engines),<br> + L"OPENSSL_ENGINES=%lsssl\\engines", install_path);<br> + openvpn_swprintf(openssl_modules, _countof(openssl_modules),<br> + L"OPENSSL_MODULES=%lsssl\\modules", install_path);<br> +<br> + _wputenv(openssl_cnf);<br> + _wputenv(openssl_engines);<br> + _wputenv(openssl_modules);<br></blockquote><div><br></div><div>I think we should set these only if not already set. Otherwise the user has no way of overriding these locations which is the original purpose of env vars. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> +}<br> +<br> #endif /* ifdef _WIN32 */<br> diff --git a/src/openvpn/win32.h b/src/openvpn/win32.h<br> index 5d3371a0..4a992d91 100644<br> --- a/src/openvpn/win32.h<br> +++ b/src/openvpn/win32.h<br> @@ -327,7 +327,13 @@ bool send_msg_iservice(HANDLE pipe, const void *data, size_t size,<br> int<br> openvpn_execve(const struct argv *a, const struct env_set *es, const unsigned int flags);<br> <br> -bool impersonate_as_system();<br> +/*<br> + * openvpn_swprintf() is currently only used by Windows code paths<br> + * and when enabled for all platforms it will currently break older<br> + * OpenBSD versions lacking vswprintf(3) support in their libc.<br> + */<br> +bool<br> +openvpn_swprintf(wchar_t* const str, const size_t size, const wchar_t* const format, ...);<br> <br> #endif /* ifndef OPENVPN_WIN32_H */<br> #endif /* ifdef _WIN32 */<br> --</blockquote><div><br></div><div>Commit message says this brings back config loading but the config loading in crypto_openssl.c stays disabled for Windows. An oversight?</div><div><br></div><div>Selva</div></div></div></div>
Hi, > Does this string concatenation like L"foo" "bar" work correctly on MSVC? I know it works on mingw, but in the past with the GUI resources we had run into issues with such usage -- iirc, MSVC wanted L"foo" L"bar". I think so, at least result of concatenation looks correct in procmon: > openvpn.exe 23312 CreateFile C:\Program Files\OpenVPN\ssl\openssl.cnf PATH NOT FOUND > Is this registry value guaranteed to end with the directory separator "\\"? Should we check it? If so, it may be easier to strip it here and add in the format with no ending "\\" in the default above. Those are set by our MSI script. But let's be on the safe side. > On linux one needs strings on the heap or const strings for putenv() as it does not make a copy unlike setenv (with some exceptions). Does _wputenv on Windows behave differently? Do local variables work? At least they work in my tests. This https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/putenv-wputenv?view=msvc-170 doesn't mention anything about automatic variables cannot be used, contrary to "man putenv". >> + _wputenv(openssl_cnf); >> + _wputenv(openssl_engines); >> + _wputenv(openssl_modules); > > > I think we should set these only if not already set. Otherwise the user has no way of overriding these locations which is the original purpose of env vars. Will do. > Commit message says this brings back config loading but the config loading in crypto_openssl.c stays disabled for Windows. An oversight? Good catch. Interesting that it works for me even with that code in crypto_openssl.c being disabled. It looks like our SSL context initialization does the right thing: > libcrypto-1_1-x64.dll!OPENSSL_init_crypto(unsigned __int64 opts, const ossl_init_settings_st * settings) Line 699 C libssl-1_1-x64.dll!OPENSSL_init_ssl(unsigned __int64 opts, const ossl_init_settings_st * settings) Line 205 C libssl-1_1-x64.dll!SSL_CTX_new(const ssl_method_st * meth) Line 3023 C openvpn.exe!tls_ctx_client_new(tls_root_ctx * ctx) Line 134 C openvpn.exe!init_ssl(const options * options, tls_root_ctx * new_ctx, bool in_chroot) Line 622 C openvpn.exe!do_init_crypto_tls_c1(context * c) Line 2741 C openvpn.exe!do_init_crypto_tls(context * c, const unsigned int flags) Line 2821 C openvpn.exe!do_init_crypto(context * c, const unsigned int flags) Line 3084 C openvpn.exe!init_instance(context * c, const env_set * env, const unsigned int flags) Line 4311 C openvpn.exe!init_instance_handle_signals(context * c, const env_set * env, const unsigned int flags) Line 4124 C openvpn.exe!tunnel_point_to_point(context * c) Line 68 C and opts got the correct bitflag: #ifndef OPENSSL_NO_AUTOLOAD_CONFIG if ((opts & OPENSSL_INIT_NO_LOAD_CONFIG) == 0) opts |= OPENSSL_INIT_LOAD_CONFIG; #endif Does that code in crypto_openssl.c cover some unusual use case? Anyway I can bring it back.
diff --git a/contrib/vcpkg-triplets/arm64-windows-ovpn.cmake b/contrib/vcpkg-triplets/arm64-windows-ovpn.cmake index 89f6a279..dd3c6c0a 100644 --- a/contrib/vcpkg-triplets/arm64-windows-ovpn.cmake +++ b/contrib/vcpkg-triplets/arm64-windows-ovpn.cmake @@ -5,5 +5,3 @@ set(VCPKG_LIBRARY_LINKAGE dynamic) if(PORT STREQUAL "lz4") set(VCPKG_LIBRARY_LINKAGE static) endif() - -set(OPENSSL_NO_AUTOLOAD_CONFIG ON) diff --git a/contrib/vcpkg-triplets/x64-windows-ovpn.cmake b/contrib/vcpkg-triplets/x64-windows-ovpn.cmake index d860eed6..7036ed2d 100644 --- a/contrib/vcpkg-triplets/x64-windows-ovpn.cmake +++ b/contrib/vcpkg-triplets/x64-windows-ovpn.cmake @@ -5,5 +5,3 @@ set(VCPKG_LIBRARY_LINKAGE dynamic) if(PORT STREQUAL "lz4") set(VCPKG_LIBRARY_LINKAGE static) endif() - -set(OPENSSL_NO_AUTOLOAD_CONFIG ON) diff --git a/contrib/vcpkg-triplets/x86-windows-ovpn.cmake b/contrib/vcpkg-triplets/x86-windows-ovpn.cmake index c1ea6ef3..7d3bf340 100644 --- a/contrib/vcpkg-triplets/x86-windows-ovpn.cmake +++ b/contrib/vcpkg-triplets/x86-windows-ovpn.cmake @@ -5,5 +5,3 @@ set(VCPKG_LIBRARY_LINKAGE dynamic) if(PORT STREQUAL "lz4") set(VCPKG_LIBRARY_LINKAGE static) endif() - -set(OPENSSL_NO_AUTOLOAD_CONFIG ON) diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c index c82d3d4d..54e758af 100644 --- a/src/openvpn/buffer.c +++ b/src/openvpn/buffer.c @@ -310,29 +310,6 @@ openvpn_snprintf(char *str, size_t size, const char *format, ...) return (len >= 0 && len < size); } -/* - * openvpn_swprintf() is currently only used by Windows code paths - * and when enabled for all platforms it will currently break older - * OpenBSD versions lacking vswprintf(3) support in their libc. - */ - -#ifdef _WIN32 -bool -openvpn_swprintf(wchar_t *const str, const size_t size, const wchar_t *const format, ...) -{ - va_list arglist; - int len = -1; - if (size > 0) - { - va_start(arglist, format); - len = vswprintf(str, size, format, arglist); - va_end(arglist); - str[size - 1] = L'\0'; - } - return (len >= 0 && len < size); -} -#endif - /* * write a string to the end of a buffer that was * truncated by buf_printf diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c index 6cff17b2..ebc7c833 100644 --- a/src/openvpn/win32.c +++ b/src/openvpn/win32.c @@ -101,6 +101,12 @@ struct semaphore netcmd_semaphore; /* GLOBAL */ */ static char *win_sys_path = NULL; /* GLOBAL */ +/** + * Set OpenSSL environment variables to a safe directory + */ +static void +set_openssl_env_vars(); + void init_win32(void) { @@ -110,6 +116,8 @@ init_win32(void) } window_title_clear(&window_title); win32_signal_clear(&win32_signal); + + set_openssl_env_vars(); } void @@ -1509,4 +1517,72 @@ send_msg_iservice(HANDLE pipe, const void *data, size_t size, return ret; } +bool +openvpn_swprintf(wchar_t* const str, const size_t size, const wchar_t* const format, ...) +{ + va_list arglist; + int len = -1; + if (size > 0) + { + va_start(arglist, format); + len = vswprintf(str, size, format, arglist); + va_end(arglist); + str[size - 1] = L'\0'; + } + return (len >= 0 && len < size); +} + +static BOOL +get_install_path(WCHAR *path, DWORD size) +{ + WCHAR reg_path[256]; + HKEY key; + BOOL res = FALSE; + openvpn_swprintf(reg_path, _countof(reg_path), L"SOFTWARE\\" PACKAGE_NAME); + + LONG status = RegOpenKeyExW(HKEY_LOCAL_MACHINE, reg_path, 0, KEY_READ, &key); + if (status != ERROR_SUCCESS) + { + return res; + } + + /* The default value of REG_KEY is the install path */ + status = RegGetValueW(key, NULL, NULL, RRF_RT_REG_SZ, NULL, (LPBYTE)path, &size); + res = status == ERROR_SUCCESS; + + RegCloseKey(key); + + return res; +} + +static void +set_openssl_env_vars() +{ + const WCHAR* ssl_fallback_dir = L"C:\\Windows\\System32\\"; + + WCHAR install_path[MAX_PATH] = { 0 }; + if (!get_install_path(install_path, _countof(install_path))) + { + /* if we cannot find installation path from the registry, + * use Windows directory as a fallback + */ + openvpn_swprintf(install_path, _countof(install_path), L"%ls", ssl_fallback_dir); + } + + WCHAR openssl_cnf[MAX_PATH] = {0}; + WCHAR openssl_engines[MAX_PATH] = {0}; + WCHAR openssl_modules[MAX_PATH] = {0}; + + openvpn_swprintf(openssl_cnf, _countof(install_path), + L"OPENSSL_CONF=%lsssl\\openssl.cnf", install_path); + openvpn_swprintf(openssl_engines, _countof(openssl_engines), + L"OPENSSL_ENGINES=%lsssl\\engines", install_path); + openvpn_swprintf(openssl_modules, _countof(openssl_modules), + L"OPENSSL_MODULES=%lsssl\\modules", install_path); + + _wputenv(openssl_cnf); + _wputenv(openssl_engines); + _wputenv(openssl_modules); +} + #endif /* ifdef _WIN32 */ diff --git a/src/openvpn/win32.h b/src/openvpn/win32.h index 5d3371a0..4a992d91 100644 --- a/src/openvpn/win32.h +++ b/src/openvpn/win32.h @@ -327,7 +327,13 @@ bool send_msg_iservice(HANDLE pipe, const void *data, size_t size, int openvpn_execve(const struct argv *a, const struct env_set *es, const unsigned int flags); -bool impersonate_as_system(); +/* + * openvpn_swprintf() is currently only used by Windows code paths + * and when enabled for all platforms it will currently break older + * OpenBSD versions lacking vswprintf(3) support in their libc. + */ +bool +openvpn_swprintf(wchar_t* const str, const size_t size, const wchar_t* const format, ...); #endif /* ifndef OPENVPN_WIN32_H */ #endif /* ifdef _WIN32 */