Re: [edk2-devel] [PATCH v2 00/12] Add the VariablePolicy feature

["Liming Gao" <liming.gao@intel.com>]

Michael:

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Tuesday, May 12, 2020 7:53 PM
> To: devel@edk2.groups.io; michael.kubacki@outlook.com
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Gao, Liming <liming.gao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@arm.com>; Andrew Fish <afish@apple.com>; Ni, Ray <ray.ni@intel.com>; Anthony Perard
> <anthony.perard@citrix.com>; Julien Grall <julien@xen.org>; Ma, Maurice <maurice.ma@intel.com>; Dong, Guo
> <guo.dong@intel.com>; You, Benjamin <benjamin.you@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2 00/12] Add the VariablePolicy feature
> 
> On 05/12/20 08:46, Michael Kubacki wrote:
> > From: Michael Kubacki <michael.kubacki@microsoft.com>
> >
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2522
> >
> > The 12 patches in this series add the VariablePolicy feature to the core,
> > deprecate Edk2VarLock (while adding a compatibility layer to reduce code
> > churn), and integrate the VariablePolicy libraries and protocols into
> > Variable Services.
> >
> > Since the integration requires multiple changes, including adding libraries,
> > a protocol, an SMI communication handler, and VariableServices integration,
> > the patches are broken up by individual library additions and then a final
> > integration. Security-sensitive changes like bypassing Authenticated
> > Variable enforcement are also broken out into individual patches so that
> > attention can be called directly to them.
> >
> > Platform porting instructions are described in this wiki entry:
> > https://github.com/tianocore/tianocore.github.io/wiki/VariablePolicy-Protocol---Enhanced-Method-for-Managing-
> Variables#platform-porting
> 
> (1) This wiki article is helpful, thanks.
> 
> I have one remark: there's a heading saying "VarCheckPolicyLib NULL
> Instance":
> 
> https://github.com/tianocore/tianocore.github.io/wiki/VariablePolicy-Protocol---Enhanced-Method-for-Managing-
> Variables#varcheckpolicylib-null-instance
> 
> I think what's meant is "NULL class", not "NULL instance".
> 
> (2) The following platform DSC files in edk2 include the non-SMM
> variable driver
> ("MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf"),
> but they are not modified by this patch series:
> 
> ArmVirtPkg/ArmVirtQemu.dsc
> ArmVirtPkg/ArmVirtQemuKernel.dsc
> ArmVirtPkg/ArmVirtXen.dsc
> OvmfPkg/OvmfXen.dsc
> UefiPayloadPkg/UefiPayloadPkgIa32.dsc
> UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc
> 
> I'm asking that you please (a) include a patch for ArmVirtPkg, (b)
> update the OvmfPkg patch to cover "OvmfXen.dsc" too.
> 
> Not sure about the UefiPayloadPkg platforms; please ask their maintainers.
> 
> (I've CC'd the OvmfXen and UefiPayloadPkg maintainers/reviewers.)

To keep each commit build pass, the patch set should first add new library instance, 
then add the library instance into each platform DSC, last update Variable driver to consume new library instance. 

This patch also impacts the platforms in edk2-platforms. If you expect the platform maintainer to update their platform DSC, 
please send the mail and let them aware this change or submit BZ to track the changes in edk2-platforms. 

