From: Matt Caswell Date: Thu, 17 Aug 2023 14:35:15 +0000 (+0100) Subject: Keep sending datagrams while we have data to send X-Git-Tag: openssl-3.2.0-alpha1~162 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=aa433014bb36bfff0af17c0eb9d25b6fb2d7d068;p=thirdparty%2Fopenssl.git Keep sending datagrams while we have data to send If we've got more data to send than will fit in a single datagram we should keep generating those datagrams until we've sent it all. Reviewed-by: Hugo Landau Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/21798) --- diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c index 9fca47bb315..5cee9f75320 100644 --- a/ssl/quic/quic_channel.c +++ b/ssl/quic/quic_channel.c @@ -2406,57 +2406,62 @@ static int ch_tx(QUIC_CHANNEL *ch) ch->rxku_pending_confirm_done = 0; - /* - * Send a packet, if we need to. Best effort. The TXP consults the CC and - * applies any limitations imposed by it, so we don't need to do it here. - * - * Best effort. In particular if TXP fails for some reason we should still - * flush any queued packets which we already generated. - */ - res = ossl_quic_tx_packetiser_generate(ch->txp, &status); - if (status.sent_pkt > 0) { - ch->have_sent_any_pkt = 1; /* Packet was sent */ - + /* Loop until we stop generating packets to send */ + do { /* - * RFC 9000 s. 10.1. 'An endpoint also restarts its idle timer when - * sending an ack-eliciting packet if no other ack-eliciting packets - * have been sent since last receiving and processing a packet.' - */ - if (status.sent_ack_eliciting && !ch->have_sent_ack_eliciting_since_rx) { - ch_update_idle(ch); - ch->have_sent_ack_eliciting_since_rx = 1; - } + * Send packet, if we need to. Best effort. The TXP consults the CC and + * applies any limitations imposed by it, so we don't need to do it here. + * + * Best effort. In particular if TXP fails for some reason we should + * still flush any queued packets which we already generated. + */ + res = ossl_quic_tx_packetiser_generate(ch->txp, &status); + if (status.sent_pkt > 0) { + ch->have_sent_any_pkt = 1; /* Packet(s) were sent */ - if (!ch->is_server && status.sent_handshake) /* - * RFC 9001 s. 4.9.1: A client MUST discard Initial keys when it - * first sends a Handshake packet. - */ - ch_discard_el(ch, QUIC_ENC_LEVEL_INITIAL); + * RFC 9000 s. 10.1. 'An endpoint also restarts its idle timer when + * sending an ack-eliciting packet if no other ack-eliciting packets + * have been sent since last receiving and processing a packet.' + */ + if (status.sent_ack_eliciting + && !ch->have_sent_ack_eliciting_since_rx) { + ch_update_idle(ch); + ch->have_sent_ack_eliciting_since_rx = 1; + } - if (ch->rxku_pending_confirm_done) - ch->rxku_pending_confirm = 0; + if (!ch->is_server && status.sent_handshake) + /* + * RFC 9001 s. 4.9.1: A client MUST discard Initial keys when it + * first sends a Handshake packet. + */ + ch_discard_el(ch, QUIC_ENC_LEVEL_INITIAL); - ch_update_ping_deadline(ch); - } + if (ch->rxku_pending_confirm_done) + ch->rxku_pending_confirm = 0; - 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 - * an emergent consequence of our design, as the TX packetiser will - * never transmit another packet when the TX PN reaches the limit. - * - * Calling the below function terminates the connection; its attempt to - * schedule a CONNECTION_CLOSE frame will not actually cause a packet to - * be transmitted for this reason. - */ - ossl_quic_channel_raise_protocol_error(ch, QUIC_ERR_INTERNAL_ERROR, 0, - "internal error (txp generate)"); - } + ch_update_ping_deadline(ch); + } + + if (!res) { + /* + * 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 an emergent consequence of our design, as the + * TX packetiser will never transmit another packet when the TX PN + * reaches the limit. + * + * Calling the below function terminates the connection; its attempt + * to schedule a CONNECTION_CLOSE frame will not actually cause a + * packet to be transmitted for this reason. + */ + ossl_quic_channel_raise_protocol_error(ch, QUIC_ERR_INTERNAL_ERROR, + 0, + "internal error (txp generate)"); + break; + } + } while (status.sent_pkt > 0); /* Flush packets to network. */ switch (ossl_qtx_flush_net(ch->qtx)) { diff --git a/test/recipes/75-test_quicapi_data/ssltraceref.txt b/test/recipes/75-test_quicapi_data/ssltraceref.txt index cbca1cad708..6b64d0652e9 100644 --- a/test/recipes/75-test_quicapi_data/ssltraceref.txt +++ b/test/recipes/75-test_quicapi_data/ssltraceref.txt @@ -81,6 +81,8 @@ Sent Datagram Length: 1200 Received Datagram Length: 1200 +Received Datagram + Length: 234 Received Packet Packet Type: Initial Version: 0x00000001 @@ -118,6 +120,13 @@ Header: NamedGroup: ecdh_x25519 (29) key_exchange: (len=32): ???????????????????????????????????????????????????????????????? +Received Packet + Packet Type: Handshake + Version: 0x00000001 + Destination Conn Id: + Source Conn Id: 0x???????????????? + Payload length: 213 + Packet Number: 0x00000001 Received Packet Packet Type: Handshake Version: 0x00000001 @@ -125,6 +134,9 @@ Received Packet Source Conn Id: 0x???????????????? Payload length: 1042 Packet Number: 0x00000000 +Received Frame: Crypto + Offset: 1022 + Len: 192 Received Frame: Crypto Offset: 0 Len: 1022 @@ -233,40 +245,6 @@ YeeuLO02zToHhnQ6KbPXOrQAqcL1kngO4g+j/ru+4AZThFkdkGnltvk= ------------------ No extensions -Sent Frame: Ack (without ECN) - Largest acked: 0 - Ack delay (raw) 0 - Ack range count: 0 - First ack range: 0 -Sent Frame: Ack (without ECN) - Largest acked: 0 - Ack delay (raw) 0 - Ack range count: 0 - First ack range: 0 -Sent Frame: Padding -Sent Packet - Packet Type: Initial - Version: 0x00000001 - Destination Conn Id: 0x???????????????? - Source Conn Id: - Payload length: 1137 - Token: - Packet Number: 0x00000001 - -Sent Datagram - Length: 1200 -Received Datagram - Length: 234 -Received Packet - Packet Type: Handshake - Version: 0x00000001 - Destination Conn Id: - Source Conn Id: 0x???????????????? - Payload length: 213 - Packet Number: 0x00000001 -Received Frame: Crypto - Offset: 1022 - Len: 192 Received TLS Record Header: Version = TLS 1.2 (0x303) @@ -289,6 +267,11 @@ Header: Finished, Length=32 verify_data (len=32): ???????????????????????????????????????????????????????????????? +Sent Frame: Ack (without ECN) + Largest acked: 0 + Ack delay (raw) 0 + Ack range count: 0 + First ack range: 0 Sent Frame: Ack (without ECN) Largest acked: 1 Ack delay (raw) 0 @@ -297,12 +280,21 @@ Sent Frame: Ack (without ECN) Sent Frame: Crypto Offset: 0 Len: 36 +Sent Frame: Padding +Sent Packet + Packet Type: Initial + Version: 0x00000001 + Destination Conn Id: 0x???????????????? + Source Conn Id: + Payload length: 1097 + Token: + Packet Number: 0x00000001 Sent Packet Packet Type: Handshake Version: 0x00000001 Destination Conn Id: 0x???????????????? Source Conn Id: Payload length: 60 - Packet Number: 0x00000001 + Packet Number: 0x00000000 Sent Datagram - Length: 81 + Length: 1200