From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f174.google.com (mail-pf1-f174.google.com [209.85.210.174]) by mx.groups.io with SMTP id smtpd.web11.6982.1600841385951996028 for ; Tue, 22 Sep 2020 23:09:46 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@corthon-com.20150623.gappssmtp.com header.s=20150623 header.b=R7oZtR+N; spf=none, err=permanent DNS error (domain: corthon.com, ip: 209.85.210.174, mailfrom: bret@corthon.com) Received: by mail-pf1-f174.google.com with SMTP id d6so14387325pfn.9 for ; Tue, 22 Sep 2020 23:09:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=corthon-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=8X6ynpCtf+tydqctpIRVE+oMoOtTOGcXTHw+ssV63j0=; b=R7oZtR+NA79FCevY9FgzK7/OyHO23yIFT/Ss9vrWQbxBOR6MpChrJeFjS56lvKlXGY RKK/lEf978guPlmhDAWRFuBdxpo0jBRP7ABbCDiFrM82/adAvGDKMoy5KDlGWYLrgcDY xi1UXI3Rsf92jLbKJZy59rCZVY4kGMr9Ly42DdtxecmzMpaTxG+T7eF7s7rn34kvcTCs qVb+tc1H9eFYZRua/ktBwx05F6HVUmZpXb+bAL8U3oGUcgrjuV4HKkCTsmGY9G9PrzQw 3YF8EGdKhuN6Ax0yuJfC8kzBZCMIZgG2+Bwpw/3hZ8s/YVc7WFlCmsWcKPtPafsnTyLT AAOQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=8X6ynpCtf+tydqctpIRVE+oMoOtTOGcXTHw+ssV63j0=; b=dQZ9QDHu7g9rfVxcRcU7c+8IVSauVf103uxicF2fXFBoA1tglou2WjgCJxZljQU1r6 m1c8jwhh2mtyh1Q1opYLOwqta28EM1poj4F2B/IdY0XbOD9Io6lO/5UD3IgRZvhelAN8 wd6Fw8fjXnBfbaDFLjydxJtnz3u34ZKe8DRMKsM1SyWMfNSrqR0JezKlSvXEkgBxQg42 Qh6B2SvjMVH3q/OBU5mhBa1vyu55K0ofGK7JOVLHeflCZntv/PNQiAHVo1oiHTdAPX8J MghunkhN4hVXRVYHLicR+aDoY8EB8kr3T59U7HWKgqbKiHP6MUWZ4oZ8JHpVzxEGAeOK 9ApQ== X-Gm-Message-State: AOAM532SV9TUCyQnuhthGdVPkS16uyCQ3WzBimCGGCDFaRdajrRNl+Dg kakGoPGTwcGUY9niGewBLlAdu4FjXxNohLcY X-Google-Smtp-Source: ABdhPJxXXw7pifo/W6qGp8HSpAV7Q9TP6D9eUXlzzkSlvaChlHHXlojZlvOf8JldXfNyJSdVV7AkAA== X-Received: by 2002:aa7:9491:0:b029:142:2501:396b with SMTP id z17-20020aa794910000b02901422501396bmr7315406pfk.48.1600841385052; Tue, 22 Sep 2020 23:09:45 -0700 (PDT) Return-Path: Received: from localhost.localdomain (174-21-140-128.tukw.qwest.net. [174.21.140.128]) by smtp.gmail.com with ESMTPSA id x4sm16960498pff.57.2020.09.22.23.09.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 22 Sep 2020 23:09:44 -0700 (PDT) From: "Bret Barkelew" X-Google-Original-From: Bret Barkelew To: devel@edk2.groups.io Cc: Jiewen Yao , Jian J Wang , Chao Zhang , Bret Barkelew , Dandan Bi Subject: [PATCH v8 11/14] SecurityPkg: Allow VariablePolicy state to delete authenticated variables Date: Tue, 22 Sep 2020 23:07:45 -0700 Message-Id: <20200923060748.3795-12-bret.barkelew@microsoft.com> X-Mailer: git-send-email 2.28.0.windows.1 In-Reply-To: <20200923060748.3795-1-bret.barkelew@microsoft.com> References: <20200923060748.3795-1-bret.barkelew@microsoft.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Bret Barkelew https://bugzilla.tianocore.org/show_bug.cgi?id=3D2522 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 Cc: Jian J Wang Cc: Chao Zhang Cc: Bret Barkelew Signed-off-by: Bret Barkelew Reviewed-by: Dandan Bi Acked-by: Jian J Wang --- SecurityPkg/Library/AuthVariableLib/AuthService.c | 30 +++++++++++++= +++---- SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf | 2 ++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c b/SecurityPk= g/Library/AuthVariableLib/AuthService.c index 2f60331f2c04..4fb609504db7 100644 --- a/SecurityPkg/Library/AuthVariableLib/AuthService.c +++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c @@ -19,12 +19,16 @@ to verify the signature.=0D =0D Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.
=0D +Copyright (c) Microsoft Corporation.=0D SPDX-License-Identifier: BSD-2-Clause-Patent=0D =0D **/=0D =0D #include "AuthServiceInternal.h"=0D =0D +#include =0D +#include =0D +=0D //=0D // Public Exponent of RSA Key.=0D //=0D @@ -217,9 +221,12 @@ NeedPhysicallyPresent( IN EFI_GUID *VendorGuid=0D )=0D {=0D - if ((CompareGuid (VendorGuid, &gEfiSecureBootEnableDisableGuid) && (StrC= mp (VariableName, EFI_SECURE_BOOT_ENABLE_NAME) =3D=3D 0))=0D - || (CompareGuid (VendorGuid, &gEfiCustomModeEnableGuid) && (StrCmp (Va= riableName, EFI_CUSTOM_MODE_NAME) =3D=3D 0))) {=0D - return TRUE;=0D + // If the VariablePolicy engine is disabled, allow deletion of any authe= nticated variables.=0D + if (IsVariablePolicyEnabled()) {=0D + if ((CompareGuid (VendorGuid, &gEfiSecureBootEnableDisableGuid) && (St= rCmp (VariableName, EFI_SECURE_BOOT_ENABLE_NAME) =3D=3D 0))=0D + || (CompareGuid (VendorGuid, &gEfiCustomModeEnableGuid) && (StrCmp (= VariableName, EFI_CUSTOM_MODE_NAME) =3D=3D 0))) {=0D + return TRUE;=0D + }=0D }=0D =0D return FALSE;=0D @@ -842,7 +849,8 @@ ProcessVariable ( &OrgVariableInfo=0D );=0D =0D - if ((!EFI_ERROR (Status)) && IsDeleteAuthVariable (OrgVariableInfo.Attri= butes, Data, DataSize, Attributes) && UserPhysicalPresent()) {=0D + // If the VariablePolicy engine is disabled, allow deletion of any authe= nticated variables.=0D + if ((!EFI_ERROR (Status)) && IsDeleteAuthVariable (OrgVariableInfo.Attri= butes, Data, DataSize, Attributes) && (UserPhysicalPresent() || !IsVariable= PolicyEnabled())) {=0D //=0D // Allow the delete operation of common authenticated variable(AT or A= W) at user physical presence.=0D //=0D @@ -1920,6 +1928,12 @@ VerifyTimeBasedPayload ( PayloadPtr =3D SigData + SigDataSize;=0D PayloadSize =3D DataSize - OFFSET_OF_AUTHINFO2_CERT_DATA - (UINTN) SigDa= taSize;=0D =0D + // If the VariablePolicy engine is disabled, allow deletion of any authe= nticated variables.=0D + if (PayloadSize =3D=3D 0 && (Attributes & EFI_VARIABLE_APPEND_WRITE) =3D= =3D 0 && !IsVariablePolicyEnabled()) {=0D + VerifyStatus =3D TRUE;=0D + goto Exit;=0D + }=0D +=0D //=0D // Construct a serialization buffer of the values of the VariableName, V= endorGuid and Attributes=0D // parameters of the SetVariable() call and the TimeStamp component of t= he=0D @@ -2173,8 +2187,12 @@ VerifyTimeBasedPayload ( Exit:=0D =0D if (AuthVarType =3D=3D AuthVarTypePk || AuthVarType =3D=3D AuthVarTypePr= iv) {=0D - Pkcs7FreeSigners (TopLevelCert);=0D - Pkcs7FreeSigners (SignerCerts);=0D + if (TopLevelCert !=3D NULL) {=0D + Pkcs7FreeSigners (TopLevelCert);=0D + }=0D + if (SignerCerts !=3D NULL) {=0D + Pkcs7FreeSigners (SignerCerts);=0D + }=0D }=0D =0D if (!VerifyStatus) {=0D diff --git a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf b/Secu= rityPkg/Library/AuthVariableLib/AuthVariableLib.inf index 8d4ce14df494..8eadeebcebd7 100644 --- a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf +++ b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf @@ -3,6 +3,7 @@ #=0D # Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
= =0D # Copyright (c) 2018, ARM Limited. All rights reserved.
=0D +# Copyright (c) Microsoft Corporation.=0D #=0D # SPDX-License-Identifier: BSD-2-Clause-Patent=0D #=0D @@ -41,6 +42,7 @@ [LibraryClasses] MemoryAllocationLib=0D BaseCryptLib=0D PlatformSecureLib=0D + VariablePolicyLib=0D =0D [Guids]=0D ## CONSUMES ## Variable:L"SetupMode"=0D --=20 2.28.0.windows.1