mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-08-06 15:17:01 +02:00
Code Issues Projects Releases Wiki Activity

BUG/MINOR: quic: Wrong datagram building when probing.

Browse Source 
This issue was revealed by chacha20 interop test which very often fails with
ngtcp2 as client. This was due to the fact that 2 application level packets could
be coalesced into the same datagram as revealed by such a capture:

Frame 380: 255 bytes on wire (2040 bits), 255 bytes captured (2040 bits)
Point-to-Point Protocol
Internet Protocol Version 4, Src: 193.167.100.100, Dst: 193.167.0.100
User Datagram Protocol
QUIC IETF
    QUIC Connection information
        [Connection Number: 0]
    [Packet Length: 187]
    QUIC Short Header DCID=ec523fe99840f9c17c868a88d649147814 PKN=333
        0... .... = Header Form: Short Header (0)
        .1.. .... = Fixed Bit: True
        ..0. .... = Spin Bit: False
        [...0 0... = Reserved: 0]
        [.... .0.. = Key Phase Bit: False]
        [.... ..00 = Packet Number Length: 1 bytes (0)]
        Destination Connection ID: ec523fe99840f9c17c868a88d649147814
        [Packet Number: 333]
        Protected Payload […]: 43537d43a3c83e47db6891bd6a4fd7d7fa31941badcb87a540e843341d6a5e493ed4c3f6e6bbff094804ee0ab06830dc1a1bbf52ace4323d2e4f6e0bd4eea73df0721d2949d05a058d3afb974e814494ebf44d1375b0e7f1fd5bcf634cf32ef9a9b4018758a49d39a24c40
    STREAM id=0 fin=0 off=294768 len=144 dir=Bidirectional origin=Client-initiated
        Frame Type: STREAM (0x000000000000000e)
            .... ...0 = Fin: False
            .... ..1. = Len(gth): True
            .... .1.. = Off(set): True
        Stream ID: 0
            .... .... .... .... .... .... .... .... .... .... .... .... .... .... .... ...0 = Stream initiator: Client-initiated (0)
            .... .... .... .... .... .... .... .... .... .... .... .... .... .... .... ..0. = Stream direction: Bidirectional (0)
        Offset: 294768
        Length: 144
        Stream Data […]: 63eef6ccee0d2ab602db3682d0e7cc09b72db6adc307d7699a211144b4b6c029cbed9beae1491c10a5fe0678d815a5303843d33c0593fedc9b64068fd0207e280d05aac2c0054fe9ab30857bc3669ee51d34756cfd2e098eb1ab31a03911f6a103f0a16f8f984d9861efdcf4433c
QUIC IETF
    [Packet Length: 38]
    QUIC Short Header DCID=ec523fe99840f9c17c868a88d649147814 PKN=334
        0... .... = Header Form: Short Header (0)
        .1.. .... = Fixed Bit: True
        ..0. .... = Spin Bit: False
        [...0 0... = Reserved: 0]
        [.... .0.. = Key Phase Bit: False]
        [.... ..00 = Packet Number Length: 1 bytes (0)]
        Destination Connection ID: ec523fe99840f9c17c868a88d649147814
        [Packet Number: 334]
        Protected Payload: b9c0e6dc3fc523574f8164c31b6cd156496212
    PING
        Frame Type: PING (0x0000000000000001)
    PADDING Length: 2
        Frame Type: PADDING (0x0000000000000000)
        [Padding Length: 2]

On the peer side these two packet are considered as a unique one
because there may be only one packet by datagram at application encryption
level and reported as a STREAM frame encoding error:

I00000332 0xec523fe99840f9c17c868a88d649147814 con recv packet len=225
mask=b2c69c7827 sample=43a3c83e47db6891bd6a4fd7d7fa3194
I00000332 0xec523fe99840f9c17c868a88d649147814 pkt rx pkn=333 dcid=0xec523fe99840f9c17c868a88d649147814 type=1RTT k=0
I00000332 0xec523fe99840f9c17c868a88d649147814 frm rx 333 1RTT STREAM(0x0e) id=0x0 fin=0 offset=294768 len=144 uni=0
ngtcp2_conn_read_pkt: ERR_FRAME_ENCODING
I00000332 0xec523fe99840f9c17c868a88d649147814 pkt tx pkn=1531039643 dcid=0xae79dfc99d6c65d6 type=1RTT k=0
I00000332 0xec523fe99840f9c17c868a88d649147814 frm tx 1531039643 1RTT CONNECTION_CLOSE(0x1c) error_code=FRAME_ENCODING_ERROR(0x7) frame_type=0 reason_len=0 reason=[]
I00000332 0xec523fe99840f9c17c868a88d649147814 frm tx 1531039643 1RTT PADDING(0x00) len=9

Note here that the sum of the two packet sizes (from capture) is the same as the
packet length reporte by ngtcp2: 187+38 = 225. It also seems that wireshark tries
to parse as much as packet into the same datagram, regardless of the QUIC protocol
rules.

Haproxy traces revealed that this could happen at least when probing the peer.
The recent low level packet building modifications aim was to build
as much as datagrams into the same buffer. But it seems that the
probing packet case treatment has been broken. That said, I have not
identified impacted commit. This issue could be reproduced inside
interop test environment (no possible git bisection).

To fix this, rely on the <probe> variable value to identify if the last
packet built by qc_prep_pkts() was a probing one, then try to
coalesce some others packet into the same datagram if this was not the case.
Of course the test on <probe> value has to be done before setting it
for the next packet.

Must be backported to 3.0.
This commit is contained in:
Frederic Lecaille 2024-06-26 16:06:53 +02:00
parent bbc2f043e3
commit 6d943b8db6
1 changed files with 9 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
src/quic_tx.c
View File

@ -611,11 +611,6 @@ static int qc_prep_pkts(struct quic_conn *qc, struct buffer *buf,
total += cur_pkt->len;
dglen += cur_pkt->len;
/* qc_do_build_pkt() is responsible to decrement probe
* value. Required to break loop on qc_may_build_pkt().
*/
probe = qel->pktns->tx.pto_probe;
/* Reset padding if datagram is big enough. */
if (dglen >= QUIC_INITIAL_PACKET_MINLEN)
padding = 0;
@ -640,6 +635,10 @@ static int qc_prep_pkts(struct quic_conn *qc, struct buffer *buf,
goto out;
}
/* Only one short packet by datagram when probing. */
if (probe && qel == qc->ael)
break;
if (LIST_ISEMPTY(frms)) {
prv_pkt = cur_pkt;
}
@ -654,6 +653,11 @@ static int qc_prep_pkts(struct quic_conn *qc, struct buffer *buf,
padding = 0;
prv_pkt = NULL;
}
/* qc_do_build_pkt() is responsible to decrement probe
* value. Required to break loop on qc_may_build_pkt().
*/
probe = qel->pktns->tx.pto_probe;
}
TRACE_DEVEL("next encryption level", QUIC_EV_CONN_PHPKTS, qc);