Re: [edk2] [Patch] NetworkPkg/IpSecDxe: Generate SPI randomly and correct IKE_SPI_BASE value

2016-09-06 Thread Ye, Ting 
Reviewed-by: Ye Ting  

-Original Message-
From: Wu, Jiaxin 
Sent: Tuesday, September 06, 2016 11:39 AM
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; Fu, Siyuan 
Subject: [Patch] NetworkPkg/IpSecDxe: Generate SPI randomly and correct 
IKE_SPI_BASE value

This path made the following update:
* Generate SPI randomly.
* Correct IKE_SPI_BASE value according RFC 4302/4303.

Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/IpSecDxe/IkeCommon.c | 102 +++-
 NetworkPkg/IpSecDxe/IkeCommon.h |  20 ---
 NetworkPkg/IpSecDxe/Ikev2/Utility.c |  11 +++-
 3 files changed, 112 insertions(+), 21 deletions(-)

diff --git a/NetworkPkg/IpSecDxe/IkeCommon.c b/NetworkPkg/IpSecDxe/IkeCommon.c 
index 6fc7c06..b1e4321 100644
--- a/NetworkPkg/IpSecDxe/IkeCommon.c
+++ b/NetworkPkg/IpSecDxe/IkeCommon.c
@@ -1,9 +1,9 @@
 /** @file
   Common operation of the IKE
   
-  Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2010 - 2016, Intel Corporation. All rights 
+ reserved.
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
   http://opensource.org/licenses/bsd-license.php.
@@ -16,14 +16,56 @@
 #include "Ike.h"
 #include "IkeCommon.h"
 #include "IpSecConfigImpl.h"
 #include "IpSecDebug.h"
 
-//
-// Initial the SPI
-//
-UINT32mNextSpi  = IKE_SPI_BASE;
+/**
+  Check whether the new generated Spi has existed.
+
+  @param[in]   IkeSaSession   Pointer to the Child SA Session.
+  @param[in]   SpiValue   SPI Value.
+
+  @retval  TRUEThis SpiValue has existed in the Child SA Session
+  @retval  FALSE   This SpiValue doesn't exist in the Child SA Session.
+  
+**/
+BOOLEAN
+IkeSpiValueExisted (
+  IN IKEV2_SA_SESSION  *IkeSaSession,
+  IN UINT32SpiValue
+  )
+{
+  LIST_ENTRY  *Entry;
+  LIST_ENTRY  *Next;
+  IKEV2_CHILD_SA_SESSION  *SaSession;
+
+  Entry = NULL;
+  Next  = NULL;
+  SaSession = NULL;
+
+  //
+  // Check whether the SPI value has existed in ChildSaEstablishSessionList.
+  //
+  NET_LIST_FOR_EACH_SAFE (Entry, Next, 
>ChildSaEstablishSessionList) {
+SaSession= IKEV2_CHILD_SA_SESSION_BY_IKE_SA (Entry);
+if (SaSession->LocalPeerSpi == SpiValue) {
+  return TRUE;
+}
+  }
+
+  //
+  // Check whether the SPI value has existed in ChildSaSessionList.
+  //
+  NET_LIST_FOR_EACH_SAFE (Entry, Next, >ChildSaSessionList) {
+SaSession= IKEV2_CHILD_SA_SESSION_BY_IKE_SA (Entry);
+if (SaSession->LocalPeerSpi == SpiValue) {
+  return TRUE;
+}
+  }
+
+  return FALSE;
+}
 
 /**
   Call Crypto Lib to generate a random value with eight-octet length.
   
   @return the 64 byte vaule.
@@ -156,23 +198,57 @@ IkePayloadFree (
   FreePool (IkePayload);
 }
 
 /**
   Generate an new SPI.
-
-  @return a SPI in 4 bytes.
+  
+  @param[in]  IkeSaSession   Pointer to IKEV2_SA_SESSION related to this 
Child SA 
+ Session.
+  @param[in out]  SpiValue   Pointer to the new generated SPI value. 
+  
+  @retval EFI_SUCCESS The operation performs successfully.
+  @retval Otherwise   The operation is failed.
 
 **/
