Don't apply max_frag_len checking if no Max Fragment Length extension
authorMatt Caswell <matt@openssl.org>
Tue, 2 Jan 2024 16:48:43 +0000 (16:48 +0000)
committerMatt Caswell <matt@openssl.org>
Thu, 18 Jan 2024 15:20:18 +0000 (15:20 +0000)
Don't check the Max Fragment Length if the it hasn't been negotiated. We
were checking it anyway, and using the default value
(SSL3_RT_MAX_PLAIN_LENGTH). This works in most cases but KTLS can cause the
record length to actually exceed this in some cases.

Fixes #23169

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23182)

ssl/record/methods/tls_common.c

index 7da423e24300e62a2b2931f5e0a4d0324a856f37..be7f34753880cbc12c1f9fc8e3e8e54b2390df5b 100644 (file)
@@ -916,11 +916,17 @@ int tls_get_more_records(OSSL_RECORD_LAYER *rl)
         }
 
         /*
-         * Check if the received packet overflows the current
-         * Max Fragment Length setting.
-         * Note: rl->max_frag_len > 0 and KTLS are mutually exclusive.
+         * Record overflow checking (e.g. checking if
+         * thisrr->length > SSL3_RT_MAX_PLAIN_LENGTH) is the responsibility of
+         * the post_process_record() function above. However we check here if
+         * the received packet overflows the current Max Fragment Length setting
+         * if there is one.
+         * Note: rl->max_frag_len != SSL3_RT_MAX_PLAIN_LENGTH and KTLS are
+         * mutually exclusive. Also note that with KTLS thisrr->length can
+         * be > SSL3_RT_MAX_PLAIN_LENGTH (and rl->max_frag_len must be ignored)
          */
-        if (thisrr->length > rl->max_frag_len) {
+        if (rl->max_frag_len != SSL3_RT_MAX_PLAIN_LENGTH
+                && thisrr->length > rl->max_frag_len) {
             RLAYERfatal(rl, SSL_AD_RECORD_OVERFLOW, SSL_R_DATA_LENGTH_TOO_LONG);
             goto end;
         }