Re: [Patch] NetworkPkg/IpSecDxe: Fix issue to parse SA Payload.

["Ye, Ting" <ting.ye@intel.com>]

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