BUG/MAJOR: quic: fix padding with short packets

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 :

  a60609f
  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 :

  cdfceb1
  MINOR: quic: refactor qc_prep_pkts() loop

This must be backported to 3.0.

This should fix github issue #2609.

(cherry picked from commit c714b6b)
Signed-off-by: Christopher Faulet <[email protected]>

Author: a-denoyelle

src/quic_tx.c

@@ -1889,20 +1889,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;