MEDIUM: quic: strengthen BUG_ON() for unpad Initial packet on client

To avoid anti-amplification limit, it is required that Initial packet
are padded to be at least 1.200 bytes long. On server side, this only
applies to ack-eliciting packets. However, for client side, this is
mandatory for every packets.

This patch adjusts qc_txb_store() BUG_ON statement used to catch too
small Initial packets. On QUIC client side, ack-eliciting flag is now
ignored, thus every packets are checked.

This is labelled as MEDIUM as this BUG_ON() is known to be easily
triggered, as QUIC datagrams encoding function are complex. However,
it's important that a QUIC endpoint respects it, else the peer will drop
the invalid packet and could immediately close the connection.
This commit is contained in:
Amaury Denoyelle 2025-09-02 09:22:24 +02:00
parent 209a54d539
commit 36d28bfca3

View File

@ -159,13 +159,14 @@ struct buffer *qc_get_txb(struct quic_conn *qc)
* Caller is responsible that there is enough space in the buffer. * Caller is responsible that there is enough space in the buffer.
*/ */
static void qc_txb_store(struct buffer *buf, uint16_t length, static void qc_txb_store(struct buffer *buf, uint16_t length,
struct quic_tx_packet *first_pkt) struct quic_tx_packet *first_pkt,
const struct quic_conn *qc)
{ {
BUG_ON_HOT(b_contig_space(buf) < QUIC_DGRAM_HEADLEN); /* this must not happen */ BUG_ON_HOT(b_contig_space(buf) < QUIC_DGRAM_HEADLEN); /* this must not happen */
/* If first packet is INITIAL, ensure datagram is sufficiently padded. */ /* If first packet is INITIAL, ensure datagram is sufficiently padded. */
BUG_ON(first_pkt->type == QUIC_PACKET_TYPE_INITIAL && BUG_ON(first_pkt->type == QUIC_PACKET_TYPE_INITIAL &&
(first_pkt->flags & QUIC_FL_TX_PACKET_ACK_ELICITING) && (qc_is_back(qc) || (first_pkt->flags & QUIC_FL_TX_PACKET_ACK_ELICITING)) &&
length < QUIC_INITIAL_PACKET_MINLEN); length < QUIC_INITIAL_PACKET_MINLEN);
write_u16(b_tail(buf), length); write_u16(b_tail(buf), length);
@ -743,7 +744,7 @@ static int qc_prep_pkts(struct quic_conn *qc, struct buffer *buf,
*/ */
if (first_pkt && (first_pkt->type != QUIC_PACKET_TYPE_INITIAL || if (first_pkt && (first_pkt->type != QUIC_PACKET_TYPE_INITIAL ||
wrlen >= QUIC_INITIAL_PACKET_MINLEN)) { wrlen >= QUIC_INITIAL_PACKET_MINLEN)) {
qc_txb_store(buf, wrlen, first_pkt); qc_txb_store(buf, wrlen, first_pkt, qc);
} }
TRACE_PROTO("could not prepare anymore packet", QUIC_EV_CONN_PHPKTS, qc, qel); TRACE_PROTO("could not prepare anymore packet", QUIC_EV_CONN_PHPKTS, qc, qel);
break; break;
@ -825,7 +826,7 @@ static int qc_prep_pkts(struct quic_conn *qc, struct buffer *buf,
} }
else { else {
/* Finalize current datagram if not all frames sent. */ /* Finalize current datagram if not all frames sent. */
qc_txb_store(buf, wrlen, first_pkt); qc_txb_store(buf, wrlen, first_pkt, qc);
first_pkt = NULL; first_pkt = NULL;
wrlen = dglen = 0; wrlen = dglen = 0;
padding = 0; padding = 0;
@ -844,7 +845,7 @@ static int qc_prep_pkts(struct quic_conn *qc, struct buffer *buf,
out: out:
if (first_pkt) if (first_pkt)
qc_txb_store(buf, wrlen, first_pkt); qc_txb_store(buf, wrlen, first_pkt, qc);
if (cc && total) { if (cc && total) {
BUG_ON(buf != &qc->tx.cc_buf); BUG_ON(buf != &qc->tx.cc_buf);