diff mbox series

[Openvpn-devel,v3] Avoid unbounded reliable TLS timeout

Message ID 20260910085028.30710-1-gert@greenie.muc.de
State New
Headers
Series [Openvpn-devel,v3] Avoid unbounded reliable TLS timeout |

Commit Message

Gert Doering Sept. 10, 2026, 8:50 a.m. UTC 
  From: Arne Schwabe <arne@rfc2549.org>

Avoid an unbounded TLS retransmit timeout, which could potentially
trigger an integer overflow.

The maximum initial timeout is defined in reliable.h and used to
constrain --tls-timeout, so that the relation between the option range
and RELIABLE_MAX_TIMEOUT_SHIFT is explicit and checked at compile time.

Github: OpenVPN/openvpn-private-issues#161
Reported-By: Mark Bregman (Fox-IT) <mark.bregman@fox-it.com>
CVE: 2026-84732
Change-Id: Id8ac9c48a8f751b0df95c6436ad3fbff3c4ae4a6
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Signed-off-by: Razvan Cojocaru <razvanc@mailbox.org>
Acked-by: Razvan Cojocaru <razvanc@mailbox.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1908
Acked-by: MaxF <max@max-fillinger.net>
(cherry picked from commit 207ac5f47e46565881dcec385020ebdcb682caa4)
---

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

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

Acked-by according to Gerrit (reflected above):
Razvan Cojocaru <razvanc@mailbox.org>
diff mbox series

Patch

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 9344f99..56041bb 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -9180,6 +9180,15 @@ 
     {
         VERIFY_PERMISSION(OPT_P_TLS_PARMS);
         options->tls_timeout = positive_atoi(p[1]);
+        /* Constrain the timeout to not have problems with
+         * RELIABLE_MAX_TIMEOUT_SHIFT creating an overflow. 65k seconds
+         * timeout is already way too much anyway */
+        if (options->tls_timeout < 1 || options->tls_timeout > RELIABLE_MAX_INITIAL_TIMEOUT)
+        {
+            msg(msglevel, "tls_timeout: Must be an integer between %d and %d, not %d",
+                1, RELIABLE_MAX_INITIAL_TIMEOUT, options->tls_timeout);
+            goto err;
+        }
     }
     else if (streq(p[0], "reneg-bytes") && p[1] && !p[2])
     {
diff --git a/src/openvpn/reliable.c b/src/openvpn/reliable.c
index c2b9439..babe4c8 100644
--- a/src/openvpn/reliable.c
+++ b/src/openvpn/reliable.c
@@ -683,11 +683,22 @@ 
             }
         }
     }
+
     if (best)
     {
+        /* The initial timeout is bounded by RELIABLE_MAX_INITIAL_TIMEOUT, so
+         * shifting it cannot overflow. */
+        static_assert(RELIABLE_MAX_INITIAL_TIMEOUT <= (INT_MAX >> RELIABLE_MAX_TIMEOUT_SHIFT),
+                      "initial reliable timeout overflows when shifted");
+        const interval_t max_timeout = rel->initial_timeout << RELIABLE_MAX_TIMEOUT_SHIFT;
+
         /* exponential backoff */
         best->next_try = local_now + best->timeout;
-        best->timeout *= 2;
+        if (best->timeout < max_timeout)
+        {
+            best->timeout *= 2;
+        }
+
         best->n_acks = 0;
         *opcode = best->opcode;
         dmsg(D_REL_DEBUG, "ACK reliable_send ID " packet_id_format " (size=%d to=%d)",
diff --git a/src/openvpn/reliable.h b/src/openvpn/reliable.h
index 766c07d..5877f20 100644
--- a/src/openvpn/reliable.h
+++ b/src/openvpn/reliable.h
@@ -41,18 +41,32 @@ 
  *  @{ */
 
 
-#define RELIABLE_ACK_SIZE 8     /**< The maximum number of packet IDs
-                                 *   waiting to be acknowledged which can
-                                 *   be stored in one \c reliable_ack
-                                 *   structure. */
+#define RELIABLE_ACK_SIZE 8
+/**< The maximum number of packet IDs
+ *   waiting to be acknowledged which can
+ *   be stored in one \c reliable_ack
+ *   structure. */
 
-#define RELIABLE_CAPACITY 12    /**< The maximum number of packets that
-                                 *   the reliability layer for one VPN
-                                 *   tunnel in one direction can store. */
+#define RELIABLE_CAPACITY 12
+/**< The maximum number of packets that
+ *   the reliability layer for one VPN
+ *   tunnel in one direction can store. */
 
-#define N_ACK_RETRANSMIT 3      /**< We retry sending a packet early if
-                                 *   this many later packets have been
-                                 *   ACKed. */
+#define N_ACK_RETRANSMIT 3
+/**< We retry sending a packet early if
+ *   this many later packets have been
+ *   ACKed. */
+
+#define RELIABLE_MAX_TIMEOUT_SHIFT 6
+/**< Maximum shift or doubling in exponential backoff
+ * we allow. This is a safeguard against an unbounded
+ * exponential backoff. With the default timeout of 2s this
+ * equals 128s */
+
+#define RELIABLE_MAX_INITIAL_TIMEOUT (1 << 16)
+/**< Maximum initial timeout (--tls-timeout) we accept.
+ * Bounded so that shifting it by RELIABLE_MAX_TIMEOUT_SHIFT
+ * cannot overflow an int. */
 
 /**
  * The acknowledgment structure in which packet IDs are stored for later