[Openvpn-devel,v2,3/7] wintun: implement opening wintun device

Message ID 1573148729-27339-4-git-send-email-lstipakov@gmail.com
State Accepted
Headers show
Series
  • Wintun support
Related show

Commit Message

Lev Stipakov Nov. 7, 2019, 5:45 p.m.
From: Lev Stipakov <lev@openvpn.net>

To open wintun device, we cannot use "\\.\Global\Wintun<luid>"
path as before. To get device path which we supply to CreateFile,
we have to use SetupAPI to:

 - enumerate network adapters with "wintun" as component id
 - for each adapter save its guid
 - open device information set
 - for each item in set
   - open corresponding registry key to get net_cfg_instance_id
   - get symbolic link name of device interface by instance id
 - path will be symbolic link name of device instance matched with adapter's guid

See https://github.com/OpenVPN/openvpn3/blob/master/openvpn/tun/win/tunutil.hpp and
https://github.com/WireGuard/wireguard-go/blob/master/tun/wintun/wintun_windows.go for
implementation examples.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
---
 src/openvpn/Makefile.am     |   2 +-
 src/openvpn/openvpn.vcxproj |   6 +-
 src/openvpn/tun.c           | 244 +++++++++++++++++++++++++++++++++++++-------
 src/openvpn/tun.h           |  14 +++
 4 files changed, 223 insertions(+), 43 deletions(-)

Comments

Simon Rozman Nov. 9, 2019, 6:45 a.m. | #1
Hi,

> -----Original Message-----
> From: Lev Stipakov [mailto:lstipakov@gmail.com]
> Sent: Thursday, November 7, 2019 6:45 PM
> To: openvpn-devel@lists.sourceforge.net
> Cc: Lev Stipakov <lev@openvpn.net>
> Subject: [Openvpn-devel] [PATCH v2 3/7] wintun: implement opening wintun
> device
> 
> +const static GUID GUID_DEVCLASS_NET = { 0x4d36e972L, 0xe325, 0x11ce, {
> +0xbf, 0xc1, 0x08, 0x00, 0x2b, 0xe1, 0x03, 0x18 } }; const static GUID
> +GUID_DEVINTERFACE_NET = { 0xcac88484, 0x7
515, 0x4c03, { 0x82, 0xe6,
> +0x71, 0xa8, 0x7a, 0xba, 0xc3, 0x61 } };
> +

GUID_DEVCLASS_NET is declared in devguid.h, GUID_DEVINTERFACE_NET in
ndisguid.h... No need to redefine them. However, while one could include
those SDK files, one needs to add the appropriate .lib files too. It's not
worth complicating for just a couple of GUIDs that will never ever change.
So, ACK.

The rest LGTM.

Acked-by: Simon Rozman <simon@rozman.si>

Best regards,
Simon
Gert Doering Nov. 9, 2019, 3:25 p.m. | #2
Your patch has been applied to the master branch.

Since this introduces a new library requirement, I gave it a test run 
on my Ubuntu 16 / MinGW linux build system.  "Builds fine".  Have not run
the result.

I have not done anything resembling proper code review.  Just basic 
"this is only changing _WIN32 code, and doesn't seem to do bad stuff 
with pointers".

What I do not really like is the inflation of the code with 

	if (!tt->wintun)

statements now.  I think this should be refactored out into an
"open_tun_wintun()" and an "open_tun_tap_windows()" function, where
all the bits that are now inside an "if (!tt->wintun)" get their
own function with less if() and less nesting.


commit e64b4a9e68bde3cb7d878d277878fb2805040e3e
Author: Lev Stipakov
Date:   Thu Nov 7 19:45:25 2019 +0200

     wintun: implement opening wintun device

     Signed-off-by: Lev Stipakov <lev@openvpn.net>
     Acked-by: Simon Rozman <simon@rozman.si>
     Message-Id: <1573148729-27339-4-git-send-email-lstipakov@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19029.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
Lev Stipakov Nov. 13, 2019, 10:48 a.m. | #3
Hi,


> What I do not really like is the inflation of the code with
>
>         if (!tt->wintun)
>
> statements now.  I think this should be refactored out into an
> "open_tun_wintun()" and an "open_tun_tap_windows()" function, where
> all the bits that are now inside an "if (!tt->wintun)" get their
> own function with less if() and less nesting.
>

I agree.

I have sent a (yet another) patch (https://patchwork.openvpn.net/patch/918/
)
which refactors open_tun() method, which now looks like this:

    tun_open_device(tt, dev_node, &device_guid);

    if (tt->wintun)
    {
        wintun_register_ring_buffer(tt);
    }
    else
    {
        tuntap_post_open(tt, device_guid);
    }

Note that it requires 6 of 7 wintun patches to be merged.

Alternatively I can rebase wintun 4/5/6 patches on top of this one.
Selva Nair Nov. 19, 2019, 4:29 a.m. | #4
Hi,

On Thu, Nov 7, 2019 at 12:49 PM Lev Stipakov <lstipakov@gmail.com> wrote:

> From: Lev Stipakov <lev@openvpn.net>
>
> To open wintun device, we cannot use "\\.\Global\Wintun<luid>"
> path as before. To get device path which we supply to CreateFile,
> we have to use SetupAPI to:
>
>  - enumerate network adapters with "wintun" as component id
>  - for each adapter save its guid
>  - open device information set
>  - for each item in set
>    - open corresponding registry key to get net_cfg_instance_id
>    - get symbolic link name of device interface by instance id
>  - path will be symbolic link name of device instance matched with
> adapter's guid
>
> See
> https://github.com/OpenVPN/openvpn3/blob/master/openvpn/tun/win/tunutil.hpp
> and
>
> https://github.com/WireGuard/wireguard-go/blob/master/tun/wintun/wintun_windows.go
> for
> implementation examples.
>
> Signed-off-by: Lev Stipakov <lev@openvpn.net>
>

