]> git.proxmox.com Git - mirror_edk2.git/blobdiff - SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
Fix the return value bug when updating public key database variable failure.
[mirror_edk2.git] / SecurityPkg / VariableAuthenticated / RuntimeDxe / AuthService.c
index cf8ad9969696a06e1c70095246724d9063fc6fc7..3db3d29e69e854f63800c8eaee3f15e01e7c2eb6 100644 (file)
@@ -15,7 +15,7 @@
   They will do basic validation for authentication data structure, then call crypto library\r
   to verify the signature.\r
 \r
-Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.<BR>\r
+Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.<BR>\r
 This program and the accompanying materials\r
 are licensed and made available under the terms and conditions of the BSD License\r
 which accompanies this distribution.  The full text of the license may be found at\r
@@ -36,6 +36,8 @@ UINT8    mPubKeyStore[MAX_KEYDB_SIZE];
 UINT32   mPubKeyNumber;\r
 UINT8    mCertDbStore[MAX_CERTDB_SIZE];\r
 UINT32   mPlatformMode;\r
+UINT8    mVendorKeyState;\r
+\r
 EFI_GUID mSignatureSupport[] = {EFI_CERT_SHA1_GUID, EFI_CERT_SHA256_GUID, EFI_CERT_RSA2048_GUID, EFI_CERT_X509_GUID};\r
 //\r
 // Public Exponent of RSA Key.\r
@@ -46,14 +48,6 @@ CONST UINT8 mRsaE[] = { 0x01, 0x00, 0x01 };
 //\r
 VOID  *mHashCtx = NULL;\r
 \r
-//\r
-// Pointer to runtime buffer.\r
-// For "Append" operation to an existing variable, a read/modify/write operation\r
-// is supported by firmware internally. Reserve runtime buffer to cache previous\r
-// variable data in runtime phase because memory allocation is forbidden in virtual mode.\r
-//\r
-VOID  *mStorageArea = NULL;\r
-\r
 //\r
 // The serialization of the values of the VariableName, VendorGuid and Attributes\r
 // parameters of the SetVariable() call and the TimeStamp component of the\r
@@ -189,14 +183,6 @@ AutenticatedVariableServiceInitialize (
     return EFI_OUT_OF_RESOURCES;\r
   }\r
 \r
-  //\r
-  // Reserved runtime buffer for "Append" operation in virtual mode.\r
-  //\r
-  mStorageArea  = AllocateRuntimePool (PcdGet32 (PcdMaxVariableSize));\r
-  if (mStorageArea == NULL) {\r
-    return EFI_OUT_OF_RESOURCES;\r
-  }\r
-\r
   //\r
   // Prepare runtime buffer for serialized data of time-based authenticated\r
   // Variable, i.e. (VariableName, VendorGuid, Attributes, TimeStamp, Data).\r
@@ -255,7 +241,7 @@ AutenticatedVariableServiceInitialize (
   }\r
   \r
   //\r
