diff mbox series

[Openvpn-devel,v3,2/8] windows: properly configure TAP driver when no IPv4 is configured

Message ID 20180613141213.5962-1-a@unstable.cc
State Changes Requested
Headers show
Series None | expand

Commit Message

Antonio Quartulli June 13, 2018, 4:12 a.m. UTC 
From: Antonio Quartulli <antonio@openvpn.net>

This patch ensures that the TAP driver on a windows host is still
configured, even though no IPv4 has been provided.

In this case the TAP driver ioctl will be invoked with a fake
0.0.0.0/0.0.0.0 IPv4 which will simply start the interface and
get it to a working state.

Trac: #208
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>

---

Changes from v2:
- rebased on top ov v4 1/8
- extended commit message
- moved did_ifconfig_setup check into test_routes()
- added longer comment into test_routes() to explain check condition


 src/openvpn/route.c |  6 +++++-
 src/openvpn/tun.c   | 24 +++++++++++++++++-------
 2 files changed, 22 insertions(+), 8 deletions(-)

Comments

Gert Doering June 18, 2018, 8:40 a.m. UTC | #1 
Hi,

On Wed, Jun 13, 2018 at 10:12:13PM +0800, Antonio Quartulli wrote:
> From: Antonio Quartulli <antonio@openvpn.net>
> 
> This patch ensures that the TAP driver on a windows host is still
> configured, even though no IPv4 has been provided.
> 
> In this case the TAP driver ioctl will be invoked with a fake
> 0.0.0.0/0.0.0.0 IPv4 which will simply start the interface and
> get it to a working state.
> 
> Trac: #208
> Signed-off-by: Antonio Quartulli <antonio@openvpn.net>

I think this is good enough, though I haven't tested it yet (compile
and run).  Will do that tomorrow, before I merge & push.

Before I go ahead with it - Selva, are you fine with this version?

gert
Selva Nair June 18, 2018, 8:52 a.m. UTC | #2 
On Mon, Jun 18, 2018 at 2:40 PM, Gert Doering <gert@greenie.muc.de> wrote:
>
> Hi,
>
> On Wed, Jun 13, 2018 at 10:12:13PM +0800, Antonio Quartulli wrote:
> > From: Antonio Quartulli <antonio@openvpn.net>
> >
> > This patch ensures that the TAP driver on a windows host is still
> > configured, even though no IPv4 has been provided.
> >
> > In this case the TAP driver ioctl will be invoked with a fake
> > 0.0.0.0/0.0.0.0 IPv4 which will simply start the interface and
> > get it to a working state.
> >
> > Trac: #208
> > Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
>
> I think this is good enough, though I haven't tested it yet (compile
> and run).  Will do that tomorrow, before I merge & push.
>
> Before I go ahead with it - Selva, are you fine with this version?

I believe this is the same as the gitlab version I had tested and
commented on. I only had some opinionated remarks (nothing critical),
so this should be good to merge.

Selva

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Gert Doering June 20, 2018, 5:53 a.m. UTC | #3 
Hi,

On Wed, Jun 13, 2018 at 10:12:13PM +0800, Antonio Quartulli wrote:
> From: Antonio Quartulli <antonio@openvpn.net>
> 
> This patch ensures that the TAP driver on a windows host is still
> configured, even though no IPv4 has been provided.
> 
> In this case the TAP driver ioctl will be invoked with a fake
> 0.0.0.0/0.0.0.0 IPv4 which will simply start the interface and
> get it to a working state.
> 
> Trac: #208
> Signed-off-by: Antonio Quartulli <antonio@openvpn.net>

Well...

> @@ -2642,7 +2642,11 @@ test_routes(const struct route_list *rl, const struct tuntap *tt)
>          ret = true;
>          adapter_up = true;
>  
> -        if (rl)
> +        /* we do this test only if we have IPv4 routes to install, and if
> +         * the tun/tap interface has seen IPv4 ifconfig - because if we
> +         * have no IPv4, the check will always fail, failing tun init
> +         */
> +        if (rl && c->c1.tuntap->did_ifconfig_setup)

