Stop complaining about IPv6 routes without gateway address.

PATCH Superseded Gert Doering 2 participants 6 messages Dec 5, 2018 – Dec 18, 2018

lore

Share & tools

lore

Gert Doering

author

2018-12-05 21:07 UTC #0 lore raw

PATCH 1/1 Stop complaining about IPv6 routes without gateway address.

The IPv6 routing code inherited assumptions and the message

"OpenVPN ROUTE6: OpenVPN needs a gateway parameter for a --route-ipv6
option and no default was specified by either --route-ipv6-gateway or --ifconfig-ipv6 options"

from the IPv4 routing code.

This was never really correct, as no gateway is needed for "into tun device" IPv6 routes, and the "--route-ipv6-gateway" option it refers to also never existed. (Routes on tap interfaces *do* need a gateway due to neighbour discovery being involved. As do routes on Windows, but there we fake the gateway in tun mode anyway).

Change the code to generally accept IPv6 routes with no gateway specification (so "--block-ipv6 --redirect-gateway ipv6" can work without additional config). When installing IPv6 routes, check if a gateway is needed (tap mode) but missing, and if yes, print correct message.

Trac: #1143

Signed-off-by Gert Doering <[email protected]...>

---
 src/openvpn/route.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

src/openvpn/route.c +11 -5

index d97e8dba..cf51063b 100644
@@ -448,11 +448,6 @@ init_route_ipv6(struct route_ipv6 *r6,
     {
         r6->gateway = rl6->remote_endpoint_ipv6;
     }
-    else
-    {
-        msg(M_WARN, PACKAGE_NAME " ROUTE6: " PACKAGE_NAME " needs a gateway parameter for a --route-ipv6 option and no default was specified by either --route-ipv6-gateway or --ifconfig-ipv6 options");
-        goto fail;
-    }
 
     /* metric */

@@ -1917,6 +1912,16 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flag
         gateway_needed = true;
     }
 
