Delete the IPv6 route to the "connected" network on tun close

PATCH Superseded Selva Nair 2 participants 10 messages Nov 24, 2016 – Mar 4, 2018

lore

Share & tools

lore

Selva Nair

author

2016-11-24 21:19 UTC #0 lore raw

PATCH 1/1 Delete the IPv6 route to the "connected" network on tun close

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 <[email protected]...>

---
 src/openvpn/route.c | 2 ++
 src/openvpn/tun.c   | 3 +++
 2 files changed, 5 insertions(+)

src/openvpn/route.c +2 -0

index fec12c1..85f969e 100644
@@ -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);

src/openvpn/tun.c +3 -0

index 560b1a8..40ce202 100644
@@ -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);

Signature · 1 line

2.1.4

Selva Nair

author

2016-11-25 05:21 UTC #1 lore raw

PATCH v2 1/1 Delete the IPv6 route to the "connected" network on tun close

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 (?)

v2: Be const correct: route in delete_route_ipv6() made non-const. None of the exisitng calls are affected.

Signed-off-by Selva Nair <[email protected]...>

---
 src/openvpn/route.c | 4 +++-
 src/openvpn/route.h | 2 +-
 src/openvpn/tun.c   | 3 +++
 3 files changed, 7 insertions(+), 2 deletions(-)

src/openvpn/route.c +3 -1

index fec12c1..34b1196 100644
@@ -2102,7 +2102,7 @@ delete_route (struct route_ipv4 *r,
 }
 
 void
-delete_route_ipv6 (const struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flags, const struct env_set *es)
+delete_route_ipv6 (struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flags, const struct env_set *es)
 {
   struct gc_arena gc;
   struct argv argv = argv_new ();

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

src/openvpn/route.h +1 -1

index c358681..70aeb65 100644
@@ -252,7 +252,7 @@ void copy_route_ipv6_option_list (struct route_ipv6_option_list *dest,
                                   struct gc_arena *a);
 
 void add_route_ipv6 (struct route_ipv6 *r, const struct tuntap *tt, unsigned int flags, const struct env_set *es);
-void delete_route_ipv6 (const struct route_ipv6 *r, const struct tuntap *tt, unsigned int flags, const struct env_set *es);
+void delete_route_ipv6 (struct route_ipv6 *r, const struct tuntap *tt, unsigned int flags, const struct env_set *es);
 
 void add_route (struct route_ipv4 *r,
 		const struct tuntap *tt,

src/openvpn/tun.c +3 -0

index 560b1a8..40ce202 100644
@@ -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);

Signature · 1 line

2.1.4

Selva Nair

author

2016-12-30 22:41 UTC #3 lore raw

Hi,

Quoted 3 lines · Selva Nair

Based on Trac #807, it seems removing ipv6 address on tun close is more than just good behaviour.

Hmm.. this patch was about deleting ipv6 routes not address, so not really related to Trac 807, nor as important as I thought. Sorry for the confusion.

Selva

Selva Nair

author

2018-03-01 16:09 UTC #5 lore raw

PATCH 1/1 Delete the IPv6 route to the "connected" network on tun close

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 <[email protected]...>
Date: Thu, Nov 24, 2016 at 4:19 PM
Subject: [PATCH] Delete the IPv6 route to the "connected" network on tun close
To: [email protected]
Cc: Selva Nair <[email protected]...>

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 <[email protected]...>

---
 src/openvpn/route.c | 2 ++
 src/openvpn/tun.c   | 3 +++
 2 files changed, 5 insertions(+)

src/openvpn/route.c +0 -0

index fec12c1..85f969e 100644
@@ -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);

src/openvpn/tun.c +3 -0

index 560b1a8..40ce202 100644
@@ -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);

--
2.1.4

Gert Doering

2018-03-01 17:51 UTC #6 lore raw

Hi,

Quoted 2 lines · Selva Nair

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

Quoted 2 lines

