public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch] NetworkPkg/IpSecDxe: Fix issue to parse SA Payload.
@ 2018-10-16  1:34 Jiaxin Wu
  2018-10-18  3:26 ` Ye, Ting
  0 siblings, 1 reply; 3+ messages in thread
From: Jiaxin Wu @ 2018-10-16  1:34 UTC (permalink / raw)
  To: edk2-devel; +Cc: Fu Siyuan, Ye Ting, Wu Jiaxin

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1251

IpSecDxe failed to create the Child SA during parsing SA Payload, the issue
was caused by the below commit:

SHA-1: 1e0db7b11987d0ec93be7dfe26102a327860fdbd
* MdeModulePkg/NetworkPkg: Checking for NULL pointer before use.

In above commit, it changed the value of IsMatch in Ikev2ChildSaParseSaPayload()
to FALSE. That's correct but it exposed the potential bug in to match the correct
proposal Data, which will cause the issue happen.

Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
 NetworkPkg/IpSecDxe/Ikev2/Utility.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/NetworkPkg/IpSecDxe/Ikev2/Utility.c b/NetworkPkg/IpSecDxe/Ikev2/Utility.c
index 0c9c929705..d61bae8c9d 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/Utility.c
+++ b/NetworkPkg/IpSecDxe/Ikev2/Utility.c
@@ -2502,15 +2502,16 @@ Ikev2ChildSaParseSaPayload (
           IntegrityAlgorithm == PreferIntegrityAlgorithm &&
           IsSupportEsn == PreferIsSupportEsn
           ) {
         IsMatch = TRUE;
       } else {
-        PreferEncryptAlgorithm   = 0;
-        PreferIntegrityAlgorithm = 0;
-        IsSupportEsn             = TRUE;
+        IntegrityAlgorithm = 0;
+        EncryptAlgorithm   = 0;
+        EncryptKeylength   = 0;
+        IsSupportEsn       = FALSE;
       }
-       ProposalData = (IKEV2_PROPOSAL_DATA*)((UINT8*)(ProposalData + 1) +
+      ProposalData = (IKEV2_PROPOSAL_DATA*)((UINT8*)(ProposalData + 1) +
                      ProposalData->NumTransforms * sizeof (IKEV2_TRANSFORM_DATA));
     }
 
     ProposalData  = (IKEV2_PROPOSAL_DATA *)((IKEV2_SA_DATA *)SaPayload->PayloadBuf + 1);
     if (IsMatch) {
-- 
2.17.1.windows.2



^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [Patch] NetworkPkg/IpSecDxe: Fix issue to parse SA Payload.
  2018-10-16  1:34 [Patch] NetworkPkg/IpSecDxe: Fix issue to parse SA Payload Jiaxin Wu
@ 2018-10-18  3:26 ` Ye, Ting
  2018-10-18  3:39   ` Wu, Jiaxin
  0 siblings, 1 reply; 3+ messages in thread
From: Ye, Ting @ 2018-10-18  3:26 UTC (permalink / raw)
  To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Fu, Siyuan, Wu, Jiaxin

Hi Jiaxin,

I am confused why we need set values to following local variables when Ikev2ParseProposalData marks them as 'out' attribute. Please adds more comments why '0' is required and updates 'out' to 'in out' if '0' is necessary.

+        IntegrityAlgorithm = 0;
+        EncryptAlgorithm   = 0;
+        EncryptKeylength   = 0;
+        IsSupportEsn       = FALSE;

Thanks,
Ting

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jiaxin Wu
Sent: Tuesday, October 16, 2018 9:34 AM
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] NetworkPkg/IpSecDxe: Fix issue to parse SA Payload.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1251

IpSecDxe failed to create the Child SA during parsing SA Payload, the issue was caused by the below commit:

SHA-1: 1e0db7b11987d0ec93be7dfe26102a327860fdbd
* MdeModulePkg/NetworkPkg: Checking for NULL pointer before use.

In above commit, it changed the value of IsMatch in Ikev2ChildSaParseSaPayload() to FALSE. That's correct but it exposed the potential bug in to match the correct proposal Data, which will cause the issue happen.

Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
 NetworkPkg/IpSecDxe/Ikev2/Utility.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/NetworkPkg/IpSecDxe/Ikev2/Utility.c b/NetworkPkg/IpSecDxe/Ikev2/Utility.c
index 0c9c929705..d61bae8c9d 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/Utility.c
+++ b/NetworkPkg/IpSecDxe/Ikev2/Utility.c
@@ -2502,15 +2502,16 @@ Ikev2ChildSaParseSaPayload (
           IntegrityAlgorithm == PreferIntegrityAlgorithm &&
           IsSupportEsn == PreferIsSupportEsn
           ) {
         IsMatch = TRUE;
       } else {
-        PreferEncryptAlgorithm   = 0;
-        PreferIntegrityAlgorithm = 0;
-        IsSupportEsn             = TRUE;
+        IntegrityAlgorithm = 0;
+        EncryptAlgorithm   = 0;
+        EncryptKeylength   = 0;
+        IsSupportEsn       = FALSE;
       }
-       ProposalData = (IKEV2_PROPOSAL_DATA*)((UINT8*)(ProposalData + 1) +
+      ProposalData = (IKEV2_PROPOSAL_DATA*)((UINT8*)(ProposalData + 1) 
+ +
                      ProposalData->NumTransforms * sizeof (IKEV2_TRANSFORM_DATA));
     }
 
     ProposalData  = (IKEV2_PROPOSAL_DATA *)((IKEV2_SA_DATA *)SaPayload->PayloadBuf + 1);
     if (IsMatch) {
--
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 related	[flat|nested] 3+ messages in thread
* Re: [Patch] NetworkPkg/IpSecDxe: Fix issue to parse SA Payload.
  2018-10-18  3:26 ` Ye, Ting
