BUG/MINOR: quic: Wrong redispatch for external data on connection socket

It is possible to receive datagram from other connection on a dedicated
quic-conn socket. This is due to a race condition between bind() and
connect() system calls.

To handle this, an explicit check is done on each datagram. If the DCID
is not associated to the connection which owns the socket, the datagram
is redispatch as if it arrived on the listener socket.

This redispatch step was not properly done because the source address
specified for the redispatch function was incorrect. Instead of using
the datagram source address, we used the address of the socket
quic-conn which received the datagram due to the above race condition.
Fix this simply by using the address from the recvmsg() system call.

The impact of this bug is minor as redispatch on connection socket
should be really rare. However, when it happens it can lead to several
kinds of problems, like for example a connection initialized with an
incorrect peer address. It can also break the Retry token check as this
relies on the peer address.

In fact, Retry token check failure was the reason this bug was found.
When using h2load with thousands of clients, the counter of Retry token
failure was unusually high. With this patch, no failure is reported
anymore for Retry.

Must be backported to 2.7.
This commit is contained in:
Frédéric Lécaille 2023-05-11 20:43:28 +02:00 committed by Amaury Denoyelle
parent 256d581fbd
commit 76d502588d

View File

@ -754,7 +754,7 @@ int qc_rcv_buf(struct quic_conn *qc)
rxbuf_tail = (unsigned char *)b_tail(&rxbuf->buf);
__b_putblk(&rxbuf->buf, (char *)dgram_buf, new_dgram->len);
if (!quic_lstnr_dgram_dispatch(rxbuf_tail, ret, l, &qc->peer_addr, &daddr,
if (!quic_lstnr_dgram_dispatch(rxbuf_tail, ret, l, &saddr, &daddr,
new_dgram, &rxbuf->dgram_list)) {
/* TODO count lost datagrams. */
b_sub(&buf, ret);