Re: [PATCH v1] NetworkPkg/TlsDxe: Fix failure to process multiple TLS records.

[Laszlo Ersek <lersek@redhat.com>]

On 10/31/18 06:42, Jiaxin Wu wrote:
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1290.
> 
> Current implementation failed to parse the multiple TLS record
> messages due to the incorrect pointer of TLS record header. This
> patch is to resolve that problem.
> 
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
>  NetworkPkg/TlsDxe/TlsImpl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/NetworkPkg/TlsDxe/TlsImpl.c b/NetworkPkg/TlsDxe/TlsImpl.c
> index ea83dbd04f..2d4169b0a5 100644
> --- a/NetworkPkg/TlsDxe/TlsImpl.c
> +++ b/NetworkPkg/TlsDxe/TlsImpl.c
> @@ -142,11 +142,11 @@ TlsEncryptPacket (
>      }
>  
>      BufferOutSize += ThisMessageSize;
>  
>      BufferInPtr += TLS_RECORD_HEADER_LENGTH + ThisPlainMessageSize;
> -    TempRecordHeader += ThisMessageSize;
> +    TempRecordHeader = (TLS_RECORD_HEADER *)((UINT8 *)TempRecordHeader + ThisMessageSize);
>    }
>  
>    FreePool (BufferIn);
>    BufferIn = NULL;
>  
> @@ -315,11 +315,11 @@ TlsDecryptPacket (
>      CopyMem (TempRecordHeader, RecordHeaderIn, TLS_RECORD_HEADER_LENGTH);
>      TempRecordHeader->Length = ThisPlainMessageSize;
>      BufferOutSize += TLS_RECORD_HEADER_LENGTH + ThisPlainMessageSize;
>  
>      BufferInPtr += TLS_RECORD_HEADER_LENGTH + ThisCipherMessageSize;
> -    TempRecordHeader += TLS_RECORD_HEADER_LENGTH + ThisPlainMessageSize;
> +    TempRecordHeader = (TLS_RECORD_HEADER *)((UINT8 *)TempRecordHeader + TLS_RECORD_HEADER_LENGTH + ThisPlainMessageSize);
>    }
>  
>    FreePool (BufferIn);
>    BufferIn = NULL;
>  
> 

What is the practical impact of this issue? In what scenario was it
encountered? What were the symptoms?

I realize the patch may have been pushed by now -- please consider
adding the information to the BZ. (Or, please answer here, and then add
the message URL to the BZ.)

Thanks
Laszlo