Hi,

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

> From: Lev Stipakov <l...@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 <l...@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
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to