Bug 1294978 - Reworked overlong record size checks and added TLS1.3 specific boundaries. r=djackson

Old overlong record check flow:
1.) There is a check for the default maximally allowed record size in ssl3gthr.c/ssl3_GatherData after reception of TLS records. In the same file the DTLS reception buffers are set to the maximum possible record size in dtls_GatherData.
2.) Next the ssl3_HandleRecord handler checks TLS and DTLS records sizes, considering possibly set size limits by the record-size-limit-extension and the maximally approximated cipher expansion possible in NSS.
3.) Until this patch there was a less strict redundant size check in ssl3con.c/ssl3_UnprotectRecord. In tls13con.c/tls13_UnprotectRecord and ssl3con.c/ssl3_UnprotectRecord the plaintext size is checked for validity after unprotecting (plaintext checks were not changed in this patch).
4.) DTLS errors regarding record size and unprotecting are inconsistently sometimes propagated to the peer (alerts) and sometimes silently dropped.

Changes:
1.) In ssl3gthr.c TLS 1.3 specific cases for overlong record checks and DTLS buffer allocation have been added.
2.) The ssl3_HandleRecord handler checks for RFC compliant records sizes (all TLS versions), considering limits set by record_size_limit_extension. This is less strict for TLS <= 1.2, stricter checks have been moved to the unprotection functions to create a similar 'check flow/levels' for all TLS versions.
3.)
- TLS <= 1.2: Moved strict check for maximum allowed plaintext + approximated maximum cipher expansion to ssl3con.c/ssl3_UnprotectRecord.
- TLS 1.3: Added strict check for maximum allowed plaintext + actually used cipher expansion to tls13con.c/tls13_UnprotectRecord.
(Maximum allowed plaintext considers limits set by record_size_limit_extension)
4.) Following RFC6347, Section 4.1.2.7 DTLS errors regarding records and unprotecting and now consistently dropped silently.

Added Tests:
- Positive tests (All (D)TLS versions): Test that largest valid plainext + encryption expansion are successfully sent and handled.
- Negative tests (All (D)TLS versions): Test that all added/updated boundaries lead to the expected alerts. Tested with smallest illegal record size for each of the mentioned checks.

Differential Revision: https://phabricator.services.mozilla.com/D138529

6 files changed, 352 insertions, 46 deletions

diff --git a/gtests/ssl_gtest/ssl_recordsize_unittest.cc b/gtests/ssl_gtest/ssl_recordsize_unittest.cc
index a18510440..8a84db574 100644
--- a/gtests/ssl_gtest/ssl_recordsize_unittest.cc
+++ b/gtests/ssl_gtest/ssl_recordsize_unittest.cc
@@ -223,7 +223,7 @@ class TlsRecordExpander : public TlsRecordFilter {
 };
 
 // Tweak the plaintext of server records so that they exceed the client's limit.
