From patchwork Thu Dec 19 22:39:11 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,1/7] tun.c: make Windows device lookup functions more general X-Patchwork-Submitter: Simon Rozman X-Patchwork-Id: 952 Message-Id: <20191219223917.1614-1-simon@rozman.si> To: openvpn-devel@lists.sourceforge.net Date: Thu, 19 Dec 2019 23:39: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..053a8232 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_tap_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_tap_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_tap_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_tap_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_tap_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_tap_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 Thu Dec 19 22:39:12 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,2/7] tun.c: upgrade get_device_guid() to return the Windows driver type X-Patchwork-Submitter: Simon Rozman X-Patchwork-Id: 950 Message-Id: <20191219223917.1614-2-simon@rozman.si> To: openvpn-devel@lists.sourceforge.net Date: Thu, 19 Dec 2019 23:39:12 +0100 From: Simon Rozman List-Id: Signed-off-by: Simon Rozman Acked-by: Lev Stipakov --- src/openvpn/tun.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 053a8232..623ed37b 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -4110,6 +4110,7 @@ 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 Thu Dec 19 22:39:13 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,3/7] tun.c: make wintun_register_ring_buffer() non-fatal on failures X-Patchwork-Submitter: Simon Rozman X-Patchwork-Id: 949 Message-Id: <20191219223917.1614-3-simon@rozman.si> To: openvpn-devel@lists.sourceforge.net Date: Thu, 19 Dec 2019 23:39: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 623ed37b..220dee87 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 Thu Dec 19 22:39:14 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,4/7] wintun: register ring buffers when iterating adapters X-Patchwork-Submitter: Simon Rozman X-Patchwork-Id: 948 Message-Id: <20191219223917.1614-4-simon@rozman.si> To: openvpn-devel@lists.sourceforge.net Date: Thu, 19 Dec 2019 23:39: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 | 110 +++++++++++++++++++++++++++------------------- 1 file changed, 64 insertions(+), 46 deletions(-) diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 220dee87..9dc9b3a2 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -6129,11 +6129,71 @@ 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]; + + 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; + } + + 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-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); + return false; + } + } + + 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 +6257,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 +6270,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 +6286,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 +6397,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 Thu Dec 19 22:39:15 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,5/7] wintun: add support for --dev-node X-Patchwork-Submitter: Simon Rozman X-Patchwork-Id: 947 Message-Id: <20191219223917.1614-5-simon@rozman.si> To: openvpn-devel@lists.sourceforge.net Date: Thu, 19 Dec 2019 23:39: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 9dc9b3a2..8508b9c0 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -6194,7 +6194,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); @@ -6207,31 +6206,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", dev_node); + } + } + else + { + if (is_picked_device_wintun) + { + msg(M_FATAL, "Adapter '%s' is Wintun, TAP-Windows expected", 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 Thu Dec 19 22:39:16 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,6/7] wintun: stop sending TAP-Windows6 ioctls to NDIS device X-Patchwork-Submitter: Simon Rozman X-Patchwork-Id: 953 Message-Id: <20191219223917.1614-6-simon@rozman.si> To: openvpn-devel@lists.sourceforge.net Date: Thu, 19 Dec 2019 23:39: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/error.c | 5 ++++- src/openvpn/forward.c | 2 +- src/openvpn/sig.c | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/openvpn/error.c b/src/openvpn/error.c index b2492f2b..8d91a131 100644 --- a/src/openvpn/error.c +++ b/src/openvpn/error.c @@ -688,7 +688,10 @@ x_check_status(int status, } #elif defined(_WIN32) /* get possible driver error from TAP-Windows driver */ - extended_msg = tap_win_getinfo(tt, &gc); + if (!tt->wintun) + { + extended_msg = tap_win_getinfo(tt, &gc); + } #endif if (!ignore_sys_error(my_errno)) { diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 6b823613..2bc9d871 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -1956,7 +1956,7 @@ pre_select(struct context *c) if (check_debug_level(D_TAP_WIN_DEBUG)) { c->c2.timeval.tv_sec = 1; - if (tuntap_defined(c->c1.tuntap)) + if (tuntap_defined(c->c1.tuntap) && !c->c1.tuntap->wintun) { tun_show_debug(c->c1.tuntap); } diff --git a/src/openvpn/sig.c b/src/openvpn/sig.c index d7f2abb8..f02aa57c 100644 --- a/src/openvpn/sig.c +++ b/src/openvpn/sig.c @@ -315,7 +315,7 @@ print_status(const struct context *c, struct status_output *so) status_printf(so, "Post-decrypt truncations," counter_format, c->c2.n_trunc_post_decrypt); #endif #ifdef _WIN32 - if (tuntap_defined(c->c1.tuntap)) + if (tuntap_defined(c->c1.tuntap) && !c->c1.tuntap->wintun) { status_printf(so, "TAP-WIN32 driver status,\"%s\"", tap_win_getinfo(c->c1.tuntap, &gc)); From patchwork Thu Dec 19 22:39:17 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,7/7] tun.c: reword the at_least_one_tap_win() error X-Patchwork-Submitter: Simon Rozman X-Patchwork-Id: 951 Message-Id: <20191219223917.1614-7-simon@rozman.si> To: openvpn-devel@lists.sourceforge.net Date: Thu, 19 Dec 2019 23:39: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 8508b9c0..14ff0259 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."); } }