[edk2-devel] [PATCH v2 11/12] OvmfPkg: Add VariablePolicy engine to OvmfPkg platform
Laszlo Ersek
lersek at redhat.com
Tue May 12 12:05:35 UTC 2020
On 05/12/20 08:46, Michael Kubacki wrote:
> From: Bret Barkelew <brbarkel at microsoft.com>
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=2522
>
> Cc: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Laszlo Ersek <lersek at redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel at arm.com>
> Signed-off-by: Michael Kubacki <michael.kubacki at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list