@ 2018-10-18  3:39   ` Wu, Jiaxin
  0 siblings, 0 replies; 3+ messages in thread
From: Wu, Jiaxin @ 2018-10-18  3:39 UTC (permalink / raw)
  To: Ye, Ting, edk2-devel@lists.01.org; +Cc: Fu, Siyuan

Thanks Ting, I will update the comments against the function.



> -----Original Message-----
> From: Ye, Ting
> Sent: Thursday, October 18, 2018 11:26 AM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: RE: [edk2] [Patch] NetworkPkg/IpSecDxe: Fix issue to parse SA
> Payload.
> 
> Hi Jiaxin,
> 
> I am confused why we need set values to following local variables when
> Ikev2ParseProposalData marks them as 'out' attribute. Please adds more
> comments why '0' is required and updates 'out' to 'in out' if '0' is necessary.
> 
> +        IntegrityAlgorithm = 0;
> +        EncryptAlgorithm   = 0;
> +        EncryptKeylength   = 0;
> +        IsSupportEsn       = FALSE;
> 
> Thanks,
> Ting
> 
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Jiaxin Wu
> Sent: Tuesday, October 16, 2018 9:34 AM
> 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] NetworkPkg/IpSecDxe: Fix issue to parse SA Payload.
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1251
> 
> IpSecDxe failed to create the Child SA during parsing SA Payload, the issue
> was caused by the below commit:
> 
> SHA-1: 1e0db7b11987d0ec93be7dfe26102a327860fdbd
> * MdeModulePkg/NetworkPkg: Checking for NULL pointer before use.
> 
> In above commit, it changed the value of IsMatch in
> Ikev2ChildSaParseSaPayload() to FALSE. That's correct but it exposed the
> potential bug in to match the correct proposal Data, which will cause the
> issue happen.
> 
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
>  NetworkPkg/IpSecDxe/Ikev2/Utility.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/NetworkPkg/IpSecDxe/Ikev2/Utility.c
> b/NetworkPkg/IpSecDxe/Ikev2/Utility.c
> index 0c9c929705..d61bae8c9d 100644
> --- a/NetworkPkg/IpSecDxe/Ikev2/Utility.c
> +++ b/NetworkPkg/IpSecDxe/Ikev2/Utility.c
> @@ -2502,15 +2502,16 @@ Ikev2ChildSaParseSaPayload (
>            IntegrityAlgorithm == PreferIntegrityAlgorithm &&
>            IsSupportEsn == PreferIsSupportEsn
>            ) {
>          IsMatch = TRUE;
>        } else {
> -        PreferEncryptAlgorithm   = 0;
> -        PreferIntegrityAlgorithm = 0;
> -        IsSupportEsn             = TRUE;
> +        IntegrityAlgorithm = 0;
> +        EncryptAlgorithm   = 0;
> +        EncryptKeylength   = 0;
> +        IsSupportEsn       = FALSE;
>        }
> -       ProposalData = (IKEV2_PROPOSAL_DATA*)((UINT8*)(ProposalData + 1)
> +
> +      ProposalData = (IKEV2_PROPOSAL_DATA*)((UINT8*)(ProposalData + 1)
> + +
>                       ProposalData->NumTransforms * sizeof
> (IKEV2_TRANSFORM_DATA));
>      }
> 
>      ProposalData  = (IKEV2_PROPOSAL_DATA *)((IKEV2_SA_DATA
> *)SaPayload->PayloadBuf + 1);
>      if (IsMatch) {
> --
> 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] 3+ messages in thread
end of thread, other threads:[~2018-10-18  3:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-10-16  1:34 [Patch] NetworkPkg/IpSecDxe: Fix issue to parse SA Payload Jiaxin Wu
2018-10-18  3:26 ` Ye, Ting
2018-10-18  3:39   ` Wu, Jiaxin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox