Revision: 19652
          http://sourceforge.net/p/edk2/code/19652
Author:   jiaxinwu
Date:     2016-01-18 01:59:16 +0000 (Mon, 18 Jan 2016)
Log Message:
-----------
NetworkPkg: Fix IpSec SPD and SAD mapping issue when SPD is updated

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 <[email protected]>
Cc: Fu Siyuan <[email protected]>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu <[email protected]>
Reviewed-by: Ye Ting <[email protected]>
Reviewed-by: Fu Siyuan <[email protected]>

Modified Paths:
--------------
    trunk/edk2/NetworkPkg/IpSecDxe/IpSecConfigImpl.c

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


------------------------------------------------------------------------------
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=267308311&iu=/4140
_______________________________________________
edk2-commits mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-commits

Reply via email to