Thanks
Liming
> 
> Thanks,
> Laszlo
> 
> >
> > Discussion of the feature can be found in multiple places throughout
> > the last year on the RFC channel, staging branches, and in devel.
> >
> > Most recently, this subject was discussed in this thread:
> > https://edk2.groups.io/g/devel/message/53712
> > (the code branches shared in that discussion are now out of date, but the
> > whitepapers and discussion are relevant).
> >
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Chao Zhang <chao.b.zhang@intel.com>
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> > Cc: Andrew Fish <afish@apple.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> >
> > Changes since v1:
> > * Fixed implementation for RuntimeDxe
> > * Add PCD to block DisableVariablePolicy
> > * Fix the DumpVariablePolicy pagination in SMM
> >
> > Bret Barkelew (12):
> >   MdeModulePkg: Define the VariablePolicy protocol interface
> >   MdeModulePkg: Define the VariablePolicyLib
> >   MdeModulePkg: Define the VariablePolicyHelperLib
> >   MdeModulePkg: Define the VarCheckPolicyLib and SMM interface
> >   MdeModulePkg: Connect VariablePolicy business logic to
> >     VariableServices
> >   MdeModulePkg: Allow VariablePolicy state to delete protected variables
> >   SecurityPkg: Allow VariablePolicy state to delete authenticated
> >     variables
> >   MdeModulePkg: Change TCG MOR variables to use VariablePolicy
> >   MdeModulePkg: Drop VarLock from RuntimeDxe variable driver
> >   MdeModulePkg: Add a shell-based functional test for VariablePolicy
> >   OvmfPkg: Add VariablePolicy engine to OvmfPkg platform
> >   EmulatorPkg: Add VariablePolicy engine to EmulatorPkg platform
> >
> >  MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c                               |  318 +++
> >  MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c                   |  396 ++++
> >  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitNull.c                     |   46 +
> >  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitRuntimeDxe.c               |   85 +
> >  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c                               |  806 +++++++
> >  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.c   | 2285 ++++++++++++++++++++
> >  MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/VariablePolicyFuncTestApp.c        | 1942 +++++++++++++++++
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c                               |   52 +-
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c                               |   60 +-
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c                                    |   49 +-
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c                                 |   53 +
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLock.c                    |   71 +
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c                        |  653 ++++++
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c                       |   14 +
> >  SecurityPkg/Library/AuthVariableLib/AuthService.c                                        |   22 +-
> >  EmulatorPkg/EmulatorPkg.dsc                                                              |    6 +
> >  MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h                                            |   54 +
> >  MdeModulePkg/Include/Library/VariablePolicyHelperLib.h                                   |  164 ++
> >  MdeModulePkg/Include/Library/VariablePolicyLib.h                                         |  207 ++
> >  MdeModulePkg/Include/Protocol/VariablePolicy.h                                           |  157 ++
> >  MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf                             |   44 +
> >  MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.uni                             |   12 +
> >  MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf                 |   36 +
> >  MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.uni                 |   12 +
> >  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf                             |   44 +
> >  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni                             |   12 +
> >  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf                   |   52 +
> >  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.inf |   41 +
> >  MdeModulePkg/MdeModulePkg.ci.yaml                                                        |    4 +-
> >  MdeModulePkg/MdeModulePkg.dec                                                            |   26 +-
> >  MdeModulePkg/MdeModulePkg.dsc                                                            |   15 +
> >  MdeModulePkg/Test/MdeModulePkgHostTest.dsc                                               |   11 +
> >  MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/Readme.md                          |   55 +
> >  MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/VariablePolicyFuncTestApp.inf      |   42 +
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf                        |    5 +
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf                               |    4 +
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf                     |   12 +
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf                      |    4 +
> >  OvmfPkg/OvmfPkgIa32.dsc                                                                  |    8 +
> >  OvmfPkg/OvmfPkgIa32X64.dsc                                                               |    8 +
> >  OvmfPkg/OvmfPkgX64.dsc                                                                   |    8 +
> >  SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf                                  |    2 +
> >  42 files changed, 7819 insertions(+), 78 deletions(-)
> >  create mode 100644 MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
> >  create mode 100644 MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c
> >  create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitNull.c
> >  create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitRuntimeDxe.c
> >  create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
> >  create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.c
> >  create mode 100644 MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/VariablePolicyFuncTestApp.c
> >  create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLock.c
> >  create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c
> >  create mode 100644 MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h
> >  create mode 100644 MdeModulePkg/Include/Library/VariablePolicyHelperLib.h
> >  create mode 100644 MdeModulePkg/Include/Library/VariablePolicyLib.h
> >  create mode 100644 MdeModulePkg/Include/Protocol/VariablePolicy.h
> >  create mode 100644 MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf
> >  create mode 100644 MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.uni
> >  create mode 100644 MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
> >  create mode 100644 MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.uni
> >  create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
> >  create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni
> >  create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf
> >  create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.inf
> >  create mode 100644 MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/Readme.md
> >  create mode 100644 MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/VariablePolicyFuncTestApp.inf
> >