[Openvpn-devel,v2,6/7] wintun: stop sending TAP-Windows6 ioctls to NDIS device

Message ID 20191220161117.1434-6-simon@rozman.si
State Superseded
Headers show
Series [Openvpn-devel,v2,1/7] tun.c: make Windows device lookup functions more general | expand

Commit Message

Simon Rozman Dec. 20, 2019, 5:11 a.m. UTC
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 <simon@rozman.si>
---
 src/openvpn/tun.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Lev Stipakov Dec. 21, 2019, 12:31 a.m. UTC | #1
Hi,

Sorry for not mentioning it earlier.

There is already check in forward.c:

    if (tuntap_defined(c->c1.tuntap))
    {
        tun_show_debug(c->c1.tuntap);
    }

Same goes for tap_win_getinfo in sig.c:

    if (tuntap_defined(c->c1.tuntap))
    {
        status_printf(so, "TAP-WIN32 driver status,\"%s\"",
                      tap_win_getinfo(c->c1.tuntap, &gc));
    }

There is one place where that check is missing, error.c:

    #elif defined(_WIN32)
        /* get possible driver error from TAP-Windows driver */
        extended_msg = tap_win_getinfo(tt, &gc);
    #endif

There is no need to call tuntap_defined() twice, so either
we remove it from calling code or from tun_* methods.
For the sake of consistency I propose to add it to error.c
and remove from tun_* methods, which would just check for !tt->wintun.

Patch

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;