QUIC QSM: Model final sizes and handle STOP_SENDING correctly

Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #21135)
openssl · Jul 16, 2023 · 418e122 · 418e122
1 parent 01715f2
commit 418e122
Show file tree

Hide file tree

Showing 6 changed files with 195 additions and 7 deletions.
diff --git a/include/internal/quic_fc.h b/include/internal/quic_fc.h
@@ -261,6 +261,13 @@ int ossl_quic_rxfc_has_cwm_changed(QUIC_RXFC *rxfc, int clear);
  */
 int ossl_quic_rxfc_get_error(QUIC_RXFC *rxfc, int clear);
 
+/*
+ * Returns 1 if the RXFC is a stream-level RXFC and the RXFC knows the final
+ * size for the stream in bytes. If this is the case and final_size is non-NULL,
+ * writes the final size to *final_size. Otherwise, returns 0.
+ */
+int ossl_quic_rxfc_get_final_size(const QUIC_RXFC *rxfc, uint64_t *final_size);
+
 # endif
 
 #endif
diff --git a/include/internal/quic_stream_map.h b/include/internal/quic_stream_map.h
@@ -13,6 +13,7 @@
 
 # include "internal/e_os.h"
 # include "internal/time.h"
+# include "internal/common.h"
 # include "internal/quic_types.h"
 # include "internal/quic_stream.h"
 # include "internal/quic_fc.h"
