Skip to content

Commit

Permalink
Remove use of SSL object for fragment length checking in record layer
Browse files Browse the repository at this point in the history
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 <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #18132)
  • Loading branch information
mattcaswell committed Aug 18, 2022
1 parent 651216d commit ffbd6e6
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 14 deletions.
9 changes: 5 additions & 4 deletions include/openssl/core_names.h
Expand Up @@ -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
}
Expand Down
8 changes: 8 additions & 0 deletions ssl/record/methods/ktls_meth.c
Expand Up @@ -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))
Expand Down
3 changes: 3 additions & 0 deletions ssl/record/methods/recmethod_local.h
Expand Up @@ -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];

Expand Down
19 changes: 10 additions & 9 deletions ssl/record/methods/tls_common.c
Expand Up @@ -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;
Expand All @@ -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];
Expand Down Expand Up @@ -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++) {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down
11 changes: 10 additions & 1 deletion ssl/record/rec_layer_s3.c
Expand Up @@ -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);

Expand Down Expand Up @@ -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 (;;) {
Expand Down

0 comments on commit ffbd6e6

Please sign in to comment.