BUG/MEDIUM: quic: fix unsuccessful handshakes on ncb_advance error
QUIC handshakes were frequently in error due to haproxy misuse of ncbuf. This resulted in one of the following scenario : - handshake rejected with CONNECTION_CLOSE due to overlapping data rejected - CRYPTO data fully received by haproxy but handshake completion signal not reported causing the client to emit PING repeatedly before timeout This was produced because ncb_advance() result was not checked after providing crypto data to the SSL stack in qc_provide_cdata(). However, this operation can fail if a too small gap is formed. In the meantime, quic_enc_level offset was always incremented. In some cases, this caused next ncb_add() to report rejected overlapping data. In other cases, no error was reported but SSL stack never received the end of CRYPTO data. Change slightly the handling of new CRYPTO frames to avoid this bug : when receiving a CRYPTO frame for the current offset, handle them directly as previously done only if quic_enc_level ncbuf is empty. In the other case, copy them to the buffer before treating contiguous data via qc_treat_rx_crypto_frms(). This change ensures that ncb_advance() operation is now conducted only in a data block : thus this is guaranteed to never fail. This bug was easily reproduced with chromium as it fragments CRYPTO frames randomly in several frames out of order. This commit has two drawbacks : - it is slightly less worst on performance because as sometimes even data at the current offset will be memcpy - if a client uses too many fragmented CRYPTO frames, this can cause repeated ncb_add() error on gap size. This can be reproduced with chrome, albeit with a slighly less frequent rate than the fixed issue. This change should fix in part github issue #1903. This must be backported up to 2.6.
This commit is contained in:
parent
7f0295f08a
commit
ff95f2d447
@ -2682,7 +2682,7 @@ static int qc_handle_crypto_frm(struct quic_conn *qc,
|
||||
frm->offset = cstream->rx.offset;
|
||||
}
|
||||
|
||||
if (frm->offset == cstream->rx.offset) {
|
||||
if (frm->offset == cstream->rx.offset && ncb_is_empty(ncbuf)) {
|
||||
if (!qc_provide_cdata(qel, qc->xprt_ctx, frm->data, frm->len,
|
||||
pkt, &cfdebug)) {
|
||||
// trace already emitted by function above
|
||||
|
Loading…
Reference in New Issue
Block a user