Re: [Patch 2/3] NetworkPkg/TlsDxe: Handle the multiple TLS record messages encryption/decryption.

[Karunakar P <karunakarp@amiindia.co.in>]

Reviewed-by: Karunakar p <karunakarp@amiindia.co.in>


-----Original Message-----
From: Jiaxin Wu [mailto:jiaxin.wu@intel.com] 
Sent: Tuesday, March 20, 2018 6:07 AM
To: edk2-devel@lists.01.org
Cc: Karunakar P; Fu Siyuan; Ye Ting
Subject: [Patch 2/3] NetworkPkg/TlsDxe: Handle the multiple TLS record messages encryption/decryption.

Cc: Karunakar P <karunakarp@amiindia.co.in>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
---
 NetworkPkg/TlsDxe/TlsImpl.c | 74 +++++++++++++++++++++++++++++++--------------
 NetworkPkg/TlsDxe/TlsImpl.h |  6 +---
 2 files changed, 52 insertions(+), 28 deletions(-)

diff --git a/NetworkPkg/TlsDxe/TlsImpl.c b/NetworkPkg/TlsDxe/TlsImpl.c index 8e1238216b..a026075f36 100644
--- a/NetworkPkg/TlsDxe/TlsImpl.c
+++ b/NetworkPkg/TlsDxe/TlsImpl.c
@@ -1,9 +1,9 @@
 /** @file
   The Miscellaneous Routines for TlsDxe driver.
 
-Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
 
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at  http://opensource.org/licenses/bsd-license.php
@@ -48,10 +48,11 @@ TlsEncryptPacket (
   UINT16              ThisPlainMessageSize;
   TLS_RECORD_HEADER   *TempRecordHeader;
   UINT16              ThisMessageSize;
   UINT32              BufferOutSize;
   UINT8               *BufferOut;
+  UINT32              RecordCount;
   INTN                Ret;
 
   Status           = EFI_SUCCESS;
   BytesCopied      = 0;
   BufferInSize     = 0;
@@ -59,10 +60,11 @@ TlsEncryptPacket (
   BufferInPtr      = NULL;
   RecordHeaderIn   = NULL;
   TempRecordHeader = NULL;
   BufferOutSize    = 0;
   BufferOut        = NULL;
+  RecordCount      = 0;
   Ret              = 0;
 
   //
   // Calculate the size according to the fragment table.
   //
@@ -89,34 +91,46 @@ TlsEncryptPacket (
       (*FragmentTable)[Index].FragmentLength
       );
     BytesCopied += (*FragmentTable)[Index].FragmentLength;
   }
 
-  BufferOut = AllocateZeroPool (MAX_BUFFER_SIZE);
+  //
+  // Count TLS record number.
+  //
+  BufferInPtr = BufferIn;
+  while ((UINTN) BufferInPtr < (UINTN) BufferIn + BufferInSize) {
+    RecordHeaderIn = (TLS_RECORD_HEADER *) BufferInPtr;
+    if (RecordHeaderIn->ContentType != TlsContentTypeApplicationData || RecordHeaderIn->Length > TLS_PLAINTEXT_RECORD_MAX_PAYLOAD_LENGTH) {
+      Status = EFI_INVALID_PARAMETER;
+      goto ERROR;
+    }
+    BufferInPtr += TLS_RECORD_HEADER_LENGTH + RecordHeaderIn->Length;
+    RecordCount ++;
+  }
+  
+  //
+  // Allocate enough buffer to hold TLS Ciphertext.
+  //
+  BufferOut = AllocateZeroPool (RecordCount * (TLS_RECORD_HEADER_LENGTH 
+ + TLS_CIPHERTEXT_RECORD_MAX_PAYLOAD_LENGTH));
   if (BufferOut == NULL) {
     Status = EFI_OUT_OF_RESOURCES;
     goto ERROR;
   }
 
   //
-  // Parsing buffer.
+  // Parsing buffer. Received packet may have multiple TLS record messages.
   //
   BufferInPtr = BufferIn;
   TempRecordHeader = (TLS_RECORD_HEADER *) BufferOut;
   while ((UINTN) BufferInPtr < (UINTN) BufferIn + BufferInSize) {
     RecordHeaderIn = (TLS_RECORD_HEADER *) BufferInPtr;
 
-    if (RecordHeaderIn->ContentType != TlsContentTypeApplicationData) {
-      Status = EFI_INVALID_PARAMETER;
-      goto ERROR;
-    }
-
     ThisPlainMessageSize = RecordHeaderIn->Length;
 
     TlsWrite (TlsInstance->TlsConn, (UINT8 *) (RecordHeaderIn + 1), ThisPlainMessageSize);
 
-    Ret = TlsCtrlTrafficOut (TlsInstance->TlsConn, (UINT8 *)(TempRecordHeader), MAX_BUFFER_SIZE - BufferOutSize);
+    Ret = TlsCtrlTrafficOut (TlsInstance->TlsConn, (UINT8 
+ *)(TempRecordHeader), TLS_RECORD_HEADER_LENGTH + 
+ TLS_CIPHERTEXT_RECORD_MAX_PAYLOAD_LENGTH);
 
     if (Ret > 0) {
       ThisMessageSize = (UINT16) Ret;
     } else {
       //
@@ -127,11 +141,11 @@ TlsEncryptPacket (
       ThisMessageSize = 0;
     }
 
     BufferOutSize += ThisMessageSize;
 
-    BufferInPtr += RECORD_HEADER_LEN + ThisPlainMessageSize;
+    BufferInPtr += TLS_RECORD_HEADER_LENGTH + ThisPlainMessageSize;
     TempRecordHeader += ThisMessageSize;
   }
 
   FreePool (BufferIn);
   BufferIn = NULL;
@@ -199,10 +213,11 @@ TlsDecryptPacket (
   UINT16              ThisCipherMessageSize;
   TLS_RECORD_HEADER   *TempRecordHeader;
   UINT16              ThisPlainMessageSize;
   UINT8               *BufferOut;
   UINT32              BufferOutSize;
+  UINT32              RecordCount;
   INTN                Ret;
 
   Status           = EFI_SUCCESS;
   BytesCopied      = 0;
   BufferIn         = NULL;
@@ -210,10 +225,11 @@ TlsDecryptPacket (
   BufferInPtr      = NULL;
   RecordHeaderIn   = NULL;
   TempRecordHeader = NULL;
   BufferOut        = NULL;
   BufferOutSize    = 0;
+  RecordCount      = 0;
   Ret              = 0;
 
   //
   // Calculate the size according to the fragment table.
   //
@@ -240,11 +256,28 @@ TlsDecryptPacket (
       (*FragmentTable)[Index].FragmentLength
       );
     BytesCopied += (*FragmentTable)[Index].FragmentLength;
   }
 
-  BufferOut = AllocateZeroPool (MAX_BUFFER_SIZE);
+  //
+  // Count TLS record number.
+  //
+  BufferInPtr = BufferIn;
+  while ((UINTN) BufferInPtr < (UINTN) BufferIn + BufferInSize) {
+    RecordHeaderIn = (TLS_RECORD_HEADER *) BufferInPtr;
+    if (RecordHeaderIn->ContentType != TlsContentTypeApplicationData || NTOHS (RecordHeaderIn->Length) > TLS_CIPHERTEXT_RECORD_MAX_PAYLOAD_LENGTH) {
+      Status = EFI_INVALID_PARAMETER;
+      goto ERROR;
+    }
+    BufferInPtr += TLS_RECORD_HEADER_LENGTH + NTOHS (RecordHeaderIn->Length);
+    RecordCount ++;
+  }
+
+  //
+  // Allocate enough buffer to hold TLS Plaintext.
+  //
+  BufferOut = AllocateZeroPool (RecordCount * (TLS_RECORD_HEADER_LENGTH 
+ + TLS_PLAINTEXT_RECORD_MAX_PAYLOAD_LENGTH));
   if (BufferOut == NULL) {
     Status = EFI_OUT_OF_RESOURCES;
     goto ERROR;
   }
 
@@ -254,26 +287,21 @@ TlsDecryptPacket (
   BufferInPtr = BufferIn;
   TempRecordHeader = (TLS_RECORD_HEADER *) BufferOut;
   while ((UINTN) BufferInPtr < (UINTN) BufferIn + BufferInSize) {
     RecordHeaderIn = (TLS_RECORD_HEADER *) BufferInPtr;
 
-    if (RecordHeaderIn->ContentType != TlsContentTypeApplicationData) {
-      Status = EFI_INVALID_PARAMETER;
-      goto ERROR;
-    }
-
     ThisCipherMessageSize = NTOHS (RecordHeaderIn->Length);
 
-    Ret = TlsCtrlTrafficIn (TlsInstance->TlsConn, (UINT8 *) (RecordHeaderIn), RECORD_HEADER_LEN + ThisCipherMessageSize);
-    if (Ret != RECORD_HEADER_LEN + ThisCipherMessageSize) {
+    Ret = TlsCtrlTrafficIn (TlsInstance->TlsConn, (UINT8 *) (RecordHeaderIn), TLS_RECORD_HEADER_LENGTH + ThisCipherMessageSize);
+    if (Ret != TLS_RECORD_HEADER_LENGTH + ThisCipherMessageSize) {
       TlsInstance->TlsSessionState = EfiTlsSessionError;
       Status = EFI_ABORTED;
       goto ERROR;
     }
 
     Ret = 0;
-    Ret = TlsRead (TlsInstance->TlsConn, (UINT8 *) (TempRecordHeader + 1), MAX_BUFFER_SIZE - BufferOutSize);
+    Ret = TlsRead (TlsInstance->TlsConn, (UINT8 *) (TempRecordHeader + 
+ 1), TLS_PLAINTEXT_RECORD_MAX_PAYLOAD_LENGTH);
 
     if (Ret > 0) {
       ThisPlainMessageSize = (UINT16) Ret;
     } else {
       //
@@ -282,16 +310,16 @@ TlsDecryptPacket (
       DEBUG ((EFI_D_WARN, "TlsDecryptPacket: No data read from TLS object.\n"));
 
       ThisPlainMessageSize = 0;
     }
 
-    CopyMem (TempRecordHeader, RecordHeaderIn, RECORD_HEADER_LEN);
+    CopyMem (TempRecordHeader, RecordHeaderIn, 
+ TLS_RECORD_HEADER_LENGTH);
     TempRecordHeader->Length = ThisPlainMessageSize;
-    BufferOutSize += RECORD_HEADER_LEN + ThisPlainMessageSize;
+    BufferOutSize += TLS_RECORD_HEADER_LENGTH + ThisPlainMessageSize;
 
-    BufferInPtr += RECORD_HEADER_LEN + ThisCipherMessageSize;
-    TempRecordHeader += RECORD_HEADER_LEN + ThisPlainMessageSize;
+    BufferInPtr += TLS_RECORD_HEADER_LENGTH + ThisCipherMessageSize;
+    TempRecordHeader += TLS_RECORD_HEADER_LENGTH + 
+ ThisPlainMessageSize;
   }
 
   FreePool (BufferIn);
   BufferIn = NULL;
 
diff --git a/NetworkPkg/TlsDxe/TlsImpl.h b/NetworkPkg/TlsDxe/TlsImpl.h index 3ae9d0d546..e04b312c19 100644
--- a/NetworkPkg/TlsDxe/TlsImpl.h
+++ b/NetworkPkg/TlsDxe/TlsImpl.h
@@ -1,9 +1,9 @@
 /** @file
   Header file of Miscellaneous Routines for TlsDxe driver.
 
-Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
 
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at  http://opensource.org/licenses/bsd-license.php
@@ -44,14 +44,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 //
 extern EFI_SERVICE_BINDING_PROTOCOL    mTlsServiceBinding;
 extern EFI_TLS_PROTOCOL                mTlsProtocol;
 extern EFI_TLS_CONFIGURATION_PROTOCOL  mTlsConfigurationProtocol;
 
-#define RECORD_HEADER_LEN 5 /// ContentType(1) + Version(2) + Length(2)
-
-#define MAX_BUFFER_SIZE   32768
-
 /**
   Encrypt the message listed in fragment.
 
   @param[in]       TlsInstance    The pointer to the TLS instance.
   @param[in, out]  FragmentTable  Pointer to a list of fragment.
--
2.16.2.windows.1