Re: [edk2-devel] [PATCH v3 07/14] ArmVirtPkg: Add VariablePolicy engine to ArmVirtPkg platform

["Laszlo Ersek" <lersek@redhat.com>]

Hi,

On 05/22/20 00:43, Michael Kubacki wrote:
> From: Bret Barkelew <brbarkel@microsoft.com>
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=2522
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Bret Barkelew <brbarkel@microsoft.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
>  ArmVirtPkg/ArmVirt.dsc.inc | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index cf44fc73890b..ee965e72b075 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -2,6 +2,7 @@
>  #  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
>  #  Copyright (c) 2014, Linaro Limited. All rights reserved.
>  #  Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
> +#  Copyright (c) Microsoft Corporation.
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -173,6 +174,8 @@ [LibraryClasses.common]
>    AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
>  !endif
>    VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
> +  VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
> +  VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
>    UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
>  
>    ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf
> @@ -246,6 +249,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>  !if $(TARGET) != RELEASE
>    DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf
>  !endif
> +  VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf
>  
>  !if $(SECURE_BOOT_ENABLE) == TRUE
>    BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> @@ -301,6 +305,9 @@ [PcdsFixedAtBuild.common]
>    gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2f
>  !endif
>  
> +  # Optional: Omit if VariablePolicy should be always-on.
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnforcementDisable|TRUE
> +
>    #  DEBUG_INIT      0x00000001  // Initialization
>    #  DEBUG_WARN      0x00000002  // Warnings
>    #  DEBUG_LOAD      0x00000004  // Load events
> 

(1) Similarly to my OvmfPkg request, please remove the
PcdAllowVariablePolicyEnforcementDisable=TRUE setting. The default
should be secure, as described in:

https://github.com/tianocore/tianocore.github.io/wiki/VariablePolicy-Protocol---Enhanced-Method-for-Managing-Variables#pcdallowvariablepolicyenforcementdisable

otherwise, the patch looks OK to me.

Thanks!
Laszlo