[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);
Jiří Engelthaler March 13, 2018, 11:36 a.m. | #3
Any discussion / opinions ? Here https://github.com/OpenVPN/openvpn
/pull/97#issuecomment-372530059 one user reported, that this patch were
useful for him.

2018-01-31 14:37 GMT+01:00 Jiří Engelthaler <engycz@gmail.com>:

> 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);
> --
> 2.14.2.windows.3
>
>
>
> 2018-01-21 23:21 GMT+01:00 Selva Nair <selva.nair@gmail.com>:
> > 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
>
<div dir="ltr">
Any discussion / opinions ? Here <a href="https://github.com/OpenVPN/openvpn/pull/97#issuecomment-372530059" target="_blank">https://github.com/<span class="gmail-il">OpenVPN</span>/<wbr><span class="gmail-il">openvpn</span>/pull/97#issuecomment-<wbr>372530059</a> one user reported, that this patch were useful for him.

<br></div><div class="gmail_extra"><br><div class="gmail_quote">2018-01-31 14:37 GMT+01:00 Jiří Engelthaler <span dir="ltr">&lt;<a href="mailto:engycz@gmail.com" target="_blank">engycz@gmail.com</a>&gt;</span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Comments accepted. Thank you Selva. This patch is also as pull request<br>
<a href="https://github.com/OpenVPN/openvpn/pull/97" rel="noreferrer" target="_blank">https://github.com/OpenVPN/<wbr>openvpn/pull/97</a><br>
<span class=""><br>
<br>
When DHCP media sense configuration is disabled, network applications<br>
including DHCP client will not receive information about link status<br>
changes and the link seems to be always connected. This lead to the<br>
non-renewal DHCP address on OpenVPN connect.<br>
<br>
DHCP media sense status can by shown with command<br>
&quot;netsh interface ipv4 show global&quot;<br>
<br>
There are several reports of problems with DHCP address renewal.<br>
<a href="https://community.openvpn.net/openvpn/ticket/665" rel="noreferrer" target="_blank">https://community.openvpn.net/<wbr>openvpn/ticket/665</a><br>
<a href="https://community.openvpn.net/openvpn/ticket/807" rel="noreferrer" target="_blank">https://community.openvpn.net/<wbr>openvpn/ticket/807</a><br>
<br>
Added checking of disabled DHCP media sense and print a warning with<br>
forced dhcp-renew option and suggestion to enable DHCP media sense.<br>
<br>
</span>Signed-off-by: Jiří Engelthaler &lt;<a href="mailto:EngyCZ@gmail.com">EngyCZ@gmail.com</a>&gt;<br>
---<br>
 src/openvpn/tun.c | 47 ++++++++++++++++++++++++++++++<wbr>+++++++++++++++++<br>
 1 file changed, 47 insertions(+)<br>
<br>
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c<br>
index 2644d996..c8ab8796 100644<br>
--- a/src/openvpn/tun.c<br>
+++ b/src/openvpn/tun.c<br>
@@ -3827,6 +3827,46 @@ get_panel_reg(struct gc_arena *gc)<br>
<span class="">     return first;<br>
 }<br>
<br>
+/*<br>
+ * Return DhcpMediaSense enabled value<br>
+ */<br>
+static bool<br>
+get_dhcp_media_sense(void)<br>
+{<br>
</span>+    const char registry_tcpip_params[] =<br>
<span class="">&quot;SYSTEM\\CurrentControlSet\\<wbr>Services\\Tcpip\\Parameters&quot;;<br>
</span>+    const char disable_dhcp_media_sense_<wbr>string[] = &quot;DisableDHCPMediaSense&quot;;<br>
<span class="">+    HKEY tcpip_params;<br>
+    LONG status;<br>
+    DWORD len;<br>
</span><span class="">+    DWORD disable_dhcp_media_sense;<br>
+    DWORD data_type;<br>
+    bool ret_value = true;<br>
+<br>
</span>+    status = RegOpenKeyEx(HKEY_LOCAL_<wbr>MACHINE, registry_tcpip_params,<br>
0, KEY_READ, &amp;tcpip_params);<br>
<span class="">+<br>
+    if (status != ERROR_SUCCESS)<br>
+    {<br>
</span>+        msg(M_WARN, &quot;Error opening registry key: %s&quot;, registry_tcpip_params);<br>
<span class="">+    }<br>
+    else<br>
+    {<br>
+        len = sizeof(disable_dhcp_media_<wbr>sense);<br>
</span>+        status = RegQueryValueEx(tcpip_params,<br>
disable_dhcp_media_sense_<wbr>string, NULL, &amp;data_type,<br>
(PBYTE)&amp;disable_dhcp_media_<wbr>sense, &amp;len);<br>
<span class="">+<br>
+        if (status == ERROR_SUCCESS &amp;&amp; data_type == REG_DWORD)<br>
+        {<br>
+            if (disable_dhcp_media_sense != 0)<br>
+            {<br>
+                ret_value = false;<br>
+            }<br>
+        }<br>
+<br>
+        RegCloseKey(tcpip_params);<br>
+    }<br>
+<br>
+    return ret_value;<br>
+}<br>
+<br>
 /*<br>
  * Check that two addresses are part of the same 255.255.255.252 subnet.<br>
  */<br>
