[Openvpn-devel,v7] dco: remove iroute at client exit time instead of delayed exit
Commit Message
From: Antonio Quartulli <antonio@mandelbit.com>
When a client exits due to one of the following reasons:
* EEN received
* AUTH_FAILED sent
* RESTART sent
OpenVPN will perform some minimal cleanup and will
then postpone the actual instance purge by 5 seconds.
If iroutes are configured for the exiting client, the actual DCO
iroutes removal is also postponed.
If during this time window the same client reconnects, a new
instance is created (for example because --duplicate-cn is set or
because the same username is provided upon authentication) and the
same IP is assigned, then OpenVPN will:
* create the new client instance;
* attempt adding the related DCO iroutes (which will fail because
EEXIST);
* 5 seconds timeout fires -> execute the delayed exit routine and
delete the DCO iroutes;
* no iroutes exists anymore on the server despite the client being
fully connected.
Note that this issue is DCO specific, because without DCO OpenVPN
creates virtual routes (no system routing table involved) and makes
the last connecting client own them.
This means that the delayed exit routine won't have any iroute to
delete.
With this patch we move the DCO iroutes deletion to the actual
client exit time in order to avoid racing with a possible addition
being executed when the client reconnects. The new flow will be:
* client exits (due to EEN or timeout)
* DCO iroutes are immediately deleted
* client re-connects -> new instance created
* DCO iroutes are added -> SUCCESS
* 5 seconds timeout fires -> old instance client is fully purged
* client is connected and iroutes are in place as expected
This issue was reported by OpenVPN Access Server developers after
observing erratic iroutes disappearance with DCO in place.
Change-Id: I0ba723d12d433e6e020588b7b0c3ba10bcf8c44f
GitHub: closes openvpn/OpenVPN#1040
Signed-off-by: Antonio Quartulli <antonio@mandelbit.com>
Acked-by: Razvan Cojocaru <razvanc@mailbox.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1683
---
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/+/1683
This mail reflects revision 7 of this Change.
Acked-by according to Gerrit (reflected above):
Razvan Cojocaru <razvanc@mailbox.org>
Comments
Thanks for reworking the v6 "tested and approved months ago" on a short
notice - but the new one is really so much nicer, and much better
localized changes ;-)
I'm still not happy with all the details (did_iroutes sits in mi,
while did_dco_iroutes sits in c, and both are "instance specific"
variables... - and also, a function called "schedule_exit()"
maybe shouldn't do more than that - so we should come back and
revisit this eventually)
That said, the problem is well-understood, and the approach to fixing
it (v6, that is) has been tested in production in the Access Server
product. v7 is the same code, just keeping the old function args
to schedule_exit() (because c->net_ctx can be dereferenced from there).
Adding to the experience from there I've run a t_server run on ubuntu+dco
with a bit of iroute activity - not stress testing, just doing basic "does
everything still work?". it does.
Archive reference URL: pointing to lore.kernel.org, because mail-archive.org
refuses to acknowledge v7 of this patch... very opinionated, them.
Your patch has been applied to the master and release/2.7 branch.
commit 2f76d3aac55a5347273013d60eb86aa17c8b8941 (master)
commit e9d8fd2bffeaf28920d1d11b17ccd5c173d3ab18 (release/2.7)
Author: Antonio Quartulli
Date: Mon Sep 28 13:37:18 2026 +0200
dco: remove iroute at client exit time instead of delayed exit
Signed-off-by: Antonio Quartulli <antonio@mandelbit.com>
Acked-by: Razvan Cojocaru <razvanc@mailbox.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1683
Message-Id: <20260928113726.32340-1-gert@greenie.muc.de>
URL: https://lore.kernel.org/openvpn-devel/20260928113726.32340-1-gert@greenie.muc.de/T/#u
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
@@ -750,6 +750,8 @@
}
ASSERT(TUNNEL_TYPE(c->c1.tuntap) == DEV_TYPE_TUN);
+ msg(D_DCO, "DCO: attempt removing iroutes from system table");
+
if (c->c2.push_ifconfig_defined)
{
for (const struct iroute *ir = c->options.iroutes; ir; ir = ir->next)
@@ -539,6 +539,23 @@
{
return false;
}
+
+ /* DCO iroutes must be removed now, because the delay introduced by this
+ * timer can create a race condition:
+ * the same client may reconnect before the old instance is purged, leading
+ * to DCO iroutes removal *after* reconnection, thus killing the routes
+ * for the new instance too.
+ *
+ * Standard/virtual iroutes (non-DCO case) are not affected because the
+ * last connecting client claiming the iroutes takes ownership. Therefore
+ * they are not removed during delayed cleanup.
+ */
+ if (c->did_dco_iroutes)
+ {
+ c->did_dco_iroutes = false;
+ dco_delete_iroutes(&c->net_ctx, c);
+ }
+
tls_set_single_session(c->c2.tls_multi);
update_time();
reset_coarse_timers(c);
@@ -479,7 +479,12 @@
const struct iroute *ir;
const struct iroute_ipv6 *ir6;
- dco_delete_iroutes(&m->top.net_ctx, &mi->context);
+ /* check if DCO iroutes were already removed when scheduling a delayed exit */
+ if (mi->context.did_dco_iroutes)
+ {
+ mi->context.did_dco_iroutes = false;
+ dco_delete_iroutes(&m->top.net_ctx, &mi->context);
+ }
if (TUNNEL_TYPE(mi->context.c1.tuntap) == DEV_TYPE_TUN)
{
@@ -1277,6 +1282,8 @@
if (TUNNEL_TYPE(mi->context.c1.tuntap) == DEV_TYPE_TUN)
{
mi->did_iroutes = true;
+ /* multi_learn_in{6}_addr_t takes care of installing the DCO iroute */
+ mi->context.did_dco_iroutes = true;
for (ir = mi->context.options.iroutes; ir != NULL; ir = ir->next)
{
if (ir->netbits >= 0)
@@ -507,6 +507,8 @@
bool did_we_daemonize; /**< Whether demonization has already
* taken place. */
+ bool did_dco_iroutes; /**< Whether DCO iroutes have been installed */
+
struct context_persist persist;
/**< Persistent %context. */
struct context_0 *c0; /**< Level 0 %context. */