This also has been ACKed and merged,  but two questions that may need some
attention:

@@ -5541,7 +5668,8 @@ void
>  open_tun(const char *dev, const char *dev_type, const char *dev_node,
> struct tuntap *tt)
>  {
>      struct gc_arena gc = gc_new();
> -    char device_path[256];
> +    char tuntap_device_path[256];
> +    char *path = NULL;
>      const char *device_guid = NULL;
>      DWORD len;
>      bool dhcp_masq = false;
> @@ -5571,6 +5699,8 @@ open_tun(const char *dev, const char *dev_type,
> const char *dev_node, struct tun
>      {
>          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);
> +
>          char actual_buffer[256];
>
>          at_least_one_tap_win(tap_reg);
> @@ -5586,24 +5716,22 @@ open_tun(const char *dev, const char *dev_type,
> const char *dev_node, struct tun
>              }
>
>              /* Open Windows TAP-Windows adapter */
> -            openvpn_snprintf(device_path, sizeof(device_path), "%s%s%s",
> +            openvpn_snprintf(tuntap_device_path,
> sizeof(tuntap_device_path), "%s%s%s",
>                               USERMODEDEVICEDIR,
>                               device_guid,
>                               TAP_WIN_SUFFIX);
>
> -            tt->hand = CreateFile(
> -                device_path,
> -                GENERIC_READ | GENERIC_WRITE,
> -                0,                /* was: FILE_SHARE_READ */
> -                0,
> -                OPEN_EXISTING,
> -                FILE_ATTRIBUTE_SYSTEM | FILE_FLAG_OVERLAPPED,
> -                0
> -                );
> +            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->hand == INVALID_HANDLE_VALUE)
>              {
> -                msg(M_ERR, "CreateFile failed on TAP device: %s",
> device_path);
> +                msg(M_ERR, "CreateFile failed on TAP device: %s",
> tuntap_device_path);
>

Doesn't this mean that  if --dev-node is specified, we'll open  tapwindows
adapter
with that name even if "--window-driver wintun" is specified? The open may
succeed
but subsequent wintun-specific processing will fail.

             }
>          }
>          else
> @@ -5613,43 +5741,78 @@ open_tun(const char *dev, const char *dev_type,
> const char *dev_node, struct tun
>              /* Try opening all TAP devices until we find one available */
>              while (true)
>              {
> +                bool is_picked_device_wintun = false;
>                  device_guid = get_unspecified_device_guid(device_number,
>                                                            actual_buffer,
>
>  sizeof(actual_buffer),
>                                                            tap_reg,
>                                                            panel_reg,
> +
> &is_picked_device_wintun,
>                                                            &gc);
>
>                  if (!device_guid)
>                  {
> -                    msg(M_FATAL, "All TAP-Windows adapters on this system
> are currently in use.");
> +                    msg(M_FATAL, "All %s adapters on this system are
> currently in use.", tt->wintun ? "wintun" : "TAP - Windows");
>

If I'm not mistaken wintun device can be opened multiple times, so we'll
never get the
"All wintun adapters on this system...." error. Instead, open will succeed
here and
something else may fail later. FILE_SHARE_READ = 0 will not save us when
the driver
does not enforce it.

Selva
<div dir="ltr"><div>Hi,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Nov 7, 2019 at 12:49 PM Lev Stipakov &lt;<a href="mailto:lstipakov@gmail.com" target="_blank">lstipakov@gmail.com</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">From: Lev Stipakov &lt;<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>&gt;<br>
<br>
To open wintun device, we cannot use &quot;\\.\Global\Wintun&lt;luid&gt;&quot;<br>
path as before. To get device path which we supply to CreateFile,<br>
we have to use SetupAPI to:<br>
<br>
 - enumerate network adapters with &quot;wintun&quot; as component id<br>
 - for each adapter save its guid<br>
 - open device information set<br>
 - for each item in set<br>
   - open corresponding registry key to get net_cfg_instance_id<br>
   - get symbolic link name of device interface by instance id<br>
 - path will be symbolic link name of device instance matched with adapter&#39;s guid<br>
<br>
See <a href="https://github.com/OpenVPN/openvpn3/blob/master/openvpn/tun/win/tunutil.hpp" rel="noreferrer" target="_blank">https://github.com/OpenVPN/openvpn3/blob/master/openvpn/tun/win/tunutil.hpp</a> and<br>
<a href="https://github.com/WireGuard/wireguard-go/blob/master/tun/wintun/wintun_windows.go" rel="noreferrer" target="_blank">https://github.com/WireGuard/wireguard-go/blob/master/tun/wintun/wintun_windows.go</a> for<br>
implementation examples.<br>
<br>
Signed-off-by: Lev Stipakov &lt;<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>&gt;<br></blockquote><div><br></div><div>This also has been ACKed and merged,  but two questions that may need some attention:<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
@@ -5541,7 +5668,8 @@ void<br>
 open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt)<br>
 {<br>
     struct gc_arena gc = gc_new();<br>
-    char device_path[256];<br>
+    char tuntap_device_path[256];<br>
+    char *path = NULL;<br>
     const char *device_guid = NULL;<br>
     DWORD len;<br>
     bool dhcp_masq = false;<br>
@@ -5571,6 +5699,8 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun<br>
     {<br>
         const struct tap_reg *tap_reg = get_tap_reg(&amp;gc);<br>
         const struct panel_reg *panel_reg = get_panel_reg(&amp;gc);<br>
+        const struct device_instance_id_interface *device_instance_id_interface = get_device_instance_id_interface(&amp;gc);<br>
+<br>
         char actual_buffer[256];<br>
<br>
         at_least_one_tap_win(tap_reg);<br>
@@ -5586,24 +5716,22 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun<br>
             }<br>
<br>
             /* Open Windows TAP-Windows adapter */<br>
-            openvpn_snprintf(device_path, sizeof(device_path), &quot;%s%s%s&quot;,<br>
+            openvpn_snprintf(tuntap_device_path, sizeof(tuntap_device_path), &quot;%s%s%s&quot;,<br>
                              USERMODEDEVICEDIR,<br>
                              device_guid,<br>
                              TAP_WIN_SUFFIX);<br>
<br>
-            tt-&gt;hand = CreateFile(<br>
-                device_path,<br>
-                GENERIC_READ | GENERIC_WRITE,<br>
-                0,                /* was: FILE_SHARE_READ */<br>
-                0,<br>
-                OPEN_EXISTING,<br>
-                FILE_ATTRIBUTE_SYSTEM | FILE_FLAG_OVERLAPPED,<br>
-                0<br>
-                );<br>
+            tt-&gt;hand = CreateFile(tuntap_device_path,<br>
+                                  GENERIC_READ | GENERIC_WRITE,<br>
+                                  0,                /* was: FILE_SHARE_READ */<br>
+                                  0,<br>
+                                  OPEN_EXISTING,<br>
+                                  FILE_ATTRIBUTE_SYSTEM | FILE_FLAG_OVERLAPPED,<br>
+                                  0);<br>
<br>
             if (tt-&gt;hand == INVALID_HANDLE_VALUE)<br>
             {<br>
-                msg(M_ERR, &quot;CreateFile failed on TAP device: %s&quot;, device_path);<br>
+                msg(M_ERR, &quot;CreateFile failed on TAP device: %s&quot;, tuntap_device_path);<br></blockquote><div><br></div><div>Doesn&#39;t this mean that  if --dev-node is specified, we&#39;ll open  tapwindows adapter</div><div>with that name even if &quot;--window-driver wintun&quot; is specified? The open may succeed</div><div>but subsequent wintun-specific processing will fail.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
             }<br>
         }<br>
         else<br>
@@ -5613,43 +5741,78 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun<br>
             /* Try opening all TAP devices until we find one available */<br>
             while (true)<br>
             {<br>
+                bool is_picked_device_wintun = false;<br>
                 device_guid = get_unspecified_device_guid(device_number,<br>
                                                           actual_buffer,<br>
                                                           sizeof(actual_buffer),<br>
                                                           tap_reg,<br>
                                                           panel_reg,<br>
+                                                          &amp;is_picked_device_wintun,<br>
                                                           &amp;gc);<br>
<br>
                 if (!device_guid)<br>
                 {<br>
-                    msg(M_FATAL, &quot;All TAP-Windows adapters on this system are currently in use.&quot;);<br>
+                    msg(M_FATAL, &quot;All %s adapters on this system are currently in use.&quot;, tt-&gt;wintun ? &quot;wintun&quot; : &quot;TAP - Windows&quot;);<br></blockquote><div><br></div><div>If I&#39;m not mistaken wintun device can be opened multiple times, so we&#39;ll never get the<br>&quot;All wintun adapters on this system....&quot; error. Instead, open will succeed here and </div><div>something else may fail later. FILE_SHARE_READ = 0 will not save us when the driver</div><div>does not enforce it.</div><div><br></div><div>Selva</div></div></div>
Lev Stipakov Nov. 19, 2019, 8:50 a.m. | #5
Hi,

Doesn't this mean that  if --dev-node is specified, we'll open  tapwindows
> adapter
> with that name even if "--window-driver wintun" is specified? The open may
> succeed
> but subsequent wintun-specific processing will fail.
>

--dev-node is not (yet) supported and open will fail (just re-tested).


> If I'm not mistaken wintun device can be opened multiple times, so we'll
> never get the
> "All wintun adapters on this system...." error.
>

Well, we get it if wintun driver is not installed :) but yep, open will
succeed.

Instead, open will succeed here and something else may fail later.
>

Right after opening we register ring buffers, and that will fail (with a
friendly message):

ERROR:  Failed to register ring buffers: 1247

We should probably make it even more user friendly and write something like
"make sure wintun adapter is not in use".

Wireguard creates wintun adapter on demand (on connect) and openvpn during
installation and then
picks the first one found on connect. Maybe that logic could be more clever
to not to pick adapter which
is currently used by wireguard.

I haven't looked yet into running wireguard and openvpn side by side (or
multiple openvpn instances
with wintun), just tested that wg and openvpn could co-exist without
problems on the same machine.
Selva Nair Nov. 19, 2019, 3:27 p.m. | #6
Hi,

On Tue, Nov 19, 2019 at 3:50 AM Lev Stipakov <lstipakov@gmail.com> wrote:

> Hi,
>
> Doesn't this mean that  if --dev-node is specified, we'll open  tapwindows
>> adapter
>> with that name even if "--window-driver wintun" is specified? The open
>> may succeed
>> but subsequent wintun-specific processing will fail.
>>
>
> --dev-node is not (yet) supported and open will fail (just re-tested).
>

I did a quick test and found the open to succeed with a misleading message
that Wintun device opened (though it opened the tap device) and then fails
on register ring buffers. I haven't applied all patches as yet, so not sure
whether this is changed in later patches.

If I'm not mistaken wintun device can be opened multiple times, so we'll
>> never get the
>> "All wintun adapters on this system...." error.
>>
>
Well, we get it if wintun driver is not installed :) but yep, open will
> succeed.
>

Instead, open will succeed here and something else may fail later.
>>
>
> Right after opening we register ring buffers, and that will fail (with a
> friendly message):
>
> ERROR:  Failed to register ring buffers: 1247
>
> We should probably make it even more user friendly and write something
> like "make sure wintun adapter is not in use".
>

Apart from the error message, there is a larger issue especially when we
use iservice. In that case, we have to preserve privilege separation and
allowing a user to open a device handle in use by another has to be
avoided. The service could keep a lock on devices it manages, but I have no
idea how we are going to lock out devices opened by other means (say
instances started by automatic service, or wireguard or some other third
party product).

Can we get the wintun folks to support exclusive open (say FILE_SHARE_READ
= 0) or expose the internal count of number of open handles? That would
also make it easier to fix this and provide better error messages.


> Wireguard creates wintun adapter on demand (on connect) and openvpn during
> installation and then
> picks the first one found on connect. Maybe that logic could be more
> clever to not to pick adapter which
> is currently used by wireguard.
>
> I haven't looked yet into running wireguard and openvpn side by side (or
> multiple openvpn instances
> with wintun), just tested that wg and openvpn could co-exist without
> problems on the same machine.
>

Hmm.. if multiple openvpn instances are not tested this is not ready for
review yet, is it?

Again, a quick test shows that, with multiple openvpn instances, it does
open an already opened device and then fails while registering ring buffers.

Selva
<div dir="ltr"><div>Hi,<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Nov 19, 2019 at 3:50 AM Lev Stipakov &lt;<a href="mailto:lstipakov@gmail.com">lstipakov@gmail.com</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">Hi,</div><div dir="ltr"><br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>Doesn&#39;t this mean that  if --dev-node is specified, we&#39;ll open  tapwindows adapter</div><div>with that name even if &quot;--window-driver wintun&quot; is specified? The open may succeed</div><div>but subsequent wintun-specific processing will fail.</div></div></div></blockquote><div><br></div><div>--dev-node is not (yet) supported and open will fail (just re-tested).</div></div></div></blockquote><div><br></div><div class="gmail_quote">I did a quick test and found the open to succeed with a misleading message that Wintun device opened (though it opened the tap device) and then fails on register ring buffers. I haven&#39;t applied all patches as yet, so not sure whether this is changed in later patches.<br></div><div class="gmail_quote"><br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>If I&#39;m not mistaken wintun device can be opened multiple times, so we&#39;ll never get the<br>&quot;All wintun adapters on this system....&quot; error.</div></div></div></blockquote></blockquote><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>Well, we get it if wintun driver is not installed :) but yep, open will succeed.</div></blockquote><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>Instead, open will succeed here and something else may fail later.</div></blockquote></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><br></div><div>Right after opening we register ring buffers, and that will fail (with a friendly message):</div><div><br></div><div>ERROR:  Failed to register ring buffers: 1247</div><div><br></div><div>We should probably make it even more user friendly and write something like &quot;make sure wintun adapter is not in use&quot;.</div></blockquote></div><div><br></div><div>Apart from the error message, there is a larger issue especially when we use iservice. In that case, we have to preserve privilege separation and allowing a user to open a device handle in use by another has to be avoided. The service could keep a lock on devices it manages, but I have no idea how we are going to lock out devices opened by other means (say instances started by automatic service, or wireguard or some other third party product).<br></div><div><br></div><div>Can we get the wintun folks to support exclusive open (say FILE_SHARE_READ = 0) or expose the internal count of number of open handles? That would also make it easier to fix this and provide better error messages.<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><br></div><div>Wireguard creates wintun adapter on demand (on connect) and openvpn during installation and then</div><div>picks the first one found on connect. Maybe that logic could be more clever to not to pick adapter which</div><div>is currently used by wireguard.</div><div><br></div><div>I haven&#39;t looked yet into running wireguard and openvpn side by side (or multiple openvpn instances</div><div>with wintun), just tested that wg and openvpn could co-exist without problems on the same machine.</div></div></div></blockquote><div><br></div><div>Hmm.. if multiple openvpn instances are not tested this is not ready for review yet, is it?<br></div><div><br></div><div>Again, a quick test shows that, with multiple openvpn instances, it does open an already opened device and then fails while registering ring buffers.</div><div><br></div><div>Selva<br></div></div></div>
Lev Stipakov Nov. 19, 2019, 5:23 p.m. | #7
Hi,

Apart from the error message, there is a larger issue especially when we
> use iservice. In that case, we have to preserve privilege separation and
> allowing a user to open a device handle in use by another has to be avoided.
>

Do you see it as a security issue when handle can be opened by another
process?

To read / write to tunnel one needs to register ring buffers, and this call
will fail for any other process. Am I missing something here?


> Can we get the wintun folks to support exclusive open (say FILE_SHARE_READ
> = 0) or expose the internal count of number of open handles? That would
> also make it easier to fix this and provide better error messages.
>

We could maybe submit a patch for that.

I agree it would be nice to have.


> Hmm.. if multiple openvpn instances are not tested this is not ready for
> review yet, is it?
>

I don't think so. I would not expect new tun driver (or code around it) to
support all features of old driver right from the beginning.

Wintun doesn't replace tap-windows6, so if someone requires functionality
which wintun doesn't yet support, one could always switch to previous
driver.

Again, a quick test shows that, with multiple openvpn instances, it does
> open an already opened device and then fails while registering ring buffers.
>

Yeah, it would be nice to eventual support multiple tunnels, but I don't
think that this is "must have" in the first version.
<div dir="auto"><div dir="ltr"><div>Hi,</div><div><br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>Apart from the error message, there is a larger issue especially when we use iservice. In that case, we have to preserve privilege separation and allowing a user to open a device handle in use by another has to be avoided.</div></div></div></blockquote><div><br></div><div>Do you see it as a security issue when handle can be opened by another process?</div><div><br></div><div>To read / write to tunnel one needs to register ring buffers, and this call will fail for any other process. Am I missing something here?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>Can we get the wintun folks to support exclusive open (say FILE_SHARE_READ = 0) or expose the internal count of number of open handles? That would also make it easier to fix this and provide better error messages.</div></div></div></blockquote><div><br></div><div>We could maybe submit a patch for that.</div><div dir="auto"><br></div><div dir="auto">I agree it would be nice to have.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>Hmm.. if multiple openvpn instances are not tested this is not ready for review yet, is it?<br></div></div></div></blockquote></div></div><div dir="auto"><br></div><div dir="auto">I don&#39;t think so. I would not expect new tun driver (or code around it) to support all features of old driver right from the beginning. </div><div dir="auto"><br></div><div dir="auto">Wintun doesn&#39;t replace tap-windows6, so if someone requires functionality which wintun doesn&#39;t yet support, one could always switch to previous driver. </div><div dir="auto"><br></div><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>Again, a quick test shows that, with multiple openvpn instances, it does open an already opened device and then fails while registering ring buffers.</div></div></div></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Yeah, it would be nice to eventual support multiple tunnels, but I don&#39;t think that this is &quot;must have&quot; in the first version. </div></div>
Selva Nair Nov. 19, 2019, 6:03 p.m. | #8
Hi Lev,

On Tue, Nov 19, 2019 at 12:23 PM Lev Stipakov <lstipakov@gmail.com> wrote:

> Hi,
>
> Apart from the error message, there is a larger issue especially when we
>> use iservice. In that case, we have to preserve privilege separation and
>> allowing a user to open a device handle in use by another has to be avoided.
>>
>
> Do you see it as a security issue when handle can be opened by another
> process?
>

I don't know the internals of wintun to know that for sure.


>
> To read / write to tunnel one needs to register ring buffers, and this
> call will fail for any other process. Am I missing something here?
>

Hopefully, Simon can confirm whether that provides a sufficient safety net.

Selva
<div dir="ltr"><div>Hi Lev,<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Nov 19, 2019 at 12:23 PM Lev Stipakov &lt;<a href="mailto:lstipakov@gmail.com" target="_blank">lstipakov@gmail.com</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="auto"><div dir="ltr"><div>Hi,</div><div><br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>Apart from the error message, there is a larger issue especially when we use iservice. In that case, we have to preserve privilege separation and allowing a user to open a device handle in use by another has to be avoided.</div></div></div></blockquote><div><br></div><div>Do you see it as a security issue when handle can be opened by another process?</div></div></div></div></blockquote><div><br></div><div>I don&#39;t know the internals of wintun to know that for sure. <br></div><div>  <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="auto"><div dir="ltr"><div class="gmail_quote"><div><br></div><div>To read / write to tunnel one needs to register ring buffers, and this call will fail for any other process. Am I missing something here?</div></div></div></div></blockquote><div><br></div><div>Hopefully, Simon can confirm whether that provides a sufficient safety net.<br></div></div><div class="gmail_quote"><br></div><div class="gmail_quote">Selva<br></div></div>

Patch

diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index fbb86ad..a091ffc 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -139,5 +139,5 @@  openvpn_LDADD = \
 	$(OPTIONAL_DL_LIBS)
 if WIN32
 openvpn_SOURCES += openvpn_win32_resources.rc block_dns.c block_dns.h
-openvpn_LDADD += -lgdi32 -lws2_32 -lwininet -lcrypt32 -liphlpapi -lwinmm -lfwpuclnt -lrpcrt4 -lncrypt
+openvpn_LDADD += -lgdi32 -lws2_32 -lwininet -lcrypt32 -liphlpapi -lwinmm -lfwpuclnt -lrpcrt4 -lncrypt -lsetupapi
 endif
diff --git a/src/openvpn/openvpn.vcxproj b/src/openvpn/openvpn.vcxproj
index e77f026..9ffef9f 100644
--- a/src/openvpn/openvpn.vcxproj
+++ b/src/openvpn/openvpn.vcxproj
@@ -91,7 +91,7 @@ 
     </ClCompile>
     <ResourceCompile />
     <Link>
-      <AdditionalDependencies>legacy_stdio_definitions.lib;Ncrypt.lib;libssl.lib;libcrypto.lib;lzo2.lib;pkcs11-helper.dll.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;%(AdditionalDependencies)</AdditionalDependencies>
+      <AdditionalDependencies>legacy_stdio_definitions.lib;Ncrypt.lib;libssl.lib;libcrypto.lib;lzo2.lib;pkcs11-helper.dll.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;setupapi.lib;%(AdditionalDependencies)</AdditionalDependencies>
       <AdditionalLibraryDirectories>$(OPENSSL_HOME)/lib;$(LZO_HOME)/lib;$(PKCS11H_HOME)/lib;%(AdditionalLibraryDirectories)</AdditionalLibraryDirectories>
       <SubSystem>Console</SubSystem>
     </Link>
@@ -117,7 +117,7 @@ 
     </ClCompile>
     <ResourceCompile />
     <Link>
-      <AdditionalDependencies>legacy_stdio_definitions.lib;Ncrypt.lib;libssl.lib;libcrypto.lib;lzo2.lib;pkcs11-helper.dll.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;%(AdditionalDependencies)</AdditionalDependencies>
+      <AdditionalDependencies>legacy_stdio_definitions.lib;Ncrypt.lib;libssl.lib;libcrypto.lib;lzo2.lib;pkcs11-helper.dll.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;setupapi.lib;%(AdditionalDependencies)</AdditionalDependencies>
       <AdditionalLibraryDirectories>$(OPENSSL_HOME)/lib;$(LZO_HOME)/lib;$(PKCS11H_HOME)/lib;%(AdditionalLibraryDirectories)</AdditionalLibraryDirectories>
       <SubSystem>Console</SubSystem>
     </Link>
@@ -301,4 +301,4 @@ 
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
   <ImportGroup Label="ExtensionTargets">
   </ImportGroup>
-</Project>
\ No newline at end of file
+</Project>
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index ce23eb6..37bf065 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -58,6 +58,9 @@ 
 
 #ifdef _WIN32
 
+const static GUID GUID_DEVCLASS_NET = { 0x4d36e972L, 0xe325, 0x11ce, { 0xbf, 0xc1, 0x08, 0x00, 0x2b, 0xe1, 0x03, 0x18 } };
+const static GUID GUID_DEVINTERFACE_NET = { 0xcac88484, 0x7515, 0x4c03, { 0x82, 0xe6, 0x71, 0xa8, 0x7a, 0xba, 0xc3, 0x61 } };
+
 /* #define SIMULATE_DHCP_FAILED */       /* simulate bad DHCP negotiation */
 
 #define NI_TEST_FIRST  (1<<0)
@@ -3444,7 +3447,123 @@  tun_finalize(
     return ret;
 }
 
-const struct tap_reg *
+static const struct device_instance_id_interface *
+get_device_instance_id_interface(struct gc_arena* gc)
+{
+    HDEVINFO dev_info_set;
+    DWORD err;
+    struct device_instance_id_interface *first = NULL;
+    struct device_instance_id_interface *last = NULL;
+
+    dev_info_set = SetupDiGetClassDevsEx(&GUID_DEVCLASS_NET, NULL, NULL, DIGCF_PRESENT, NULL, NULL, NULL);
+    if (dev_info_set == INVALID_HANDLE_VALUE)
+    {
+        err = GetLastError();
+        msg(M_FATAL, "Error [%u] opening device information set key: %s", (unsigned int)err, strerror_win32(err, gc));
+    }
+
+    for (DWORD i = 0;; ++i)
+    {
+        SP_DEVINFO_DATA device_info_data;
+        BOOL res;
+        HKEY dev_key;
+        char net_cfg_instance_id_string[] = "NetCfgInstanceId";
+        char net_cfg_instance_id[256];
+        char device_instance_id[256];
+        DWORD len;
+        DWORD data_type;
+        LONG status;
+        ULONG dev_interface_list_size;
+        CONFIGRET cr;
+        struct buffer dev_interface_list;
+
+        ZeroMemory(&device_info_data, sizeof(SP_DEVINFO_DATA));
+        device_info_data.cbSize = sizeof(SP_DEVINFO_DATA);
+        res = SetupDiEnumDeviceInfo(dev_info_set, i, &device_info_data);
+        if (!res)
+        {
+            if (GetLastError() == ERROR_NO_MORE_ITEMS)
+            {
+                break;
+            }
+            else
+            {
+                continue;
+            }
+        }
+
+        dev_key = SetupDiOpenDevRegKey(dev_info_set, &device_info_data, DICS_FLAG_GLOBAL, 0, DIREG_DRV, KEY_QUERY_VALUE);
+        if (dev_key == INVALID_HANDLE_VALUE)
+        {
+            continue;
+        }
+
+        len = sizeof(net_cfg_instance_id);
+        data_type = REG_SZ;
+        status = RegQueryValueEx(dev_key,
+                                 net_cfg_instance_id_string,
+                                 NULL,
+                                 &data_type,
+                                 net_cfg_instance_id,
+                                 &len);
+        if (status != ERROR_SUCCESS)
+        {
+            goto next;
+        }
+
+        len = sizeof(device_instance_id);
+        res = SetupDiGetDeviceInstanceId(dev_info_set, &device_info_data, device_instance_id, len, &len);
+        if (!res)
+        {
+            goto next;
+        }
+
+        cr = CM_Get_Device_Interface_List_Size(&dev_interface_list_size,
+                                               (LPGUID)& GUID_DEVINTERFACE_NET,
+                                               device_instance_id,
+                                               CM_GET_DEVICE_INTERFACE_LIST_PRESENT);
+
+        if (cr != CR_SUCCESS)
+        {
+            goto next;
+        }
+
+        dev_interface_list = alloc_buf_gc(dev_interface_list_size, gc);
+        cr = CM_Get_Device_Interface_List((LPGUID)& GUID_DEVINTERFACE_NET, device_instance_id,
+                                          BPTR(&dev_interface_list),
+                                          dev_interface_list_size,
+                                          CM_GET_DEVICE_INTERFACE_LIST_PRESENT);
+        if (cr != CR_SUCCESS)
+        {
+            goto next;
+        }
+
+        struct device_instance_id_interface* dev_if;
+        ALLOC_OBJ_CLEAR_GC(dev_if, struct device_instance_id_interface, gc);
+        dev_if->net_cfg_instance_id = string_alloc(net_cfg_instance_id, gc);
+        dev_if->device_interface_list = string_alloc(BSTR(&dev_interface_list), gc);
+
+        /* link into return list */
+        if (!first)
+        {
+            first = dev_if;
+        }
+        if (last)
+        {
+            last->next = dev_if;
+        }
+        last = dev_if;
+
+    next:
+        RegCloseKey(dev_key);
+    }
+
+    SetupDiDestroyDeviceInfoList(dev_info_set);
+
+    return first;
+}
+
+static const struct tap_reg *
 get_tap_reg(struct gc_arena *gc)
 {
     HKEY adapter_key;
@@ -3541,11 +3660,13 @@  get_tap_reg(struct gc_arena *gc)
                 if (status == ERROR_SUCCESS && data_type == REG_SZ)
                 {
                     if (!strcmp(component_id, TAP_WIN_COMPONENT_ID) ||
-                        !strcmp(component_id, "root\\" TAP_WIN_COMPONENT_ID))
+                        !strcmp(component_id, "root\\" TAP_WIN_COMPONENT_ID) ||
+                        !strcmp(component_id, WINTUN_COMPONENT_ID))
                     {
                         struct tap_reg *reg;
                         ALLOC_OBJ_CLEAR_GC(reg, struct tap_reg, gc);
                         reg->guid = string_alloc(net_cfg_instance_id, gc);
+                        reg->wintun = !strcmp(component_id, WINTUN_COMPONENT_ID);
 
                         /* link into return list */
                         if (!first)
@@ -3569,7 +3690,7 @@  get_tap_reg(struct gc_arena *gc)
     return first;
 }
 
-const struct panel_reg *
+static const struct panel_reg *
 get_panel_reg(struct gc_arena *gc)
 {
     LONG status;
@@ -3776,7 +3897,7 @@  show_tap_win_adapters(int msglev, int warnlev)
     const struct tap_reg *tap_reg = get_tap_reg(&gc);
     const struct panel_reg *panel_reg = get_panel_reg(&gc);
 
-    msg(msglev, "Available TAP-WIN32 adapters [name, GUID]:");
+    msg(msglev, "Available TAP-WIN32 / Wintun adapters [name, GUID, driver]:");
 
     /* loop through each TAP-Windows adapter registry entry */
     for (tr = tap_reg; tr != NULL; tr = tr->next)
@@ -3788,7 +3909,7 @@  show_tap_win_adapters(int msglev, int warnlev)
         {
             if (!strcmp(tr->guid, pr->guid))
             {
-                msg(msglev, "'%s' %s", pr->name, tr->guid);
+                msg(msglev, "'%s' %s %s", pr->name, tr->guid, tr->wintun ? "wintun" : "tap-windows6");
                 ++links;
             }
         }
@@ -3907,6 +4028,7 @@  get_unspecified_device_guid(const int device_number,
                             int actual_name_size,
                             const struct tap_reg *tap_reg_src,
                             const struct panel_reg *panel_reg_src,
+                            bool *wintun,
                             struct gc_arena *gc)
 {
     const struct tap_reg *tap_reg = tap_reg_src;
@@ -3956,6 +4078,10 @@  get_unspecified_device_guid(const int device_number,
     /* Save GUID for return value */
     ret = alloc_buf_gc(256, gc);
     buf_printf(&ret, "%s", tap_reg->guid);
+    if (wintun != NULL)
+    {
+        *wintun = tap_reg->wintun;
+    }
     return BSTR(&ret);
 }
 
@@ -4733,6 +4859,7 @@  tap_allow_nonadmin_access(const char *dev_node)
                                                       sizeof(actual_buffer),
                                                       tap_reg,
                                                       panel_reg,
+                                                      NULL,
                                                       &gc);
 
             if (!device_guid)
@@ -5267,9 +5394,9 @@  netsh_get_id(const char *dev_node, struct gc_arena *gc)
     }
     else
     {
-        guid = get_unspecified_device_guid(0, BPTR(&actual), BCAP(&actual), tap_reg, panel_reg, gc);
+        guid = get_unspecified_device_guid(0, BPTR(&actual), BCAP(&actual), tap_reg, panel_reg, NULL, gc);
 
-        if (get_unspecified_device_guid(1, NULL, 0, tap_reg, panel_reg, gc)) /* ambiguous if more than one TAP-Windows adapter */
+        if (get_unspecified_device_guid(1, NULL, 0, tap_reg, panel_reg, NULL, gc)) /* ambiguous if more than one TAP-Windows adapter */
         {
             guid = NULL;
         }
@@ -5541,7 +5668,8 @@  void
 open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt)
 {
     struct gc_arena gc = gc_new();
-    char device_path[256];
+    char tuntap_device_path[256];
+    char *path = NULL;
     const char *device_guid = NULL;
     DWORD len;
     bool dhcp_masq = false;
@@ -5571,6 +5699,8 @@  open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun
     {
         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);
+
         char actual_buffer[256];
 
         at_least_one_tap_win(tap_reg);
@@ -5586,24 +5716,22 @@  open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun
             }
 
             /* Open Windows TAP-Windows adapter */
-            openvpn_snprintf(device_path, sizeof(device_path), "%s%s%s",
+            openvpn_snprintf(tuntap_device_path, sizeof(tuntap_device_path), "%s%s%s",
                              USERMODEDEVICEDIR,
                              device_guid,
                              TAP_WIN_SUFFIX);
 
-            tt->hand = CreateFile(
-                device_path,
-                GENERIC_READ | GENERIC_WRITE,
-                0,                /* was: FILE_SHARE_READ */
-                0,
-                OPEN_EXISTING,
-                FILE_ATTRIBUTE_SYSTEM | FILE_FLAG_OVERLAPPED,
-                0
-                );
+            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->hand == INVALID_HANDLE_VALUE)
             {
-                msg(M_ERR, "CreateFile failed on TAP device: %s", device_path);
+                msg(M_ERR, "CreateFile failed on TAP device: %s", tuntap_device_path);
             }
         }
         else
@@ -5613,43 +5741,78 @@  open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun
             /* Try opening all TAP devices until we find one available */
             while (true)
             {
+                bool is_picked_device_wintun = false;
                 device_guid = get_unspecified_device_guid(device_number,
                                                           actual_buffer,
                                                           sizeof(actual_buffer),
                                                           tap_reg,
                                                           panel_reg,
+                                                          &is_picked_device_wintun,
                                                           &gc);
 
                 if (!device_guid)
                 {
-                    msg(M_FATAL, "All TAP-Windows adapters on this system are currently in use.");
+                    msg(M_FATAL, "All %s adapters on this system are currently in use.", tt->wintun ? "wintun" : "TAP - Windows");
                 }
 
-                /* Open Windows TAP-Windows adapter */
-                openvpn_snprintf(device_path, sizeof(device_path), "%s%s%s",
-                                 USERMODEDEVICEDIR,
-                                 device_guid,
-                                 TAP_WIN_SUFFIX);
-
-                tt->hand = CreateFile(
-                    device_path,
-                    GENERIC_READ | GENERIC_WRITE,
-                    0,                /* was: FILE_SHARE_READ */
-                    0,
-                    OPEN_EXISTING,
-                    FILE_ATTRIBUTE_SYSTEM | FILE_FLAG_OVERLAPPED,
-                    0
-                    );
+                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
+                {
+                    if (is_picked_device_wintun)
+                    {
+                        /* 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 TAP device: %s", device_path);
+                    msg(D_TUNTAP_INFO, "CreateFile failed on %s device: %s", tt->wintun ? "wintun" : "TAP", tuntap_device_path);
                 }
                 else
                 {
                     break;
                 }
 
+            next:
                 device_number++;
             }
         }
@@ -5659,10 +5822,11 @@  open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun
         tt->actual_name = string_alloc(actual_buffer, NULL);
     }
 
-    msg(M_INFO, "TAP-WIN32 device [%s] opened: %s", tt->actual_name, device_path);
+    msg(M_INFO, "%s device [%s] opened: %s", tt->wintun ? "Wintun" : "TAP-WIN32", tt->actual_name, path);
     tt->adapter_index = get_adapter_index(device_guid);
 
     /* get driver version info */
+    if (!tt->wintun)
     {
         ULONG info[3];
         CLEAR(info);
@@ -5702,6 +5866,7 @@  open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun
     }
 
     /* get driver MTU */
+    if (!tt->wintun)
     {
         ULONG mtu;
         if (DeviceIoControl(tt->hand, TAP_WIN_IOCTL_GET_MTU,
@@ -5761,7 +5926,7 @@  open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun
 
     /* set point-to-point mode if TUN device */
 
-    if (tt->type == DEV_TYPE_TUN)
+    if ((tt->type == DEV_TYPE_TUN) && !tt->wintun)
     {
         if (!tt->did_ifconfig_setup && !tt->did_ifconfig_ipv6_setup)
         {
@@ -5816,7 +5981,7 @@  open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun
 
     /* should we tell the TAP-Windows driver to masquerade as a DHCP server as a means
      * of setting the adapter address? */
-    if (dhcp_masq)
+    if (dhcp_masq && !tt->wintun)
     {
         uint32_t ep[4];
 
@@ -5894,6 +6059,7 @@  open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun
     }
 
     /* set driver media status to 'connected' */
+    if (!tt->wintun)
     {
         ULONG status = TRUE;
         if (!DeviceIoControl(tt->hand, TAP_WIN_IOCTL_SET_MEDIA_STATUS,
diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h
index df935f6..19cab7e 100644
--- a/src/openvpn/tun.h
+++ b/src/openvpn/tun.h
@@ -27,6 +27,8 @@ 
 #ifdef _WIN32
 #include <winioctl.h>
 #include <tap-windows.h>
+#include <setupapi.h>
+#include <cfgmgr32.h>
 #endif
 
 #include "buffer.h"
@@ -38,6 +40,10 @@ 
 #include "misc.h"
 #include "networking.h"
 
+#ifdef _WIN32
+#define WINTUN_COMPONENT_ID "wintun"
+#endif
+
 #if defined(_WIN32) || defined(TARGET_ANDROID)
 
 #define TUN_ADAPTER_INDEX_INVALID ((DWORD)-1)
@@ -340,6 +346,7 @@  route_order(void)
 struct tap_reg
 {
     const char *guid;
+    bool wintun;
     struct tap_reg *next;
 };
 
@@ -350,6 +357,13 @@  struct panel_reg
     struct panel_reg *next;
 };
 
+struct device_instance_id_interface
+{
+    const char *net_cfg_instance_id;
+    const char *device_interface_list;
+    struct device_instance_id_interface *next;
+};
+
 int ascii2ipset(const char *name);
 
 const char *ipset2ascii(int index);