[Openvpn-devel] Add a warning for disabled DHCP media sense on Window

Message ID 20171205121952.13008-1-EngyCZ@gmail.com
State New
Headers show
Series
  • [Openvpn-devel] Add a warning for disabled DHCP media sense on Window
Related show

Commit Message

Jiří Engelthaler Dec. 5, 2017, 12:19 p.m.
When DHCP media sense configuration is disabled, network applications
including DHCP client will not receive information about link status
changes and the link seems to be always connected. This lead to the
non-renewal DHCP address on OpenVPN connect.

DHCP media sense status can by shown with command
"netsh interface ipv4 show global"

There are several reports of problems with DHCP address renewal.
https://community.openvpn.net/openvpn/ticket/665
https://community.openvpn.net/openvpn/ticket/807

Added checking of disabled DHCP media sense and print a warning with
forced dhcp-renew option and suggestion to enable DHCP media sense.

Signed-off-by: Jiří Engelthaler <EngyCZ@gmail.com>
---
 src/openvpn/tun.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

Comments

Selva Nair Jan. 21, 2018, 10:21 p.m. | #1
Hi,

I haven't tested this, but looks like a useful thing to do. The code
looks good too. Some minor comments below:

On Tue, Dec 5, 2017 at 7:19 AM, Jiří Engelthaler <EngyCZ@gmail.com> wrote:
> When DHCP media sense configuration is disabled, network applications
> including DHCP client will not receive information about link status
> changes and the link seems to be always connected. This lead to the
> non-renewal DHCP address on OpenVPN connect.
>
> DHCP media sense status can by shown with command
> "netsh interface ipv4 show global"
>
> There are several reports of problems with DHCP address renewal.
> https://community.openvpn.net/openvpn/ticket/665
> https://community.openvpn.net/openvpn/ticket/807
>
> Added checking of disabled DHCP media sense and print a warning with
> forced dhcp-renew option and suggestion to enable DHCP media sense.

Typo in commit header: "Window" should be "Windows"

>
> Signed-off-by: Jiří Engelthaler <EngyCZ@gmail.com>
> ---
>  src/openvpn/tun.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
>
> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
> index 25831ce3..35811975 100644
> --- a/src/openvpn/tun.c
> +++ b/src/openvpn/tun.c
> @@ -62,6 +62,8 @@
>  #define NI_IP_NETMASK  (1<<1)
>  #define NI_OPTIONS     (1<<2)
>
> +#define TCPIP_PARAMS "SYSTEM\\CurrentControlSet\\Services\\Tcpip\\Parameters"
> +

This is only used inside get_dhcp_media_sense. Better move it there as
a const char *. Less macros, the better.

>  static void netsh_ifconfig(const struct tuntap_options *to,
>                             const char *flex_name,
>                             const in_addr_t ip,
> @@ -3827,6 +3829,56 @@ get_panel_reg(struct gc_arena *gc)
>      return first;
>  }
>
> +/*
> + * Return DhcpMediaSense enabled value
> + */
> +static bool
> +get_dhcp_media_sense(void)
> +{
> +    HKEY tcpip_params;
> +    LONG status;
> +    DWORD len;
> +    char  disable_dhcp_media_sense_string[] = "DisableDHCPMediaSense";

This would benefit from a const.

> +    DWORD disable_dhcp_media_sense;
> +    DWORD data_type;
> +    bool ret_value = true;
> +
> +    status = RegOpenKeyEx(
> +        HKEY_LOCAL_MACHINE,
> +        TCPIP_PARAMS,
> +        0,
> +        KEY_READ,
> +        &tcpip_params);

I'm not sure whether its specified in the newly adopted style, but
please avoid writing one argument per line.  Original has many such,
but not preferred in new code.

> +
> +    if (status != ERROR_SUCCESS)
> +    {
> +        msg(M_WARN, "Error opening registry key: %s", TCPIP_PARAMS);
> +    }
> +    else
> +    {
> +        len = sizeof(disable_dhcp_media_sense);
> +        status = RegQueryValueEx(
> +            tcpip_params,
> +            disable_dhcp_media_sense_string,
> +            NULL,
> +            &data_type,
> +            (PBYTE)&disable_dhcp_media_sense,
> +            &len);

Same here.

> +
> +        if (status == ERROR_SUCCESS && data_type == REG_DWORD)
> +        {
> +            if (disable_dhcp_media_sense != 0)
> +            {
> +                ret_value = false;
> +            }
> +        }
> +
> +        RegCloseKey(tcpip_params);
> +    }
> +
> +    return ret_value;
> +}
> +
>  /*
>   * Check that two addresses are part of the same 255.255.255.252 subnet.
>   */
> @@ -5901,6 +5953,13 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun
>      {
>          uint32_t ep[4];
>
> +        /* Check DHCP media sense value */
> +        if (!tt->options.dhcp_renew && !get_dhcp_media_sense())
> +        {
> +            msg(M_WARN, "WARNING: DHCP media sense disabled, dhcp_renew option forced. You can enable media sense with \"netsh interface ipv4 set global dhcpmediasense=enabled\" shell command");
> +            tt->options.dhcp_renew = true;
> +        }
> +
>          /* We will answer DHCP requests with a reply to set IP/subnet to these values */
>          ep[0] = htonl(tt->local);
>          ep[1] = htonl(tt->adapter_netmask);

Thanks,

Selva

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Jiří Engelthaler Jan. 31, 2018, 1:37 p.m. | #2
Comments accepted. Thank you Selva. This patch is also as pull request
https://github.com/OpenVPN/openvpn/pull/97


When DHCP media sense configuration is disabled, network applications
including DHCP client will not receive information about link status
changes and the link seems to be always connected. This lead to the
non-renewal DHCP address on OpenVPN connect.

DHCP media sense status can by shown with command
"netsh interface ipv4 show global"

There are several reports of problems with DHCP address renewal.
https://community.openvpn.net/openvpn/ticket/665
https://community.openvpn.net/openvpn/ticket/807

Added checking of disabled DHCP media sense and print a warning with
forced dhcp-renew option and suggestion to enable DHCP media sense.

Signed-off-by: Jiří Engelthaler <EngyCZ@gmail.com>
---
 src/openvpn/tun.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 2644d996..c8ab8796 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -3827,6 +3827,46 @@ get_panel_reg(struct gc_arena *gc)
     return first;
 }

