ossl_quic_tx_packetiser_generate(): Always report if packets were sent

Even in case of later failure we need to flush
the previous packets.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21700)

diff --git a/include/internal/quic_txp.h b/include/internal/quic_txp.h
index 5ea464ddd87cd76322ec60fa1ba51d666307f470..09d552ef04362a175340598034f47b9f59cb888b 100644 (file)
--- a/include/internal/quic_txp.h
+++ b/include/internal/quic_txp.h
@@ -81,20 +81,17 @@ void ossl_quic_tx_packetiser_record_received_closing_bytes(
  * generate any frames, and generating a datagram which coalesces packets for
  * any ELs which do.
  *
- * Returns TX_PACKETISER_RES_FAILURE on failure (e.g. allocation error),
- * TX_PACKETISER_RES_NO_PKT if no packets were sent (e.g. because nothing wants
- * to send anything), and TX_PACKETISER_RES_SENT_PKT if packets were sent.
+ * Returns 0 on failure (e.g. allocation error or other errors), 1 otherwise.
  *
- * *status is filled with status information about the generated packet, if any.
+ * *status is filled with status information about the generated packet.
+ * It is always filled even in case of failure. In particular, packets can be
+ * sent even if failure is later returned.
  * See QUIC_TXP_STATUS for details.
  */
-#define TX_PACKETISER_RES_FAILURE   0
-#define TX_PACKETISER_RES_NO_PKT    1
-#define TX_PACKETISER_RES_SENT_PKT  2
-
 typedef struct quic_txp_status_st {
     int sent_ack_eliciting; /* Was an ACK-eliciting packet sent? */
     int sent_handshake; /* Was a Handshake packet sent? */
+    size_t sent_pkt; /* Number of packets sent (0 if nothing was sent) */
 } QUIC_TXP_STATUS;
 
 int ossl_quic_tx_packetiser_generate(OSSL_QUIC_TX_PACKETISER *txp,

diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c
index 2163990ae78ccc36c5839fb33ea1575e3ca8b938..9fca47bb3151699237bce3d71afc8e1f37ade2a1 100644 (file)
--- a/ssl/quic/quic_channel.c
+++ b/ssl/quic/quic_channel.c
@@ -2372,6 +2372,7 @@ undesirable:
 static int ch_tx(QUIC_CHANNEL *ch)
 {
     QUIC_TXP_STATUS status;
+    int res;
 
     /*
      * RFC 9000 s. 10.2.2: Draining Connection State:
@@ -2412,8 +2413,8 @@ static int ch_tx(QUIC_CHANNEL *ch)
      * Best effort. In particular if TXP fails for some reason we should still
      * flush any queued packets which we already generated.
      */
-    switch (ossl_quic_tx_packetiser_generate(ch->txp, &status)) {
-    case TX_PACKETISER_RES_SENT_PKT:
+    res = ossl_quic_tx_packetiser_generate(ch->txp, &status);
+    if (status.sent_pkt > 0) {
         ch->have_sent_any_pkt = 1; /* Packet was sent */
 
         /*
@@ -2437,13 +2438,12 @@ static int ch_tx(QUIC_CHANNEL *ch)
             ch->rxku_pending_confirm = 0;
 
         ch_update_ping_deadline(ch);
-        break;
-
-    case TX_PACKETISER_RES_NO_PKT:
-        break; /* No packet was sent */
+    }
 
-    default:
+    if (!res) {
         /*
+         * Internal failure (e.g.  allocation, assertion).
+         *
          * One case where TXP can fail is if we reach a TX PN of 2**62 - 1. As
          * per RFC 9000 s. 12.3, if this happens we MUST close the connection
          * without sending a CONNECTION_CLOSE frame. This is actually handled as
@@ -2456,7 +2456,6 @@ static int ch_tx(QUIC_CHANNEL *ch)
          */
         ossl_quic_channel_raise_protocol_error(ch, QUIC_ERR_INTERNAL_ERROR, 0,
                                                "internal error (txp generate)");
-        break; /* Internal failure (e.g.  allocation, assertion) */
     }
 
     /* Flush packets to network. */

diff --git a/ssl/quic/quic_txp.c b/ssl/quic/quic_txp.c
index f0c6d96fff920c429a817467e0ce634e80d32ddc..54b691c9bab08efddeca180f7060ef264ae13f51 100644 (file)
--- a/ssl/quic/quic_txp.c
+++ b/ssl/quic/quic_txp.c
@@ -702,7 +702,7 @@ int ossl_quic_tx_packetiser_generate(OSSL_QUIC_TX_PACKETISER *txp,
      *      sent to the FIFM.
      *
      */
-    int res = TX_PACKETISER_RES_FAILURE, rc;
+    int res = 0, rc;
     uint32_t archetype, enc_level;
     uint32_t conn_close_enc_level = QUIC_ENC_LEVEL_NUM;
     struct txp_pkt pkt[QUIC_ENC_LEVEL_NUM];
@@ -715,6 +715,8 @@ int ossl_quic_tx_packetiser_generate(OSSL_QUIC_TX_PACKETISER *txp,
          ++enc_level)
         pkt[enc_level].h_valid = 0;
 
+    memset(status, 0, sizeof(*status));
+
     /*
      * Should not be needed, but a sanity check in case anyone else has been
      * using the QTX.
@@ -798,8 +800,6 @@ int ossl_quic_tx_packetiser_generate(OSSL_QUIC_TX_PACKETISER *txp,
     }
 
     /* 4. Commit */
-    memset(status, 0, sizeof(*status));
-
     for (enc_level = QUIC_ENC_LEVEL_INITIAL;
          enc_level < QUIC_ENC_LEVEL_NUM;
          ++enc_level) {
@@ -833,7 +833,7 @@ int ossl_quic_tx_packetiser_generate(OSSL_QUIC_TX_PACKETISER *txp,
            && pkt[QUIC_ENC_LEVEL_HANDSHAKE].h.bytes_appended > 0);
 
     /* Flush & Cleanup */
-    res = pkts_done > 0 ? TX_PACKETISER_RES_SENT_PKT : TX_PACKETISER_RES_NO_PKT;
+    res = 1;
 out:
     ossl_qtx_finish_dgram(txp->args.qtx);
 
@@ -842,6 +842,8 @@ out:
          ++enc_level)
         txp_pkt_cleanup(&pkt[enc_level], txp);
 
+    status->sent_pkt = pkts_done;
+
     return res;
 }
 

diff --git a/test/quic_txp_test.c b/test/quic_txp_test.c
index e6acd5af80287e0ad803f554b349cce5f31c63bd..7c3e84b41a5e5888781df4ef0b9bd703fae625de 100644 (file)
--- a/test/quic_txp_test.c
+++ b/test/quic_txp_test.c
@@ -1244,16 +1244,16 @@ static int run_script(int script_idx, const struct script_op *script)
     for (op = script, opn = 0; op->opcode != OPK_END; ++op, ++opn) {
         switch (op->opcode) {
         case OPK_TXP_GENERATE:
-            if (!TEST_int_eq(ossl_quic_tx_packetiser_generate(h.txp, &status),
-                             TX_PACKETISER_RES_SENT_PKT))
+            if (!TEST_true(ossl_quic_tx_packetiser_generate(h.txp, &status))
+                && !TEST_size_t_gt(status.sent_pkt, 0))
                 goto err;
 
             ossl_qtx_finish_dgram(h.args.qtx);
             ossl_qtx_flush_net(h.args.qtx);
             break;
         case OPK_TXP_GENERATE_NONE:
-            if (!TEST_int_eq(ossl_quic_tx_packetiser_generate(h.txp, &status),
-                             TX_PACKETISER_RES_NO_PKT))
+            if (!TEST_true(ossl_quic_tx_packetiser_generate(h.txp, &status))
+                && !TEST_size_t_eq(status.sent_pkt, 0))
                 goto err;
 
             break;