From: Matt Caswell Date: Wed, 31 Aug 2022 15:39:36 +0000 (+0100) Subject: Move the record block_padding capability fully into the record layer X-Git-Tag: openssl-3.2.0-alpha1~2028 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=eb7d6c2a9b3b9d1582e3e1b65c9d431cf3209207;p=thirdparty%2Fopenssl.git Move the record block_padding capability fully into the record layer Previously we were referencing the block_padding value through the SSL_CONNECTION. Now it is held within OSSL_RECORD_LAYER. Reviewed-by: Hugo Landau Reviewed-by: Richard Levitte (Merged from https://github.com/openssl/openssl/pull/19198) --- diff --git a/include/openssl/core_names.h b/include/openssl/core_names.h index 487b928e44e..d0b9c37bc5a 100644 --- a/include/openssl/core_names.h +++ b/include/openssl/core_names.h @@ -572,6 +572,7 @@ extern "C" { #define OSSL_LIBSSL_RECORD_LAYER_PARAM_TLSTREE "tlstree" #define OSSL_LIBSSL_RECORD_LAYER_PARAM_MAX_FRAG_LEN "max_frag_len" #define OSSL_LIBSSL_RECORD_LAYER_PARAM_MAX_EARLY_DATA "max_early_data" +#define OSSL_LIBSSL_RECORD_LAYER_PARAM_BLOCK_PADDING "block_padding" # ifdef __cplusplus } diff --git a/ssl/record/methods/ktls_meth.c b/ssl/record/methods/ktls_meth.c index 663dbd1d37b..7f6c9602f0c 100644 --- a/ssl/record/methods/ktls_meth.c +++ b/ssl/record/methods/ktls_meth.c @@ -404,15 +404,14 @@ static int ktls_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, if (!ktls_int_check_supported_cipher(rl, ciph, md, taglen)) return OSSL_RECORD_RETURN_NON_FATAL_ERR; - /* - * TODO(RECLAYER): For the write side we need to add a check for - * use of s->record_padding_cb - */ - /* All future data will get encrypted by ktls. Flush the BIO or skip ktls */ if (rl->direction == OSSL_RECORD_DIRECTION_WRITE) { - if (BIO_flush(rl->bio) <= 0) - return OSSL_RECORD_RETURN_NON_FATAL_ERR; + if (BIO_flush(rl->bio) <= 0) + return OSSL_RECORD_RETURN_NON_FATAL_ERR; + + /* KTLS does not support record padding */ + if (rl->padding != NULL || rl->block_padding > 0) + return OSSL_RECORD_RETURN_NON_FATAL_ERR; } if (!ktls_configure_crypto(rl->libctx, rl->version, ciph, md, rl->sequence, diff --git a/ssl/record/methods/recmethod_local.h b/ssl/record/methods/recmethod_local.h index 5fa451d916f..850c7ae8fa6 100644 --- a/ssl/record/methods/recmethod_local.h +++ b/ssl/record/methods/recmethod_local.h @@ -167,6 +167,9 @@ struct ossl_record_layer_st /* The amount of early data that we have sent/received */ size_t early_data_count; + /* TLSv1.3 record padding */ + size_t block_padding; + /* Only used by SSLv3 */ unsigned char mac_secret[EVP_MAX_MD_SIZE]; diff --git a/ssl/record/methods/tls_common.c b/ssl/record/methods/tls_common.c index e3cc6c14dc9..85b5bcd483b 100644 --- a/ssl/record/methods/tls_common.c +++ b/ssl/record/methods/tls_common.c @@ -1101,11 +1101,20 @@ int tls_set_options(OSSL_RECORD_LAYER *rl, const OSSL_PARAM *options) return 0; } - p = OSSL_PARAM_locate_const(options, - OSSL_LIBSSL_RECORD_LAYER_READ_BUFFER_LEN); - if (p != NULL && !OSSL_PARAM_get_size_t(p, &rl->rbuf.default_len)) { - ERR_raise(ERR_LIB_SSL, SSL_R_FAILED_TO_GET_PARAMETER); - return 0; + if (rl->direction == OSSL_RECORD_DIRECTION_READ) { + p = OSSL_PARAM_locate_const(options, + OSSL_LIBSSL_RECORD_LAYER_READ_BUFFER_LEN); + if (p != NULL && !OSSL_PARAM_get_size_t(p, &rl->rbuf.default_len)) { + ERR_raise(ERR_LIB_SSL, SSL_R_FAILED_TO_GET_PARAMETER); + return 0; + } + } else { + p = OSSL_PARAM_locate_const(options, + OSSL_LIBSSL_RECORD_LAYER_PARAM_BLOCK_PADDING); + if (p != NULL && !OSSL_PARAM_get_size_t(p, &rl->block_padding)) { + ERR_raise(ERR_LIB_SSL, SSL_R_FAILED_TO_GET_PARAMETER); + return 0; + } } if (rl->level == OSSL_RECORD_PROTECTION_LEVEL_APPLICATION) { @@ -1666,20 +1675,20 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates, if (rl->padding != NULL) { padding = rl->padding(rl->cbarg, thistempl->type, rlen); - } else if (s->block_padding > 0) { - size_t mask = s->block_padding - 1; + } else if (rl->block_padding > 0) { + size_t mask = rl->block_padding - 1; size_t remainder; /* optimize for power of 2 */ - if ((s->block_padding & mask) == 0) + if ((rl->block_padding & mask) == 0) remainder = rlen & mask; else - remainder = rlen % s->block_padding; + remainder = rlen % rl->block_padding; /* don't want to add a block of padding if we don't have to */ if (remainder == 0) padding = 0; else - padding = s->block_padding - remainder; + padding = rl->block_padding - remainder; } if (padding > 0) { /* do not allow the record to exceed max plaintext length */ diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c index 6d0251407ff..9152ea86aac 100644 --- a/ssl/record/rec_layer_s3.c +++ b/ssl/record/rec_layer_s3.c @@ -1198,7 +1198,8 @@ static size_t rlayer_padding_wrapper(void *cbarg, int type, size_t len) SSL_CONNECTION *s = cbarg; SSL *ssl = SSL_CONNECTION_GET_SSL(s); - return s->record_padding_cb(ssl, type, len, s->record_padding_arg); + return s->rlayer.record_padding_cb(ssl, type, len, + s->rlayer.record_padding_arg); } static const OSSL_DISPATCH rlayer_dispatch[] = { @@ -1315,6 +1316,9 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version, &s->rlayer.default_read_buf_len); *opts++ = OSSL_PARAM_construct_int(OSSL_LIBSSL_RECORD_LAYER_PARAM_READ_AHEAD, &s->rlayer.read_ahead); + } else { + *opts++ = OSSL_PARAM_construct_size_t(OSSL_LIBSSL_RECORD_LAYER_PARAM_BLOCK_PADDING, + &s->rlayer.block_padding); } *opts = OSSL_PARAM_construct_end(); @@ -1414,7 +1418,7 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version, continue; break; case OSSL_FUNC_RLAYER_PADDING: - if (s->record_padding_cb == NULL) + if (s->rlayer.record_padding_cb == NULL) continue; break; default: diff --git a/ssl/record/record.h b/ssl/record/record.h index 19021ca0135..03151e58d60 100644 --- a/ssl/record/record.h +++ b/ssl/record/record.h @@ -174,6 +174,11 @@ typedef struct record_layer_st { unsigned int alert_count; DTLS_RECORD_LAYER *d; + /* TLS1.3 padding callback */ + size_t (*record_padding_cb)(SSL *s, int type, size_t len, void *arg); + void *record_padding_arg; + size_t block_padding; + /* How many records we have read from the record layer */ size_t num_recs; /* The next record from the record layer that we need to process */ diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index f77bd7b2799..35758df33bf 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -797,9 +797,9 @@ SSL *ossl_ssl_connection_new(SSL_CTX *ctx) s->msg_callback_arg = ctx->msg_callback_arg; s->verify_mode = ctx->verify_mode; s->not_resumable_session_cb = ctx->not_resumable_session_cb; - s->record_padding_cb = ctx->record_padding_cb; - s->record_padding_arg = ctx->record_padding_arg; - s->block_padding = ctx->block_padding; + s->rlayer.record_padding_cb = ctx->record_padding_cb; + s->rlayer.record_padding_arg = ctx->record_padding_arg; + s->rlayer.block_padding = ctx->block_padding; s->sid_ctx_length = ctx->sid_ctx_length; if (!ossl_assert(s->sid_ctx_length <= sizeof(s->sid_ctx))) goto err; @@ -5397,7 +5397,7 @@ int SSL_set_record_padding_callback(SSL *ssl, b = SSL_get_wbio(ssl); if (b == NULL || !BIO_get_ktls_send(b)) { - sc->record_padding_cb = cb; + sc->rlayer.record_padding_cb = cb; return 1; } return 0; @@ -5410,7 +5410,7 @@ void SSL_set_record_padding_callback_arg(SSL *ssl, void *arg) if (sc == NULL) return; - sc->record_padding_arg = arg; + sc->rlayer.record_padding_arg = arg; } void *SSL_get_record_padding_callback_arg(const SSL *ssl) @@ -5420,7 +5420,7 @@ void *SSL_get_record_padding_callback_arg(const SSL *ssl) if (sc == NULL) return NULL; - return sc->record_padding_arg; + return sc->rlayer.record_padding_arg; } int SSL_set_block_padding(SSL *ssl, size_t block_size) @@ -5432,9 +5432,9 @@ int SSL_set_block_padding(SSL *ssl, size_t block_size) /* block size of 0 or 1 is basically no padding */ if (block_size == 1) - sc->block_padding = 0; + sc->rlayer.block_padding = 0; else if (block_size <= SSL3_RT_MAX_PLAIN_LENGTH) - sc->block_padding = block_size; + sc->rlayer.block_padding = block_size; else return 0; return 1; diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h index 61b77602bac..90e6209f777 100644 --- a/ssl/ssl_local.h +++ b/ssl/ssl_local.h @@ -1793,11 +1793,6 @@ struct ssl_connection_st { */ uint32_t early_data_count; - /* TLS1.3 padding callback */ - size_t (*record_padding_cb)(SSL *s, int type, size_t len, void *arg); - void *record_padding_arg; - size_t block_padding; - /* The number of TLS1.3 tickets to automatically send */ size_t num_tickets; /* The number of TLS1.3 tickets actually sent so far */ diff --git a/ssl/tls13_enc.c b/ssl/tls13_enc.c index 1e8e189b7c9..431a7fff708 100644 --- a/ssl/tls13_enc.c +++ b/ssl/tls13_enc.c @@ -735,7 +735,7 @@ int tls13_change_cipher_state(SSL_CONNECTION *s, int which) goto skip_ktls; /* ktls does not support record padding */ - if (s->record_padding_cb != NULL) + if (s->rlayer.record_padding_cb != NULL || s->rlayer.block_padding > 0) goto skip_ktls; /* check that cipher is supported */