From c34334152d48a2f3c8f89f1ef7f27616f89a5462 Mon Sep 17 00:00:00 2001 From: Marios Paouris Date: Mon, 23 Sep 2024 08:42:09 +0300 Subject: [PATCH] Rework adapter search. Enumerate adapters to a list to decouple searching from enumerating. Add adapters with of not interested types to the list, to facilitate name collision detection, if needed. Get Wintun adapter guid by calling APIs instead of searching again. Also, disabled list-taps on cross mingw builds Signed-off-by: Marios Paouris --- .gitlab-ci.yml | 14 ++-- openconnect-internal.h | 1 + tests/list-taps.c | 14 +++- tests/wintun-names.c | 61 ++++++++++------- tun-win32.c | 148 ++++++++++++++++++++++++++++++++++------- wintun.c | 26 ++++++++ 6 files changed, 209 insertions(+), 55 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index d086b1e8..892c426d 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -603,8 +603,9 @@ MinGW32/GnuTLS: - make -j4 # Setup wine path so tests won't fail due to unresolved dll dependencies - export WINEPATH=/usr/i686-w64-mingw32/sys-root/mingw/bin\;. -# Wintun tests cannot be run under wine, wintun cannot create the interface on wine. - - make VERBOSE=1 XFAIL_TESTS="wintun-names.exe" -j4 check +# Wintun tests cannot be run under wine, wintun cannot create the interface on wine, +# Tap tests also: wine does not support windows drivers + - make VERBOSE=1 XFAIL_TESTS="list-taps.exe wintun-names.exe" -j4 check tags: - saas-linux-small-amd64 except: @@ -628,7 +629,8 @@ MinGW32/OpenSSL: # Setup wine path so tests won't fail due to unresolved dll dependencies - export WINEPATH=/usr/i686-w64-mingw32/sys-root/mingw/bin\;. # Wintun tests cannot be run under wine, wintun cannot create the interface on wine. - - make VERBOSE=1 XFAIL_TESTS="wintun-names.exe" -j4 check +# Tap tests also: wine does not support windows drivers + - make VERBOSE=1 XFAIL_TESTS="list-taps.exe wintun-names.exe" -j4 check tags: - saas-linux-small-amd64 except: @@ -653,7 +655,8 @@ MinGW64/GnuTLS: # Setup wine path so tests won't fail due to unresolved dll dependencies - export WINEPATH=/usr/x86_64-w64-mingw32/sys-root/mingw/bin\;. # Wintun tests cannot be run under wine, wintun cannot create the interface on wine. - - make VERBOSE=1 XFAIL_TESTS="wintun-names.exe" -j4 check +# Tap tests also: wine does not support windows drivers + - make VERBOSE=1 XFAIL_TESTS="list-taps.exe wintun-names.exe" -j4 check tags: - saas-linux-small-amd64 except: @@ -678,7 +681,8 @@ MinGW64/OpenSSL: # Setup wine path so tests won't fail due to unresolved dll dependencies - export WINEPATH=/usr/x86_64-w64-mingw32/sys-root/mingw/bin\;. # Wintun tests cannot be run under wine, wintun cannot create the interface on wine. - - make VERBOSE=1 XFAIL_TESTS="wintun-names.exe" -j4 check +# Tap tests also: wine does not support windows drivers + - make VERBOSE=1 XFAIL_TESTS="list-taps.exe wintun-names.exe" -j4 check tags: - saas-linux-small-amd64 except: diff --git a/openconnect-internal.h b/openconnect-internal.h index 5abfe98d..1d22e723 100644 --- a/openconnect-internal.h +++ b/openconnect-internal.h @@ -1268,6 +1268,7 @@ intptr_t os_setup_wintun(struct openconnect_info *vpninfo); int setup_wintun_fd(struct openconnect_info *vpninfo, intptr_t tun_fd); intptr_t open_wintun(struct openconnect_info *vpninfo, char *guid, wchar_t *wname); int create_wintun(struct openconnect_info *vpninfo); +int get_wintun_adapter_guid(struct openconnect_info *vpninfo, char *buf, size_t buf_len); #endif /* {gnutls,openssl}-dtls.c */ diff --git a/tests/list-taps.c b/tests/list-taps.c index 0ea5e1de..8482896e 100644 --- a/tests/list-taps.c +++ b/tests/list-taps.c @@ -43,12 +43,22 @@ struct openconnect_info { static intptr_t print_tun(struct openconnect_info *vpninfo, int type, char *guid, wchar_t *wname) { printf("Found %s device '%S' guid %s\n", - type ? "Wintun" : "Tap", wname, guid); + (type == ADAPTER_WINTUN) ? "Wintun" : "Tap", wname, guid); return 0; } int main(void) { - search_taps(NULL, print_tun); + intptr_t ret; + struct oc_adapter_info *list = NULL; + + list = get_adapter_list(NULL); + + if (!list) + return 1; + + search_taps(NULL, list, print_tun); + + free_adapter_list(list); return 0; } diff --git a/tests/wintun-names.c b/tests/wintun-names.c index 9bc9b5f0..22430de4 100644 --- a/tests/wintun-names.c +++ b/tests/wintun-names.c @@ -186,18 +186,24 @@ cleanupWintun: return NULL; } -wchar_t * currentAdapterName; - -static intptr_t check_tun(struct openconnect_info *vpninfo, int type, char *guid, wchar_t *wname) +static intptr_t check_tun(struct openconnect_info *vpninfo, struct oc_adapter_info *list, wchar_t *wname) { - if (currentAdapterName != 0) { - if ( ! wcscmp(currentAdapterName, wname) ) { + if (list) { + struct oc_adapter_info *found = find_adapter_by_name(vpninfo, list, wname); + if (!found || found->type != ADAPTER_WINTUN) { + wprintf(L"Device %s was%s found%s", + (!found ? " not" : ""), (found? ", but is not of expected type" : "") + ); + return OPEN_TUN_SOFTFAIL; + } + + if ( ! wcscmp(wname, found->ifname) ) { wprintf(L"Found %s device '%ls' guid %s\n", - type ? "Wintun" : "Tap", wname, guid); + (found->type == ADAPTER_WINTUN) ? "Wintun" : "Tap", wname, found->guid); return VALID_WINTUN_HANDLE; } } - return 0; + return OPEN_TUN_SOFTFAIL; } int main(void) @@ -225,21 +231,25 @@ int main(void) wsprintfW(adapterName, L"testAdapterNameForRunningLoops_"); int add = 0; int len = 0; + struct oc_adapter_info *list; do { + ret = OPEN_TUN_HARDFAIL; wsprintfW(adapterName, L"%s%d", adapterName, add); add = (add+1)%10; len = wcslen(adapterName); Log(WINTUN_LOG_INFO, L"len=%3d name=%ls", len, adapterName); adapter = create_wintun_adapter(adapterName); - if (adapter == NULL) - ret = OPEN_TUN_HARDFAIL; - else { - currentAdapterName = adapterName; - ret = search_taps(NULL, check_tun); - WintunCloseAdapter(adapter); - Log(WINTUN_LOG_INFO, L"Wintun adapter closed"); + if (adapter) { + list = get_adapter_list(NULL); + if (list) { + ret = check_tun(NULL, list, adapterName); + free_adapter_list(list); + } + + WintunCloseAdapter(adapter); + Log(WINTUN_LOG_INFO, L"Wintun adapter closed"); } } while ( (len < (MAX_ADAPTER_NAME - 1)) && (ret == VALID_WINTUN_HANDLE) ); @@ -257,21 +267,22 @@ int main(void) } Log(WINTUN_LOG_INFO, L"len=%3d name=%ls", len, adapterName); + + ret = OPEN_TUN_HARDFAIL; adapter = create_wintun_adapter(adapterName); - if (adapter == NULL) { - /* last test was not succesful*/ - FreeLibrary(Wintun); - return 2; - } - else { - currentAdapterName = adapterName; - ret = search_taps(NULL, check_tun); - WintunCloseAdapter(adapter); - Log(WINTUN_LOG_INFO, L"Wintun adapter closed"); + if (adapter) { + list = get_adapter_list(NULL); + if (list) { + ret = check_tun(NULL, list, adapterName); + free_adapter_list(list); + } + + WintunCloseAdapter(adapter); + Log(WINTUN_LOG_INFO, L"Wintun adapter closed"); } FreeLibrary(Wintun); - return 0; + return (ret == VALID_WINTUN_HANDLE ? 0 : 2); } diff --git a/tun-win32.c b/tun-win32.c index a014b2a5..0a1d1436 100644 --- a/tun-win32.c +++ b/tun-win32.c @@ -62,15 +62,68 @@ #define ADAPTERS_KEY CONTROL_KEY "Class\\" NETDEV_GUID #define CONNECTIONS_KEY CONTROL_KEY "Network\\" NETDEV_GUID -#define ADAPTER_TUNTAP 0 -#define ADAPTER_WINTUN 1 +#define ADAPTER_NONE 0 +#define ADAPTER_TUNTAP 1 +#define ADAPTER_WINTUN 2 typedef intptr_t (tap_callback)(struct openconnect_info *vpninfo, int type, char *idx, wchar_t *name); #define SEARCH_CONTINUE 0 #define SEARCH_DONE 1 -static intptr_t search_taps(struct openconnect_info *vpninfo, tap_callback *cb) +/* a linked list of adapter information */ +struct oc_adapter_info { + int type; + char *guid; + wchar_t *ifname; + struct oc_adapter_info *next; +}; + +static void free_adapter_list(struct oc_adapter_info *first) +{ + struct oc_adapter_info *next = first; + + while (next) { + struct oc_adapter_info *this = next; + next = next->next; + + free(this->guid); + free(this->ifname); + free(this); + } +} + +static struct oc_adapter_info * find_adapter_by_name(struct openconnect_info *vpninfo, struct oc_adapter_info *adapter_list, wchar_t *wname) +{ + struct oc_adapter_info *this = adapter_list; + struct oc_adapter_info *found = NULL; + + while (this && !found ) { + if (!wcscmp(this->ifname, wname)) { + found = this; + } + this = this->next; + } + + return found; +} + +static intptr_t search_taps(struct openconnect_info *vpninfo, struct oc_adapter_info *adapter_list, tap_callback *cb) +{ + struct oc_adapter_info *this = adapter_list; + intptr_t ret = OPEN_TUN_SOFTFAIL; + + while (this && ret == OPEN_TUN_SOFTFAIL) { + if (this->type != ADAPTER_NONE) { + ret = cb(vpninfo, this->type, this->guid, this->ifname); + } + this = this->next; + } + + return ret; +} + +static struct oc_adapter_info * get_adapter_list(struct openconnect_info *vpninfo) { LONG status; HKEY adapters_key, hkey; @@ -80,22 +133,28 @@ static intptr_t search_taps(struct openconnect_info *vpninfo, tap_callback *cb) wchar_t name[MAX_ADAPTER_NAME]; char keyname[strlen(CONNECTIONS_KEY) + sizeof(buf) + 1 + strlen("\\Connection")]; int i = 0; - intptr_t ret = OPEN_TUN_SOFTFAIL; + struct oc_adapter_info *first = NULL, *last = NULL; status = RegOpenKeyExA(HKEY_LOCAL_MACHINE, ADAPTERS_KEY, 0, KEY_READ, &adapters_key); if (status) { + char *errstr = openconnect__win32_strerror(status); vpn_progress(vpninfo, PRG_ERR, - _("Error accessing registry key for network adapters\n")); - return -EIO; + _("Error accessing registry key for network adapters: %s (%ld)\n"), + errstr, status); + free(errstr); + return NULL; } - while (ret == OPEN_TUN_SOFTFAIL) { + + while (status != ERROR_NO_MORE_ITEMS) { len = sizeof(buf); status = RegEnumKeyExA(adapters_key, i++, buf, &len, NULL, NULL, NULL, NULL); if (status) { - if (status != ERROR_NO_MORE_ITEMS) - ret = OPEN_TUN_HARDFAIL; + if (status != ERROR_NO_MORE_ITEMS) { + free_adapter_list(first); + first = NULL; + } break; } @@ -104,8 +163,12 @@ static intptr_t search_taps(struct openconnect_info *vpninfo, tap_callback *cb) status = RegOpenKeyExA(HKEY_LOCAL_MACHINE, keyname, 0, KEY_QUERY_VALUE, &hkey); - if (status) + if (status) { + vpn_progress(vpninfo, PRG_TRACE, + _("Cannot open registry key %s: %s (%ld)\n"), + keyname, openconnect__win32_strerror(status), status); continue; + } len = sizeof(buf); status = RegQueryValueExA(hkey, "ComponentId", NULL, &type, @@ -129,6 +192,7 @@ static intptr_t search_taps(struct openconnect_info *vpninfo, tap_callback *cb) RegCloseKey(hkey); continue; } + if (!stricmp(buf, TAP_COMPONENT_ID) || !stricmp(buf, "root\\" TAP_COMPONENT_ID) || !stricmp(buf, TAP_OVPNCONNECT_COMPONENT_ID) || !stricmp(buf, "root\\" TAP_OVPNCONNECT_COMPONENT_ID)) @@ -138,8 +202,7 @@ static intptr_t search_taps(struct openconnect_info *vpninfo, tap_callback *cb) else { vpn_progress(vpninfo, PRG_TRACE, _("%s\\ComponentId is unknown '%s'\n"), keyname, buf); - RegCloseKey(hkey); - continue; + adapter_type = ADAPTER_NONE; } vpn_progress(vpninfo, PRG_TRACE, _("Found %s at %s\n"), @@ -192,12 +255,33 @@ static intptr_t search_taps(struct openconnect_info *vpninfo, tap_callback *cb) continue; } - ret = cb(vpninfo, adapter_type, buf, name); + struct oc_adapter_info *new = malloc(sizeof(struct oc_adapter_info)); + + if (!new) { + vpn_progress(vpninfo, PRG_ERR, + _("Cannot allocate memory for adapter info\n")); + free_adapter_list(first); + first = NULL; + break; + } + + new->type = adapter_type; + new->ifname = wcsdup(name); + new->guid = strdup(buf); + new->next = NULL; + + if (last) { + last->next = new; + } + if (!first) { + first = new; + } + last = new; } RegCloseKey(adapters_key); - return ret; + return first; } #ifndef __LIST_TAPS__ @@ -464,7 +548,7 @@ static intptr_t open_tun(struct openconnect_info *vpninfo, int adapter_type, cha vpn_progress(vpninfo, PRG_DEBUG, _("Ignoring non-matching interface \"%S\"\n"), wname); - return 0; + return OPEN_TUN_SOFTFAIL; } if (adapter_type == ADAPTER_TUNTAP) @@ -500,7 +584,7 @@ static intptr_t open_tun(struct openconnect_info *vpninfo, int adapter_type, cha vpn_progress(vpninfo, adapter_type ? PRG_ERR : PRG_INFO, _("Using %s device '%s', index %d\n"), - adapter_type ? "Wintun" : "TAP-Windows", + (adapter_type == ADAPTER_WINTUN) ? "Wintun" : "TAP-Windows", vpninfo->ifname, vpninfo->tun_idx); if (adapter_type == ADAPTER_WINTUN) vpn_progress(vpninfo, PRG_ERR, @@ -534,31 +618,46 @@ static intptr_t create_ifname_w(struct openconnect_info *vpninfo, intptr_t os_setup_tun(struct openconnect_info *vpninfo) { intptr_t ret; + struct oc_adapter_info *list = NULL; + + list = get_adapter_list(vpninfo); + + if (!list) + goto safe_return; if (vpninfo->ifname) { ret = create_ifname_w(vpninfo, vpninfo->ifname); if (ret) - return ret; + goto safe_return; } - ret = search_taps(vpninfo, open_tun); + ret = search_taps(vpninfo, list, open_tun); if (ret == OPEN_TUN_SOFTFAIL) { if (!vpninfo->ifname_w) { ret = create_ifname_w(vpninfo, vpninfo->hostname); if (ret) - return ret; + goto safe_return; } /* Try creating a Wintun instead of TAP */ int retw = create_wintun(vpninfo); if (!retw) { - ret = search_taps(vpninfo, open_tun); - - if (ret == OPEN_TUN_SOFTFAIL) + char wintun_adapter_guid[40]; + /* we have a wintun adapter. Get its guid and open it via the callback */ + if (get_wintun_adapter_guid(vpninfo, wintun_adapter_guid, sizeof(wintun_adapter_guid))) { ret = OPEN_TUN_HARDFAIL; - if (ret == OPEN_TUN_HARDFAIL) + } + else { + ret = open_tun(vpninfo, ADAPTER_WINTUN, wintun_adapter_guid, vpninfo->ifname_w); + + if (ret == OPEN_TUN_SOFTFAIL) + ret = OPEN_TUN_HARDFAIL; + } + + if (ret == OPEN_TUN_HARDFAIL) { os_shutdown_wintun(vpninfo); + } } else if (retw == -EPERM) { ret = OPEN_TUN_HARDFAIL; vpn_progress(vpninfo, PRG_ERR, @@ -575,6 +674,9 @@ intptr_t os_setup_tun(struct openconnect_info *vpninfo) if (check_address_conflicts(vpninfo) < 0) ret = OPEN_TUN_HARDFAIL; /* already complained about it */ +safe_return: + if (list) + free_adapter_list(list); return ret; } diff --git a/wintun.c b/wintun.c index 59e69238..9b13062f 100644 --- a/wintun.c +++ b/wintun.c @@ -112,6 +112,32 @@ int create_wintun(struct openconnect_info *vpninfo) return (ret == ERROR_ACCESS_DENIED ? -EPERM : -EIO); } +int get_wintun_adapter_guid(struct openconnect_info *vpninfo, char *buf, size_t buf_len) +{ + if (!vpninfo->wintun_adapter) { + vpn_progress(vpninfo, PRG_ERR, _("Wintun adapter has not been opened\n")); + return 1; + } + + NET_LUID luid; + WintunGetAdapterLUID(vpninfo->wintun_adapter, &luid); + + GUID guid; + DWORD ret = ConvertInterfaceLuidToGuid(&luid, &guid); + if (ret != NO_ERROR ) { + vpn_progress(vpninfo, PRG_ERR, _("Unable to get Wintun adapter Guid\n")); + return 1; + } + + snprintf(buf, buf_len, "{%08lX-%04hX-%04hX-%02hhX%02hhX-%02hhX%02hhX%02hhX%02hhX%02hhX%02hhX}", + guid.Data1, guid.Data2, guid.Data3, + guid.Data4[0], guid.Data4[1], guid.Data4[2], guid.Data4[3], + guid.Data4[4], guid.Data4[5], guid.Data4[6], guid.Data4[7] + ); + + return 0; +} + intptr_t open_wintun(struct openconnect_info *vpninfo, char *guid, wchar_t *wname) { intptr_t ret; -- 2.50.1