From: Frederic Lecaille <flecaille@haproxy.com>
Date: Wed, 2 Jul 2025 14:37:02 +0000 (+0200)
Subject: BUG/MINOR: quic-be: Malformed coalesced Initial packets
X-Git-Tag: v3.3-dev3~31
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=87ada46f38054e4debb852ae4b98feec4676f4af;p=thirdparty%2Fhaproxy.git

BUG/MINOR: quic-be: Malformed coalesced Initial packets

This bug fix completes this patch which was not sufficient:

   MINOR: quic-be: Allow sending 1200 bytes Initial datagrams

This patch could not allow the build of well formed Initial packets coalesced to
others (Handshake) packets. Indeed, the <padding> parameter passed to qc_build_pkt()
is deduced from a first value: <padding> value and must be set to 1 for
the last encryption level. As a client, the last encryption level is always
the Handshake encryption level. But <padding> was always set to 1 for a QUIC
client, leading the first Initial packet to be malformed because considered
as the second one into the same datagram.

So, this patch sets <padding> value passed to qc_build_pkt() to 1 only when there
is no last encryption level at all, to allow the build of Initial only packets
(not coalesced) or when it frames to send (coalesced packets).

No need to backport.
---

diff --git a/src/quic_tx.c b/src/quic_tx.c
index 8888fa77c..5cf57f639 100644
--- a/src/quic_tx.c
+++ b/src/quic_tx.c
@@ -674,37 +674,51 @@ static int qc_prep_pkts(struct quic_conn *qc, struct buffer *buf,
 			}
 
 			/* RFC 9000 14.1 Initial datagram size
+			 *
+			 * A client MUST expand the payload of all UDP datagrams carrying Initial
+			 * packets to at least the smallest allowed maximum datagram size of 1200
+			 * bytes by adding PADDING frames to the Initial packet or by coalescing
+			 * the Initial packet; see Section 12.2. Initial packets can even be
+			 * coalesced with invalid packets, which a receiver will discard.
 			 *
 			 * Similarly, a server MUST expand the payload of all UDP
 			 * datagrams carrying ack-eliciting Initial packets to at least the
 			 * smallest allowed maximum datagram size of 1200 bytes.
 			 */
 			if (qel == qc->iel && (!l || !LIST_ISEMPTY(frms) || probe)) {
-				 /* Ensure that no ack-eliciting packets are sent into too small datagrams */
+				 /* Ensure that no Initial packets are sent into too small datagrams */
 				if (end - pos < QUIC_INITIAL_PACKET_MINLEN) {
 					TRACE_PROTO("No more enough room to build an Initial packet",
 					            QUIC_EV_CONN_PHPKTS, qc);
 					break;
 				}
 
-				/* padding will be set for last QEL, except when probing:
-				 * to build a PING only non coalesced Initial datagram for
-				 * instance when blocked by the anti-amplification limit,
-				 * this datagram MUST be padded.
+				/* padding MUST ALWAYS be set for the last QEL, except:
+				 * - for a listener, when probing, that is to say
+				 *   to build a PING only non coalesced Initial datagram for
+				 *   instance when blocked by the anti-amplification limit,
+				 *   this datagram MUST be padded.
 				 */
 				padding = 1;
 			}
 
 			pkt_type = quic_enc_level_pkt_type(qc, qel);
-			/* <paddding> parameter for qc_build_pkt() must not be set to 1 when
+			/* For listeners:
+			 * <paddding> parameter for qc_build_pkt() must not be set to 1 when
 			 * building PING only Initial datagram (a datagram with an Initial
 			 * packet inside containing only a PING frame as ack-eliciting
 			 * frame). This is the case when both <probe> and LIST_EMPTY(<frms>)
 			 * conditions are verified (see qc_do_build_pkt()).
+			 *
+			 * For clients:
+			 * <padding> must be set to 1 only the current packet cannot be coalesed,
+			 * i.e. if the next qel is not present or empty.
 			 */
 			cur_pkt = qc_build_pkt(&pos, end, qel, tls_ctx, frms,
 			                       qc, ver, dglen, pkt_type, must_ack,
-			                       padding && (!l || (!next_qel && (!probe || !LIST_ISEMPTY(frms)))),
+			                       padding &&
+			                       ((!l && (!next_qel || LIST_ISEMPTY(next_qel->send_frms))) ||
+			                        (l && !next_qel && (!probe || !LIST_ISEMPTY(frms)))),
 			                       probe, cc, &err);
 			if (!cur_pkt) {
 				switch (err) {
@@ -1773,6 +1787,10 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end,
 	int ret = 0;
 
 	TRACE_ENTER(QUIC_EV_CONN_TXPKT, qc);
+	TRACE_PRINTF(TRACE_LEVEL_DEVELOPER, QUIC_EV_CONN_TXPKT, qc, 0, 0, 0,
+	             "dglen=%u pn=%llu must_ack=%d padding=%d cc=%d probe=%d qel=%c",
+	             (unsigned int)dglen, (ull)pn, must_ack, padding, cc, probe,
+	             quic_enc_level_char_from_qel(qel, qc));
 
 	/* Length field value with CRYPTO frames if present. */
 	len_frms = 0;