From 9dd90232d537f0ccd457fe1e23f4cbe83917c70a Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 25 May 2022 15:16:48 +0100 Subject: [PATCH] Move early data counting out of the SSL object and into the record layer Reviewed-by: Hugo Landau Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/18132) --- include/openssl/core_names.h | 11 ++--- ssl/record/methods/ktls_meth.c | 3 +- ssl/record/methods/recmethod_local.h | 13 +++++- ssl/record/methods/tls_common.c | 66 +++++++++++++++++++++++----- ssl/record/rec_layer_s3.c | 32 +++++++++++++- ssl/record/record.h | 4 ++ ssl/record/record_local.h | 5 ++- ssl/record/recordmethod.h | 2 + ssl/record/ssl3_record.c | 18 +++++--- 9 files changed, 128 insertions(+), 26 deletions(-) diff --git a/include/openssl/core_names.h b/include/openssl/core_names.h index 2ce0a19c5820b..82ad706eb7b8f 100644 --- a/include/openssl/core_names.h +++ b/include/openssl/core_names.h @@ -558,11 +558,12 @@ 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_MAX_FRAG_LEN "max_frag_len" +#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" +#define OSSL_LIBSSL_RECORD_LAYER_PARAM_MAX_EARLY_DATA "max_early_data" # ifdef __cplusplus } diff --git a/ssl/record/methods/ktls_meth.c b/ssl/record/methods/ktls_meth.c index a396c111e59fa..e83cda603bd86 100644 --- a/ssl/record/methods/ktls_meth.c +++ b/ssl/record/methods/ktls_meth.c @@ -574,6 +574,7 @@ ktls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers, const EVP_MD *md, const SSL_COMP *comp, BIO *prev, BIO *transport, BIO *next, BIO_ADDR *local, BIO_ADDR *peer, const OSSL_PARAM *settings, const OSSL_PARAM *options, + const OSSL_DISPATCH *fns, void *cbarg, OSSL_RECORD_LAYER **retrl, /* TODO(RECLAYER): Remove me */ SSL_CONNECTION *s) @@ -584,7 +585,7 @@ ktls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers, key, keylen, iv, ivlen, mackey, mackeylen, ciph, taglen, mactype, md, comp, prev, transport, next, local, peer, settings, - options, retrl, s); + options, fns, cbarg, retrl, s); if (ret != OSSL_RECORD_RETURN_SUCCESS) return ret; diff --git a/ssl/record/methods/recmethod_local.h b/ssl/record/methods/recmethod_local.h index 74d8f0132865e..f6c2e4b682ffb 100644 --- a/ssl/record/methods/recmethod_local.h +++ b/ssl/record/methods/recmethod_local.h @@ -68,6 +68,7 @@ struct ossl_record_layer_st int version; int role; int direction; + int level; /* * A BIO containing any data read in the previous epoch that was destined @@ -142,6 +143,12 @@ struct ossl_record_layer_st /* The negotiated maximum fragment length */ unsigned int max_frag_len; + /* The maxium amount of early data we can receive/send */ + uint32_t max_early_data; + + /* The amount of early data that we have sent/received */ + size_t early_data_count; + /* Only used by SSLv3 */ unsigned char mac_secret[EVP_MAX_MD_SIZE]; @@ -161,7 +168,10 @@ struct ossl_record_layer_st size_t taglen; - /* Function pointers for version specific functions */ + /* Callbacks */ + void *cbarg; + OSSL_FUNC_rlayer_skip_early_data_fn *rlayer_skip_early_data; + /* Function pointers for version specific functions */ struct record_functions_st *funcs; }; @@ -220,6 +230,7 @@ tls_int_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers, BIO *transport, BIO *next, BIO_ADDR *local, BIO_ADDR *peer, const OSSL_PARAM *settings, const OSSL_PARAM *options, + const OSSL_DISPATCH *fns, void *cbarg, OSSL_RECORD_LAYER **retrl, /* TODO(RECLAYER): Remove me */ SSL_CONNECTION *s); diff --git a/ssl/record/methods/tls_common.c b/ssl/record/methods/tls_common.c index 27b1089092943..0f6aaebacadb7 100644 --- a/ssl/record/methods/tls_common.c +++ b/ssl/record/methods/tls_common.c @@ -383,6 +383,30 @@ static int tls_record_app_data_waiting(OSSL_RECORD_LAYER *rl) return 1; } +static int rlayer_early_data_count_ok(OSSL_RECORD_LAYER *rl, size_t length, + size_t overhead, int send) +{ + uint32_t max_early_data = rl->max_early_data; + + if (max_early_data == 0) { + RLAYERfatal(rl, send ? SSL_AD_INTERNAL_ERROR : SSL_AD_UNEXPECTED_MESSAGE, + SSL_R_TOO_MUCH_EARLY_DATA); + return 0; + } + + /* If we are dealing with ciphertext we need to allow for the overhead */ + max_early_data += overhead; + + if (rl->early_data_count + length > max_early_data) { + RLAYERfatal(rl, send ? SSL_AD_INTERNAL_ERROR : SSL_AD_UNEXPECTED_MESSAGE, + SSL_R_TOO_MUCH_EARLY_DATA); + return 0; + } + rl->early_data_count += length; + + return 1; +} + /* * MAX_EMPTY_RECORDS defines the number of consecutive, empty records that * will be processed per call to ssl3_get_record. Without this limit an @@ -705,7 +729,7 @@ static int tls_get_more_records(OSSL_RECORD_LAYER *rl, /* RLAYERfatal() already got called */ goto end; } - if (num_recs == 1 && ossl_statem_skip_early_data(s)) { + if (num_recs == 1 && rl->rlayer_skip_early_data(rl->cbarg)) { /* * Valid early_data that we cannot decrypt will fail here. We treat * it like an empty record. @@ -713,9 +737,9 @@ static int tls_get_more_records(OSSL_RECORD_LAYER *rl, thisrr = &rr[0]; - if (!ossl_early_data_count_ok(s, thisrr->length, - EARLY_DATA_CIPHERTEXT_OVERHEAD, 0)) { - /* SSLfatal() already called */ + if (!rlayer_early_data_count_ok(rl, thisrr->length, + EARLY_DATA_CIPHERTEXT_OVERHEAD, 0)) { + /* RLAYERfatal() already called */ goto end; } @@ -812,11 +836,11 @@ static int tls_get_more_records(OSSL_RECORD_LAYER *rl, } } - if (s->early_data_state == SSL_EARLY_DATA_READING) { + if (rl->level == OSSL_RECORD_PROTECTION_LEVEL_EARLY) { thisrr = &rr[0]; if (thisrr->type == SSL3_RT_APPLICATION_DATA - && !ossl_early_data_count_ok(s, thisrr->length, 0, 0)) { - /* SSLfatal already called */ + && !rlayer_early_data_count_ok(rl, thisrr->length, 0, 0)) { + /* RLAYERfatal already called */ goto end; } } @@ -1011,7 +1035,9 @@ tls_int_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers, const EVP_MD *md, const SSL_COMP *comp, BIO *prev, BIO *transport, BIO *next, BIO_ADDR *local, BIO_ADDR *peer, const OSSL_PARAM *settings, - const OSSL_PARAM *options, OSSL_RECORD_LAYER **retrl, + const OSSL_PARAM *options, + const OSSL_DISPATCH *fns, void *cbarg, + OSSL_RECORD_LAYER **retrl, /* TODO(RECLAYER): Remove me */ SSL_CONNECTION *s) { @@ -1053,6 +1079,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; } + } else if (strcmp(p->key, OSSL_LIBSSL_RECORD_LAYER_PARAM_MAX_EARLY_DATA) == 0) { + if (!OSSL_PARAM_get_uint32(p, &rl->max_early_data)) { + 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; @@ -1084,6 +1115,7 @@ tls_int_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers, rl->version = vers; rl->role = role; rl->direction = direction; + rl->level = level; if (level == OSSL_RECORD_PROTECTION_LEVEL_NONE) rl->is_first_record = 1; @@ -1099,6 +1131,18 @@ tls_int_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers, goto err; rl->next = next; + rl->cbarg = cbarg; + for (; fns->function_id != 0; fns++) { + switch (fns->function_id) { + case OSSL_FUNC_RLAYER_SKIP_EARLY_DATA: + rl->rlayer_skip_early_data = OSSL_FUNC_rlayer_skip_early_data(fns); + break; + default: + /* Just ignore anything we don't understand */ + break; + } + } + *retrl = rl; return OSSL_RECORD_RETURN_SUCCESS; err: @@ -1117,6 +1161,7 @@ tls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers, const EVP_MD *md, const SSL_COMP *comp, BIO *prev, BIO *transport, BIO *next, BIO_ADDR *local, BIO_ADDR *peer, const OSSL_PARAM *settings, const OSSL_PARAM *options, + const OSSL_DISPATCH *fns, void *cbarg, OSSL_RECORD_LAYER **retrl, /* TODO(RECLAYER): Remove me */ SSL_CONNECTION *s) @@ -1127,7 +1172,7 @@ tls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers, key, keylen, iv, ivlen, mackey, mackeylen, ciph, taglen, mactype, md, comp, prev, transport, next, local, peer, settings, - options, retrl, s); + options, fns, cbarg, retrl, s); if (ret != OSSL_RECORD_RETURN_SUCCESS) return ret; @@ -1192,6 +1237,7 @@ dtls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers, const EVP_MD *md, const SSL_COMP *comp, BIO *prev, BIO *transport, BIO *next, BIO_ADDR *local, BIO_ADDR *peer, const OSSL_PARAM *settings, const OSSL_PARAM *options, + const OSSL_DISPATCH *fns, void *cbarg, OSSL_RECORD_LAYER **retrl, /* TODO(RECLAYER): Remove me */ SSL_CONNECTION *s) @@ -1203,7 +1249,7 @@ dtls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers, key, keylen, iv, ivlen, mackey, mackeylen, ciph, taglen, mactype, md, comp, prev, transport, next, local, peer, settings, - options, retrl, s); + options, fns, cbarg, retrl, s); if (ret != OSSL_RECORD_RETURN_SUCCESS) return ret; diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c index 501fba8afbbc9..ccd51a6d57675 100644 --- a/ssl/record/rec_layer_s3.c +++ b/ssl/record/rec_layer_s3.c @@ -1749,6 +1749,11 @@ size_t RECORD_LAYER_get_rrec_length(RECORD_LAYER *rl) return SSL3_RECORD_get_length(&rl->rrec[0]); } +static const OSSL_DISPATCH rlayer_dispatch[] = { + { OSSL_FUNC_RLAYER_SKIP_EARLY_DATA, (void (*)(void))ossl_statem_skip_early_data }, + { 0, NULL } +}; + static const OSSL_RECORD_METHOD *ssl_select_next_record_layer(SSL_CONNECTION *s, int level) { @@ -1797,12 +1802,14 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version, const SSL_COMP *comp) { OSSL_PARAM options[4], *opts = options; - OSSL_PARAM settings[3], *set = settings; + OSSL_PARAM settings[4], *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; + int use_early_data = 0; + uint32_t max_early_data; meth = ssl_select_next_record_layer(s, level); @@ -1845,6 +1852,26 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version, *set++ = OSSL_PARAM_construct_uint(OSSL_LIBSSL_RECORD_LAYER_PARAM_MAX_FRAG_LEN, &maxfrag); + /* + * The record layer must check the amount of early data sent or received + * using the early keys. A server also needs to worry about rejected early + * data that might arrive when the handshake keys are in force. + */ + /* TODO(RECLAYER): Check this when doing the "write" record layer */ + if (s->server && direction == OSSL_RECORD_DIRECTION_READ) { + use_early_data = (level == OSSL_RECORD_PROTECTION_LEVEL_EARLY + || level == OSSL_RECORD_PROTECTION_LEVEL_HANDSHAKE); + } else if (!s->server && direction == OSSL_RECORD_DIRECTION_WRITE) { + use_early_data = (level == OSSL_RECORD_PROTECTION_LEVEL_EARLY); + } + if (use_early_data) { + max_early_data = ossl_get_max_early_data(s); + + if (max_early_data != 0) + *set++ = OSSL_PARAM_construct_uint(OSSL_LIBSSL_RECORD_LAYER_PARAM_MAX_EARLY_DATA, + &max_early_data); + } + *set = OSSL_PARAM_construct_end(); for (;;) { @@ -1865,7 +1892,8 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version, mackey, mackeylen, ciph, taglen, mactype, md, comp, prev, s->rbio, s->rrlnext, NULL, NULL, settings, - options, &s->rrl, s); + options, rlayer_dispatch, s, + &s->rrl, s); BIO_free(prev); switch (rlret) { case OSSL_RECORD_RETURN_FATAL: diff --git a/ssl/record/record.h b/ssl/record/record.h index 39b44e1682e00..7b78400794f1c 100644 --- a/ssl/record/record.h +++ b/ssl/record/record.h @@ -10,6 +10,7 @@ typedef struct ssl_connection_st SSL_CONNECTION; typedef struct ssl3_buffer_st SSL3_BUFFER; +#include #include "recordmethod.h" /***************************************************************************** @@ -290,3 +291,6 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version, int direction, const EVP_CIPHER *ciph, size_t taglen, int mactype, const EVP_MD *md, const SSL_COMP *comp); + +# define OSSL_FUNC_RLAYER_SKIP_EARLY_DATA 1 +OSSL_CORE_MAKE_FUNC(const OSSL_PARAM *, rlayer_skip_early_data,(void *cbarg)) \ No newline at end of file diff --git a/ssl/record/record_local.h b/ssl/record/record_local.h index 397cba617d518..c2b0109d8dc42 100644 --- a/ssl/record/record_local.h +++ b/ssl/record/record_local.h @@ -122,5 +122,6 @@ __owur int tls1_cbc_remove_padding_and_mac(size_t *reclen, OSSL_LIB_CTX *libctx); int dtls1_process_record(SSL_CONNECTION *s, DTLS1_BITMAP *bitmap); __owur int dtls1_get_record(SSL_CONNECTION *s); -int ossl_early_data_count_ok(SSL_CONNECTION *s, size_t length, - size_t overhead, int send); +uint32_t ossl_get_max_early_data(SSL_CONNECTION *s); +int ossl_early_data_count_ok(SSL_CONNECTION *s, size_t length, size_t overhead, + int send); diff --git a/ssl/record/recordmethod.h b/ssl/record/recordmethod.h index b86de6518afa4..2d7812fa00272 100644 --- a/ssl/record/recordmethod.h +++ b/ssl/record/recordmethod.h @@ -165,6 +165,8 @@ struct ossl_record_method_st { BIO_ADDR *peer, const OSSL_PARAM *settings, const OSSL_PARAM *options, + const OSSL_DISPATCH *fns, + void *cbarg, OSSL_RECORD_LAYER **ret, /* TODO(RECLAYER): Remove me */ SSL_CONNECTION *s); diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c index 87bfd62bd913a..42811e39c71e0 100644 --- a/ssl/record/ssl3_record.c +++ b/ssl/record/ssl3_record.c @@ -63,8 +63,7 @@ void SSL3_RECORD_set_seq_num(SSL3_RECORD *r, const unsigned char *seq_num) memcpy(r->seq_num, seq_num, SEQ_NUM_SIZE); } -int ossl_early_data_count_ok(SSL_CONNECTION *s, size_t length, - size_t overhead, int send) +uint32_t ossl_get_max_early_data(SSL_CONNECTION *s) { uint32_t max_early_data; SSL_SESSION *sess = s->session; @@ -91,6 +90,16 @@ int ossl_early_data_count_ok(SSL_CONNECTION *s, size_t length, max_early_data = s->recv_max_early_data < sess->ext.max_early_data ? s->recv_max_early_data : sess->ext.max_early_data; + return max_early_data; +} + +int ossl_early_data_count_ok(SSL_CONNECTION *s, size_t length, size_t overhead, + int send) +{ + uint32_t max_early_data; + + max_early_data = ossl_get_max_early_data(s); + if (max_early_data == 0) { SSLfatal(s, send ? SSL_AD_INTERNAL_ERROR : SSL_AD_UNEXPECTED_MESSAGE, SSL_R_TOO_MUCH_EARLY_DATA); @@ -110,8 +119,7 @@ int ossl_early_data_count_ok(SSL_CONNECTION *s, size_t length, return 1; } - -int ssl3_do_uncompress(SSL_CONNECTION *sc, SSL3_RECORD *rr) +int ssl3_do_uncompress(SSL_CONNECTION *ssl, SSL3_RECORD *rr) { #ifndef OPENSSL_NO_COMP int i; @@ -123,7 +131,7 @@ int ssl3_do_uncompress(SSL_CONNECTION *sc, SSL3_RECORD *rr) if (rr->comp == NULL) return 0; - i = COMP_expand_block(sc->expand, rr->comp, + i = COMP_expand_block(ssl->expand, rr->comp, SSL3_RT_MAX_PLAIN_LENGTH, rr->data, (int)rr->length); if (i < 0) return 0;