[PATCH v2 0/2] NetworkPkg/IpSecDxe: IKE functionality issue fix

[PATCH v2 0/2] NetworkPkg/IpSecDxe: IKE functionality issue fix

[Jiaxin Wu]

*v2: update the commit log and refine the code comments.

Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Zhang Lubo <lubo.zhang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>

Jiaxin Wu (2):
  NetworkPkg/IpSecDxe: Fix UEFI IKE Initial Exchange failure
  NetworkPkg/IpSecDxe: Fix wrong IKE header "FLAG" update

 NetworkPkg/IpSecDxe/Ikev2/ChildSa.c  | 14 +++++++-------
 NetworkPkg/IpSecDxe/Ikev2/Exchange.c |  2 +-
 NetworkPkg/IpSecDxe/Ikev2/Info.c     | 17 ++++++++++++-----
 NetworkPkg/IpSecDxe/Ikev2/Payload.h  | 11 ++++++++---
 NetworkPkg/IpSecDxe/Ikev2/Sa.c       |  5 ++---
 5 files changed, 30 insertions(+), 19 deletions(-)

-- 
1.9.5.msysgit.1

[PATCH v2 1/2] NetworkPkg/IpSecDxe: Fix UEFI IKE Initial Exchange failure

[Jiaxin Wu]

*v2: update the commit log.

IKE Initial Exchange message should cover below process:
           Initiator                    Responder
Message1 HDR,SAil,KEi,Ni  ------>
Message2                  <------   HDR,SArl,KEr,Nr,[CERTREQ]
Message3 HDR,SK{}         ------>
Message4                  <------   HDR,SK{}

If Initial Exchange message is initiated by Linux IKE, it works well.
But the failure will happen if it's initiated by UEFI IKE. This issue
is caused by the no status check of NotifyCookiePayload.

While parsing the IKEv2 packet for IKE_SA_INIT exchange, if the packet
doesn't contain COOKIE Notify payload, EFI_INVALID_PARAMETER will be
returned from Ikev2ParserNotifyCookiePayload(). Current implementation
return this error status directly, then the session will be broken. The
correct behavior should check this status. If no COOKIE Notify payload,
initiator don't need to retry the IKE_SA_INIT.

Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Zhang Lubo <lubo.zhang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
---
 NetworkPkg/IpSecDxe/Ikev2/Sa.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/NetworkPkg/IpSecDxe/Ikev2/Sa.c b/NetworkPkg/IpSecDxe/Ikev2/Sa.c