This code is a bit optimstic :-) - there is no "c" here...

route.c: In function 'test_routes':
route.c:2649:19: error: 'c' undeclared (first use in this function)
         if (rl && c->c1.tuntap->did_ifconfig_setup)
                   ^

... because it's just "tt" here, so "tt->did_ifconfig_setup" should
be fine...  at least it compiled :-) - I've sent you a link to an 
installer for testing.

Amended patch attached

gert
Selva Nair June 20, 2018, 9:31 a.m. UTC | #4 
Hi,

On Wed, Jun 20, 2018 at 11:53 AM, Gert Doering <gert@greenie.muc.de> wrote:
> Hi,
>
> On Wed, Jun 13, 2018 at 10:12:13PM +0800, Antonio Quartulli wrote:
>> From: Antonio Quartulli <antonio@openvpn.net>
>>
>> This patch ensures that the TAP driver on a windows host is still
>> configured, even though no IPv4 has been provided.
>>
>> In this case the TAP driver ioctl will be invoked with a fake
>> 0.0.0.0/0.0.0.0 IPv4 which will simply start the interface and
>> get it to a working state.
>>
>> Trac: #208
>> Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
>
> Well...
>
>> @@ -2642,7 +2642,11 @@ test_routes(const struct route_list *rl, const struct tuntap *tt)
>>          ret = true;
>>          adapter_up = true;
>>
>> -        if (rl)
>> +        /* we do this test only if we have IPv4 routes to install, and if
>> +         * the tun/tap interface has seen IPv4 ifconfig - because if we
>> +         * have no IPv4, the check will always fail, failing tun init
>> +         */
>> +        if (rl && c->c1.tuntap->did_ifconfig_setup)
>
> This code is a bit optimstic :-) - there is no "c" here...

When did this change?  In a previous version this was done as

diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 7d9a338..51e43c2 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -292,7 +292,9 @@ check_add_routes_action(struct context *c, const
bool errors)
 void
 check_add_routes_dowork(struct context *c)
 {
-    if (test_routes(c->c1.route_list, c->c1.tuntap))
+    /* skip route check if no IPv4 was configured */
+    if (!c->c1.tuntap->did_ifconfig_setup
+        || test_routes(c->c1.route_list, c->c1.tuntap))
     {
         check_add_routes_action(c, false);
     }

That said, moving this to route.c but corrected as in Gert's
attachment looks better.

Selva

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Gert Doering June 20, 2018, 11:11 a.m. UTC | #5 
Hi,

On Wed, Jun 20, 2018 at 03:31:39PM -0400, Selva Nair wrote:
> > This code is a bit optimstic :-) - there is no "c" here...
> 
> When did this change?  In a previous version this was done as

v2 to v3, because I complained that check_add_routes_dowork() is 
hard enough to read as it is, and suggested to put the check
elsewhere.

All my fault :-)

gert
Antonio Quartulli June 21, 2018, 9:27 p.m. UTC | #6 
Hi,

On 20/06/18 23:53, Gert Doering wrote:
[cut]
> 
> ... because it's just "tt" here, so "tt->did_ifconfig_setup" should
> be fine...  at least it compiled :-) - I've sent you a link to an 
> installer for testing.
>

Thanks for the installer Gert!
I just tried it on Windows 10 (not my laptop! just in case somebody
thinks I use Windows :-P) and unfortunately it does not work as expected.

The failure is reported as:

TUN: adding address failed using service: The parameter is incorrect.
[status=87 if_index=42]

I have to double check the code to understand what is different from
what Selva tested. In the meantime, you have the full log attached to
this email in case somebody wants to give it a look.


Cheers,
Gert Doering June 21, 2018, 11:39 p.m. UTC | #7 
HI,

On Fri, Jun 22, 2018 at 03:27:02PM +0800, Antonio Quartulli wrote:
> The failure is reported as:
> 
> TUN: adding address failed using service: The parameter is incorrect.
> [status=87 if_index=42]