+    if (gateway_needed && IN6_IS_ADDR_UNSPECIFIED(&r6->gateway) )
+    {
+        msg(M_WARN, "ROUTE6 WARNING: " PACKAGE_NAME " needs a gateway "
+            "parameter for a --route-ipv6 option and no default was set via "
+            "--ifconfig-ipv6 option.  Not installing IPv6 route to %s/%d.",
+            network, r6->netbits );
+        status = false;
+        goto done;
+    }
+
 #if defined(TARGET_LINUX)
 #ifdef ENABLE_IPROUTE
     argv_printf(&argv, "%s -6 route add %s/%d dev %s",

@@ -2114,6 +2119,7 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flag
     msg(M_FATAL, "Sorry, but I don't know how to do 'route ipv6' commands on this operating system.  Try putting your routes in a --route-up script");
 #endif /* if defined(TARGET_LINUX) */
 
+done:
     if (status)
     {
         r6->flags |= RT_ADDED;

Signature · 1 line

2.18.1

Gert Doering

author

2018-12-05 21:24 UTC #1 lore raw

Hi,

Quoted 6 lines · Gert Doering

The IPv6 routing code inherited assumptions and the message
"OpenVPN ROUTE6: OpenVPN needs a gateway parameter for a --route-ipv6
option and no default was specified by either --route-ipv6-gateway or --ifconfig-ipv6 options"

Quoted 7 lines

from the IPv4 routing code.
This was never really correct, as no gateway is needed for "into tun device" IPv6 routes, and the "--route-ipv6-gateway" option it refers to also never existed. (Routes on tap interfaces *do* need a gateway due to neighbour discovery being involved. As do routes on Windows, but there we fake the gateway in tun mode anyway).

Mmmh, ditch this patch.

I've forgotten the patch from James Bekkema (which was sent in a weird format so patchwork did not recognize it as such). Both patches are compatible, but my new message does not take this into account

v2 coming tomorrow.

gert

Signature · 6 lines

"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
                             Robert A. Heinlein, The Moon is a Harsh Mistress

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

Gert Doering

author

2018-12-05 21:40 UTC #2 lore raw

PATCH v2 1/1 Stop complaining about IPv6 routes without gateway address.

The IPv6 routing code inherited assumptions and the message

"OpenVPN ROUTE6: OpenVPN needs a gateway parameter for a --route-ipv6
option and no default was specified by either --route-ipv6-gateway or --ifconfig-ipv6 options"

from the IPv4 routing code.

While commit d24e1b179b95 introduces support for "--route-ipv6-gateway", the message is still falsely triggered for IPv6 routes in tun mode.

Trac: #1143

Signed-off-by Gert Doering <[email protected]...>

---
 src/openvpn/route.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

src/openvpn/route.c +11 -5

index d97e8dba..ac38bf15 100644
@@ -448,11 +448,6 @@ init_route_ipv6(struct route_ipv6 *r6,
     {
         r6->gateway = rl6->remote_endpoint_ipv6;
     }
-    else
-    {
-        msg(M_WARN, PACKAGE_NAME " ROUTE6: " PACKAGE_NAME " needs a gateway parameter for a --route-ipv6 option and no default was specified by either --route-ipv6-gateway or --ifconfig-ipv6 options");
-        goto fail;
-    }
 
     /* metric */

@@ -1917,6 +1912,16 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flag
         gateway_needed = true;
     }
 
+    if (gateway_needed && IN6_IS_ADDR_UNSPECIFIED(&r6->gateway) )
+    {
+        msg(M_WARN, "ROUTE6 WARNING: " PACKAGE_NAME " needs a gateway "
+            "parameter for a --route-ipv6 option and no default was set via "
+            "--ifconfig-ipv6 or --route-ipv6-gateway option.  Not installing "
+            "IPv6 route to %s/%d.", network, r6->netbits );
+        status = false;
+        goto done;
+    }
+
 #if defined(TARGET_LINUX)
 #ifdef ENABLE_IPROUTE
     argv_printf(&argv, "%s -6 route add %s/%d dev %s",

@@ -2114,6 +2119,7 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flag
     msg(M_FATAL, "Sorry, but I don't know how to do 'route ipv6' commands on this operating system.  Try putting your routes in a --route-up script");
 #endif /* if defined(TARGET_LINUX) */
 
+done:
     if (status)
     {
         r6->flags |= RT_ADDED;

Signature · 1 line

2.18.1

Antonio Quartulli

Acked-by on v2 1/1

2018-12-17 06:53 UTC #3 lore raw

Hi,

Quoted 14 lines · Gert Doering

The IPv6 routing code inherited assumptions and the message
"OpenVPN ROUTE6: OpenVPN needs a gateway parameter for a --route-ipv6
option and no default was specified by either --route-ipv6-gateway or --ifconfig-ipv6 options"
from the IPv4 routing code.
This was never really correct, as no gateway is needed for "into tun device" IPv6 routes, and the "--route-ipv6-gateway" option it refers to also never existed. (Routes on tap interfaces *do* need a gateway due to neighbour discovery being involved. As do routes on Windows, but there we fake the gateway in tun mode anyway).

Quoted 9 lines

While commit d24e1b179b95 introduces support for "--route-ipv6-gateway", the message is still falsely triggered for IPv6 routes in tun mode.
Change the code to generally accept IPv6 routes with no gateway specification (so "--block-ipv6 --redirect-gateway ipv6" can work without additional config). When installing IPv6 routes, check if a gateway is needed (tap mode) but missing, and if yes, print correct message.

Thanks for this commit message. It was very clear as of what the patch is addressing, how and why,

Quoted 24 lines

Trac: #1143

Signed-off-by Gert Doering <[email protected]...>

---
 src/openvpn/route.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

src/openvpn/route.c +0 -5

index d97e8dba..ac38bf15 100644
@@ -448,11 +448,6 @@ init_route_ipv6(struct route_ipv6 *r6,
     {
         r6->gateway = rl6->remote_endpoint_ipv6;
     }
-    else
-    {
-        msg(M_WARN, PACKAGE_NAME " ROUTE6: " PACKAGE_NAME " needs a gateway parameter for a --route-ipv6 option and no default was specified by either --route-ipv6-gateway or --ifconfig-ipv6 options");
-        goto fail;
-    }
 
     /* metric */

src/openvpn/route.c add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flag · line 1915

Quoted 4 lines

src/openvpn/route.c +1 -0

@@ -1917,6 +1912,16 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flag
         gateway_needed = true;
     }
 
+    if (gateway_needed && IN6_IS_ADDR_UNSPECIFIED(&r6->gateway) )

please remove the space before the last ')'

src/openvpn/route.c add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flag · line 1920

Quoted 5 lines

src/openvpn/route.c +5 -0

+    {
+        msg(M_WARN, "ROUTE6 WARNING: " PACKAGE_NAME " needs a gateway "
+            "parameter for a --route-ipv6 option and no default was set via "
+            "--ifconfig-ipv6 or --route-ipv6-gateway option.  Not installing "
+            "IPv6 route to %s/%d.", network, r6->netbits );

please remove the space before the last ')'

As quickly discussed during the meeting, it might be better to have the string all on one line to make it easier to grep. You could go on a new line after M_WARN, and then right after the long string.

One dis-advantage I have seen while testing this patch is that we now get one message for each route we try to install.

However, I think it is ok because the message explicitly mentions each failing route.

Quoted 9 lines

src/openvpn/route.c +4 -0

+        status = false;
+        goto done;
+    }
+
 #if defined(TARGET_LINUX)
 #ifdef ENABLE_IPROUTE
     argv_printf(&argv, "%s -6 route add %s/%d dev %s",

src/openvpn/route.c add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flag · line 2125

Quoted 7 lines

src/openvpn/route.c +1 -0

@@ -2114,6 +2119,7 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flag
     msg(M_FATAL, "Sorry, but I don't know how to do 'route ipv6' commands on this operating system.  Try putting your routes in a --route-up script");
 #endif /* if defined(TARGET_LINUX) */
 
+done:
     if (status)
     {
         r6->flags |= RT_ADDED;

Other than those comments above, the patch looks good and my tests did not reveal any issue.

Acked-by Antonio Quartulli <[email protected]...>

Regards,

Signature · 1 line

Antonio Quartulli

Gert Doering

author

2018-12-18 14:27 UTC #4 lore raw

Hi,

just as a side note, for the archives:

Quoted 2 lines · Antonio Quartulli

One dis-advantage I have seen while testing this patch is that we now get one message for each route we try to install.

That's no different from today - we hit the message at a different spot (when trying to install the route, as opposed to "when processing the 'route-ipv6' option") - but we got one message per route before.

Just sayin' ;-)

Will go and uncrustify...

gert

Signature · 6 lines

"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
                             Robert A. Heinlein, The Moon is a Harsh Mistress

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

Gert Doering

author

2018-12-18 14:39 UTC #5 lore raw

In reply to Gert Doering

Patch has been applied to the master branch.

I haven't put it into release/2.4 as this is more a "make things nicer" than a bugfix - plus it needs the "--route-ipv6-gateway" feature from d24e1b179b95, which is "new feature" and thus not in 2.4.

Reformatted braces as suggested. Interesting enough, our uncrustify rules permit both "a space before the closing bracked" and "no space" (and I think this is good enough, sometimes you just want the extra space for readability - but maybe not in this case).

I've left the message as it was, because "putting it all in one line" creates a 220 characters wide line - which is definitely in "Steffan will come and kick me" land.

commit 14d7e0e496f15563005fffc6d4791a95444ddf23 (master)

Author: Gert Doering
Date:   Wed Dec 5 22:40:37 2018 +0100

     Stop complaining about IPv6 routes without gateway address.

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

--
kind regards,

Gert Doering