From patchwork Fri Dec 20 16:11:11 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,v2,1/7] tun.c: make Windows device lookup functions more general X-Patchwork-Submitter: Simon Rozman X-Patchwork-Id: 959 Message-Id: <20191220161117.1434-1-simon@rozman.si> To: openvpn-devel@lists.sourceforge.net Date: Fri, 20 Dec 2019 17:11:11 +0100 From: Simon Rozman List-Id: Since the introduction of Wintun, not all network devices in Windows are TAP-Windows6. Rather than returning a simple true/false answer, a couple of functions were reworked to return a corresponding struct tap_reg * or NULL instead. As it would make the code `tr = is_tap_win(...)` a bit awkward those functions (both static) were renamed to better reflect their nature. Signed-off-by: Simon Rozman Acked-by: Lev Stipakov --- src/openvpn/tun.c | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index ad497a71..0d6f40fe 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -3976,10 +3976,10 @@ show_tap_win_adapters(int msglev, int warnlev) } /* - * Confirm that GUID is a TAP-Windows adapter. + * Lookup a TAP-Windows or Wintun adapter by GUID. */ -static bool -is_tap_win(const char *guid, const struct tap_reg *tap_reg) +static const struct tap_reg * +get_adapter_by_guid(const char *guid, const struct tap_reg *tap_reg) { const struct tap_reg *tr; @@ -3987,11 +3987,11 @@ is_tap_win(const char *guid, const struct tap_reg *tap_reg) { if (guid && !strcmp(tr->guid, guid)) { - return true; + return tr; } } - return false; + return NULL; } static const char * @@ -4010,16 +4010,16 @@ guid_to_name(const char *guid, const struct panel_reg *panel_reg) return NULL; } -static const char * -name_to_guid(const char *name, const struct tap_reg *tap_reg, const struct panel_reg *panel_reg) +static const struct tap_reg * +get_adapter_by_name(const char *name, const struct tap_reg *tap_reg, const struct panel_reg *panel_reg) { const struct panel_reg *pr; for (pr = panel_reg; pr != NULL; pr = pr->next) { - if (name && !strcmp(pr->name, name) && is_tap_win(pr->guid, tap_reg)) + if (name && !strcmp(pr->name, name)) { - return pr->guid; + return get_adapter_by_guid(pr->guid, tap_reg); } } @@ -4116,6 +4116,7 @@ get_device_guid(const char *name, { struct buffer ret = alloc_buf_gc(256, gc); struct buffer actual = clear_buf(); + const struct tap_reg *tr; /* Make sure we have at least one TAP adapter */ if (!tap_reg) @@ -4131,7 +4132,8 @@ get_device_guid(const char *name, } /* Check if GUID was explicitly specified as --dev-node parameter */ - if (is_tap_win(name, tap_reg)) + tr = get_adapter_by_guid(name, tap_reg); + if (tr) { const char *act = guid_to_name(name, panel_reg); buf_printf(&ret, "%s", name); @@ -4148,11 +4150,11 @@ get_device_guid(const char *name, /* Lookup TAP adapter in network connections list */ { - const char *guid = name_to_guid(name, tap_reg, panel_reg); - if (guid) + tr = get_adapter_by_name(name, tap_reg, panel_reg); + if (tr) { buf_printf(&actual, "%s", name); - buf_printf(&ret, "%s", guid); + buf_printf(&ret, "%s", tr->guid); return BSTR(&ret); } } @@ -4696,11 +4698,14 @@ get_adapter_index_flexible(const char *name) /* actual name or GUID */ { const struct tap_reg *tap_reg = get_tap_reg(&gc); const struct panel_reg *panel_reg = get_panel_reg(&gc); - const char *guid = name_to_guid(name, tap_reg, panel_reg); - index = get_adapter_index_method_1(guid); - if (index == TUN_ADAPTER_INDEX_INVALID) + const struct tap_reg *tr = get_adapter_by_name(name, tap_reg, panel_reg); + if (tr) { - index = get_adapter_index_method_2(guid); + 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) From patchwork Fri Dec 20 16:11:12 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,v2,2/7] tun.c: upgrade get_device_guid() to return the Windows driver type X-Patchwork-Submitter: Simon Rozman X-Patchwork-Id: 958 Message-Id: <20191220161117.1434-2-simon@rozman.si> To: openvpn-devel@lists.sourceforge.net Date: Fri, 20 Dec 2019 17:11:12 +0100 From: Simon Rozman List-Id: Signed-off-by: Simon Rozman Acked-by: Lev Stipakov --- src/openvpn/tun.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 0d6f40fe..f90f201d 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -4104,12 +4104,13 @@ get_unspecified_device_guid(const int device_number, /* * Lookup a --dev-node adapter name in the registry - * returning the GUID and optional actual_name. + * returning the GUID and optional actual_name and device type */ static const char * get_device_guid(const char *name, char *actual_name, int actual_name_size, + bool *wintun, const struct tap_reg *tap_reg, const struct panel_reg *panel_reg, struct gc_arena *gc) @@ -4145,6 +4146,10 @@ get_device_guid(const char *name, { buf_printf(&actual, "%s", name); } + if (wintun) + { + *wintun = tr->wintun; + } return BSTR(&ret); } @@ -4154,6 +4159,10 @@ get_device_guid(const char *name, if (tr) { buf_printf(&actual, "%s", name); + if (wintun) + { + *wintun = tr->wintun; + } buf_printf(&ret, "%s", tr->guid); return BSTR(&ret); } @@ -4838,7 +4847,7 @@ tap_allow_nonadmin_access(const char *dev_node) if (dev_node) { /* Get the device GUID for the device specified with --dev-node. */ - device_guid = get_device_guid(dev_node, actual_buffer, sizeof(actual_buffer), tap_reg, panel_reg, &gc); + device_guid = get_device_guid(dev_node, actual_buffer, sizeof(actual_buffer), NULL, tap_reg, panel_reg, &gc); if (!device_guid) { @@ -5412,7 +5421,7 @@ netsh_get_id(const char *dev_node, struct gc_arena *gc) if (dev_node) { - guid = get_device_guid(dev_node, BPTR(&actual), BCAP(&actual), tap_reg, panel_reg, gc); + guid = get_device_guid(dev_node, BPTR(&actual), BCAP(&actual), NULL, tap_reg, panel_reg, gc); } else { @@ -6132,7 +6141,7 @@ tun_open_device(struct tuntap *tt, const char *dev_node, const char **device_gui if (dev_node) { /* Get the device GUID for the device specified with --dev-node. */ - *device_guid = get_device_guid(dev_node, actual_buffer, sizeof(actual_buffer), tap_reg, panel_reg, &gc); + *device_guid = get_device_guid(dev_node, actual_buffer, sizeof(actual_buffer), NULL, tap_reg, panel_reg, &gc); if (!*device_guid) { From patchwork Fri Dec 20 16:11:13 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,v2,3/7] tun.c: make wintun_register_ring_buffer() non-fatal on failures X-Patchwork-Submitter: Simon Rozman X-Patchwork-Id: 954 Message-Id: <20191220161117.1434-3-simon@rozman.si> To: openvpn-devel@lists.sourceforge.net Date: Fri, 20 Dec 2019 17:11:13 +0100 From: Simon Rozman List-Id: Wintun allows multiple handles to be opened on it's NDIS device pipe. Just by succeeding to open the pipe does not warrant the adapter is unused. When iterating for available Wintun adapter, we will need to try registering ring buffers with each one to actually determine which one is used and which one is not. Therefore, a failure to register ring buffers should be detectable, but not M_FATAL. Signed-off-by: Simon Rozman Acked-by: Lev Stipakov --- src/openvpn/tun.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index f90f201d..c6bbbd41 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -5647,11 +5647,12 @@ register_dns_service(const struct tuntap *tt) gc_free(&gc); } -static void +static bool service_register_ring_buffers(const struct tuntap *tt) { HANDLE msg_channel = tt->options.msg_channel; ack_message_t ack; + bool ret = true; struct gc_arena gc = gc_new(); register_ring_buffers_message_t msg = { @@ -5669,13 +5670,13 @@ service_register_ring_buffers(const struct tuntap *tt) if (!send_msg_iservice(msg_channel, &msg, sizeof(msg), &ack, "Register ring buffers")) { - gc_free(&gc); - return; + ret = false; } else if (ack.error_number != NO_ERROR) { - msg(M_FATAL, "Register ring buffers failed using service: %s [status=0x%x]", + msg(M_NONFATAL, "Register ring buffers failed using service: %s [status=0x%x]", strerror_win32(ack.error_number, &gc), ack.error_number); + ret = false; } else { @@ -5683,6 +5684,7 @@ service_register_ring_buffers(const struct tuntap *tt) } gc_free(&gc); + return ret; } void @@ -5922,9 +5924,11 @@ tuntap_set_ip_addr(struct tuntap *tt, gc_free(&gc); } -static void +static bool wintun_register_ring_buffer(struct tuntap *tt) { + bool ret = true; + tt->wintun_send_ring = (struct tun_ring *)MapViewOfFile(tt->wintun_send_ring_handle, FILE_MAP_ALL_ACCESS, 0, @@ -5939,7 +5943,7 @@ wintun_register_ring_buffer(struct tuntap *tt) if (tt->options.msg_channel) { - service_register_ring_buffers(tt); + ret = service_register_ring_buffers(tt); } else { @@ -5953,13 +5957,16 @@ wintun_register_ring_buffer(struct tuntap *tt) tt->rw_handle.read, tt->rw_handle.write)) { - msg(M_FATAL, "ERROR: Failed to register ring buffers: %lu", GetLastError()); + msg(M_NONFATAL, "Failed to register ring buffers: %lu", GetLastError()); + ret = false; } if (!RevertToSelf()) { msg(M_FATAL, "ERROR: RevertToSelf error: %lu", GetLastError()); } } + + return ret; } static void @@ -6367,7 +6374,10 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun if (tt->wintun) { - wintun_register_ring_buffer(tt); + if (!wintun_register_ring_buffer(tt)) + { + msg(M_FATAL, "Failed to register ring buffers"); + } } else { From patchwork Fri Dec 20 16:11:14 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,v2,4/7] wintun: register ring buffers when iterating adapters X-Patchwork-Submitter: Simon Rozman X-Patchwork-Id: 955 Message-Id: <20191220161117.1434-4-simon@rozman.si> To: openvpn-devel@lists.sourceforge.net Date: Fri, 20 Dec 2019 17:11:14 +0100 From: Simon Rozman List-Id: Wintun adapters may be considered available if ring buffer registration succeeded. Therefore, we must attempt to register ring buffers when iterating adapters and continue on failure. Signed-off-by: Simon Rozman --- src/openvpn/tun.c | 112 +++++++++++++++++++++++++++------------------- 1 file changed, 66 insertions(+), 46 deletions(-) diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index c6bbbd41..f56682ef 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -6129,11 +6129,73 @@ tuntap_dhcp_mask(const struct tuntap *tt, const char *device_guid) gc_free(&gc); } +static bool +tun_try_open_device(struct tuntap *tt, const char *device_guid, const struct device_instance_id_interface *device_instance_id_interface) +{ + const char *path = NULL; + char tuntap_device_path[256]; + HANDLE h; + + if (tt->wintun) + { + const struct device_instance_id_interface *dev_if; + + /* Open Wintun adapter */ + for (dev_if = device_instance_id_interface; dev_if != NULL; dev_if = dev_if->next) + { + if (strcmp(dev_if->net_cfg_instance_id, device_guid) == 0) + { + path = dev_if->device_interface_list; + break; + } + } + if (path == NULL) + { + return false; + } + } + else + { + /* Open TAP-Windows adapter */ + openvpn_snprintf(tuntap_device_path, sizeof(tuntap_device_path), "%s%s%s", + USERMODEDEVICEDIR, + device_guid, + TAP_WIN_SUFFIX); + path = tuntap_device_path; + } + + h = CreateFile(path, + GENERIC_READ | GENERIC_WRITE, + 0, /* was: FILE_SHARE_READ */ + 0, + OPEN_EXISTING, + FILE_ATTRIBUTE_SYSTEM | FILE_FLAG_OVERLAPPED, + 0); + if (h == INVALID_HANDLE_VALUE) + { + msg(D_TUNTAP_INFO, "CreateFile failed on %s device: %s", tt->wintun ? "Wintun" : "TAP-Windows", path); + return false; + } + + if (tt->wintun) + { + /* Wintun adapter may be considered "open" after ring buffers are successfuly registered. */ + if (!wintun_register_ring_buffer(tt)) + { + msg(D_TUNTAP_INFO, "Failed to register %s adapter ring buffers", device_guid); + CloseHandle(h); + return false; + } + } + + tt->hand = h; + return true; +} + static void tun_open_device(struct tuntap *tt, const char *dev_node, const char **device_guid) { struct gc_arena gc = gc_new(); - char *path = NULL; char tuntap_device_path[256]; const struct tap_reg* tap_reg = get_tap_reg(&gc); const struct panel_reg* panel_reg = get_panel_reg(&gc); @@ -6197,27 +6259,11 @@ tun_open_device(struct tuntap *tt, const char *dev_node, const char **device_gui if (tt->wintun) { - const struct device_instance_id_interface* dev_if; - if (!is_picked_device_wintun) { /* wintun driver specified but picked adapter is not wintun, proceed to next one */ goto next; } - - path = NULL; - for (dev_if = device_instance_id_interface; dev_if != NULL; dev_if = dev_if->next) - { - if (strcmp(dev_if->net_cfg_instance_id, *device_guid) == 0) - { - path = (char*)dev_if->device_interface_list; - break; - } - } - if (path == NULL) - { - goto next; - } } else { @@ -6226,28 +6272,9 @@ tun_open_device(struct tuntap *tt, const char *dev_node, const char **device_gui /* tap-windows6 driver specified but picked adapter is wintun, proceed to next one */ goto next; } - - /* Open Windows TAP-Windows adapter */ - openvpn_snprintf(tuntap_device_path, sizeof(tuntap_device_path), "%s%s%s", - USERMODEDEVICEDIR, - *device_guid, - TAP_WIN_SUFFIX); - path = tuntap_device_path; } - tt->hand = CreateFile(path, - GENERIC_READ | GENERIC_WRITE, - 0, /* was: FILE_SHARE_READ */ - 0, - OPEN_EXISTING, - FILE_ATTRIBUTE_SYSTEM | FILE_FLAG_OVERLAPPED, - 0); - - if (tt->hand == INVALID_HANDLE_VALUE) - { - msg(D_TUNTAP_INFO, "CreateFile failed on %s device: %s", tt->wintun ? "wintun" : "TAP", tuntap_device_path); - } - else + if (tun_try_open_device(tt, *device_guid, device_instance_id_interface)) { break; } @@ -6261,7 +6288,7 @@ tun_open_device(struct tuntap *tt, const char *dev_node, const char **device_gui * GUID using the registry */ tt->actual_name = string_alloc(actual_buffer, NULL); - msg(M_INFO, "%s device [%s] opened: %s", tt->wintun ? "Wintun" : "TAP-WIN32", tt->actual_name, path); + msg(M_INFO, "%s device [%s] opened", tt->wintun ? "Wintun" : "TAP-WIN32", tt->actual_name); tt->adapter_index = get_adapter_index(*device_guid); gc_free(&gc); @@ -6372,14 +6399,7 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun tun_open_device(tt, dev_node, &device_guid); - if (tt->wintun) - { - if (!wintun_register_ring_buffer(tt)) - { - msg(M_FATAL, "Failed to register ring buffers"); - } - } - else + if (!tt->wintun) { tuntap_post_open(tt, device_guid); } From patchwork Fri Dec 20 16:11:15 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,v2,5/7] wintun: add support for --dev-node X-Patchwork-Submitter: Simon Rozman X-Patchwork-Id: 956 Message-Id: <20191220161117.1434-5-simon@rozman.si> To: openvpn-devel@lists.sourceforge.net Date: Fri, 20 Dec 2019 17:11:15 +0100 From: Simon Rozman List-Id: Signed-off-by: Simon Rozman Acked-by: Lev Stipakov --- src/openvpn/tun.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index f56682ef..18f06bb6 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -6196,7 +6196,6 @@ static void tun_open_device(struct tuntap *tt, const char *dev_node, const char **device_guid) { struct gc_arena gc = gc_new(); - char tuntap_device_path[256]; const struct tap_reg* tap_reg = get_tap_reg(&gc); const struct panel_reg* panel_reg = get_panel_reg(&gc); const struct device_instance_id_interface* device_instance_id_interface = get_device_instance_id_interface(&gc); @@ -6209,31 +6208,34 @@ tun_open_device(struct tuntap *tt, const char *dev_node, const char **device_gui */ if (dev_node) { + bool is_picked_device_wintun = false; + /* Get the device GUID for the device specified with --dev-node. */ - *device_guid = get_device_guid(dev_node, actual_buffer, sizeof(actual_buffer), NULL, tap_reg, panel_reg, &gc); + *device_guid = get_device_guid(dev_node, actual_buffer, sizeof(actual_buffer), &is_picked_device_wintun, tap_reg, panel_reg, &gc); if (!*device_guid) { - msg(M_FATAL, "TAP-Windows adapter '%s' not found", dev_node); + msg(M_FATAL, "Adapter '%s' not found", dev_node); } - /* Open Windows TAP-Windows adapter */ - openvpn_snprintf(tuntap_device_path, sizeof(tuntap_device_path), "%s%s%s", - USERMODEDEVICEDIR, - *device_guid, - TAP_WIN_SUFFIX); - - tt->hand = CreateFile(tuntap_device_path, - GENERIC_READ | GENERIC_WRITE, - 0, /* was: FILE_SHARE_READ */ - 0, - OPEN_EXISTING, - FILE_ATTRIBUTE_SYSTEM | FILE_FLAG_OVERLAPPED, - 0); + if (tt->wintun) + { + if (!is_picked_device_wintun) + { + msg(M_FATAL, "Adapter '%s' is TAP-Windows, Wintun expected. If you want to use this device, remove --windows-driver wintun.", dev_node); + } + } + else + { + if (is_picked_device_wintun) + { + msg(M_FATAL, "Adapter '%s' is Wintun, TAP-Windows expected. If you want to use this device, add --windows-driver wintun.", dev_node); + } + } - if (tt->hand == INVALID_HANDLE_VALUE) + if (!tun_try_open_device(tt, *device_guid, device_instance_id_interface)) { - msg(M_ERR, "CreateFile failed on TAP device: %s", tuntap_device_path); + msg(M_FATAL, "Failed to open %s adapter: %s", tt->wintun ? "Wintun" : "TAP-Windows", dev_node); } } else From patchwork Fri Dec 20 16:11:16 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,v2,6/7] wintun: stop sending TAP-Windows6 ioctls to NDIS device X-Patchwork-Submitter: Simon Rozman X-Patchwork-Id: 960 Message-Id: <20191220161117.1434-6-simon@rozman.si> To: openvpn-devel@lists.sourceforge.net Date: Fri, 20 Dec 2019 17:11:16 +0100 From: Simon Rozman List-Id: Wintun doesn't have its own I/O device. Rather, it taps on existing Windows-provided NDIS device. Sending TAP-Windows6 IOCTL requests to it is risky, as TAP-Windows6 is using one of the well-known device types (FILE_DEVICE_UNKNOWN) with function IDs as 1, 2, 3 etc. raising a chance of collision as NDIS might react to one of these IOCTLs. Signed-off-by: Simon Rozman --- src/openvpn/tun.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 18f06bb6..6762402c 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -6412,7 +6412,7 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun const char * tap_win_getinfo(const struct tuntap *tt, struct gc_arena *gc) { - if (tt && tt->hand != NULL) + if (tuntap_defined(tt) && !tt->wintun) { struct buffer out = alloc_buf_gc(256, gc); DWORD len; @@ -6430,7 +6430,7 @@ tap_win_getinfo(const struct tuntap *tt, struct gc_arena *gc) void tun_show_debug(struct tuntap *tt) { - if (tt && tt->hand != NULL) + if (tuntap_defined(tt) && !tt->wintun) { struct buffer out = alloc_buf(1024); DWORD len; From patchwork Fri Dec 20 16:11:17 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,v2,7/7] tun.c: reword the at_least_one_tap_win() error X-Patchwork-Submitter: Simon Rozman X-Patchwork-Id: 957 Message-Id: <20191220161117.1434-7-simon@rozman.si> To: openvpn-devel@lists.sourceforge.net Date: Fri, 20 Dec 2019 17:11:17 +0100 From: Simon Rozman List-Id: Signed-off-by: Simon Rozman Acked-by: Lev Stipakov --- src/openvpn/tun.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 6762402c..4e16f989 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -4031,7 +4031,7 @@ at_least_one_tap_win(const struct tap_reg *tap_reg) { if (!tap_reg) { - msg(M_FATAL, "There are no TAP-Windows adapters on this system. You should be able to create a TAP-Windows adapter by going to Start -> All Programs -> TAP-Windows -> Utilities -> Add a new TAP-Windows virtual ethernet adapter."); + msg(M_FATAL, "There are no TAP-Windows nor Wintun adapters on this system. You should be able to create an adapter by using tapctl.exe utility."); } }