SecurityPkg: Make time based AuthVariable update atomic

System may break during time based AuthVariable update, causing certdb inconsistent. 2 ways are used to ensure update atomic.
 1. Delete cert in certdb after variable is deleted
 2. Clean up certdb on variable initialization

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

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

SecurityPkg/Library/AuthVariableLib/AuthService.c

@@ -1205,10 +1205,6 @@ ProcessVariable (
     //
     // Allow the delete operation of common authenticated variable at user physical presence.
     //
-    if ((Attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) != 0) {
-      Status = DeleteCertsFromDb (VariableName, VendorGuid);
-    }
-    if (!EFI_ERROR (Status)) {
     Status = AuthServiceInternalUpdateVariable (
               VariableName,
               VendorGuid,
@@ -1216,7 +1212,10 @@ ProcessVariable (
               0,
               0
               );
+    if (!EFI_ERROR (Status) && ((Attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) != 0)) {
+      Status = DeleteCertsFromDb (VariableName, VendorGuid);
     }
+
     return Status;
   }
 
@@ -1964,6 +1963,109 @@ InsertCertsToDb (
   return Status;
 }
 
+/**
+  Clean up signer's certificates for common authenticated variable
+  by corresponding VariableName and VendorGuid from "certdb".
+  Sytem may break down during Timebased Variable update & certdb update,
+  make them inconsistent,  this function is called in AuthVariable Init to ensure 
+  consistency
+  
+  @retval  EFI_NOT_FOUND         Fail to find matching certs.
+  @retval  EFI_SUCCESS           Find matching certs and output parameters.
+
+**/
+EFI_STATUS
+CleanCertsFromDb (
+  VOID
+  ){
+  UINT32                  Offset;
+  AUTH_CERT_DB_DATA       *Ptr;
+  UINT32                  NameSize;
+  UINT32                  NodeSize;
+  CHAR16                  *VariableName;
+  EFI_STATUS              Status;
+  BOOLEAN                 CertCleaned;
+  UINT8                   *Data;
+  UINTN                   DataSize;
+  UINT8                   *AuthVarData;
+  UINTN                   AuthVarDataSize;
+  EFI_GUID                AuthVarGuid;
+
+  Status = EFI_SUCCESS;
+
+  //
+  // Get corresponding certificates by VendorGuid and VariableName.
+  //
+  do {
+    CertCleaned = FALSE;
+
+    //
+    // Get latest variable "certdb"
+    //
+    Status = AuthServiceInternalFindVariable (
+               EFI_CERT_DB_NAME,
+               &gEfiCertDbGuid,
+               (VOID **) &Data,
+               &DataSize
+               );
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+
+    if ((DataSize == 0) || (Data == NULL)) {
+      ASSERT (FALSE);
+      return EFI_NOT_FOUND;
+    }
+
+    Offset = sizeof (UINT32);
+
+    while (Offset < (UINT32) DataSize) {
+      Ptr = (AUTH_CERT_DB_DATA *) (Data + Offset);
+      //
+      // Check whether VendorGuid matches.
+      //
+      NodeSize = ReadUnaligned32 (&Ptr->CertNodeSize);
+      NameSize = ReadUnaligned32 (&Ptr->NameSize);
+
+      //
+      // Get VarName tailed with '\0'
+      //
+      VariableName = AllocateZeroPool((NameSize + 1) * sizeof(CHAR16));
+      if (VariableName == NULL) {
+        return EFI_OUT_OF_RESOURCES;
+      }
+      CopyMem (VariableName, (UINT8 *) Ptr + sizeof (AUTH_CERT_DB_DATA), NameSize * sizeof(CHAR16));
+      //
+      // Keep VarGuid  aligned
+      //
+      CopyMem (&AuthVarGuid, &Ptr->VendorGuid, sizeof(EFI_GUID));
+
+      //
+      // Find corresponding time auth variable
+      //
+      Status = AuthServiceInternalFindVariable (
+                 VariableName,
+                 &AuthVarGuid,
+                 (VOID **) &AuthVarData,
+                 &AuthVarDataSize
+                 );
+
+      if (EFI_ERROR(Status)) {
+        Status      = DeleteCertsFromDb(VariableName, &AuthVarGuid);
+        CertCleaned = TRUE;
+        DEBUG((EFI_D_INFO, "Recovery!! Cert for Auth Variable %s Guid %g is removed for consistency\n", VariableName, &AuthVarGuid));
+        FreePool(VariableName);
+        break;
+      }
+
+      FreePool(VariableName);
+      Offset = Offset + NodeSize;
+    }
+  } while (CertCleaned);
+
+  return Status;
+}
+
 /**
   Process variable with EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS set
 
@@ -2285,16 +2387,7 @@ VerifyTimeBasedPayload (
       goto Exit;
     }
 
-    //
+    if ((OrgTimeStamp == NULL) && (PayloadSize != 0)) {
-    // Delete signer's certificates when delete the common authenticated variable.
-    //
-    if ((PayloadSize == 0) && (OrgTimeStamp != NULL) && ((Attributes & EFI_VARIABLE_APPEND_WRITE) == 0)) {
-      Status = DeleteCertsFromDb (VariableName, VendorGuid);
-      if (EFI_ERROR (Status)) {
-        VerifyStatus = FALSE;
-        goto Exit;
-      }
-    } else if ((OrgTimeStamp == NULL) && (PayloadSize != 0)) {
       //
       // Insert signer's certificates when adding a new common authenticated variable.
       //
@@ -2389,6 +2482,7 @@ VerifyTimeBasedPayloadAndUpdate (
   UINTN                            PayloadSize;
   EFI_VARIABLE_AUTHENTICATION_2    *CertData;
   AUTH_VARIABLE_INFO               OrgVariableInfo;
+  BOOLEAN                          IsDel;
 
   ZeroMem (&OrgVariableInfo, sizeof (OrgVariableInfo));
   FindStatus = mAuthVarLibContextIn->FindVariable (
@@ -2412,8 +2506,12 @@ VerifyTimeBasedPayloadAndUpdate (
     return Status;
   }
 
-  if ((PayloadSize == 0) && (VarDel != NULL)) {
+  if (!EFI_ERROR(FindStatus)
-    *VarDel = TRUE;
+   && (PayloadSize == 0)
+   && ((Attributes & EFI_VARIABLE_APPEND_WRITE) == 0)) {
+    IsDel = TRUE;
+  } else {
+    IsDel = FALSE;
   }
 
   CertData = (EFI_VARIABLE_AUTHENTICATION_2 *) Data;
@@ -2421,7 +2519,7 @@ VerifyTimeBasedPayloadAndUpdate (
   //
   // Final step: Update/Append Variable if it pass Pkcs7Verify
   //
-  return AuthServiceInternalUpdateVariableWithTimeStamp (
+  Status = AuthServiceInternalUpdateVariableWithTimeStamp (
              VariableName,
              VendorGuid,
              PayloadPtr,
@@ -2429,4 +2527,21 @@ VerifyTimeBasedPayloadAndUpdate (
              Attributes,
              &CertData->TimeStamp
              );
+
+  //
+  // Delete signer's certificates when delete the common authenticated variable.
+  //
+  if (IsDel && AuthVarType == AuthVarTypePriv && !EFI_ERROR(Status) ) {
+    Status = DeleteCertsFromDb (VariableName, VendorGuid);
+  }
+
+  if (VarDel != NULL) {
+    if (IsDel && !EFI_ERROR(Status)) {
+      *VarDel = TRUE;
+    } else {
+      *VarDel = FALSE;
+    }
+  }
+
+  return Status;
 }

SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h

@@ -186,6 +186,22 @@ DeleteCertsFromDb (
   IN     EFI_GUID         *VendorGuid
   );
 
+/**
+  Clean up signer's certificates for common authenticated variable
+  by corresponding VariableName and VendorGuid from "certdb".
+  Sytem may break down during Timebased Variable update & certdb update,
+  make them inconsistent,  this function is called in AuthVariable Init to ensure 
+  consistency
+  
+  @retval  EFI_NOT_FOUND         Fail to find matching certs.
+  @retval  EFI_SUCCESS           Find matching certs and output parameters.
+
+**/
+EFI_STATUS
+CleanCertsFromDb (
+  VOID
+  );
+
 /**
   Filter out the duplicated EFI_SIGNATURE_DATA from the new data by comparing to the original data.
 

SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c

@@ -352,6 +352,15 @@ AuthVariableLibInitialize (
     if (EFI_ERROR (Status)) {
       return Status;
     }
+  } else {
+    //
+    // Clean up Certs to make certDB & Time based auth variable consistent
+    //
+    Status = CleanCertsFromDb();
+    if (EFI_ERROR (Status)) {
+      DEBUG ((EFI_D_INFO, "Clean up CertDB fail! Status %x\n", Status));
+      return Status;
+    }
   }
 
   //