Improve comments across record size limit changes
Signed-off-by: Waleed Elmelegy <waleed.elmelegy@arm.com>
diff --git a/library/ssl_misc.h b/library/ssl_misc.h
index 7195d63..2e621be 100644
--- a/library/ssl_misc.h
+++ b/library/ssl_misc.h
@@ -2704,6 +2704,7 @@
#if defined(MBEDTLS_SSL_RECORD_SIZE_LIMIT)
#define MBEDTLS_SSL_RECORD_SIZE_LIMIT_EXTENSION_DATA_LENGTH (2)
+/* This value is defined by RFC 8449 */
#define MBEDTLS_SSL_RECORD_SIZE_LIMIT_MIN (64)
MBEDTLS_CHECK_RETURN_CRITICAL
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index f92e40a..517af78 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -3521,15 +3521,15 @@
if (ssl->transform_out != NULL &&
ssl->transform_out->tls_version == MBEDTLS_SSL_VERSION_TLS1_3) {
- /* RFC 8449, section 4:
- *
- * This value [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).
- *
- * Thus, round down to a multiple of MBEDTLS_SSL_CID_TLS1_3_PADDING_GRANULARITY
- * and subtract 1 (for the content type that will be added later)
+ /*
+ * In TLS 1.3 case, when records are protected, `max_len` as computed
+ * above is the maximum length of the TLSInnerPlaintext structure that
+ * along the plaintext payload contains the inner content type (one byte)
+ * and some zero padding. Given the algorithm used for padding
+ * in mbedtls_ssl_encrypt_buf(), compute the maximum length for
+ * the plaintext payload. Round down to a multiple of
+ * MBEDTLS_SSL_CID_TLS1_3_PADDING_GRANULARITY and
+ * subtract 1.
*/
max_len = ((max_len / MBEDTLS_SSL_CID_TLS1_3_PADDING_GRANULARITY) *
MBEDTLS_SSL_CID_TLS1_3_PADDING_GRANULARITY) - 1;
diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh
index 12605f5..30e6a72 100755
--- a/tests/ssl-opt.sh
+++ b/tests/ssl-opt.sh
@@ -4862,16 +4862,18 @@
-c "EncryptedExtensions: record_size_limit(28) extension received." \
-c "RecordSizeLimit: 16385 Bytes" \
-# In the following (9) tests, --recordsize is the value used by the G_NEXT_CLI (3.7.2) to configure the
-# maximum record size using "https://gnutls.org/reference/gnutls-gnutls.html#gnutls-record-set-max-size".
-# There is currently a lower limit of 512, caused by this function not respecting the
-# "%ALLOW_SMALL_RECORDS" priority string and not using the more recent function
-# https://gnutls.org/reference/gnutls-gnutls.html#gnutls-record-set-max-recv-size.
+# In the following tests, --recordsize is the value used by the G_NEXT_CLI (3.7.2) to configure the
+# maximum record size using gnutls_record_set_max_size()
+# (https://gnutls.org/reference/gnutls-gnutls.html#gnutls-record-set-max-size).
+# There is currently a lower limit of 512, caused by gnutls_record_set_max_size()
+# not respecting the "%ALLOW_SMALL_RECORDS" priority string and not using the
+# more recent function gnutls_record_set_max_recv_size()
+# (https://gnutls.org/reference/gnutls-gnutls.html#gnutls-record-set-max-recv-size).
# There is currently an upper limit of 4096, caused by the cli arg parser:
# https://gitlab.com/gnutls/gnutls/-/blob/3.7.2/src/cli-args.def#L395.
-# Thus, these tests are currently limit to that value range.
-# Moreover, the value sent in the extension is expected to be larger by one compared
-# to the value passed on the cli:
+# Thus, these tests are currently limited to the value range 512-4096.
+# Also, the value sent in the extension will be one larger than the value
+# set at the command line:
# https://gitlab.com/gnutls/gnutls/-/blob/3.7.2/lib/ext/record_size_limit.c#L142
# Currently test certificates being used do not fit in 513 record size limit
@@ -5050,6 +5052,8 @@
-s "Maximum outgoing record payload length is 16383" \
-s "Maximum incoming record payload length is 16384"
+# End of Record size limit tests
+
# Tests for renegotiation
# Renegotiation SCSV always added, regardless of SSL_RENEGOTIATION