From patchwork Fri Sep 4 16:08:42 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gert Doering X-Patchwork-Id: 5315 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:2190:b0:892:1b45:3040 with SMTP id s16csp3824104mae; Fri, 4 Sep 2026 09:09:07 -0700 (PDT) X-Forwarded-Encrypted: i=2; AKwUvBxbLk5HnerC4YeO2YB3b7+6ZYYV7r1rFBEmxCj0o8SMqSe2jxP5uq1xtDDzCC1kukCEyoM06rHVKOY=@openvpn.net X-Received: by 2002:a05:6870:a0a2:b0:475:e235:903 with SMTP id 586e51a60fabf-475e2350fa9mr2538844fac.33.1788538147612; Fri, 04 Sep 2026 09:09:07 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1788538147; cv=none; d=google.com; s=arc-20260327; b=swbd3RTzmX47NuspuzdzTaFVmzu6/ZGgM2ioT2tnnCrrCYX/kQjQdQ0I4bDW+Wmb5X lpAe34qoiwyEnNoTJuSJjACYiSv9hN9+KvmDgJ/dOKIhim/dx3gaPT8Tm9C+7uyAG+8/ YQIdteFbki9n9KF++10Le6ir2TbbRlUKJVcw35bOC7R7mJLK/SIkN9stEKTkoV/B0MgF u6PqR9l4rCbmkQ5OFO7XtB/17kDTPonB5lkIkll2GNP5W4+gAmtTUNZzwQ+wjrGvz/qj z3f/Xjv6J0LdhvVwZneV3IpCT4HTWbLYIQwlDGsgNIt5LFRDDLhyJw1X3X+24XV97oqF eOXA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20260327; h=errors-to:content-transfer-encoding:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence:subject :mime-version:references:in-reply-to:message-id:date:to:from :dkim-signature:dkim-signature:dkim-signature; bh=0r6GIAvhN27AhsxoCpP3LNM10mSx8vnzF0C+mR+gmEM=; fh=4NbAC/LsuMLI0S0hprUlLSLCiHwg6SCAifhH718Jh0Q=; b=OvfFym5bhEuwTYDEzgtGMJftvfc2DqN2EqUTi4yDNKm9/jppsDoVf/vfkWHPGIYuht /ETfBVEysxGR7SZpbT2zK1JAFOzM9Mwk8rOtBDkMbKBZXU0hZ8L5mDMiH8eJVMvl58B0 60Nl8UsvxIp3EggooEk6sWRGJd+cppke7jMiERDKu4VmqOy9ETDg8gQocNY1Hm1uiRT7 FXWv9nKYAi/v8HiBroNAPEiuKezeLnPSIgbZ8rgyuhXENJIjdIClAD1Lmh9LCVTLXZ3m EW4VUQ8aChl2aTdje5H4OMwsRdr+9vptA4KNMHOI5/BO+Fg6M6E1Lf8xyKcYsfuYBzNB zG4w==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@lists.sourceforge.net header.s=beta header.b=OOV0CYCd; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=KxF7bF3G; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=CydhgZJr; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=muc.de Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id 586e51a60fabf-47555b26c24si4473631fac.239.2026.09.04.09.09.07 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 04 Sep 2026 09:09:07 -0700 (PDT) Received-SPF: pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) client-ip=216.105.38.7; Authentication-Results: mx.google.com; dkim=pass header.i=@lists.sourceforge.net header.s=beta header.b=OOV0CYCd; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=KxF7bF3G; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=CydhgZJr; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=muc.de DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.sourceforge.net; s=beta; h=Content-Transfer-Encoding:Content-Type: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: Subject:MIME-Version:References:In-Reply-To:Message-ID:Date:To:From:Sender: Reply-To:Cc:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=0r6GIAvhN27AhsxoCpP3LNM10mSx8vnzF0C+mR+gmEM=; b=OOV0CYCdRQxGYJPZRgcTp8Xzfn h1OgU/2fF5eJtTzFzrngKOim/ECu6ZBbYPmsF/pK7dyN9w2/BsgjlhU/qa6kGxSKG56i9rCg/kAwG cGLRpCjEdINeoyASe0oMSR+y5TeMXxUrKe5E8GctJOtCRgg0UDJI2loBCmmgu16u7E9E=; Received: from [127.0.0.1] (helo=sfs-ml-3.v29.lw.sourceforge.com) by sfs-ml-3.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1x2WTN-0007W6-2z; Fri, 04 Sep 2026 16:09:01 +0000 Received: from [172.30.29.66] (helo=mx.sourceforge.net) by sfs-ml-3.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1x2WTL-0007Vp-4G for openvpn-devel@lists.sourceforge.net; Fri, 04 Sep 2026 16:08:59 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-ID:Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=y4cZCPZSYl0GQD/6xBHkICnYgWi9fDvMDwfmedhLm3w=; b=KxF7bF3GKhy5dqomEzmZkDuSdo 5+DyMIfILyYtFF/gxL9jyL58jurWEKVwUVW93QyxrCWe+NL08m+MDbIIwSvg2AE3y17DkY0XpZPAD lZ98vWch+dqSTAqQrko+fg9s0/cfczO+JJYmFir39LsFfPCldYqTRN4CEmc1vw90bZeE=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-ID: Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=y4cZCPZSYl0GQD/6xBHkICnYgWi9fDvMDwfmedhLm3w=; b=CydhgZJrsUyD2nGQeM90i2Q+cu tIZnfufOz7oN+iwgyGtOnBiwzX+nJ3toEQf/Gz+rVvYz1blYa6bIqqFlWF5y2fqcv6xr+Z+9WnAdo 3zdl/iwugzygHaJnIdZQH7xlsakeDQqbSLNuAxBUcORFgSDGU9y7NAV6SsHscuY6gFaE=; Received: from [193.149.48.129] (helo=blue.greenie.muc.de) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.95) id 1x2WTI-0003JS-GB for openvpn-devel@lists.sourceforge.net; Fri, 04 Sep 2026 16:08:58 +0000 Received: from blue.greenie.muc.de (localhost [127.0.0.1]) by blue.greenie.muc.de (8.18.1/8.18.1) with ESMTP id 684G8nlr005567 for ; Fri, 4 Sep 2026 18:08:49 +0200 Received: (from gert@localhost) by blue.greenie.muc.de (8.18.2/8.18.1/Submit) id 684G8nIo005566 for openvpn-devel@lists.sourceforge.net; Fri, 4 Sep 2026 18:08:49 +0200 From: Gert Doering To: openvpn-devel@lists.sourceforge.net Date: Fri, 4 Sep 2026 18:08:42 +0200 Message-ID: <20260904160848.5553-1-gert@greenie.muc.de> X-Mailer: git-send-email 2.53.0 In-Reply-To: References: MIME-Version: 1.0 X-Spam-Score: 1.3 (+) X-Spam-Report: Spam detection software, running on the system "sfi-spamd-2.hosts.colo.sdot.me", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: From: Lev Stipakov Add regression coverage for the reliability-layer denial-of-service fix to test_packet_id, which already links reliable.c: - the retransmission timeout must stay positive and bounded no matter how often the fast-retransmit path is forced, so it can no longer overflow; - reliable_send_purge() must ignore ACKs for packet I [...] Content analysis details: (1.3 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 1.3 RDNS_NONE Delivered to internal network by a host with no rDNS X-Headers-End: 1x2WTI-0003JS-GB Subject: [Openvpn-devel] [PATCH v3] reliable: add unit tests for ACK and backoff DoS hardening X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: 1875340533145813388 X-GMAIL-MSGID: 1875418176675512308 From: Lev Stipakov Add regression coverage for the reliability-layer denial-of-service fix to test_packet_id, which already links reliable.c: - the retransmission timeout must stay positive and bounded no matter how often the fast-retransmit path is forced, so it can no longer overflow; - reliable_send_purge() must ignore ACKs for packet IDs that could never have been in flight (out of the send window, or wrapped-around), so a peer cannot inflate n_acks and force early retransmits; - a legitimate ACK for a real higher packet ID still removes that entry and counts towards fast retransmit (positive control). These tests fail on the unfixed code and pass with the fix. Signed-off-by: Lev Stipakov Acked-by: Razvan Cojocaru Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1894 Change-Id: I3c6ad247614fb204a279b73e98f8c4704e16da2a --- 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/+/1894 This mail reflects revision 3 of this Change. Acked-by according to Gerrit (reflected above): Razvan Cojocaru diff --git a/tests/unit_tests/openvpn/test_packet_id.c b/tests/unit_tests/openvpn/test_packet_id.c index a5d50de..dc77553 100644 --- a/tests/unit_tests/openvpn/test_packet_id.c +++ b/tests/unit_tests/openvpn/test_packet_id.c @@ -372,6 +372,150 @@ } +/* The fix keeps the timeout well below this; the broken code grows past it. + * A plain number, so the test builds with or without the fix. */ +#define SANE_TIMEOUT_BOUND (10 * 1000 * 1000) + +static struct reliable * +test_reliable_new(void) +{ + struct reliable *rel = malloc(sizeof(struct reliable)); + assert_non_null(rel); + /* reliable_init() zeroes everything, so each test only sets the + * fields it actually needs. */ + reliable_init(rel, 100, 50, 8, false); + rel->initial_timeout = 2; + return rel; +} + +/* + * Each retransmit doubles the timeout. If a peer keeps forcing retransmits, + * the broken code doubles it forever: after about 30 rounds it overflows and + * turns zero or negative, and then the packet is resent nonstop (the flood). + * The fix caps the doubling. This test forces many retransmits and checks the + * timeout never overflows or grows without limit. + */ +static void +test_reliable_backoff_is_bounded(void **state) +{ + (void)state; + now = 1000; + + struct reliable *rel = test_reliable_new(); + + struct reliable_entry *e = &rel->array[0]; + e->active = true; + e->packet_id = 1; + e->timeout = rel->initial_timeout; + rel->packet_id = 2; + + for (int i = 0; i < 40; ++i) + { + /* make the packet due for a fast retransmit */ + e->n_acks = N_ACK_RETRANSMIT; + + int opcode; + struct buffer *buf = reliable_send(rel, &opcode); + /* our one active packet is the one picked to send */ + assert_ptr_equal(buf, &e->buf); + + /* a zero or negative timeout would resend with no delay (the flood) */ + assert_true(e->timeout > 0); + /* the timeout must stop growing, not double forever */ + assert_true(e->timeout <= SANE_TIMEOUT_BOUND); + } + + reliable_free(rel); +} + +/* + * An ACK should only count if it is for a packet we actually sent. If the + * broken code accepts ACKs for packets that were never sent, a peer can force + * early retransmits at will (which then feeds the timeout overflow above). + * These two cases send such bogus ACKs and check they are ignored. + */ +static void +test_reliable_purge_ignores_forged_acks(void **state) +{ + (void)state; + + /* Case (a): an ACK for pid 0x40000000, which we never sent (we only sent + * 0 and 1). The old "e->packet_id < pid" check treats it as newer and + * counts it. It should be ignored. */ + { + struct reliable *rel = test_reliable_new(); + struct reliable_entry *e = &rel->array[0]; + e->active = true; + e->packet_id = 1; + rel->packet_id = 2; /* only pids 0 and 1 were ever sent */ + + struct reliable_ack ack = { .len = 1, .packet_id = { 0x40000000 } }; + reliable_send_purge(rel, &ack); + + /* the bogus ACK must not be counted */ + assert_int_equal(e->n_acks, 0); + /* and must not drop our real packet */ + assert_true(e->active); + reliable_free(rel); + } + + /* Case (b): an ACK for pid 0xFFFFFFFF. It is inside the send window, so + * it passes the window check, but ids wrap around and 0xFFFFFFFF is really + * older than our pid 1. A plain "<" thinks it is newer and counts it; the + * wraparound-aware comparison must not. */ + { + struct reliable *rel = test_reliable_new(); + struct reliable_entry *e = &rel->array[0]; + e->active = true; + e->packet_id = 1; + rel->packet_id = 2; + + struct reliable_ack ack = { .len = 1, .packet_id = { 0xFFFFFFFF } }; + reliable_send_purge(rel, &ack); + + /* the bogus ACK must not be counted */ + assert_int_equal(e->n_acks, 0); + /* and must not drop our real packet */ + assert_true(e->active); + reliable_free(rel); + } +} + +/* + * Sanity check: a real ACK must still work. Acknowledging a higher packet + * should drop that packet and count once towards resending the older one. + * The fix must not break this. + */ +static void +test_reliable_purge_legitimate_ack(void **state) +{ + (void)state; + + struct reliable *rel = test_reliable_new(); + + struct reliable_entry *e0 = &rel->array[0]; + e0->active = true; + e0->packet_id = 1; + + struct reliable_entry *e1 = &rel->array[1]; + e1->active = true; + e1->packet_id = 2; + + rel->packet_id = 3; /* pids 0,1,2 sent; 1 and 2 still waiting */ + + struct reliable_ack ack = { .len = 1, .packet_id = { 2 } }; + reliable_send_purge(rel, &ack); + + /* packet 2 was acked, so it is dropped */ + assert_false(e1->active); + /* packet 1 is older, so it gets one ACK towards an early resend */ + assert_int_equal(e0->n_acks, 1); + /* packet 1 was not acked, so it stays */ + assert_true(e0->active); + + reliable_free(rel); +} + int main(void) { @@ -394,7 +538,10 @@ cmocka_unit_test(test_get_num_output_sequenced_available), cmocka_unit_test(test_copy_acks_to_lru), - cmocka_unit_test(test_packet_id_window) + cmocka_unit_test(test_packet_id_window), + cmocka_unit_test(test_reliable_backoff_is_bounded), + cmocka_unit_test(test_reliable_purge_ignores_forged_acks), + cmocka_unit_test(test_reliable_purge_legitimate_ack) };