Browse code

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

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 coming in after that can trigger the HMAC code
to open a sessions again if the whole session lasted less than two
minutes (with default settings).

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

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>

Arne Schwabe authored on 2022/10/17 00:49:53
Showing 3 changed files
... ...
@@ -163,6 +163,13 @@ User-visible Changes
163 163
 - :code:`link_mtu` parameter is removed from environment or replaced with 0 when scripts are
164 164
   called with parameters. This parameter is unreliable and no longer internally calculated.
165 165
 
166
+- In point-to-point OpenVPN setups (no ``--server``), using
167
+  ``--explict-exit-notiy`` on one end would terminate the other side at
168
+  session end.  This is considered a no longer useful default and has
169
+  been changed to "restart on reception of explicit-exit-notify message".
170
+  If the old behaviour is still desired, ``--remap-usr1 SIGTERM`` can be used.
171
+
172
+
166 173
 Overview of changes in 2.5
167 174
 ==========================
168 175
 
... ...
@@ -431,7 +431,7 @@ process_received_occ_msg(struct context *c)
431 431
 
432 432
         case OCC_EXIT:
433 433
             dmsg(D_PACKET_CONTENT, "RECEIVED OCC_EXIT");
434
-            c->sig->signal_received = SIGTERM;
434
+            c->sig->signal_received = SIGUSR1;
435 435
             c->sig->signal_text = "remote-exit";
436 436
             break;
437 437
     }
... ...
@@ -193,7 +193,25 @@ void
193 193
 receive_exit_message(struct context *c)
194 194
 {
195 195
     dmsg(D_STREAM_ERRORS, "Exit message received by peer");
196
-    c->sig->signal_received = SIGTERM;
196
+    /* With control channel exit notification, we want to give the session
197
+     * enough time to handle retransmits and acknowledgment, so that eventual
198
+     * retries from the client to resend the exit or ACKs will not trigger
199
+     * a new session (we already forgot the session but the packet's HMAC
200
+     * is still valid).  This could happen for the entire period that the
201
+     * HMAC timeslot is still valid, but waiting five seconds here does not
202
+     * hurt much, takes care of the retransmits, and is easier code-wise.
203
+     *
204
+     * This does not affect OCC exit since the HMAC session code will
205
+     * ignore DATA packets
206
+     * */
207
+    if (c->options.mode == MODE_SERVER)
208
+    {
209
+        schedule_exit(c, c->options.scheduled_exit_interval, SIGTERM);
210
+    }
211
+    else
212
+    {
213
+        c->sig->signal_received = SIGUSR1;
214
+    }
197 215
     c->sig->signal_text = "remote-exit";
198 216
 #ifdef ENABLE_MANAGEMENT
199 217
     if (management)