This is surprising.  I initially assumed that this is related to IPv4,
but we do not even use the service for IPv4 today...

This *might* trigger something that the iservice logs to the windows
event log - so it might be worth having a look :-)

gert

PS: as a side note - with XP and Vista dropped, we might consider ripping
out every win32-api besides IPAPI and iservice, for 2.5...
Gert Doering June 21, 2018, 11:46 p.m. UTC | #8 
Hi,

On Fri, Jun 22, 2018 at 03:27:02PM +0800, Antonio Quartulli wrote:
> Fri Jun 22 13:43:51 2018 us=116232 PUSH: Received control message: 'PUSH_REPLY,redirect-gateway !ipv4 ipv6,tun-ipv6,ping 10,ping-restart 120,ifconfig-ipv6 2002::1001/112 2002::2,peer-id 0,cipher AES-256-GCM'

As an afterthought - the error could be totally unrelated to the changes,
and the win32 API refusing 2002:: addresses because it knows that they
are not "free for grabs"...  (6to4, normally auto-configured unless
disabled, with 2002:<ipv4>:: being the prefix that a host on <ipv4> or
a network behind a router on <ipv4> can use - but that's old stuff and
should no longer be used in any case).

gert
Antonio Quartulli June 21, 2018, 11:49 p.m. UTC | #9 
Hi,

On 22/06/18 17:46, Gert Doering wrote:
> Hi,
> 
> On Fri, Jun 22, 2018 at 03:27:02PM +0800, Antonio Quartulli wrote:
>> Fri Jun 22 13:43:51 2018 us=116232 PUSH: Received control message: 'PUSH_REPLY,redirect-gateway !ipv4 ipv6,tun-ipv6,ping 10,ping-restart 120,ifconfig-ipv6 2002::1001/112 2002::2,peer-id 0,cipher AES-256-GCM'
> 
> As an afterthought - the error could be totally unrelated to the changes,
> and the win32 API refusing 2002:: addresses because it knows that they
> are not "free for grabs"...  (6to4, normally auto-configured unless
> disabled, with 2002:<ipv4>:: being the prefix that a host on <ipv4> or
> a network behind a router on <ipv4> can use - but that's old stuff and
> should no longer be used in any case).

After changing the prefix to 2001:db8:cafe::/112 the error is gone.

However, the assert at the end of the log in argv.c:299 is still there.
This might be some argv_new()/reset() that has been misplaced (maybe?).

Cheers,
Selva Nair June 22, 2018, 8:12 a.m. UTC | #10 
Hi,

On Fri, Jun 22, 2018 at 5:49 AM, Antonio Quartulli <a@unstable.cc> wrote:

> Hi,
>
> On 22/06/18 17:46, Gert Doering wrote:
> > Hi,
> >
> > On Fri, Jun 22, 2018 at 03:27:02PM +0800, Antonio Quartulli wrote:
> >> Fri Jun 22 13:43:51 2018 us=116232 PUSH: Received control message:
> 'PUSH_REPLY,redirect-gateway !ipv4 ipv6,tun-ipv6,ping 10,ping-restart
> 120,ifconfig-ipv6 2002::1001/112 2002::2,peer-id 0,cipher AES-256-GCM'
> >
> > As an afterthought - the error could be totally unrelated to the changes,
> > and the win32 API refusing 2002:: addresses because it knows that they
> > are not "free for grabs"...  (6to4, normally auto-configured unless
> > disabled, with 2002:<ipv4>:: being the prefix that a host on <ipv4> or
> > a network behind a router on <ipv4> can use - but that's old stuff and
> > should no longer be used in any case).
>
> After changing the prefix to 2001:db8:cafe::/112 the error is gone.
>
> However, the assert at the end of the log in argv.c:299 is still there.
> This might be some argv_new()/reset() that has been misplaced (maybe?).
>

