[Openvpn-devel,M] Change in openvpn[master]: redirect-gateway: add route toward new remote host when using persist...

Message ID 59b9f9a5fbf15b28541a27925a75d25abd90da63-HTML@gerrit.openvpn.net
State New
Headers show
Series [Openvpn-devel,M] Change in openvpn[master]: redirect-gateway: add route toward new remote host when using persist... | expand

Commit Message

d12fk (Code Review) Feb. 25, 2025, 9:48 a.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/+/901?usp=email

to review the following change.


Change subject: redirect-gateway: add route toward new remote host when using persist-tun
......................................................................

redirect-gateway: add route toward new remote host when using persist-tun

When both --redirect-gateway and --persist-tun are used together,
a route must be added for each new remote host we attempt to connect to.

Change-Id: Ic7b486ccb85df7fc1d6a573ac1315d235728822c
Signed-off-by: Marco Baffo <marco@mandelbit.com>
---
M src/openvpn/init.c
M src/openvpn/route.c
M src/openvpn/route.h
3 files changed, 188 insertions(+), 22 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/01/901/1

Patch

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index b57e5f8..63b7bbf 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2184,6 +2184,10 @@ 
                         "route-pre-down",
                         c->c2.es);
 
+            if (c->mode == CM_P2P && c->options.persist_tun)
+            {
+                del_route_towards_remote(c);
+            }
             delete_routes(c->c1.route_list, c->c1.route_ipv6_list,
                           c->c1.tuntap, ROUTE_OPTION_FLAGS(&c->options),
                           c->c2.es, &c->net_ctx);
@@ -2245,6 +2249,11 @@ 
                         c->c2.es);
         }
 
+        if (c->mode == CM_P2P && c->options.persist_tun)
+        {
+            del_route_towards_remote(c);
+        }
+
         del_wfp_block(c, adapter_index);
     }
     gc_free(&gc);
@@ -4825,6 +4834,17 @@ 
         }
     }
 
+    /**
+     * When using both --redirect-gateway and --persist-tun,
+     * if the connection to the server is lost, a /32 (or /128 if IPv6) route must be added
+     * to ensure connectivity to the next remote.
+     */
+    if (c->mode == CM_P2P
+        && c->options.persist_tun)
+    {
+        add_route_towards_remote(c);
+    }
+
     /*
      * Actually do UID/GID downgrade, and chroot, if requested.
      * May be delayed by --client, --pull, or --up-delay.
diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index bc41492..1e4deb5 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -43,6 +43,7 @@ 
 #include "options.h"
 #include "networking.h"
 #include "integer.h"
+#include "openvpn.h"
 
 #include "memdbg.h"
 
@@ -884,30 +885,14 @@ 
     }
 
     /* add VPN server host route if needed */