-  // Create "SetupMode" varable with BS+RT attribute set.\r
+  // Create "SetupMode" variable with BS+RT attribute set.\r
   //\r
   FindVariable (EFI_SETUP_MODE_NAME, &gEfiGlobalVariableGuid, &Variable, &mVariableModuleGlobal->VariableGlobal, FALSE);\r
   if (PkVariable.CurrPtr == NULL) {\r
@@ -279,7 +265,7 @@ AutenticatedVariableServiceInitialize (
   }\r
   \r
   //\r
-  // Create "SignatureSupport" varable with BS+RT attribute set.\r
+  // Create "SignatureSupport" variable with BS+RT attribute set.\r
   //\r
   FindVariable (EFI_SIGNATURE_SUPPORT_NAME, &gEfiGlobalVariableGuid, &Variable, &mVariableModuleGlobal->VariableGlobal, FALSE);\r
   Status  = UpdateVariable (\r
@@ -328,7 +314,7 @@ AutenticatedVariableServiceInitialize (
   }\r
 \r
   //\r
-  // Create "SecureBoot" varable with BS+RT attribute set.\r
+  // Create "SecureBoot" variable with BS+RT attribute set.\r
   //\r
   if (SecureBootEnable == SECURE_BOOT_ENABLE && mPlatformMode == USER_MODE) {\r
     SecureBootMode = SECURE_BOOT_MODE_ENABLE;\r
@@ -356,30 +342,23 @@ AutenticatedVariableServiceInitialize (
   DEBUG ((EFI_D_INFO, "Variable %s is %x\n", EFI_SECURE_BOOT_ENABLE_NAME, SecureBootEnable));\r
 \r
   //\r
-  // Check "CustomMode" variable's existence.\r
+  // Initialize "CustomMode" in STANDARD_SECURE_BOOT_MODE state.\r
   //\r
   FindVariable (EFI_CUSTOM_MODE_NAME, &gEfiCustomModeEnableGuid, &Variable, &mVariableModuleGlobal->VariableGlobal, FALSE);\r
-  if (Variable.CurrPtr != NULL) {\r
-    CustomMode = *(GetVariableDataPtr (Variable.CurrPtr));\r
-  } else {\r
-    //\r
-    // "CustomMode" not exist, initialize it in STANDARD_SECURE_BOOT_MODE.\r
-    //\r
-    CustomMode = STANDARD_SECURE_BOOT_MODE;\r
-    Status = UpdateVariable (\r
-               EFI_CUSTOM_MODE_NAME,\r
-               &gEfiCustomModeEnableGuid,\r
-               &CustomMode,\r
-               sizeof (UINT8),\r
-               EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,\r
-               0,\r
-               0,\r
-               &Variable,\r
-               NULL\r
-               );\r
-    if (EFI_ERROR (Status)) {\r
-      return Status;\r
-    }\r
+  CustomMode = STANDARD_SECURE_BOOT_MODE;\r
+  Status = UpdateVariable (\r
+             EFI_CUSTOM_MODE_NAME,\r
+             &gEfiCustomModeEnableGuid,\r
+             &CustomMode,\r
+             sizeof (UINT8),\r
+             EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,\r
+             0,\r
+             0,\r
+             &Variable,\r
+             NULL\r
+             );\r
+  if (EFI_ERROR (Status)) {\r
+    return Status;\r
   }\r
   \r
   DEBUG ((EFI_D_INFO, "Variable %s is %x\n", EFI_CUSTOM_MODE_NAME, CustomMode));\r
@@ -416,6 +395,54 @@ AutenticatedVariableServiceInitialize (
     }\r
   }  \r
 \r
+  //\r
+  // Check "VendorKeysNv" variable's existence and create "VendorKeys" variable accordingly.\r
+  //\r
+  FindVariable (EFI_VENDOR_KEYS_NV_VARIABLE_NAME, &gEfiVendorKeysNvGuid, &Variable, &mVariableModuleGlobal->VariableGlobal, FALSE);\r
+  if (Variable.CurrPtr != NULL) {\r
+    mVendorKeyState = *(GetVariableDataPtr (Variable.CurrPtr));\r
+  } else {\r
+    //\r
+    // "VendorKeysNv" not exist, initialize it in VENDOR_KEYS_VALID state.\r
+    //\r
+    mVendorKeyState = VENDOR_KEYS_VALID;\r
+    Status = UpdateVariable (\r
+               EFI_VENDOR_KEYS_NV_VARIABLE_NAME,\r
+               &gEfiVendorKeysNvGuid,\r
+               &mVendorKeyState,\r
+               sizeof (UINT8),\r
+               EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS,\r
+               0,\r
+               0,\r
+               &Variable,\r
+               NULL\r
+               );\r
+    if (EFI_ERROR (Status)) {\r
+      return Status;\r
+    }\r
+  }\r
+\r
+  //\r
+  // Create "VendorKeys" variable with BS+RT attribute set.\r
+  //\r
+  FindVariable (EFI_VENDOR_KEYS_VARIABLE_NAME, &gEfiGlobalVariableGuid, &Variable, &mVariableModuleGlobal->VariableGlobal, FALSE);\r
+  Status = UpdateVariable (\r
+             EFI_VENDOR_KEYS_VARIABLE_NAME,\r
+             &gEfiGlobalVariableGuid,\r
+             &mVendorKeyState,\r
+             sizeof (UINT8),\r
+             EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS,\r
+             0,\r
+             0,\r
+             &Variable,\r
+             NULL\r
+             );\r
+  if (EFI_ERROR (Status)) {\r
+    return Status;\r
+  }\r
+\r
+  DEBUG ((EFI_D_INFO, "Variable %s is %x\n", EFI_VENDOR_KEYS_VARIABLE_NAME, mVendorKeyState));\r
+\r
   return Status;\r
 }\r
 \r
@@ -437,6 +464,8 @@ AddPubKeyInStore (
   UINT32                  Index;\r
   VARIABLE_POINTER_TRACK  Variable;\r
   UINT8                   *Ptr;\r
+  UINT8                   *Data;\r
+  UINTN                   DataSize;\r
 \r
   if (PubKey == NULL) {\r
     return 0;\r
@@ -449,7 +478,11 @@ AddPubKeyInStore (
              &mVariableModuleGlobal->VariableGlobal,\r
              FALSE\r
              );\r
-  ASSERT_EFI_ERROR (Status);\r
+  if (EFI_ERROR (Status)) {\r
+    DEBUG ((EFI_D_ERROR, "Get public key database variable failure, Status = %r\n", Status));\r
+    return 0;\r
+  }\r
+\r
   //\r
   // Check whether the public key entry does exist.\r
   //\r
@@ -468,9 +501,49 @@ AddPubKeyInStore (
     //\r
     if (mPubKeyNumber == MAX_KEY_NUM) {\r
       //\r
-      // Notes: Database is full, need enhancement here, currently just return 0.\r
+      // Public key dadatase is full, try to reclaim invalid key.\r
       //\r
-      return 0;\r
+      if (AtRuntime ()) {\r
+        //\r
+        // NV storage can't reclaim at runtime.\r
+        //\r
+        return 0;\r
+      }\r
+      \r
+      Status = Reclaim (\r
+                 mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase,\r
+                 &mVariableModuleGlobal->NonVolatileLastVariableOffset,\r
+                 FALSE,\r
+                 NULL,\r
+                 NULL,\r
+                 0,\r
+                 TRUE\r
+                 );\r
+      if (EFI_ERROR (Status)) {\r
+        return 0;\r
+      }\r
+\r
+      Status = FindVariable (\r
+                 AUTHVAR_KEYDB_NAME,\r
+                 &gEfiAuthenticatedVariableGuid,\r
+                 &Variable,\r
+                 &mVariableModuleGlobal->VariableGlobal,\r
+                 FALSE\r
+                 );\r
+      if (EFI_ERROR (Status)) {\r
+        DEBUG ((EFI_D_ERROR, "Get public key database variable failure, Status = %r\n", Status));\r
+        return 0;\r
+      }\r
+\r
+      DataSize  = DataSizeOfVariable (Variable.CurrPtr);\r
+      Data      = GetVariableDataPtr (Variable.CurrPtr);\r
+      ASSERT ((DataSize != 0) && (Data != NULL));\r
+      CopyMem (mPubKeyStore, (UINT8 *) Data, DataSize);\r
+      mPubKeyNumber = (UINT32) (DataSize / EFI_CERT_TYPE_RSA2048_SIZE);\r
+\r
+      if (mPubKeyNumber == MAX_KEY_NUM) {\r
+        return 0;\r
+      }     \r
     }\r
 \r
     CopyMem (mPubKeyStore + mPubKeyNumber * EFI_CERT_TYPE_RSA2048_SIZE, PubKey, EFI_CERT_TYPE_RSA2048_SIZE);\r
@@ -489,7 +562,10 @@ AddPubKeyInStore (
                &Variable,\r
                NULL\r
                );\r
-    ASSERT_EFI_ERROR (Status);\r
+    if (EFI_ERROR (Status)) {\r
+      DEBUG ((EFI_D_ERROR, "Update public key database variable failure, Status = %r\n", Status));\r
+      return 0;\r
+    }\r
   }\r
 \r
   return Index;\r
@@ -526,7 +602,9 @@ VerifyCounterBasedPayload (
   EFI_CERT_BLOCK_RSA_2048_SHA256  *CertBlock;\r
   UINT8                           Digest[SHA256_DIGEST_SIZE];\r
   VOID                            *Rsa;\r
-\r
+  UINTN                           PayloadSize;\r
+  \r
+  PayloadSize = DataSize - AUTHINFO_SIZE;\r
   Rsa         = NULL;\r
   CertData    = NULL;\r
   CertBlock   = NULL;\r
@@ -558,7 +636,14 @@ VerifyCounterBasedPayload (
   if (!Status) {\r
     goto Done;\r
   }\r
-  Status  = Sha256Update (mHashCtx, Data + AUTHINFO_SIZE, (UINTN) (DataSize - AUTHINFO_SIZE));\r
+  Status  = Sha256Update (mHashCtx, Data + AUTHINFO_SIZE, PayloadSize);\r
+  if (!Status) {\r
+    goto Done;\r
+  }\r
+  //\r
+  // Hash Size.\r
+  //\r
+  Status  = Sha256Update (mHashCtx, &PayloadSize, sizeof (UINTN));\r
   if (!Status) {\r
     goto Done;\r
   }\r
@@ -628,7 +713,6 @@ UpdatePlatformMode (
 {\r
   EFI_STATUS              Status;\r
   VARIABLE_POINTER_TRACK  Variable;\r
-  UINT32                  VarAttr;\r
   UINT8                   SecureBootMode;\r
   UINT8                   SecureBootEnable;\r
   UINTN                   VariableDataSize;\r
@@ -689,13 +773,12 @@ UpdatePlatformMode (
     }\r
   }\r
 \r
-  VarAttr = EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS;\r
   Status  = UpdateVariable (\r
               EFI_SECURE_BOOT_MODE_NAME,\r
               &gEfiGlobalVariableGuid,\r
               &SecureBootMode,\r
               sizeof(UINT8),\r
-              VarAttr,\r
+              EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS,\r
               0,\r
               0,\r
               &Variable,\r
@@ -866,6 +949,56 @@ CheckSignatureListFormat(
   return EFI_SUCCESS;\r
 }\r
 \r
+/**\r
+  Update "VendorKeys" variable to record the out of band secure boot key modification.\r
+\r
+  @return EFI_SUCCESS           Variable is updated successfully.\r
+  @return Others                Failed to update variable.\r
+  \r
+**/\r
+EFI_STATUS\r
+VendorKeyIsModified (\r
+  VOID\r
+  )\r
+{\r
+  EFI_STATUS              Status;\r
+  VARIABLE_POINTER_TRACK  Variable;\r
+\r
+  if (mVendorKeyState == VENDOR_KEYS_MODIFIED) {\r
+    return EFI_SUCCESS;\r
+  }\r
+  mVendorKeyState = VENDOR_KEYS_MODIFIED;\r
+  \r
+  FindVariable (EFI_VENDOR_KEYS_NV_VARIABLE_NAME, &gEfiVendorKeysNvGuid, &Variable, &mVariableModuleGlobal->VariableGlobal, FALSE);\r
+  Status = UpdateVariable (\r
+             EFI_VENDOR_KEYS_NV_VARIABLE_NAME,\r
+             &gEfiVendorKeysNvGuid,\r
+             &mVendorKeyState,\r
+             sizeof (UINT8),\r
+             EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS,\r
+             0,\r
+             0,\r
+             &Variable,\r
+             NULL\r
+             );\r
+  if (EFI_ERROR (Status)) {\r
+    return Status;\r
+  }\r
+\r
+  FindVariable (EFI_VENDOR_KEYS_VARIABLE_NAME, &gEfiGlobalVariableGuid, &Variable, &mVariableModuleGlobal->VariableGlobal, FALSE);\r
+  return UpdateVariable (\r
+           EFI_VENDOR_KEYS_VARIABLE_NAME,\r
+           &gEfiGlobalVariableGuid,\r
+           &mVendorKeyState,\r
+           sizeof (UINT8),\r
+           EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS,\r
+           0,\r
+           0,\r
+           &Variable,\r
+           NULL\r
+           );\r
+}\r
+\r
 /**\r
   Process variable with platform key for verification.\r
 \r
@@ -940,6 +1073,13 @@ ProcessVarWithPk (
                Variable,\r
                &((EFI_VARIABLE_AUTHENTICATION_2 *) Data)->TimeStamp\r
                );\r
+    if (EFI_ERROR(Status)) {\r
+      return Status;\r
+    }\r
+\r
+    if ((mPlatformMode != SETUP_MODE) || IsPk) {\r
+      Status = VendorKeyIsModified ();\r
+    }\r
   } else if (mPlatformMode == USER_MODE) {\r
     //\r
     // Verify against X509 Cert in PK database.\r
@@ -1072,6 +1212,13 @@ ProcessVarWithKek (
                Variable,\r
                &((EFI_VARIABLE_AUTHENTICATION_2 *) Data)->TimeStamp\r
                );\r
+    if (EFI_ERROR (Status)) {\r
+      return Status;\r
+    }\r
+\r
+    if (mPlatformMode != SETUP_MODE) {\r
+      Status = VendorKeyIsModified ();\r
+    }\r
   }\r
 \r
   return Status;\r
@@ -1099,6 +1246,7 @@ ProcessVarWithKek (
   @return EFI_INVALID_PARAMETER           Invalid parameter.\r
   @return EFI_WRITE_PROTECTED             Variable is write-protected and needs authentication with\r
                                           EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS set.\r
+  @return EFI_OUT_OF_RESOURCES            The Database to save the public key is full.\r
   @return EFI_SECURITY_VIOLATION          The variable is with EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS\r
                                           set, but the AuthInfo does NOT pass the validation\r
                                           check carried out by the firmware.\r
@@ -1137,6 +1285,22 @@ ProcessVariable (
     return EFI_SECURITY_VIOLATION;\r
   }\r
   \r
+  //\r
+  // A time-based authenticated variable and a count-based authenticated variable\r
+  // can't be updated by each other.\r
+  // \r
+  if (Variable->CurrPtr != NULL) {    \r
+    if (((Attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) != 0) &&\r
+        ((Variable->CurrPtr->Attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) != 0)) {\r
+      return EFI_SECURITY_VIOLATION;      \r
+    }\r
+    \r
+    if (((Attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) != 0) && \r
+        ((Variable->CurrPtr->Attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) != 0)) {\r
+      return EFI_SECURITY_VIOLATION;      \r
+    }\r
+  }\r
+    \r
   //\r
   // Process Time-based Authenticated variable.\r
   //\r
@@ -1237,7 +1401,7 @@ ProcessVariable (
     //\r
     KeyIndex = AddPubKeyInStore (PubKey);\r
     if (KeyIndex == 0) {\r
-      return EFI_SECURITY_VIOLATION;\r
+      return EFI_OUT_OF_RESOURCES;\r
     }\r
   }\r
 \r
@@ -1252,20 +1416,24 @@ ProcessVariable (
   will be appended to the original EFI_SIGNATURE_LIST, duplicate EFI_SIGNATURE_DATA\r
   will be ignored.\r
 \r
-  @param[in, out]  Data            Pointer to original EFI_SIGNATURE_LIST.\r
-  @param[in]       DataSize        Size of Data buffer.\r
-  @param[in]       NewData         Pointer to new EFI_SIGNATURE_LIST to be appended.\r
-  @param[in]       NewDataSize     Size of NewData buffer.\r
+  @param[in, out]  Data              Pointer to original EFI_SIGNATURE_LIST.\r
+  @param[in]       DataSize          Size of Data buffer.\r
+  @param[in]       FreeBufSize       Size of free data buffer \r
+  @param[in]       NewData           Pointer to new EFI_SIGNATURE_LIST to be appended.\r
+  @param[in]       NewDataSize       Size of NewData buffer.\r
+  @param[out]      MergedBufSize     Size of the merged buffer\r
 \r
-  @return Size of the merged buffer.\r
+  @return EFI_BUFFER_TOO_SMALL if input Data buffer overflowed\r
 \r
 **/\r
-UINTN\r
+EFI_STATUS\r
 AppendSignatureList (\r
   IN  OUT VOID            *Data,\r
   IN  UINTN               DataSize,\r
+  IN  UINTN               FreeBufSize,\r
   IN  VOID                *NewData,\r
-  IN  UINTN               NewDataSize\r
+  IN  UINTN               NewDataSize,\r
+  OUT UINTN               *MergedBufSize\r
   )\r
 {\r
   EFI_SIGNATURE_LIST  *CertList;\r
@@ -1324,15 +1492,25 @@ AppendSignatureList (
         // New EFI_SIGNATURE_DATA, append it.\r
         //\r
         if (CopiedCount == 0) {\r
+          if (FreeBufSize < sizeof (EFI_SIGNATURE_LIST) + NewCertList->SignatureHeaderSize) {\r
+            return EFI_BUFFER_TOO_SMALL;\r
+          }\r
+\r
           //\r
           // Copy EFI_SIGNATURE_LIST header for only once.\r
           //\r
+\r
           CopyMem (Tail, NewCertList, sizeof (EFI_SIGNATURE_LIST) + NewCertList->SignatureHeaderSize);\r
           Tail = Tail + sizeof (EFI_SIGNATURE_LIST) + NewCertList->SignatureHeaderSize;\r
+          FreeBufSize -= sizeof (EFI_SIGNATURE_LIST) + NewCertList->SignatureHeaderSize;\r
         }\r
 \r
+        if (FreeBufSize < NewCertList->SignatureSize) {\r
+          return EFI_BUFFER_TOO_SMALL;\r
+        }\r
         CopyMem (Tail, NewCert, NewCertList->SignatureSize);\r
         Tail += NewCertList->SignatureSize;\r
+        FreeBufSize -= NewCertList->SignatureSize;\r
         CopiedCount++;\r
       }\r
 \r
@@ -1352,7 +1530,8 @@ AppendSignatureList (
     NewCertList = (EFI_SIGNATURE_LIST *) ((UINT8 *) NewCertList + NewCertList->SignatureListSize);\r
   }\r
 \r
-  return (Tail - (UINT8 *) Data);\r
+  *MergedBufSize = (Tail - (UINT8 *) Data);\r
+  return EFI_SUCCESS;\r
 }\r
 \r
 /**\r
@@ -2004,7 +2183,24 @@ VerifyTimeBasedPayload (
 \r
   if (AuthVarType == AuthVarTypePk) {\r
     //\r
-    // Get platform key from variable.\r
+    // Verify that the signature has been made with the current Platform Key (no chaining for PK).\r
+    // First, get signer's certificates from SignedData.\r
+    //\r
+    VerifyStatus = Pkcs7GetSigners (\r
+                     SigData,\r
+                     SigDataSize,\r
+                     &SignerCerts,\r
+                     &CertStackSize,\r
+                     &RootCert,\r
+                     &RootCertSize\r
+                     );\r
+    if (!VerifyStatus) {\r
+      goto Exit;\r
+    }\r
+\r
+    //\r
+    // Second, get the current platform key from variable. Check whether it's identical with signer's certificates\r
+    // in SignedData. If not, return error immediately.\r
     //\r
     Status = FindVariable (\r
                EFI_PLATFORM_KEY_NAME,\r
@@ -2014,14 +2210,16 @@ VerifyTimeBasedPayload (
                FALSE\r
                );\r
     if (EFI_ERROR (Status)) {\r
-      return Status;\r
+      VerifyStatus = FALSE;\r
+      goto Exit;\r
     }\r
-\r
     CertList = (EFI_SIGNATURE_LIST *) GetVariableDataPtr (PkVariable.CurrPtr);\r
     Cert     = (EFI_SIGNATURE_DATA *) ((UINT8 *) CertList + sizeof (EFI_SIGNATURE_LIST) + CertList->SignatureHeaderSize);\r
-    RootCert      = Cert->SignatureData;\r
-    RootCertSize  = CertList->SignatureSize - (sizeof (EFI_SIGNATURE_DATA) - 1);\r
-\r
+    if ((RootCertSize != (CertList->SignatureSize - (sizeof (EFI_SIGNATURE_DATA) - 1))) ||\r
+        (CompareMem (Cert->SignatureData, RootCert, RootCertSize) != 0)) {\r
+      VerifyStatus = FALSE;\r
+      goto Exit;\r
+    }\r
 \r
     //\r
     // Verify Pkcs7 SignedData via Pkcs7Verify library.\r
@@ -2139,13 +2337,13 @@ VerifyTimeBasedPayload (
     //\r
     // Delete signer's certificates when delete the common authenticated variable.\r
     //\r
-    if ((PayloadSize == 0) && (Variable->CurrPtr != NULL)) {\r
+    if ((PayloadSize == 0) && (Variable->CurrPtr != NULL) && ((Attributes & EFI_VARIABLE_APPEND_WRITE) == 0)) {\r
       Status = DeleteCertsFromDb (VariableName, VendorGuid);\r
       if (EFI_ERROR (Status)) {\r
         VerifyStatus = FALSE;\r
         goto Exit;\r
       }\r
-    } else if (Variable->CurrPtr == NULL) {\r
+    } else if (Variable->CurrPtr == NULL && PayloadSize != 0) {\r
       //\r
       // Insert signer's certificates when adding a new common authenticated variable.\r
       //\r
@@ -2177,7 +2375,7 @@ VerifyTimeBasedPayload (
 \r
 Exit:\r
 \r
-  if (AuthVarType == AuthVarTypePriv) {\r
+  if (AuthVarType == AuthVarTypePk || AuthVarType == AuthVarTypePriv) {\r
     Pkcs7FreeSigners (RootCert);\r
     Pkcs7FreeSigners (SignerCerts);\r
   }\r