index 4cbfac3..f9421ed 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/Sa.c
+++ b/NetworkPkg/IpSecDxe/Ikev2/Sa.c
@@ -285,13 +285,12 @@ Ikev2InitPskParser (
   // payload with the cookie data, initiator MUST retry the IKE_SA_INIT with a
   // Notify payload of type COOKIE containing the responder suppplied cookie data
   // as first payload and all other payloads unchanged.
   //
   if (IkeSaSession->SessionCommon.IsInitiator) {
-    if (NotifyPayload != NULL) {
-      Status = Ikev2ParserNotifyCookiePayload (NotifyPayload, IkeSaSession);
-      return Status;
+    if (NotifyPayload != NULL && !EFI_ERROR(Ikev2ParserNotifyCookiePayload (NotifyPayload, IkeSaSession))) {
+      return EFI_SUCCESS;
     }
   }
 
   if ((KeyPayload == NULL) || (SaPayload == NULL) || (NoncePayload == NULL)) {
     return EFI_INVALID_PARAMETER;
-- 
1.9.5.msysgit.1

[PATCH v2 2/2] NetworkPkg/IpSecDxe: Fix wrong IKE header "FLAG" update

[Jiaxin Wu]

*v2: update the commit log and refine the code comments.

There are three kinds of IKE Exchange process:
#1. Initial Exchange
#2. CREATE_CHILD_SA_Exchange
#3. Information Exchange

The IKE header "FLAG" update is incorrect in #2 and #3 exchange,
which may cause the continue session failure. This patch is used
to correct the updates of IKE header "FLAG" according the RFC4306
section 3.1.

Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Zhang Lubo <lubo.zhang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
---
 NetworkPkg/IpSecDxe/Ikev2/ChildSa.c  | 14 +++++++-------
 NetworkPkg/IpSecDxe/Ikev2/Exchange.c |  2 +-
 NetworkPkg/IpSecDxe/Ikev2/Info.c     | 17 ++++++++++++-----
 NetworkPkg/IpSecDxe/Ikev2/Payload.h  | 11 ++++++++---
 4 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c b/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
index 1f0199b..eaccad2 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
+++ b/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
@@ -74,13 +74,11 @@ Ikev2CreateChildGenerator (
     } else {
       IkePacket->Header->MessageId     = ChildSaSession->MessageId;
     }    
     
     if (ChildSaSession->SessionCommon.IsInitiator) {
-      IkePacket->Header->Flags = IKE_HEADER_FLAGS_CHILD_INIT;
-    } else {
-      IkePacket->Header->Flags = IKE_HEADER_FLAGS_RESPOND;
+      IkePacket->Header->Flags = IKE_HEADER_FLAGS_INIT;
     }
       
   } else {
     IkeSaSession  = (IKEV2_SA_SESSION *) SaSession;
     //
@@ -94,15 +92,17 @@ Ikev2CreateChildGenerator (
     } else {
       IkePacket->Header->MessageId     = IkeSaSession->MessageId;
     }    
     
     if (IkeSaSession->SessionCommon.IsInitiator) {
-      IkePacket->Header->Flags = IKE_HEADER_FLAGS_CHILD_INIT;
-    } else {
-      IkePacket->Header->Flags = IKE_HEADER_FLAGS_RESPOND;
+      IkePacket->Header->Flags = IKE_HEADER_FLAGS_INIT;
     }
-  } 
+  }
+
+  if (MessageId != NULL) {
+    IkePacket->Header->Flags |= IKE_HEADER_FLAGS_RESPOND;
+  }
    
   //
   // According to RFC4306, Chapter 4.
   // A minimal implementation may support the CREATE_CHILD_SA exchange only to
   // recognize requests and reject them with a Notify payload of type NO_ADDITIONAL_SAS.
diff --git a/NetworkPkg/IpSecDxe/Ikev2/Exchange.c b/NetworkPkg/IpSecDxe/Ikev2/Exchange.c
index 1eddbfb..5609964 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/Exchange.c
+++ b/NetworkPkg/IpSecDxe/Ikev2/Exchange.c
@@ -703,11 +703,11 @@ Ikev2HandleChildSa (
 
 ON_REPLY:
   //
   // Generate the reply packet if needed and send it out.
   //
-  if (IkePacket->Header->Flags != IKE_HEADER_FLAGS_RESPOND) {
+  if (!(IkePacket->Header->Flags & IKE_HEADER_FLAGS_RESPOND)) {
     Reply = mIkev2CreateChild.Generator ((UINT8 *) IkeSaSession, &IkePacket->Header->MessageId);
     if (Reply != NULL) {
       Status = Ikev2SendIkePacket (UdpService, (UINT8 *) &(IkeSaSession->SessionCommon), Reply, 0);
       if (EFI_ERROR (Status)) {
         //
diff --git a/NetworkPkg/IpSecDxe/Ikev2/Info.c b/NetworkPkg/IpSecDxe/Ikev2/Info.c
index 23e47ce..0d2b290 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/Info.c
+++ b/NetworkPkg/IpSecDxe/Ikev2/Info.c
@@ -126,11 +126,15 @@ Ikev2InfoGenerator (
     } else {
       //
       // The input parameter is not correct.
       //
       goto ERROR_EXIT;
-    } 
+    }
+
+    if (IkeSaSession->SessionCommon.IsInitiator) {
+      IkePacket->Header->Flags = IKE_HEADER_FLAGS_INIT ;
+    }  
   } else {
     //
     // Delete the Child SA Information Exchagne
     //
     ChildSaSession                     = (IKEV2_CHILD_SA_SESSION *) SaSession;
@@ -178,17 +182,20 @@ Ikev2InfoGenerator (
 
     //
     // Change the IsOnDeleting Flag
     //
     ChildSaSession->SessionCommon.IsOnDeleting = TRUE;
+
+    if (ChildSaSession->SessionCommon.IsInitiator) {
+      IkePacket->Header->Flags = IKE_HEADER_FLAGS_INIT ;
+    }
   }
 
-  if (InfoContext == NULL) {
-    IkePacket->Header->Flags = IKE_HEADER_FLAGS_INIT;
-  } else {
-    IkePacket->Header->Flags = IKE_HEADER_FLAGS_RESPOND;
+  if (InfoContext != NULL) {
+    IkePacket->Header->Flags |= IKE_HEADER_FLAGS_RESPOND;
   }
+  
   return IkePacket;
 
 ERROR_EXIT:
    if (IkePacket != NULL) {
      FreePool (IkePacket);
diff --git a/NetworkPkg/IpSecDxe/Ikev2/Payload.h b/NetworkPkg/IpSecDxe/Ikev2/Payload.h
index 6096a3b..7a85792 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/Payload.h
+++ b/NetworkPkg/IpSecDxe/Ikev2/Payload.h
@@ -1,9 +1,9 @@
 /** @file
   The Definitions related to IKEv2 payload.
 
-  Copyright (c) 2010, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
 
   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.
@@ -35,15 +35,20 @@
 #define IKEV2_PAYLOAD_TYPE_ENCRYPT  46
 #define IKEV2_PAYLOAD_TYPE_CP       47
 #define IKEV2_PAYLOAD_TYPE_EAP      48
 
 //
-// IKE header Flag for IKEv2
+// IKE header Flag (1 octet) for IKEv2, defined in RFC 4306 section 3.1 
+//
+// I(nitiator) (bit 3 of Flags, 0x08) - This bit MUST be set in messages sent by the 
+//                                      original initiator of the IKE_SA
+//
+// R(esponse) (bit 5 of Flags, 0x20)  - This bit indicates that this message is a response to 
+//                                      a message containing the same message ID.
 //
 #define IKE_HEADER_FLAGS_INIT       0x08
 #define IKE_HEADER_FLAGS_RESPOND    0x20
-#define IKE_HEADER_FLAGS_CHILD_INIT 0
 
 //
 // IKE Header Exchange Type for IKEv2
 //
 #define IKEV2_EXCHANGE_TYPE_INIT         34
-- 
1.9.5.msysgit.1

Re: [PATCH v2 0/2] NetworkPkg/IpSecDxe: IKE functionality issue fix

[Ye, Ting]

Series Reviewed-by: Ye Ting <ting.ye@intel.com> 

-----Original Message-----
From: Wu, Jiaxin 
Sent: Thursday, August 18, 2016 2:38 PM
To: edk2-devel@lists.01.org
Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Zhang, Lubo <lubo.zhang@intel.com>
Subject: [PATCH v2 0/2] NetworkPkg/IpSecDxe: IKE functionality issue fix

*v2: update the commit log and refine the code comments.

Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Zhang Lubo <lubo.zhang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>

Jiaxin Wu (2):
  NetworkPkg/IpSecDxe: Fix UEFI IKE Initial Exchange failure
  NetworkPkg/IpSecDxe: Fix wrong IKE header "FLAG" update

 NetworkPkg/IpSecDxe/Ikev2/ChildSa.c  | 14 +++++++-------  NetworkPkg/IpSecDxe/Ikev2/Exchange.c |  2 +-
 NetworkPkg/IpSecDxe/Ikev2/Info.c     | 17 ++++++++++++-----
 NetworkPkg/IpSecDxe/Ikev2/Payload.h  | 11 ++++++++---
 NetworkPkg/IpSecDxe/Ikev2/Sa.c       |  5 ++---
 5 files changed, 30 insertions(+), 19 deletions(-)

--
1.9.5.msysgit.1