git: f6e5fcdc848f - main - OpenSSL: Cleanup record length checks for KTLS

From: John Baldwin <jhb_at_FreeBSD.org>
Date: Wed, 04 May 2022 20:33:54 UTC
The branch main has been updated by jhb:

URL: https://cgit.FreeBSD.org/src/commit/?id=f6e5fcdc848fc24fe71f01ba583f13e762c0ab54

commit f6e5fcdc848fc24fe71f01ba583f13e762c0ab54
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2022-05-04 20:07:36 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2022-05-04 20:07:36 +0000

    OpenSSL: Cleanup record length checks for KTLS
    
    In some corner cases the check for packets
    which exceed the allowed record length was missing
    when KTLS is initially enabled, when some
    unprocessed packets are still pending.
    
    Approved by:    jkim
    Obtained from:  OpenSSL commit 8fff986d52606e1a33f9404504535e2e2aee3e8b
    MFC after:      1 week
    Sponsored by:   Netflix
    Differential Revision:  https://reviews.freebsd.org/D34972
---
 crypto/openssl/ssl/record/ssl3_record.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/crypto/openssl/ssl/record/ssl3_record.c b/crypto/openssl/ssl/record/ssl3_record.c
index 9dda123d44fa..4fd22019ee7b 100644
--- a/crypto/openssl/ssl/record/ssl3_record.c
+++ b/crypto/openssl/ssl/record/ssl3_record.c
@@ -190,7 +190,7 @@ int ssl3_get_record(SSL *s)
 
     rr = RECORD_LAYER_get_rrec(&s->rlayer);
     rbuf = RECORD_LAYER_get_rbuf(&s->rlayer);
-    is_ktls_left = (rbuf->left > 0);
+    is_ktls_left = (SSL3_BUFFER_get_left(rbuf) > 0);
     max_recs = s->max_pipelines;
     if (max_recs == 0)
         max_recs = 1;
@@ -412,7 +412,11 @@ int ssl3_get_record(SSL *s)
                 len -= SSL3_RT_MAX_COMPRESSED_OVERHEAD;
 #endif
 
-            if (thisrr->length > len && !BIO_get_ktls_recv(s->rbio)) {
+            /* KTLS may use all of the buffer */
+            if (BIO_get_ktls_recv(s->rbio) && !is_ktls_left)
+                len = SSL3_BUFFER_get_left(rbuf);
+
+            if (thisrr->length > len) {
                 SSLfatal(s, SSL_AD_RECORD_OVERFLOW, SSL_F_SSL3_GET_RECORD,
                          SSL_R_ENCRYPTED_LENGTH_TOO_LONG);
                 return -1;
@@ -775,16 +779,28 @@ int ssl3_get_record(SSL *s)
             return -1;
         }
 
-        if (thisrr->length > SSL3_RT_MAX_PLAIN_LENGTH && !BIO_get_ktls_recv(s->rbio)) {
+        /*
+         * Usually thisrr->length is the length of a single record, but when
+         * KTLS handles the decryption, thisrr->length may be larger than
+         * SSL3_RT_MAX_PLAIN_LENGTH because the kernel may have coalesced
+         * multiple records.
+         * Therefore we have to rely on KTLS to check the plaintext length
+         * limit in the kernel.
+         */
+        if (thisrr->length > SSL3_RT_MAX_PLAIN_LENGTH
+                && (!BIO_get_ktls_recv(s->rbio) || is_ktls_left)) {
             SSLfatal(s, SSL_AD_RECORD_OVERFLOW, SSL_F_SSL3_GET_RECORD,
                      SSL_R_DATA_LENGTH_TOO_LONG);
             return -1;
         }
 
-        /* If received packet overflows current Max Fragment Length setting */
+        /*
+         * Check if the received packet overflows the current
+         * Max Fragment Length setting.
+         * Note: USE_MAX_FRAGMENT_LENGTH_EXT and KTLS are mutually exclusive.
+         */
         if (s->session != NULL && USE_MAX_FRAGMENT_LENGTH_EXT(s->session)
-                && thisrr->length > GET_MAX_FRAGMENT_LENGTH(s->session)
-                && !BIO_get_ktls_recv(s->rbio)) {
+                && thisrr->length > GET_MAX_FRAGMENT_LENGTH(s->session)) {
             SSLfatal(s, SSL_AD_RECORD_OVERFLOW, SSL_F_SSL3_GET_RECORD,
                      SSL_R_DATA_LENGTH_TOO_LONG);
             return -1;