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

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

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