SecurityPkg: Allow VariablePolicy state to delete authenticated variables

https://bugzilla.tianocore.org/show_bug.cgi?id=2522

Causes AuthService to check
IsVariablePolicyEnabled() before enforcing
write protections to allow variable deletion
when policy engine is disabled.

Only allows deletion, not modification.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Bret Barkelew <brbarkel@microsoft.com>
Signed-off-by: Bret Barkelew <brbarkel@microsoft.com>
Reviewed-by: Dandan Bi <dandan.bi@intel.com>
Acked-by: Jian J Wang <jian.j.wang@intel.com>

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c b/SecurityPkg/Library/AuthVariableLib/AuthService.c
index 2f60331f2c048403b7d85311d23d17388124ea84..4fb609504db76b4d910f964b4ecd8eee9e0c5cee 100644 (file)
--- a/SecurityPkg/Library/AuthVariableLib/AuthService.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c
@@ -19,12 +19,16 @@
   to verify the signature.\r
 \r
 Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR>\r
+Copyright (c) Microsoft Corporation.\r
 SPDX-License-Identifier: BSD-2-Clause-Patent\r
 \r
 **/\r
 \r
 #include "AuthServiceInternal.h"\r
 \r
+#include <Protocol/VariablePolicy.h>\r
+#include <Library/VariablePolicyLib.h>\r
+\r
 //\r
 // Public Exponent of RSA Key.\r
 //\r
@@ -217,9 +221,12 @@ NeedPhysicallyPresent(
   IN     EFI_GUID       *VendorGuid\r
   )\r
 {\r
-  if ((CompareGuid (VendorGuid, &gEfiSecureBootEnableDisableGuid) && (StrCmp (VariableName, EFI_SECURE_BOOT_ENABLE_NAME) == 0))\r
-    || (CompareGuid (VendorGuid, &gEfiCustomModeEnableGuid) && (StrCmp (VariableName, EFI_CUSTOM_MODE_NAME) == 0))) {\r
-    return TRUE;\r
+  // If the VariablePolicy engine is disabled, allow deletion of any authenticated variables.\r
+  if (IsVariablePolicyEnabled()) {\r
+    if ((CompareGuid (VendorGuid, &gEfiSecureBootEnableDisableGuid) && (StrCmp (VariableName, EFI_SECURE_BOOT_ENABLE_NAME) == 0))\r
+      || (CompareGuid (VendorGuid, &gEfiCustomModeEnableGuid) && (StrCmp (VariableName, EFI_CUSTOM_MODE_NAME) == 0))) {\r
+      return TRUE;\r
+    }\r
   }\r
 \r
   return FALSE;\r
@@ -842,7 +849,8 @@ ProcessVariable (
              &OrgVariableInfo\r
              );\r
 \r
-  if ((!EFI_ERROR (Status)) && IsDeleteAuthVariable (OrgVariableInfo.Attributes, Data, DataSize, Attributes) && UserPhysicalPresent()) {\r
+  // If the VariablePolicy engine is disabled, allow deletion of any authenticated variables.\r
+  if ((!EFI_ERROR (Status)) && IsDeleteAuthVariable (OrgVariableInfo.Attributes, Data, DataSize, Attributes) && (UserPhysicalPresent() || !IsVariablePolicyEnabled())) {\r
     //\r
     // Allow the delete operation of common authenticated variable(AT or AW) at user physical presence.\r
     //\r
@@ -1920,6 +1928,12 @@ VerifyTimeBasedPayload (
   PayloadPtr = SigData + SigDataSize;\r
   PayloadSize = DataSize - OFFSET_OF_AUTHINFO2_CERT_DATA - (UINTN) SigDataSize;\r
 \r
+  // If the VariablePolicy engine is disabled, allow deletion of any authenticated variables.\r
+  if (PayloadSize == 0 && (Attributes & EFI_VARIABLE_APPEND_WRITE) == 0 && !IsVariablePolicyEnabled()) {\r
+    VerifyStatus = TRUE;\r
+    goto Exit;\r
+  }\r
+\r
   //\r
   // Construct a serialization buffer of the values of the VariableName, VendorGuid and Attributes\r
   // parameters of the SetVariable() call and the TimeStamp component of the\r
@@ -2173,8 +2187,12 @@ VerifyTimeBasedPayload (
 Exit:\r
 \r
   if (AuthVarType == AuthVarTypePk || AuthVarType == AuthVarTypePriv) {\r
-    Pkcs7FreeSigners (TopLevelCert);\r
-    Pkcs7FreeSigners (SignerCerts);\r
+    if (TopLevelCert != NULL) {\r
+        Pkcs7FreeSigners (TopLevelCert);\r
+    }\r
+    if (SignerCerts != NULL) {\r
+        Pkcs7FreeSigners (SignerCerts);\r
+    }\r
   }\r
 \r
   if (!VerifyStatus) {\r

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
index 8d4ce14df49435cf4db6a3eba7e9abc74490a6ea..8eadeebcebd70c5aed551a9be10043de78ef7c0c 100644 (file)
--- a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
+++ b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
@@ -3,6 +3,7 @@
 #\r
 #  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>\r
 #  Copyright (c) 2018, ARM Limited. All rights reserved.<BR>\r
+#  Copyright (c) Microsoft Corporation.\r
 #\r
 #  SPDX-License-Identifier: BSD-2-Clause-Patent\r
 #\r
@@ -41,6 +42,7 @@
   MemoryAllocationLib\r
   BaseCryptLib\r
   PlatformSecureLib\r
+  VariablePolicyLib\r
 \r
 [Guids]\r
   ## CONSUMES            ## Variable:L"SetupMode"\r