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.zh...@intel.com>
---
 SecurityPkg/Library/AuthVariableLib/AuthService.c  | 151 ++++++++++++++++++---
 .../Library/AuthVariableLib/AuthServiceInternal.h  |  16 +++
 .../Library/AuthVariableLib/AuthVariableLib.c      |   9 ++
 3 files changed, 154 insertions(+), 22 deletions(-)

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c 
b/SecurityPkg/Library/AuthVariableLib/AuthService.c
index b3e8933..aa23c88 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthService.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c
@@ -1965,6 +1965,101 @@ InsertCertsToDb (
 }
 
 /**
+  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;
+
+  CertCleaned = FALSE;
+  Status      = EFI_SUCCESS;
+
+  //
+  // Get corresponding certificates by VendorGuid and VariableName.
+  //
+  while (CertCleaned) {
+    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);
+      
+      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));
+      //
+      // Find corresponding time auth variable
+      //
+      Status = AuthServiceInternalFindVariable (
+                 VariableName,
+                 &Ptr->VendorGuid,
+                 (VOID **) &AuthVarData,
+                 &AuthVarDataSize
+                 );
+  
+      if (EFI_ERROR(Status)) {
+        Status      = DeleteCertsFromDb(VariableName, &Ptr->VendorGuid);
+        CertCleaned = TRUE;
+        DEBUG((EFI_D_INFO, "Recovery!! Cert for Auth Variable %s Guid %g is 
removed for consistency\n", VariableName, Ptr->VendorGuid));
+        FreePool(VariableName);
+        break;
+      }
+
+      FreePool(VariableName);
+      Offset = Offset + NodeSize;
+    }
+  }
+
+  return Status;
+}
+
+/**
   Process variable with EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS set
 
   Caution: This function may receive untrusted input.
@@ -2285,16 +2380,7 @@ VerifyTimeBasedPayload (
       goto Exit;
     }
 
-    //
-    // 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)) {
+    if ((OrgTimeStamp == NULL) && (PayloadSize != 0)) {
       //
       // Insert signer's certificates when adding a new common authenticated 
variable.
       //
@@ -2412,21 +2498,42 @@ VerifyTimeBasedPayloadAndUpdate (
     return Status;
   }
 
-  if ((PayloadSize == 0) && (VarDel != NULL)) {
-    *VarDel = TRUE;
-  }
-
   CertData = (EFI_VARIABLE_AUTHENTICATION_2 *) Data;
 
   //
   // Final step: Update/Append Variable if it pass Pkcs7Verify
   //
-  return AuthServiceInternalUpdateVariableWithTimeStamp (
-           VariableName,
-           VendorGuid,
-           PayloadPtr,
-           PayloadSize,
-           Attributes,
-           &CertData->TimeStamp
-           );
+  Status = AuthServiceInternalUpdateVariableWithTimeStamp (
+             VariableName,
+             VendorGuid,
+             PayloadPtr,
+             PayloadSize,
+             Attributes,
+             &CertData->TimeStamp
+             );
+
+  //
+  // Delete signer's certificates when delete the common authenticated 
variable.
+  //
+  if (EFI_ERROR(Status)
+   && (AuthVarType == AuthVarTypePriv)
+   && (PayloadSize == 0) 
+   && !EFI_ERROR(FindStatus)
+   && ((Attributes & EFI_VARIABLE_APPEND_WRITE) == 0)) {
+    Status = DeleteCertsFromDb (VariableName, VendorGuid);
+  }
+
+  
+  if ((VarDel != NULL)) {
+    if (!EFI_ERROR(Status) 
+     && (PayloadSize == 0) 
+     && !EFI_ERROR(FindStatus)
+     && ((Attributes & EFI_VARIABLE_APPEND_WRITE) == 0) ) {
+      *VarDel = TRUE;
+    } else {
+      *VarDel = FALSE;
+    }
+  }
+
+  return Status;
 }
diff --git a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h 
b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
index 08fff3f..add05c2 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
+++ b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
@@ -187,6 +187,22 @@ DeleteCertsFromDb (
   );
 
 /**
+  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.
 
   @param[in]        Data          Pointer to original EFI_SIGNATURE_LIST.
diff --git a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c 
b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
index 0bb0918..02df309 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
+++ b/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;
+    }
   }
 
   //
-- 
1.9.5.msysgit.1


------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to