SecurityPkg Variable: Implement VarCheck PROTOCOL

and follow UEFI spec to check UEFI defined variables.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>

git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@16580 6f19259b-4bc3-4df7-8a09-765794883524

1 files changed, 21 insertions, 157 deletions

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
index 04b992d5bf..86e3616e30 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
@@ -16,7 +16,7 @@
   VariableServiceSetVariable() should also check authenticate data to avoid buffer overflow,

   integer overflow. It should also check attribute to avoid authentication bypass.

 

-Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.<BR>

+Copyright (c) 2009 - 2015, 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

@@ -58,52 +58,6 @@ BOOLEAN                mEndOfDxe              = FALSE;
 ///

 BOOLEAN                mEnableLocking         = TRUE;

 

-//

-// To prevent name collisions with possible future globally defined variables,

-// other internal firmware data variables that are not defined here must be

-// saved with a unique VendorGuid other than EFI_GLOBAL_VARIABLE or

-// any other GUID defined by the UEFI Specification. Implementations must

-// only permit the creation of variables with a UEFI Specification-defined

-// VendorGuid when these variables are documented in the UEFI Specification.

-//

-GLOBAL_VARIABLE_ENTRY mGlobalVariableList[] = {

-  {EFI_LANG_CODES_VARIABLE_NAME,             VARIABLE_ATTRIBUTE_BS_RT},

-  {EFI_LANG_VARIABLE_NAME,                   VARIABLE_ATTRIBUTE_NV_BS_RT},

-  {EFI_TIME_OUT_VARIABLE_NAME,               VARIABLE_ATTRIBUTE_NV_BS_RT},

-  {EFI_PLATFORM_LANG_CODES_VARIABLE_NAME,    VARIABLE_ATTRIBUTE_BS_RT},

-  {EFI_PLATFORM_LANG_VARIABLE_NAME,          VARIABLE_ATTRIBUTE_NV_BS_RT},

-  {EFI_CON_IN_VARIABLE_NAME,                 VARIABLE_ATTRIBUTE_NV_BS_RT},

-  {EFI_CON_OUT_VARIABLE_NAME,                VARIABLE_ATTRIBUTE_NV_BS_RT},

-  {EFI_ERR_OUT_VARIABLE_NAME,                VARIABLE_ATTRIBUTE_NV_BS_RT},

-  {EFI_CON_IN_DEV_VARIABLE_NAME,             VARIABLE_ATTRIBUTE_BS_RT},

-  {EFI_CON_OUT_DEV_VARIABLE_NAME,            VARIABLE_ATTRIBUTE_BS_RT},

-  {EFI_ERR_OUT_DEV_VARIABLE_NAME,            VARIABLE_ATTRIBUTE_BS_RT},

-  {EFI_BOOT_ORDER_VARIABLE_NAME,             VARIABLE_ATTRIBUTE_NV_BS_RT},

-  {EFI_BOOT_NEXT_VARIABLE_NAME,              VARIABLE_ATTRIBUTE_NV_BS_RT},

-  {EFI_BOOT_CURRENT_VARIABLE_NAME,           VARIABLE_ATTRIBUTE_BS_RT},

-  {EFI_BOOT_OPTION_SUPPORT_VARIABLE_NAME,    VARIABLE_ATTRIBUTE_BS_RT},

-  {EFI_DRIVER_ORDER_VARIABLE_NAME,           VARIABLE_ATTRIBUTE_NV_BS_RT},

-  {EFI_HW_ERR_REC_SUPPORT_VARIABLE_NAME,     VARIABLE_ATTRIBUTE_NV_BS_RT},

-  {EFI_SETUP_MODE_NAME,                      VARIABLE_ATTRIBUTE_BS_RT},

-  {EFI_KEY_EXCHANGE_KEY_NAME,                VARIABLE_ATTRIBUTE_NV_BS_RT_AT},

-  {EFI_PLATFORM_KEY_NAME,                    VARIABLE_ATTRIBUTE_NV_BS_RT_AT},

-  {EFI_SIGNATURE_SUPPORT_NAME,               VARIABLE_ATTRIBUTE_BS_RT},

-  {EFI_SECURE_BOOT_MODE_NAME,                VARIABLE_ATTRIBUTE_BS_RT},

-  {EFI_KEK_DEFAULT_VARIABLE_NAME,            VARIABLE_ATTRIBUTE_BS_RT},

-  {EFI_PK_DEFAULT_VARIABLE_NAME,             VARIABLE_ATTRIBUTE_BS_RT},

-  {EFI_DB_DEFAULT_VARIABLE_NAME,             VARIABLE_ATTRIBUTE_BS_RT},

-  {EFI_DBX_DEFAULT_VARIABLE_NAME,            VARIABLE_ATTRIBUTE_BS_RT},

-  {EFI_DBT_DEFAULT_VARIABLE_NAME,            VARIABLE_ATTRIBUTE_BS_RT},

-  {EFI_OS_INDICATIONS_SUPPORT_VARIABLE_NAME, VARIABLE_ATTRIBUTE_BS_RT},

-  {EFI_OS_INDICATIONS_VARIABLE_NAME,         VARIABLE_ATTRIBUTE_NV_BS_RT},

-  {EFI_VENDOR_KEYS_VARIABLE_NAME,            VARIABLE_ATTRIBUTE_BS_RT},

-};

