| Message ID | 20171205121952.13008-1-EngyCZ@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [Openvpn-devel] Add a warning for disabled DHCP media sense on Window | expand |
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
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);
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"><<a href="mailto:engycz@gmail.com" target="_blank">engycz@gmail.com</a>></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> "netsh interface ipv4 show global"<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 <<a href="mailto:EngyCZ@gmail.com">EngyCZ@gmail.com</a>><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="">"SYSTEM\\CurrentControlSet\\<wbr>Services\\Tcpip\\Parameters";<br> </span>+ const char disable_dhcp_media_sense_<wbr>string[] = "DisableDHCPMediaSense";<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, &tcpip_params);<br> <span class="">+<br> + if (status != ERROR_SUCCESS)<br> + {<br> </span>+ msg(M_WARN, "Error opening registry key: %s", 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, &data_type,<br> (PBYTE)&disable_dhcp_media_<wbr>sense, &len);<br> <span class="">+<br> + if (status == ERROR_SUCCESS && 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->options.dhcp_renew && !get_dhcp_media_sense())<br> + {<br> + msg(M_WARN, "WARNING: DHCP media sense disabled,<br> dhcp_renew option forced. You can enable media sense with \"netsh<br> interface ipv4 set global dhcpmediasense=enabled\" shell command");<br> + tt->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->local);<br> ep[1] = htonl(tt->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 <<a href="mailto:selva.nair@gmail.com">selva.nair@gmail.com</a>>:<br> > Hi,<br> ><br> > I haven't tested this, but looks like a useful thing to do. The code<br> > looks good too. Some minor comments below:<br> ><br> > On Tue, Dec 5, 2017 at 7:19 AM, Jiří Engelthaler <<a href="mailto:EngyCZ@gmail.com">EngyCZ@gmail.com</a>> wrote:<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> >> "netsh interface ipv4 show global"<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> > Typo in commit header: "Window" should be "Windows"<br> ><br> >><br> >> Signed-off-by: Jiří Engelthaler <<a href="mailto:EngyCZ@gmail.com">EngyCZ@gmail.com</a>><br> >> ---<br> >> src/openvpn/tun.c | 59 ++++++++++++++++++++++++++++++<wbr>+++++++++++++++++++++++++<br> >> 1 file changed, 59 insertions(+)<br> >><br> >> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c<br> >> index 25831ce3..35811975 100644<br> >> --- a/src/openvpn/tun.c<br> >> +++ b/src/openvpn/tun.c<br> >> @@ -62,6 +62,8 @@<br> >> #define NI_IP_NETMASK (1<<1)<br> >> #define NI_OPTIONS (1<<2)<br> >><br> >> +#define TCPIP_PARAMS "SYSTEM\\CurrentControlSet\\<wbr>Services\\Tcpip\\Parameters"<br> >> +<br> ><br> > This is only used inside get_dhcp_media_sense. Better move it there as<br> > a const char *. Less macros, the better.<br> ><br> >> static void netsh_ifconfig(const struct tuntap_options *to,<br> >> const char *flex_name,<br> >> const in_addr_t ip,<br> >> @@ -3827,6 +3829,56 @@ get_panel_reg(struct gc_arena *gc)<br> >> return first;<br> >> }<br> >><br> >> +/*<br> >> + * Return DhcpMediaSense enabled value<br> >> + */<br> >> +static bool<br> >> +get_dhcp_media_sense(void)<br> >> +{<br> >> + HKEY tcpip_params;<br> >> + LONG status;<br> >> + DWORD len;<br> >> + char disable_dhcp_media_sense_<wbr>string[] = "DisableDHCPMediaSense";<br> ><br> > This would benefit from a const.<br> ><br> >> + DWORD disable_dhcp_media_sense;<br> >> + DWORD data_type;<br> >> + bool ret_value = true;<br> >> +<br> >> + status = RegOpenKeyEx(<br> >> + HKEY_LOCAL_MACHINE,<br> >> + TCPIP_PARAMS,<br> >> + 0,<br> >> + KEY_READ,<br> >> + &tcpip_params);<br> ><br> > I'm not sure whether its specified in the newly adopted style, but<br> > please avoid writing one argument per line. Original has many such,<br> > but not preferred in new code.<br> ><br> >> +<br> >> + if (status != ERROR_SUCCESS)<br> >> + {<br> >> + msg(M_WARN, "Error opening registry key: %s", TCPIP_PARAMS);<br> >> + }<br> >> + else<br> >> + {<br> >> + len = sizeof(disable_dhcp_media_<wbr>sense);<br> >> + status = RegQueryValueEx(<br> >> + tcpip_params,<br> >> + disable_dhcp_media_sense_<wbr>string,<br> >> + NULL,<br> >> + &data_type,<br> >> + (PBYTE)&disable_dhcp_media_<wbr>sense,<br> >> + &len);<br> ><br> > Same here.<br> ><br> >> +<br> >> + if (status == ERROR_SUCCESS && 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> >> @@ -5901,6 +5953,13 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun<br> >> {<br> >> uint32_t ep[4];<br> >><br> >> + /* Check DHCP media sense value */<br> >> + if (!tt->options.dhcp_renew && !get_dhcp_media_sense())<br> >> + {<br> >> + 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");<br> >> + tt->options.dhcp_renew = true;<br> >> + }<br> >> +<br> >> /* We will answer DHCP requests with a reply to set IP/subnet to these values */<br> >> ep[0] = htonl(tt->local);<br> >> ep[1] = htonl(tt->adapter_netmask);<br> ><br> > Thanks,<br> ><br> > 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
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
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"><<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>></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 <<a href="mailto:engycz@gmail.com">engycz@gmail.com</a>> wrote:<br> ><br> > 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> > 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'm not convinced that this is the right<br> approach. The media sense can be disabled on individual adaptors too<br> isn't it? -- say tap adaptor media status set as "always connected". And<br> that won'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
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
Am 14.03.18 um 17:17 schrieb Selva Nair: > 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. 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. Arne
Hi, On Thu, Feb 13, 2020 at 03:53:04PM +0100, Arne Schwabe wrote: > 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. I am now going to close it. This seems to be a fairly rare problem, and if "--dhcp-renew" fixes it without extra code, this sounds like an acceptable solution. If Jiri wants this to be merged, the process would be "re-send a v2, rebased on current master, with the changes Selva suggested integrated". (We do not do pull requests, so I have not looked, and will not look, whether PR 97 has/had an updated patch already) gert
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);
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(+)