NetworkPkg: Fix IpSec SPD and SAD mapping issue when SPD is updated
authorJiaxin Wu <jiaxin.wu@intel.com>
Mon, 18 Jan 2016 01:59:16 +0000 (01:59 +0000)
committerjiaxinwu <jiaxinwu@Edk2>
Mon, 18 Jan 2016 01:59:16 +0000 (01:59 +0000)
The current implementation doesn't handle the relationship between
SPD and SAD well, which may introduce some security and connection
issue after SPD updated.
For SPD SetData policy:
 A) When delete the existed SPD entry, its related SAs also should be
removed from its Sas list(SadEntry->BySpd). If the SA entry is
established by IKE, we can remove it from global SAD list(SadEntry->List)
and then free it directly since its SpdEntry will be freed later.
 B) SPD SetData operation should do some setting date validity-check.
For example, whether the SaId specified by setting Data is valid. If
the setting date is invalid, EFI_INVALID_PARAMETER should be returned.

Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
Reviewed-by: Ye Ting <ting.ye@intel.com>
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@19652 6f19259b-4bc3-4df7-8a09-765794883524

NetworkPkg/IpSecDxe/IpSecConfigImpl.c

index 8c7724c..e1b24e4 100644 (file)
@@ -1,7 +1,7 @@
 /** @file\r
   The implementation of IPSEC_CONFIG_PROTOCOL.\r
 \r
-  Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.<BR>\r
+  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>\r
 \r
   This program and the accompanying materials\r
   are licensed and made available under the terms and conditions of the BSD License\r
@@ -211,7 +211,7 @@ CompareSpdSelector (
   }\r
   \r
   //\r
-  // Compare the all LocalAddress fields in the two Spdselectors.\r
+  // Compare the all LocalAddress and RemoteAddress fields in the two Spdselectors.\r
   // First, SpdSel1->LocalAddress to SpdSel2->LocalAddress && Compare \r
   // SpdSel1->RemoteAddress to SpdSel2->RemoteAddress. If all match, return\r
   // TRUE.\r
@@ -372,7 +372,7 @@ IsSubSpdSelector (
   }\r
   \r
   //\r
-  // Compare the all LocalAddress fields in the two Spdselectors.\r
+  // Compare the all LocalAddress and RemoteAddress fields in the two Spdselectors.\r
   // First, SpdSel1->LocalAddress to SpdSel2->LocalAddress && Compare \r
   // SpdSel1->RemoteAddress to SpdSel2->RemoteAddress. If all match, return\r
   // TRUE.\r
@@ -429,9 +429,9 @@ IsSubSpdSelector (
   }\r
   \r
   //\r
-  // Compare the all LocalAddress fields in the two Spdselectors.\r
-  // First, SpdSel1->LocalAddress to SpdSel2->LocalAddress && Compare \r
-  // SpdSel1->RemoteAddress to SpdSel2->RemoteAddress. If all match, return\r
+  // Compare the all LocalAddress and RemoteAddress fields in the two Spdselectors.\r
+  // First, SpdSel1->LocalAddress to SpdSel2->RemoteAddress && Compare \r
+  // SpdSel1->RemoteAddress to SpdSel2->LocalAddress. If all match, return\r
   // TRUE.\r
   //\r
   for (Index = 0; Index < SpdSel1->LocalAddressCount; Index++) {\r
@@ -1018,6 +1018,8 @@ UnfixPadEntry (
                                      mode is Tunnel, and its tunnel option is NULL.\r
                                    - The Action of Data is protected and its policy \r
                                      mode is not Tunnel and it tunnel option is not NULL.\r
+                                   - SadEntry requied to be set into new SpdEntry's Sas has \r
+                                     been found but it is invalid.\r
   @retval EFI_OUT_OF_RESOURCED  The required system resource could not be allocated.\r
   @retval EFI_SUCCESS           The specified configuration data was obtained successfully.\r
 \r
@@ -1039,6 +1041,7 @@ SetSpdEntry (
   LIST_ENTRY              *Entry;\r
   LIST_ENTRY              *Entry2;\r
   LIST_ENTRY              *NextEntry;\r
+  LIST_ENTRY              *NextEntry2;\r
   IPSEC_SPD_ENTRY         *SpdEntry;\r
   IPSEC_SAD_ENTRY         *SadEntry;\r
   UINTN                   SpdEntrySize;\r
@@ -1097,11 +1100,22 @@ SetSpdEntry (
       SpdSas = &SpdEntry->Data->Sas;\r
       \r
       //\r
-      // TODO: Deleted the related SAs.\r
+      // Remove the related SAs from Sas(SadEntry->BySpd). If the SA entry is established by \r
+      // IKE, remove from mConfigData list(SadEntry->List) and then free it directly since its \r
+      // SpdEntry will be freed later.\r
       //\r
-      NET_LIST_FOR_EACH (Entry2, SpdSas) {\r
-        SadEntry                  = IPSEC_SAD_ENTRY_FROM_SPD (Entry2);\r
-        SadEntry->Data->SpdEntry  = NULL;\r
+      NET_LIST_FOR_EACH_SAFE (Entry2, NextEntry2, SpdSas) {\r
+        SadEntry = IPSEC_SAD_ENTRY_FROM_SPD (Entry2);\r
+        \r
+        if (SadEntry->Data->SpdEntry != NULL) {\r
+          RemoveEntryList (&SadEntry->BySpd);\r
+          SadEntry->Data->SpdEntry = NULL;\r
+        }\r
+        \r
+        if (!(SadEntry->Data->ManualSet)) {\r
+          RemoveEntryList (&SadEntry->List);\r
+          FreePool (SadEntry);\r
+        }\r
       }\r
       \r
       //\r
@@ -1194,20 +1208,30 @@ SetSpdEntry (
   NET_LIST_FOR_EACH (Entry, SadList) {\r
     SadEntry = IPSEC_SAD_ENTRY_FROM_LIST (Entry);\r
 \r
-    for (Index = 0; Index < SpdData->SaIdCount; Index++) {\r
-\r
-      if (CompareSaId (\r
-            (EFI_IPSEC_CONFIG_SELECTOR *) &SpdData->SaId[Index],\r
-            (EFI_IPSEC_CONFIG_SELECTOR *) SadEntry->Id\r
-            )) {\r
-        if (SadEntry->Data->SpdEntry != NULL) {  \r
-          RemoveEntryList (&SadEntry->BySpd);\r
+      for (Index = 0; Index < SpdData->SaIdCount; Index++) {\r
+        if (CompareSaId (\r
+              (EFI_IPSEC_CONFIG_SELECTOR *) &SpdData->SaId[Index],\r
+              (EFI_IPSEC_CONFIG_SELECTOR *) SadEntry->Id\r
+              )) {\r
+          //\r
+          // Check whether the found SadEntry is vaild.\r
+          //\r
+          if (IsSubSpdSelector (\r
+                (EFI_IPSEC_CONFIG_SELECTOR *) SadEntry->Data->SpdSelector,\r
+                (EFI_IPSEC_CONFIG_SELECTOR *) SpdEntry->Selector\r
+                )) {\r
+            if (SadEntry->Data->SpdEntry != NULL) {\r
+              RemoveEntryList (&SadEntry->BySpd);\r
+            }\r
+            InsertTailList (&SpdEntry->Data->Sas, &SadEntry->BySpd);\r
+            SadEntry->Data->SpdEntry = SpdEntry;\r
+          } else {\r
+            return EFI_INVALID_PARAMETER;\r
+          }\r
         }\r
-        InsertTailList (&SpdEntry->Data->Sas, &SadEntry->BySpd);\r
-        SadEntry->Data->SpdEntry = SpdEntry;             \r
-      }\r
-    }\r
+      }      \r
   }\r
+  \r
   //\r
   // Insert the new SPD entry.\r
   //\r