(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...

[..]

Quoted 20 lines

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 <[email protected]...>
---
 src/openvpn/route.c | 2 ++
 src/openvpn/tun.c   | 3 +++
 2 files changed, 5 insertions(+)
src/openvpn/route.c +0 -0
index fec12c1..85f969e 100644
@@ -2124,6 +2124,8 @@ delete_route_ipv6 (const struct route_ipv6 *r6,
const struct tuntap *tt, unsigne
   gc_init (&gc);

src/openvpn/route.c

Quoted 2 lines

src/openvpn/route.c +2 -0
+  route_ipv6_clear_host_bits (r6);
+

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

delete_route_connected_v6_net()

now (2cea72005cb5a825c).

Quoted 5 lines

src/openvpn/tun.c +0 -0
index 560b1a8..40ce202 100644

src/openvpn/tun.c close_tun (struct tuntap *tt) · line 5671

Quoted 9 lines

src/openvpn/tun.c +3 -0

@@ -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

Signature · 3 lines

now what should I write here...

Gert Doering - Munich, Germany                             [email protected]...

Selva Nair

author

2018-03-01 19:57 UTC #7 lore raw

Hi,

Quoted 6 lines · Gert Doering

Hi,
Quoted 2 lines · Selva Nair
This one is too old to cleanly apply, but still sending again just to get it into patchwork.

Quoted 8 lines

Oh, completely fell of my radar. But now that you mention it, yes, there was something about clearing bits :-)
Quoted 2 lines
(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.

Quoted 41 lines

[..]
Quoted 20 lines
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 <[email protected]...>
---
 src/openvpn/route.c | 2 ++
 src/openvpn/tun.c   | 3 +++
 2 files changed, 5 insertions(+)
src/openvpn/route.c +0 -0
index fec12c1..85f969e 100644
@@ -2124,6 +2124,8 @@ delete_route_ipv6 (const struct route_ipv6 *r6,
const struct tuntap *tt, unsigne
   gc_init (&gc);
src/openvpn/route.c
Quoted 2 lines
src/openvpn/route.c +2 -0
+  route_ipv6_clear_host_bits (r6);
+
This is no longer needed, as the clearing is done in
delete_route_connected_v6_net()
now (2cea72005cb5a825c).
Quoted 10 lines
src/openvpn/tun.c +2 -0
index 560b1a8..40ce202 100644
@@ -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);

Quoted 8 lines

src/openvpn/tun.c close_tun (struct tuntap *tt) · line 5671
Quoted 4 lines
src/openvpn/tun.c +1 -0
+
               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.

Quoted 3 lines

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

[email protected]...

author

2018-03-01 20:34 UTC #8 lore raw

PATCH v3 1/1 Delete the IPv6 route to the "connected" network on tun close

In reply to Selva Nair

From: Selva Nair <[email protected]...>

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

v3: Mar 1, 2017: avoid code repetition and rebase to master

Signed-off-by Selva Nair <[email protected]...>

---
 src/openvpn/tun.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

src/openvpn/tun.c +3 -3

index 9f3b98d..263cacd 100644
@@ -6185,6 +6185,9 @@ close_tun(struct tuntap *tt)
     {
         if (tt->did_ifconfig_ipv6_setup)
         {
+            /* remove route pointing to interface */
+            delete_route_connected_v6_net(tt, NULL);
+
             if (tt->options.msg_channel)
             {
                 do_address_service(false, AF_INET6, tt);

@@ -6198,9 +6201,6 @@ close_tun(struct tuntap *tt)
                 const char *ifconfig_ipv6_local;
                 struct argv argv = argv_new();
 
-                /* remove route pointing to interface */
-                delete_route_connected_v6_net(tt, NULL);
-
                 /* "store=active" is needed in Windows 8(.1) to delete the
                  * address we added (pointed out by Cedric Tabary).
                  */

Signature · 1 line

2.6.2

Gert Doering

Acked-by on v3 1/1

2018-03-04 09:04 UTC #9 lore raw

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

Acked-by Gert Doering <[email protected]...>

Test compiled, not actually run - but "stare at code" is convincing enough.

Your patch has been applied to the master and release/2.4 branch.

commit b607900ba937b5f45796d2e3810ef91a32826927 (master) commit 716fdb24be7857e242c3174a51485446502481ea (release/2.4)

Author: Selva Nair
Date:   Thu Mar 1 15:34:44 2018 -0500

     Delete the IPv6 route to the connected network on tun close

     Signed-off-by: Selva Nair <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg16598.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering