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

2018-11-06 Thread Laszlo Ersek
On 11/06/18 03:13, Wu, Jiaxin wrote:
> Already share the info @ https://bugzilla.tianocore.org/show_bug.cgi?id=1290

Awesome, thanks!
Laszlo

>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Tuesday, November 6, 2018 2:33 AM
>> To: Wu, Jiaxin ; edk2-devel@lists.01.org
>> Cc: Ye, Ting ; Fu, Siyuan 
>> Subject: Re: [edk2] [PATCH v1] NetworkPkg/TlsDxe: Fix failure to process
>> multiple TLS records.
>>
>> 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 
>>> Cc: Fu Siyuan 
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Wu Jiaxin 
>>> ---
>>>  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

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


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

2018-11-05 Thread Laszlo Ersek
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 
> Cc: Fu Siyuan 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wu Jiaxin 
> ---
>  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
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


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

2018-10-30 Thread Fu, Siyuan
Reviewed-by: Fu Siyuan 


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Jiaxin Wu
> Sent: Wednesday, October 31, 2018 1:42 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; Wu,
> Jiaxin 
> Subject: [edk2] [PATCH v1] NetworkPkg/TlsDxe: Fix failure to process
> multiple TLS records.
> 
> 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 
> Cc: Fu Siyuan 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wu Jiaxin 
> ---
>  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;
> 
> --
> 2.17.1.windows.2
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


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

2018-10-30 Thread Jiaxin Wu
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 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wu Jiaxin 
---
 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;
 
-- 
2.17.1.windows.2

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel