BUG/MINOR: quic: reserve length field for long header encoding

Long header packets have a mandatory Length field, which contains the
size of Packet number and payload, encoded as a variable-length integer.
Its value can thus only be determined after the payload size is known,
which depends on the remaining buffer space after this variable-length
field.

Packet payload are encoded in two steps. First, a list of input frames
is processed until the packet buffer is full. CRYPTO and STREAM frames
payload can be splitted if need to fill the buffer. Real encoding is
then performed as a second stage operation, first with Length field,
then with the selected frames themselves.

Before this patch, no space was reserved in the buffer for Length field
when attaching the frames to the packet. This could result in a error as
the packet payload would be too large for the remaining space.

In practice, this issue was rarely encounted, mostly as a side-effect
from another issue linked to CRYPTO frame encoding. Indeed, a wrong
calculation is performed on CRYPTO splitting, which results in frame
payload shorter by a few bytes than expected. This however ensured there
would be always enough room for the Length field and payload during
encoding. As CRYPTO frames are the only big enough content emitted with
a Long header packet, this renders the current issue mostly non
reproducible.

Fix the original issue by reserving some space for Length field prior to
frame payload calculation, using a maximum value based on the remaining
room space. Packet length is then reduced if needed when encoding is
performed, which ensures there is always enough room for the selected
frames.

Note that the other issue impacting CRYPTO frame encoding is not yet
fixed. This could result in datagrams with Long header packets not
completely extended to the full MTU. The issue will be addressed in
another patch.

This should be backported up to 2.6.

diff --git a/src/quic_tx.c b/src/quic_tx.c
index d283ec9be0afc2e1e7cfba3266e5b5b5a3973ac2..82b3f5fa044376d6d2d8d0560b4fa4ef7c60b40b 100644 (file)
--- a/src/quic_tx.c
+++ b/src/quic_tx.c
@@ -1908,7 +1908,7 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end,
                            const struct quic_version *ver, struct list *frms)
 {
        unsigned char *beg, *payload;
-       size_t len, len_sz, len_frms, padding_len;
+       size_t len, len_sz = 0, len_frms, padding_len;
        struct quic_frame frm;
        struct quic_frame ack_frm;
        struct quic_frame cc_frm;
@@ -1963,6 +1963,17 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end,
        }
 
        head_len = pos - beg;
+
+       if (pkt->type != QUIC_PACKET_TYPE_SHORT) {
+               /* Reserve enough bytes for packet length. Real value will be
+                * recalculated later after payload length is determined.
+                */
+               len_sz = quic_int_getsize(end - pos);
+               if (end - pos <= len_sz)
+                       goto no_room;
+               pos += len_sz;
+       }
+
        /* Build an ACK frame if required. */
        ack_frm_len = 0;
        /* Do not ack and probe at the same time. */
@@ -2050,11 +2061,15 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end,
        }
        add_ping_frm = 0;
        padding_len = 0;
-       len_sz = quic_int_getsize(len);
        /* Add this packet size to <dglen> */
        dglen += head_len + len;
-       if (pkt->type != QUIC_PACKET_TYPE_SHORT)
+
+       if (pkt->type != QUIC_PACKET_TYPE_SHORT) {
+               /* Remove reserved space for packet length. */
+               pos -= len_sz;
+               len_sz = quic_int_getsize(len);
                dglen += len_sz;
+       }
 
        if (padding && dglen < QUIC_INITIAL_PACKET_MINLEN) {
                padding_len = QUIC_INITIAL_PACKET_MINLEN - dglen;