From: Matt Caswell Date: Mon, 23 May 2022 10:31:53 +0000 (+0100) Subject: Remove use of SSL object for fragment length checking in record layer X-Git-Tag: openssl-3.2.0-alpha1~2233 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ffbd6e67874475e025e942e0ee9f51badfea42b5;p=thirdparty%2Fopenssl.git Remove use of SSL object for fragment length checking in record layer Pass the max fragment length to the record layer when it is applicable to avoid the need to go through the SSL object. Reviewed-by: Hugo Landau Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/18132) --- diff --git a/include/openssl/core_names.h b/include/openssl/core_names.h index aadce3c034d..2ce0a19c582 100644 --- a/include/openssl/core_names.h +++ b/include/openssl/core_names.h @@ -558,10 +558,11 @@ extern "C" { /* Libssl record layer */ -#define OSSL_LIBSSL_RECORD_LAYER_PARAM_OPTIONS "options" -#define OSSL_LIBSSL_RECORD_LAYER_PARAM_MODE "mode" -#define OSSL_LIBSSL_RECORD_LAYER_PARAM_READ_AHEAD "read_ahead" -#define OSSL_LIBSSL_RECORD_LAYER_PARAM_USE_ETM "use_etm" +#define OSSL_LIBSSL_RECORD_LAYER_PARAM_OPTIONS "options" +#define OSSL_LIBSSL_RECORD_LAYER_PARAM_MODE "mode" +#define OSSL_LIBSSL_RECORD_LAYER_PARAM_READ_AHEAD "read_ahead" +#define OSSL_LIBSSL_RECORD_LAYER_PARAM_USE_ETM "use_etm" +#define OSSL_LIBSSL_RECORD_LAYER_PARAM_MAX_FRAG_LEN "max_frag_len" # ifdef __cplusplus } diff --git a/ssl/record/methods/ktls_meth.c b/ssl/record/methods/ktls_meth.c index dbebb8acf53..e608b530ff4 100644 --- a/ssl/record/methods/ktls_meth.c +++ b/ssl/record/methods/ktls_meth.c @@ -462,8 +462,16 @@ static int ktls_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, return OSSL_RECORD_RETURN_NON_FATAL_ERR; /* ktls supports only the maximum fragment size */ + if (rl->max_frag_len > 0 && rl->max_frag_len != SSL3_RT_MAX_PLAIN_LENGTH) + return OSSL_RECORD_RETURN_NON_FATAL_ERR; +#if 0 + /* + * TODO(RECLAYER): We will need to reintroduce the check of the send + * fragment for KTLS once we do the record write side implementation + */ if (ssl_get_max_send_fragment(s) != SSL3_RT_MAX_PLAIN_LENGTH) return OSSL_RECORD_RETURN_NON_FATAL_ERR; +#endif /* check that cipher is supported */ if (!ktls_int_check_supported_cipher(rl, ciph, md, taglen)) diff --git a/ssl/record/methods/recmethod_local.h b/ssl/record/methods/recmethod_local.h index 56fd278e76c..6a7d63fd0f9 100644 --- a/ssl/record/methods/recmethod_local.h +++ b/ssl/record/methods/recmethod_local.h @@ -136,6 +136,9 @@ struct ossl_record_layer_st /* Set to 1 if this is the first handshake. 0 otherwise */ int is_first_handshake; + /* The negotiated maximum fragment length */ + unsigned int max_frag_len; + /* 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 83111270039..14f40dbb9c1 100644 --- a/ssl/record/methods/tls_common.c +++ b/ssl/record/methods/tls_common.c @@ -413,7 +413,6 @@ static int tls_get_more_records(OSSL_RECORD_LAYER *rl, size_t more, n; SSL3_RECORD *rr, *thisrr; SSL3_BUFFER *rbuf; - SSL_SESSION *sess; unsigned char *p; unsigned char md[EVP_MAX_MD_SIZE]; unsigned int version; @@ -437,7 +436,6 @@ static int tls_get_more_records(OSSL_RECORD_LAYER *rl, max_recs = s->max_pipelines; if (max_recs == 0) max_recs = 1; - sess = s->session; do { thisrr = &rr[num_recs]; @@ -740,9 +738,9 @@ static int tls_get_more_records(OSSL_RECORD_LAYER *rl, } OSSL_TRACE_END(TLS); /* r->length is now the compressed data plus mac */ - if ((sess != NULL) - && (rl->enc_read_ctx != NULL) - && (!rl->use_etm && EVP_MD_CTX_get0_md(rl->read_hash) != NULL)) { + if (rl->enc_read_ctx != NULL + && !rl->use_etm + && EVP_MD_CTX_get0_md(rl->read_hash) != NULL) { /* rl->read_hash != NULL => mac_size != -1 */ for (j = 0; j < num_recs; j++) { @@ -786,10 +784,9 @@ static int tls_get_more_records(OSSL_RECORD_LAYER *rl, /* * Check if the received packet overflows the current * Max Fragment Length setting. - * Note: USE_MAX_FRAGMENT_LENGTH_EXT and KTLS are mutually exclusive. + * Note: rl->max_frag_len > 0 and KTLS are mutually exclusive. */ - if (s->session != NULL && USE_MAX_FRAGMENT_LENGTH_EXT(s->session) - && thisrr->length > GET_MAX_FRAGMENT_LENGTH(s->session)) { + if (rl->max_frag_len > 0 && thisrr->length > rl->max_frag_len) { RLAYERfatal(rl, SSL_AD_RECORD_OVERFLOW, SSL_R_DATA_LENGTH_TOO_LONG); goto end; } @@ -1050,7 +1047,11 @@ tls_int_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers, RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, SSL_R_FAILED_TO_GET_PARAMETER); goto err; } - break; + } else if (strcmp(p->key, OSSL_LIBSSL_RECORD_LAYER_PARAM_MAX_FRAG_LEN) == 0) { + if (!OSSL_PARAM_get_uint(p, &rl->max_frag_len)) { + RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, SSL_R_FAILED_TO_GET_PARAMETER); + goto err; + } } else { RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, SSL_R_UNKNOWN_MANDATORY_PARAMETER); goto err; diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c index 6f40b73c2b1..501fba8afbb 100644 --- a/ssl/record/rec_layer_s3.c +++ b/ssl/record/rec_layer_s3.c @@ -1797,11 +1797,12 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version, const SSL_COMP *comp) { OSSL_PARAM options[4], *opts = options; - OSSL_PARAM settings[2], *set = settings; + OSSL_PARAM settings[3], *set = settings; const OSSL_RECORD_METHOD *origmeth = s->rrlmethod; SSL_CTX *sctx = SSL_CONNECTION_GET_CTX(s); const OSSL_RECORD_METHOD *meth; int use_etm; + unsigned int maxfrag = SSL3_RT_MAX_PLAIN_LENGTH; meth = ssl_select_next_record_layer(s, level); @@ -1836,6 +1837,14 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version, if (use_etm) *set++ = OSSL_PARAM_construct_int(OSSL_LIBSSL_RECORD_LAYER_PARAM_USE_ETM, &use_etm); + + if (s->session != NULL && USE_MAX_FRAGMENT_LENGTH_EXT(s->session)) + maxfrag = GET_MAX_FRAGMENT_LENGTH(s->session); + + if (maxfrag != SSL3_RT_MAX_PLAIN_LENGTH) + *set++ = OSSL_PARAM_construct_uint(OSSL_LIBSSL_RECORD_LAYER_PARAM_MAX_FRAG_LEN, + &maxfrag); + *set = OSSL_PARAM_construct_end(); for (;;) {