[Openvpn-devel] Fwd: [PATCH] Delete the IPv6 route to the "connected" network on tun close

Message ID CAKuzo_gBWiY==v8bjuaOgq8F2R=gq0a5RNN28VvXphj1p2e2vA@mail.gmail.com
State Superseded
Headers show
Series
  • [Openvpn-devel] Fwd: [PATCH] Delete the IPv6 route to the "connected" network on tun close
Related show

Commit Message

Selva Nair March 1, 2018, 4:09 p.m.
This one is too old to cleanly apply, but still sending again
just to get it into patchwork.
(For some reason bouncing to patchwork somehow never works for me, else
this could have beaten the oldest pending patch record :)

---------- Forwarded message ----------
From: Selva Nair <selva.nair@gmail.com>
Date: Thu, Nov 24, 2016 at 4:19 PM
Subject: [PATCH] Delete the IPv6 route to the "connected" network on tun close
To: openvpn-devel@lists.sourceforge.net
Cc: Selva Nair <selva.nair@gmail.com>


This was missing on Windows when interactive service is in use.

- Added route_ipv6_clear_host_bits(r6) to delete_route_ipv6: this is
  required for Windows IP-helper API. Won't hurt other platforms (?)

Signed-off-by: Selva Nair <selva.nair@gmail.com>
---
 src/openvpn/route.c | 2 ++
 src/openvpn/tun.c   | 3 +++
 2 files changed, 5 insertions(+)

--
2.1.4

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

Comments

Gert Doering March 1, 2018, 5:51 p.m. | #1
Hi,

On Thu, Mar 01, 2018 at 11:09:32AM -0500, Selva Nair wrote:
> This one is too old to cleanly apply, but still sending again
> just to get it into patchwork.

Oh, completely fell of my radar.  But now that you mention it, yes, 
there was something about clearing bits :-)

> (For some reason bouncing to patchwork somehow never works for me, else
> this could have beaten the oldest pending patch record :)

Bouncing my own usually works, bouncing others "sometimes".  Might be
related to SPF records and/or DKIM/DMARC checks...

