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

SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c

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

SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c

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