1. Remove extra attribute for SetupMode, SignatureSupport and SecureBoot variable.

2. Allow the PK owner to update db/dbx variable directly.

Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
Reviewed-by: Dong Guo <guo.dong@intel.com>
Reviewed-by: Ye Ting <ting.ye@intel.com>
git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@13536 6f19259b-4bc3-4df7-8a09-765794883524

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
index 566d398567f7070027d9b8be5e6af81bab955c0a..b25c04368412cd78924ea1d32496655a7c1d8196 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
@@ -254,68 +254,46 @@ AutenticatedVariableServiceInitialize (
   }\r
   \r
   //\r
-  // Check "SetupMode" variable's existence.\r
-  // If it doesn't exist, check PK database's existence to determine the value.\r
-  // Then create a new one with EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS set.\r
+  // Create "SetupMode" varable with BS+RT attribute set.\r
   //\r
-  Status = FindVariable (\r
+  FindVariable (EFI_SETUP_MODE_NAME, &gEfiGlobalVariableGuid, &Variable, &mVariableModuleGlobal->VariableGlobal, FALSE);\r
+  if (PkVariable.CurrPtr == NULL) {\r
+    mPlatformMode = SETUP_MODE;\r
+  } else {\r
+    mPlatformMode = USER_MODE;\r
+  }\r
+  Status = UpdateVariable (\r
              EFI_SETUP_MODE_NAME,\r
              &gEfiGlobalVariableGuid,\r
+             &mPlatformMode,\r
+             sizeof(UINT8),\r
+             EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,\r
+             0,\r
+             0,\r
              &Variable,\r
-             &mVariableModuleGlobal->VariableGlobal,\r
-             FALSE\r
+             NULL\r
              );\r
-\r
-  if (Variable.CurrPtr == NULL) {\r
-    if (PkVariable.CurrPtr == NULL) {\r
-      mPlatformMode = SETUP_MODE;\r
-    } else {\r
-      mPlatformMode = USER_MODE;\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_SETUP_MODE_NAME,\r
-                &gEfiGlobalVariableGuid,\r
-                &mPlatformMode,\r
-                sizeof(UINT8),\r
-                VarAttr,\r
-                0,\r
-                0,\r
-                &Variable,\r
-                NULL\r
-                );\r
-    if (EFI_ERROR (Status)) {\r
-      return Status;\r
-    }\r
-  } else {\r
-    mPlatformMode = *(GetVariableDataPtr (Variable.CurrPtr));\r
+  if (EFI_ERROR (Status)) {\r
+    return Status;\r
   }\r
+  \r
   //\r
-  // Check "SignatureSupport" variable's existence.\r
-  // If it doesn't exist, then create a new one with EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS set.\r
+  // Create "SignatureSupport" varable with BS+RT attribute set.\r
   //\r
-  Status = FindVariable (\r
-             EFI_SIGNATURE_SUPPORT_NAME,\r
-             &gEfiGlobalVariableGuid,\r
-             &Variable,\r
-             &mVariableModuleGlobal->VariableGlobal,\r
-             FALSE\r
-             );\r
-\r
-  if (Variable.CurrPtr == NULL) {\r
-    VarAttr = EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS;\r
-    Status  = UpdateVariable (\r
-                EFI_SIGNATURE_SUPPORT_NAME,\r
-                &gEfiGlobalVariableGuid,\r
-                mSignatureSupport,\r
-                sizeof(mSignatureSupport),\r
-                VarAttr,\r
-                0,\r
-                0,\r
-                &Variable,\r
-                NULL\r
-                );\r
+  FindVariable (EFI_SIGNATURE_SUPPORT_NAME, &gEfiGlobalVariableGuid, &Variable, &mVariableModuleGlobal->VariableGlobal, FALSE);\r
+  Status  = UpdateVariable (\r
+              EFI_SIGNATURE_SUPPORT_NAME,\r
+              &gEfiGlobalVariableGuid,\r
+              mSignatureSupport,\r
+              sizeof(mSignatureSupport),\r
+              EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,\r
+              0,\r
+              0,\r
+              &Variable,\r
+              NULL\r
+              );\r
+  if (EFI_ERROR (Status)) {\r
+    return Status;\r
   }\r
 \r
   //\r
@@ -348,6 +326,9 @@ AutenticatedVariableServiceInitialize (
     }\r
   }\r
 \r
+  //\r
+  // Create "SecureBoot" varable with BS+RT attribute set.\r
+  //\r
   if (SecureBootEnable == SECURE_BOOT_ENABLE && mPlatformMode == USER_MODE) {\r
     SecureBootMode = SECURE_BOOT_MODE_ENABLE;\r
   } else {\r
@@ -359,7 +340,7 @@ AutenticatedVariableServiceInitialize (
              &gEfiGlobalVariableGuid,\r
              &SecureBootMode,\r
              sizeof (UINT8),\r
-             EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS,\r
+             EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS,\r
              0,\r
              0,\r
              &Variable,\r
@@ -660,22 +641,12 @@ UpdatePlatformMode (
     return Status;\r
   }\r
 \r
-  mPlatformMode  = Mode;\r
-  VarAttr        = EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS;\r
-  Status         = UpdateVariable (\r
-                     EFI_SETUP_MODE_NAME,\r
-                     &gEfiGlobalVariableGuid,\r
-                     &mPlatformMode,\r
-                     sizeof(UINT8),\r
-                     VarAttr,\r
-                     0,\r
-                     0,\r
-                     &Variable,\r
-                     NULL\r
-                     );\r
-  if (EFI_ERROR (Status)) {\r
-    return Status;\r
-  }\r
+  //\r
+  // Update the value of SetupMode variable by a simple mem copy, this could avoid possible\r
+  // variable storage reclaim at runtime.\r
+  //\r
+  mPlatformMode = (UINT8) Mode;\r
+  CopyMem (GetVariableDataPtr (Variable.CurrPtr), &mPlatformMode, sizeof(UINT8));\r
 \r
   if (AtRuntime ()) {\r
     //\r
@@ -912,7 +883,7 @@ ProcessVarWithPk (
   if ((Attributes & EFI_VARIABLE_NON_VOLATILE) == 0 || \r
       (Attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) == 0) {\r
     //\r
-    // PK and KEK should set EFI_VARIABLE_NON_VOLATILE attribute and should be a time-based\r
+    // PK, KEK and db/dbx should set EFI_VARIABLE_NON_VOLATILE attribute and should be a time-based\r
     // authenticated variable.\r
     //\r
     return EFI_INVALID_PARAMETER;\r

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
index bae5f7bc45bdfa1361e659b5e7377d05c9675a02..93d0e25eac9c5e80ccdb7376fe02a0f01117bea2 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
@@ -1955,6 +1955,33 @@ IsHwErrRecVariable (
   return TRUE;\r
 }\r
 \r
+/**\r
+  This code checks if variable should be treated as read-only variable.\r
+\r
+  @param[in]      VariableName            Name of the Variable.\r
+  @param[in]      VendorGuid              GUID of the Variable.\r
+\r
+  @retval TRUE      This variable is read-only variable.\r
+  @retval FALSE     This variable is NOT read-only variable.\r
+  \r
+**/\r
+BOOLEAN\r
+IsReadOnlyVariable (\r
+  IN     CHAR16         *VariableName,\r
+  IN     EFI_GUID       *VendorGuid\r
+  )\r
+{\r
+  if (CompareGuid (VendorGuid, &gEfiGlobalVariableGuid)) {\r
+    if ((StrCmp (VariableName, EFI_SETUP_MODE_NAME) == 0) ||\r
+        (StrCmp (VariableName, EFI_SIGNATURE_SUPPORT_NAME) == 0) ||\r
+        (StrCmp (VariableName, EFI_SECURE_BOOT_MODE_NAME) == 0)) {\r
+      return TRUE;\r
+    }\r
+  }\r
+  \r
+  return FALSE;\r
+}\r
+\r
 /**\r
 \r
   This code finds variable in storage blocks (Volatile or Non-Volatile).\r
@@ -2232,6 +2259,10 @@ VariableServiceSetVariable (
     return EFI_INVALID_PARAMETER;\r
   }\r
 \r
+  if (IsReadOnlyVariable (VariableName, VendorGuid)) {\r
+    return EFI_WRITE_PROTECTED;\r
+  }\r
+\r
   if (DataSize != 0 && Data == NULL) {\r
     return EFI_INVALID_PARAMETER;\r
   }\r
@@ -2349,7 +2380,10 @@ VariableServiceSetVariable (
     Status = ProcessVarWithPk (VariableName, VendorGuid, Data, DataSize, &Variable, Attributes, FALSE);\r
   } else if (CompareGuid (VendorGuid, &gEfiImageSecurityDatabaseGuid) && \r
           ((StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE) == 0) || (StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE1) == 0))) {\r
-    Status = ProcessVarWithKek (VariableName, VendorGuid, Data, DataSize, &Variable, Attributes);\r
+    Status = ProcessVarWithPk (VariableName, VendorGuid, Data, DataSize, &Variable, Attributes, FALSE);\r
+    if (EFI_ERROR (Status)) {\r
+      Status = ProcessVarWithKek (VariableName, VendorGuid, Data, DataSize, &Variable, Attributes);\r
+    }\r
   } else {\r
     Status = ProcessVariable (VariableName, VendorGuid, Data, DataSize, &Variable, Attributes);\r
   }\r