mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-08-05 22:56:57 +02:00
Code Issues Projects Releases Wiki Activity

BUG/MAJOR: quic: fix padding with short packets

Browse Source 
QUIC sending functions were extended to be more flexible. Of all the
changes, they support now iterating over a variable instance of QEL
instance of only 2 previously. This change has rendered PADDING emission
less previsible, which was adjusted via the following patch :

  a60609f1aa
  BUG/MINOR: quic: fix padding of INITIAL packets

Its main purpose was to ensure PADDING would only be generated for the
last iterated QEL instance, to avoid unnecessary padding. In parallel, a
BUG_ON() statement ensure that built INITIAL packets are always padded
to 1.200 bytes as necessary before emitted them.

This BUG_ON() statement caused crash in one particular occurence : when
building datagrams that mixed Initial long packets and 1-RTT short
packets. This last occurence type does not have a length field in its
header, contrary to Long packets. This caused a miscalculation for the
necessary padding size, with INITIAL packets not padded enough to reach
the necessary 1.200 bytes size.

This issue was detected on 3.0.2. It can be reproduced by using 0-RTT
combined with latency. Here are the used commands :

  $ ngtcp2-client --tp-file=/tmp/ngtcp2-tp.txt \
    --session-file=/tmp/ngtcp2-session.txt --exit-on-all-streams-close \
    127.0.0.1 20443 "https://[::]/?s=32o"
  $ sudo tc qdisc add dev lo root netem latency 500ms

Note that this issue cannot be reproduced on current dev version.
Indeed, it seems that the following patch introduce a slight change in
packet building ordering :

  cdfceb10ae
  MINOR: quic: refactor qc_prep_pkts() loop

This must be backported to 3.0.

This should fix github issue #2609.
This commit is contained in:
Amaury Denoyelle 2024-06-18 15:20:32 +02:00
parent 7422f16da3
commit c714b6bb55
1 changed files with 14 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
src/quic_tx.c
View File

@ -1888,20 +1888,23 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end,
padding_len = 0;
len_sz = quic_int_getsize(len);
/* Add this packet size to <dglen> */
dglen += head_len + len_sz + len;
/* Note that <padding> is true only when building an Handshake packet
* coalesced to an Initial packet.
*/
dglen += head_len + len;
if (pkt->type != QUIC_PACKET_TYPE_SHORT)
dglen += len_sz;
if (padding && dglen < QUIC_INITIAL_PACKET_MINLEN) {
/* This is a maximum padding size */
padding_len = QUIC_INITIAL_PACKET_MINLEN - dglen;
/* The length field value is of this packet is <len> + <padding_len>
* the size of which may be greater than the initial computed size
* <len_sz>. So, let's deduce the difference between these to packet
* sizes from <padding_len>.
*/
padding_len -= quic_int_getsize(len + padding_len) - len_sz;
len += padding_len;
/* Update size of packet length field with new PADDING data. */
if (pkt->type != QUIC_PACKET_TYPE_SHORT) {
size_t len_sz_diff = quic_int_getsize(len) - len_sz;
if (len_sz_diff) {
padding_len -= len_sz_diff;
len_sz += len_sz_diff;
dglen += len_sz_diff;
}
}
}
else if (len_frms && len_frms < QUIC_PACKET_PN_MAXLEN) {
len += padding_len = QUIC_PACKET_PN_MAXLEN - len_frms;