Fix dtls_get_max_record_overhead()

We fix dtls_get_max_record_overhead() to give a better value for the max
record overhead. We can't realistically handle the compression case so we
just ignore that.

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19516)

diff --git a/ssl/record/methods/dtls_meth.c b/ssl/record/methods/dtls_meth.c
index cb3e3057366b38d936931062df302a890d38d981..b16c400deab2522613a82cef63e7635357997c58 100644 (file)
--- a/ssl/record/methods/dtls_meth.c
+++ b/ssl/record/methods/dtls_meth.c
@@ -7,6 +7,7 @@
  * https://www.openssl.org/source/license.html
  */
 
+#include <assert.h>
 #include "../../ssl_local.h"
 #include "../record_local.h"
 #include "recmethod_local.h"
@@ -737,31 +738,35 @@ int dtls_post_encryption_processing(OSSL_RECORD_LAYER *rl,
 
 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;
-    }
+    size_t blocksize = 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;
+        blocksize = EVP_CIPHER_CTX_get_block_size(rl->enc_ctx);
 
-    return DTLS1_RT_HEADER_LENGTH + mac_size + blocksize;
+    /*
+     * If we have a cipher in place then the tag is mandatory. If the cipher is
+     * CBC mode then an explicit IV is also mandatory. If we know the digest,
+     * then we check it is consistent with the taglen. In the case of stitched
+     * ciphers or AEAD ciphers we don't now the digest (or there isn't one) so
+     * we just trust that the taglen is correct.
+     */
+    assert(rl->enc_ctx == NULL  || ((blocksize == 0 || rl->eivlen > 0)
+                                    && rl->taglen > 0));
+    assert(rl->md == NULL || (int)rl->taglen == EVP_MD_size(rl->md));
+
+    /*
+     * Record overhead consists of the record header, the explicit IV, any
+     * expansion due to cbc padding, and the mac/tag len. There could be
+     * further expansion due to compression - but we don't know what this will
+     * be without knowing the length of the data. However when this function is
+     * called we don't know what the length will be yet - so this is a catch-22.
+     * We *could* use SSL_3_RT_MAX_COMPRESSED_OVERHEAD which is an upper limit
+     * for the maximum record size. But this value is larger than our fallback
+     * MTU size - so isn't very helpful. We just ignore potential expansion
+     * due to compression.
+     */
+    return DTLS1_RT_HEADER_LENGTH + rl->eivlen + blocksize + rl->taglen;
 }
 
 const OSSL_RECORD_METHOD ossl_dtls_record_method = {

diff --git a/ssl/record/methods/recmethod_local.h b/ssl/record/methods/recmethod_local.h
index 9b85ce9e146ae684a8c96463c077c16364a080d8..80cf8fa97376d5d793c7dd68c84076e5f8e6a7a5 100644 (file)
--- a/ssl/record/methods/recmethod_local.h
+++ b/ssl/record/methods/recmethod_local.h
@@ -148,6 +148,7 @@ struct ossl_record_layer_st
     int role;
     int direction;
     int level;
+    const EVP_MD *md;
     /* DTLS only */
     uint16_t epoch;
 

diff --git a/ssl/record/methods/tls13_meth.c b/ssl/record/methods/tls13_meth.c
index 5cd40a4fe2fc27198eaaf294330b64d642162e0c..3ce52b380a70d83451898be478ec9a3fe9daf6ca 100644 (file)
--- a/ssl/record/methods/tls13_meth.c
+++ b/ssl/record/methods/tls13_meth.c
@@ -39,8 +39,6 @@ static int tls13_set_crypto_state(OSSL_RECORD_LAYER *rl, int level,
         return OSSL_RECORD_RETURN_FATAL;
     }
 
-    rl->taglen = taglen;
-
     mode = EVP_CIPHER_get_mode(ciph);
 
     if (EVP_CipherInit_ex(ciph_ctx, ciph, NULL, NULL, NULL, enc) <= 0

diff --git a/ssl/record/methods/tls_common.c b/ssl/record/methods/tls_common.c
index 8dc1bf3be052290736709d41d19f976315d2c763..ea763d93b6544439e4aa42f548392811096073dd 100644 (file)
--- a/ssl/record/methods/tls_common.c
+++ b/ssl/record/methods/tls_common.c
@@ -1237,6 +1237,8 @@ tls_int_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
     rl->role = role;
     rl->direction = direction;
     rl->level = level;
+    rl->taglen = taglen;
+    rl->md = md;
 
     rl->alert = SSL_AD_NO_ALERT;
 

diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c
index d3a5df29c2f3b75367080be78e04f2e09074e01f..026521052495fba66573ad2b6ec41807d0d68036 100644 (file)
--- a/ssl/t1_enc.c
+++ b/ssl/t1_enc.c
@@ -209,12 +209,25 @@ int tls1_change_cipher_state(SSL_CONNECTION *s, int which)
         goto err;
     }
 
-    if (EVP_CIPHER_get_mode(c) == EVP_CIPH_CCM_MODE) {
+    switch (EVP_CIPHER_get_mode(c)) {
+    case EVP_CIPH_GCM_MODE:
+        taglen = EVP_GCM_TLS_TAG_LEN;
+        break;
+    case EVP_CIPH_CCM_MODE:
         if ((s->s3.tmp.new_cipher->algorithm_enc
                 & (SSL_AES128CCM8 | SSL_AES256CCM8)) != 0)
             taglen = EVP_CCM8_TLS_TAG_LEN;
         else
             taglen = EVP_CCM_TLS_TAG_LEN;
+        break;
+    default:
+        if (EVP_CIPHER_is_a(c, "CHACHA20-POLY1305")) {
+            taglen = EVP_CHACHAPOLY_TLS_TAG_LEN;
+        } else {
+            /* MAC secret size corresponds to the MAC output size */
+            taglen = s->s3.tmp.new_mac_secret_size;
+        }
+        break;
     }
 
     if (which & SSL3_CC_READ) {