Attention is currently required from: plaisthos, razvanc.
Hello plaisthos, razvanc,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/1908?usp=email
to look at the new patch set (#3).
Change subject: Avoid unbounded reliable TLS timeout
......................................................................
Avoid unbounded reliable TLS timeout
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) <[email protected]>
CVE: 2026-84732
Change-Id: Id8ac9c48a8f751b0df95c6436ad3fbff3c4ae4a6
Signed-off-by: Arne Schwabe <[email protected]>
Signed-off-by: Razvan Cojocaru <[email protected]>
Acked-by: MaxF <[email protected]>
(cherry picked from commit 207ac5f47e46565881dcec385020ebdcb682caa4)
---
M src/openvpn/options.c
M src/openvpn/reliable.c
M src/openvpn/reliable.h
3 files changed, 45 insertions(+), 11 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/08/1908/3
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
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1908?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: release/2.6
Gerrit-Change-Id: Id8ac9c48a8f751b0df95c6436ad3fbff3c4ae4a6
Gerrit-Change-Number: 1908
Gerrit-PatchSet: 3
Gerrit-Owner: MaxF <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-Reviewer: razvanc <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-Attention: razvanc <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