@@ -118,6 +119,19 @@ struct quic_stream_st {
     /* Temporary value used by TXP. */
     uint64_t        txp_txfc_new_credit_consumed;
 
+    /*
+     * The final size of the send stream. Although this information can be
+     * discerned from a QUIC_SSTREAM, it is stored separately as we need to keep
+     * track of this even if we have thrown away the QUIC_SSTREAM. Use
+     * ossl_quic_stream_send_get_final_size to determine if this contain a
+     * valid value or if there is no final size yet for a sending part.
+     *
+     * For the receive part, the final size is tracked by the stream-level RXFC;
+     * use ossl_quic_stream_recv_get_final_size or
+     * ossl_quic_rxfc_get_final_size.
+     */
+    uint64_t        send_final_size;
+
     /*
      * Send stream part and receive stream part buffer management objects.
      *
@@ -433,6 +447,63 @@ static ossl_inline ossl_unused int ossl_quic_stream_recv_is_reset(const QUIC_STR
         || s->recv_state == QUIC_RSTREAM_STATE_RESET_READ;
 }
 
+/*
+ * Returns 1 if the stream has a send part and that part has a final size.
+ *
+ * If final_size is non-NULL, *final_size is the final size (on success) or an
+ * undefined value otherwise.
+ */
+static ossl_inline ossl_unused int ossl_quic_stream_send_get_final_size(const QUIC_STREAM *s,
+                                                                        uint64_t *final_size)
+{
+    switch (s->send_state) {
+    default:
+    case QUIC_SSTREAM_STATE_NONE:
+        return 0;
+    case QUIC_SSTREAM_STATE_SEND:
+        /*
+         * SEND may or may not have had a FIN - even if we have a FIN we do not
+         * move to DATA_SENT until we have actually sent all the data. So
+         * ask the QUIC_SSTREAM.
+         */
+        return ossl_quic_sstream_get_final_size(s->sstream, final_size);
+    case QUIC_SSTREAM_STATE_DATA_SENT:
+    case QUIC_SSTREAM_STATE_DATA_RECVD:
+    case QUIC_SSTREAM_STATE_RESET_SENT:
+    case QUIC_SSTREAM_STATE_RESET_RECVD:
+        if (final_size != NULL)
+            *final_size = s->send_final_size;
+        return 1;
+    }
+}
+
+/*
+ * Returns 1 if the stream has a receive part and that part has a final size.
+ *
+ * If final_size is non-NULL, *final_size is the final size (on success) or an
+ * undefined value otherwise.
+ */
+static ossl_inline ossl_unused int ossl_quic_stream_recv_get_final_size(const QUIC_STREAM *s,
+                                                                        uint64_t *final_size)
+{
+    switch (s->recv_state) {
+    default:
+    case QUIC_RSTREAM_STATE_NONE:
+    case QUIC_RSTREAM_STATE_RECV:
+        return 0;
+
+    case QUIC_RSTREAM_STATE_SIZE_KNOWN:
+    case QUIC_RSTREAM_STATE_DATA_RECVD:
+    case QUIC_RSTREAM_STATE_DATA_READ:
+    case QUIC_RSTREAM_STATE_RESET_RECVD:
+    case QUIC_RSTREAM_STATE_RESET_READ:
+        if (!ossl_assert(ossl_quic_rxfc_get_final_size(&s->rxfc, final_size)))
+            return 0;
+
+        return 1;
+    }
+}
+
 /*
  * QUIC Stream Map
  * ===============
@@ -630,11 +701,13 @@ int ossl_quic_stream_map_notify_reset_stream_acked(QUIC_STREAM_MAP *qsm,
 /*
  * Transitions from the RECV to SIZE_KNOWN receive stream states. This should be
  * called once a STREAM frame is received for the stream with the FIN bit set.
+ * final_size should be the final size of the stream in bytes.
  *
  * Returns 1 if the transition was taken.
  */
 int ossl_quic_stream_map_notify_size_known_recv_part(QUIC_STREAM_MAP *qsm,
-                                                     QUIC_STREAM *qs);
+                                                     QUIC_STREAM *qs,
+                                                     uint64_t final_size);
 
 /* SIZE_KNOWN -> DATA_RECVD */
 int ossl_quic_stream_map_notify_totally_received(QUIC_STREAM_MAP *qsm,
@@ -665,6 +738,15 @@ int ossl_quic_stream_map_stop_sending_recv_part(QUIC_STREAM_MAP *qsm,
                                                 QUIC_STREAM *qs,
                                                 uint64_t aec);
 
+/*
+ * Marks the stream as wanting a STOP_SENDING frame transmitted. It is not valid
+ * to vall this if ossl_quic_stream_map_stop_sending_recv_part() has not been
+ * called. For TXP use.
+ */
+int ossl_quic_stream_map_schedule_stop_sending(QUIC_STREAM_MAP *qsm,
+                                               QUIC_STREAM *qs);
+
+
 /*
  * Accept Queue Management
  * =======================

diff --git a/ssl/quic/quic_fc.c b/ssl/quic/quic_fc.c
@@ -393,3 +393,14 @@ int ossl_quic_rxfc_get_error(QUIC_RXFC *rxfc, int clear)
 
     return r;
 }
+
+int ossl_quic_rxfc_get_final_size(const QUIC_RXFC *rxfc, uint64_t *final_size)
+{
+    if (!rxfc->is_fin)
+        return 0;
+
+    if (final_size != NULL)
+        *final_size = rxfc->hwm;
+
+    return 1;
+}
diff --git a/ssl/quic/quic_rx_depack.c b/ssl/quic/quic_rx_depack.c
@@ -488,6 +488,45 @@ static int depack_do_frame_stream(PACKET *pkt, QUIC_CHANNEL *ch,
         return 0;
     }
 
+    switch (stream->recv_state) {
+    case QUIC_RSTREAM_STATE_RECV:
+    case QUIC_RSTREAM_STATE_SIZE_KNOWN:
+        /*
+         * It only makes sense to process incoming STREAM frames in these
+         * states.
+         */
+        break;
+
+    case QUIC_RSTREAM_STATE_DATA_RECVD:
+    case QUIC_RSTREAM_STATE_DATA_READ:
+    case QUIC_RSTREAM_STATE_RESET_RECVD:
+    case QUIC_RSTREAM_STATE_RESET_READ:
+    default:
+        /*
+         * We have no use for STREAM frames once the receive part reaches any of
+         * these states, so just ignore.
+         */
+        return 1;
+    }
+
+    /* If we are in RECV, auto-transition to SIZE_KNOWN on FIN. */
+    if (frame_data.is_fin
+        && !ossl_quic_stream_recv_get_final_size(stream, NULL)) {
+
+        /* State was already checked above, so can't fail. */
+        ossl_quic_stream_map_notify_size_known_recv_part(&ch->qsm, stream,
+                                                         frame_data.offset
+                                                         + frame_data.len);
+    }
+
+    /*
+     * If we requested STOP_SENDING do not bother buffering the data. Note that
+     * this must happen after RXFC checks above as even if we sent STOP_SENDING
+     * we must still enforce correct flow control (RFC 9000 s. 3.5).
+     */
+    if (stream->stop_sending)
+        return 1; /* not an error - packet reordering, etc. */
+
     /*
      * The receive stream buffer may or may not choose to consume the data
      * without copying by reffing the OSSL_QRX_PKT. In this case

diff --git a/ssl/quic/quic_stream_map.c b/ssl/quic/quic_stream_map.c
@@ -161,6 +161,8 @@ QUIC_STREAM *ossl_quic_stream_map_alloc(QUIC_STREAM_MAP *qsm,
         ? QUIC_RSTREAM_STATE_RECV
         : QUIC_RSTREAM_STATE_NONE;
 
+    s->send_final_size  = UINT64_MAX;
+
     lh_QUIC_STREAM_insert(qsm->map, s);
     return s;
 }
@@ -487,7 +489,8 @@ int ossl_quic_stream_map_notify_reset_stream_acked(QUIC_STREAM_MAP *qsm,
  */
 
 int ossl_quic_stream_map_notify_size_known_recv_part(QUIC_STREAM_MAP *qsm,
-                                                     QUIC_STREAM *qs)
+                                                     QUIC_STREAM *qs,
+                                                     uint64_t final_size)
 {
     switch (qs->recv_state) {
     default:
@@ -497,7 +500,7 @@ int ossl_quic_stream_map_notify_size_known_recv_part(QUIC_STREAM_MAP *qsm,
         return 0;
 
     case QUIC_RSTREAM_STATE_RECV:
-        qs->recv_state = QUIC_RSTREAM_STATE_SIZE_KNOWN;
+        qs->recv_state      = QUIC_RSTREAM_STATE_SIZE_KNOWN;
         return 1;
     }
 }
@@ -513,7 +516,8 @@ int ossl_quic_stream_map_notify_totally_received(QUIC_STREAM_MAP *qsm,
         return 0;
 
     case QUIC_RSTREAM_STATE_SIZE_KNOWN:
-        qs->recv_state = QUIC_RSTREAM_STATE_DATA_RECVD;
+        qs->recv_state          = QUIC_RSTREAM_STATE_DATA_RECVD;
+        qs->want_stop_sending   = 0;
         return 1;
     }
 }