-TEST_P(TlsConnectTls13, RecordSizePlaintextExceed) {
+TEST_F(TlsConnectStreamTls13, RecordSizePlaintextExceed) {
   EnsureTlsSetup();
   auto server_expand = MakeTlsFilter<TlsRecordExpander>(server_, 1);
   server_expand->EnableDecryption();
@@ -247,7 +247,7 @@ TEST_P(TlsConnectTls13, RecordSizePlaintextExceed) {
 // This requires a much larger expansion than for plaintext to trigger the
 // guard, which runs before decryption (current allowance is 320 octets,
 // see MAX_EXPANSION in ssl3con.c).
-TEST_P(TlsConnectTls13, RecordSizeCiphertextExceed) {
+TEST_F(TlsConnectStreamTls13, RecordSizeCiphertextExceed) {
   EnsureTlsSetup();
 
   client_->SetOption(SSL_RECORD_SIZE_LIMIT, 64);
@@ -329,7 +329,7 @@ class TlsRecordPadder : public TlsRecordFilter {
   size_t padding_;
 };
 
-TEST_P(TlsConnectTls13, RecordSizeExceedPad) {
+TEST_F(TlsConnectStreamTls13, RecordSizeExceedPad) {
   EnsureTlsSetup();
   auto server_max = std::make_shared<TlsRecordMaximum>(server_);
   auto server_expand = std::make_shared<TlsRecordPadder>(server_, 1);
@@ -494,4 +494,233 @@ TEST_F(RecordSizeDefaultsTest, RecordSizeGetValue) {
   EXPECT_EQ(3000, v);
 }
 
-}  // namespace nss_test
+class TlsCtextResizer : public TlsRecordFilter {
+ public:
+  TlsCtextResizer(const std::shared_ptr<TlsAgent>& a, size_t size)
+      : TlsRecordFilter(a), size_(size) {}
+
+ protected:
+  virtual PacketFilter::Action FilterRecord(const TlsRecordHeader& header,
+                                            const DataBuffer& data,
+                                            DataBuffer* changed) {
+    // allocate and initialise buffer
+    changed->Allocate(size_);
+
+    // copy record data (partially)
+    changed->Write(0, data.data(),
+                   ((data.len() >= size_) ? size_ : data.len()));
+
+    return CHANGE;
+  }
+
+ private:
+  size_t size_;
+};
+
+/* (D)TLS overlong record test for maximum default record size of
+ * 2^14 + (256 (TLS 1.3) OR 2048 (TLS <= 1.2)
+ * [RFC8446, Section 5.2; RFC5246 , Section 6.2.3].
+ * This should fail the first size check in ssl3gthr.c/ssl3_GatherData().
+ * DTLS Record errors are dropped silently. [RFC6347, Section 4.1.2.7]. */
+TEST_P(TlsConnectGeneric, RecordGatherOverlong) {
+  EnsureTlsSetup();
+
+  size_t max_ctext = MAX_FRAGMENT_LENGTH;
+  if (version_ >= SSL_LIBRARY_VERSION_TLS_1_3) {
+    max_ctext += TLS_1_3_MAX_EXPANSION;
+  } else {
+    max_ctext += TLS_1_2_MAX_EXPANSION;
+  }
+
+  Connect();
+
+  MakeTlsFilter<TlsCtextResizer>(server_, max_ctext + 1);
+  // Dummy record will be overwritten
+  server_->SendData(0xf0);
+
+  /* Drop DTLS Record Errors silently [RFC6347, Section 4.1.2.7]. */
+  if (variant_ == ssl_variant_datagram) {
+    size_t received = client_->received_bytes();
+    client_->ReadBytes(max_ctext + 1);
+    ASSERT_EQ(received, client_->received_bytes());
+  } else {
+    client_->ExpectSendAlert(kTlsAlertRecordOverflow);
+    client_->ReadBytes(max_ctext + 1);
+    server_->ExpectReceiveAlert(kTlsAlertRecordOverflow);
+    server_->Handshake();
+  }
+}
+
+/* (D)TLS overlong record test with recordSizeLimit Extension and plus RFC
+ * specified maximum Expansion: 2^14 + (256 (TLS 1.3) OR 2048 (TLS <= 1.2)
+ * [RFC8446, Section 5.2; RFC5246 , Section 6.2.3].
+ * DTLS Record errors are dropped silently. [RFC6347, Section 4.1.2.7]. */
+TEST_P(TlsConnectGeneric, RecordSizeExtensionOverlong) {
+  EnsureTlsSetup();
+
+  // Set some boundary
+  size_t max_ctext = 1000;
+
+  client_->SetOption(SSL_RECORD_SIZE_LIMIT, max_ctext);
+
+  if (version_ >= SSL_LIBRARY_VERSION_TLS_1_3) {
+    // The record size limit includes the inner content type byte
+    max_ctext += TLS_1_3_MAX_EXPANSION - 1;
+  } else {
+    max_ctext += TLS_1_2_MAX_EXPANSION;
+  }
+
+  Connect();
+
+  MakeTlsFilter<TlsCtextResizer>(server_, max_ctext + 1);
+  // Dummy record will be overwritten
+  server_->SendData(0xf);
+
+  /* Drop DTLS Record Errors silently [RFC6347, Section 4.1.2.7].
+   * For DTLS 1.0 and 1.2 the package is dropped before the size check because
+   * of the modification. This just tests that no error is thrown as required.
+   */
+  if (variant_ == ssl_variant_datagram) {
+    size_t received = client_->received_bytes();
+    client_->ReadBytes(max_ctext + 1);
+    ASSERT_EQ(received, client_->received_bytes());
+  } else {
+    client_->ExpectSendAlert(kTlsAlertRecordOverflow);
+    client_->ReadBytes(max_ctext + 1);
+    server_->ExpectReceiveAlert(kTlsAlertRecordOverflow);
+    server_->Handshake();
+  }
+}
+
+/* For TLS <= 1.2:
+ * MAX_EXPANSION is the amount by which a record might plausibly be expanded
+ * when protected.  It's the worst case estimate, so the sum of block cipher
+ * padding (up to 256 octets), HMAC (48 octets for SHA-384), and IV (16
+ * octets for AES). */
+#define MAX_EXPANSION (256 + 48 + 16)
+
+/* (D)TLS overlong record test for specific ciphersuite expansion.
+ * Testing the smallest illegal record.
+ * This check is performed in ssl3con.c/ssl3_UnprotectRecord() OR
+ * tls13con.c/tls13_UnprotectRecord() and enforces stricter size limitations,
+ * dependent on the implemented cipher suites, than the RFC.
+ * DTLS Record errors are dropped silently. [RFC6347, Section 4.1.2.7]. */
+TEST_P(TlsConnectGeneric, RecordExpansionOverlong) {
+  EnsureTlsSetup();
+
+  // Set some boundary
+  size_t max_ctext = 1000;
+
+  client_->SetOption(SSL_RECORD_SIZE_LIMIT, max_ctext);
+
+  if (version_ >= SSL_LIBRARY_VERSION_TLS_1_3) {
+    // For TLS1.3 all ciphers expand the cipherext by 16B
+    // The inner content type byte is included in the record size limit
+    max_ctext += 16;
+  } else {
+    // For TLS<=1.2 the max possible expansion in the NSS implementation is 320
+    max_ctext += MAX_EXPANSION;
+  }
+
+  Connect();
+
+  MakeTlsFilter<TlsCtextResizer>(server_, max_ctext + 1);
+  // Dummy record will be overwritten
+  server_->SendData(0xf);
+
+  /* Drop DTLS Record Errors silently [RFC6347, Section 4.1.2.7].
+   * For DTLS 1.0 and 1.2 the package is dropped before the size check because
+   * of the modification. This just tests that no error is thrown as required/
+   * no bytes are received. */
+  if (variant_ == ssl_variant_datagram) {
+    size_t received = client_->received_bytes();
+    client_->ReadBytes(max_ctext + 1);
+    ASSERT_EQ(received, client_->received_bytes());
+  } else {
+    client_->ExpectSendAlert(kTlsAlertRecordOverflow);
+    client_->ReadBytes(max_ctext + 1);
+    server_->ExpectReceiveAlert(kTlsAlertRecordOverflow);
+    server_->Handshake();
+  }
+}
+
+/* (D)TLS longest allowed record default size test. */
+TEST_P(TlsConnectGeneric, RecordSizeDefaultLong) {
+  EnsureTlsSetup();
+  Connect();
+
+  // Maximum allowed plaintext size
+  size_t max = MAX_FRAGMENT_LENGTH;
+
+  /* For TLS 1.0 the first byte of application data is sent in a single record
+   * as explained in the documentation of SSL_CBC_RANDOM_IV in ssl.h.
+   * Because of that we use TlsCTextResizer to send a record of max size.
+   * A bad record mac alert is expected since we modify the record. */
+  if (version_ == SSL_LIBRARY_VERSION_TLS_1_0 &&
+      variant_ == ssl_variant_stream) {
+    // Set size to maxi plaintext + max allowed expansion
+    MakeTlsFilter<TlsCtextResizer>(server_, max + MAX_EXPANSION);
+    // Dummy record will be overwritten
+    server_->SendData(0xF);
+    // Expect alert
+    client_->ExpectSendAlert(kTlsAlertBadRecordMac);
+    // Receive record
+    client_->ReadBytes(max);
+    // Handle alert on server side
+    server_->ExpectReceiveAlert(kTlsAlertBadRecordMac);
+    server_->Handshake();
+  } else {  // Everything but TLS 1.0
+    // Send largest legal plaintext as single record
+    // by setting SendData() block size to max.
+    server_->SendData(max, max);
+    // Receive record
+    client_->ReadBytes(max);
+    // Assert that data was received successfully
+    ASSERT_EQ(client_->received_bytes(), max);
+  }
+}
+
+/* (D)TLS longest allowed record size limit extension test. */
+TEST_P(TlsConnectGeneric, RecordSizeLimitLong) {
+  EnsureTlsSetup();
+
+  // Set some boundary
+  size_t max = 1000;
+  client_->SetOption(SSL_RECORD_SIZE_LIMIT, max);
+
+  Connect();
+
+  // For TLS 1.3 the InnerContentType byte is included in the record size limit
+  if (version_ == SSL_LIBRARY_VERSION_TLS_1_3) {
+    max--;
+  }
+
+  /* For TLS 1.0 the first byte of application data is sent in a single record
+   * as explained in the documentation of SSL_CBC_RANDOM_IV in ssl.h.
+   * Because of that we use TlsCTextResizer to send a record of max size.
+   * A bad record mac alert is expected since we modify the record. */
+  if (version_ == SSL_LIBRARY_VERSION_TLS_1_0 &&
+      variant_ == ssl_variant_stream) {
+    // Set size to maxi plaintext + max allowed expansion
+    MakeTlsFilter<TlsCtextResizer>(server_, max + MAX_EXPANSION);
+    // Dummy record will be overwritten
+    server_->SendData(0xF);
+    // Expect alert
+    client_->ExpectSendAlert(kTlsAlertBadRecordMac);
+    // Receive record
+    client_->ReadBytes(max);
+    // Handle alert on server side
+    server_->ExpectReceiveAlert(kTlsAlertBadRecordMac);
+    server_->Handshake();
+  } else {  // Everything but TLS 1.0
+    // Send largest legal plaintext as single record
+    // by setting SendData() block size to max.
+    server_->SendData(max, max);
+    // Receive record
+    client_->ReadBytes(max);
+    // Assert that data was received successfully
+    ASSERT_EQ(client_->received_bytes(), max);
+  }
+}
+
+}  // namespace nss_test
\ No newline at end of file
diff --git a/gtests/ssl_gtest/test_io.cc b/gtests/ssl_gtest/test_io.cc
index 4a7f91459..e4651a235 100644
--- a/gtests/ssl_gtest/test_io.cc
+++ b/gtests/ssl_gtest/test_io.cc
@@ -110,7 +110,6 @@ int32_t DummyPrSocket::Recv(PRFileDesc *f, void *buf, int32_t buflen,
 
   auto &front = input_.front();
   if (static_cast<size_t>(buflen) < front.len()) {
-    PR_ASSERT(false);
     PR_SetError(PR_BUFFER_OVERFLOW_ERROR, 0);
     return -1;
   }
diff --git a/lib/ssl/ssl3con.c b/lib/ssl/ssl3con.c
index 9261684b4..0375cf42c 100644
--- a/lib/ssl/ssl3con.c
+++ b/lib/ssl/ssl3con.c
@@ -12908,6 +12908,12 @@ ssl_CBCExtractMAC(sslBuffer *plaintext,
     }
 }
 
+/* MAX_EXPANSION is the amount by which a record might plausibly be expanded
+ * when protected.  It's the worst case estimate, so the sum of block cipher
+ * padding (up to 256 octets), HMAC (48 octets for SHA-384), and IV (16
+ * octets for AES). */
+#define MAX_EXPANSION (256 + 48 + 16)
+
 /* Unprotect an SSL3 record and leave the result in plaintext.
  *
  * If SECFailure is returned, we:
@@ -12994,14 +13000,18 @@ ssl3_UnprotectRecord(sslSocket *ss,
     PRINT_BUF(80, (ss, "ciphertext:", cText->buf->buf + ivLen,
                    cText->buf->len - ivLen));
 
-    isTLS = (PRBool)(spec->version > SSL_LIBRARY_VERSION_3_0);
-
-    if (isTLS && cText->buf->len - ivLen > (MAX_FRAGMENT_LENGTH + 2048)) {
+    /* Check if the ciphertext can be valid if we assume maximum plaintext and
+     * add the maximum possible ciphersuite expansion.
+     * This way we detect overlong plaintexts/padding before decryption.
+     * This check enforces size limitations more strict than the RFC.
+     * [RFC5246, Section 6.2.3] */
+    if (cText->buf->len > (spec->recordSizeLimit + MAX_EXPANSION)) {
         *alert = record_overflow;
         PORT_SetError(SSL_ERROR_RX_RECORD_TOO_LONG);
         return SECFailure;
     }
 
+    isTLS = (PRBool)(spec->version > SSL_LIBRARY_VERSION_3_0);
     rType = (SSLContentType)cText->hdr[0];
     rVersion = ((SSL3ProtocolVersion)cText->hdr[1] << 8) |
                (SSL3ProtocolVersion)cText->hdr[2];
@@ -13226,12 +13236,6 @@ ssl3_GetCipherSpec(sslSocket *ss, SSL3Ciphertext *cText)
     return NULL;
 }
 
-/* MAX_EXPANSION is the amount by which a record might plausibly be expanded
- * when protected.  It's the worst case estimate, so the sum of block cipher
- * padding (up to 256 octets), HMAC (48 octets for SHA-384), and IV (16
- * octets for AES). */
-#define MAX_EXPANSION (256 + 48 + 16)
-
 /* if cText is non-null, then decipher and check the MAC of the
  * SSL record from cText->buf (typically gs->inbuf)
  * into databuf (typically gs->buf), and any previous contents of databuf
@@ -13258,10 +13262,10 @@ SECStatus
 ssl3_HandleRecord(sslSocket *ss, SSL3Ciphertext *cText)
 {
     SECStatus rv;
-    PRBool isTLS;
+    PRBool isTLS, isTLS13;
     DTLSEpoch epoch;
     ssl3CipherSpec *spec = NULL;
-    PRUint16 recordSizeLimit;
+    PRUint16 recordSizeLimit, cTextSizeLimit;
     PRBool outOfOrderSpec = PR_FALSE;
     SSLContentType rType;
     sslBuffer *plaintext = &ss->gs.buf;
@@ -13322,20 +13326,37 @@ ssl3_HandleRecord(sslSocket *ss, SSL3Ciphertext *cText)
         return SECFailure;
     }
 
+    isTLS13 = (PRBool)(ss->version >= SSL_LIBRARY_VERSION_TLS_1_3);
     recordSizeLimit = spec->recordSizeLimit;
-    if (cText->buf->len > recordSizeLimit + MAX_EXPANSION) {
+    cTextSizeLimit = recordSizeLimit;
+    cTextSizeLimit += (isTLS13) ? TLS_1_3_MAX_EXPANSION : TLS_1_2_MAX_EXPANSION;
+
+    /* Check if the specified recordSizeLimit and the RFC8446 specified max
+     * expansion are respected. recordSizeLimit is probably at the default for
+     * the first (hello) handshake message and then set to a smaller size by
+     * the Record Size Limit Extension.
+     * Stricter expansion size checks dependent on implemented cipher suites
+     * are performed in ssl3con.c/ssl3_UnprotectRecord() OR
+     * tls13con.c/tls13_UnprotextRecord().
+     * After Decryption the plaintext size is checked (l. 13424). This also
+     * applies to unencrypted records. */
+    if (cText->buf->len > cTextSizeLimit) {
         ssl_ReleaseSpecReadLock(ss); /*****************************/
+        /* Drop DTLS Record Errors silently [RFC6347, Section 4.1.2.7] */
+        if (IS_DTLS(ss)) {
+            return SECSuccess;
+        }
         SSL3_SendAlert(ss, alert_fatal, record_overflow);
         PORT_SetError(SSL_ERROR_RX_RECORD_TOO_LONG);
         return SECFailure;
     }
 
-    if (plaintext->space < recordSizeLimit + MAX_EXPANSION) {
-        rv = sslBuffer_Grow(plaintext, recordSizeLimit + MAX_EXPANSION);
+    if (plaintext->space < cTextSizeLimit) {
+        rv = sslBuffer_Grow(plaintext, cTextSizeLimit);
         if (rv != SECSuccess) {
             ssl_ReleaseSpecReadLock(ss); /*************************/
             SSL_DBG(("%d: SSL3[%d]: HandleRecord, tried to get %d bytes",
-                     SSL_GETPID(), ss->fd, recordSizeLimit + MAX_EXPANSION));
+                     SSL_GETPID(), ss->fd, cTextSizeLimit));
             /* sslBuffer_Grow has set a memory error code. */
             /* Perhaps we should send an alert. (but we have no memory!) */
             return SECFailure;
@@ -13444,6 +13465,10 @@ ssl3_HandleRecord(sslSocket *ss, SSL3Ciphertext *cText)
     /* Check the length of the plaintext. */
     if (isTLS && plaintext->len > recordSizeLimit) {
         plaintext->len = 0;
+        /* Drop DTLS Record Errors silently [RFC6347, Section 4.1.2.7] */
+        if (IS_DTLS(ss)) {
+            return SECSuccess;
+        }
         SSL3_SendAlert(ss, alert_fatal, record_overflow);
         PORT_SetError(SSL_ERROR_RX_RECORD_TOO_LONG);
         return SECFailure;
diff --git a/lib/ssl/ssl3gthr.c b/lib/ssl/ssl3gthr.c
index 45cfb31bc..101241f0a 100644
--- a/lib/ssl/ssl3gthr.c
+++ b/lib/ssl/ssl3gthr.c
@@ -174,13 +174,26 @@ ssl3_GatherData(sslSocket *ss, sslGather *gs, int flags, ssl2Gather *ssl2gs)
                     }
                 }
 
-                /* This is the max length for an encrypted SSLv3+ fragment. */
-                if (!v2HdrLength &&
-                    gs->remainder > (MAX_FRAGMENT_LENGTH + 2048)) {
-                    SSL3_SendAlert(ss, alert_fatal, record_overflow);
-                    gs->state = GS_INIT;
-                    PORT_SetError(SSL_ERROR_RX_RECORD_TOO_LONG);
-                    return SECFailure;
+                /* If it is NOT an SSLv2 header */
+                if (!v2HdrLength) {
+                    /* Check if default RFC specified max ciphertext/record
+                     * limits are respected. Checks for used record size limit
+                     * extension boundaries are done in
+                     * ssl3con.c/ssl3_HandleRecord() for tls and dtls records.
+                     *
+                     * -> For TLS 1.2 records MUST NOT be longer than
+                     * 2^14 + 2048 bytes.
+                     * -> For TLS 1.3 records MUST NOT exceed 2^14 + 256 bytes.
+                     * -> For older versions this MAY be enforced, we do it.
+                     * [RFC8446 Section 5.2, RFC5246 Section 6.2.3]. */
+                    if (gs->remainder > TLS_1_2_MAX_CTEXT_LENGTH ||
+                        (gs->remainder > TLS_1_3_MAX_CTEXT_LENGTH &&
+                         ss->version >= SSL_LIBRARY_VERSION_TLS_1_3)) {
+                        SSL3_SendAlert(ss, alert_fatal, record_overflow);
+                        gs->state = GS_INIT;
+                        PORT_SetError(SSL_ERROR_RX_RECORD_TOO_LONG);
+                        return SECFailure;
+                    }
                 }
 
                 gs->state = GS_DATA;
@@ -267,7 +280,7 @@ dtls_GatherData(sslSocket *ss, sslGather *gs, int flags)
     int nb;
     PRUint8 contentType;
     unsigned int headerLen;
-    SECStatus rv;
+    SECStatus rv = SECSuccess;
     PRBool dtlsLengthPresent = PR_TRUE;
 
     SSL_TRC(30, ("dtls_GatherData"));
@@ -281,18 +294,33 @@ dtls_GatherData(sslSocket *ss, sslGather *gs, int flags)
         gs->dtlsPacketOffset = 0;
         gs->dtlsPacket.len = 0;
 
-        /* Resize to the maximum possible size so we can fit a full datagram */
-        /* This is the max fragment length for an encrypted fragment
-        ** plus the size of the record header.
-        ** This magic constant is copied from ssl3_GatherData, with 5 changed
-        ** to 13 (the size of the record header).
-        */
-        if (gs->dtlsPacket.space < MAX_FRAGMENT_LENGTH + 2048 + 13) {
-            rv = sslBuffer_Grow(&gs->dtlsPacket,
-                                MAX_FRAGMENT_LENGTH + 2048 + 13);
-            if (rv != SECSuccess) {
-                return -1; /* Code already set. */
+        /* Resize to the maximum possible size so we can fit a full datagram.
+         * This leads to record_overflow errors if records/ciphertexts greater
+         * than the buffer (= maximum record) size are to be received.
+         * DTLS Record errors are dropped silently. [RFC6347, Section 4.1.2.7].
+         * Checks for record size limit extension boundaries are performed in
+         * ssl3con.c/ssl3_HandleRecord() for tls and dtls records.
+         *
+         * -> For TLS 1.2 records MUST NOT be longer than 2^14 + 2048 bytes.
+         * -> For TLS 1.3 records MUST NOT exceed 2^14 + 256 bytes.
+         * -> For older versions this MAY be enforced, we do it.
+         * [RFC8446 Section 5.2, RFC5246 Section 6.2.3]. */
+        if (ss->version <= SSL_LIBRARY_VERSION_TLS_1_2) {
+            if (gs->dtlsPacket.space < DTLS_1_2_MAX_PACKET_LENGTH) {
+                rv = sslBuffer_Grow(&gs->dtlsPacket, DTLS_1_2_MAX_PACKET_LENGTH);
             }
+        } else { /* version >= TLS 1.3 */
+            if (gs->dtlsPacket.space != DTLS_1_3_MAX_PACKET_LENGTH) {
+                /* During Hello and version negotiation older DTLS versions with
+                 * greater possible packets are used. The buffer must therefore
+                 * be "truncated" by clearing and reallocating it */
+                sslBuffer_Clear(&gs->dtlsPacket);
+                rv = sslBuffer_Grow(&gs->dtlsPacket, DTLS_1_3_MAX_PACKET_LENGTH);
+            }
+        }
+
+        if (rv != SECSuccess) {
+            return -1; /* Code already set. */
         }
 
         /* recv() needs to read a full datagram at a time */
@@ -306,6 +334,8 @@ dtls_GatherData(sslSocket *ss, sslGather *gs, int flags)
         } else /* if (nb < 0) */ {
             SSL_DBG(("%d: SSL3[%d]: recv error %d", SSL_GETPID(), ss->fd,
                      PR_GetError()));
+            /* DTLS Record Errors, including overlong records, are silently
+             * dropped [RFC6347, Section 4.1.2.7]. */
             return -1;
         }
 
diff --git a/lib/ssl/ssl3prot.h b/lib/ssl/ssl3prot.h
index 31db46b41..9eeb5a302 100644
--- a/lib/ssl/ssl3prot.h
+++ b/lib/ssl/ssl3prot.h
@@ -31,7 +31,20 @@ typedef PRUint16 ssl3CipherSuite;
 /* SSL3_RECORD_HEADER_LENGTH + epoch/sequence_number */
 #define DTLS_RECORD_HEADER_LENGTH 13
 
+/* Max values for TLS records/ciphertexts
+ * For TLS 1.2 records MUST NOT be longer than 2^14 + 2048
+ * For TLS 1.3 records MUST NOT exceed 2^14 + 256 bytes.
+ * [RFC8446 Section 5.2, RFC5246 Section 6.2.3]. */
 #define MAX_FRAGMENT_LENGTH 16384
+#define TLS_1_2_MAX_EXPANSION 2048
+#define TLS_1_3_MAX_EXPANSION (255 + 1)
+#define TLS_1_3_MAX_CTEXT_LENGTH ((MAX_FRAGMENT_LENGTH) + (TLS_1_3_MAX_EXPANSION))
+#define TLS_1_2_MAX_CTEXT_LENGTH ((MAX_FRAGMENT_LENGTH) + (TLS_1_2_MAX_EXPANSION))
+
+/* DTLS_X_X_MAX_PACKET_LENGTH = TLS_X_X_MAX_RECORD_LENGTH + HEADER_LENGTH,
+ * used for DTLS datagram buffer size setting. We do not support DTLS CID! */
+#define DTLS_1_3_MAX_PACKET_LENGTH ((TLS_1_3_MAX_CTEXT_LENGTH) + (SSL3_RECORD_HEADER_LENGTH))
+#define DTLS_1_2_MAX_PACKET_LENGTH ((TLS_1_2_MAX_CTEXT_LENGTH) + (DTLS_RECORD_HEADER_LENGTH))
 
 typedef enum { change_cipher_spec_choice = 1 } SSL3ChangeCipherSpecChoice;
 
diff --git a/lib/ssl/tls13con.c b/lib/ssl/tls13con.c
index b934c2ccb..6a1f352bc 100644
--- a/lib/ssl/tls13con.c
+++ b/lib/ssl/tls13con.c
@@ -5791,7 +5791,7 @@ tls13_ProtectRecord(sslSocket *ss,
  *
  * If SECFailure is returned, we:
  * 1. Set |*alert| to the alert to be sent.
- * 2. Call PORT_SetError() witn an appropriate code.
+ * 2. Call PORT_SetError() with an appropriate code.
  */
 SECStatus
 tls13_UnprotectRecord(sslSocket *ss,
@@ -5804,6 +5804,7 @@ tls13_UnprotectRecord(sslSocket *ss,
     const ssl3BulkCipherDef *cipher_def = spec->cipherDef;
     const int ivLen = cipher_def->iv_size + cipher_def->explicit_nonce_size;
     const int tagLen = cipher_def->tag_size;
+    const int innerTypeLen = 1;
 
     PRUint8 aad[21];
     unsigned int aadLen;
@@ -5840,6 +5841,17 @@ tls13_UnprotectRecord(sslSocket *ss,
         return SECFailure;
     }
 
+    /* Check if the ciphertext can be valid if we assume maximum plaintext and
+     * add the specific ciphersuite expansion.
+     * This way we detect overlong plaintexts/padding before decryption.
+     * This check enforces size limitations more strict than the RFC.
+     * (see RFC8446, Section 5.2) */
+    if (cText->buf->len > (spec->recordSizeLimit + innerTypeLen + tagLen)) {
+        *alert = record_overflow;
+        PORT_SetError(SSL_ERROR_RX_RECORD_TOO_LONG);
+        return SECFailure;
+    }
+
     /* Check the version number in the record. Stream only. */
     if (!IS_DTLS(ss)) {
         SSL3ProtocolVersion version =
@@ -5887,11 +5899,9 @@ tls13_UnprotectRecord(sslSocket *ss,
     }
 
     /* There is a similar test in ssl3_HandleRecord, but this test is needed to
-     * account for padding.  It's safe to do this here (including the alert),
-     * because it only confirms that the record exceeded the size limit, which
-     * is apparent from the size of the ciphertext. */
-    if (plaintext->len > spec->recordSizeLimit + 1) {
-        SSL3_SendAlert(ss, alert_fatal, record_overflow);
+     * account for padding. */
+    if (plaintext->len > spec->recordSizeLimit + innerTypeLen) {
+        *alert = record_overflow;
         PORT_SetError(SSL_ERROR_RX_RECORD_TOO_LONG);
         return SECFailure;
     }
@@ -6342,4 +6352,4 @@ tls13_MaybeTls13(sslSocket *ss)
     }
 
     return PR_FALSE;
-}
+}
\ No newline at end of file