QUIC: Automatically drain non-concluded streams, bugfixes

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21484)

diff --git a/ssl/quic/quic_impl.c b/ssl/quic/quic_impl.c
index 3264fb9c9435b5ed665eb97e7af2cd77b82cc591..848b1b4f52359ee82477f1fda462e403190bbd83 100644 (file)
--- a/ssl/quic/quic_impl.c
+++ b/ssl/quic/quic_impl.c
@@ -1169,7 +1169,7 @@ int ossl_quic_conn_shutdown(SSL *s, uint64_t flags,
     int stream_flush = ((flags & SSL_SHUTDOWN_FLAG_NO_STREAM_FLUSH) == 0);
 
     if (!expect_quic(s, &ctx))
-        return 0;
+        return -1;
 
     if (ctx.is_stream)
         /* TODO(QUIC): Semantics currently undefined for QSSOs */
@@ -1177,6 +1177,11 @@ int ossl_quic_conn_shutdown(SSL *s, uint64_t flags,
 
     quic_lock(ctx.qc);
 
+    if (ossl_quic_channel_is_terminated(ctx.qc->ch)) {
+        quic_unlock(ctx.qc);
+        return 1;
+    }
+
     /* Phase 1: Stream Flushing */
     if (stream_flush) {
         qc_shutdown_flush_init(ctx.qc);

diff --git a/ssl/quic/quic_stream_map.c b/ssl/quic/quic_stream_map.c
index e50da2b101b2ae9a532a72a08fe155e782e3d746..b23429a3780c299ed6254a73206e3251b9454711 100644 (file)
--- a/ssl/quic/quic_stream_map.c
+++ b/ssl/quic/quic_stream_map.c
@@ -16,6 +16,8 @@
  */
 DEFINE_LHASH_OF_EX(QUIC_STREAM);
 
+static void shutdown_flush_done(QUIC_STREAM_MAP *qsm, QUIC_STREAM *qs);
+
 /* Circular list management. */
 static void list_insert_tail(QUIC_STREAM_LIST_NODE *l,
                              QUIC_STREAM_LIST_NODE *n)
@@ -327,6 +329,10 @@ void ossl_quic_stream_map_update_state(QUIC_STREAM_MAP *qsm, QUIC_STREAM *s)
     if (s->send_state == QUIC_SSTREAM_STATE_DATA_SENT
         && ossl_quic_sstream_is_totally_acked(s->sstream))
         ossl_quic_stream_map_notify_totally_acked(qsm, s);
+    else if (s->shutdown_flush
+             && s->send_state == QUIC_SSTREAM_STATE_SEND
+             && ossl_quic_sstream_is_totally_acked(s->sstream))
+        shutdown_flush_done(qsm, s);
 
     if (!s->ready_for_gc) {
         s->ready_for_gc = qsm_ready_for_gc(qsm, s);
@@ -765,14 +771,19 @@ static int eligible_for_shutdown_flush(QUIC_STREAM *qs)
 {
     /*
      * We only care about servicing the send part of a stream (if any) during
-     * shutdown flush. A stream needs to have a final size and that final size
-     * needs to be a result of normal conclusion of a stream via
-     * SSL_stream_conclude (as opposed to due to reset). If the application did
-     * not conclude a stream before deleting it we assume it does not care about
-     * data being flushed during connection termination.
+     * shutdown flush. We make sure we flush a stream if it is either
+     * non-terminated or was terminated normally such as via
+     * SSL_stream_conclude. A stream which was terminated via a reset is not
+     * flushed, and we will have thrown away the send buffer in that case
+     * anyway.
      */
-    return ossl_quic_stream_has_send_buffer(qs)
-        && ossl_quic_stream_send_get_final_size(qs, NULL);
+    switch (qs->send_state) {
+    case QUIC_SSTREAM_STATE_SEND:
+    case QUIC_SSTREAM_STATE_DATA_SENT:
+        return !ossl_quic_sstream_is_totally_acked(qs->sstream);
+    default:
+        return 0;
+    }
 }
 
 static void begin_shutdown_flush_each(QUIC_STREAM *qs, void *arg)

diff --git a/ssl/quic/quic_txp.c b/ssl/quic/quic_txp.c
index 55755eddf8f738bb462b269088cb135931b33437..f244488a4c789c027a5bec77de8424b7c4e1105d 100644 (file)
--- a/ssl/quic/quic_txp.c
+++ b/ssl/quic/quic_txp.c
@@ -2969,8 +2969,9 @@ OSSL_TIME ossl_quic_tx_packetiser_get_deadline(OSSL_QUIC_TX_PACKETISER *txp)
         }
 
     /* When will CC let us send more? */
-    deadline = ossl_time_min(deadline,
-                             txp->args.cc_method->get_wakeup_deadline(txp->args.cc_data));
+    if (txp->args.cc_method->get_tx_allowance(txp->args.cc_data) == 0)
+        deadline = ossl_time_min(deadline,
+                                 txp->args.cc_method->get_wakeup_deadline(txp->args.cc_data));
 
     return deadline;
 }

diff --git a/test/helpers/quictestlib.c b/test/helpers/quictestlib.c
index 0d99d4556c4b9c22f9c7215225d0f13eca04c31a..d20afb45859bf38104e0cda2dcd31afdbfe66702 100644 (file)
--- a/test/helpers/quictestlib.c
+++ b/test/helpers/quictestlib.c
@@ -337,8 +337,17 @@ int qtest_create_quic_connection(QUIC_TSERVER *qtserv, SSL *clientssl)
 int qtest_shutdown(QUIC_TSERVER *qtserv, SSL *clientssl)
 {
     /* Busy loop in non-blocking mode. It should be quick because its local */
-    while (SSL_shutdown(clientssl) != 1)
+    for (;;) {
+        int rc = SSL_shutdown(clientssl);
+
+        if (rc == 1)
+            break;
+
+        if (rc < 0)
+            return 0;
+
         ossl_quic_tserver_tick(qtserv);
+    }
 
     return 1;
 }

diff --git a/test/quicapitest.c b/test/quicapitest.c
index 780224f07c6c11cf8418359db090ba43507a73fc..9c79572208e1a4e507d2fb5b0d0db21133e0c0bb 100644 (file)
--- a/test/quicapitest.c
+++ b/test/quicapitest.c
@@ -133,8 +133,13 @@ static int test_quic_write_read(int idx)
             goto end;
     }
 
-    if (!TEST_true(qtest_shutdown(qtserv, clientquic)))
-        goto end;
+    if (idx < 1)
+        /*
+         * Blocking SSL_shutdown cannot be tested here due to requirement to
+         * tick TSERVER during drainage.
+         */
+        if (!TEST_true(qtest_shutdown(qtserv, clientquic)))
+            goto end;
 
     ret = 1;