Use the configured max_send_fragment value in the write record layer

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19343)

diff --git a/ssl/record/methods/dtls_meth.c b/ssl/record/methods/dtls_meth.c
index 7dcf984aed7855666974d9245b2493fa86ce2df3..3349d16cff142ad2bfe93cf960af83cb63cdb3e6 100644 (file)
--- a/ssl/record/methods/dtls_meth.c
+++ b/ssl/record/methods/dtls_meth.c
@@ -253,7 +253,7 @@ static int dtls_process_record(OSSL_RECORD_LAYER *rl, DTLS_BITMAP *bitmap)
      * Check if the received packet overflows the current Max Fragment
      * Length setting.
      */
-    if (rl->max_frag_len > 0 && rr->length > rl->max_frag_len) {
+    if (rr->length > rl->max_frag_len) {
         RLAYERfatal(rl, SSL_AD_RECORD_OVERFLOW, SSL_R_DATA_LENGTH_TOO_LONG);
         goto end;
     }
@@ -482,8 +482,7 @@ int dtls_get_more_records(OSSL_RECORD_LAYER *rl)
          * If received packet overflows maximum possible fragment length then
          * silently discard it
          */
-        if (rl->max_frag_len > 0
-                && rr->length > rl->max_frag_len + SSL3_RT_MAX_ENCRYPTED_OVERHEAD) {
+        if (rr->length > rl->max_frag_len + SSL3_RT_MAX_ENCRYPTED_OVERHEAD) {
             /* record too long, silently discard it */
             rr->length = 0;
             rl->packet_length = 0;
@@ -713,5 +712,6 @@ const OSSL_RECORD_METHOD ossl_dtls_record_method = {
     dtls_set_in_init,
     tls_get_state,
     tls_set_options,
-    tls_get_compression
+    tls_get_compression,
+    tls_set_max_frag_len
 };

diff --git a/ssl/record/methods/ktls_meth.c b/ssl/record/methods/ktls_meth.c
index 1f44810d1debb17b9a7c60f6b5e89d183fdf73a4..dbf42b4d461ac8654612a755029907b18c76ad8d 100644 (file)
--- a/ssl/record/methods/ktls_meth.c
+++ b/ssl/record/methods/ktls_meth.c
@@ -307,16 +307,8 @@ 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)
+    if (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))
@@ -476,5 +468,6 @@ const OSSL_RECORD_METHOD ossl_ktls_record_method = {
     NULL,
     tls_get_state,
     tls_set_options,
-    tls_get_compression
+    tls_get_compression,
+    tls_set_max_frag_len
 };

diff --git a/ssl/record/methods/recmethod_local.h b/ssl/record/methods/recmethod_local.h
index 27461ff6454cdf8d009eea5c1ae4df304350b5a2..eda50050e2b712205662d3744a516f57188b7c21 100644 (file)
--- a/ssl/record/methods/recmethod_local.h
+++ b/ssl/record/methods/recmethod_local.h
@@ -173,7 +173,10 @@ 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 */
+    /*
+     * The smaller of the configured and negotiated maximum fragment length
+     * or SSL3_RT_MAX_PLAIN_LENGTH if none
+     */
     unsigned int max_frag_len;
 
     /* The maxium amount of early data we can receive/send */
@@ -329,6 +332,7 @@ void tls_get_state(OSSL_RECORD_LAYER *rl, const char **shortstr,
                    const char **longstr);
 int tls_set_options(OSSL_RECORD_LAYER *rl, const OSSL_PARAM *options);
 const COMP_METHOD *tls_get_compression(OSSL_RECORD_LAYER *rl);
+void tls_set_max_frag_len(OSSL_RECORD_LAYER *rl, size_t max_frag_len);
 int tls_setup_read_buffer(OSSL_RECORD_LAYER *rl);
 int tls_setup_write_buffer(OSSL_RECORD_LAYER *rl, size_t numwpipes,
                            size_t firstlen, size_t nextlen);

diff --git a/ssl/record/methods/tls_common.c b/ssl/record/methods/tls_common.c
index 1c44d3f688856c20e2d39e3cfbce9e266a6cd4b3..50132d7771a59790db0d220068f1c67d23b99928 100644 (file)
--- a/ssl/record/methods/tls_common.c
+++ b/ssl/record/methods/tls_common.c
@@ -144,8 +144,6 @@ int tls_setup_write_buffer(OSSL_RECORD_LAYER *rl, size_t numwpipes,
     size_t align = 0, headerlen;
     SSL3_BUFFER *wb;
     size_t currpipe;
-    /* TODO(RECLAYER): Remove me */
-    SSL_CONNECTION *s = (SSL_CONNECTION *)rl->cbarg;
     size_t defltlen = 0;
 
     if (firstlen == 0 || (numwpipes > 1 && nextlen == 0)) {
@@ -158,8 +156,8 @@ int tls_setup_write_buffer(OSSL_RECORD_LAYER *rl, size_t numwpipes,
         align = SSL3_ALIGN_PAYLOAD - 1;
 #endif
 
-        defltlen = ssl_get_max_send_fragment(s)
-            + SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD + headerlen + align;
+        defltlen = rl->max_frag_len + SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD
+                   + headerlen + align;
 #ifndef OPENSSL_NO_COMP
         if (tls_allow_compression(rl))
             defltlen += SSL3_RT_MAX_COMPRESSED_OVERHEAD;
@@ -237,8 +235,8 @@ int tls_setup_read_buffer(OSSL_RECORD_LAYER *rl)
 #endif
 
     if (b->buf == NULL) {
-        len = SSL3_RT_MAX_PLAIN_LENGTH
-            + SSL3_RT_MAX_ENCRYPTED_OVERHEAD + headerlen + align;
+        len = rl->max_frag_len
+              + SSL3_RT_MAX_ENCRYPTED_OVERHEAD + headerlen + align;
 #ifndef OPENSSL_NO_COMP
         if (tls_allow_compression(rl))
             len += SSL3_RT_MAX_COMPRESSED_OVERHEAD;
@@ -905,7 +903,7 @@ int tls_get_more_records(OSSL_RECORD_LAYER *rl)
          * Max Fragment Length setting.
          * Note: rl->max_frag_len > 0 and KTLS are mutually exclusive.
          */
-        if (rl->max_frag_len > 0 && thisrr->length > rl->max_frag_len) {
+        if (thisrr->length > rl->max_frag_len) {
             RLAYERfatal(rl, SSL_AD_RECORD_OVERFLOW, SSL_R_DATA_LENGTH_TOO_LONG);
             goto end;
         }
@@ -1214,6 +1212,12 @@ tls_int_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
     if (rl == NULL)
         return OSSL_RECORD_RETURN_FATAL;
 
+    /*
+     * Default the value for max_frag_len. This may be overridden by the
+     * settings
+     */
+    rl->max_frag_len = SSL3_RT_MAX_PLAIN_LENGTH;
+
     /* Loop through all the settings since they must all be understood */
     if (settings != NULL) {
         for (p = settings; p->key != NULL; p++) {
@@ -1512,8 +1516,6 @@ int tls_write_records_default(OSSL_RECORD_LAYER *rl,
     size_t len, wpinited = 0;
     size_t j, prefix = 0;
     int using_ktls;
-    /* TODO(RECLAYER): REMOVE ME */
-    SSL_CONNECTION *s = rl->cbarg;
     OSSL_RECORD_TEMPLATE prefixtempl;
     OSSL_RECORD_TEMPLATE *thistempl;
 
@@ -1688,7 +1690,7 @@ int tls_write_records_default(OSSL_RECORD_LAYER *rl,
                 && rl->enc_ctx != NULL
                 && (!rl->allow_plain_alerts
                     || thistempl->type != SSL3_RT_ALERT)) {
-            size_t rlen, max_send_fragment;
+            size_t rlen;
 
             if (!WPACKET_put_bytes_u8(thispkt, thistempl->type)) {
                 RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
@@ -1697,11 +1699,10 @@ int tls_write_records_default(OSSL_RECORD_LAYER *rl,
             SSL3_RECORD_add_length(thiswr, 1);
 
             /* Add TLS1.3 padding */
-            max_send_fragment = ssl_get_max_send_fragment(s);
             rlen = SSL3_RECORD_get_length(thiswr);
-            if (rlen < max_send_fragment) {
+            if (rlen < rl->max_frag_len) {
                 size_t padding = 0;
-                size_t max_padding = max_send_fragment - rlen;
+                size_t max_padding = rl->max_frag_len - rlen;
 
                 if (rl->padding != NULL) {
                     padding = rl->padding(rl->cbarg, thistempl->type, rlen);
@@ -2063,6 +2064,18 @@ const COMP_METHOD *tls_get_compression(OSSL_RECORD_LAYER *rl)
 #endif
 }
 
+void tls_set_max_frag_len(OSSL_RECORD_LAYER *rl, size_t max_frag_len)
+{
+    rl->max_frag_len = max_frag_len;
+    /*
+     * We don't need to adjust buffer sizes. Write buffer sizes are
+     * automatically checked anyway. We should only be changing the read buffer
+     * size during the handshake, so we will create a new buffer when we create
+     * the new record layer. We can't change the existing buffer because it may
+     * already have data in it.
+     */
+}
+
 const OSSL_RECORD_METHOD ossl_tls_record_method = {
     tls_new_record_layer,
     tls_free,
@@ -2086,5 +2099,6 @@ const OSSL_RECORD_METHOD ossl_tls_record_method = {
     NULL,
     tls_get_state,
     tls_set_options,
-    tls_get_compression
+    tls_get_compression,
+    tls_set_max_frag_len
 };

diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index 7d81a02111d6f5e8a3944ab94f97a6abe0188a00..d77acde232b1b8467a7ee2c6bd97d874b57605fb 100644 (file)
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -1135,7 +1135,9 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version,
     SSL_CTX *sctx = SSL_CONNECTION_GET_CTX(s);
     const OSSL_RECORD_METHOD *meth;
     int use_etm, stream_mac = 0, tlstree = 0;
-    unsigned int maxfrag = SSL3_RT_MAX_PLAIN_LENGTH;
+    unsigned int maxfrag = (direction == OSSL_RECORD_DIRECTION_WRITE)
+                           ? ssl_get_max_send_fragment(s)
+                           : SSL3_RT_MAX_PLAIN_LENGTH;
     int use_early_data = 0;
     uint32_t max_early_data;
     COMP_METHOD *compm = (comp == NULL) ? NULL : comp->method;
@@ -1205,9 +1207,16 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version,
         *set++ = OSSL_PARAM_construct_int(OSSL_LIBSSL_RECORD_LAYER_PARAM_TLSTREE,
                                           &tlstree);
 
-    if (s->session != NULL && USE_MAX_FRAGMENT_LENGTH_EXT(s->session))
+    /*
+     * We only need to do this for the read side. The write side should already
+     * have the correct value due to the ssl_get_max_send_fragment() call above
+     */
+    if (direction == OSSL_RECORD_DIRECTION_READ
+            && 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);

diff --git a/ssl/record/recordmethod.h b/ssl/record/recordmethod.h
index 6c84737a7cddaf5a8ad146356ec673dbe5e420bd..a60b2470fa8bacd6b84088ccad8df59fa175e95d 100644 (file)
--- a/ssl/record/recordmethod.h
+++ b/ssl/record/recordmethod.h
@@ -302,6 +302,13 @@ struct ossl_record_method_st {
     int (*set_options)(OSSL_RECORD_LAYER *rl, const OSSL_PARAM *options);
 
     const COMP_METHOD *(*get_compression)(OSSL_RECORD_LAYER *rl);
+
+    /*
+     * Set the maximum fragment length to be used for the record layer. This
+     * will override any previous value supplied for the "max_frag_len"
+     * setting during construction of the record layer.
+     */
+    void (*set_max_frag_len)(OSSL_RECORD_LAYER *rl, size_t max_frag_len);
 };
 
 

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 8d252d82bcad4a91d59b22fb04fff76fd4fbf8e4..fb43b9b3690aa5c73addd4d01719c8db09288d7b 100644 (file)
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -2789,6 +2789,7 @@ long SSL_ctrl(SSL *s, int cmd, long larg, void *parg)
         sc->max_send_fragment = larg;
         if (sc->max_send_fragment < sc->split_send_fragment)
             sc->split_send_fragment = sc->max_send_fragment;
+        sc->rlayer.wrlmethod->set_max_frag_len(sc->rlayer.wrl, larg);
         return 1;
     case SSL_CTRL_SET_SPLIT_SEND_FRAGMENT:
         if ((size_t)larg > sc->max_send_fragment || larg == 0)

diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 47ad5110aba11067a4ff64c1c291f9d5cc4e6f9d..880189d9989fef8c6f6b68ceab94baff9001dd65 100644 (file)
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -1707,14 +1707,18 @@ static int final_maxfragmentlen(SSL_CONNECTION *s, unsigned int context,
         return 0;
     }
 
-    /* Current SSL buffer is lower than requested MFL */
-    if (s->session && USE_MAX_FRAGMENT_LENGTH_EXT(s->session)
-            && s->max_send_fragment < GET_MAX_FRAGMENT_LENGTH(s->session))
+    if (s->session && USE_MAX_FRAGMENT_LENGTH_EXT(s->session)) {
+        s->rlayer.rrlmethod->set_max_frag_len(s->rlayer.rrl,
+                                              GET_MAX_FRAGMENT_LENGTH(s->session));
+        s->rlayer.wrlmethod->set_max_frag_len(s->rlayer.wrl,
+                                              ssl_get_max_send_fragment(s));
         /* trigger a larger buffer reallocation */
-        if (!ssl3_setup_buffers(s)) {
+        /* TODO(RECLAYER): Remove me when DTLS moved to write record layer */
+        if (SSL_CONNECTION_IS_DTLS(s) && !ssl3_setup_buffers(s)) {
             /* SSLfatal() already called */
             return 0;
         }
+    }
 
     return 1;
 }