Add a warning for disabled DHCP media sense on Window

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 <[email protected]...>

---
 src/openvpn/tun.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

src/openvpn/tun.c +0 -0

index 25831ce3..35811975 100644

src/openvpn/tun.c +0 -0

 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;
 }
 

src/openvpn/tun.c +5 -0

+
+    if (status != ERROR_SUCCESS)
+    {
+        msg(M_WARN, "Error opening registry key: %s", TCPIP_PARAMS);
+    }

src/openvpn/tun.c +15 -0

+
+        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];
 

Hi,

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

Quoted 8 lines · Jiří Engelthaler

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"

Quoted 6 lines

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"

Quoted 11 lines

Signed-off-by Jiří Engelthaler <[email protected]...>

---
 src/openvpn/tun.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

src/openvpn/tun.c +0 -0

index 25831ce3..35811975 100644

src/openvpn/tun.c line 66

Quoted 5 lines

src/openvpn/tun.c +2 -0

@@ -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.

Quoted 7 lines

src/openvpn/tun.c +0 -0

 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;
 }
 

src/openvpn/tun.c get_panel_reg(struct gc_arena *gc) · line 3841

Quoted 10 lines

src/openvpn/tun.c +10 -0

+/*
+ * 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.

src/openvpn/tun.c get_panel_reg(struct gc_arena *gc) · line 3851

Quoted 10 lines

src/openvpn/tun.c +10 -0

+    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.

Quoted 5 lines

src/openvpn/tun.c +5 -0

+
+    if (status != ERROR_SUCCESS)
+    {
+        msg(M_WARN, "Error opening registry key: %s", TCPIP_PARAMS);
+    }

src/openvpn/tun.c get_panel_reg(struct gc_arena *gc) · line 3866

Quoted 10 lines

src/openvpn/tun.c +10 -0

+    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.

Quoted 22 lines

src/openvpn/tun.c +15 -0

+
+        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];
 

src/openvpn/tun.c open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun · line 5965

Quoted 10 lines

src/openvpn/tun.c +7 -0

+        /* 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

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 <[email protected]....>

---
 src/openvpn/tun.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

src/openvpn/tun.c +20 -0

index 2644d996..c8ab8796 100644
@@ -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);

src/openvpn/tun.c +5 -0

+    }
+    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);

src/openvpn/tun.c +15 -0

+
+        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];

src/openvpn/tun.c +4 -0

+        /* 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");

src/openvpn/tun.c +3 -0

+            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);

--
2.14.2.windows.3

2018-01-21 23:21 GMT+01:00 Selva Nair <[email protected]...>:

Quoted 6 lines

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 <[email protected]...>

Quoted 8 lines · wrote:

Quoted 8 lines

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"

Quoted 13 lines

Quoted 6 lines

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"

Quoted 4 lines

Signed-off-by Jiří Engelthaler <[email protected]...>

---
 src/openvpn/tun.c | 59 ++++++++++++++++++++++++++++++

+++++++++++++++++++++++++

Quoted 11 lines

src/openvpn/tun.c line 65

Quoted 11 lines

 1 file changed, 59 insertions(+)

src/openvpn/tun.c +1 -0

index 25831ce3..35811975 100644
@@ -62,6 +62,8 @@
 #define NI_IP_NETMASK  (1<<1)
 #define NI_OPTIONS     (1<<2)
 
+#define TCPIP_PARAMS "SYSTEM\\CurrentControlSet\\

Services\\Tcpip\\Parameters"

Quoted 67 lines

src/openvpn/tun.c line 66

Quoted 1 line

src/openvpn/tun.c +1 -0

+

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

Quoted 7 lines

src/openvpn/tun.c +0 -0

 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;
 }
 

src/openvpn/tun.c get_panel_reg(struct gc_arena *gc) · line 3841

Quoted 10 lines

src/openvpn/tun.c +10 -0

+/*
+ * 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.

src/openvpn/tun.c get_panel_reg(struct gc_arena *gc) · line 3851

Quoted 10 lines

src/openvpn/tun.c +10 -0

+    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.

Quoted 5 lines

src/openvpn/tun.c +5 -0

+
+    if (status != ERROR_SUCCESS)
+    {
+        msg(M_WARN, "Error opening registry key: %s", TCPIP_PARAMS);
+    }

src/openvpn/tun.c get_panel_reg(struct gc_arena *gc) · line 3866

Quoted 10 lines

src/openvpn/tun.c +10 -0

+    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.

Quoted 9 lines

src/openvpn/tun.c +9 -0

+
+        if (status == ERROR_SUCCESS && data_type == REG_DWORD)
+        {
+            if (disable_dhcp_media_sense != 0)
+            {
+                ret_value = false;
+            }
+        }
+

Quoted 8 lines

src/openvpn/tun.c get_panel_reg(struct gc_arena *gc) · line 3883

Quoted 8 lines

src/openvpn/tun.c +6 -0

+        RegCloseKey(tcpip_params);
+    }
+
+    return ret_value;
+}
+
 /*
  * Check that two addresses are part of the same 255.255.255.252

subnet.

Quoted 2 lines

src/openvpn/tun.c open_tun(const char *dev, const char *dev_type,

Quoted 2 lines

src/openvpn/tun.c +0 -0

  */

