* [PATCH v1] NetworkPkg/TlsDxe: Fix failure to process multiple TLS records.
@ 2018-10-31 5:42 Jiaxin Wu
2018-10-31 5:53 ` Fu, Siyuan
2018-11-05 18:32 ` Laszlo Ersek
0 siblings, 2 replies; 5+ messages in thread
From: Jiaxin Wu @ 2018-10-31 5:42 UTC (permalink / raw)
To: edk2-devel; +Cc: Ye Ting, Fu Siyuan, Wu Jiaxin
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;
--
2.17.1.windows.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v1] NetworkPkg/TlsDxe: Fix failure to process multiple TLS records.
2018-10-31 5:42 [PATCH v1] NetworkPkg/TlsDxe: Fix failure to process multiple TLS records Jiaxin Wu
@ 2018-10-31 5:53 ` Fu, Siyuan
2018-11-05 18:32 ` Laszlo Ersek
1 sibling, 0 replies; 5+ messages in thread
From: Fu, Siyuan @ 2018-10-31 5:53 UTC (permalink / raw)
To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Ye, Ting, Wu, Jiaxin
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@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 <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu,
> Jiaxin <jiaxin.wu@intel.com>
> 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 <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;
>
> --
> 2.17.1.windows.2
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1] NetworkPkg/TlsDxe: Fix failure to process multiple TLS records.
2018-10-31 5:42 [PATCH v1] NetworkPkg/TlsDxe: Fix failure to process multiple TLS records Jiaxin Wu
2018-10-31 5:53 ` Fu, Siyuan
@ 2018-11-05 18:32 ` Laszlo Ersek
2018-11-06 2:13 ` Wu, Jiaxin
1 sibling, 1 reply; 5+ messages in thread
From: Laszlo Ersek @ 2018-11-05 18:32 UTC (permalink / raw)
To: Jiaxin Wu, edk2-devel; +Cc: Ye Ting, Fu Siyuan
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1] NetworkPkg/TlsDxe: Fix failure to process multiple TLS records.
2018-11-05 18:32 ` Laszlo Ersek
@ 2018-11-06 2:13 ` Wu, Jiaxin
2018-11-06 14:25 ` Laszlo Ersek
0 siblings, 1 reply; 5+ messages in thread
From: Wu, Jiaxin @ 2018-11-06 2:13 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel@lists.01.org; +Cc: Ye, Ting, Fu, Siyuan
Already share the info @ https://bugzilla.tianocore.org/show_bug.cgi?id=1290
Thanks,
Jiaxin
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Tuesday, November 6, 2018 2:33 AM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> 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 <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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1] NetworkPkg/TlsDxe: Fix failure to process multiple TLS records.
2018-11-06 2:13 ` Wu, Jiaxin
@ 2018-11-06 14:25 ` Laszlo Ersek
0 siblings, 0 replies; 5+ messages in thread
From: Laszlo Ersek @ 2018-11-06 14:25 UTC (permalink / raw)
To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Ye, Ting, Fu, Siyuan
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:lersek@redhat.com]
>> Sent: Tuesday, November 6, 2018 2:33 AM
>> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
>> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
>> 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 <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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-11-06 14:25 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-10-31 5:42 [PATCH v1] NetworkPkg/TlsDxe: Fix failure to process multiple TLS records Jiaxin Wu
2018-10-31 5:53 ` Fu, Siyuan
2018-11-05 18:32 ` Laszlo Ersek
2018-11-06 2:13 ` Wu, Jiaxin
2018-11-06 14:25 ` Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox