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

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

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

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
index 0bb09189ee4604e4d3cc9515e3ce82586d4628d1..02df309802ef1c9a56afc94d9aa7d356ec38f3d0 100644 (file)
--- a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
@@ -352,6 +352,15 @@ AuthVariableLibInitialize (
     if (EFI_ERROR (Status)) {\r
       return Status;\r
     }\r
+  } else {\r
+    //\r
+    // Clean up Certs to make certDB & Time based auth variable consistent\r
+    //\r
+    Status = CleanCertsFromDb();\r
+    if (EFI_ERROR (Status)) {\r
+      DEBUG ((EFI_D_INFO, "Clean up CertDB fail! Status %x\n", Status));\r
+      return Status;\r
+    }\r
   }\r
 \r
   //\r