BUG/MEDIUM: quic: fix ACK ECN frame parsing

ACK frames are either of type 0x02 or 0x03. The latter is an indication
that it contains extra ECN related fields. In haproxy QUIC stack, this
is considered as a different frame type, set to QUIC_FT_ACK_ECN, with
its own set of builder/parser functions.

This patch fixes ACK ECN parsing function. Indeed, the latter suffered
from two issues. First, 'first ACK range' and 'ACK ranges' were
inverted. Then, the three remaining ECN fields were simply ignored by
the parsing function.

This issue can cause desynchronization in the frames parsing code, which
may result in various result. Most of the time, the connection will be
aborted by haproxy due to an invalid frame content read.

Note that this issue was not detected earlier as most clients do not
enable ECN support if the peer is not able to emit ACK ECN frame first,
which haproxy currently never sends. Nevertheless, this is not the case
for every client implementation, thus proper ACK ECN parsing is
mandatory for a proper QUIC stack support.

Fix this by adjusting quic_parse_ack_ecn_frame() function. The remaining
ECN fields are parsed to ensure correct packet parsing. Currently, they
are not used by the congestion controller.

This must be backported up to 2.6.

diff --git a/src/quic_frame.c b/src/quic_frame.c
index 0e9c3cc3d2265d998dd8b073d6bcfe749940bc49..499f3f69e786bd711410d53e30e16cf8a3d788e1 100644 (file)
--- a/src/quic_frame.c
+++ b/src/quic_frame.c
@@ -363,11 +363,16 @@ static int quic_parse_ack_ecn_frame(struct quic_frame *frm, struct quic_conn *qc
                                     const unsigned char **pos, const unsigned char *end)
 {
        struct qf_ack *ack_frm = &frm->ack;
+       /* TODO implement ECN advertising */
+       uint64_t ect0, ect1, ecn_ce;
 
        return quic_dec_int(&ack_frm->largest_ack, pos, end) &&
-               quic_dec_int(&ack_frm->ack_delay, pos, end) &&
-               quic_dec_int(&ack_frm->first_ack_range, pos, end) &&
-               quic_dec_int(&ack_frm->ack_range_num, pos, end);
+              quic_dec_int(&ack_frm->ack_delay, pos, end) &&
+              quic_dec_int(&ack_frm->ack_range_num, pos, end) &&
+              quic_dec_int(&ack_frm->first_ack_range, pos, end) &&
+              quic_dec_int(&ect0, pos, end) &&
+              quic_dec_int(&ect1, pos, end) &&
+              quic_dec_int(&ecn_ce, pos, end);
 }
 
 /* Encode a RESET_STREAM frame at <pos> buffer position.