diff options
author | Matt Caswell <matt@openssl.org> | 2023-02-23 16:31:49 +0000 |
---|---|---|
committer | Pauli <pauli@openssl.org> | 2023-04-12 11:02:01 +1000 |
commit | 7257188b7054cf8acfc4837e38486459e0930718 (patch) | |
tree | c0abc99f056e96dcc2b7123805033fbe00eda9b4 | |
parent | dfb8e185134df90fd3f21fb6ec625e7c295fdcea (diff) |
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)
-rw-r--r-- | include/internal/quic_tls.h | 15 | ||||
-rw-r--r-- | ssl/quic/quic_channel.c | 31 | ||||
-rw-r--r-- | ssl/quic/quic_tls.c | 33 |
3 files changed, 52 insertions, 27 deletions
diff --git a/include/internal/quic_tls.h b/include/internal/quic_tls.h index 2d6007e79f..133e247c26 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 a873054f90..460a7d77db 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 088a59f28a..2a7cc03e55 100644 --- a/ssl/quic/quic_tls.c +++ b/ssl/quic/quic_tls.c @@ -54,18 +54,12 @@ struct ossl_record_layer_st { 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; } |