+/*
+ * Return DhcpMediaSense enabled value
+ */
+static bool
+get_dhcp_media_sense(void)
+{
+    const char registry_tcpip_params[] =
"SYSTEM\\CurrentControlSet\\Services\\Tcpip\\Parameters";
+    const char disable_dhcp_media_sense_string[] = "DisableDHCPMediaSense";
+    HKEY tcpip_params;
+    LONG status;
+    DWORD len;
+    DWORD disable_dhcp_media_sense;
+    DWORD data_type;
+    bool ret_value = true;
+
+    status = RegOpenKeyEx(HKEY_LOCAL_MACHINE, registry_tcpip_params,
0, KEY_READ, &tcpip_params);
+
+    if (status != ERROR_SUCCESS)
+    {
+        msg(M_WARN, "Error opening registry key: %s", registry_tcpip_params);
+    }
+    else
+    {
+        len = sizeof(disable_dhcp_media_sense);
+        status = RegQueryValueEx(tcpip_params,
disable_dhcp_media_sense_string, NULL, &data_type,
(PBYTE)&disable_dhcp_media_sense, &len);
+
+        if (status == ERROR_SUCCESS && data_type == REG_DWORD)
+        {
+            if (disable_dhcp_media_sense != 0)
+            {
+                ret_value = false;
+            }
+        }
+
+        RegCloseKey(tcpip_params);
+    }
+
+    return ret_value;
+}
+
 /*
  * Check that two addresses are part of the same 255.255.255.252 subnet.
  */
@@ -5898,6 +5938,13 @@ open_tun(const char *dev, const char *dev_type,
const char *dev_node, struct tun
     {
         uint32_t ep[4];

+        /* Check DHCP media sense value */
+        if (!tt->options.dhcp_renew && !get_dhcp_media_sense())
+        {
+            msg(M_WARN, "WARNING: DHCP media sense disabled,
dhcp_renew option forced. You can enable media sense with \"netsh
interface ipv4 set global dhcpmediasense=enabled\" shell command");
+            tt->options.dhcp_renew = true;
+        }
+
         /* We will answer DHCP requests with a reply to set IP/subnet
to these values */
         ep[0] = htonl(tt->local);
         ep[1] = htonl(tt->adapter_netmask);

Patch

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 25831ce3..35811975 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -62,6 +62,8 @@ 
 #define NI_IP_NETMASK  (1<<1)
 #define NI_OPTIONS     (1<<2)
 
+#define TCPIP_PARAMS "SYSTEM\\CurrentControlSet\\Services\\Tcpip\\Parameters"
+
 static void netsh_ifconfig(const struct tuntap_options *to,
                            const char *flex_name,
                            const in_addr_t ip,
@@ -3827,6 +3829,56 @@  get_panel_reg(struct gc_arena *gc)
     return first;
 }
 
+/*
+ * Return DhcpMediaSense enabled value
+ */
+static bool
+get_dhcp_media_sense(void)
+{
+    HKEY tcpip_params;
+    LONG status;
+    DWORD len;
+    char  disable_dhcp_media_sense_string[] = "DisableDHCPMediaSense";
+    DWORD disable_dhcp_media_sense;
+    DWORD data_type;
+    bool ret_value = true;
+
+    status = RegOpenKeyEx(
+        HKEY_LOCAL_MACHINE,
+        TCPIP_PARAMS,
+        0,
+        KEY_READ,
+        &tcpip_params);
+    
+    if (status != ERROR_SUCCESS)
+    {
+        msg(M_WARN, "Error opening registry key: %s", TCPIP_PARAMS);
+    }
+    else
+    {
+        len = sizeof(disable_dhcp_media_sense);
+        status = RegQueryValueEx(
+            tcpip_params,
+            disable_dhcp_media_sense_string,
+            NULL,
+            &data_type,
+            (PBYTE)&disable_dhcp_media_sense,
+            &len);
+
+        if (status == ERROR_SUCCESS && data_type == REG_DWORD)
+        {
+            if (disable_dhcp_media_sense != 0)
+            {
+                ret_value = false;
+            }
+        }
+
+        RegCloseKey(tcpip_params);
+    }
+
+    return ret_value;
+}
+
 /*
  * Check that two addresses are part of the same 255.255.255.252 subnet.
  */
@@ -5901,6 +5953,13 @@  open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun
     {
         uint32_t ep[4];
 
+        /* Check DHCP media sense value */
+        if (!tt->options.dhcp_renew && !get_dhcp_media_sense())
+        {
+            msg(M_WARN, "WARNING: DHCP media sense disabled, dhcp_renew option forced. You can enable media sense with \"netsh interface ipv4 set global dhcpmediasense=enabled\" shell command");
+            tt->options.dhcp_renew = true;
+        }
+
         /* We will answer DHCP requests with a reply to set IP/subnet to these values */
         ep[0] = htonl(tt->local);
         ep[1] = htonl(tt->adapter_netmask);