@@ -5901,6 +5953,13 @@ open_tun(const char *dev, const char *dev_type,

const char *dev_node, struct tun

Quoted 7 lines

src/openvpn/tun.c open_tun(const char *dev, const char *dev_type, · line 5959

Quoted 7 lines

src/openvpn/tun.c +4 -0

     {
         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");

Quoted 4 lines

src/openvpn/tun.c open_tun(const char *dev, const char *dev_type, · line 5963

Quoted 4 lines

src/openvpn/tun.c +3 -0

+            tt->options.dhcp_renew = true;
+        }
+
         /* We will answer DHCP requests with a reply to set IP/subnet

to these values */

Quoted 6 lines

src/openvpn/tun.c open_tun(const char *dev, const char *dev_type, · line 5965

Quoted 2 lines

src/openvpn/tun.c +0 -0

         ep[0] = htonl(tt->local);
         ep[1] = htonl(tt->adapter_netmask);

Thanks,

Selva

Any discussion / opinions ? Here https://github.com/OpenVPN/openvpn/pull/97#issuecomment-372530059
one user reported, that this patch were useful for him.

Hi,

Quoted 2 lines · Jiří Engelthaler

Any discussion / opinions ? Here https://github.com/OpenVPN/

openvpn/pull/97#issuecomment-372530059

Quoted 1 line

one user reported, that this patch were useful for him.

Though I agree that checking for whether media sense is globally disabled would help some, I'm not convinced that this is the right approach. The media sense can be disabled on individual adaptors too isn't it? -- say tap adaptor media status set as "always connected". And that won't be caught or fixed by this check.

If dhcp-renew will cure this, why not just make --dhcp-renew the default? That way we get rid of another config option and no new code is needed. Is there an argument why that option should not be on by default?

Selva

Hi.
You right. Disabled dhcp media sense can be set as global for all
interfaces or in adapter specific setting if it supports it (TAP supports it).
But for must users is the global settings hidden deep in the system and for me it took several months to find the cause of my problems. Interface specific settings can be cleared by uninstalling and reinstalling OpenVPN.

Hi,

Quoted 7 lines · Jiří Engelthaler

Hi.
You right. Disabled dhcp media sense can be set as global for all
interfaces or in adapter specific setting if it supports it (TAP supports it).
But for must users is the global settings hidden deep in the system and for me it took several months to find the cause of my problems. Interface specific settings can be cleared by uninstalling and reinstalling OpenVPN.

Well, Windows ships with media sense enabled, isn't it? So if its disabled, either the user did that or some third party application did it for some reason. Without knowing those unknowables, we can't even decide whether to suggest the user to enable it back or not.

Whereas, using --dhcp-renew (or making it the default) looks much simpler and something in our control. Anyone knows why it shouldn't be the default? I don't see how it could hurt except that openvpn needs to fork to run --dhcp-renew when the parent process is waiting for the the interface to get the address. But that's not a big deal on a Windows client.

This on of the old patches that are still pending. It seems that the original submitter never replied. This is still something we want to merge or should we just "close" due to timeout? I am not too familiar with the DHCP problem to have a good opinion on that.