[Openvpn-devel,S] Change in openvpn[master]: route: copied 'gateway_needed' logic from add_route_ipv6 to add_route

Message ID a3be8d2efdf2f993be2cbf321a7887a5c77038ea-HTML@gerrit.openvpn.net
State New
Headers show
Series [Openvpn-devel,S] Change in openvpn[master]: route: copied 'gateway_needed' logic from add_route_ipv6 to add_route | expand

Commit Message

ordex (Code Review) July 18, 2024, 2:03 p.m. UTC
Attention is currently required from: flichtenheld, plaisthos.

Hello plaisthos, flichtenheld,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/677?usp=email

to review the following change.


Change subject: route: copied 'gateway_needed' logic from add_route_ipv6 to add_route
......................................................................

route: copied 'gateway_needed' logic from add_route_ipv6 to add_route

Under certain circumstances it may not be necessary to pass the
gateway when adding a new route via net_route_v4_add() API function.
add_route_ipv6() already accounts for some of these cases and
therefore this patch copies the same logic to add_route().

Change-Id: Id1ec0c6e4c391604ec5dbb8b7122f2e47ad186d1
Signed-off-by: Marco Baffo <marco@mandelbit.com>
---
M src/openvpn/route.c
1 file changed, 31 insertions(+), 3 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/77/677/1

Patch

diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index 91f2032..bc8f561 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -1571,6 +1571,7 @@ 
 {
     int status = 0;
     int is_local_route;
+    bool gateway_needed = false;
 
     if (!(r->flags & RT_DEFINED))
     {
@@ -1580,8 +1581,20 @@ 
     struct argv argv = argv_new();
     struct gc_arena gc = gc_new();
 
-#if !defined(TARGET_LINUX)
+#if defined(TARGET_LINUX)
+    const char *iface = tt->actual_name;
+    if (rgi && rgi->iface[0] != '\0')  /* vpn server special route */
+    {
+        iface = rgi->iface;
+        if (r->gateway != 0)
+        {
+            gateway_needed = true;
+        }
+    }
+#endif
+
     const char *network = print_in_addr_t(r->network, 0, &gc);
+#if !defined(TARGET_LINUX)
 #if !defined(TARGET_AIX)
     const char *netmask = print_in_addr_t(r->netmask, 0, &gc);
 #endif
@@ -1594,8 +1607,22 @@ 
         goto done;
     }
 
+    if (tt->type == DEV_TYPE_TAP && !(r->flags & RT_METRIC_DEFINED && r->metric == 0))
+    {
+        gateway_needed = true;
+    }
+
+    if (gateway_needed && r->gateway == 0)
+    {
+        msg(M_WARN, "ROUTE WARNING: " PACKAGE_NAME " needs a gateway "
+            "parameter for a --route option and no default was set via "
+            "--route-gateway or --ifconfig option. Not installing "
+            "IPv4 route to %s/%d.", network, netmask_to_netbits2(r->netmask));
+        status = 0;
+        goto done;
+    }
+
 #if defined(TARGET_LINUX)
-    const char *iface = NULL;
     int metric = -1;
 
     if (is_on_link(is_local_route, flags, rgi))
@@ -1610,7 +1637,8 @@ 
 
     status = RTA_SUCCESS;
     int ret = net_route_v4_add(ctx, &r->network, netmask_to_netbits2(r->netmask),
-                               &r->gateway, iface, 0, metric);
+                               gateway_needed ? &r->gateway : NULL,
+                               iface, 0, metric);
     if (ret == -EEXIST)
     {
         msg(D_ROUTE, "NOTE: Linux route add command failed because route exists");