diff mbox series

[Openvpn-devel,1/2] Skip DNS address validation

Message ID 20200205124615.15758-2-domagoj@pensa.hr
State Accepted
Headers
Series Couple of fixes |

Commit Message

Domagoj Pensa Feb. 5, 2020, 1:46 a.m. UTC 
  When adding IPv4 DNS servers without interactive service use
"validate=no", on Windows 7 and higher, to skip time consuming automatic
address validation, that is on by default.

Fix uses adapted code from commit 786e06a

Signed-off-by: Domagoj Pensa <domagoj@pensa.hr>
---
 src/openvpn/tun.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Lev Stipakov Feb. 5, 2020, 4:27 a.m. UTC | #1 
Hi,

Built and tested with msvc, works as expected - "validate=no" is added to
netsh command line.

There is a similar commit in Simon's repo (not yet sent to ml) :
https://github.com/rozmansi/openvpn/commit/6b746cb0bf72a75e9963cc1a037c18cfb856702a


I haven't noticed any slowness on my machine, but since fix has been
implemented separately
by two persons and there is similar code for ipv6, I am ok with that.

Acked-by: Lev Stipakov <lstipakov@gmail.com>
<div dir="ltr"><div dir="ltr">Hi,<div><br></div><div>Built and tested with msvc, works as expected - &quot;validate=no&quot; is added to netsh command line.</div><div><br></div><div>There is a similar commit in Simon&#39;s repo (not yet sent to ml) : <a href="https://github.com/rozmansi/openvpn/commit/6b746cb0bf72a75e9963cc1a037c18cfb856702a">https://github.com/rozmansi/openvpn/commit/6b746cb0bf72a75e9963cc1a037c18cfb856702a</a>  </div><div><br></div><div>I haven&#39;t noticed any slowness on my machine, but since fix has been implemented separately</div><div>by two persons and there is similar code for ipv6, I am ok with that.</div><div><br></div><div>Acked-by: Lev Stipakov &lt;<a href="mailto:lstipakov@gmail.com">lstipakov@gmail.com</a>&gt;</div></div></div>
Selva Nair Feb. 5, 2020, 4:47 a.m. UTC | #2 
Hi,

On Wed, Feb 5, 2020 at 10:28 AM Lev Stipakov <lstipakov@gmail.com> wrote:
>
> Hi,
>
> Built and tested with msvc, works as expected - "validate=no" is added to netsh command line.
>
> There is a similar commit in Simon's repo (not yet sent to ml) : https://github.com/rozmansi/openvpn/commit/6b746cb0bf72a75e9963cc1a037c18cfb856702a
>
> I haven't noticed any slowness on my machine, but since fix has been implemented separately
> by two persons and there is similar code for ipv6, I am ok with that.

>
> Acked-by: Lev Stipakov <lstipakov@gmail.com>

We explicitly added validate=no for IPv6 in
commit 786e06ade9f5dfad8ac360499187fa8e536d15cb
for the same reason as in this patch. The ipv4 DNS code belongs to an
era when this
option was not available.

ACK from me too.


Selva

>
> Acked-by: Lev Stipakov <lstipakov@gmail.com>
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Simon Rozman Feb. 5, 2020, 10:58 p.m. UTC | #3 
Hi,

My thoughts exactly: as Lev pointed out:
https://github.com/rozmansi/openvpn/commit/6b746cb0bf72a75e9963cc1a037c18cfb
856702a

Acked-by: Simon Rozman <simon@rozman.si>

Domagoj, if it's not too much for you, maybe document the reason why DNS
validation is so slow in the commit message. My wording went like this:

> DNS validation usually fails, as the pushed routes should be added first
> to make DNS servers not part of the OpenVPN subnet reachable before
> instructing Windows to use them.

Maybe Gert can update the commit message when applying?

One day somebody might revert that DNS validation back to default, as the
long-term shot would be to upgrade the OpenVPN to setup routes first, then
configure DNS servers.

But then there's ValdikSS with thousands of routes in his .ovpn setup...

Best regards,
Simon
Domagoj Pensa Feb. 5, 2020, 11:52 p.m. UTC | #4 
Hi!

On Thu, Feb 06, 2020 at 09:58:37AM +0000, Simon Rozman wrote:
> Hi,
> 
> My thoughts exactly: as Lev pointed out:
> https://github.com/rozmansi/openvpn/commit/6b746cb0bf72a75e9963cc1a037c18cfb
> 856702a
> 
> Acked-by: Simon Rozman <simon@rozman.si>
> 
> Domagoj, if it's not too much for you, maybe document the reason why DNS
> validation is so slow in the commit message. My wording went like this:
> 
> > DNS validation usually fails, as the pushed routes should be added first
> > to make DNS servers not part of the OpenVPN subnet reachable before
> > instructing Windows to use them.
> 
> Maybe Gert can update the commit message when applying?

Absolutely, Gert can add your additional description in the commit.

Regards,
Domagoj
Domagoj Pensa Feb. 11, 2020, 9:45 p.m. UTC | #5 
Hi!

My I ask if there is anything else I can (or should) do regarding this 
patch? Perhaps send patch again with revised/updated description as 
suggested by Simon?

Thank you!

Regards,
Domagoj
Gert Doering March 13, 2020, 8:17 a.m. UTC | #6 
Your patch has been applied to the master branch.

Sorry for the delay, it's been a very busy month.

I have not done any testing besides "test compile", but the code looks good 
and I trust Lev and Silva here.

commit 04f4b4feec2790b620419bdc4fa2c7ae4f2451bd
Author: Domagoj Pensa
Date:   Wed Feb 5 13:46:14 2020 +0100

     Skip DNS address validation

     Signed-off-by: Domagoj Pensa <domagoj@pensa.hr>
     Acked-by: Lev Stipakov <lstipakov@gmail.com>
     Acked-by: Selva Nair <selva.nair@gmail.com>
     Message-Id: <20200205124615.15758-2-domagoj@pensa.hr>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19355.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
Domagoj Pensa March 15, 2020, 8:35 p.m. UTC | #7 
On Fri, Mar 13, 2020 at 08:17:15PM +0100, Gert Doering wrote:
> Your patch has been applied to the master branch.
> 
> Sorry for the delay, it's been a very busy month.
> 

No problem at all.
Thank you for applying patch! :)

Regards,
Domagoj
diff mbox series

Patch

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index af09e676..9f369f74 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -5216,6 +5216,7 @@  netsh_ifconfig_options(const char *type,
     struct gc_arena gc = gc_new();
     struct argv argv = argv_new();
     bool delete_first = false;
+    bool is_dns = !strcmp(type, "dns");
 
     /* first check if we should delete existing DNS/WINS settings from TAP interface */
     if (test_first)
@@ -5259,6 +5260,14 @@  netsh_ifconfig_options(const char *type,
                             type,
                             flex_name,
                             print_in_addr_t(addr_list[i], 0, &gc));
+
+                /* disable slow address validation on Windows 7 and higher */
+                /* only for DNS */
+                if (is_dns && win32_version_info() >= WIN_7)
+                {
+                    argv_printf_cat(&argv, "%s", "validate=no");
+                }
+
                 netsh_command(&argv, 2, M_FATAL);
 
                 ++count;