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)

diff --git a/ssl/record/methods/dtls_meth.c b/ssl/record/methods/dtls_meth.c
index e6c71ed1e7b9b5c05d8b30144951254161d45f7e..d219d7d02e0199bb37c73aece5fb577cd27b3a3b 100644 (file)
--- 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 14db12ad5b6620161fc5145f12e8f159caacd7cd..ba8e2a4523f5ab048a654cff6dc7b2b0abe45444 100644 (file)
--- 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 238684a77b25f4063eb9483620b7a7e216286675..fe2620a8ea2a8acd0d8577e7cc97bf78f99cf6d9 100644 (file)
--- 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 a60b2470fa8bacd6b84088ccad8df59fa175e95d..36ba59e3c20daab9629fe43bf15fc032e89e278e 100644 (file)
--- 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 b673c860abc2c5c3525d5d1db680f958109f7812..2e71014ef8aced4c4ef7ea014016b993633f75af 100644 (file)
--- 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;