[Openvpn-devel] OVPN Interactive Service for non-admin users
Hi! I am developing an eduVPN client for Windows. Imagine the eduVPN client as a custom OpenVPN GUI. The client uses openvpn.exe for connecting, the configuration file is provided by eduVPN server once user authenticates using OAuth. User running the eduVPN client is not an administrator. Elevation is out of the question. I would like to use the Interactive Service to start openvpn.exe, but I have some problems: 1. The configuration file is dynamically downloaded by the eduVPN client and stored somewhere user can write (user's temporary folder for example). But the Interactive Service was specifically programmed to allow configurations from "C:\Program Files\OpenVPN\config" folder only. But user running eduVPN client can't write to this folder. 2. Interactive Service can launch openvpn.exe using any configuration file if user is a member of the "OpenVPN Administrators" group. Then, I would need to add all users of the computer to that group, again requiring elevation. Is there any specific reason, why Interactive Service is so paranoid, knowing that it launches openvpn.exe and all external scripts as the interactive user anyway? I have a work-around for this paradox in my sleeve: the eduVPN setup shall create an "eduVPN" subfolder in the "C:\Program Files\OpenVPN\config" folder, and grant all users desirable permissions*: a sort of public spool folder. But that would open the OpenVPN Interactive Service to any user and application. This is why we would like your opinion first. Best regards, Simon Rozman Amebis, d. o. o., Kamnik * By desirable permissions I mean: SYSTEM/Administrators = full access, Users = create new files, CREATOR OWNER = R/W) smime.p7s Description: S/MIME cryptographic signature -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] OVPN Interactive Service for non-admin users
Hi Selva, Is there any specific reason, why Interactive Service is so paranoid, knowing that it launches openvpn.exe and all external scripts as the interactive user anyway? The service does privileged operations so some admin has to bless a user to allow certain options when launching openvpn.exe. In other words, options allowed in user editable configs are restricted unless the user is in a designated group. I don't quite agree. OpenVPN needs elevation to set up connection because it runs in user space. IPsec VPN doesn't require elevation for the very same task since it runs in kernel space. Therefore, elevation for OpenVPN is required for technical reasons, not security. Thus, an explicit blessing from the admin is an exaggeration. I have a work-around for this paradox in my sleeve: the eduVPN setup shall create an "eduVPN" subfolder in the "C:\Program Files\OpenVPN\config" folder, and grant all users desirable permissions*: a sort of public spool folder. Setting up such a folder requires admin rights. If your installer has admin rights, just add all users to "OpenVPN Administrators" group or set the registry key ovpn_admin_group to "Users" The installer will require admin rights of course. Here we agree installing software (VPN especially) needs an admin approval. Thank you for your excellent advice. I haven't thought of that before. However, I will not follow it for the following reason… eduVPN will not claim OpenVPN for all by itself. It will install it when missing, but will leave everything to its defaults. We would still like to leave the user an option to make use of OpenVPN for other purposes. Tweaking registry is not a step in this direction. But that would open the OpenVPN Interactive Service to any user and application. This is why we would like your opinion first. Yes the service will then launch openvpn with arbitrary configs as any user, but that is what you want isn't it? True, I want that indeed. I was just trying to find the official way of doing it only to learn it's against OpenVPN team's principles. :( Well, I'll do it anyway. And I suggest you take it as a compliment: the OpenVPN is great for its flexibility so people can and will use it in a million of bizarre ways. :) Best regards, Simon smime.p7s Description: S/MIME cryptographic signature -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] OVPN Interactive Service for non-admin users
Hi, > > Wasn't this changed in the latest version, allowing config files to be under > user home/profile directory? > Nope, 2.4.3 refuses to run the openvpn.exe if --config points to an .ovpn file in the user home directory (namely user's temporary folder). I also did a brief openvpnserv source code audit not to find anything supporting it. If you add that option, that would void entire Interactive Service "security" scheme, wouldn't it? But that's what I wanted in the first place, as I believe Interactive Service "security" scheme makes no sense. Why does OpenVPN restrict non-admin users from using Interactive Service in the first place, while Windows' out-of-the-box VPN connects them just fine? If you are afraid a malware would start connecting - they already can: using Windows' VPN. Flushing ARP cache, client DNS registration, and other tasks OpenVPN can't perform as non-admin user is a technical issue of OpenVPN running in user space. Not a security one. Interactive Service overcomes that, but in the same time it assumes it's a security sensitive issue. This limitation can and will be turned off with one or another simple administrator task (performed by eduVPN setup). So, this is no biggie... Just me ranting. :) Best regards, Simon smime.p7s Description: S/MIME cryptographic signature -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] tap.c: fix adapter renaming
Hi,
> +// stripped version of ExecCommand in interactive.c static DWORD
C++ style comment.
> +// rename adapter via netsh call
C++ style comment.
> +const TCHAR* szFmt = _T("netsh interface set interface name=\"%s\"
> newname=\"%s\"");
> +size_t ncmdline = _tcslen(szFmt) + _tcslen(szOldName) +
> _tcslen(szName) + 1;
> +WCHAR* szCmdLine = malloc(ncmdline * sizeof(TCHAR));
> +_stprintf_s(szCmdLine, ncmdline, szFmt, szOldName, szName);
For the record:
1. `netsh interface set interface` does not accept adapter index. Therefore,
the interface to rename must be selected by name. I'd prefer more explicit
selection like adapter GUID or interface index, but selecting by name seems the
only way here. Interface indexes are a thing of the TCP/IP, so it kind of makes
sense lower layers are not operating with them. Ack.
2. I've tested `netsh interface set interface` to ignore case when selecting
adapter. Ack.
3. I've tested `netsh interface set interface` to work when renaming adapter
back to the original name. Ack.
Reviewed the code, compiled, debugged, tested.
Acked-by: Simon Rozman
Regards,
Simon
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] openvpnmsica: remove adapter renaming
Hi, Adapter name already is configurable in openvpn-build/windows-msi/msi.wxs: https://github.com/Amebis/openvpn-build/blob/5f5ba807de2bad50d01a5b08dfc6fad98ee41213/windows-msi/msi.wxs#L1300 Regards, Simon From: Selva Nair Sent: Wednesday, September 2, 2020 3:12 PM To: Lev Stipakov Cc: Lev Stipakov ; openvpn-devel Subject: Re: [Openvpn-devel] [PATCH] openvpnmsica: remove adapter renaming Hi, I would suggest to keep this renaming but make it not fatal. A descriptive name is nice to have and we could even make the name configurable at some point in future. Selva ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v2] openvpnmsica: make adapter renaming non-fatal
Hi,
> -Original Message-
> From: Lev Stipakov
> Sent: Wednesday, September 2, 2020 11:37 PM
> To: openvpn-devel@lists.sourceforge.net
> Cc: Lev Stipakov
> Subject: [Openvpn-devel] [PATCH v2] openvpnmsica: make adapter renaming
> non-fatal
>
> From: Lev Stipakov
>
> For some users renaming adapter
>
> Local Area Connection > OpenVPN TAP-Windows6
>
> mysteriously fails, see https://github.com/OpenVPN/openvpn-
> build/issues/187
>
> Since renaming is just a "nice to have", make it non-fatal
> and, in case of error, only log message and don't display messagebox.
>
> Signed-off-by: Lev Stipakov
> ---
>
> v2: only log error, don't display messagebox
>
> src/openvpnmsica/dllmain.c | 2 +-
> src/openvpnmsica/openvpnmsica.c | 9 +++--
> src/tapctl/main.c | 2 +-
> src/tapctl/tap.c| 11 +++
> src/tapctl/tap.h| 6 +-
> 5 files changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/src/openvpnmsica/dllmain.c b/src/openvpnmsica/dllmain.c
> index 201fd9af..34946ed8 100644
> --- a/src/openvpnmsica/dllmain.c
> +++ b/src/openvpnmsica/dllmain.c
> @@ -193,6 +193,6 @@ x_msg_va(const unsigned int flags, const char
> *format, va_list arglist)
> }
> }
>
> -MsiProcessMessage(s->hInstall, INSTALLMESSAGE_ERROR, hRecordProg);
> +MsiProcessMessage(s->hInstall, (flags & M_WARN) ?
> INSTALLMESSAGE_INFO : INSTALLMESSAGE_ERROR, hRecordProg);
> MsiCloseHandle(hRecordProg);
> }
> diff --git a/src/openvpnmsica/openvpnmsica.c
> b/src/openvpnmsica/openvpnmsica.c
> index 31e90bd2..f203f736 100644
> --- a/src/openvpnmsica/openvpnmsica.c
> +++ b/src/openvpnmsica/openvpnmsica.c
> @@ -1096,12 +1096,9 @@ ProcessDeferredAction(_In_ MSIHANDLE hInstall)
> dwResult = tap_create_adapter(NULL, NULL, szHardwareId,
> &bRebootRequired, &guidAdapter);
> if (dwResult == ERROR_SUCCESS)
> {
> -/* Set adapter name. */
> -dwResult = tap_set_adapter_name(&guidAdapter, szName);
> -if (dwResult != ERROR_SUCCESS)
> -{
> -tap_delete_adapter(NULL, &guidAdapter,
> &bRebootRequired);
> -}
> +/* Set adapter name. May fail on some machines, but
> that is not critical - use silent
> + flag to mute messagebox and print error only to log
> */
> +tap_set_adapter_name(&guidAdapter, szName, TRUE);
> }
> }
> else if (wcsncmp(szArg[i], L"deleteN=", 8) == 0)
> diff --git a/src/tapctl/main.c b/src/tapctl/main.c
> index 31bb2ec7..d5bc7290 100644
> --- a/src/tapctl/main.c
> +++ b/src/tapctl/main.c
> @@ -237,7 +237,7 @@ _tmain(int argc, LPCTSTR argv[])
> }
>
> /* Rename the adapter. */
> -dwResult = tap_set_adapter_name(&guidAdapter, szName);
> +dwResult = tap_set_adapter_name(&guidAdapter, szName,
> FALSE);
> if (dwResult != ERROR_SUCCESS)
> {
> StringFromIID((REFIID)&guidAdapter, &szAdapterId);
> diff --git a/src/tapctl/tap.c b/src/tapctl/tap.c
> index 7cb3dedc..0dfe239f 100644
> --- a/src/tapctl/tap.c
> +++ b/src/tapctl/tap.c
> @@ -1140,9 +1140,12 @@ ExecCommand(const WCHAR* cmdline)
> DWORD
> tap_set_adapter_name(
> _In_ LPCGUID pguidAdapter,
> -_In_ LPCTSTR szName)
> +_In_ LPCTSTR szName,
> +_In_ BOOL bSilent)
> {
> DWORD dwResult;
> +int msg_flag = bSilent ? M_WARN : M_NONFATAL;
> +msg_flag |= M_ERRNO;
>
> if (pguidAdapter == NULL || szName == NULL)
> {
> @@ -1176,7 +1179,7 @@ tap_set_adapter_name(
> if (dwResult != ERROR_SUCCESS)
> {
> SetLastError(dwResult); /* MSDN does not mention RegOpenKeyEx()
> to set GetLastError(). But we do have an error code. Set last error
> manually. */
> -msg(M_NONFATAL | M_ERRNO, "%s: RegOpenKeyEx(HKLM, \"%"
> PRIsLPTSTR "\") failed", __FUNCTION__, szRegKey);
> +msg(msg_flag, "%s: RegOpenKeyEx(HKLM, \"%" PRIsLPTSTR "\")
> failed", __FUNCTION__, szRegKey);
> goto cleanup_szAdapterId;
> }
>
> @@ -1185,7 +1188,7 @@ tap_set_adapter_name(
> if (dwResult != ERROR_SUCCESS)
> {
> SetLastError(dwResult);
> -msg(M_NONFATAL | M_ERRNO, "%s: Error reading adapter name",
> __FUNCTION__);
> +msg(msg_flag, "%s: Error reading adapter name", __FUNCTION__);
> goto cleanup_hKey;
> }
>
> @@ -1203,7 +1206,7 @@ tap_set_adapter_name(
> if (dwResult != ERROR_SUCCESS)
> {
> SetLastError(dwResult);
> -msg(M_NONFATAL | M_ERRNO, "%s: Error renaming adapter",
> __FUNCTION__);
> +msg(msg_flag, "%s: Error renaming adapter", __FUNCTION__);
> goto cleanup_hKey;
> }
>
> diff --git a/src/tapctl/tap.h b/src/tapctl/tap.h
> index 102de32d..1f531cf2 100644
> --- a/src/tapctl/tap.h
> +++ b/src/tapctl/tap.h
> @@ -117,13 +117,17 @@ tap_enable_adapter(
> * @par
[Openvpn-devel] [PATCH 3/3] netsh: Delete WINS servers on TUN close
Signed-off-by: Simon Rozman
---
src/openvpn/tun.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index b1cd7a1b..80ae6958 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -6706,6 +6706,16 @@ netsh_delete_address_dns(const struct tuntap *tt, bool
ipv6, struct gc_arena *gc
netsh_command(&argv, 1, M_WARN);
}
+if (!ipv6 && tt->options.wins_len > 0)
+{
+argv_printf(&argv,
+"%s%s interface ipv4 delete winsservers %lu all",
+get_win_sys_path(),
+NETSH_PATH_SUFFIX,
+tt->adapter_index);
+netsh_command(&argv, 1, M_WARN);
+}
+
if (ipv6 && tt->type == DEV_TYPE_TUN)
{
delete_route_connected_v6_net(tt);
--
2.28.0.windows.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 1/3] netsh: Specify interfaces by index rather than name
This is more efficient and less error prone.
Signed-off-by: Simon Rozman
---
src/openvpn/route.c | 26 +++---
src/openvpn/tun.c | 88 +
2 files changed, 53 insertions(+), 61 deletions(-)
diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index bd6b968b..d75aa5f4 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -1987,25 +1987,24 @@ add_route_ipv6(struct route_ipv6 *r6, const struct
tuntap *tt,
}
else
{
-struct buffer out = alloc_buf_gc(64, &gc);
+DWORD adapter_index;
if (r6->adapter_index) /* vpn server special route */
{
-buf_printf(&out, "interface=%lu", r6->adapter_index );
+adapter_index = r6->adapter_index;
gateway_needed = true;
}
else
{
-buf_printf(&out, "interface=%lu", tt->adapter_index );
+adapter_index = tt->adapter_index;
}
-device = buf_bptr(&out);
-/* netsh interface ipv6 add route 2001:db8::/32 MyTunDevice */
-argv_printf(&argv, "%s%s interface ipv6 add route %s/%d %s",
+/* netsh interface ipv6 add route 2001:db8::/32 42 */
+argv_printf(&argv, "%s%s interface ipv6 add route %s/%d %lu",
get_win_sys_path(),
NETSH_PATH_SUFFIX,
network,
r6->netbits,
-device);
+adapter_index);
/* next-hop depends on TUN or TAP mode:
* - in TAP mode, we use the "real" next-hop
@@ -2431,25 +2430,24 @@ delete_route_ipv6(const struct route_ipv6 *r6, const
struct tuntap *tt,
}
else
{
-struct buffer out = alloc_buf_gc(64, &gc);
+DWORD adapter_index;
if (r6->adapter_index) /* vpn server special route */
{
-buf_printf(&out, "interface=%lu", r6->adapter_index );
+adapter_index = r6->adapter_index;
gateway_needed = true;
}
else
{
-buf_printf(&out, "interface=%lu", tt->adapter_index );
+adapter_index = tt->adapter_index;
}
-device = buf_bptr(&out);
-/* netsh interface ipv6 delete route 2001:db8::/32 MyTunDevice */
-argv_printf(&argv, "%s%s interface ipv6 delete route %s/%d %s",
+/* netsh interface ipv6 delete route 2001:db8::/32 42 */
+argv_printf(&argv, "%s%s interface ipv6 delete route %s/%d %lu",
get_win_sys_path(),
NETSH_PATH_SUFFIX,
network,
r6->netbits,
-device);
+adapter_index);
/* next-hop depends on TUN or TAP mode:
* - in TAP mode, we use the "real" next-hop
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index faa02504..8fd3229f 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -68,7 +68,7 @@ const static GUID GUID_DEVINTERFACE_NET = { 0xcac88484,
0x7515, 0x4c03, { 0x82,
#define NI_OPTIONS (1<<2)
static void netsh_ifconfig(const struct tuntap_options *to,
- const char *flex_name,
+ DWORD adapter_index,
const in_addr_t ip,
const in_addr_t netmask,
const unsigned int flags);
@@ -79,7 +79,7 @@ static void windows_set_mtu(const int iface_index,
static void netsh_set_dns6_servers(const struct in6_addr *addr_list,
const int addr_len,
- const char *flex_name);
+ DWORD adapter_index);
static void netsh_command(const struct argv *a, int n, int msglevel);
@@ -1103,10 +1103,9 @@ do_ifconfig_ipv6(struct tuntap *tt, const char *ifname,
int tun_mtu,
}
else
{
-/* example: netsh interface ipv6 set address interface=42
+/* example: netsh interface ipv6 set address 42
* 2001:608:8003::d/bits store=active
*/
-char iface[64];
/* in TUN mode, we only simulate a subnet, so the interface
* is configured with /128 + a route to fe80::8. In TAP mode,
@@ -1114,10 +1113,8 @@ do_ifconfig_ipv6(struct tuntap *tt, const char *ifname,
int tun_mtu,
*/
int netbits = (tt->type == DEV_TYPE_TUN) ? 128 : tt->netbits_ipv6;
-openvpn_snprintf(iface, sizeof(iface), "interface=%lu",
- tt->adapter_index);
-argv_printf(&argv, "%s%s interface ipv6 set address %s %s/%d
store=active",
-get_win_sys_path(), NETSH_PATH_SUFFIX, iface,
+argv_printf(&argv, "%s%s interface ipv6 set address %lu %s/%d
store=active",
+get_win_sys_path(), NETSH_PATH_SUFFIX, tt->adapter_index,
ifconfig_ipv6_local, netbits);
netsh_command(&argv,
[Openvpn-devel] [PATCH] openvpnmsica: Simplify find_adapters() to void return
As the find_adapters() failure is not critical and FindSystemInfo()
should continue regardless, the find_adapters() has been simplified not
to return result code. It still logs any error thou.
Signed-off-by: Simon Rozman
---
src/openvpnmsica/openvpnmsica.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/openvpnmsica/openvpnmsica.c b/src/openvpnmsica/openvpnmsica.c
index f203f736..de1cf65c 100644
--- a/src/openvpnmsica/openvpnmsica.c
+++ b/src/openvpnmsica/openvpnmsica.c
@@ -248,7 +248,7 @@ cleanup_OpenSCManager:
}
-static UINT
+static void
find_adapters(
_In_ MSIHANDLE hInstall,
_In_z_ LPCTSTR szzHardwareIDs,
@@ -262,12 +262,12 @@ find_adapters(
uiResult = tap_list_adapters(NULL, szzHardwareIDs, &pAdapterList);
if (uiResult != ERROR_SUCCESS)
{
-return uiResult;
+return;
}
else if (pAdapterList == NULL)
{
/* No adapters - no fun. */
-return ERROR_SUCCESS;
+return;
}
/* Get IPv4/v6 info for all network adapters. Actually, we're interested
in link status only: up/down? */
@@ -394,7 +394,6 @@ cleanup_pAdapterAdresses:
free(pAdapterAdresses);
cleanup_pAdapterList:
tap_free_adapter_list(pAdapterList);
-return uiResult;
}
--
2.28.0.windows.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 2/3] netsh: Clear existing IPv6 DNS servers before configuring new ones
When there are no IPv6 DNS published, the adapter state is not
sanitized and might contain IPv6 DNS server from a previous session.
netsh_ifconfig_options() clears DNS servers for IPv4 already.
Signed-off-by: Simon Rozman
---
src/openvpn/tun.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 8fd3229f..b1cd7a1b 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -5281,7 +5281,6 @@ ip_addr_member_of(const in_addr_t addr, const
IP_ADDR_STRING *ias)
* Set the ipv6 dns servers on the specified interface.
* The list of dns servers currently set on the interface
* are cleared first.
- * No action is taken if number of addresses (addr_len) < 1.
*/
static void
netsh_set_dns6_servers(const struct in6_addr *addr_list,
@@ -5291,6 +5290,13 @@ netsh_set_dns6_servers(const struct in6_addr *addr_list,
struct gc_arena gc = gc_new();
struct argv argv = argv_new();
+/* delete existing DNS settings from TAP interface */
+argv_printf(&argv, "%s%s interface ipv6 delete dns %lu all",
+get_win_sys_path(),
+NETSH_PATH_SUFFIX,
+adapter_index);
+netsh_command(&argv, 2, M_FATAL);
+
for (int i = 0; i < addr_len; ++i)
{
const char *fmt = (i == 0) ?
--
2.28.0.windows.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] openvpnmsica: Skip legacy TAP-Windows6 adapters from evaluation
Legacy TAP-Windows6 adapters (marked as IF_TYPE_ETHERNET_CSMACD 0x6)
fail to upgrade to the new driver on Windows 7: Device cannot start
(Code 10).
Ignoring those adapters on Windows 7 triggers creation of a new TAP
adapter on setup eliminating the need for user intervention.
Signed-off-by: Simon Rozman
---
src/openvpnmsica/openvpnmsica.c | 13
src/tapctl/tap.c| 37 +
src/tapctl/tap.h| 3 +++
3 files changed, 49 insertions(+), 4 deletions(-)
diff --git a/src/openvpnmsica/openvpnmsica.c b/src/openvpnmsica/openvpnmsica.c
index f203f736..dd6ecd74 100644
--- a/src/openvpnmsica/openvpnmsica.c
+++ b/src/openvpnmsica/openvpnmsica.c
@@ -303,10 +303,18 @@ find_adapters(
}
}
+OSVERSIONINFOEX osvi = { sizeof(OSVERSIONINFOEX),
HIBYTE(_WIN32_WINNT_WIN8), LOBYTE(_WIN32_WINNT_WIN8) };
+DWORDLONG const dwlConditionMask =
VerSetConditionMask(VerSetConditionMask(0, VER_MAJORVERSION,
VER_GREATER_EQUAL), VER_MINORVERSION, VER_GREATER_EQUAL);
+BOOL bSkipLegacyAdapters = !VerifyVersionInfo(&osvi, VER_MAJORVERSION |
VER_MINORVERSION, dwlConditionMask);
+
/* Count adapters. */
size_t adapter_count = 0;
for (struct tap_adapter_node *pAdapter = pAdapterList; pAdapter; pAdapter
= pAdapter->pNext)
{
+if (bSkipLegacyAdapters && pAdapter->dwIfType != IF_TYPE_PROP_VIRTUAL)
+{
+continue;
+}
adapter_count++;
}
@@ -331,6 +339,11 @@ find_adapters(
for (struct tap_adapter_node *pAdapter = pAdapterList; pAdapter; pAdapter
= pAdapter->pNext)
{
+if (bSkipLegacyAdapters && pAdapter->dwIfType != IF_TYPE_PROP_VIRTUAL)
+{
+continue;
+}
+
/* Convert adapter GUID to UTF-16 string. (LPOLESTR defaults to
LPWSTR) */
LPOLESTR szAdapterId = NULL;
StringFromIID((REFIID)&pAdapter->guid, &szAdapterId);
diff --git a/src/tapctl/tap.c b/src/tapctl/tap.c
index dd4a10a3..0dfc7555 100644
--- a/src/tapctl/tap.c
+++ b/src/tapctl/tap.c
@@ -29,6 +29,7 @@
#include
#include
+#include
#include
#include
#include
@@ -551,6 +552,8 @@ get_reg_string(
*
* @param pguidAdapter A pointer to GUID that receives network adapter ID.
*
+ * @param pdwIfType A pointer to DWORD that receives interface type.
+ *
* @return ERROR_SUCCESS on success; Win32 error code otherwise
**/
static DWORD
@@ -558,7 +561,8 @@ get_net_adapter_guid(
_In_ HDEVINFO hDeviceInfoSet,
_In_ PSP_DEVINFO_DATA pDeviceInfoData,
_In_ int iNumAttempts,
-_Out_ LPGUID pguidAdapter)
+_Out_ LPGUID pguidAdapter,
+_Out_opt_ LPDWORD pdwIfType)
{
DWORD dwResult = ERROR_BAD_ARGUMENTS;
@@ -613,6 +617,23 @@ get_net_adapter_guid(
dwResult = SUCCEEDED(CLSIDFromString(szCfgGuidString,
(LPCLSID)pguidAdapter)) ? ERROR_SUCCESS : ERROR_INVALID_DATA;
free(szCfgGuidString);
+
+if (pdwIfType)
+{
+DWORD dwValueType = REG_NONE, dwSize = sizeof(*pdwIfType);
+dwResult = RegQueryValueEx(
+hKey,
+TEXT("*IfType"),
+NULL,
+&dwValueType,
+(BYTE *)pdwIfType,
+&dwSize);
+if (dwResult != ERROR_SUCCESS || dwValueType != REG_DWORD ||
dwSize != sizeof(*pdwIfType))
+{
+*pdwIfType = IF_TYPE_OTHER;
+}
+}
+
break;
}
@@ -839,7 +860,7 @@ tap_create_adapter(
}
/* Get network adapter ID from registry. Retry for max 30sec. */
-dwResult = get_net_adapter_guid(hDevInfoList, &devinfo_data, 30,
pguidAdapter);
+dwResult = get_net_adapter_guid(hDevInfoList, &devinfo_data, 30,
pguidAdapter, NULL);
cleanup_remove_device:
if (dwResult != ERROR_SUCCESS)
@@ -981,7 +1002,7 @@ execute_on_first_adapter(
/* Get adapter GUID. */
GUID guidAdapter;
-dwResult = get_net_adapter_guid(hDevInfoList, &devinfo_data, 1,
&guidAdapter);
+dwResult = get_net_adapter_guid(hDevInfoList, &devinfo_data, 1,
&guidAdapter, NULL);
if (dwResult != ERROR_SUCCESS)
{
/* Something is wrong with this device. Skip it. */
@@ -1259,7 +1280,8 @@ tap_list_adapters(
/* Get adapter GUID. */
GUID guidAdapter;
-dwResult = get_net_adapter_guid(hDevInfoList, &devinfo_data, 1,
&guidAdapter);
+DWORD dwIfType;
+dwResult = get_net_adapter_guid(hDevInfoList, &devinfo_data, 1,
&guidAdapter, &dwIfType);
if (dwResult != ERROR_SUCCESS)
{
/* Something is wrong with this device. Skip it. */
@@ -1321,6 +1343,7 @@ tap_list_adapters(
memcpy(node->szzHardwareIDs, szzDeviceHardwareIDs, hwid_size);
node->szName = (LPTSTR)((LPBYTE)node->szzHardwareIDs + hwid_size);
memcpy(node->szName, szName, name_size);
+node->dwIfType = dwIfType;
node->pNext = NULL;
if (
[Openvpn-devel] [PATCH 5/5] iservice: Resolve MSVC C4996 warnings
Lots of string functions were declared unsafe in favor of ..._s()
counterparts. However, the code already is careful about the buffer
size. Code analysis is just not smart enough (yet) to detect this.
The code was refactored to use ..._s() variants MSVC is considering as
"safe".
Signed-off-by: Simon Rozman
---
src/openvpnserv/automatic.c | 8
src/openvpnserv/common.c | 4 ++--
src/openvpnserv/interactive.c | 2 +-
src/openvpnserv/service.c | 4 ++--
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/openvpnserv/automatic.c b/src/openvpnserv/automatic.c
index 3f2ca345..0ba222a0 100644
--- a/src/openvpnserv/automatic.c
+++ b/src/openvpnserv/automatic.c
@@ -137,7 +137,7 @@ modext(LPTSTR dest, size_t size, LPCTSTR src, LPCTSTR
newext)
if (size > 0 && (_tcslen(src) + 1) <= size)
{
-_tcscpy(dest, src);
+_tcscpy_s(dest, size, src);
dest [size - 1] = TEXT('\0');
i = _tcslen(dest);
while (i-- > 0)
@@ -154,8 +154,8 @@ modext(LPTSTR dest, size_t size, LPCTSTR src, LPCTSTR
newext)
}
if (_tcslen(dest) + _tcslen(newext) + 2 <= size)
{
-_tcscat(dest, TEXT("."));
-_tcscat(dest, newext);
+_tcscat_s(dest, size, TEXT("."));
+_tcscat_s(dest, size, newext);
return true;
}
dest[0] = TEXT('\0');
@@ -271,7 +271,7 @@ ServiceStartAutomatic(DWORD dwArgc, LPTSTR *lpszArgv)
BOOL more_files;
TCHAR find_string[MAX_PATH];
-openvpn_sntprintf(find_string, MAX_PATH, TEXT("%s\\*"),
settings.config_dir);
+openvpn_sntprintf(find_string, _countof(find_string), TEXT("%s\\*"),
settings.config_dir);
find_handle = FindFirstFile(find_string, &find_obj);
if (find_handle == INVALID_HANDLE_VALUE)
diff --git a/src/openvpnserv/common.c b/src/openvpnserv/common.c
index 958643df..48769be4 100644
--- a/src/openvpnserv/common.c
+++ b/src/openvpnserv/common.c
@@ -37,7 +37,7 @@ openvpn_vsntprintf(LPTSTR str, size_t size, LPCTSTR format,
va_list arglist)
int len = -1;
if (size > 0)
{
-len = _vsntprintf(str, size, format, arglist);
+len = _vsntprintf_s(str, size, _TRUNCATE, format, arglist);
str[size - 1] = 0;
}
return (len >= 0 && (size_t)len < size);
@@ -311,7 +311,7 @@ get_win_sys_path(void)
if (!GetSystemDirectoryW(win_sys_path, _countof(win_sys_path)))
{
-wcsncpy(win_sys_path, default_sys_path, _countof(win_sys_path));
+wcscpy_s(win_sys_path, _countof(win_sys_path), default_sys_path);
win_sys_path[_countof(win_sys_path) - 1] = L'\0';
}
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index b073a0d5..ed83d2a3 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -1067,7 +1067,7 @@ netsh_dns_cmd(const wchar_t *action, const wchar_t
*proto, const wchar_t *if_nam
if (IsWindows7OrGreater())
{
-wcsncat(cmdline, L" validate=no", ncmdline - wcslen(cmdline) - 1);
+wcscat_s(cmdline, ncmdline, L" validate=no");
}
err = ExecCommand(argv0, cmdline, timeout);
diff --git a/src/openvpnserv/service.c b/src/openvpnserv/service.c
index 8efe25f9..8101f83d 100644
--- a/src/openvpnserv/service.c
+++ b/src/openvpnserv/service.c
@@ -61,14 +61,14 @@ CmdInstallServices()
TCHAR path[512];
int i, ret = _service_max;
-if (GetModuleFileName(NULL, path + 1, 510) == 0)
+if (GetModuleFileName(NULL, path + 1, _countof(path) - 2) == 0)
{
_tprintf(TEXT("Unable to install service - %s\n"), GetLastErrorText());
return 1;
}
path[0] = TEXT('\"');
-_tcscat(path, TEXT("\""));
+_tcscat_s(path, _countof(path), TEXT("\""));
svc_ctl_mgr = OpenSCManager(NULL, NULL, SC_MANAGER_CONNECT |
SC_MANAGER_CREATE_SERVICE);
if (svc_ctl_mgr == NULL)
--
2.30.0.windows.2
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 4/5] tapctl: Resolve MSVC C4996 warnings
wcsncat() was declared unsafe in favour of wcsncat_s(). However, the
string concatenation follows the string length check, making wcsncat()
safe too. Code analysis is just not smart enough (yet) to detect this.
The code was refactored to use wcscat_s() MSVC is considering as "safe".
Signed-off-by: Simon Rozman
---
src/tapctl/tap.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/tapctl/tap.c b/src/tapctl/tap.c
index dd4a10a3..3f76c43a 100644
--- a/src/tapctl/tap.c
+++ b/src/tapctl/tap.c
@@ -2,7 +2,7 @@
* tapctl -- Utility to manipulate TUN/TAP adapters on Windows
*https://community.openvpn.net/openvpn/wiki/Tapctl
*
- * Copyright (C) 2018-2020 Simon Rozman
+ * Copyright (C) 2018-2021 Simon Rozman
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2
@@ -73,14 +73,13 @@ find_function(const WCHAR *libname, const char *funcname,
HMODULE *m)
return NULL;
}
-size_t len = _countof(libpath) - wcslen(libpath) - 1;
-if (len < wcslen(libname) + 1)
+if (wcslen(libpath) + 1 /*\*/ + wcslen(libname) >= _countof(libpath))
{
SetLastError(ERROR_INSUFFICIENT_BUFFER);
return NULL;
}
-wcsncat(libpath, L"\\", len);
-wcsncat(libpath, libname, len-1);
+wcscat_s(libpath, _countof(libpath), L"\\");
+wcscat_s(libpath, _countof(libpath), libname);
*m = LoadLibraryW(libpath);
if (*m == NULL)
--
2.30.0.windows.2
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 2/5] tun.c: Remove dead code
Signed-off-by: Simon Rozman
---
src/openvpn/tun.c | 34 --
1 file changed, 34 deletions(-)
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 6c51a52d..6b7c8ef1 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -85,8 +85,6 @@ static void netsh_command(const struct argv *a, int n, int
msglevel);
static const char *netsh_get_id(const char *dev_node, struct gc_arena *gc);
-static DWORD get_adapter_index_flexible(const char *name);
-
static bool
do_address_service(const bool add, const short family, const struct tuntap *tt)
{
@@ -4877,38 +4875,6 @@ get_adapter_index(const char *guid)
return index;
}
-static DWORD
-get_adapter_index_flexible(const char *name) /* actual name or GUID */
-{
-struct gc_arena gc = gc_new();
-DWORD index;
-index = get_adapter_index_method_1(name);
-if (index == TUN_ADAPTER_INDEX_INVALID)
-{
-index = get_adapter_index_method_2(name);
-}
-if (index == TUN_ADAPTER_INDEX_INVALID)
-{
-const struct tap_reg *tap_reg = get_tap_reg(&gc);
-const struct panel_reg *panel_reg = get_panel_reg(&gc);
-const struct tap_reg *tr = get_adapter_by_name(name, tap_reg,
panel_reg);
-if (tr)
-{
-index = get_adapter_index_method_1(tr->guid);
-if (index == TUN_ADAPTER_INDEX_INVALID)
-{
-index = get_adapter_index_method_2(tr->guid);
-}
-}
-}
-if (index == TUN_ADAPTER_INDEX_INVALID)
-{
-msg(M_INFO, "NOTE: could not get adapter index for name/GUID '%s'",
name);
-}
-gc_free(&gc);
-return index;
-}
-
/*
* Return a string representing a PIP_ADDR_STRING
*/
--
2.30.0.windows.2
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 1/5] MSVC: Disable LZ4
Commit 24596b25 ("build: Remove compat-lz4") removed lz4 compat layer,
but openvpn-build\msvc doesn't provide LZ4 library either.
Signed-off-by: Simon Rozman
---
config-msvc.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/config-msvc.h b/config-msvc.h
index e430ca96..53d97902 100644
--- a/config-msvc.h
+++ b/config-msvc.h
@@ -9,7 +9,6 @@
#define ENABLE_FRAGMENT 1
#define ENABLE_HTTP_PROXY 1
#define ENABLE_LZO 1
-#define ENABLE_LZ4 1
#define ENABLE_MANAGEMENT 1
#define ENABLE_MULTIHOME 1
#define ENABLE_PKCS11 1
--
2.30.0.windows.2
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 3/5] interactive.c: Resolve MSVC C4996 warning
It's about using a standard recommended alias for the wcsdup():
> warning C4996: 'wcsdup': The POSIX name for this item is deprecated.
> Instead, use the ISO C and C++ conformant name: _wcsdup. See online
> help for details.
And the documentation says:
> The Microsoft-implemented POSIX function names strdup and wcsdup are
> deprecated aliases for the _strdup and _wcsdup functions. By default,
> they generate Compiler warning (level 3) C4996. The names are
> deprecated because they don't follow the Standard C rules for
> implementation-specific names. However, the functions are still
> supported.
>
> We recommend you use _strdup and _wcsdup instead. Or, you can continue
> to use these function names, and disable the warning. For more
> information, see Turn off the warning and POSIX function names.
Reference:
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/strdup-wcsdup
Signed-off-by: Simon Rozman
---
src/openvpnserv/interactive.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index 5d5cbfe6..b073a0d5 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -899,7 +899,7 @@ ExecCommand(const WCHAR *argv0, const WCHAR *cmdline, DWORD
timeout)
si.cb = sizeof(si);
/* CreateProcess needs a modifiable cmdline: make a copy */
-cmdline_dup = wcsdup(cmdline);
+cmdline_dup = _wcsdup(cmdline);
if (cmdline_dup && CreateProcessW(argv0, cmdline_dup, NULL, NULL, FALSE,
proc_flags, NULL, NULL, &si, &pi) )
{
@@ -1181,7 +1181,7 @@ SetDNSDomain(const wchar_t *if_name, const char *domain,
undo_lists_t *lists)
/* Add to undo list if domain is non-empty */
if (err == 0 && wdomain[0] && lists)
{
-wchar_t *tmp_name = wcsdup(if_name);
+wchar_t *tmp_name = _wcsdup(if_name);
if (!tmp_name || AddListItem(&(*lists)[undo_domain], tmp_name))
{
free(tmp_name);
@@ -1272,7 +1272,7 @@ HandleDNSConfigMessage(const dns_cfg_message_t *msg,
undo_lists_t *lists)
if (msg->addr_len > 0)
{
-wchar_t *tmp_name = wcsdup(wide_name);
+wchar_t *tmp_name = _wcsdup(wide_name);
if (!tmp_name || AddListItem(&(*lists)[undo_type], tmp_name))
{
free(tmp_name);
--
2.30.0.windows.2
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 4/5] tapctl: Resolve MSVC C4996 warnings
Hi, > > -73,14 +73,13 @@ find_function(const WCHAR *libname, const char > *funcname, HMODULE *m) > > return NULL; > > } > > > > -size_t len = _countof(libpath) - wcslen(libpath) - 1; > > -if (len < wcslen(libname) + 1) > > +if (wcslen(libpath) + 1 /*\*/ + wcslen(libname) >= > > + _countof(libpath)) > > This random inline comment feels extremely weird. It's trying to describe the "+ 1" amounts for a backslash \ being strcat-ed in the process below. Regards, Simon ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 1/5] MSVC: Disable LZ4
Hi,
> > Commit 24596b25 ("build: Remove compat-lz4") removed lz4 compat layer,
> > but openvpn-build\msvc doesn't provide LZ4 library either.
>
> What would be needed to actually *build* with LZ4 on MSVC? That is,
> build it as prerequisite as LZO is built?
>
> The idea wasn't to remove LZ4 from builds, just to remove the bundled
> LZ4 "because all platforms have it now, so we do not need to maintain
> our own copy". But it seems that was a bit shortsighted wrt windows
> building...
Thank you and Arne for explaining this. I should have followed the discussion
on the OpenVPN meetings. Unfortunately, my workload doesn't allow me to follow
on anything these days. So, I am not in condition to prepare LZ4 building in
openvpn-build/msvc either.
I can live with LZ4 disabled in my sandbox only.
Shall we drop this patch for now?
Regards, Simon
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2 4/5] tapctl: Resolve MSVC C4996 warnings
wcsncat() was declared unsafe in favour of wcsncat_s(). However, the
string concatenation follows the string length check, making wcsncat()
safe too. Code analysis is just not smart enough (yet) to detect this.
The code was refactored to use wcscat_s() MSVC is considering as "safe".
Signed-off-by: Simon Rozman
---
src/tapctl/tap.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/tapctl/tap.c b/src/tapctl/tap.c
index dd4a10a3..563c07f6 100644
--- a/src/tapctl/tap.c
+++ b/src/tapctl/tap.c
@@ -2,7 +2,7 @@
* tapctl -- Utility to manipulate TUN/TAP adapters on Windows
*https://community.openvpn.net/openvpn/wiki/Tapctl
*
- * Copyright (C) 2018-2020 Simon Rozman
+ * Copyright (C) 2018-2021 Simon Rozman
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2
@@ -73,14 +73,15 @@ find_function(const WCHAR *libname, const char *funcname,
HMODULE *m)
return NULL;
}
-size_t len = _countof(libpath) - wcslen(libpath) - 1;
-if (len < wcslen(libname) + 1)
+/* +1 for the path seperator '\' */
+const size_t path_length = wcslen(libpath) + 1 + wcslen(libname);
+if (path_length >= _countof(libpath))
{
SetLastError(ERROR_INSUFFICIENT_BUFFER);
return NULL;
}
-wcsncat(libpath, L"\\", len);
-wcsncat(libpath, libname, len-1);
+wcscat_s(libpath, _countof(libpath), L"\\");
+wcscat_s(libpath, _countof(libpath), libname);
*m = LoadLibraryW(libpath);
if (*m == NULL)
--
2.30.0.windows.2
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] openvpnserv: Cache last error before it is overridden
FormatMessage() sets the last error according to its own success. This
looses the original error code leading to mismatched error message and
error number when sprintfted together resulting in confusing event log
message.
Signed-off-by: Simon Rozman
---
src/openvpnserv/common.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/openvpnserv/common.c b/src/openvpnserv/common.c
index 48769be4..ebd08677 100644
--- a/src/openvpnserv/common.c
+++ b/src/openvpnserv/common.c
@@ -228,12 +228,14 @@ out:
LPCTSTR
GetLastErrorText()
{
+DWORD error;
static TCHAR buf[256];
DWORD len;
LPTSTR tmp = NULL;
+error = GetLastError();
len = FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER |
FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ARGUMENT_ARRAY,
-NULL, GetLastError(), LANG_NEUTRAL, (LPTSTR)&tmp, 0,
NULL);
+NULL, error, LANG_NEUTRAL, (LPTSTR)&tmp, 0, NULL);
if (len == 0 || (long) _countof(buf) < (long) len + 14)
{
@@ -242,7 +244,7 @@ GetLastErrorText()
else
{
tmp[_tcslen(tmp) - 2] = TEXT('\0'); /* remove CR/LF characters */
-openvpn_sntprintf(buf, _countof(buf), TEXT("%s (0x%x)"), tmp,
GetLastError());
+openvpn_sntprintf(buf, _countof(buf), TEXT("%s (0x%x)"), tmp, error);
}
if (tmp)
--
2.30.0.windows.2
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
