diff options
author | Matt Caswell <matt@openssl.org> | 2022-10-14 15:30:55 +0100 |
---|---|---|
committer | Matt Caswell <matt@openssl.org> | 2022-10-20 14:39:33 +0100 |
commit | 4f428e86d8bc68f95446eef96129c6ad98b57104 (patch) | |
tree | 26de6cd9b749c23dd4db6f79f902c77e7dea608f | |
parent | b9e37f8f573de1951655f6d8684f2f65ffc6905b (diff) |
Ensure the record layer is responsible for calculating record overheads
Don't calculate the potential record layer expansion outside of the
record layer. We move some code that was doing that into the record
layer.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19424)
-rw-r--r-- | ssl/record/methods/dtls_meth.c | 32 | ||||
-rw-r--r-- | ssl/record/methods/ktls_meth.c | 3 | ||||
-rw-r--r-- | ssl/record/methods/tls_common.c | 3 | ||||
-rw-r--r-- | ssl/record/recordmethod.h | 6 | ||||
-rw-r--r-- | ssl/statem/statem_dtls.c | 26 |
5 files changed, 46 insertions, 24 deletions
diff --git a/ssl/record/methods/dtls_meth.c b/ssl/record/methods/dtls_meth.c index e6c71ed1e7..d219d7d02e 100644 --- a/ssl/record/methods/dtls_meth.c +++ b/ssl/record/methods/dtls_meth.c @@ -823,6 +823,35 @@ int dtls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates, return ret; } +static size_t dtls_get_max_record_overhead(OSSL_RECORD_LAYER *rl) +{ + size_t blocksize, mac_size; + + /* + * TODO(RECLAYER): Review this. This is what the existing code did. + * I suspect it's not quite right. What about IV? AEAD Tag? Compression + * expansion? + */ + if (rl->md_ctx != NULL) { + if (rl->enc_ctx != NULL + && (EVP_CIPHER_get_flags(EVP_CIPHER_CTX_get0_cipher(rl->enc_ctx)) & + EVP_CIPH_FLAG_AEAD_CIPHER) != 0) + mac_size = 0; + else + mac_size = EVP_MD_CTX_get_size(rl->md_ctx); + } else { + mac_size = 0; + } + + if (rl->enc_ctx != NULL && + (EVP_CIPHER_CTX_get_mode(rl->enc_ctx) == EVP_CIPH_CBC_MODE)) + blocksize = 2 * EVP_CIPHER_CTX_get_block_size(rl->enc_ctx); + else + blocksize = 0; + + return DTLS1_RT_HEADER_LENGTH + mac_size + blocksize; +} + const OSSL_RECORD_METHOD ossl_dtls_record_method = { dtls_new_record_layer, dtls_free, @@ -847,5 +876,6 @@ const OSSL_RECORD_METHOD ossl_dtls_record_method = { tls_get_state, tls_set_options, tls_get_compression, - tls_set_max_frag_len + tls_set_max_frag_len, + dtls_get_max_record_overhead }; diff --git a/ssl/record/methods/ktls_meth.c b/ssl/record/methods/ktls_meth.c index 14db12ad5b..ba8e2a4523 100644 --- a/ssl/record/methods/ktls_meth.c +++ b/ssl/record/methods/ktls_meth.c @@ -578,5 +578,6 @@ const OSSL_RECORD_METHOD ossl_ktls_record_method = { tls_get_state, tls_set_options, tls_get_compression, - tls_set_max_frag_len + tls_set_max_frag_len, + NULL }; diff --git a/ssl/record/methods/tls_common.c b/ssl/record/methods/tls_common.c index 238684a77b..fe2620a8ea 100644 --- a/ssl/record/methods/tls_common.c +++ b/ssl/record/methods/tls_common.c @@ -2037,5 +2037,6 @@ const OSSL_RECORD_METHOD ossl_tls_record_method = { tls_get_state, tls_set_options, tls_get_compression, - tls_set_max_frag_len + tls_set_max_frag_len, + NULL }; diff --git a/ssl/record/recordmethod.h b/ssl/record/recordmethod.h index a60b2470fa..36ba59e3c2 100644 --- a/ssl/record/recordmethod.h +++ b/ssl/record/recordmethod.h @@ -309,6 +309,12 @@ struct ossl_record_method_st { * setting during construction of the record layer. */ void (*set_max_frag_len)(OSSL_RECORD_LAYER *rl, size_t max_frag_len); + + /* + * The maximum expansion in bytes that the record layer might add while + * writing a record + */ + size_t (*get_max_record_overhead)(OSSL_RECORD_LAYER *rl); }; diff --git a/ssl/statem/statem_dtls.c b/ssl/statem/statem_dtls.c index b673c860ab..2e71014ef8 100644 --- a/ssl/statem/statem_dtls.c +++ b/ssl/statem/statem_dtls.c @@ -116,7 +116,7 @@ int dtls1_do_write(SSL_CONNECTION *s, int type) size_t written; size_t curr_mtu; int retry = 1; - size_t len, frag_off, mac_size, blocksize, used_len; + size_t len, frag_off, overhead, used_len; SSL *ssl = SSL_CONNECTION_GET_SSL(s); if (!dtls1_query_mtu(s)) @@ -132,21 +132,7 @@ int dtls1_do_write(SSL_CONNECTION *s, int type) return -1; } - if (s->write_hash) { - if (s->enc_write_ctx - && (EVP_CIPHER_get_flags(EVP_CIPHER_CTX_get0_cipher(s->enc_write_ctx)) & - EVP_CIPH_FLAG_AEAD_CIPHER) != 0) - mac_size = 0; - else - mac_size = EVP_MD_CTX_get_size(s->write_hash); - } else - mac_size = 0; - - if (s->enc_write_ctx && - (EVP_CIPHER_CTX_get_mode(s->enc_write_ctx) == EVP_CIPH_CBC_MODE)) - blocksize = 2 * EVP_CIPHER_CTX_get_block_size(s->enc_write_ctx); - else - blocksize = 0; + overhead = s->rlayer.wrlmethod->get_max_record_overhead(s->rlayer.wrl); frag_off = 0; s->rwstate = SSL_NOTHING; @@ -187,8 +173,7 @@ int dtls1_do_write(SSL_CONNECTION *s, int type) } } - used_len = BIO_wpending(s->wbio) + DTLS1_RT_HEADER_LENGTH - + mac_size + blocksize; + used_len = BIO_wpending(s->wbio) + overhead; if (s->d1->mtu > used_len) curr_mtu = s->d1->mtu - used_len; else @@ -203,9 +188,8 @@ int dtls1_do_write(SSL_CONNECTION *s, int type) s->rwstate = SSL_WRITING; return ret; } - used_len = DTLS1_RT_HEADER_LENGTH + mac_size + blocksize; - if (s->d1->mtu > used_len + DTLS1_HM_HEADER_LENGTH) { - curr_mtu = s->d1->mtu - used_len; + if (s->d1->mtu > overhead + DTLS1_HM_HEADER_LENGTH) { + curr_mtu = s->d1->mtu - overhead; } else { /* Shouldn't happen */ return -1; |