-GLOBAL_VARIABLE_ENTRY mGlobalVariableList2[] = {

-  {L"Boot####",                              VARIABLE_ATTRIBUTE_NV_BS_RT},

-  {L"Driver####",                            VARIABLE_ATTRIBUTE_NV_BS_RT},

-  {L"Key####",                               VARIABLE_ATTRIBUTE_NV_BS_RT},

-};

-

 /**

 

   SecureBoot Hook for auth variable update.

@@ -891,8 +845,8 @@ Reclaim (
           HwErrVariableTotalSize += VariableSize;

         } else if ((Variable->Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) != EFI_VARIABLE_HARDWARE_ERROR_RECORD) {

           CommonVariableTotalSize += VariableSize;

+          }

         }

-      }

       Variable = NextVariable;

     }

 

@@ -926,8 +880,8 @@ Reclaim (
           HwErrVariableTotalSize += VariableSize;

         } else if ((!IsVolatile) && ((Variable->Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) != EFI_VARIABLE_HARDWARE_ERROR_RECORD)) {

           CommonVariableTotalSize += VariableSize;

+          }

         }

-      }

       Variable = NextVariable;

     }

 

@@ -974,9 +928,9 @@ Reclaim (
             HwErrVariableTotalSize += VariableSize;

           } else if ((!IsVolatile) && ((Variable->Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) != EFI_VARIABLE_HARDWARE_ERROR_RECORD)) {

             CommonVariableTotalSize += VariableSize;

+            }

           }

         }

-      }

 

       Variable = NextVariable;

     }

@@ -997,7 +951,7 @@ Reclaim (
           HwErrVariableTotalSize += NewVariableSize;

         } else if ((NewVariable->Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) != EFI_VARIABLE_HARDWARE_ERROR_RECORD) {

           CommonVariableTotalSize += NewVariableSize;

-        }

+          }

         if ((HwErrVariableTotalSize > PcdGet32 (PcdHwErrStorageSize)) ||

             (CommonVariableTotalSize > VariableStoreHeader->Size - sizeof (VARIABLE_STORE_HEADER) - PcdGet32 (PcdHwErrStorageSize))) {

           //

@@ -1046,7 +1000,7 @@ Reclaim (
           mVariableModuleGlobal->HwErrVariableTotalSize += HEADER_ALIGN (VariableSize);

         } else if ((Variable->Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) != EFI_VARIABLE_HARDWARE_ERROR_RECORD) {

           mVariableModuleGlobal->CommonVariableTotalSize += HEADER_ALIGN (VariableSize);

-        }

+          }

 

         NextVariable = GetNextVariablePtr (NextVariable);

       }

@@ -2329,7 +2283,7 @@ UpdateVariable (
         }

         UpdateVariableInfo (VariableName, VendorGuid, FALSE, FALSE, TRUE, FALSE, FALSE);

         FlushHobVariableToFlash (VariableName, VendorGuid);

-      }

+        }

       goto Done;

     }

     //

@@ -2414,7 +2368,7 @@ UpdateVariable (
       mVariableModuleGlobal->HwErrVariableTotalSize += HEADER_ALIGN (VarSize);

     } else {

       mVariableModuleGlobal->CommonVariableTotalSize += HEADER_ALIGN (VarSize);

-    }

+      }

     //

     // update the memory copy of Flash region.

     //

@@ -2586,63 +2540,6 @@ IsHwErrRecVariable (
 }

 

 /**

-  This code checks if variable guid is global variable guid first.

-  If yes, further check if variable name is in mGlobalVariableList or mGlobalVariableList2 and attributes matched.

-

-  @param[in] VariableName       Pointer to variable name.

-  @param[in] VendorGuid         Variable Vendor Guid.

-  @param[in] Attributes         Attributes of the variable.

-

-  @retval EFI_SUCCESS           Variable is not global variable, or Variable is global variable, variable name is in the lists and attributes matched.

-  @retval EFI_INVALID_PARAMETER Variable is global variable, but variable name is not in the lists or attributes unmatched.

-

-**/

