On 05/12/20 08:46, Michael Kubacki wrote: > From: Bret Barkelew <brbar...@microsoft.com> > > https://bugzilla.tianocore.org/show_bug.cgi?id=2522 > > Cc: Jordan Justen <jordan.l.jus...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Ard Biesheuvel <ard.biesheu...@arm.com> > Signed-off-by: Michael Kubacki <michael.kuba...@microsoft.com> > --- > OvmfPkg/OvmfPkgIa32.dsc | 8 ++++++++ > OvmfPkg/OvmfPkgIa32X64.dsc | 8 ++++++++ > OvmfPkg/OvmfPkgX64.dsc | 8 ++++++++ > 3 files changed, 24 insertions(+)
(1) Repeating my request from under the blurb, please cover "OvmfXen.dsc" too. > > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc > index 41ac3202961b..7c7b33a8bec3 100644 > --- a/OvmfPkg/OvmfPkgIa32.dsc > +++ b/OvmfPkg/OvmfPkgIa32.dsc > @@ -3,6 +3,7 @@ > # > # Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR> > # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR> > +# Copyright (c) Microsoft Corporation. > # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -196,6 +197,8 @@ > > 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 > > > # > @@ -334,6 +337,7 @@ > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf > PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf > QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf > + > VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf > > [LibraryClasses.common.UEFI_DRIVER] > PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf > @@ -492,6 +496,9 @@ > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize|0x40000 > !endif > > + # Optional: Omit if VariablePolicy should be always-on. > + > gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnforcementDisable|TRUE > + > gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 > > gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 (2) Based on the wiki article section here (which I find very useful): https://github.com/tianocore/tianocore.github.io/wiki/VariablePolicy-Protocol---Enhanced-Method-for-Managing-Variables#pcdallowvariablepolicyenforcementdisable I'd like us to drop this hunk. The default should be secure. Now, I realize that people might want to set this PCD to TRUE in OVMF, for testing various things. Maybe the unit tests / functional tests introduced in this series even *depend* on the PCD being TRUE (I can't tell, I haven't checked). That's OK; for accommodating that, we have two options: (2a) build OVMF with the appropriate --pcd=... switch passed to "build", (2b) for controlling the PCD dynamically (on the QEMU command line): - the PCD would have permit the dynamic access method in the DEC file, - the modules consuming the PCD would have to do so in their entry point functions / library constructors, and use the cached copy thenceforth, - the OVMF DSC files would have to get a dynamic default (value FALSE), - and OVMF would need another NULL class library for setting the PCD from fw_cfg. Please refer to <https://bugzilla.tianocore.org/show_bug.cgi?id=2681> for details on that. The patch looks OK to me otherwise. Thanks! Laszlo > @@ -945,6 +952,7 @@ > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf { > <LibraryClasses> > NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf > + NULL|MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > } > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf > > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc > index c2f11aee2cec..8d5c6b3fc4b6 100644 > --- a/OvmfPkg/OvmfPkgIa32X64.dsc > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc > @@ -3,6 +3,7 @@ > # > # Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR> > # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR> > +# Copyright (c) Microsoft Corporation. > # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -200,6 +201,8 @@ > > 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 > > > # > @@ -338,6 +341,7 @@ > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf > PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf > QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf > + > VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf > > [LibraryClasses.common.UEFI_DRIVER] > PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf > @@ -496,6 +500,9 @@ > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize|0x40000 > !endif > > + # Optional: Omit if VariablePolicy should be always-on. > + > gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnforcementDisable|TRUE > + > gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 > > gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 > @@ -959,6 +966,7 @@ > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf { > <LibraryClasses> > NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf > + NULL|MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > } > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf > > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > index 643e6041ad53..960d43eb1e84 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -3,6 +3,7 @@ > # > # Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR> > # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR> > +# Copyright (c) Microsoft Corporation. > # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -200,6 +201,8 @@ > > 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 > > > # > @@ -338,6 +341,7 @@ > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf > PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf > QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf > + > VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf > > [LibraryClasses.common.UEFI_DRIVER] > PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf > @@ -496,6 +500,9 @@ > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize|0x40000 > !endif > > + # Optional: Omit if VariablePolicy should be always-on. > + > gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnforcementDisable|TRUE > + > gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 > > gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 > @@ -956,6 +963,7 @@ > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf { > <LibraryClasses> > NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf > + NULL|MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > } > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#59271): https://edk2.groups.io/g/devel/message/59271 Mute This Topic: https://groups.io/mt/74153785/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
