From aca70ca81c6fcf38554aa95a3a2c75e1eeb1a085 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Mon, 26 Sep 2022 17:44:11 +0100 Subject: [PATCH] Defer record header preparation to the protocol methods We introduce a new function to prepare the record header. KTLS has its own version since this is done by the kernel. Reviewed-by: Hugo Landau Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/19343) --- ssl/record/methods/ktls_meth.c | 21 ++++++++-- ssl/record/methods/recmethod_local.h | 17 ++++++-- ssl/record/methods/ssl3_meth.c | 3 +- ssl/record/methods/tls13_meth.c | 3 +- ssl/record/methods/tls1_meth.c | 3 +- ssl/record/methods/tls_common.c | 63 ++++++++++++++++------------ ssl/record/methods/tlsany_meth.c | 4 +- 7 files changed, 76 insertions(+), 38 deletions(-) diff --git a/ssl/record/methods/ktls_meth.c b/ssl/record/methods/ktls_meth.c index 7e4567797d3..60fd80465d7 100644 --- a/ssl/record/methods/ktls_meth.c +++ b/ssl/record/methods/ktls_meth.c @@ -461,7 +461,7 @@ static int ktls_initialise_write_packets(OSSL_RECORD_LAYER *rl, SSL3_BUFFER *wb; /* - * We just use the application buffer directly, and don't use any WPACKET + * We just use the application buffer directly and don't use any WPACKET * structures */ wb = &bufs[0]; @@ -470,8 +470,8 @@ static int ktls_initialise_write_packets(OSSL_RECORD_LAYER *rl, /* * ktls doesn't modify the buffer, but to avoid a warning we need * to discard the const qualifier. - * This doesn't leak memory because the buffers have been - * released when switching to ktls. + * This doesn't leak memory because the buffers have never been allocated + * with KTLS */ SSL3_BUFFER_set_buf(wb, (unsigned char *)templates[0].buf); SSL3_BUFFER_set_offset(wb, 0); @@ -480,6 +480,18 @@ static int ktls_initialise_write_packets(OSSL_RECORD_LAYER *rl, return 1; } +static int ktls_prepare_record_header(OSSL_RECORD_LAYER *rl, + WPACKET *thispkt, + OSSL_RECORD_TEMPLATE *templ, + unsigned int rectype, + unsigned char **recdata) +{ + /* The kernel writes the record header, so nothing to do */ + *recdata = NULL; + + return 1; +} + static struct record_functions_st ossl_ktls_funcs = { ktls_set_crypto_state, ktls_cipher, @@ -493,7 +505,8 @@ static struct record_functions_st ossl_ktls_funcs = { tls_write_records_default, ktls_allocate_write_buffers, ktls_initialise_write_packets, - NULL + NULL, + ktls_prepare_record_header }; const OSSL_RECORD_METHOD ossl_ktls_record_method = { diff --git a/ssl/record/methods/recmethod_local.h b/ssl/record/methods/recmethod_local.h index c088f5947ba..d7c526c2d59 100644 --- a/ssl/record/methods/recmethod_local.h +++ b/ssl/record/methods/recmethod_local.h @@ -99,6 +99,12 @@ struct record_functions_st /* Get the actual record type to be used for a given template */ unsigned int (*get_record_type)(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *template); + + /* Write the record header data to the WPACKET */ + int (*prepare_record_header)(OSSL_RECORD_LAYER *rl, WPACKET *thispkt, + OSSL_RECORD_TEMPLATE *templ, + unsigned int rectype, + unsigned char **recdata); }; struct ossl_record_layer_st @@ -368,6 +374,9 @@ int tls_write_records_multiblock(OSSL_RECORD_LAYER *rl, size_t tls_get_max_records_default(OSSL_RECORD_LAYER *rl, int type, size_t len, size_t maxfrag, size_t *preffrag); +size_t tls_get_max_records_multiblock(OSSL_RECORD_LAYER *rl, int type, + size_t len, size_t maxfrag, + size_t *preffrag); int tls_allocate_write_buffers_default(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates, size_t numtempl, size_t *prefix); @@ -388,9 +397,11 @@ int tls1_initialise_write_packets(OSSL_RECORD_LAYER *rl, WPACKET *pkt, SSL3_BUFFER *bufs, size_t *wpinited); -size_t tls_get_max_records_multiblock(OSSL_RECORD_LAYER *rl, int type, - size_t len, size_t maxfrag, - size_t *preffrag); +int tls_prepare_record_header_default(OSSL_RECORD_LAYER *rl, + WPACKET *thispkt, + OSSL_RECORD_TEMPLATE *templ, + unsigned int rectype, + unsigned char **recdata); int tls_write_records_default(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates, size_t numtempl); diff --git a/ssl/record/methods/ssl3_meth.c b/ssl/record/methods/ssl3_meth.c index 6b932246930..6803cb5975c 100644 --- a/ssl/record/methods/ssl3_meth.c +++ b/ssl/record/methods/ssl3_meth.c @@ -314,5 +314,6 @@ struct record_functions_st ssl_3_0_funcs = { /* These 2 functions are defined in tls1_meth.c */ tls1_allocate_write_buffers, tls1_initialise_write_packets, - NULL + NULL, + tls_prepare_record_header_default }; diff --git a/ssl/record/methods/tls13_meth.c b/ssl/record/methods/tls13_meth.c index 5044778e3b8..b388ab86faf 100644 --- a/ssl/record/methods/tls13_meth.c +++ b/ssl/record/methods/tls13_meth.c @@ -266,5 +266,6 @@ struct record_functions_st tls_1_3_funcs = { tls_write_records_default, tls_allocate_write_buffers_default, tls_initialise_write_packets_default, - tls13_get_record_type + tls13_get_record_type, + tls_prepare_record_header_default }; diff --git a/ssl/record/methods/tls1_meth.c b/ssl/record/methods/tls1_meth.c index 7ea4886926f..56d015f71fa 100644 --- a/ssl/record/methods/tls1_meth.c +++ b/ssl/record/methods/tls1_meth.c @@ -655,7 +655,8 @@ struct record_functions_st tls_1_funcs = { tls_write_records_multiblock, /* Defined in tls_multib.c */ tls1_allocate_write_buffers, tls1_initialise_write_packets, - NULL + NULL, + tls_prepare_record_header_default }; struct record_functions_st dtls_1_funcs = { diff --git a/ssl/record/methods/tls_common.c b/ssl/record/methods/tls_common.c index 8594b3d855b..27a97d287ca 100644 --- a/ssl/record/methods/tls_common.c +++ b/ssl/record/methods/tls_common.c @@ -1533,6 +1533,35 @@ int tls_initialise_write_packets_default(OSSL_RECORD_LAYER *rl, return 1; } +int tls_prepare_record_header_default(OSSL_RECORD_LAYER *rl, + WPACKET *thispkt, + OSSL_RECORD_TEMPLATE *templ, + unsigned int rectype, + unsigned char **recdata) +{ + size_t maxcomplen; + + *recdata = NULL; + + maxcomplen = templ->buflen; + if (rl->compctx != NULL) + maxcomplen += SSL3_RT_MAX_COMPRESSED_OVERHEAD; + + if (!WPACKET_put_bytes_u8(thispkt, rectype) + || !WPACKET_put_bytes_u16(thispkt, templ->version) + || !WPACKET_start_sub_packet_u16(thispkt) + || (rl->eivlen > 0 + && !WPACKET_allocate_bytes(thispkt, rl->eivlen, NULL)) + || (maxcomplen > 0 + && !WPACKET_reserve_bytes(thispkt, maxcomplen, + recdata))) { + RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + return 0; + } + + return 1; +} + int tls_write_records_default(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates, size_t numtempl) @@ -1579,7 +1608,6 @@ int tls_write_records_default(OSSL_RECORD_LAYER *rl, memset(wr, 0, sizeof(wr)); for (j = 0; j < numtempl + prefix; j++) { unsigned char *compressdata = NULL; - size_t maxcomplen; unsigned int rectype; thispkt = &pkt[j]; @@ -1598,23 +1626,8 @@ int tls_write_records_default(OSSL_RECORD_LAYER *rl, SSL3_RECORD_set_type(thiswr, rectype); SSL3_RECORD_set_rec_version(thiswr, thistempl->version); - maxcomplen = thistempl->buflen; - if (rl->compctx != NULL) - maxcomplen += SSL3_RT_MAX_COMPRESSED_OVERHEAD; - - /* - * When using offload kernel will write the header. - * Otherwise write the header now - */ - if (!using_ktls - && (!WPACKET_put_bytes_u8(thispkt, rectype) - || !WPACKET_put_bytes_u16(thispkt, thistempl->version) - || !WPACKET_start_sub_packet_u16(thispkt) - || (rl->eivlen > 0 - && !WPACKET_allocate_bytes(thispkt, rl->eivlen, NULL)) - || (maxcomplen > 0 - && !WPACKET_reserve_bytes(thispkt, maxcomplen, - &compressdata)))) { + if (!rl->funcs->prepare_record_header(rl, thispkt, thistempl, rectype, + &compressdata)) { RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); goto err; } @@ -1637,16 +1650,12 @@ int tls_write_records_default(OSSL_RECORD_LAYER *rl, RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, SSL_R_COMPRESSION_FAILURE); goto err; } - } else { - if (using_ktls) { - SSL3_RECORD_reset_data(&wr[j]); - } else { - if (!WPACKET_memcpy(thispkt, thiswr->input, thiswr->length)) { - RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - SSL3_RECORD_reset_input(&wr[j]); + } else if (compressdata != NULL) { + if (!WPACKET_memcpy(thispkt, thiswr->input, thiswr->length)) { + RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + goto err; } + SSL3_RECORD_reset_input(&wr[j]); } if (rl->version == TLS1_3_VERSION diff --git a/ssl/record/methods/tlsany_meth.c b/ssl/record/methods/tlsany_meth.c index 8a9075bfd0f..eaabcf20a94 100644 --- a/ssl/record/methods/tlsany_meth.c +++ b/ssl/record/methods/tlsany_meth.c @@ -147,7 +147,8 @@ struct record_functions_st tls_any_funcs = { tls_write_records_default, tls_allocate_write_buffers_default, tls_initialise_write_packets_default, - NULL + NULL, + tls_prepare_record_header_default }; static int dtls_any_set_protocol_version(OSSL_RECORD_LAYER *rl, int vers) @@ -172,5 +173,6 @@ struct record_functions_st dtls_any_funcs = { NULL, NULL, NULL, + NULL, NULL }; -- 2.47.3