-    if (need_remote_ipv6_route)
+    if (need_remote_ipv6_route
+        && !(rl6->iflags & RL_DID_LOCAL))
     {
         if ( (rl6->rgi6.flags & (RGI_ADDR_DEFINED|RGI_IFACE_DEFINED) ) ==
              (RGI_ADDR_DEFINED|RGI_IFACE_DEFINED) )
         {
-            struct route_ipv6 *r6;
-            ALLOC_OBJ_CLEAR_GC(r6, struct route_ipv6, &rl6->gc);
-
-            r6->network = *remote_host_ipv6;
-            r6->netbits = 128;
-            if (!(rl6->rgi6.flags & RGI_ON_LINK) )
-            {
-                r6->gateway = rl6->rgi6.gateway.addr_ipv6;
-            }
-            r6->metric = 1;
-#ifdef _WIN32
-            r6->adapter_index = rl6->rgi6.adapter_index;
-#else
-            r6->iface = rl6->rgi6.iface;
-#endif
-            r6->flags = RT_DEFINED | RT_METRIC_DEFINED;
-
-            r6->next = rl6->routes_ipv6;
-            rl6->routes_ipv6 = r6;
+            rl6->routes_ipv6 = create_host_route_ipv6(*remote_host_ipv6, rl6);
+            rl6->iflags |= RL_DID_LOCAL;
         }
         else
         {
@@ -919,6 +904,30 @@ 
     return ret;
 }
 
+struct route_ipv6 *
+create_host_route_ipv6(struct in6_addr remote_host_ipv6, struct route_ipv6_list *rl6)
+{
+    struct route_ipv6 *r6;
+    ALLOC_OBJ_CLEAR_GC(r6, struct route_ipv6, &rl6->gc);
+
+    r6->network = remote_host_ipv6;
+    r6->netbits = 128;
+    if (!(rl6->rgi6.flags & RGI_ON_LINK) )
+    {
+        r6->gateway = rl6->rgi6.gateway.addr_ipv6;
+    }
+    r6->metric = 1;
+#ifdef _WIN32
+    r6->adapter_index = rl6->rgi6.adapter_index;
+#else
+    r6->iface = rl6->rgi6.iface;
+#endif
+    r6->flags = RT_DEFINED | RT_METRIC_DEFINED;
+    r6->next = rl6->routes_ipv6;
+
+    return r6;
+}
+
 static bool
 add_route3(in_addr_t network,
            in_addr_t netmask,
@@ -1055,7 +1064,8 @@ 
                 /* if remote_host is not ipv4 (ie: ipv6), just skip
                  * adding this special /32 route */
                 if ((rl->spec.flags & RTSA_REMOTE_HOST)
-                    && rl->spec.remote_host != IPV4_INVALID_ADDR)
+                    && rl->spec.remote_host != IPV4_INVALID_ADDR
+                    && !(rl->iflags & RL_DID_LOCAL))
                 {
                     ret = add_route3(rl->spec.remote_host, IPV4_NETMASK_HOST,
                                      rl->rgi.gateway.addr, tt, flags | ROUTE_REF_GW,
@@ -1282,7 +1292,7 @@ 
         {
             delete_route_ipv6(r6, tt, flags, es, ctx);
         }
-        rl6->iflags &= ~RL_ROUTES_ADDED;
+        rl6->iflags &= ~(RL_ROUTES_ADDED | RL_DID_LOCAL);
     }
 
     if (rl6)
@@ -1291,6 +1301,131 @@ 
     }
 }
 
+void
+add_route_towards_remote(struct context *c)
+{
+    ASSERT(c->c1.link_socket_addrs);
+
+    int current_remote_family = c->c1.link_socket_addrs[0].actual.dest.addr.sa.sa_family;
+
+    if (current_remote_family == AF_INET && c->c1.route_list
+        && (c->c1.route_list->flags & RG_REROUTE_GW))
+    {
+        if (c->c1.route_list->iflags & RL_DID_LOCAL)
+        {
+            del_route_towards_remote(c);
+        }
+
+        in_addr_t current_remote = ntohl(c->c1.link_socket_addrs[0].actual.dest.addr.in4.sin_addr.s_addr);
+
+        if (add_route3(current_remote,
+                       IPV4_NETMASK_HOST,
+                       c->c1.route_list->rgi.gateway.addr,
+                       c->c1.tuntap,
+                       ROUTE_OPTION_FLAGS(&c->options) | ROUTE_REF_GW,
+                       &c->c1.route_list->rgi,
+                       c->c2.es,
+                       &c->net_ctx))
+        {
+            c->c1.route_list->iflags |= RL_DID_LOCAL;
+            c->c1.route_list->spec.remote_host = current_remote;
+        }
+    }
+    else if (current_remote_family == AF_INET6 && c->c1.route_ipv6_list
+             && (c->c1.route_ipv6_list->flags & RG_REROUTE_GW))
+    {
+        if (c->c1.route_ipv6_list->iflags & RL_DID_LOCAL)
+        {
+            del_route_towards_remote(c);
+        }
+
+        const struct in6_addr *remote_host_ipv6 = &(c->c1.link_socket_addrs[0].actual.dest.addr.in6.sin6_addr);
+        struct route_ipv6_list *rl6 = c->c1.route_ipv6_list;
+
+        if ((rl6->rgi6.flags & (RGI_ADDR_DEFINED|RGI_IFACE_DEFINED)) ==
+            (RGI_ADDR_DEFINED|RGI_IFACE_DEFINED))
+        {
+            struct route_ipv6 *r6 = create_host_route_ipv6(*remote_host_ipv6, rl6);
+
+            if (add_route_ipv6(r6, c->c1.tuntap,
+                               ROUTE_OPTION_FLAGS(&c->options), c->c2.es, &c->net_ctx))
+            {
+                rl6->routes_ipv6 = r6;
+                rl6->iflags |= RL_DID_LOCAL;
+            }
+        }
+        else
+        {
+            msg(M_WARN, "ROUTE6: IPv6 route overlaps with IPv6 remote address, but could not determine IPv6 gateway address + interface, expect failure\n" );
+        }
+    }
+}
+
+void
+del_route_towards_remote(struct context *c)
+{
+    /* If the function is called from do_close_tun() it means that the socket
+     * has already been closed and c->c2.link_sockets[0]->info.lsa` used in
+     * `add_route_towards_remote()` cleaned up. So we should use
+     * `c->c1.link_socket_addrs[0]` instead.
+     */
+    ASSERT(c->c1.link_socket_addrs);
+
+    int current_remote_family = 0;
+
+    if (c->c1.link_socket_addrs[0].actual.dest.addr.sa.sa_family)
+    {
+        current_remote_family = c->c1.link_socket_addrs[0].actual.dest.addr.sa.sa_family;
+    }
+    else if (c->c1.link_socket_addrs[0].current_remote)
+    {
+        current_remote_family = c->c1.link_socket_addrs[0].current_remote->ai_family;
+    }
+
+    if (current_remote_family == AF_INET && c->c1.route_list
+        && (c->c1.route_list->flags & RG_REROUTE_GW)
+        && (c->c1.route_list->iflags & RL_DID_LOCAL))
+    {
+        del_route3(c->c1.route_list->spec.remote_host,
+                   IPV4_NETMASK_HOST,
+                   c->c1.route_list->rgi.gateway.addr,
+                   c->c1.tuntap,
+                   ROUTE_OPTION_FLAGS(&c->options) | ROUTE_REF_GW,
+                   &c->c1.route_list->rgi,
+                   c->c2.es,
+                   &c->net_ctx);
+        c->c1.route_list->iflags &= ~RL_DID_LOCAL;
+    }
+    else if (current_remote_family == AF_INET6 && c->c1.route_ipv6_list
+             && (c->c1.route_ipv6_list->flags & RG_REROUTE_GW)
+             && (c->c1.route_ipv6_list->iflags & RL_DID_LOCAL))
+    {
+        struct in6_addr *remote_host_ipv6 = &c->c1.route_ipv6_list->remote_host_ipv6;
+        struct route_ipv6 *host_route = create_host_route_ipv6(*remote_host_ipv6, c->c1.route_ipv6_list);
+
+        for (struct route_ipv6 *prev = NULL, *r6 = c->c1.route_ipv6_list->routes_ipv6; r6; r6 = r6->next)
+        {
+            if (memcmp(&r6->network, &host_route->network, sizeof(struct in6_addr))
+                || r6->netbits != host_route->netbits
+                || memcmp(&r6->gateway, &host_route->gateway, sizeof(struct in6_addr)))
+            {
+                continue;
+            }
+
+            delete_route_ipv6(r6, c->c1.tuntap, ROUTE_OPTION_FLAGS(&c->options), c->c2.es, &c->net_ctx);
+            if (!prev)
+            {
+                c->c1.route_ipv6_list->routes_ipv6 = r6->next;
+            }
+            else
+            {
+                prev->next = r6->next;
+            }
+            c->c1.route_ipv6_list->iflags &= ~RL_DID_LOCAL;
+        }
+    }
+}
+
 #ifndef ENABLE_SMALL
 
 static const char *
diff --git a/src/openvpn/route.h b/src/openvpn/route.h
index 98ea79e..b1b23f9 100644
--- a/src/openvpn/route.h
+++ b/src/openvpn/route.h
@@ -286,6 +286,14 @@ 
                const struct route_gateway_info *rgi, const struct env_set *es,
                openvpn_net_ctx_t *ctx);
 
+/* Function to add a route towards the new remote when using
+ * `--redirect-gateway` and `--persist-tun` options together. */
+void add_route_towards_remote(struct context *c);
+
+/* Function to delete the route towards the remote when using
+ * `--redirect-gateway` and `--persist-tun` options together. */
+void del_route_towards_remote(struct context *c);
+
 void add_route_to_option_list(struct route_option_list *l,
                               const char *network,
                               const char *netmask,
@@ -353,6 +361,9 @@ 
                            const struct route_gateway_info *rgi,
                            const struct route_ipv6_gateway_info *rgi6);
 
+/* Return a `route_ipv6 *` /128 IPv6 route toward the remote host */
+struct route_ipv6 *create_host_route_ipv6(struct in6_addr remote_host_ipv6, struct route_ipv6_list *rl6);
+
 /*
  * Test if addr is reachable via a local interface (return ILA_LOCAL),
  * or if it needs to be routed via the default gateway (return