[..]
> This was missing on Windows when interactive service is in use.
> 
> - Added route_ipv6_clear_host_bits(r6) to delete_route_ipv6: this is
>   required for Windows IP-helper API. Won't hurt other platforms (?)
> 
> Signed-off-by: Selva Nair <selva.nair@gmail.com>
> ---
>  src/openvpn/route.c | 2 ++
>  src/openvpn/tun.c   | 3 +++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/src/openvpn/route.c b/src/openvpn/route.c
> index fec12c1..85f969e 100644
> --- a/src/openvpn/route.c
> +++ b/src/openvpn/route.c
> @@ -2124,6 +2124,8 @@ delete_route_ipv6 (const struct route_ipv6 *r6,
> const struct tuntap *tt, unsigne
> 
>    gc_init (&gc);
> 
> +  route_ipv6_clear_host_bits (r6);
> +

This is no longer needed, as the clearing is done in

  delete_route_connected_v6_net()

now (2cea72005cb5a825c).

> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
> index 560b1a8..40ce202 100644
> --- a/src/openvpn/tun.c
> +++ b/src/openvpn/tun.c
> @@ -5663,6 +5663,9 @@ close_tun (struct tuntap *tt)
>          {
>            if (tt->options.msg_channel)
>              {
> +              /* remove route pointing to interface */
> +              delete_route_connected_v6_net(tt, NULL);
> +
>                do_address_service (false, AF_INET6, tt);
>               if (tt->options.dns6_len > 0)
>                   do_dns6_service (false, tt);

So this part remains, but to avoid code duplication I'd move the
"delete_route_connected_v6_net()" call from the else() branch up 
and before the if (tt->options.msg_channel) clause.

(I'm not adamant on it, though - leaving it there is "all we do is 
in one place").

Looking at the context, this patch is amazingly old... "before the
code reorganization", so it somehow missed 2.4.0...

thanks for bringing it back :-)

gert
Selva Nair March 1, 2018, 7:57 p.m. | #2
Hi,

On Thu, Mar 1, 2018 at 12:51 PM, Gert Doering <gert@greenie.muc.de> wrote:
> Hi,
>
> On Thu, Mar 01, 2018 at 11:09:32AM -0500, Selva Nair wrote:
>> This one is too old to cleanly apply, but still sending again
>> just to get it into patchwork.
>
> Oh, completely fell of my radar.  But now that you mention it, yes,
> there was something about clearing bits :-)
>
>> (For some reason bouncing to patchwork somehow never works for me, else
>> this could have beaten the oldest pending patch record :)
>
> Bouncing my own usually works, bouncing others "sometimes".  Might be
> related to SPF records and/or DKIM/DMARC checks...

One difference is that I'm bouncing @gmail.com mails using mutt. But
its sending through gmail's smtp server, so shouldn't raise any red
flags. And DKIM/SPF failures should be even worse for mails send
through the list. Probably sourceforge.net is whitelisted in
patchwork... Anyway, I can live with it.

>
> [..]
>> This was missing on Windows when interactive service is in use.
>>
>> - Added route_ipv6_clear_host_bits(r6) to delete_route_ipv6: this is
>>   required for Windows IP-helper API. Won't hurt other platforms (?)
>>
>> Signed-off-by: Selva Nair <selva.nair@gmail.com>
>> ---
>>  src/openvpn/route.c | 2 ++
>>  src/openvpn/tun.c   | 3 +++
>>  2 files changed, 5 insertions(+)
>>
>> diff --git a/src/openvpn/route.c b/src/openvpn/route.c
>> index fec12c1..85f969e 100644
>> --- a/src/openvpn/route.c
>> +++ b/src/openvpn/route.c
>> @@ -2124,6 +2124,8 @@ delete_route_ipv6 (const struct route_ipv6 *r6,
>> const struct tuntap *tt, unsigne
>>
>>    gc_init (&gc);
>>
>> +  route_ipv6_clear_host_bits (r6);
>> +
>
> This is no longer needed, as the clearing is done in
>
>   delete_route_connected_v6_net()
>
> now (2cea72005cb5a825c).
>
>> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
>> index 560b1a8..40ce202 100644
>> --- a/src/openvpn/tun.c
>> +++ b/src/openvpn/tun.c
>> @@ -5663,6 +5663,9 @@ close_tun (struct tuntap *tt)
>>          {
>>            if (tt->options.msg_channel)
>>              {
>> +              /* remove route pointing to interface */
>> +              delete_route_connected_v6_net(tt, NULL);
>> +
>>                do_address_service (false, AF_INET6, tt);
>>               if (tt->options.dns6_len > 0)
>>                   do_dns6_service (false, tt);
>
> So this part remains, but to avoid code duplication I'd move the
> "delete_route_connected_v6_net()" call from the else() branch up
> and before the if (tt->options.msg_channel) clause.

Good point. A new version was unavoidable because of the recent
clear_host_bits fix and uncrustify changes.

Actually there was an old v2 but no longer required. So v3 is coming.

> Looking at the context, this patch is amazingly old... "before the
> code reorganization", so it somehow missed 2.4.0...
> thanks for bringing it back :-)

Trying to delete old local branches. There are two other ones which
need David's attention but it seems he is too busy and no one else
would touch plugins code :)

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/route.c b/src/openvpn/route.c
index fec12c1..85f969e 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -2124,6 +2124,8 @@  delete_route_ipv6 (const struct route_ipv6 *r6,
const struct tuntap *tt, unsigne

   gc_init (&gc);

+  route_ipv6_clear_host_bits (r6);
+
   network = print_in6_addr( r6->network, 0, &gc);
   gateway = print_in6_addr( r6->gateway, 0, &gc);

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 560b1a8..40ce202 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -5663,6 +5663,9 @@  close_tun (struct tuntap *tt)
         {
           if (tt->options.msg_channel)
             {
+              /* remove route pointing to interface */
+              delete_route_connected_v6_net(tt, NULL);
+
               do_address_service (false, AF_INET6, tt);
              if (tt->options.dns6_len > 0)
                  do_dns6_service (false, tt);