While the assert is definitely due to buggy code (as discussed in the other
thread),
I cant reproduce this. Both the version I had tested earlier and the v2/v3
patches
work without throwing this error.

Earlier I had tested with

pull-filter ignore "ifconfig "
route 192.168.0.0  255.255.255.0                    # test a tunnel route
route 192.168.1.0 255.255.255.0 net_gateway  # test an external route
redirect-gateway ipv6 !ipv4

Now retested by adding

pull-filter ignore route-gateway

as otherwise the v4 tunnel gateway was not null and the v4 route succeeded
with a simple warning (gateway unreacahble). With the route gateway removed,
the v4 route fails with

Fri Jun 22 13:52:08 2018 OpenVPN ROUTE: OpenVPN needs a gateway parameter
for a --route option and no default was specified by either --route-gateway
or --ifconfig options
Fri Jun 22 13:52:08 2018 OpenVPN ROUTE: failed to parse/resolve route for
host/network: 192.168.0.0
Fri Jun 22 13:52:08 2018 GDG6: remote_host_ipv6=n/a
Fri Jun 22 13:52:08 2018 GetBestInterfaceEx() returned if=15
Fri Jun 22 13:52:08 2018 GDG6: II=15 DP=::/0 NH=fe80::2d0:b7ff:febe:d8bc
Fri Jun 22 13:52:08 2018 GDG6: Metric=256, Loopback=0, AA=1, I=0
Fri Jun 22 13:52:08 2018 ROUTE6_GATEWAY fe80::2d0:b7ff:febe:d8bc I=15

Again, no assert.

Possibly you have some v4 route that triggers the assert, but I can't
figure which.
Or are you testing using a v6 only server -- even if so, its not clear how
that would
matter.

My tap adapter has a link local address (169.254.98.86) on it possibly due
to a previous dhcp failure. May be I need to get rid of that (how?) to
activate
this code path?

Selva
<div dir="ltr">Hi,<br><div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jun 22, 2018 at 5:49 AM, Antonio Quartulli <span dir="ltr">&lt;<a href="mailto:a@unstable.cc" target="_blank">a@unstable.cc</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br>
<span class="gmail-"><br>
On 22/06/18 17:46, Gert Doering wrote:<br>
&gt; Hi,<br>
&gt; <br>
&gt; On Fri, Jun 22, 2018 at 03:27:02PM +0800, Antonio Quartulli wrote:<br>
&gt;&gt; Fri Jun 22 13:43:51 2018 us=116232 PUSH: Received control message: &#39;PUSH_REPLY,redirect-gateway !ipv4 ipv6,tun-ipv6,ping 10,ping-restart 120,ifconfig-ipv6 2002::1001/112 2002::2,peer-id 0,cipher AES-256-GCM&#39;<br>
&gt; <br>
&gt; As an afterthought - the error could be totally unrelated to the changes,<br>
&gt; and the win32 API refusing 2002:: addresses because it knows that they<br>
&gt; are not &quot;free for grabs&quot;...  (6to4, normally auto-configured unless<br>
&gt; disabled, with 2002:&lt;ipv4&gt;:: being the prefix that a host on &lt;ipv4&gt; or<br>
&gt; a network behind a router on &lt;ipv4&gt; can use - but that&#39;s old stuff and<br>
&gt; should no longer be used in any case).<br>
<br>
</span>After changing the prefix to 2001:db8:cafe::/112 the error is gone.<br>
<br>
However, the assert at the end of the log in argv.c:299 is still there.<br>
This might be some argv_new()/reset() that has been misplaced (maybe?).<br></blockquote><div><br></div><div>While the assert is definitely due to buggy code (as discussed in the other thread),<br></div><div>I cant reproduce this. Both the version I had tested earlier and the v2/v3 patches<br></div><div>work without throwing this error.<br><br></div><div>Earlier I had tested with <br><br>pull-filter ignore &quot;ifconfig &quot; <br>route 192.168.0.0  255.255.255.0                    # test a tunnel route<br>route 192.168.1.0 255.255.255.0 net_gateway  # test an external route<br>redirect-gateway ipv6 !ipv4<br><br>Now retested by adding<br><br>pull-filter ignore route-gateway<br></div><div><br>as otherwise the v4 tunnel gateway was not null and the v4 route succeeded<br>with a simple warning (gateway unreacahble). With the route gateway removed,<br>the v4 route fails with<br><br>Fri Jun 22 13:52:08 2018 OpenVPN ROUTE: OpenVPN needs a gateway parameter for a --route option and no default was specified by either --route-gateway or --ifconfig options<br>Fri Jun 22 13:52:08 2018 OpenVPN ROUTE: failed to parse/resolve route for host/network: 192.168.0.0<br>Fri Jun 22 13:52:08 2018 GDG6: remote_host_ipv6=n/a<br>Fri Jun 22 13:52:08 2018 GetBestInterfaceEx() returned if=15<br>Fri Jun 22 13:52:08 2018 GDG6: II=15 DP=::/0 NH=fe80::2d0:b7ff:febe:d8bc<br>Fri Jun 22 13:52:08 2018 GDG6: Metric=256, Loopback=0, AA=1, I=0<br>Fri Jun 22 13:52:08 2018 ROUTE6_GATEWAY fe80::2d0:b7ff:febe:d8bc I=15<br><br></div><div>Again, no assert.<br><br></div><div>Possibly you have some v4 route that triggers the assert, but I can&#39;t figure which.<br></div><div>Or are you testing using a v6 only server -- even if so, its not clear how that would<br>matter.<br></div><div><br></div><div>My tap adapter has a link local address (169.254.98.86) on it possibly due<br>to a previous dhcp failure. May be I need to get rid of that (how?) to activate<br>this code path?<br></div><div><br></div><div>Selva<br></div></div></div></div></div>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Gert Doering June 22, 2018, 8:27 a.m. UTC | #11 
Hi,

