From d7dea408c64c327cab6aebf4ccad93405b675565 Mon Sep 17 00:00:00 2001 From: Frederic Lecaille Date: Fri, 5 Sep 2025 15:41:27 +0200 Subject: [PATCH] BUG/MINOR: quic: too short PADDING frame for too short packets This bug arrvived with this commit: MINOR: quic: centralize padding for HP sampling on packet building What was missed is the fact that at the centralization point for the PADDING frame to add for too short packet, payload length already includes <*pn_len> the packet number field length value. So when computing the length of the PADDING frame, the packet field length must not be considered and added to the payload length (). This bug leaded too short PADDING frame to too short packets. This was the case, most of times with Application level packets with a 1-byte packet number field followed by a 1-byte PING frame. A 1-byte PADDING frame was added in this case in place of a correct 2-bytes PADDINF frame. The header packet protection of such packet could not be removed by the clients as for instance for ngtcp2 with such traces: I00001828 0x5a135c81e803f092c74bac64a85513b657 pkt could not decrypt packet number As the header protection could no be removed, the header keyupdate bit could also not be read by packet analyzers such as pyshark used during the keyupdate tests. No need to backport. --- include/haproxy/quic_conn-t.h | 3 --- src/quic_tx.c | 14 +++++++++----- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/include/haproxy/quic_conn-t.h b/include/haproxy/quic_conn-t.h index 75493915d..4100e3061 100644 --- a/include/haproxy/quic_conn-t.h +++ b/include/haproxy/quic_conn-t.h @@ -145,9 +145,6 @@ enum quic_pkt_type { #define QUIC_PACKET_PNL_BITMASK 0x03 #define QUIC_PACKET_PN_MAXLEN 4 -/* TLS algo supported by QUIC uses a 16-bytes sample for HP. */ -#define QUIC_HP_SAMPLE_LEN 16 - /* * 0 1 2 3 * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 diff --git a/src/quic_tx.c b/src/quic_tx.c index 8a8f05dc3..7b57e0135 100644 --- a/src/quic_tx.c +++ b/src/quic_tx.c @@ -1990,12 +1990,16 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end, */ /* Add padding if packet is too small for HP sampling as specified - * above. QUIC TLS algos relies on 16 bytes sample extracted 4 bytes - * after PN offset. Thus, pn and payload must be at least 4 bytes long, - * so that the sample will be extracted as the AEAD tag. + * above. QUIC TLS algos relies on 16 bytes sample extracted + * QUIC_PACKET_PN_MAXLEN(4) bytes after the PN offset Thus, pn and payload + * must be at least QUIC_PACKET_PN_MAXLEN(4) bytes long, so that the sample + * will be extracted as the AEAD tag. + * + * Note that from here, includes <*pn_len>, the total frame lenghts, + * and QUIC_TLS_TAG_LEN(16). */ - if (*pn_len + len < QUIC_PACKET_PN_MAXLEN + QUIC_HP_SAMPLE_LEN) { - padding_len = QUIC_PACKET_PN_MAXLEN + QUIC_HP_SAMPLE_LEN - (*pn_len + len); + if (len < QUIC_PACKET_PN_MAXLEN + QUIC_TLS_TAG_LEN) { + padding_len = QUIC_PACKET_PN_MAXLEN + QUIC_TLS_TAG_LEN - len; TRACE_PRINTF(TRACE_LEVEL_DEVELOPER, QUIC_EV_CONN_PHPKTS, qc, 0, 0, 0, "adding padding pn=%llu padding_len=%zu *pn_len=%zu" " len=%zu len_frms=%zu",