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&data=02%7C01%7CBret.Barkelew%40microsoft.com%7C2d4a699617424da6381f08d807a3d094%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637267747454210698&sdata=10egANvpHPv6bNbdaNyL4%2F3tOk9eG03HUKCADhQix68%3D&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&data=02%7C01%7CBret.Barkelew%40microsoft.com%7C2d4a699617424da6381f08d807a3d094%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637267747454210698&sdata=%2BAYYshrhsUe22N%2Bq29KTBwBSfPZ%2BKMI%2BHfXnlAC1UDA%3D&reserved=0
Mute This Topic: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.io%2Fmt%2F74646426%2F1822150&data=02%7C01%7CBret.Barkelew%40microsoft.com%7C2d4a699617424da6381f08d807a3d094%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637267747454210698&sdata=mKJ2mIwadixEeJXSPlitdokFxojYhLgituGitz5Y%2FKQ%3D&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&data=02%7C01%7CBret.Barkelew%40microsoft.com%7C2d4a699617424da6381f08d807a3d094%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637267747454210698&sdata=tAo%2FXEtUjmykPq%2BZgx5USuiQdkQyX7pMaTQ%2FMaUCfuE%3D&reserved=0 [brbarkel@microsoft.com]
-=-=-=-=-=-=
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#61383): https://edk2.groups.io/g/devel/message/61383
Mute This Topic: https://groups.io/mt/74933177/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-