Add support for rstream get/release record in the QUIC TLS layer

The QUIC TLS layer was taking an internal copy of rstream data while
reading. The QUIC rstream code has recently been extended to enable a
get/release model which avoids the need for this internal copy, so we use
that instead.

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

diff --git a/include/internal/quic_tls.h b/include/internal/quic_tls.h
index 2d6007e79f4e981d3bfe2221cddada36a0dcaed8..133e247c26cedcd876831ef8cc8146ea05534c0e 100644 (file)
--- a/include/internal/quic_tls.h
+++ b/include/internal/quic_tls.h
@@ -32,9 +32,18 @@ typedef struct quic_tls_args_st {
     int (*crypto_send_cb)(const unsigned char *buf, size_t buf_len,
                           size_t *consumed, void *arg);
     void *crypto_send_cb_arg;
-    int (*crypto_recv_cb)(unsigned char *buf, size_t buf_len,
-                          size_t *bytes_read, void *arg);
-    void *crypto_recv_cb_arg;
+
+    /*
+     * Call to receive crypto stream data. A pointer to the underlying buffer
+     * is provided, and subsequently released to avoid unnecessary copying of
+     * data.
+     */
+    int (*crypto_recv_rcd_cb)(const unsigned char **buf, size_t *bytes_read,
+                              void *arg);
+    void *crypto_recv_rcd_cb_arg;
+    int (*crypto_release_rcd_cb)(size_t bytes_read, void *arg);
+    void *crypto_release_rcd_cb_arg;
+
 
     /* Called when a traffic secret is available for a given encryption level. */
     int (*yield_secret_cb)(uint32_t enc_level, int direction /* 0=RX, 1=TX */,

diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c
index a873054f906e90b80f630b20977b68552081e3c6..460a7d77dbed59de22b4e3928270a1bad1857656 100644 (file)
--- a/ssl/quic/quic_channel.c
+++ b/ssl/quic/quic_channel.c
@@ -57,8 +57,9 @@ static int ch_on_handshake_yield_secret(uint32_t enc_level, int direction,
                                         const unsigned char *secret,
                                         size_t secret_len,
                                         void *arg);
-static int ch_on_crypto_recv(unsigned char *buf, size_t buf_len,
-                             size_t *bytes_read, void *arg);
+static int ch_on_crypto_recv_record(const unsigned char **buf,
+                                    size_t *bytes_read, void *arg);
+static int ch_on_crypto_release_record(size_t bytes_read, void *arg);
 static int crypto_ensure_empty(QUIC_RSTREAM *rstream);
 static int ch_on_crypto_send(const unsigned char *buf, size_t buf_len,
                              size_t *consumed, void *arg);
@@ -248,8 +249,10 @@ static int ch_init(QUIC_CHANNEL *ch)
     tls_args.s                          = ch->tls;
     tls_args.crypto_send_cb             = ch_on_crypto_send;
     tls_args.crypto_send_cb_arg         = ch;
-    tls_args.crypto_recv_cb             = ch_on_crypto_recv;
-    tls_args.crypto_recv_cb_arg         = ch;
+    tls_args.crypto_recv_rcd_cb         = ch_on_crypto_recv_record;
+    tls_args.crypto_recv_rcd_cb_arg     = ch;
+    tls_args.crypto_release_rcd_cb      = ch_on_crypto_release_record;
+    tls_args.crypto_release_rcd_cb_arg  = ch;
     tls_args.yield_secret_cb            = ch_on_handshake_yield_secret;
     tls_args.yield_secret_cb_arg        = ch;
     tls_args.got_transport_params_cb    = ch_on_transport_params;
@@ -534,8 +537,8 @@ static int crypto_ensure_empty(QUIC_RSTREAM *rstream)
     return avail == 0;
 }
 
-static int ch_on_crypto_recv(unsigned char *buf, size_t buf_len,
-                             size_t *bytes_read, void *arg)
+static int ch_on_crypto_recv_record(const unsigned char **buf,
+                                    size_t *bytes_read, void *arg)
 {
     QUIC_CHANNEL *ch = arg;
     QUIC_RSTREAM *rstream;
@@ -569,8 +572,20 @@ static int ch_on_crypto_recv(unsigned char *buf, size_t buf_len,
     if (rstream == NULL)
         return 0;
 
-    return ossl_quic_rstream_read(rstream, buf, buf_len, bytes_read,
-                                  &is_fin);
+    return ossl_quic_rstream_get_record(rstream, buf, bytes_read,
+                                        &is_fin);
+}
+
+static int ch_on_crypto_release_record(size_t bytes_read, void *arg)
+{
+    QUIC_CHANNEL *ch = arg;
+    QUIC_RSTREAM *rstream;
+
+    rstream = ch->crypto_recv[ossl_quic_enc_level_to_pn_space(ch->rx_enc_level)];
+    if (rstream == NULL)
+        return 0;
+
+    return ossl_quic_rstream_release_record(rstream, bytes_read);
 }
 
 static int ch_on_handshake_yield_secret(uint32_t enc_level, int direction,

diff --git a/ssl/quic/quic_tls.c b/ssl/quic/quic_tls.c
index 088a59f28ab4c3f6bcda2ad347a4ba95a4c6bf8c..2a7cc03e553a429278056871d743e91988344e47 100644 (file)
--- a/ssl/quic/quic_tls.c
+++ b/ssl/quic/quic_tls.c
@@ -53,19 +53,13 @@ struct ossl_record_layer_st {
     /* If we are part way through a write, a copy of the template */
     OSSL_RECORD_TEMPLATE template;
 
-    /*
-     * Temp buffer for storing received data (copied from the stream receive
-     * buffer)
-     */
-    unsigned char recbuf[SSL3_RT_MAX_PLAIN_LENGTH];
-
     /*
      * If we hit an error, what alert code should be used
      */
     int alert;
 
-    /* Set if recbuf is populated with data */
-    unsigned int recread : 1;
+    /* Amount of crypto stream data we have received but not yet released  */
+    size_t recread;
 
     /* Callbacks */
     OSSL_FUNC_rlayer_msg_callback_fn *msg_callback;
@@ -348,11 +342,11 @@ static int quic_read_record(OSSL_RECORD_LAYER *rl, void **rechandle,
     BIO_clear_retry_flags(rl->dummybio);
 
     /*
-     * TODO(QUIC): There seems to be an unnecessary copy here. It would be
-     *             better to send back a ref direct to the underlying buffer
+     * TODO(QUIC): Cast data to const, which should be safe....can read_record
+     * be modified to pass this as const to start with?
      */
-    if (!rl->qtls->args.crypto_recv_cb(rl->recbuf, sizeof(rl->recbuf), datalen,
-                                       rl->qtls->args.crypto_recv_cb_arg)) {
+    if (!rl->qtls->args.crypto_recv_rcd_cb((const unsigned char **)data, datalen,
+                                           rl->qtls->args.crypto_recv_rcd_cb_arg)) {
         QUIC_TLS_FATAL(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
         return OSSL_RECORD_RETURN_FATAL;
     }
@@ -365,8 +359,7 @@ static int quic_read_record(OSSL_RECORD_LAYER *rl, void **rechandle,
     *rechandle = rl;
     *rversion = TLS1_3_VERSION;
     *type = SSL3_RT_HANDSHAKE;
-    *data = rl->recbuf;
-    rl->recread = 1;
+    rl->recread = *datalen;
     /* epoch/seq_num are not relevant for TLS */
 
     if (rl->msg_callback != NULL) {
@@ -399,11 +392,18 @@ static int quic_read_record(OSSL_RECORD_LAYER *rl, void **rechandle,
 
 static int quic_release_record(OSSL_RECORD_LAYER *rl, void *rechandle)
 {
-    if (!ossl_assert(rl->recread == 1) || !ossl_assert(rl == rechandle)) {
+    if (!ossl_assert(rl->recread > 0) || !ossl_assert(rl == rechandle)) {
         QUIC_TLS_FATAL(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
         return 0;
     }
 
+    if (!rl->qtls->args.crypto_release_rcd_cb(rl->recread,
+                                              rl->qtls->args.crypto_release_rcd_cb_arg)) {
+        QUIC_TLS_FATAL(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
+        return OSSL_RECORD_RETURN_FATAL;
+    }
+
+
     rl->recread = 0;
     return 1;
 }
@@ -602,7 +602,8 @@ QUIC_TLS *ossl_quic_tls_new(const QUIC_TLS_ARGS *args)
     QUIC_TLS *qtls;
 
     if (args->crypto_send_cb == NULL
-        || args->crypto_recv_cb == NULL) {
+        || args->crypto_recv_rcd_cb == NULL
+        || args->crypto_release_rcd_cb == NULL) {
         ERR_raise(ERR_LIB_SSL, ERR_R_PASSED_NULL_PARAMETER);
         return NULL;
     }