[Openvpn-devel,v3] Change exit signal in P2P to be a SIGUSR1 and delay CC exit in P2MP

Message ID 20221016154953.2483509-1-arne@rfc2549.org
State Accepted
Headers show
Series [Openvpn-devel,v3] Change exit signal in P2P to be a SIGUSR1 and delay CC exit in P2MP | expand

Commit Message

Arne Schwabe Oct. 16, 2022, 3:49 p.m. UTC
From the implemention of explicit-notify and the fact that it is a an
OCC message (basically the rudimentary predecessor to control channel),
this message is very old.

I think in the past this feature fit nicely to the weird inetd + openvpn mode
that seems to have far to many hacks still left in our code. With inetd,
it made sense that the server instance quits if you press C-c on the client.

In our current state where inetd is no longer supported, this behaviour
to exit makes little sense and this patch changes the behaviour to SIGUSR1.

Testing this lead to a confused v2 of the patch and also finally the insight
that if a CC channel exit is triggered too early the remaining control channel
packets that come after that can trigger the HMAC code to open a sessions
again if the whole session lasted less than two minutes (with default

Patch v2: use different signals for p2mp and p2p
Patch v3: use dealyed exit for P2MP/CC exit and USR1 for everything else

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
 src/openvpn/occ.c  |  2 +-
 src/openvpn/push.c | 20 +++++++++++++++++++-
 2 files changed, 20 insertions(+), 2 deletions(-)


Gert Doering Oct. 17, 2022, 8:25 a.m. UTC | #1
Acked-by: Gert Doering <gert@greenie.muc.de>

Tested the whole lot again.  Only difference to v1 is in p2mp mode with 
incoming TLS EEN, which now logs

10:15:34 cron2-freebsd-tc-amd64/ Exit message received by peer
10:15:34 cron2-freebsd-tc-amd64/ Delayed exit in 5 seconds
10:15:38 read UDPv6 [ECONNREFUSED]: Connection refused (fd=5,code=111)
10:15:39 us=240199 cron2-freebsd-tc-amd64/ SIGTERM[soft,delayed-exit] received, client-instance exiting

(the ECONNREFUSED is "the client exited right away, so never saw the
control-plane ACK" - calling "--explicit-exit-notify 10" on the client
makes those last packets succeed, with no adverse effects on unwanted

I have worked a bit on the commit message, and a lot on the new comment
block - I hope it is now clearer what could happen, and how this change
avoids the problem.

I have also added a Changes.rst entry (user-visible changes), pointing
out that "--remap-usr1 SIGTERM" will bring back the old behaviour, if
someone relies on it.

Your patch has been applied to the master branch.

commit d468dff7bdfd79059818c190ddf41b125bb658de
Author: Arne Schwabe
Date:   Sun Oct 16 17:49:53 2022 +0200

     Change exit signal in P2P to be a SIGUSR1 and delay CC exit in P2MP

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20221016154953.2483509-1-arne@rfc2549.org>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25403.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>

kind regards,

Gert Doering


diff --git a/src/openvpn/occ.c b/src/openvpn/occ.c
index 1ed0d3771..eb1f2fae7 100644
--- a/src/openvpn/occ.c
+++ b/src/openvpn/occ.c
@@ -431,7 +431,7 @@  process_received_occ_msg(struct context *c)
         case OCC_EXIT:
-            c->sig->signal_received = SIGTERM;
+            c->sig->signal_received = SIGUSR1;
             c->sig->signal_text = "remote-exit";
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 26259c6b8..d9fa64870 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -193,7 +193,25 @@  void
 receive_exit_message(struct context *c)
     dmsg(D_STREAM_ERRORS, "Exit message received by peer");
-    c->sig->signal_received = SIGTERM;
+    /* With control channel exit notification, we want the session to give
+     * enough time to handle retransmits and acknowledgment, so lat coming
+     * retries from the client to resend the exit or ACKs do not trigger
+     * a new session since we already killed it but the packet still has
+     * a valid HMAC. This can only happen for the period for the HMAC
+     * timeslot is still valid but waiting five seconds here does not
+     * hurt much and is the better alternative.
+     *
+     * This does not affect OCC exit since the HMAC session code will
+     * ignore DATA packets
+     * */
+    if (c->options.mode == MODE_SERVER)
+    {
+        schedule_exit(c, c->options.scheduled_exit_interval, SIGTERM);
+    }
+    else
+    {
+        c->sig->signal_received = SIGUSR1;
+    }
     c->sig->signal_text = "remote-exit";
     if (management)