[Openvpn-devel,2/3,v2] Enable dhcp on tap adapter using interactive service

Message ID 1538510474-27602-2-git-send-email-selva.nair@gmail.com
State Accepted
Headers show
Series [Openvpn-devel] Enable dhcp on tap adapter using interactive service | expand

Commit Message

Selva Nair Oct. 2, 2018, 10:01 a.m. UTC
From: Selva Nair <selva.nair@gmail.com>

Currently, if dhcp on the TAP interface is disabled, OpenVPN
on Windows tries to enable it using netsh but that succeeds only when
run with admin privileges.

When interactive service is available, delegate this task to the
service.

Trac #1111
Tested on Windows 7

Signed-off-by: Selva Nair <selva.nair@gmail.com>
---
v2 changes: As suggested by Lev
- In comment, clarify the 10 chars room is for printing 32 bit int
- Use get_win_sys_path() added in the accompanying patch (1 of 3)
 
Refactoring of writing to the message channel is in the
following patch.

 include/openvpn-msg.h         |  8 ++++++-
 src/openvpn/tun.c             | 53 ++++++++++++++++++++++++++++++++++++++++++-
 src/openvpnserv/interactive.c | 47 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 106 insertions(+), 2 deletions(-)

Comments

Lev Stipakov Oct. 2, 2018, 11:53 p.m. UTC | #1
ACK

ti 2. lokak. 2018 klo 23.02 selva.nair@gmail.com kirjoitti:

