From: Frank Lichtenheld Date: Wed, 5 Aug 2026 13:43:36 +0000 (+0200) Subject: ssl: Do not queue control ciphertext while a packet is still queued X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f264f6274b1696d28b277f5dd0fa93d889eb6f80;p=thirdparty%2Fopenvpn.git ssl: Do not queue control ciphertext while a packet is still queued An outgoing control channel packet is handed to the link layer as a buffer descriptor pointing into the reliable send buffer it was built from, and the packet id sits in that buffer's headroom, right in front of the payload. If the entry is reused before the packet has been written out, buf_copy_n() writes the new payload and reliable_mark_active_outgoing() prepends the new packet id exactly over the packet id of the queued packet, while its opcode, ACK array and length stay untouched. The queued packet then goes out with somebody else's packet id. Observed in a TCP p2p handshake: both peers reset simultaneously, the peer's two HARD_RESET packets arrive back to back, so io_wait_dowork() takes the residual data shortcut (event_set_status = SOCKET_READ) and does not write out our already queued HARD_RESET retransmit. The ACK in the second peer reset then purges our reset from the send window, tls_process_state() moves to S_START and queues the ClientHello into the very same (now inactive) entry. Result on the wire: a HARD_RESET with the ClientHello's packet id 1, followed by the ClientHello with the same id 1. The receiver consumes the reset, advances its receive window, and drops the real ClientHello as a replay - the handshake deadlocks until it times out. The send path in tls_process_state() and the dedicated ACK path in tls_process() are already guarded by to_link->len, only the ciphertext queueing was not. Guard it as well. A pending to_link makes tls_process() report itself as active, so we are called again as soon as the packet has been written out. In the error path this can drop a TLS alert that we would have queued, which is in line with that path not ensuring delivery anyway. Change-Id: Ib0e7c9d3a2f4e6b8c1d5a9f7e3b2c4d6a8f1e5b3 Signed-off-by: Frank Lichtenheld Acked-by: Arne Schwabe Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1831 Message-Id: <20260805134336.163392-1-frank@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg38133.html Signed-off-by: Gert Doering --- diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index d5100af76..d53901881 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -2769,8 +2769,15 @@ write_outgoing_tls_ciphertext(struct tls_session *session, bool *continue_tls_pr static bool check_outgoing_ciphertext(struct key_state *ks, struct tls_session *session, - bool *continue_tls_process) + struct buffer *to_link, bool *continue_tls_process) { + if (to_link->len) + { + dmsg(D_TLS_DEBUG, + "Deferring outgoing ciphertext, previous packet not written out yet"); + return true; + } + /* Outgoing Ciphertext to reliable buffer */ if (ks->state >= S_START) { @@ -2950,7 +2957,7 @@ tls_process_state(struct tls_multi *multi, struct tls_session *session, struct b dmsg(D_TLS_DEBUG, "Outgoing Plaintext -> TLS"); } } - if (!check_outgoing_ciphertext(ks, session, &continue_tls_process)) + if (!check_outgoing_ciphertext(ks, session, to_link, &continue_tls_process)) { goto error; } @@ -2962,7 +2969,7 @@ error: /* Shut down the TLS session but do a last read from the TLS * object to be able to read potential TLS alerts */ key_state_ssl_shutdown(&ks->ks_ssl); - check_outgoing_ciphertext(ks, session, &continue_tls_process); + check_outgoing_ciphertext(ks, session, to_link, &continue_tls_process); /* Put ourselves in the pre error state that will only send out the * control channel packets but nothing else */