From: Hugo Landau <hlandau@openssl.org>
Date: Tue, 6 Jun 2023 15:25:10 +0000 (+0100)
Subject: QUIC CONFORMANCE: Validate RESET_STREAM final sizes correctly
X-Git-Tag: openssl-3.2.0-alpha1~455
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2cc0e2dddfe02c5fad74524bc975051021413ea5;p=thirdparty%2Fopenssl.git

QUIC CONFORMANCE: Validate RESET_STREAM final sizes correctly

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

diff --git a/include/internal/quic_stream_map.h b/include/internal/quic_stream_map.h
index 47224e1654f..e1ad864a726 100644
--- a/include/internal/quic_stream_map.h
+++ b/include/internal/quic_stream_map.h
@@ -720,7 +720,8 @@ int ossl_quic_stream_map_notify_totally_read(QUIC_STREAM_MAP *qsm,
 /* RECV/SIZE_KNOWN/DATA_RECVD -> RESET_RECVD */
 int ossl_quic_stream_map_notify_reset_recv_part(QUIC_STREAM_MAP *qsm,
                                                 QUIC_STREAM *qs,
-                                                uint64_t app_error_code);
+                                                uint64_t app_error_code,
+                                                uint64_t final_size);
 
 /* RESET_RECVD -> RESET_READ */
 int ossl_quic_stream_map_notify_app_read_reset_recv_part(QUIC_STREAM_MAP *qsm,
diff --git a/ssl/quic/quic_rx_depack.c b/ssl/quic/quic_rx_depack.c
index c5dbd77dbd3..d4a140d9748 100644
--- a/ssl/quic/quic_rx_depack.c
+++ b/ssl/quic/quic_rx_depack.c
@@ -137,6 +137,7 @@ static int depack_do_frame_reset_stream(PACKET *pkt,
 {
     OSSL_QUIC_FRAME_RESET_STREAM frame_data;
     QUIC_STREAM *stream = NULL;
+    uint64_t fce;
 
     if (!ossl_quic_wire_decode_frame_reset_stream(pkt, &frame_data)) {
         ossl_quic_channel_raise_protocol_error(ch,
@@ -166,6 +167,35 @@ static int depack_do_frame_reset_stream(PACKET *pkt,
         return 0;
     }
 
+    /*
+     * The final size field of the RESET_STREAM frame must be used to determine
+     * how much flow control credit the aborted stream was considered to have
+     * consumed.
+     *
+     * We also need to ensure that if we already have a final size for the
+     * stream, the RESET_STREAM frame's Final Size field matches this; we SHOULD
+     * terminate the connection otherwise (RFC 9000 s. 4.5). The RXFC takes care
+     * of this for us.
+     */
+    if (!ossl_quic_rxfc_on_rx_stream_frame(&stream->rxfc,
+                                           frame_data.final_size, /*is_fin=*/1)) {
+        ossl_quic_channel_raise_protocol_error(ch,
+                                               QUIC_ERR_INTERNAL_ERROR,
+                                               OSSL_QUIC_FRAME_TYPE_RESET_STREAM,
+                                               "internal error (flow control)");
+        return 0;
+    }
+
+    /* Has a flow control error occurred? */
+    fce = ossl_quic_rxfc_get_error(&stream->rxfc, 0);
+    if (fce != QUIC_ERR_NO_ERROR) {
+        ossl_quic_channel_raise_protocol_error(ch,
+                                               fce,
+                                               OSSL_QUIC_FRAME_TYPE_RESET_STREAM,
+                                               "flow control violation");
+        return 0;
+    }
+
     /*
      * Depending on the receive part state this is handled either as a reset
      * transition or a no-op (e.g. if a reset has already been received before,
@@ -173,7 +203,8 @@ static int depack_do_frame_reset_stream(PACKET *pkt,
      * protoocol error conditions we need to check for here.
      */
     ossl_quic_stream_map_notify_reset_recv_part(&ch->qsm, stream,
-                                                frame_data.app_error_code);
+                                                frame_data.app_error_code,
+                                                frame_data.final_size);
 
     ossl_quic_stream_map_update_state(&ch->qsm, stream);
     return 1;
diff --git a/ssl/quic/quic_stream_map.c b/ssl/quic/quic_stream_map.c
index 007bef521c7..ccd54edb4e6 100644
--- a/ssl/quic/quic_stream_map.c
+++ b/ssl/quic/quic_stream_map.c
@@ -562,8 +562,11 @@ int ossl_quic_stream_map_notify_totally_read(QUIC_STREAM_MAP *qsm,
 
 int ossl_quic_stream_map_notify_reset_recv_part(QUIC_STREAM_MAP *qsm,
                                                 QUIC_STREAM *qs,
-                                                uint64_t app_error_code)
+                                                uint64_t app_error_code,
+                                                uint64_t final_size)
 {
+    uint64_t prev_final_size;
+
     switch (qs->recv_state) {
     default:
     case QUIC_RSTREAM_STATE_NONE:
@@ -573,6 +576,11 @@ int ossl_quic_stream_map_notify_reset_recv_part(QUIC_STREAM_MAP *qsm,
     case QUIC_RSTREAM_STATE_RECV:
     case QUIC_RSTREAM_STATE_SIZE_KNOWN:
     case QUIC_RSTREAM_STATE_DATA_RECVD:
+        if (ossl_quic_stream_recv_get_final_size(qs, &prev_final_size)
+            && prev_final_size != final_size)
+            /* Cannot change previous final size. */
+            return 0;
+
         qs->recv_state              = QUIC_RSTREAM_STATE_RESET_RECVD;
         qs->peer_reset_stream_aec   = app_error_code;