From: Matt Caswell Date: Thu, 23 Feb 2023 16:31:49 +0000 (+0000) Subject: Add support for rstream get/release record in the QUIC TLS layer X-Git-Tag: openssl-3.2.0-alpha1~1017 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7257188b7054cf8acfc4837e38486459e0930718;p=thirdparty%2Fopenssl.git 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 Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/20404) --- diff --git a/include/internal/quic_tls.h b/include/internal/quic_tls.h index 2d6007e79f4..133e247c26c 100644 --- 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 a873054f906..460a7d77dbe 100644 --- 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 088a59f28ab..2a7cc03e553 100644 --- 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; }