]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MEDIUM: quic: prevent crash on Retry sending
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Wed, 19 Apr 2023 08:04:41 +0000 (10:04 +0200)
committerAmaury Denoyelle <adenoyelle@haproxy.com>
Wed, 19 Apr 2023 08:18:58 +0000 (10:18 +0200)
The following commit introduced a regression :
  commit 1a5cc19cecfa84bfd0fdd7b9d5d20899cce40662
  MINOR: quic: adjust Rx packet type parsing

Since this commit, qv variable was left to NULL as version is stored
directly in quic_rx_packet instance. In most cases, this only causes
traces to skip version printing. However, qv is dereferenced when
sending a Retry which causes a segfault.

To fix this, simply remove qv variable and use pkt->version instead,
both for traces and send_retry() invocation.

This bug was detected thanks to QUIC interop runner. It can easily be
reproduced by using quic-force-retry on the bind line.

This must be backported up to 2.7.

src/quic_conn.c

index b6f1467753424bed620151a6a52b583a36a6ac76..02e4044a49fc054d69ec29307fda2efed07b3fba 100644 (file)
@@ -6867,7 +6867,6 @@ static int quic_rx_pkt_parse(struct quic_rx_packet *pkt,
        const unsigned char *beg = buf;
        struct proxy *prx;
        struct quic_counters *prx_counters;
-       const struct quic_version *qv = NULL;
 
        TRACE_ENTER(QUIC_EV_CONN_LPKT);
 
@@ -6952,7 +6951,7 @@ static int quic_rx_pkt_parse(struct quic_rx_packet *pkt,
                        if (!quic_dec_int(&token_len, (const unsigned char **)&buf, end) ||
                                end - buf < token_len) {
                                TRACE_PROTO("Packet dropped",
-                                           QUIC_EV_CONN_LPKT, NULL, NULL, NULL, qv);
+                                           QUIC_EV_CONN_LPKT, NULL, NULL, NULL, pkt->version);
                                goto drop;
                        }
 
@@ -6962,10 +6961,10 @@ static int quic_rx_pkt_parse(struct quic_rx_packet *pkt,
                        if (global.cluster_secret && !token_len) {
                                if (l->bind_conf->options & BC_O_QUIC_FORCE_RETRY) {
                                        TRACE_PROTO("Initial without token, sending retry",
-                                                   QUIC_EV_CONN_LPKT, NULL, NULL, NULL, qv);
-                                       if (send_retry(l->rx.fd, &dgram->saddr, pkt, qv)) {
+                                                   QUIC_EV_CONN_LPKT, NULL, NULL, NULL, pkt->version);
+                                       if (send_retry(l->rx.fd, &dgram->saddr, pkt, pkt->version)) {
                                                TRACE_PROTO("Error during Retry generation",
-                                                           QUIC_EV_CONN_LPKT, NULL, NULL, NULL, qv);
+                                                           QUIC_EV_CONN_LPKT, NULL, NULL, NULL, pkt->version);
                                                goto drop_silent;
                                        }
 
@@ -6978,7 +6977,7 @@ static int quic_rx_pkt_parse(struct quic_rx_packet *pkt,
                                 * cluster secret.
                                 */
                                TRACE_PROTO("Packet dropped", QUIC_EV_CONN_LPKT,
-                                           NULL, NULL, NULL, qv);
+                                           NULL, NULL, NULL, pkt->version);
                                goto drop;
                        }
 
@@ -6989,7 +6988,7 @@ static int quic_rx_pkt_parse(struct quic_rx_packet *pkt,
                else if (pkt->type != QUIC_PACKET_TYPE_0RTT) {
                        if (pkt->dcid.len != QUIC_HAP_CID_LEN) {
                                TRACE_PROTO("Packet dropped",
-                                           QUIC_EV_CONN_LPKT, NULL, NULL, NULL, qv);
+                                           QUIC_EV_CONN_LPKT, NULL, NULL, NULL, pkt->version);
                                goto drop;
                        }
                }
@@ -6997,7 +6996,7 @@ static int quic_rx_pkt_parse(struct quic_rx_packet *pkt,
                if (!quic_dec_int(&len, (const unsigned char **)&buf, end) ||
                        end - buf < len) {
                        TRACE_PROTO("Packet dropped",
-                                   QUIC_EV_CONN_LPKT, NULL, NULL, NULL, qv);
+                                   QUIC_EV_CONN_LPKT, NULL, NULL, NULL, pkt->version);
                        goto drop;
                }
 
@@ -7056,7 +7055,7 @@ static int quic_rx_pkt_parse(struct quic_rx_packet *pkt,
                pkt->len = end - beg;
        }
 
-       TRACE_PROTO("RX pkt parsed", QUIC_EV_CONN_LPKT, NULL, pkt, NULL, qv);
+       TRACE_PROTO("RX pkt parsed", QUIC_EV_CONN_LPKT, NULL, pkt, NULL, pkt->version);
        TRACE_LEAVE(QUIC_EV_CONN_LPKT);
        return 0;
 
@@ -7065,7 +7064,7 @@ static int quic_rx_pkt_parse(struct quic_rx_packet *pkt,
  drop_silent:
        if (!pkt->len)
                pkt->len = end - beg;
-       TRACE_PROTO("RX pkt parsing failed", QUIC_EV_CONN_LPKT, NULL, pkt, NULL, qv);
+       TRACE_PROTO("RX pkt parsing failed", QUIC_EV_CONN_LPKT, NULL, pkt, NULL, pkt->version);
        TRACE_LEAVE(QUIC_EV_CONN_LPKT);
        return -1;
 }