</span>@@ -5898,6 +5938,13 @@ open_tun(const char *dev, const char *dev_type,<br>
<span class="im HOEnZb">const char *dev_node, struct tun<br>
     {<br>
         uint32_t ep[4];<br>
<br>
+        /* Check DHCP media sense value */<br>
+        if (!tt-&gt;options.dhcp_renew &amp;&amp; !get_dhcp_media_sense())<br>
+        {<br>
+            msg(M_WARN, &quot;WARNING: DHCP media sense disabled,<br>
dhcp_renew option forced. You can enable media sense with \&quot;netsh<br>
interface ipv4 set global dhcpmediasense=enabled\&quot; shell command&quot;);<br>
+            tt-&gt;options.dhcp_renew = true;<br>
+        }<br>
+<br>
         /* We will answer DHCP requests with a reply to set IP/subnet<br>
to these values */<br>
         ep[0] = htonl(tt-&gt;local);<br>
         ep[1] = htonl(tt-&gt;adapter_netmask);<br>
</span><span class="HOEnZb"><font color="#888888">--<br>
2.14.2.windows.3<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
<br>
2018-01-21 23:21 GMT+01:00 Selva Nair &lt;<a href="mailto:selva.nair@gmail.com">selva.nair@gmail.com</a>&gt;:<br>
&gt; Hi,<br>
&gt;<br>
&gt; I haven&#39;t tested this, but looks like a useful thing to do. The code<br>
&gt; looks good too. Some minor comments below:<br>
&gt;<br>
&gt; On Tue, Dec 5, 2017 at 7:19 AM, Jiří Engelthaler &lt;<a href="mailto:EngyCZ@gmail.com">EngyCZ@gmail.com</a>&gt; wrote:<br>
&gt;&gt; When DHCP media sense configuration is disabled, network applications<br>
&gt;&gt; including DHCP client will not receive information about link status<br>
&gt;&gt; changes and the link seems to be always connected. This lead to the<br>
&gt;&gt; non-renewal DHCP address on OpenVPN connect.<br>
&gt;&gt;<br>
&gt;&gt; DHCP media sense status can by shown with command<br>
&gt;&gt; &quot;netsh interface ipv4 show global&quot;<br>
&gt;&gt;<br>
&gt;&gt; There are several reports of problems with DHCP address renewal.<br>
&gt;&gt; <a href="https://community.openvpn.net/openvpn/ticket/665" rel="noreferrer" target="_blank">https://community.openvpn.net/<wbr>openvpn/ticket/665</a><br>
&gt;&gt; <a href="https://community.openvpn.net/openvpn/ticket/807" rel="noreferrer" target="_blank">https://community.openvpn.net/<wbr>openvpn/ticket/807</a><br>
&gt;&gt;<br>
&gt;&gt; Added checking of disabled DHCP media sense and print a warning with<br>
&gt;&gt; forced dhcp-renew option and suggestion to enable DHCP media sense.<br>
&gt;<br>
&gt; Typo in commit header: &quot;Window&quot; should be &quot;Windows&quot;<br>
&gt;<br>
&gt;&gt;<br>
&gt;&gt; Signed-off-by: Jiří Engelthaler &lt;<a href="mailto:EngyCZ@gmail.com">EngyCZ@gmail.com</a>&gt;<br>
&gt;&gt; ---<br>
&gt;&gt;  src/openvpn/tun.c | 59 ++++++++++++++++++++++++++++++<wbr>+++++++++++++++++++++++++<br>
&gt;&gt;  1 file changed, 59 insertions(+)<br>
&gt;&gt;<br>
&gt;&gt; diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c<br>
&gt;&gt; index 25831ce3..35811975 100644<br>
&gt;&gt; --- a/src/openvpn/tun.c<br>
&gt;&gt; +++ b/src/openvpn/tun.c<br>
&gt;&gt; @@ -62,6 +62,8 @@<br>
&gt;&gt;  #define NI_IP_NETMASK  (1&lt;&lt;1)<br>
&gt;&gt;  #define NI_OPTIONS     (1&lt;&lt;2)<br>
&gt;&gt;<br>
&gt;&gt; +#define TCPIP_PARAMS &quot;SYSTEM\\CurrentControlSet\\<wbr>Services\\Tcpip\\Parameters&quot;<br>
&gt;&gt; +<br>
&gt;<br>
&gt; This is only used inside get_dhcp_media_sense. Better move it there as<br>
&gt; a const char *. Less macros, the better.<br>
&gt;<br>
&gt;&gt;  static void netsh_ifconfig(const struct tuntap_options *to,<br>
&gt;&gt;                             const char *flex_name,<br>
&gt;&gt;                             const in_addr_t ip,<br>
&gt;&gt; @@ -3827,6 +3829,56 @@ get_panel_reg(struct gc_arena *gc)<br>
&gt;&gt;      return first;<br>
&gt;&gt;  }<br>
&gt;&gt;<br>
&gt;&gt; +/*<br>
&gt;&gt; + * Return DhcpMediaSense enabled value<br>
&gt;&gt; + */<br>
&gt;&gt; +static bool<br>
&gt;&gt; +get_dhcp_media_sense(void)<br>
&gt;&gt; +{<br>
&gt;&gt; +    HKEY tcpip_params;<br>
&gt;&gt; +    LONG status;<br>
&gt;&gt; +    DWORD len;<br>
&gt;&gt; +    char  disable_dhcp_media_sense_<wbr>string[] = &quot;DisableDHCPMediaSense&quot;;<br>
&gt;<br>
&gt; This would benefit from a const.<br>
&gt;<br>
&gt;&gt; +    DWORD disable_dhcp_media_sense;<br>
&gt;&gt; +    DWORD data_type;<br>
&gt;&gt; +    bool ret_value = true;<br>
&gt;&gt; +<br>
&gt;&gt; +    status = RegOpenKeyEx(<br>
&gt;&gt; +        HKEY_LOCAL_MACHINE,<br>
&gt;&gt; +        TCPIP_PARAMS,<br>
&gt;&gt; +        0,<br>
&gt;&gt; +        KEY_READ,<br>
&gt;&gt; +        &amp;tcpip_params);<br>
&gt;<br>
&gt; I&#39;m not sure whether its specified in the newly adopted style, but<br>
&gt; please avoid writing one argument per line.  Original has many such,<br>
&gt; but not preferred in new code.<br>
&gt;<br>
&gt;&gt; +<br>
&gt;&gt; +    if (status != ERROR_SUCCESS)<br>
&gt;&gt; +    {<br>
&gt;&gt; +        msg(M_WARN, &quot;Error opening registry key: %s&quot;, TCPIP_PARAMS);<br>
&gt;&gt; +    }<br>
&gt;&gt; +    else<br>
&gt;&gt; +    {<br>
&gt;&gt; +        len = sizeof(disable_dhcp_media_<wbr>sense);<br>
&gt;&gt; +        status = RegQueryValueEx(<br>
&gt;&gt; +            tcpip_params,<br>
&gt;&gt; +            disable_dhcp_media_sense_<wbr>string,<br>
&gt;&gt; +            NULL,<br>
&gt;&gt; +            &amp;data_type,<br>
&gt;&gt; +            (PBYTE)&amp;disable_dhcp_media_<wbr>sense,<br>
&gt;&gt; +            &amp;len);<br>
&gt;<br>
&gt; Same here.<br>
&gt;<br>
&gt;&gt; +<br>
&gt;&gt; +        if (status == ERROR_SUCCESS &amp;&amp; data_type == REG_DWORD)<br>
&gt;&gt; +        {<br>
&gt;&gt; +            if (disable_dhcp_media_sense != 0)<br>
&gt;&gt; +            {<br>
&gt;&gt; +                ret_value = false;<br>
&gt;&gt; +            }<br>
&gt;&gt; +        }<br>
&gt;&gt; +<br>
&gt;&gt; +        RegCloseKey(tcpip_params);<br>
&gt;&gt; +    }<br>
&gt;&gt; +<br>
&gt;&gt; +    return ret_value;<br>
&gt;&gt; +}<br>
&gt;&gt; +<br>
&gt;&gt;  /*<br>
&gt;&gt;   * Check that two addresses are part of the same 255.255.255.252 subnet.<br>
&gt;&gt;   */<br>
&gt;&gt; @@ -5901,6 +5953,13 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun<br>
&gt;&gt;      {<br>
&gt;&gt;          uint32_t ep[4];<br>
&gt;&gt;<br>
&gt;&gt; +        /* Check DHCP media sense value */<br>
&gt;&gt; +        if (!tt-&gt;options.dhcp_renew &amp;&amp; !get_dhcp_media_sense())<br>
&gt;&gt; +        {<br>
&gt;&gt; +            msg(M_WARN, &quot;WARNING: DHCP media sense disabled, dhcp_renew option forced. You can enable media sense with \&quot;netsh interface ipv4 set global dhcpmediasense=enabled\&quot; shell command&quot;);<br>
&gt;&gt; +            tt-&gt;options.dhcp_renew = true;<br>
&gt;&gt; +        }<br>
&gt;&gt; +<br>
&gt;&gt;          /* We will answer DHCP requests with a reply to set IP/subnet to these values */<br>
&gt;&gt;          ep[0] = htonl(tt-&gt;local);<br>
&gt;&gt;          ep[1] = htonl(tt-&gt;adapter_netmask);<br>
&gt;<br>
&gt; Thanks,<br>
&gt;<br>
&gt; Selva<br>
</div></div></blockquote></div><br></div>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Selva Nair March 14, 2018, 3:33 a.m. | #4
Hi,

On Tue, Mar 13, 2018 at 7:36 AM, Jiří Engelthaler <engycz@gmail.com> wrote:
>
> Any discussion / opinions ? Here https://github.com/OpenVPN/openvpn/pull/97#issuecomment-372530059
> 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

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Jiří Engelthaler March 14, 2018, 7:32 a.m. | #5
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.

Jiří

2018-03-14 4:33 GMT+01:00 Selva Nair <selva.nair@gmail.com>:

> Hi,
>
> On Tue, Mar 13, 2018 at 7:36 AM, Jiří Engelthaler <engycz@gmail.com>
> wrote:
> >
> > Any discussion / opinions ? Here https://github.com/OpenVPN/
> openvpn/pull/97#issuecomment-372530059
> > 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
>
<div dir="ltr"><div>
<div><div>Hi.<br></div>  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).<br></div><div>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.</div>

<br></div>Jiří<br></div><div class="gmail_extra"><br><div class="gmail_quote">2018-03-14 4:33 GMT+01:00 Selva Nair <span dir="ltr">&lt;<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>&gt;</span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<span class=""><br>
On Tue, Mar 13, 2018 at 7:36 AM, Jiří Engelthaler &lt;<a href="mailto:engycz@gmail.com">engycz@gmail.com</a>&gt; wrote:<br>
&gt;<br>
&gt; Any discussion / opinions ? Here <a href="https://github.com/OpenVPN/openvpn/pull/97#issuecomment-372530059" rel="noreferrer" target="_blank">https://github.com/OpenVPN/<wbr>openvpn/pull/97#issuecomment-<wbr>372530059</a><br>
&gt; one user reported, that this patch were useful for him.<br>
<br>
</span>Though I agree that checking for whether media sense is globally<br>
disabled would help some, I&#39;m not convinced that this is the right<br>
approach. The media sense can be disabled on individual adaptors too<br>
isn&#39;t it? -- say tap adaptor media status set as &quot;always connected&quot;. And<br>
that won&#39;t be caught or fixed by this check.<br>
<br>
If dhcp-renew will cure this, why not just make --dhcp-renew the<br>
default? That way we get rid of another config option and no new<br>
code is needed. Is there an argument why that option should not be on<br>
by default?<br>
<span class="HOEnZb"><font color="#888888"><br>
Selva<br>
</font></span></blockquote></div><br></div>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Selva Nair March 14, 2018, 4:17 p.m. | #6
Hi,

On Wed, Mar 14, 2018 at 3:32 AM, Jiří Engelthaler <engycz@gmail.com> wrote:
> 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.

Selva

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

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