Comply with the received Record Size Limit extension

Fixes Mbed-TLS#7010

Signed-off-by: Jan Bruckner <[email protected]>

include/mbedtls/ssl.h

@@ -1184,6 +1184,11 @@ struct mbedtls_ssl_session {
     unsigned char MBEDTLS_PRIVATE(mfl_code);     /*!< MaxFragmentLength negotiated by peer */
 #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */
 
+/*!< Record Size Limit for outgoing data requested by peer */
+#if defined(MBEDTLS_SSL_RECORD_SIZE_LIMIT)
+    uint16_t MBEDTLS_PRIVATE(record_size_limit);
+#endif /* MBEDTLS_SSL_RECORD_SIZE_LIMIT */
+
     unsigned char MBEDTLS_PRIVATE(exported);
 
     /** TLS version negotiated in the session. Used if and when renegotiating

library/ssl_misc.h

@@ -450,6 +450,24 @@ size_t mbedtls_ssl_get_output_max_frag_len(const mbedtls_ssl_context *ssl);
 size_t mbedtls_ssl_get_input_max_frag_len(const mbedtls_ssl_context *ssl);
 #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */
 
+#if defined(MBEDTLS_SSL_RECORD_SIZE_LIMIT)
+/**
+ * \brief          Return the record size limit (payload, in bytes) for
+ *                 the output buffer. This is the value requested by the
+ *                 peer (using RFC 8449), or, if there is none, MBEDTLS_SSL_OUT_CONTENT_LEN.
+ *                 TODO: adapt this description, depending on how we decide to handle the space
+ *                  required by content type and padding (see comment at the end of
+ *                  mbedtls_ssl_get_output_record_size_limit()).
+ *
+ * \sa             mbedtls_ssl_get_max_out_record_payload()
+ *
+ * \param ssl      SSL context
+ *
+ * \return         Current record size limit for the output buffer.
+ */
+size_t mbedtls_ssl_get_output_record_size_limit(const mbedtls_ssl_context *ssl);
+#endif /* MBEDTLS_SSL_RECORD_SIZE_LIMIT */
+
 #if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH)
 static inline size_t mbedtls_ssl_get_output_buflen(const mbedtls_ssl_context *ctx)
 {

library/ssl_tls.c

@@ -2467,6 +2467,7 @@ mbedtls_ssl_mode_t mbedtls_ssl_get_mode_from_ciphersuite(
  *       uint32 ticket_age_add;
  *       uint8 ticket_flags;
  *       opaque resumption_key<0..255>;
+ *       uint16 record_size_limit;
  *       select ( endpoint ) {
  *            case client: ClientOnlyData;
  *            case server: uint64 start_time;
@@ -2499,6 +2500,10 @@ static int ssl_tls13_session_save(const mbedtls_ssl_session *session,
     }
     needed += session->resumption_key_len;  /* resumption_key */
 
+#if defined(MBEDTLS_SSL_RECORD_SIZE_LIMIT)
+    needed += 2; /* record_size_limit */
+#endif /* MBEDTLS_SSL_RECORD_SIZE_LIMIT */
+
 #if defined(MBEDTLS_HAVE_TIME)
     needed += 8; /* start_time or ticket_received */
 #endif
@@ -2538,6 +2543,11 @@ static int ssl_tls13_session_save(const mbedtls_ssl_session *session,
     memcpy(p, session->resumption_key, session->resumption_key_len);
     p += session->resumption_key_len;
 
+#if defined(MBEDTLS_SSL_RECORD_SIZE_LIMIT)
+    MBEDTLS_PUT_UINT16_BE(session->record_size_limit, p, 0);
+    p += 2;
+#endif /* MBEDTLS_SSL_RECORD_SIZE_LIMIT */
+
 #if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_SRV_C)
     if (session->endpoint == MBEDTLS_SSL_IS_SERVER) {
         MBEDTLS_PUT_UINT64_BE((uint64_t) session->start, p, 0);
@@ -2606,6 +2616,14 @@ static int ssl_tls13_session_load(mbedtls_ssl_session *session,
     memcpy(session->resumption_key, p, session->resumption_key_len);
     p += session->resumption_key_len;
 
+#if defined(MBEDTLS_SSL_RECORD_SIZE_LIMIT)
+    if (end - p < 2) {
+        return MBEDTLS_ERR_SSL_BAD_INPUT_DATA;
+    }
+    session->record_size_limit = MBEDTLS_GET_UINT16_BE(p, 0);
+    p += 2;
+#endif /* MBEDTLS_SSL_RECORD_SIZE_LIMIT */
+
 #if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_SRV_C)
     if (session->endpoint == MBEDTLS_SSL_IS_SERVER) {
         if (end - p < 8) {
@@ -3403,6 +3421,7 @@ int mbedtls_ssl_get_max_out_record_payload(const mbedtls_ssl_context *ssl)
     size_t max_len = MBEDTLS_SSL_OUT_CONTENT_LEN;
 
 #if !defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) && \
+    !defined(MBEDTLS_SSL_RECORD_SIZE_LIMIT) && \
     !defined(MBEDTLS_SSL_PROTO_DTLS)
     (void) ssl;
 #endif
@@ -3415,6 +3434,14 @@ int mbedtls_ssl_get_max_out_record_payload(const mbedtls_ssl_context *ssl)
     }
 #endif
 
+#if defined(MBEDTLS_SSL_RECORD_SIZE_LIMIT)
+    const size_t record_size_limit = mbedtls_ssl_get_output_record_size_limit(ssl);
+
+    if (max_len > record_size_limit) {
+        max_len = record_size_limit;
+    }
+#endif
+
 #if defined(MBEDTLS_SSL_PROTO_DTLS)
     if (mbedtls_ssl_get_current_mtu(ssl) != 0) {
         const size_t mtu = mbedtls_ssl_get_current_mtu(ssl);

library/ssl_tls13_client.c

@@ -2158,12 +2158,11 @@ static int ssl_tls13_parse_encrypted_extensions(mbedtls_ssl_context *ssl,
 
                 ret = mbedtls_ssl_tls13_parse_record_size_limit_ext(
                     ssl, p, p + extension_data_len);
-
-                /* TODO: Return unconditionally here until we handle the record
-                 * size limit correctly. Once handled correctly, only return in
-                 * case of errors. */
-                return ret;
-
+                if (ret != 0) {
+                    MBEDTLS_SSL_DEBUG_RET(
+                        1, ("mbedtls_ssl_tls13_parse_record_size_limit_ext"), ret);
+                    return ret;
+                }
                 break;
 #endif /* MBEDTLS_SSL_RECORD_SIZE_LIMIT */
 

library/ssl_tls13_generic.c

@@ -1660,7 +1660,7 @@ int mbedtls_ssl_tls13_parse_record_size_limit_ext(mbedtls_ssl_context *ssl,
 
     MBEDTLS_SSL_DEBUG_MSG(2, ("RecordSizeLimit: %u Bytes", record_size_limit));
 
-    /* RFC 8449, section 4
+    /* RFC 8449, section 4:
      *
      * Endpoints MUST NOT send a "record_size_limit" extension with a value
      * smaller than 64.  An endpoint MUST treat receipt of a smaller value
@@ -1673,13 +1673,46 @@ int mbedtls_ssl_tls13_parse_record_size_limit_ext(mbedtls_ssl_context *ssl,
         return MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER;
     }
 
-    MBEDTLS_SSL_DEBUG_MSG(
-        2, ("record_size_limit extension is still in development. Aborting handshake."));
+    ssl->session_negotiate->record_size_limit = record_size_limit;
 
-    MBEDTLS_SSL_PEND_FATAL_ALERT(
-        MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_EXT,
-        MBEDTLS_ERR_SSL_UNSUPPORTED_EXTENSION);
-    return MBEDTLS_ERR_SSL_UNSUPPORTED_EXTENSION;
+    return 0;
+}
+
+size_t mbedtls_ssl_get_output_record_size_limit(const mbedtls_ssl_context *ssl)
+{
+    const size_t max_len = MBEDTLS_SSL_OUT_CONTENT_LEN;
+    size_t record_size_limit = max_len;
+
+    if (ssl->session != NULL &&
+        ssl->session->record_size_limit >= MBEDTLS_SSL_RECORD_SIZE_LIMIT_MIN &&
+        ssl->session->record_size_limit < max_len) {
+        record_size_limit = ssl->session->record_size_limit;
+    }
+
+    // TODO: this is currently untested
+    /* During a handshake, use the value being negotiated */
+    if (ssl->session_negotiate != NULL &&
+        ssl->session_negotiate->record_size_limit >= MBEDTLS_SSL_RECORD_SIZE_LIMIT_MIN &&
+        ssl->session_negotiate->record_size_limit < max_len) {
+        record_size_limit = ssl->session_negotiate->record_size_limit;
+    }
+
+    /* RFC 8449, section 4:
+     *
+     * This value [the record size limit] is the length of the plaintext of a protected record.
+     * The value includes the content type and padding added in TLS 1.3 (that is, the complete
+     * length of TLSInnerPlaintext).
+     *
+     * TODO:
+     *  Therefore, we have to subtract these from the peer's record_size_limit at some point:
+     *  - content type: 1 byte
+     *  - maximum padding: (MBEDTLS_SSL_CID_TLS1_3_PADDING_GRANULARITY - 1) bytes
+     *  Note: Always using the maximum padding in this calculation leads to an unnecessarily short
+     *   record (up to (MBEDTLS_SSL_CID_TLS1_3_PADDING_GRANULARITY - 1) available bytes are not used).
+     *  Note: Doing the subtraction here is not correct, since it will always reduce the outgoing
+     *   paylod length, even if the record size limit extension is not used.
+     */
+    return record_size_limit;
 }
 #endif /* MBEDTLS_SSL_RECORD_SIZE_LIMIT */
 

library/ssl_tls13_server.c

@@ -1660,14 +1660,11 @@ static int ssl_tls13_parse_client_hello(mbedtls_ssl_context *ssl,
 
                 ret = mbedtls_ssl_tls13_parse_record_size_limit_ext(
                     ssl, p, extension_data_end);
-
-                /*
-                 * TODO: Return unconditionally here until we handle the record
-                 *       size limit correctly.
-                 *       Once handled correctly, only return in case of errors.
-                 */
-                return ret;
-
+                if (ret != 0) {
+                    MBEDTLS_SSL_DEBUG_RET(
+                        1, ("mbedtls_ssl_tls13_parse_record_size_limit_ext"), ret);
+                    return ret;
+                }
                 break;
 #endif /* MBEDTLS_SSL_RECORD_SIZE_LIMIT */
 

programs/ssl/ssl_server2.c

@@ -3505,7 +3505,7 @@ int main(int argc, char *argv[])
         mbedtls_printf("    [ Record expansion is unknown ]\n");
     }
 
-#if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH)
+#if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) || defined(MBEDTLS_SSL_RECORD_SIZE_LIMIT)
     mbedtls_printf("    [ Maximum incoming record payload length is %u ]\n",
                    (unsigned int) mbedtls_ssl_get_max_in_record_payload(&ssl));
     mbedtls_printf("    [ Maximum outgoing record payload length is %u ]\n",

scripts/config.py

@@ -215,7 +215,6 @@ def realfull_adapter(_name, active, section):
     'MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN', # build dependency (clang+memsan)
     'MBEDTLS_TEST_CONSTANT_FLOW_VALGRIND', # build dependency (valgrind headers)
     'MBEDTLS_X509_REMOVE_INFO', # removes a feature
-    'MBEDTLS_SSL_RECORD_SIZE_LIMIT', # in development, currently breaks other tests
 ])
 
 def is_seamless_alt(name):

tests/scripts/all.sh

@@ -4025,11 +4025,8 @@ component_test_tls13_only_record_size_limit () {
     msg "test_suite_ssl: TLS 1.3 only, record size limit extension enabled"
     cd tests; ./test_suite_ssl; cd ..
 
-    msg "ssl-opt.sh: (TLS 1.3 only, record size limit extension tests only)"
-    # Both the server and the client will currently abort the handshake when they encounter the
-    # record size limit extension. There is no way to prevent gnutls-cli from sending the extension
-    # which makes all G_NEXT_CLI + P_SRV tests fail. Thus, run only the tests for the this extension.
-    tests/ssl-opt.sh -f "Record Size Limit"
+    msg "ssl-opt.sh: (TLS 1.3 only, record size limit extension enabled)"
+    tests/ssl-opt.sh
 }
 
 component_build_mingw () {