diff mbox series

[Openvpn-devel,v1] options: limit ping and keepalive values to one day

Message ID 20260722205210.8060-1-gert@greenie.muc.de
State New
Headers
Series [Openvpn-devel,v1] options: limit ping and keepalive values to one day |

Commit Message

Gert Doering July 22, 2026, 8:52 p.m. UTC 
  From: Marco Baffo <marco@mandelbit.com>

A correct configuration should not require such large ping intervals or
timeouts, and an upper limit of one day is already generous.

Limit --ping, --ping-exit, --ping-restart and --keepalive values to
86400 seconds. Since --keepalive doubles the timeout in server mode,
limit its timeout argument to 43200 seconds there.

Related: https://lore.kernel.org/all/20260722044756.872870-1-marco@mandelbit.com/

Change-Id: Ib18e1ed0851bc0fe6d8448e601d11d6abaa710c1
Signed-off-by: Marco Baffo <marco@mandelbit.com>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1798
---

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/+/1798
This mail reflects revision 1 of this Change.

Acked-by according to Gerrit (reflected above):
Frank Lichtenheld <frank@lichtenheld.com>
diff mbox series

Patch

diff --git a/doc/man-sections/link-options.rst b/doc/man-sections/link-options.rst
index df8c917..77c3c41 100644
--- a/doc/man-sections/link-options.rst
+++ b/doc/man-sections/link-options.rst
@@ -74,7 +74,9 @@ 
      keepalive interval timeout
 
   Send ping once every ``interval`` seconds, restart if ping is not received
-  for ``timeout`` seconds.
+  for ``timeout`` seconds. Both values are limited to 86400 seconds. Since the
+  server-side timeout is doubled, ``timeout`` is limited to 43200 seconds in
+  server mode.
 
   This option can be used on both client and server side, but it is enough
   to add this on the server side as it will push appropriate ``--ping``
@@ -247,6 +249,8 @@ 
   cause ping packets to be sent in both directions since OpenVPN ping
   packets are not echoed like IP ping packets).
 
+  The maximum value for ``n`` is 86400 seconds.
+
   This option has two intended uses:
 
   (1)  Compatibility with stateful firewalls. The periodic ping will ensure
@@ -264,6 +268,8 @@ 
   ``--inactive``, ``--ping`` and ``--ping-exit`` to create a two-tiered
   inactivity disconnect.
 
+  The maximum value for ``n`` is 86400 seconds.
+
   For example,
   ::
 
@@ -278,6 +284,8 @@ 
   ``n`` seconds pass without reception of a ping or other packet from
   remote.
 
+  The maximum value for ``n`` is 86400 seconds.
+
   This option is useful in cases where the remote peer has a dynamic IP
   address and a low-TTL DNS name is used to track the IP address using a
   service such as https://www.nsupdate.info/ + a dynamic DNS client such as
diff --git a/src/openvpn/helper.c b/src/openvpn/helper.c
index 4c540a6..6fd2689 100644
--- a/src/openvpn/helper.c
+++ b/src/openvpn/helper.c
@@ -563,6 +563,12 @@ 
             msg(M_USAGE,
                 "--keepalive conflicts with --ping, --ping-exit, or --ping-restart.  If you use --keepalive, you don't need any of the other --ping directives.");
         }
+        if (o->mode == MODE_SERVER && o->keepalive_timeout > PING_TIMEOUT_MAX / 2)
+        {
+            msg(M_USAGE,
+                "The second parameter to --keepalive must not exceed %d in server mode.",
+                PING_TIMEOUT_MAX / 2);
+        }
 
         /*
          * Expand.
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 87218d4..003b460 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -6789,24 +6789,29 @@ 
     else if (streq(p[0], "keepalive") && p[1] && p[2] && !p[3])
     {
         VERIFY_PERMISSION(OPT_P_GENERAL);
-        atoi_constrained(p[1], &options->keepalive_ping, "keepalive ping", 1, INT_MAX, msglevel);
-        atoi_constrained(p[2], &options->keepalive_timeout, "keepalive timeout", 1, INT_MAX, msglevel);
+        atoi_constrained(p[1], &options->keepalive_ping, "keepalive ping",
+                         1, PING_TIMEOUT_MAX, msglevel);
+        atoi_constrained(p[2], &options->keepalive_timeout, "keepalive timeout",
+                         1, PING_TIMEOUT_MAX, msglevel);
     }
     else if (streq(p[0], "ping") && p[1] && !p[2])
     {
         VERIFY_PERMISSION(OPT_P_TIMER);
-        options->ping_send_timeout = positive_atoi(p[1], msglevel);
+        atoi_constrained(p[1], &options->ping_send_timeout, p[0],
+                         0, PING_TIMEOUT_MAX, msglevel);
     }
     else if (streq(p[0], "ping-exit") && p[1] && !p[2])
     {
         VERIFY_PERMISSION(OPT_P_TIMER);
-        options->ping_rec_timeout = positive_atoi(p[1], msglevel);
+        atoi_constrained(p[1], &options->ping_rec_timeout, p[0],
+                         0, PING_TIMEOUT_MAX, msglevel);
         options->ping_rec_timeout_action = PING_EXIT;
     }
     else if (streq(p[0], "ping-restart") && p[1] && !p[2])
     {
         VERIFY_PERMISSION(OPT_P_TIMER);
-        options->ping_rec_timeout = positive_atoi(p[1], msglevel);
+        atoi_constrained(p[1], &options->ping_rec_timeout, p[0],
+                         0, PING_TIMEOUT_MAX, msglevel);
         options->ping_rec_timeout_action = PING_RESTART;
     }
     else if (streq(p[0], "ping-timer-rem") && !p[1])
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index a111cf8..2f7fe30 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -56,6 +56,8 @@ 
 #define OPTION_PARM_SIZE 256
 #define OPTION_LINE_SIZE 256
 
+#define PING_TIMEOUT_MAX 86400 /* one day (in seconds) */
+
 extern const char title_string[];
 
 /* certain options are saved before --pull modifications are applied */