-EFI_STATUS

-EFIAPI

-CheckEfiGlobalVariable (

-  IN CHAR16             *VariableName,

-  IN EFI_GUID           *VendorGuid,

-  IN UINT32             Attributes

-  )

-{

-  UINTN     Index;

-  UINTN     NameLength;

-

-  if (CompareGuid (VendorGuid, &gEfiGlobalVariableGuid)){

-    //

-    // Try list 1, exactly match.

-    //

-    for (Index = 0; Index < sizeof (mGlobalVariableList)/sizeof (mGlobalVariableList[0]); Index++) {

-      if ((StrCmp (mGlobalVariableList[Index].Name, VariableName) == 0) &&

-          (Attributes == 0 || (Attributes & (~EFI_VARIABLE_APPEND_WRITE)) == mGlobalVariableList[Index].Attributes)) {

-        return EFI_SUCCESS;

-      }

-    }

-

-    //

-    // Try list 2.

-    //

-    NameLength = StrLen (VariableName) - 4;

-    for (Index = 0; Index < sizeof (mGlobalVariableList2)/sizeof (mGlobalVariableList2[0]); Index++) {

-      if ((StrLen (VariableName) == StrLen (mGlobalVariableList2[Index].Name)) &&

-          (StrnCmp (mGlobalVariableList2[Index].Name, VariableName, NameLength) == 0) &&

-          IsHexaDecimalDigitCharacter (VariableName[NameLength]) &&

-          IsHexaDecimalDigitCharacter (VariableName[NameLength + 1]) &&

-          IsHexaDecimalDigitCharacter (VariableName[NameLength + 2]) &&

-          IsHexaDecimalDigitCharacter (VariableName[NameLength + 3]) &&

-          (Attributes == 0 || (Attributes & (~EFI_VARIABLE_APPEND_WRITE)) == mGlobalVariableList2[Index].Attributes)) {

-        return EFI_SUCCESS;

-      }

-    }

-

-    DEBUG ((EFI_D_INFO, "[Variable]: set global variable with invalid variable name or attributes - %g:%s:%x\n", VendorGuid, VariableName, Attributes));

-    return EFI_INVALID_PARAMETER;

-  }

-

-  return EFI_SUCCESS;

-}

-

-/**

   Mark a variable that will become read-only after leaving the DXE phase of execution.

 

   @param[in] This          The VARIABLE_LOCK_PROTOCOL instance.

@@ -2666,6 +2563,7 @@ VariableLockRequestToLock (
   )

 {

   VARIABLE_ENTRY                  *Entry;

+  CHAR16                          *Name;

 

   if (VariableName == NULL || VariableName[0] == 0 || VendorGuid == NULL) {

     return EFI_INVALID_PARAMETER;

@@ -2675,7 +2573,7 @@ VariableLockRequestToLock (
     return EFI_ACCESS_DENIED;

   }

 

-  Entry = AllocateRuntimePool (sizeof (*Entry) + StrSize (VariableName));

+  Entry = AllocateRuntimeZeroPool (sizeof (*Entry) + StrSize (VariableName));

   if (Entry == NULL) {

     return EFI_OUT_OF_RESOURCES;

   }

@@ -2684,8 +2582,8 @@ VariableLockRequestToLock (
 

   AcquireLockOnlyAtBootTime(&mVariableModuleGlobal->VariableGlobal.VariableServicesLock);

 

-  Entry->Name = (CHAR16 *) (Entry + 1);

-  StrCpy   (Entry->Name, VariableName);

+  Name = (CHAR16 *) ((UINTN) Entry + sizeof (*Entry));

+  StrnCpy   (Name, VariableName, StrLen (VariableName));

   CopyGuid (&Entry->Guid, VendorGuid);

   InsertTailList (&mLockedVariableList, &Entry->Link);

 

@@ -2695,39 +2593,6 @@ VariableLockRequestToLock (
 }

 

 /**

-  This code checks if variable should be treated as read-only variable.

-

-  @param[in]      VariableName            Name of the Variable.

-  @param[in]      VendorGuid              GUID of the Variable.

-

-  @retval TRUE      This variable is read-only variable.

-  @retval FALSE     This variable is NOT read-only variable.

-

-**/