> From: Selva Nair <selva.nair@gmail.com>
>
> Currently, if dhcp on the TAP interface is disabled, OpenVPN
> on Windows tries to enable it using netsh but that succeeds only when
> run with admin privileges.
>
> When interactive service is available, delegate this task to the
> service.
>
> Trac #1111
> Tested on Windows 7
>
> Signed-off-by: Selva Nair <selva.nair@gmail.com>
> ---
> v2 changes: As suggested by Lev
> - In comment, clarify the 10 chars room is for printing 32 bit int
> - Use get_win_sys_path() added in the accompanying patch (1 of 3)
>
> Refactoring of writing to the message channel is in the
> following patch.
>
>  include/openvpn-msg.h         |  8 ++++++-
>  src/openvpn/tun.c             | 53
> ++++++++++++++++++++++++++++++++++++++++++-
>  src/openvpnserv/interactive.c | 47 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 106 insertions(+), 2 deletions(-)
>
> diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h
> index 82ecfe8..66177a2 100644
> --- a/include/openvpn-msg.h
> +++ b/include/openvpn-msg.h
> @@ -37,7 +37,8 @@ typedef enum {
>      msg_flush_neighbors,
>      msg_add_block_dns,
>      msg_del_block_dns,
> -    msg_register_dns
> +    msg_register_dns,
> +    msg_enable_dhcp,
>  } message_type_t;
>
>  typedef struct {
> @@ -111,4 +112,9 @@ typedef struct {
>      interface_t iface;
>  } block_dns_message_t;
>
> +typedef struct {
> +    message_header_t header;
> +    interface_t iface;
> +} enable_dhcp_message_t;
> +
>  #endif /* ifndef OPENVPN_MSG_H_ */
> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
> index 50f158c..a2d5315 100644
> --- a/src/openvpn/tun.c
> +++ b/src/openvpn/tun.c
> @@ -5203,6 +5203,49 @@ netsh_enable_dhcp(const struct tuntap_options *to,
>      argv_reset(&argv);
>  }
>
> +/* Enable dhcp on tap adapter using iservice */
> +static bool
> +service_enable_dhcp(const struct tuntap *tt)
> +{
> +    DWORD len;
> +    bool ret = false;
> +    ack_message_t ack;
> +    struct gc_arena gc = gc_new();
> +    HANDLE pipe = tt->options.msg_channel;
> +
> +    enable_dhcp_message_t dhcp = {
> +        .header = {
> +            msg_enable_dhcp,
> +            sizeof(enable_dhcp_message_t),
> +            0
> +        },
> +        .iface = { .index = tt->adapter_index, .name = "" }
> +    };
> +
> +    if (!WriteFile(pipe, &dhcp, sizeof(dhcp), &len, NULL)
> +        || !ReadFile(pipe, &ack, sizeof(ack), &len, NULL))
> +    {
> +        msg(M_WARN, "Enable_dhcp: could not talk to service: %s [%lu]",
> +            strerror_win32(GetLastError(), &gc), GetLastError());
> +        goto out;
> +    }
> +
> +    if (ack.error_number != NO_ERROR)
> +    {
> +        msg(M_NONFATAL, "TUN: enabling dhcp using service failed: %s
> [status=%u if_index=%d]",
> +            strerror_win32(ack.error_number, &gc), ack.error_number,
> dhcp.iface.index);
> +    }
> +    else
> +    {
> +        msg(M_INFO, "DHCP enabled on interface %d using service",
> dhcp.iface.index);
> +        ret = true;
> +    }
> +
> +out:
> +    gc_free(&gc);
> +    return ret;
> +}
> +
>  /*
>   * Return a TAP name for netsh commands.
>   */
> @@ -5683,7 +5726,15 @@ open_tun(const char *dev, const char *dev_type,
> const char *dev_node, struct tun
>               */
>              if (dhcp_status(tt->adapter_index) == DHCP_STATUS_DISABLED)
>              {
> -                netsh_enable_dhcp(&tt->options, tt->actual_name);
> +                /* try using the service if available, else directly
> execute netsh */
> +                if (tt->options.msg_channel)
> +                {
> +                    service_enable_dhcp(tt);
> +                }
> +                else
> +                {
> +                    netsh_enable_dhcp(&tt->options, tt->actual_name);
> +                }
>              }
>              dhcp_masq = true;
>              dhcp_masq_post = true;
> diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
> index 0489684..c0fdc19 100644
> --- a/src/openvpnserv/interactive.c
> +++ b/src/openvpnserv/interactive.c
> @@ -1165,6 +1165,45 @@ out:
>      return err;
>  }
>
> +static DWORD
> +HandleEnableDHCPMessage(const enable_dhcp_message_t *dhcp)
> +{
> +    DWORD err = 0;
> +    DWORD timeout = 5000; /* in milli seconds */
> +    wchar_t argv0[MAX_PATH];
> +
> +    /* Path of netsh */
> +    swprintf(argv0, _countof(argv0), L"%s\\%s", get_win_sys_path(),
> L"netsh.exe");
> +    argv0[_countof(argv0) - 1] = L'\0';
> +
> +    /* cmd template:
> +     * netsh interface ipv4 set address name=$if_index source=dhcp
> +     */
> +    const wchar_t *fmt = L"netsh interface ipv4 set address name=\"%d\"
> source=dhcp";
> +
> +    /* max cmdline length in wchars -- include room for if index:
> +     * 10 chars for 32 bit int in decimal and +1 for NUL
> +     */
> +    size_t ncmdline = wcslen(fmt) + 10 + 1;
> +    wchar_t *cmdline = malloc(ncmdline*sizeof(wchar_t));
> +    if (!cmdline)
> +    {
> +        err = ERROR_OUTOFMEMORY;
> +        return err;
> +    }
> +
> +    openvpn_sntprintf(cmdline, ncmdline, fmt, dhcp->iface.index);
> +
> +    err = ExecCommand(argv0, cmdline, timeout);
> +
> +    /* Note: This could fail if dhcp is already enabled, so the caller
> +     * may not want to treat errors as FATAL.
> +     */
> +
> +    free(cmdline);
> +    return err;
> +}
> +
>  static VOID
>  HandleMessage(HANDLE pipe, DWORD bytes, DWORD count, LPHANDLE events,
> undo_lists_t *lists)
>  {
> @@ -1176,6 +1215,7 @@ HandleMessage(HANDLE pipe, DWORD bytes, DWORD count,
> LPHANDLE events, undo_lists
>          flush_neighbors_message_t flush_neighbors;
>          block_dns_message_t block_dns;
>          dns_cfg_message_t dns;
> +        enable_dhcp_message_t dhcp;
>      } msg;
>      ack_message_t ack = {
>          .header = {
> @@ -1236,6 +1276,13 @@ HandleMessage(HANDLE pipe, DWORD bytes, DWORD
> count, LPHANDLE events, undo_lists
>              ack.error_number = HandleDNSConfigMessage(&msg.dns, lists);
>              break;
>
> +        case msg_enable_dhcp:
> +            if (msg.header.size == sizeof(msg.dhcp))
> +            {
> +                ack.error_number = HandleEnableDHCPMessage(&msg.dhcp);
> +            }
> +            break;
> +
>          default:
>              ack.error_number = ERROR_MESSAGE_TYPE;
>              MsgToEventLog(MSG_FLAGS_ERROR, TEXT("Unknown message type
> %d"), msg.header.type);
> --
> 2.1.4
>
>
>
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>
Gert Doering Oct. 5, 2018, 3:24 a.m. UTC | #2
Acked-by: Gert Doering <gert@greenie.muc.de>

Another ACK from me.  Code looks good.  Thanks.

Your patch has been applied to the master and release/2.4 branch (bugfix,
so openvpn 2.4.x on windows can fully run unprivileged).

Test compiled on ubuntu 16.04 / mingw, not actually run.

commit b4fc8bbd6b1d0211dd6982c4accedfbe4ae7e3ed (master)
commit 78d4f0ad106720d8988e144f6d30ec11d84dee2f (master)
Author: Selva Nair
Date:   Tue Oct 2 16:01:13 2018 -0400

     Enable dhcp on tap adapter using interactive service

     Signed-off-by: Selva Nair <selva.nair@gmail.com>
     Acked-by: Lev Stipakov <lstipakov@gmail.com>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <1538510474-27602-2-git-send-email-selva.nair@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17517.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h
index 82ecfe8..66177a2 100644
--- a/include/openvpn-msg.h
+++ b/include/openvpn-msg.h
@@ -37,7 +37,8 @@  typedef enum {
     msg_flush_neighbors,
     msg_add_block_dns,
     msg_del_block_dns,
-    msg_register_dns
+    msg_register_dns,
+    msg_enable_dhcp,
 } message_type_t;
 
 typedef struct {
@@ -111,4 +112,9 @@  typedef struct {
     interface_t iface;
 } block_dns_message_t;
 
+typedef struct {
+    message_header_t header;
+    interface_t iface;
+} enable_dhcp_message_t;
+
 #endif /* ifndef OPENVPN_MSG_H_ */
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 50f158c..a2d5315 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -5203,6 +5203,49 @@  netsh_enable_dhcp(const struct tuntap_options *to,
     argv_reset(&argv);
 }
 
+/* Enable dhcp on tap adapter using iservice */
+static bool
+service_enable_dhcp(const struct tuntap *tt)
+{
+    DWORD len;
+    bool ret = false;
+    ack_message_t ack;
+    struct gc_arena gc = gc_new();
+    HANDLE pipe = tt->options.msg_channel;
+
+    enable_dhcp_message_t dhcp = {
+        .header = {
+            msg_enable_dhcp,
+            sizeof(enable_dhcp_message_t),
+            0
+        },
+        .iface = { .index = tt->adapter_index, .name = "" }
+    };
+
+    if (!WriteFile(pipe, &dhcp, sizeof(dhcp), &len, NULL)
+        || !ReadFile(pipe, &ack, sizeof(ack), &len, NULL))
+    {
+        msg(M_WARN, "Enable_dhcp: could not talk to service: %s [%lu]",
+            strerror_win32(GetLastError(), &gc), GetLastError());
+        goto out;
+    }
+
+    if (ack.error_number != NO_ERROR)
+    {
+        msg(M_NONFATAL, "TUN: enabling dhcp using service failed: %s [status=%u if_index=%d]",
+            strerror_win32(ack.error_number, &gc), ack.error_number, dhcp.iface.index);
+    }
+    else
+    {
+        msg(M_INFO, "DHCP enabled on interface %d using service", dhcp.iface.index);
+        ret = true;
+    }
+
+out:
+    gc_free(&gc);
+    return ret;
+}
+
 /*
  * Return a TAP name for netsh commands.
  */
@@ -5683,7 +5726,15 @@  open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun
              */
             if (dhcp_status(tt->adapter_index) == DHCP_STATUS_DISABLED)
             {
-                netsh_enable_dhcp(&tt->options, tt->actual_name);
+                /* try using the service if available, else directly execute netsh */
+                if (tt->options.msg_channel)
+                {
+                    service_enable_dhcp(tt);
+                }
+                else
+                {
+                    netsh_enable_dhcp(&tt->options, tt->actual_name);
+                }
             }
             dhcp_masq = true;
             dhcp_masq_post = true;
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index 0489684..c0fdc19 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -1165,6 +1165,45 @@  out:
     return err;
 }
 
+static DWORD
+HandleEnableDHCPMessage(const enable_dhcp_message_t *dhcp)
+{
+    DWORD err = 0;
+    DWORD timeout = 5000; /* in milli seconds */
+    wchar_t argv0[MAX_PATH];
+
+    /* Path of netsh */
+    swprintf(argv0, _countof(argv0), L"%s\\%s", get_win_sys_path(), L"netsh.exe");
+    argv0[_countof(argv0) - 1] = L'\0';
+
+    /* cmd template:
+     * netsh interface ipv4 set address name=$if_index source=dhcp
+     */
+    const wchar_t *fmt = L"netsh interface ipv4 set address name=\"%d\" source=dhcp";
+
+    /* max cmdline length in wchars -- include room for if index:
+     * 10 chars for 32 bit int in decimal and +1 for NUL
+     */
+    size_t ncmdline = wcslen(fmt) + 10 + 1;
+    wchar_t *cmdline = malloc(ncmdline*sizeof(wchar_t));
+    if (!cmdline)
+    {
+        err = ERROR_OUTOFMEMORY;
+        return err;
+    }
+
+    openvpn_sntprintf(cmdline, ncmdline, fmt, dhcp->iface.index);
+
+    err = ExecCommand(argv0, cmdline, timeout);
+
+    /* Note: This could fail if dhcp is already enabled, so the caller
+     * may not want to treat errors as FATAL.
+     */
+
+    free(cmdline);
+    return err;
+}
+
 static VOID
 HandleMessage(HANDLE pipe, DWORD bytes, DWORD count, LPHANDLE events, undo_lists_t *lists)
 {
@@ -1176,6 +1215,7 @@  HandleMessage(HANDLE pipe, DWORD bytes, DWORD count, LPHANDLE events, undo_lists
         flush_neighbors_message_t flush_neighbors;
         block_dns_message_t block_dns;
         dns_cfg_message_t dns;
+        enable_dhcp_message_t dhcp;
     } msg;
     ack_message_t ack = {
         .header = {
@@ -1236,6 +1276,13 @@  HandleMessage(HANDLE pipe, DWORD bytes, DWORD count, LPHANDLE events, undo_lists
             ack.error_number = HandleDNSConfigMessage(&msg.dns, lists);
             break;
 
+        case msg_enable_dhcp:
+            if (msg.header.size == sizeof(msg.dhcp))
+            {
+                ack.error_number = HandleEnableDHCPMessage(&msg.dhcp);
+            }
+            break;
+
         default:
             ack.error_number = ERROR_MESSAGE_TYPE;
             MsgToEventLog(MSG_FLAGS_ERROR, TEXT("Unknown message type %d"), msg.header.type);