Attention is currently required from: plaisthos, razvanc. MaxF has uploaded this change for review. ( http://gerrit.openvpn.net/c/openvpn/+/1908?usp=email )
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/1 diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 9344f99..873efaf 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 %lld", + 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: newchange Gerrit-Project: openvpn Gerrit-Branch: release/2.6 Gerrit-Change-Id: Id8ac9c48a8f751b0df95c6436ad3fbff3c4ae4a6 Gerrit-Change-Number: 1908 Gerrit-PatchSet: 1 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