On Fri, Jun 22, 2018 at 02:12:24PM -0400, Selva Nair wrote:
> My tap adapter has a link local address (169.254.98.86) on it possibly due
> to a previous dhcp failure. May be I need to get rid of that (how?) to
> activate this code path?

I guess this is related - the branch in question is triggered by
is_on_link(), so maybe in Antonio's case the tap adapter is set to
0.0.0.0 - or it actually tried to install a route related to
redirect-gateway.

Are you connecting to your VPN server over IPv6 or IPv4?  You already
spotted one weirdness with "redirect-gateway !ipv4" which still seems
to setup "something" in the route_list - so maybe it's related to that,
but needs "VPN over IPv4" to trigger.

In other words, no idea - hoping for a log from Antonio that shows the 
"route.exe ..." call with "IF <ifindex>" so we can see what openvpn
tried to do :-)

gert
Selva Nair June 22, 2018, 8:52 a.m. UTC | #12 
On Fri, Jun 22, 2018 at 2:27 PM, Gert Doering <gert@greenie.muc.de> wrote:
> Hi,
>
> On Fri, Jun 22, 2018 at 02:12:24PM -0400, Selva Nair wrote:
>> My tap adapter has a link local address (169.254.98.86) on it possibly due
>> to a previous dhcp failure. May be I need to get rid of that (how?) to
>> activate this code path?
>
> I guess this is related - the branch in question is triggered by
> is_on_link(), so maybe in Antonio's case the tap adapter is set to
> 0.0.0.0 - or it actually tried to install a route related to
> redirect-gateway.
>
> Are you connecting to your VPN server over IPv6 or IPv4?

This physical link is v4.

I tried without !ipv4 in redirect-gateway and got:

(i) without route-gateway defined: redirecting v4 fails with a warning
"route-gateway or ifconfig is missing"

(ii) with route-gateway: the route is added but warns "route gateway
not reachable by any active
interfaces" -- fair enough.

Selva

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Antonio Quartulli June 22, 2018, 4:07 p.m. UTC | #13 
Hi,