-UINT32
+EFI_STATUS
 IkeGenerateSpi (
-  VOID
+  IN  IKEV2_SA_SESSION *IkeSaSession,
+  OUT UINT32   *SpiValue
   )
 {
-  //
-  // TODO: should generate SPI randomly to avoid security issue
-  //
-  return mNextSpi++;
+  EFI_STATUS   Status;
+
+  Status = EFI_SUCCESS;
+ 
+  while (TRUE) {
+//
+// Generate SPI randomly
+//
+Status = IpSecCryptoIoGenerateRandomBytes ((UINT8 *)SpiValue, sizeof 
(UINT32));
+if (EFI_ERROR (Status)) {
+  break;
+}
+
+//
+// The set of SPI values in the range 1 through 255 are reserved by the 
+// Internet Assigned Numbers Authority (IANA) for future use; a reserved 
+// SPI value will not normally be assigned by IANA unless the use of the 
+// assigned SPI value is specified in an RFC.
+//
+if (*SpiValue < IKE_SPI_BASE) {
+  *SpiValue += IKE_SPI_BASE; 
+}
+
+//
+// Check whether the new generated SPI has existed.
+//
+if (!IkeSpiValueExisted (IkeSaSession, *SpiValue)) {
+  break;
+}
+  }
+  
+  return Status;
 }
 
 /**
   Generate a random data for IV
 
diff --git a/NetworkPkg/IpSecDxe/IkeCommon.h b/NetworkPkg/IpSecDxe/IkeCommon.h 
index 714ecaa..7f7fd4d 100644
--- a/NetworkPkg/IpSecDxe/IkeCommon.h
+++ b/NetworkPkg/IpSecDxe/IkeCommon.h
@@ -1,9 +1,9 @@
 /** @file
   Common operation of the IKE.
 
-  Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2010 - 2016,

Re: [edk2] [Patch] NetworkPkg/IpSecDxe: Generate SPI randomly and correct IKE_SPI_BASE value

2016-09-05 Thread Fu, Siyuan 
Reviewed-by: Fu Siyuan 



> -Original Message-
> From: Wu, Jiaxin
> Sent: Tuesday, September 6, 2016 11:39 AM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan 
> Subject: [Patch] NetworkPkg/IpSecDxe: Generate SPI randomly and correct
> IKE_SPI_BASE value
> 
> This path made the following update:
> * Generate SPI randomly.
> * Correct IKE_SPI_BASE value according RFC 4302/4303.
> 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiaxin Wu 
> ---
>  NetworkPkg/IpSecDxe/IkeCommon.c | 102
> +++-
>  NetworkPkg/IpSecDxe/IkeCommon.h |  20 ---
>  NetworkPkg/IpSecDxe/Ikev2/Utility.c |  11 +++-
>  3 files changed, 112 insertions(+), 21 deletions(-)
> 
> diff --git a/NetworkPkg/IpSecDxe/IkeCommon.c
> b/NetworkPkg/IpSecDxe/IkeCommon.c
> index 6fc7c06..b1e4321 100644
> --- a/NetworkPkg/IpSecDxe/IkeCommon.c
> +++ b/NetworkPkg/IpSecDxe/IkeCommon.c
> @@ -1,9 +1,9 @@
>  /** @file
>Common operation of the IKE
> 
> -  Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.
> +  Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
> 
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the
> BSD License
>which accompanies this distribution.  The full text of the license may
> be found at
>http://opensource.org/licenses/bsd-license.php.
> @@ -16,14 +16,56 @@
>  #include "Ike.h"
>  #include "IkeCommon.h"
>  #include "IpSecConfigImpl.h"
>  #include "IpSecDebug.h"
> 
> -//
> -// Initial the SPI
> -//
> -UINT32mNextSpi  = IKE_SPI_BASE;
> +/**
> +  Check whether the new generated Spi has existed.
> +
> +  @param[in]   IkeSaSession   Pointer to the Child SA Session.
> +  @param[in]   SpiValue   SPI Value.
> +
> +  @retval  TRUEThis SpiValue has existed in the Child SA Session
> +  @retval  FALSE   This SpiValue doesn't exist in the Child SA Session.
> +
> +**/
> +BOOLEAN
> +IkeSpiValueExisted (
> +  IN IKEV2_SA_SESSION  *IkeSaSession,
> +  IN UINT32SpiValue
> +  )
> +{
> +  LIST_ENTRY  *Entry;
> +  LIST_ENTRY  *Next;
> +  IKEV2_CHILD_SA_SESSION  *SaSession;
> +
> +  Entry = NULL;
> +  Next  = NULL;
> +  SaSession = NULL;
> +
> +  //
> +  // Check whether the SPI value has existed in
> ChildSaEstablishSessionList.
> +  //
> +  NET_LIST_FOR_EACH_SAFE (Entry, Next, 
> >ChildSaEstablishSessionList) {
> +SaSession= IKEV2_CHILD_SA_SESSION_BY_IKE_SA (Entry);
> +if (SaSession->LocalPeerSpi == SpiValue) {
> +  return TRUE;
> +}
> +  }
> +
> +  //
> +  // Check whether the SPI value has existed in ChildSaSessionList.
> +  //
> +  NET_LIST_FOR_EACH_SAFE (Entry, Next, >ChildSaSessionList)
> {
> +SaSession= IKEV2_CHILD_SA_SESSION_BY_IKE_SA (Entry);
> +if (SaSession->LocalPeerSpi == SpiValue) {
> +  return TRUE;
> +}
> +  }
> +
> +  return FALSE;
> +}
> 
>  /**
>Call Crypto Lib to generate a random value with eight-octet length.
> 
>@return the 64 byte vaule.
> @@ -156,23 +198,57 @@ IkePayloadFree (
>FreePool (IkePayload);
>  }
> 
>  /**
>Generate an new SPI.
> -
> -  @return a SPI in 4 bytes.
> +
> +  @param[in]  IkeSaSession   Pointer to IKEV2_SA_SESSION related to
> this Child SA
> + Session.
> +  @param[in out]  SpiValue   Pointer to the new generated SPI value.
> +
> +  @retval EFI_SUCCESS The operation performs successfully.
> +  @retval Otherwise   The operation is failed.
> 
>  **/
> -UINT32
> +EFI_STATUS
>  IkeGenerateSpi (
> -  VOID
> +  IN  IKEV2_SA_SESSION *IkeSaSession,
> +  OUT UINT32   *SpiValue
>)
>  {
> -  //
> -  // TODO: should generate SPI randomly to avoid security issue
> -  //
> -  return mNextSpi++;
> +  EFI_STATUS   Status;
> +
> +  Status = EFI_SUCCESS;
> +
> +  while (TRUE) {
> +//
> +// Generate SPI randomly
> +//
> +Status = IpSecCryptoIoGenerateRandomBytes ((UINT8 *)SpiValue, sizeof
> (UINT32));
> +if (EFI_ERROR (Status)) {
> +  break;
> +}
> +
> +//
> +// The set of SPI values in the range 1 through 255 are reserved by
> the
> +// Internet Assigned Numbers Authority (IANA) for future use; a
> reserved
> +// SPI value will not normally be assigned by IANA unless the use of
> the
> +// assigned SPI value is specified in an RFC.
> +//
> +if (*SpiValue < IKE_SPI_BASE) {
> +  *SpiValue += IKE_SPI_BASE;
> +}
> +
> +//
> +// Check whether the new generated SPI has existed.
> +//
> +if (!IkeSpiValueExisted (IkeSaSession, *SpiValue)) {
> +  break;
> +}
> +  }
> +
> +  return Status;
>  }
> 
>  /**
>Generate a random data for IV
> 
> diff --git a/NetworkPkg/IpSecDxe/IkeCommon.h
>