Re: [edk2] [PATCH v2] NetworkPkg/IpSecDxe: Fix issue to parse SA Payload.

2018-10-19 Thread Ye, Ting


Reviewed-by: Ye Ting  

-Original Message-
From: Wu, Jiaxin 
Sent: Thursday, October 18, 2018 4:44 PM
To: edk2-devel@lists.01.org
Cc: Fu, Siyuan ; Ye, Ting ; Wu, Jiaxin 

Subject: [PATCH v2] NetworkPkg/IpSecDxe: Fix issue to parse SA Payload.

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

*v2: Correct the type of parameters in Ikev2ParseProposalData(), and refined 
the corresponding description.

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 
Cc: Ye Ting 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wu Jiaxin 
---
 NetworkPkg/IpSecDxe/Ikev2/Utility.c | 64 ++---
 1 file changed, 41 insertions(+), 23 deletions(-)

diff --git a/NetworkPkg/IpSecDxe/Ikev2/Utility.c 
b/NetworkPkg/IpSecDxe/Ikev2/Utility.c
index 0c9c929705..63de56f09e 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/Utility.c
+++ b/NetworkPkg/IpSecDxe/Ikev2/Utility.c
@@ -1993,34 +1993,51 @@ Ikev2IsSupportAlg (  }
 
 /**
   Get the preferred algorithm types from ProposalData.
 
-  @param[in]  ProposalData  Pointer to related IKEV2_PROPOSAL_DATA.
-  @param[out] PreferEncryptAlgorithmOutput of preferred encrypt algorithm.
-  @param[out] PreferIntegrityAlgorithm  Output of preferred integrity 
algorithm.
-  @param[out] PreferPrfAlgorithmOutput of preferred PRF algorithm. Only
-for IKE SA.
-  @param[out] PreferDhGroup Output of preferred DH group. Only for
-IKE SA.
-  @param[out] PreferEncryptKeylengthOutput of preferred encrypt key length
-in bytes.
-  @param[out] IsSupportEsn  Output of value about the Extented 
Sequence
-Number is support or not. Only for 
Child SA.
-  @param[in]  IsChildSa If it is ture, the ProposalData is for 
IKE
-SA. Otherwise the proposalData is for 
Child SA.
+  @param[in]  ProposalData  Pointer to related 
IKEV2_PROPOSAL_DATA.
+  @param[in, out] PreferEncryptAlgorithmPointer to buffer which is used to 
store the
+preferred encrypt algorithm.
+Input value shall be initialized 
to zero that
+indicates to be parsed from 
ProposalData.
+Output of preferred encrypt 
algorithm.
+  @param[in, out] PreferIntegrityAlgorithm  Pointer to buffer which is used to 
store the
+preferred integrity algorithm.
+Input value shall be initialized 
to zero that
+indicates to be parsed from 
ProposalData.
+Output of preferred integrity 
algorithm.
+  @param[in, out] PreferPrfAlgorithmPointer to buffer which is used to 
store the
+preferred PRF algorithm.
+Input value shall be initialized 
to zero that
+indicates to be parsed from 
ProposalData.
+Output of preferred PRF algorithm. 
Only
+for IKE SA.
+  @param[in, out] PreferDhGroup Pointer to buffer which is used to 
store the
+preferred DH group.
+Input value shall be initialized 
to zero that
+indicates to be parsed from 
ProposalData.
+Output of preferred DH group. Only 
for
+IKE SA.
+  @param[in, out] PreferEncryptKeylengthPointer to buffer which is used to 
store the
+preferred encrypt key length in 
bytes.
+  @param[in, out] IsSupportEsn  Pointer to buffer which is used to 
store the
+value about the Extented Sequence 
Number is
+support or not. Only for Child SA.
+  @param[in]  IsChildSa If it is ture, the ProposalData is 
for IKE
+SA. Otherwise the proposalData is 
for Child SA.
 
 **/
 VOID
 Ikev2ParseProposalData (
   IN 

[edk2] [PATCH v2] NetworkPkg/IpSecDxe: Fix issue to parse SA Payload.

2018-10-18 Thread Jiaxin Wu
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1251

*v2: Correct the type of parameters in Ikev2ParseProposalData(), and refined
the corresponding description.

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 
Cc: Ye Ting 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wu Jiaxin 
---
 NetworkPkg/IpSecDxe/Ikev2/Utility.c | 64 ++---
 1 file changed, 41 insertions(+), 23 deletions(-)

diff --git a/NetworkPkg/IpSecDxe/Ikev2/Utility.c 
b/NetworkPkg/IpSecDxe/Ikev2/Utility.c
index 0c9c929705..63de56f09e 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/Utility.c
+++ b/NetworkPkg/IpSecDxe/Ikev2/Utility.c
@@ -1993,34 +1993,51 @@ Ikev2IsSupportAlg (
 }
 
 /**
   Get the preferred algorithm types from ProposalData.
 
-  @param[in]  ProposalData  Pointer to related IKEV2_PROPOSAL_DATA.
-  @param[out] PreferEncryptAlgorithmOutput of preferred encrypt algorithm.
-  @param[out] PreferIntegrityAlgorithm  Output of preferred integrity 
algorithm.
-  @param[out] PreferPrfAlgorithmOutput of preferred PRF algorithm. Only
-for IKE SA.
-  @param[out] PreferDhGroup Output of preferred DH group. Only for
-IKE SA.
-  @param[out] PreferEncryptKeylengthOutput of preferred encrypt key length
-in bytes.
-  @param[out] IsSupportEsn  Output of value about the Extented 
Sequence
-Number is support or not. Only for 
Child SA.
-  @param[in]  IsChildSa If it is ture, the ProposalData is for 
IKE
-SA. Otherwise the proposalData is for 
Child SA.
+  @param[in]  ProposalData  Pointer to related 
IKEV2_PROPOSAL_DATA.
+  @param[in, out] PreferEncryptAlgorithmPointer to buffer which is used to 
store the
+preferred encrypt algorithm.
+Input value shall be initialized 
to zero that
+indicates to be parsed from 
ProposalData.
+Output of preferred encrypt 
algorithm.
+  @param[in, out] PreferIntegrityAlgorithm  Pointer to buffer which is used to 
store the
+preferred integrity algorithm.
+Input value shall be initialized 
to zero that
+indicates to be parsed from 
ProposalData.
+Output of preferred integrity 
algorithm.
+  @param[in, out] PreferPrfAlgorithmPointer to buffer which is used to 
store the
+preferred PRF algorithm.
+Input value shall be initialized 
to zero that
+indicates to be parsed from 
ProposalData.
+Output of preferred PRF algorithm. 
Only
+for IKE SA.
+  @param[in, out] PreferDhGroup Pointer to buffer which is used to 
store the
+preferred DH group.
+Input value shall be initialized 
to zero that
+indicates to be parsed from 
ProposalData.
+Output of preferred DH group. Only 
for
+IKE SA.
+  @param[in, out] PreferEncryptKeylengthPointer to buffer which is used to 
store the
+preferred encrypt key length in 
bytes.
+  @param[in, out] IsSupportEsn  Pointer to buffer which is used to 
store the
+value about the Extented Sequence 
Number is
+support or not. Only for Child SA.
+  @param[in]  IsChildSa If it is ture, the ProposalData is 
for IKE
+SA. Otherwise the proposalData is 
for Child SA.
 
 **/
 VOID
 Ikev2ParseProposalData (
   IN IKEV2_PROPOSAL_DATA  *ProposalData,
- OUT UINT16   *PreferEncryptAlgorithm,
- OUT UINT16   *PreferIntegrityAlgorithm,
- OUT UINT16   *PreferPrfAlgorithm,
- OUT UINT16   *PreferDhGroup,
- OUT UINTN