On 23/06/18 02:27, Gert Doering wrote:
> Hi,
> 
> On Fri, Jun 22, 2018 at 02:12:24PM -0400, Selva Nair wrote:
>> My tap adapter has a link local address (169.254.98.86) on it possibly due
>> to a previous dhcp failure. May be I need to get rid of that (how?) to
>> activate this code path?
> 
> I guess this is related - the branch in question is triggered by
> is_on_link(), so maybe in Antonio's case the tap adapter is set to
> 0.0.0.0 - or it actually tried to install a route related to
> redirect-gateway.
> 
> Are you connecting to your VPN server over IPv6 or IPv4?  You already
> spotted one weirdness with "redirect-gateway !ipv4" which still seems
> to setup "something" in the route_list - so maybe it's related to that,
> but needs "VPN over IPv4" to trigger.
> 
> In other words, no idea - hoping for a log from Antonio that shows the 
> "route.exe ..." call with "IF <ifindex>" so we can see what openvpn
> tried to do :-)
> 

attached you have the log, however there is no message containing
"route.exe ..".

Gert, the ASSERT was triggered with your binary: was it simply master +
2/8 (this patch)?

What I used for my last test is:
* master +
* all the remaining patches from ipv6-only +
* the fix for argv_printf()

The server side is running master + ipv6-only and there is no --server
directive in the configuration.

In the log "Ethernet 2" is the TAP interface".

My guts say that the issue might be related to "gateway-redirect",
because only 8/8 makes sure that no logic is activated for IPv4 and
without that patch there might be some IPv4 route being installed
nonetheless.

Cheers,
Selva Nair June 22, 2018, 5:09 p.m. UTC | #14 
Hi,

On Fri, Jun 22, 2018 at 10:07 PM, Antonio Quartulli <a@unstable.cc> wrote:
> Hi,
>
> On 23/06/18 02:27, Gert Doering wrote:
>> Hi,
>>
>> On Fri, Jun 22, 2018 at 02:12:24PM -0400, Selva Nair wrote:
>>> My tap adapter has a link local address (169.254.98.86) on it possibly due
>>> to a previous dhcp failure. May be I need to get rid of that (how?) to
>>> activate this code path?
>>
>> I guess this is related - the branch in question is triggered by
>> is_on_link(), so maybe in Antonio's case the tap adapter is set to
>> 0.0.0.0 - or it actually tried to install a route related to
>> redirect-gateway.
>>
>> Are you connecting to your VPN server over IPv6 or IPv4?  You already
>> spotted one weirdness with "redirect-gateway !ipv4" which still seems
>> to setup "something" in the route_list - so maybe it's related to that,
>> but needs "VPN over IPv4" to trigger.
>>
>> In other words, no idea - hoping for a log from Antonio that shows the
>> "route.exe ..." call with "IF <ifindex>" so we can see what openvpn
>> tried to do :-)
>>
>
> attached you have the log, however there is no message containing
> "route.exe ..".

No assert (with argv patch applied?) and no sign of any v4 routes.

>
> Gert, the ASSERT was triggered with your binary: was it simply master +
> 2/8 (this patch)?
>
> What I used for my last test is:
> * master +
> * all the remaining patches from ipv6-only +
> * the fix for argv_printf()

So I guess there wont be any assert with such a build even if the argv patch
is left out. That would match with what I re-tested.

>
> The server side is running master + ipv6-only and there is no --server
> directive in the configuration.
>
> In the log "Ethernet 2" is the TAP interface".
>
> My guts say that the issue might be related to "gateway-redirect",
> because only 8/8 makes sure that no logic is activated for IPv4 and
> without that patch there might be some IPv4 route being installed
> nonetheless.

Very likely. It would be nice to find the on-link route that triggers that code
path to ensure it doesn't cause any other issues.

But not very critical as the assert is fixed and route errors are not FATAL.

Selva

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Antonio Quartulli June 22, 2018, 5:13 p.m. UTC | #15 
Hi,

On 23/06/18 11:09, Selva Nair wrote:
>> My guts say that the issue might be related to "gateway-redirect",
>> because only 8/8 makes sure that no logic is activated for IPv4 and
>> without that patch there might be some IPv4 route being installed
>> nonetheless.
> 
> Very likely. It would be nice to find the on-link route that triggers that code
> path to ensure it doesn't cause any other issues.

My server was on the same LAN (connecting via v4) as the client - should
that be considered on-link? maybe this is what triggers that code path.

> 
> But not very critical as the assert is fixed and route errors are not FATAL.
> 

If you like the argv patch v2, would you mind giving your blessing? :-)

Thanks!
Cheers,

>
Gert Doering June 22, 2018, 9:33 p.m. UTC | #16 
Hi,

On Sat, Jun 23, 2018 at 11:13:07AM +0800, Antonio Quartulli wrote:
> > Very likely. It would be nice to find the on-link route that triggers that code
> > path to ensure it doesn't cause any other issues.
> 
> My server was on the same LAN (connecting via v4) as the client - should
> that be considered on-link? maybe this is what triggers that code path.

Oh, definitely.  I had the feeling it was related to "server over v4",
but that would trigger "on-link" too.

Actually it might trigger the on-link ASSERT for the "ipv4 and ipv6"
case as well, as soon as "redirect-gateway ipv4" is involved.

gert
diff mbox series

Patch

diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index 8990a986..be07bd65 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -2642,7 +2642,11 @@  test_routes(const struct route_list *rl, const struct tuntap *tt)
         ret = true;
         adapter_up = true;
 
-        if (rl)
+        /* we do this test only if we have IPv4 routes to install, and if
+         * the tun/tap interface has seen IPv4 ifconfig - because if we
+         * have no IPv4, the check will always fail, failing tun init
+         */
+        if (rl && c->c1.tuntap->did_ifconfig_setup)
         {
             struct route_ipv4 *r;
             for (r = rl->routes, len = 0; r; r = r->next, ++len)
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 878fd845..c0d1ac18 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -5703,7 +5703,10 @@  open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun
             msg(M_FATAL, "ERROR: --dev tun also requires --ifconfig");
         }
 
-        if (tt->topology == TOP_SUBNET)
+        /* send 0/0/0 to the TAP driver even if we have no IPv4 configured to
+         * ensure it is somehow initialized.
+         */
+        if (!tt->did_ifconfig_setup || tt->topology == TOP_SUBNET)
         {
             in_addr_t ep[3];
             BOOL status;
@@ -5716,12 +5719,19 @@  open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun
                                      ep, sizeof(ep),
                                      ep, sizeof(ep), &len, NULL);
 
-            msg(status ? M_INFO : M_FATAL, "Set TAP-Windows TUN subnet mode network/local/netmask = %s/%s/%s [%s]",
-                print_in_addr_t(ep[1], IA_NET_ORDER, &gc),
-                print_in_addr_t(ep[0], IA_NET_ORDER, &gc),
-                print_in_addr_t(ep[2], IA_NET_ORDER, &gc),
-                status ? "SUCCEEDED" : "FAILED");
-
+            if (tt->did_ifconfig_setup)
+            {
+                msg(status ? M_INFO : M_FATAL, "Set TAP-Windows TUN subnet mode network/local/netmask = %s/%s/%s [%s]",
+                    print_in_addr_t(ep[1], IA_NET_ORDER, &gc),
+                    print_in_addr_t(ep[0], IA_NET_ORDER, &gc),
+                    print_in_addr_t(ep[2], IA_NET_ORDER, &gc),
+                    status ? "SUCCEEDED" : "FAILED");
+            }
+            else
+            {
+                msg(status ? M_INFO : M_FATAL, "Set TAP-Windows TUN with fake IPv4 [%s]",
+                    status ? "SUCCEEDED" : "FAILED");
+            }
         }
         else
         {