From b09e246aba584cd17d1d027f735f238b1b7f082c Mon Sep 17 00:00:00 2001
From: Matt Caswell <matt@openssl.org>
Date: Mon, 8 May 2023 13:51:39 +0100
Subject: Properly handling stream/crypto frames while tracing

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20914)
---
 ssl/quic/quic_rx_depack.c |  4 ++--
 ssl/quic/quic_trace.c     | 24 +++++++++++-------------
 ssl/quic/quic_wire.c      | 33 +++++++++++++++++++++++----------
 3 files changed, 36 insertions(+), 25 deletions(-)

(limited to 'ssl')

diff --git a/ssl/quic/quic_rx_depack.c b/ssl/quic/quic_rx_depack.c
index 996de79bdf..740f95f23a 100644
--- a/ssl/quic/quic_rx_depack.c
+++ b/ssl/quic/quic_rx_depack.c
@@ -196,7 +196,7 @@ static int depack_do_frame_crypto(PACKET *pkt, QUIC_CHANNEL *ch,
 
     *datalen = 0;
 
-    if (!ossl_quic_wire_decode_frame_crypto(pkt, &f)) {
+    if (!ossl_quic_wire_decode_frame_crypto(pkt, 0, &f)) {
         ossl_quic_channel_raise_protocol_error(ch,
                                                QUIC_ERR_FRAME_ENCODING_ERROR,
                                                OSSL_QUIC_FRAME_TYPE_CRYPTO,
@@ -395,7 +395,7 @@ static int depack_do_frame_stream(PACKET *pkt, QUIC_CHANNEL *ch,
 
     *datalen = 0;
 
-    if (!ossl_quic_wire_decode_frame_stream(pkt, &frame_data)) {
+    if (!ossl_quic_wire_decode_frame_stream(pkt, 0, &frame_data)) {
         ossl_quic_channel_raise_protocol_error(ch,
                                                QUIC_ERR_FRAME_ENCODING_ERROR,
                                                frame_type,
diff --git a/ssl/quic/quic_trace.c b/ssl/quic/quic_trace.c
index aa25e8f028..d84a2f65c6 100644
--- a/ssl/quic/quic_trace.c
+++ b/ssl/quic/quic_trace.c
@@ -110,7 +110,7 @@ static int frame_crypto(BIO *bio, PACKET *pkt)
 {
     OSSL_QUIC_FRAME_CRYPTO frame_data;
 
-    if (!ossl_quic_wire_decode_frame_crypto(pkt, &frame_data))
+    if (!ossl_quic_wire_decode_frame_crypto(pkt, 1, &frame_data))
         return 0;
 
     BIO_printf(bio, "    Offset: %lu\n", frame_data.offset);
@@ -173,12 +173,19 @@ static int frame_stream(BIO *bio, PACKET *pkt, uint64_t frame_type)
         return 0;
     }
 
-    if (!ossl_quic_wire_decode_frame_stream(pkt, &frame_data))
+    if (!ossl_quic_wire_decode_frame_stream(pkt, 1, &frame_data))
         return 0;
 
     BIO_printf(bio, "    Stream id: %lu\n", frame_data.stream_id);
     BIO_printf(bio, "    Offset: %lu\n", frame_data.offset);
-    BIO_printf(bio, "    Len: %lu\n", frame_data.len);
+    /*
+     * It would be nice to find a way of passing the implicit length through
+     * to the msg_callback. But this is not currently possible.
+     */
+    if (frame_data.has_explicit_len)
+        BIO_printf(bio, "    Len: %lu\n", frame_data.len);
+    else
+        BIO_puts(bio, "    Len: <implicit length>\n");
 
     return 1;
 }
@@ -532,6 +539,7 @@ int ossl_quic_trace(int write_p, int version, int content_type,
 
     case SSL3_RT_QUIC_FRAME_PADDING:
     case SSL3_RT_QUIC_FRAME_FULL:
+    case SSL3_RT_QUIC_FRAME_HEADER:
         {
             BIO_puts(bio, write_p ? "Sent" : "Received");
             BIO_puts(bio, " Frame: ");
@@ -545,16 +553,6 @@ int ossl_quic_trace(int write_p, int version, int content_type,
         }
         break;
 
-    case SSL3_RT_QUIC_FRAME_HEADER:
-        {
-            BIO_puts(bio, write_p ? "Sent" : "Received");
-            BIO_puts(bio, " Frame Data\n");
-
-            /* TODO(QUIC): Implement me */
-            BIO_puts(bio, "  <content skipped>\n");
-        }
-        break;
-
     default:
         /* Unrecognised content_type. We defer to SSL_trace */
         return 0;
diff --git a/ssl/quic/quic_wire.c b/ssl/quic/quic_wire.c
index 5d35c98b67..7df32c77b2 100644
--- a/ssl/quic/quic_wire.c
+++ b/ssl/quic/quic_wire.c
@@ -591,6 +591,7 @@ int ossl_quic_wire_decode_frame_stop_sending(PACKET *pkt,
 }
 
 int ossl_quic_wire_decode_frame_crypto(PACKET *pkt,
+                                       int nodata,
                                        OSSL_QUIC_FRAME_CRYPTO *f)
 {
     if (!expect_frame_header(pkt, OSSL_QUIC_FRAME_TYPE_CRYPTO)
@@ -599,13 +600,17 @@ int ossl_quic_wire_decode_frame_crypto(PACKET *pkt,
             || f->len > SIZE_MAX /* sizeof(uint64_t) > sizeof(size_t)? */)
         return 0;
 
-    if (PACKET_remaining(pkt) < f->len)
-        return 0;
+    if (nodata) {
+        f->data = NULL;
+    } else {
+        if (PACKET_remaining(pkt) < f->len)
+            return 0;
 
-    f->data = PACKET_data(pkt);
+        f->data = PACKET_data(pkt);
 
-    if (!PACKET_forward(pkt, (size_t)f->len))
-        return 0;
+        if (!PACKET_forward(pkt, (size_t)f->len))
+            return 0;
+    }
 
     return 1;
 }
@@ -633,6 +638,7 @@ int ossl_quic_wire_decode_frame_new_token(PACKET               *pkt,
 }
 
 int ossl_quic_wire_decode_frame_stream(PACKET *pkt,
+                                       int nodata,
                                        OSSL_QUIC_FRAME_STREAM *f)
 {
     uint64_t frame_type;
@@ -658,14 +664,21 @@ int ossl_quic_wire_decode_frame_stream(PACKET *pkt,
         if (!PACKET_get_quic_vlint(pkt, &f->len))
             return 0;
     } else {
-        f->len = PACKET_remaining(pkt);
+        if (nodata)
+            f->len = 0;
+        else
+            f->len = PACKET_remaining(pkt);
     }
 
-    f->data = PACKET_data(pkt);
+    if (nodata) {
+        f->data = NULL;
+    } else {
+        f->data = PACKET_data(pkt);
 
-    if (f->len > SIZE_MAX /* sizeof(uint64_t) > sizeof(size_t)? */
-        || !PACKET_forward(pkt, (size_t)f->len))
-        return 0;
+        if (f->len > SIZE_MAX /* sizeof(uint64_t) > sizeof(size_t)? */
+            || !PACKET_forward(pkt, (size_t)f->len))
+            return 0;
+    }
 
     return 1;
 }
-- 
cgit v1.2.3