Re: [EXTERNAL] [edk2-devel] [PATCH v5 11/14] SecurityPkg: Allow VariablePolicy state to delete authenticated variables

["Bret Barkelew" <bret.barkelew@microsoft.com>]

[-- Attachment #1: Type: text/plain, Size: 6232 bytes --]

Bump. This specific patch needs Reviews.


- Bret

________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Bret Barkelew via groups.io <bret=corthon.com@groups.io>
Sent: Tuesday, June 2, 2020 11:58 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Yao, Jiewen <jiewen.yao@intel.com>; Jian J Wang <jian.j.wang@intel.com>; Chao Zhang <chao.b.zhang@intel.com>
Subject: [EXTERNAL] [edk2-devel] [PATCH v5 11/14] SecurityPkg: Allow VariablePolicy state to delete authenticated variables

https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2522&amp;data=02%7C01%7CBret.Barkelew%40microsoft.com%7C2d4a699617424da6381f08d807a3d094%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637267747454210698&amp;sdata=10egANvpHPv6bNbdaNyL4%2F3tOk9eG03HUKCADhQix68%3D&amp;reserved=0

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>
---
 SecurityPkg/Library/AuthVariableLib/AuthService.c       | 22 ++++++++++++++++----
 SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf |  2 ++
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c b/SecurityPkg/Library/AuthVariableLib/AuthService.c
index 2f60331f2c04..aca9a5620c28 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthService.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c
@@ -19,12 +19,16 @@
   to verify the signature.



 Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR>

+Copyright (c) Microsoft Corporation.

 SPDX-License-Identifier: BSD-2-Clause-Patent



 **/



 #include "AuthServiceInternal.h"



+#include <Protocol/VariablePolicy.h>

+#include <Library/VariablePolicyLib.h>

+

 //

 // Public Exponent of RSA Key.

 //

@@ -217,9 +221,12 @@ NeedPhysicallyPresent(
   IN     EFI_GUID       *VendorGuid

   )

 {

-  if ((CompareGuid (VendorGuid, &gEfiSecureBootEnableDisableGuid) && (StrCmp (VariableName, EFI_SECURE_BOOT_ENABLE_NAME) == 0))

-    || (CompareGuid (VendorGuid, &gEfiCustomModeEnableGuid) && (StrCmp (VariableName, EFI_CUSTOM_MODE_NAME) == 0))) {

-    return TRUE;

+  // If the VariablePolicy engine is disabled, allow deletion of any authenticated variables.

+  if (IsVariablePolicyEnabled()) {

+    if ((CompareGuid (VendorGuid, &gEfiSecureBootEnableDisableGuid) && (StrCmp (VariableName, EFI_SECURE_BOOT_ENABLE_NAME) == 0))

+      || (CompareGuid (VendorGuid, &gEfiCustomModeEnableGuid) && (StrCmp (VariableName, EFI_CUSTOM_MODE_NAME) == 0))) {

+      return TRUE;

+    }

   }



   return FALSE;

@@ -842,7 +849,8 @@ ProcessVariable (
              &OrgVariableInfo

              );



-  if ((!EFI_ERROR (Status)) && IsDeleteAuthVariable (OrgVariableInfo.Attributes, Data, DataSize, Attributes) && UserPhysicalPresent()) {

+  // If the VariablePolicy engine is disabled, allow deletion of any authenticated variables.

+  if ((!EFI_ERROR (Status)) && IsDeleteAuthVariable (OrgVariableInfo.Attributes, Data, DataSize, Attributes) && (UserPhysicalPresent() || !IsVariablePolicyEnabled())) {

     //

     // Allow the delete operation of common authenticated variable(AT or AW) at user physical presence.

     //

@@ -1960,6 +1968,12 @@ VerifyTimeBasedPayload (


   CopyMem (Buffer, PayloadPtr, PayloadSize);



+  // If the VariablePolicy engine is disabled, allow deletion of any authenticated variables.

+  if (PayloadSize == 0 && (Attributes & EFI_VARIABLE_APPEND_WRITE) == 0 && !IsVariablePolicyEnabled()) {

+    VerifyStatus = TRUE;

+    goto Exit;

+  }

+

   if (AuthVarType == AuthVarTypePk) {

     //

     // Verify that the signature has been made with the current Platform Key (no chaining for PK).

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
index 8d4ce14df494..8eadeebcebd7 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
+++ b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
@@ -3,6 +3,7 @@
 #

 #  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>

 #  Copyright (c) 2018, ARM Limited. All rights reserved.<BR>

+#  Copyright (c) Microsoft Corporation.

 #

 #  SPDX-License-Identifier: BSD-2-Clause-Patent

 #

@@ -41,6 +42,7 @@ [LibraryClasses]
   MemoryAllocationLib

   BaseCryptLib

   PlatformSecureLib

+  VariablePolicyLib



 [Guids]

   ## CONSUMES            ## Variable:L"SetupMode"

--
2.26.2.windows.1.8.g01c50adf56.20200515075929


-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60637): https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F60637&amp;data=02%7C01%7CBret.Barkelew%40microsoft.com%7C2d4a699617424da6381f08d807a3d094%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637267747454210698&amp;sdata=%2BAYYshrhsUe22N%2Bq29KTBwBSfPZ%2BKMI%2BHfXnlAC1UDA%3D&amp;reserved=0
Mute This Topic: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.io%2Fmt%2F74646426%2F1822150&amp;data=02%7C01%7CBret.Barkelew%40microsoft.com%7C2d4a699617424da6381f08d807a3d094%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637267747454210698&amp;sdata=mKJ2mIwadixEeJXSPlitdokFxojYhLgituGitz5Y%2FKQ%3D&amp;reserved=0
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Funsub&amp;data=02%7C01%7CBret.Barkelew%40microsoft.com%7C2d4a699617424da6381f08d807a3d094%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637267747454210698&amp;sdata=tAo%2FXEtUjmykPq%2BZgx5USuiQdkQyX7pMaTQ%2FMaUCfuE%3D&amp;reserved=0  [brbarkel@microsoft.com]
-=-=-=-=-=-=


[-- Attachment #2: Type: text/html, Size: 10912 bytes --]