QUIC TXP: Fix use of implicit-length STREAM frames in presence of PAT…

…H_REPSONSE frames Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from #22615) (cherry picked from commit 3bef14c)
openssl · Nov 8, 2023 · ab3b836 · ab3b836
1 parent d025b22
commit ab3b836
Showing 1 changed file with 14 additions and 26 deletions.
diff --git a/ssl/quic/quic_txp.c b/ssl/quic/quic_txp.c
@@ -846,8 +846,17 @@ int ossl_quic_tx_packetiser_generate(OSSL_QUIC_TX_PACKETISER *txp,
             && total_dgram_size < min_dpl) {
             size_t deficit = min_dpl - total_dgram_size;
 
+            if (!ossl_assert(!pkt[first_el].h.done_implicit))
+                goto out;
+
             if (!txp_pkt_append_padding(&pkt[first_el], txp, deficit))
                 goto out;
+
+            /*
+             * Padding frames make a packet ineligible for being a non-inflight
+             * packet.
+             */
+            pkt[first_el].tpkt->ackm_pkt.is_inflight = 1;
         }
     }
 
@@ -2137,7 +2146,6 @@ static int txp_generate_stream_frames(OSSL_QUIC_TX_PACKETISER *txp,
                                       QUIC_SSTREAM *sstream,
                                       QUIC_TXFC *stream_txfc,
                                       QUIC_STREAM *next_stream,
-                                      size_t min_ppl,
                                       int *have_ack_eliciting,
                                       int *packet_full,
                                       uint64_t *new_credit_consumed)
@@ -2151,7 +2159,7 @@ static int txp_generate_stream_frames(OSSL_QUIC_TX_PACKETISER *txp,
     WPACKET *wpkt;
     QUIC_TXPIM_CHUNK chunk;
     size_t i, j, space_left;
-    int needs_padding_if_implicit, can_fill_payload, use_explicit_len;
+    int can_fill_payload, use_explicit_len;
     int could_have_following_chunk;
     uint64_t orig_len;
     uint64_t hdr_len_implicit, payload_len_implicit;
@@ -2222,13 +2230,6 @@ static int txp_generate_stream_frames(OSSL_QUIC_TX_PACKETISER *txp,
             goto err; /* can't fit anything */
         }
 
-        /*
-         * If using the implicit-length representation would need padding, we
-         * can't use it.
-         */
-        needs_padding_if_implicit = (h->bytes_appended + hdr_len_implicit
-                                     + payload_len_implicit < min_ppl);
-
         /*
          * If there is a next stream, we don't use the implicit length so we can
          * add more STREAM frames after this one, unless there is enough data
@@ -2246,7 +2247,7 @@ static int txp_generate_stream_frames(OSSL_QUIC_TX_PACKETISER *txp,
 
         /* Choose between explicit or implicit length representations. */
         use_explicit_len = !((can_fill_payload || !could_have_following_chunk)
-                             && !needs_padding_if_implicit);
+                             && !pkt->force_pad);
 
         if (use_explicit_len) {
             /*
@@ -2354,7 +2355,6 @@ static void txp_enlink_tmp(QUIC_STREAM **tmp_head, QUIC_STREAM *stream)
 
 static int txp_generate_stream_related(OSSL_QUIC_TX_PACKETISER *txp,
                                        struct txp_pkt *pkt,
-                                       size_t min_ppl,
                                        int *have_ack_eliciting,
                                        QUIC_STREAM **tmp_head)
 {
@@ -2495,7 +2495,7 @@ static int txp_generate_stream_related(OSSL_QUIC_TX_PACKETISER *txp,
             if (!txp_generate_stream_frames(txp, pkt,
                                             stream->id, stream->sstream,
                                             &stream->txfc,
-                                            snext, min_ppl,
+                                            snext,
                                             have_ack_eliciting,
                                             &packet_full,
                                             &stream->txp_txfc_new_credit_consumed)) {
@@ -2537,7 +2537,6 @@ static int txp_generate_for_el(OSSL_QUIC_TX_PACKETISER *txp,
     QUIC_CFQ_ITEM *cfq_item;
     QUIC_TXPIM_PKT *tpkt = NULL;
     struct tx_helper *h = &pkt->h;
-    size_t min_ppl = 0;
 
     /* Maximum PN reached? */
     if (!ossl_quic_pn_valid(txp->next_pn[pn_space]))
@@ -2740,7 +2739,7 @@ static int txp_generate_for_el(OSSL_QUIC_TX_PACKETISER *txp,
 
     /* Stream-specific frames */
     if (a.allow_stream_rel && txp->handshake_complete)
-        if (!txp_generate_stream_related(txp, pkt, min_ppl,
+        if (!txp_generate_stream_related(txp, pkt,
                                          &have_ack_eliciting,
                                          &pkt->stream_head))
             goto fatal_err;
@@ -2768,18 +2767,7 @@ static int txp_generate_for_el(OSSL_QUIC_TX_PACKETISER *txp,
         have_ack_eliciting = 1;
     }
 
-    /* PADDING */
-    if (a.allow_padding && h->bytes_appended < min_ppl) {
-        WPACKET *wpkt = tx_helper_begin(h);
-        if (wpkt == NULL)
-            goto fatal_err;
-
-        if (!ossl_quic_wire_encode_padding(wpkt, min_ppl - h->bytes_appended)
-            || !tx_helper_commit(h))
-            goto fatal_err;
-
-        can_be_non_inflight = 0;
-    }
+    /* PADDING is added by ossl_quic_tx_packetiser_generate(). */
 
     /*
      * ACKM Data