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)
This commit is contained in:
Tomas Mraz 2023-08-18 17:08:18 +02:00
parent 96014840b6
commit 64fd69911e
4 changed files with 22 additions and 24 deletions

View File

@ -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,

View File

@ -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. */

View File

@ -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;
}

View File

@ -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;