BUG/MINOR: quic: fix padding issue on INITIAL retransmit

On loss detection timer expiration, qc_dgrams_retransmit() is used to
reemit lost packets. Different code paths are present depending on the
active encryption level.

If Initial level is still initialized, retransmit is performed both for
Initial and Handshake spaces, by first retrieving the list of lost
frames for each of them.

Prior to this patch, Handshake level was always registered for emission
after Initial, even if it dit not have any frame to reemit. In this
case, most of the time it would result in a datagram containing Initial
reemitted frames packet coalesced with a Handshake packet consisting
only of a PADDING frame. This is because padding is only added for the
last registered QEL.

For QUIC backend support, this may cause issues. This is because
contrary to QUIC server side, Initial and Handshake levels keys are not
derived simultaneously for a QUIC client. Thus, if the latter keys are
unavailable, Handshake packet cannot be encoded in sending, leaving a
single Initial packet. However, this is now too late to add PADDING.
Thus the resulting datagram is invalid : this triggers the BUG_ON()
assert failure located on qc_txb_store().

This patch fixes this by amending qc_dgrams_retransmit(). Now, Handshake
level is only registered for emission if there is frame to retransmit,
which implies that Handshake keys are already available. Thus, PADDING
will now either be added at Initial or Handshake level as expected.

Note that this issue should not be present on QUIC frontend, due to
Initial and Handshake keys derivation almost simultaneously. However,
this should still be backported up to 3.0.

diff --git a/src/quic_tx.c b/src/quic_tx.c
index dd6f7f714f138a94e5b6823b7634957d4f11c89a..2fd149065c9eca8d12baaf760bb34aca4ef05ff3 100644 (file)
--- a/src/quic_tx.c
+++ b/src/quic_tx.c
@@ -1008,15 +1008,15 @@ int qc_dgrams_retransmit(struct quic_conn *qc)
 
                        qc_prep_hdshk_fast_retrans(qc, &ifrms, &hfrms);
                        TRACE_DEVEL("Avail. ack eliciting frames", QUIC_EV_CONN_FRMLIST, qc, &ifrms);
-                       TRACE_DEVEL("Avail. ack eliciting frames", QUIC_EV_CONN_FRMLIST, qc, &hfrms);
                        if (!LIST_ISEMPTY(&ifrms)) {
                                ipktns->tx.pto_probe = 1;
-                               if (!LIST_ISEMPTY(&hfrms))
-                                       hpktns->tx.pto_probe = 1;
-
                                qel_register_send(&send_list, qc->iel, &ifrms);
-                               if (qc->hel)
+
+                               if (!LIST_ISEMPTY(&hfrms)) {
+                                       TRACE_DEVEL("Avail. ack eliciting frames", QUIC_EV_CONN_FRMLIST, qc, &hfrms);
+                                       hpktns->tx.pto_probe = 1;
                                        qel_register_send(&send_list, qc->hel, &hfrms);
+                               }
 
                                sret = qc_send(qc, 1, &send_list, 0);
                                qc_free_frm_list(qc, &ifrms);