diff mbox series

[Openvpn-devel,v1] ssl: Ignore hard reset packets with a non-zero packet id

Message ID 20260731114605.11596-1-gert@greenie.muc.de
State New
Headers
Series [Openvpn-devel,v1] ssl: Ignore hard reset packets with a non-zero packet id |

Commit Message

Gert Doering July 31, 2026, 11:45 a.m. UTC 
  From: Frank Lichtenheld <frank@lichtenheld.com>

A hard reset is always the first packet of a session, so it always
carries reliable packet id 0. tls_process_state() relies on that when it
treats a received reset as the early negotiation packet only for packet
id 0, and the stateless three-way handshake relies on it as well (see
the comment in session_skip_to_pre_start()).

A reset claiming a different id is therefore bogus.

We had a bug that could cause hard reset replays with packet id 1 in
specific scenarios (P2P TCP). In that case we accepted the packet id
at face value and then ignored the control packet that actually had
id 1 as an replay. This caused a difficult to diagnose dead connection
that was stuck just before TLS negotiation. The check added handles
this specific scenario well in that we just ignore the bogus reset
but do not abort the connection attempt. Starting fresh might retrigger
the bug. If there would be a separate bug where the client only sends
hard resets with packet id 1 we will still get logging on the server
side now.

Change-Id: I3c7d1f9e5b2a4c6d8e1f3a5b7c9d2e4f6a8b1c3d
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1832
---

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

Acked-by according to Gerrit (reflected above):
Arne Schwabe <arne-openvpn@rfc2549.org>

Comments

Gert Doering July 31, 2026, 8:09 p.m. UTC | #1 
Applied this to my t_server testbed, and it nicely displays how the
server ignores the misbehaving client (patch in #1831)

Jul 31 20:02:56 gentoo tap-tcp-p2p[4964]: TLS Error: received P_CONTROL_HARD_RESET_CLIENT_V2 with packet id 1 from [AF_INET6]::ffff:194.97.140.21:14610 -- 0 was expected, ignoring packet
Jul 31 20:03:18 gentoo tap-tcp-p2p[4964]: TLS Error: received P_CONTROL_HARD_RESET_CLIENT_V2 with packet id 1 from [AF_INET6]2001:608:0:814::f000:21:33270 -- 0 was expected, ignoring packet

.. and all "9" instances (p2p tcp tls) are now succeeding.  Great :-)

Your patch has been applied to the master, release/2.7 and release/2.7
branch (bugfix).  Not backported to 2.5 as it's annoying but not very
critical.

commit d1e67f419f1ea9121d44fa4b91c59e7209785e57 (master)
commit 6d7685f41b0d3efad549543eae7081a1aa9e57f2 (release/2.7)
commit 6ad370fa03806a7bf73eda8384131bc6175e4e09 (release/2.6)
Author: Frank Lichtenheld
Date:   Fri Jul 31 13:45:54 2026 +0200

     ssl: Ignore hard reset packets with a non-zero packet id

     Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
     Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1832
     Message-Id: <20260731114605.11596-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg38098.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 7f2e850..46ccc12 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -3903,8 +3903,22 @@ 
         /* Extract the packet ID from the packet */
         if (reliable_ack_read_packet_id(buf, &id))
         {
+            /* A hard reset always is the first packet of a session, so it
+             * always must use packet id 0. Ignore it if it claims another id.
+             * In a specific existing bug these packets were replays of an
+             * already handled reset, so ignoring it is better than aborting
+             * the connection attempt.
+             */
+            if (is_hard_reset_method2(op) && id != 0)
+            {
+                msg(D_TLS_ERRORS,
+                    "TLS Error: received %s with packet id " packet_id_format
+                    " from %s -- 0 was expected, ignoring packet",
+                    packet_opcode_name(op), (packet_id_print_type)id,
+                    print_link_socket_actual(from, &gc));
+            }
             /* Avoid deadlock by rejecting packet that would de-sequentialize receive buffer */
-            if (reliable_wont_break_sequentiality(ks->rec_reliable, id))
+            else if (reliable_wont_break_sequentiality(ks->rec_reliable, id))
             {
                 if (reliable_not_replay(ks->rec_reliable, id))
                 {