-BOOLEAN

-IsReadOnlyVariable (

-  IN     CHAR16         *VariableName,

-  IN     EFI_GUID       *VendorGuid

-  )

-{

-  if (CompareGuid (VendorGuid, &gEfiGlobalVariableGuid)) {

-    if ((StrCmp (VariableName, EFI_SETUP_MODE_NAME) == 0) ||

-        (StrCmp (VariableName, EFI_SIGNATURE_SUPPORT_NAME) == 0) ||

-        (StrCmp (VariableName, EFI_SECURE_BOOT_MODE_NAME) == 0) ||

-        (StrCmp (VariableName, EFI_VENDOR_KEYS_VARIABLE_NAME) == 0) ||

-        (StrCmp (VariableName, EFI_KEK_DEFAULT_VARIABLE_NAME) == 0) ||

-        (StrCmp (VariableName, EFI_PK_DEFAULT_VARIABLE_NAME) == 0) ||

-        (StrCmp (VariableName, EFI_DB_DEFAULT_VARIABLE_NAME) == 0) ||

-        (StrCmp (VariableName, EFI_DBX_DEFAULT_VARIABLE_NAME) == 0) ||

-        (StrCmp (VariableName, EFI_DBT_DEFAULT_VARIABLE_NAME) == 0)) {

-      return TRUE;

-    }

-  }

-

-  return FALSE;

-}

-

-/**

 

   This code finds variable in storage blocks (Volatile or Non-Volatile).

 

@@ -3015,6 +2880,7 @@ VariableServiceSetVariable (
   UINTN                               PayloadSize;

   LIST_ENTRY                          *Link;

   VARIABLE_ENTRY                      *Entry;

+  CHAR16                              *Name;

 

   //

   // Check input parameters.

@@ -3023,10 +2889,6 @@ VariableServiceSetVariable (
     return EFI_INVALID_PARAMETER;

   }

 

-  if (IsReadOnlyVariable (VariableName, VendorGuid)) {

-    return EFI_WRITE_PROTECTED;

-  }

-

   if (DataSize != 0 && Data == NULL) {

     return EFI_INVALID_PARAMETER;

   }

@@ -3105,11 +2967,6 @@ VariableServiceSetVariable (
     }

   }

 

-  Status = CheckEfiGlobalVariable (VariableName, VendorGuid, Attributes);

-  if (EFI_ERROR (Status)) {

-    return Status;

-  }

-

   AcquireLockOnlyAtBootTime(&mVariableModuleGlobal->VariableGlobal.VariableServicesLock);

 

   //

@@ -3136,7 +2993,8 @@ VariableServiceSetVariable (
         ; Link = GetNextNode (&mLockedVariableList, Link)

         ) {

       Entry = BASE_CR (Link, VARIABLE_ENTRY, Link);

-      if (CompareGuid (&Entry->Guid, VendorGuid) && (StrCmp (Entry->Name, VariableName) == 0)) {

+      Name = (CHAR16 *) ((UINTN) Entry + sizeof (*Entry));

+      if (CompareGuid (&Entry->Guid, VendorGuid) && (StrCmp (Name, VariableName) == 0)) {

         Status = EFI_WRITE_PROTECTED;

         DEBUG ((EFI_D_INFO, "[Variable]: Changing readonly variable after leaving DXE phase - %g:%s\n", VendorGuid, VariableName));

         goto Done;

@@ -3144,6 +3002,11 @@ VariableServiceSetVariable (
     }

   }

 

+  Status = InternalVarCheckSetVariableCheck (VariableName, VendorGuid, Attributes, PayloadSize, (VOID *) ((UINTN) Data + DataSize - PayloadSize));

+  if (EFI_ERROR (Status)) {

+    goto Done;

+  }

+

   //

   // Check whether the input variable is already existed.

   //

@@ -3161,6 +3024,7 @@ VariableServiceSetVariable (
       // 2. The only attribute differing is EFI_VARIABLE_APPEND_WRITE

       //

       Status = EFI_INVALID_PARAMETER;

+      DEBUG ((EFI_D_INFO, "[Variable]: Rewritten a preexisting variable with different attributes - %g:%s\n", VendorGuid, VariableName));

       goto Done;

     }

   }