mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-09-24 07:11:20 +02:00
Code Issues Projects Releases Wiki Activity

BUG/MINOR: quic: fix malformed probing packet building

Browse Source 
This bug arrived with this commit:

   cdfceb10a MINOR: quic: refactor qc_prep_pkts() loop

which prevents haproxy from sending PING only packets/datagrams (some
packets/datagrams with only PING frame as ack-eliciting frames inside).
Such packets/datagrams are useful in rare cases during retransmissions
when one wants to probe the peer without exceeding the anti-amplification
limit.

Modify the condition passed to qc_build_pkt() to add padding to the current
datagram. One does not want to do that when probing the peer without ack-eliciting
frames passed as <frms> parameter. Indeed qc_build_pkt() calls qc_do_build_pkt()
which supports this case: if <probe> is true (probing required), qc_do_build_pkt()
handles the case where some padding must be added to a PING only packet/datagram.
This is the case when probing with an empty <frms> frame list of ack-eliciting
frames without exceeding the anti-amplification limit from qc_dgrams_retransmit().

Add some comments to qc_build_pkt() and qc_do_build_pkt() to clarify this
as this code is easy to break!

Thank you for @Tristan971 for having reported this issue in GH #2709.

Must be backported to 3.0.
This commit is contained in:
Frederic Lecaille 2024-11-04 18:50:10 +01:00
parent 444a19ea38
commit 217e467e89
1 changed files with 27 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
src/quic_tx.c
View File

@ -623,14 +623,24 @@ static int qc_prep_pkts(struct quic_conn *qc, struct buffer *buf,
break;
}
/* padding will be set for last QEL */
/* padding will be set for last QEL, except when probing:
* to build a PING only non coalesced Initial datagram for
* instance when blocked by the anti-amplification limit,
* this datagram MUST be padded.
*/
padding = 1;
}
pkt_type = quic_enc_level_pkt_type(qc, qel);
/* <paddding> parameter for qc_build_pkt() must not be set to 1 when
* building PING only Initial datagram (a datagram with an Initial
* packet inside containing only a PING frame as ack-eliciting
* frame). This is the case when both <probe> and LIST_EMPTY(<frms>)
* conditions are verified (see qc_do_build_pkt()).
*/
cur_pkt = qc_build_pkt(&pos, end, qel, tls_ctx, frms,
qc, ver, dglen, pkt_type,
must_ack, padding && !next_qel,
qc, ver, dglen, pkt_type, must_ack,
padding && !next_qel && (!probe || !LIST_ISEMPTY(frms)),
probe, cc, &err);
if (!cur_pkt) {
switch (err) {
@ -1812,6 +1822,20 @@ static inline uint64_t quic_compute_ack_delay_us(unsigned int time_received,
* number field in this packet. <pn_len> will also have the packet number
* length as value.
*
* NOTE: This function does not build all the possible combinations of packets
* depending on its list of parameters. In most cases, <frms> frame list is
* not empty. So, this function first tries to build this list of frames.
* Then some padding is added to this packet if <padding> boolean is set true.
* The unique case one wants to do that is when a first Initial packet was
* previously built into the same datagram as the currently built one and when
* this packet is supposed to pad the datagram, if needed, to build an at
* least 1200 bytes long Initial datagram.
* If <padding> is not true, if the packet is too short, the packet is also
* padded. This is very often the case when no frames are provided by <frms>
* and when probing with only a PING frame.
* Finally, if <frms> was empty, if <probe> boolean is true this function builds
* a PING only packet handling also the cases where it must be padded.
*
* Return 1 if succeeded (enough room to buile this packet), O if not.
*/
static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end,