@@ -599,10 +603,56 @@ int ossl_quic_stream_map_stop_sending_recv_part(QUIC_STREAM_MAP *qsm,
     if (qs->stop_sending)
         return 0;
 
+    switch (qs->recv_state) {
+    default:
+    case QUIC_RSTREAM_STATE_NONE:
+        /* Send-only stream, so this makes no sense. */
+    case QUIC_RSTREAM_STATE_DATA_RECVD:
+    case QUIC_RSTREAM_STATE_DATA_READ:
+        /*
+         * Not really any point in STOP_SENDING if we already received all data.
+         */
+    case QUIC_RSTREAM_STATE_RESET_RECVD:
+    case QUIC_RSTREAM_STATE_RESET_READ:
+        /* No point in STOP_SENDING if the peer already reset their send part. */
+        return 0;
+
+    case QUIC_RSTREAM_STATE_RECV:
+    case QUIC_RSTREAM_STATE_SIZE_KNOWN:
+        /*
+         * It does make sense to send STOP_SENDING for a receive part of a
+         * stream which has a known size (because we have received a FIN) but
+         * which still has other (previous) stream data yet to be received.
+         */
+        break;
+    }
+
     qs->stop_sending        = 1;
     qs->stop_sending_aec    = aec;
-    qs->want_stop_sending   = 1;
+    return ossl_quic_stream_map_schedule_stop_sending(qsm, qs);
+}
+
+int ossl_quic_stream_map_schedule_stop_sending(QUIC_STREAM_MAP *qsm, QUIC_STREAM *qs)
+{
+    if (!qs->stop_sending)
+        return 0;
+
+    /*
+     * Ignore the call as a no-op if already scheduled, or in a state
+     * where it makes no sense to send STOP_SENDING.
+     */
+    if (qs->want_stop_sending)
+        return 1;
+
+    switch (qs->recv_state) {
+    default:
+        return 1; /* ignore */
+    case QUIC_RSTREAM_STATE_RECV:
+    case QUIC_RSTREAM_STATE_SIZE_KNOWN:
+        break;
+    }
 
+    qs->want_stop_sending = 1;
     ossl_quic_stream_map_update_state(qsm, qs);
     return 1;
 }

diff --git a/ssl/quic/quic_txp.c b/ssl/quic/quic_txp.c
@@ -1161,8 +1161,7 @@ static void on_regen_notify(uint64_t frame_type, uint64_t stream_id,
                 if (s == NULL)
                     return;
 
-                s->want_stop_sending = 1;
-                ossl_quic_stream_map_update_state(txp->args.qsm, s);
+                ossl_quic_stream_map_schedule_stop_sending(txp->args.qsm, s);
             }
             break;
         case OSSL_QUIC_FRAME_TYPE_RESET_STREAM: