[Openvpn-devel] [PATCH v1] multi: don't let stale-routes-check delete permanent routes

From: Antonio Quartulli <[email protected]>

When --stale-routes-check is enabled the periodic check removed every
aged entry from the virtual address routing table, including the
permanent routes installed from --iroute (e.g. via --client-config-dir)
and the client's pushed ifconfig address. Once those entries were gone,
traffic towards the iroute networks was no longer forwarded and the
affected client suffered an outage until it reconnected.

Routes learned from configuration (iroutes and pushed ifconfig
addresses) and genuinely dynamic routes (TAP source addresses learned
from the data channel) were both added with flags == 0, so
check_stale_routes() could not tell them apart and aged out all of
them. Their last_reference is only refreshed when they are hit as a
packet destination, which never happens for an iroute network (CIDR
lookups create a separate cached child route instead) nor for an idle
client's pushed address, so both were eventually deleted.

Mark the config-derived routes with a new MULTI_ROUTE_PERMANENT flag in
the two learn helpers that install them, and skip permanent routes in
check_stale_routes(). Dynamically learned routes keep flags == 0
and are still aged out, preserving the documented purpose of the
option. Cleanup on disconnect is unaffected: a halted instance makes
multi_route_defined() return false, so the reaper still removes the
permanent routes when the client goes away.

Change-Id: I3f9834cc13c49b9249653d7d8637383f50c2fb87
Github: closes OpenVPN/openvpn#1063
Signed-off-by: Antonio Quartulli <[email protected]>
Acked-by: Arne Schwabe <[email protected]>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1729
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1729
This mail reflects revision 1 of this Change.

Acked-by according to Gerrit (reflected above):
Arne Schwabe <[email protected]>

        
diff --git a/doc/man-sections/server-options.rst b/doc/man-sections/server-options.rst
index eb8e273..c9d29f6 100644
--- a/doc/man-sections/server-options.rst
+++ b/doc/man-sections/server-options.rst
@@ -671,6 +671,11 @@
 
   If ``t`` is not present it defaults to ``n``.
 
+  Only dynamically learned routes are subject to this check. Routes added from
+  configuration, such as ``--iroute`` entries and a client's pushed ifconfig
+  address, are never removed by it; they are dropped only when the client
+  disconnects.
+
   This option helps to keep the dynamic routing table small. See also
   ``--max-routes-per-client``
 
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index a957fdf..b08ff08 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -1191,7 +1191,7 @@
         addr.netbits = (uint8_t)netbits;
     }
 
-    struct multi_instance *owner = multi_learn_addr(m, mi, &addr, 0);
+    struct multi_instance *owner = multi_learn_addr(m, mi, &addr, MULTI_ROUTE_PERMANENT);
 #ifdef ENABLE_MANAGEMENT
     if (management && owner)
     {
@@ -1236,7 +1236,7 @@
         mroute_addr_mask_host_bits(&addr);
     }
 
-    struct multi_instance *owner = multi_learn_addr(m, mi, &addr, 0);
+    struct multi_instance *owner = multi_learn_addr(m, mi, &addr, MULTI_ROUTE_PERMANENT);
 #ifdef ENABLE_MANAGEMENT
     if (management && owner)
     {
@@ -1360,7 +1360,7 @@
     while ((he = hash_iterator_next(&hi)) != NULL)
     {
         struct multi_route *r = (struct multi_route *)he->value;
-        if (multi_route_defined(m, r)
+        if (multi_route_defined(m, r) && !(r->flags & MULTI_ROUTE_PERMANENT)
             && difftime(now, r->last_reference) >= m->top.options.stale_routes_ageing_time)
         {
             dmsg(D_MULTI_DEBUG, "MULTI: Deleting stale route for address '%s'",
diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h
index 3ed08d4..22cfeee 100644
--- a/src/openvpn/multi.h
+++ b/src/openvpn/multi.h
@@ -237,8 +237,9 @@
     struct mroute_addr addr;
     struct multi_instance *instance;
 
-#define MULTI_ROUTE_CACHE   (1 << 0)
-#define MULTI_ROUTE_AGEABLE (1 << 1)
+#define MULTI_ROUTE_CACHE     (1 << 0)
+#define MULTI_ROUTE_AGEABLE   (1 << 1)
+#define MULTI_ROUTE_PERMANENT (1 << 2) /* config-derived (iroute / pushed ifconfig); never stale-aged */
     unsigned int flags;
 
     unsigned int cache_generation;


_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel