[PATCH v1 0/9] Add the VariablePolicy feature

public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed

* [PATCH v1 0/9] Add the VariablePolicy feature
@ 2020-04-10 18:36 Michael Kubacki
  2020-04-11  2:24 ` Yao, Jiewen
  2020-04-14  0:41 ` [edk2-devel] " Nate DeSimone
  0 siblings, 2 replies; 6+ messages in thread
From: Michael Kubacki @ 2020-04-10 18:36 UTC (permalink / raw)
  To: devel; +Cc: Jiewen Yao, Chao Zhang, Jian J Wang, Hao A Wu, Liming Gao

From: Michael Kubacki <michael.kubacki@microsoft.com>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2522

The 9 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.

The 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).

On a separate note, shallow threading might not work on this patch series
due to changes made by the SMTP server. Please bear with me while I am
investigating if this can be changed.

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>
Signed-off-by: Bret Barkelew <brbarkel@microsoft.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>

Bret Barkelew (9):
  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/Library/VarCheckPolicyLib/VarCheckPolicyLib.c                               |  211 ++
 MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c                   |  396 ++++
 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c                               |  773 +++++++
 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.c   | 2285 ++++++++++++++++++++
 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                                 |   51 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLock.c                    |   71 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c                        |  445 ++++
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c                       |   15 +
 SecurityPkg/Library/AuthVariableLib/AuthService.c                                        |   22 +-
 MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h                                            |   43 +
 MdeModulePkg/Include/Library/VariablePolicyHelperLib.h                                   |  164 ++
 MdeModulePkg/Include/Library/VariablePolicyLib.h                                         |  206 ++
 MdeModulePkg/Include/Protocol/VariablePolicy.h                                           |  156 ++
 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                             |   38 +
 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni                             |   12 +
 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.inf |   41 +
 MdeModulePkg/MdeModulePkg.dec                                                            |   17 +-
 MdeModulePkg/MdeModulePkg.dsc                                                            |    7 +
 MdeModulePkg/Test/MdeModulePkgHostTest.dsc                                               |    8 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf                        |    5 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf                               |    4 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf                     |    8 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf                      |    4 +
 SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf                                  |    2 +
 31 files changed, 5172 insertions(+), 77 deletions(-)
 create mode 100644 MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
 create mode 100644 MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c
 create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
 create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.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/VariablePolicyUnitTest/VariablePolicyUnitTest.inf

-- 
2.16.3.windows.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 0/9] Add the VariablePolicy feature
  2020-04-10 18:36 [PATCH v1 0/9] Add the VariablePolicy feature Michael Kubacki
@ 2020-04-11  2:24 ` Yao, Jiewen
  2020-04-13 17:16   ` Michael Kubacki
  2020-04-14  0:41 ` [edk2-devel] " Nate DeSimone
  1 sibling, 1 reply; 6+ messages in thread
From: Yao, Jiewen @ 2020-04-11  2:24 UTC (permalink / raw)
  To: michael.kubacki@outlook.com, devel@edk2.groups.io
  Cc: Zhang, Chao B, Wang, Jian J, Wu, Hao A, Gao, Liming

Hi Michael
Thanks for the work.

I remember the feedback before that I have concern on having an API to *DisableVariablePolicy*, and I prefer we have a way to disable the *DisableVariablePolicy*.

May I know how that is addressed in this patch?

Thank you
Yao Jiewen




> -----Original Message-----
> From: michael.kubacki@outlook.com <michael.kubacki@outlook.com>
> Sent: Saturday, April 11, 2020 2:36 AM
> To: devel@edk2.groups.io
> 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>
> Subject: [PATCH v1 0/9] Add the VariablePolicy feature
> 
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2522
> 
> The 9 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.
> 
> The 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).
> 
> On a separate note, shallow threading might not work on this patch series
> due to changes made by the SMTP server. Please bear with me while I am
> investigating if this can be changed.
> 
> 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>
> Signed-off-by: Bret Barkelew <brbarkel@microsoft.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> Bret Barkelew (9):
>   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/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
> |  211 ++
>  MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c
> |  396 ++++
>  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c                               |
> 773 +++++++
> 
> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicy
> UnitTest.c   | 2285 ++++++++++++++++++++
>  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
> |   51 +
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLock.c
> |   71 +
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c
> |  445 ++++
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
> |   15 +
>  SecurityPkg/Library/AuthVariableLib/AuthService.c                                        |   22
> +-
>  MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h                                            |
> 43 +
>  MdeModulePkg/Include/Library/VariablePolicyHelperLib.h                                   |
> 164 ++
>  MdeModulePkg/Include/Library/VariablePolicyLib.h                                         |  206
> ++
>  MdeModulePkg/Include/Protocol/VariablePolicy.h                                           |  156
> ++
>  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                             |
> 38 +
>  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni
> |   12 +
> 
> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicy
> UnitTest.inf |   41 +
>  MdeModulePkg/MdeModulePkg.dec                                                            |   17 +-
>  MdeModulePkg/MdeModulePkg.dsc                                                            |    7 +
>  MdeModulePkg/Test/MdeModulePkgHostTest.dsc                                               |
> 8 +
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> |    5 +
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> |    4 +
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
> |    8 +
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> |    4 +
>  SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf                                  |    2
> +
>  31 files changed, 5172 insertions(+), 77 deletions(-)
>  create mode 100644
> MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
>  create mode 100644
> MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c
>  create mode 100644
> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
>  create mode 100644
> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicy
> UnitTest.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/VariablePolicyUnitTest/VariablePolicy
> UnitTest.inf
> 
> --
> 2.16.3.windows.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 0/9] Add the VariablePolicy feature
  2020-04-11  2:24 ` Yao, Jiewen
@ 2020-04-13 17:16   ` Michael Kubacki
  2020-04-14  5:24     ` [EXTERNAL] " Bret Barkelew
  0 siblings, 1 reply; 6+ messages in thread
From: Michael Kubacki @ 2020-04-13 17:16 UTC (permalink / raw)
  To: Yao, Jiewen, devel@edk2.groups.io
  Cc: Zhang, Chao B, Wang, Jian J, Wu, Hao A, Gao, Liming,
	Bret Barkelew

This particular series was Bret's work so I'll let him speak to it.

Thanks,
Michael

On 4/10/2020 7:24 PM, Yao, Jiewen wrote:
> Hi Michael
> Thanks for the work.
> 
> I remember the feedback before that I have concern on having an API to *DisableVariablePolicy*, and I prefer we have a way to disable the *DisableVariablePolicy*.
> 
> May I know how that is addressed in this patch?
> 
> Thank you
> Yao Jiewen
> 
> 
> 
> 
>> -----Original Message-----
>> From: michael.kubacki@outlook.com <michael.kubacki@outlook.com>
>> Sent: Saturday, April 11, 2020 2:36 AM
>> To: devel@edk2.groups.io
>> 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>
>> Subject: [PATCH v1 0/9] Add the VariablePolicy feature
>>
>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>
>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2522
>>
>> The 9 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.
>>
>> The 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).
>>
>> On a separate note, shallow threading might not work on this patch series
>> due to changes made by the SMTP server. Please bear with me while I am
>> investigating if this can be changed.
>>
>> 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>
>> Signed-off-by: Bret Barkelew <brbarkel@microsoft.com>
>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
>>
>> Bret Barkelew (9):
>>    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/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
>> |  211 ++
>>   MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c
>> |  396 ++++
>>   MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c                               |
>> 773 +++++++
>>
>> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicy
>> UnitTest.c   | 2285 ++++++++++++++++++++
>>   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
>> |   51 +
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLock.c
>> |   71 +
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c
>> |  445 ++++
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
>> |   15 +
>>   SecurityPkg/Library/AuthVariableLib/AuthService.c                                        |   22
>> +-
>>   MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h                                            |
>> 43 +
>>   MdeModulePkg/Include/Library/VariablePolicyHelperLib.h                                   |
>> 164 ++
>>   MdeModulePkg/Include/Library/VariablePolicyLib.h                                         |  206
>> ++
>>   MdeModulePkg/Include/Protocol/VariablePolicy.h                                           |  156
>> ++
>>   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                             |
>> 38 +
>>   MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni
>> |   12 +
>>
>> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicy
>> UnitTest.inf |   41 +
>>   MdeModulePkg/MdeModulePkg.dec                                                            |   17 +-
>>   MdeModulePkg/MdeModulePkg.dsc                                                            |    7 +
>>   MdeModulePkg/Test/MdeModulePkgHostTest.dsc                                               |
>> 8 +
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
>> |    5 +
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
>> |    4 +
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
>> |    8 +
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
>> |    4 +
>>   SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf                                  |    2
>> +
>>   31 files changed, 5172 insertions(+), 77 deletions(-)
>>   create mode 100644
>> MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
>>   create mode 100644
>> MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c
>>   create mode 100644
>> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
>>   create mode 100644
>> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicy
>> UnitTest.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/VariablePolicyUnitTest/VariablePolicy
>> UnitTest.inf
>>
>> --
>> 2.16.3.windows.1
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [EXTERNAL] Re: [PATCH v1 0/9] Add the VariablePolicy feature
  2020-04-13 17:16   ` Michael Kubacki
@ 2020-04-14  5:24     ` Bret Barkelew
  2020-04-15  2:59       ` Yao, Jiewen
  0 siblings, 1 reply; 6+ messages in thread
From: Bret Barkelew @ 2020-04-14  5:24 UTC (permalink / raw)
  To: Michael Kubacki, Yao, Jiewen, devel@edk2.groups.io
  Cc: Zhang, Chao B, Wang, Jian J, Wu, Hao A, Gao, Liming

[-- Attachment #1: Type: text/plain, Size: 9134 bytes --]

Jiewen,

Thanks (as always 😉) for the feedback! I’ll consider how best to address this and provide an update later this week after some others have had a chance to look at it.

- Bret

From: Michael Kubacki<mailto:michael.kubacki@outlook.com>
Sent: Monday, April 13, 2020 10:17 AM
To: Yao, Jiewen<mailto:jiewen.yao@intel.com>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Zhang, Chao B<mailto:chao.b.zhang@intel.com>; Wang, Jian J<mailto:jian.j.wang@intel.com>; Wu, Hao A<mailto:hao.a.wu@intel.com>; Gao, Liming<mailto:liming.gao@intel.com>; Bret Barkelew<mailto:Bret.Barkelew@microsoft.com>
Subject: [EXTERNAL] Re: [PATCH v1 0/9] Add the VariablePolicy feature

This particular series was Bret's work so I'll let him speak to it.

Thanks,
Michael

On 4/10/2020 7:24 PM, Yao, Jiewen wrote:
> Hi Michael
> Thanks for the work.
>
> I remember the feedback before that I have concern on having an API to *DisableVariablePolicy*, and I prefer we have a way to disable the *DisableVariablePolicy*.
>
> May I know how that is addressed in this patch?
>
> Thank you
> Yao Jiewen
>
>
>
>
>> -----Original Message-----
>> From: michael.kubacki@outlook.com <michael.kubacki@outlook.com>
>> Sent: Saturday, April 11, 2020 2:36 AM
>> To: devel@edk2.groups.io
>> 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>
>> Subject: [PATCH v1 0/9] Add the VariablePolicy feature
>>
>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>
>> REF:https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2522&amp;data=02%7C01%7CBret.Barkelew%40microsoft.com%7Ce2e70011eb234e05925108d7dfce776d%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637223950173802418&amp;sdata=qALCkFg05umllXL46sG5nAMmst99oyLYbyGSsqEWYtY%3D&amp;reserved=0
>>
>> The 9 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.
>>
>> The 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://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F53712&amp;data=02%7C01%7CBret.Barkelew%40microsoft.com%7Ce2e70011eb234e05925108d7dfce776d%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637223950173802418&amp;sdata=HupEk9iq0qxeXA5NYCNFoUV0uXa%2BvqYV81UX76bH9eQ%3D&amp;reserved=0
>> (the code branches shared in that discussion are now out of date, but the
>> whitepapers and discussion are relevant).
>>
>> On a separate note, shallow threading might not work on this patch series
>> due to changes made by the SMTP server. Please bear with me while I am
>> investigating if this can be changed.
>>
>> 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>
>> Signed-off-by: Bret Barkelew <brbarkel@microsoft.com>
>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
>>
>> Bret Barkelew (9):
>>    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/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
>> |  211 ++
>>   MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c
>> |  396 ++++
>>   MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c                               |
>> 773 +++++++
>>
>> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicy
>> UnitTest.c   | 2285 ++++++++++++++++++++
>>   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
>> |   51 +
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLock.c
>> |   71 +
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c
>> |  445 ++++
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
>> |   15 +
>>   SecurityPkg/Library/AuthVariableLib/AuthService.c                                        |   22
>> +-
>>   MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h                                            |
>> 43 +
>>   MdeModulePkg/Include/Library/VariablePolicyHelperLib.h                                   |
>> 164 ++
>>   MdeModulePkg/Include/Library/VariablePolicyLib.h                                         |  206
>> ++
>>   MdeModulePkg/Include/Protocol/VariablePolicy.h                                           |  156
>> ++
>>   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                             |
>> 38 +
>>   MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni
>> |   12 +
>>
>> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicy
>> UnitTest.inf |   41 +
>>   MdeModulePkg/MdeModulePkg.dec                                                            |   17 +-
>>   MdeModulePkg/MdeModulePkg.dsc                                                            |    7 +
>>   MdeModulePkg/Test/MdeModulePkgHostTest.dsc                                               |
>> 8 +
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
>> |    5 +
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
>> |    4 +
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
>> |    8 +
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
>> |    4 +
>>   SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf                                  |    2
>> +
>>   31 files changed, 5172 insertions(+), 77 deletions(-)
>>   create mode 100644
>> MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
>>   create mode 100644
>> MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c
>>   create mode 100644
>> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
>>   create mode 100644
>> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicy
>> UnitTest.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/VariablePolicyUnitTest/VariablePolicy
>> UnitTest.inf
>>
>> --
>> 2.16.3.windows.1
>


[-- Attachment #2: Type: text/html, Size: 16711 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [EXTERNAL] Re: [PATCH v1 0/9] Add the VariablePolicy feature
  2020-04-14  5:24     ` [EXTERNAL] " Bret Barkelew
@ 2020-04-15  2:59       ` Yao, Jiewen
  0 siblings, 0 replies; 6+ messages in thread
From: Yao, Jiewen @ 2020-04-15  2:59 UTC (permalink / raw)
  To: Bret Barkelew, Michael Kubacki, devel@edk2.groups.io
  Cc: Zhang, Chao B, Wang, Jian J, Wu, Hao A, Gao, Liming

[-- Attachment #1: Type: text/plain, Size: 10085 bytes --]

Cool. Thank you Bret!

From: Bret Barkelew <Bret.Barkelew@microsoft.com>
Sent: Tuesday, April 14, 2020 1:25 PM
To: Michael Kubacki <michael.kubacki@outlook.com>; Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
Cc: 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>
Subject: RE: [EXTERNAL] Re: [PATCH v1 0/9] Add the VariablePolicy feature

Jiewen,

Thanks (as always 😉) for the feedback! I’ll consider how best to address this and provide an update later this week after some others have had a chance to look at it.

- Bret

From: Michael Kubacki<mailto:michael.kubacki@outlook.com>
Sent: Monday, April 13, 2020 10:17 AM
To: Yao, Jiewen<mailto:jiewen.yao@intel.com>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Zhang, Chao B<mailto:chao.b.zhang@intel.com>; Wang, Jian J<mailto:jian.j.wang@intel.com>; Wu, Hao A<mailto:hao.a.wu@intel.com>; Gao, Liming<mailto:liming.gao@intel.com>; Bret Barkelew<mailto:Bret.Barkelew@microsoft.com>
Subject: [EXTERNAL] Re: [PATCH v1 0/9] Add the VariablePolicy feature

This particular series was Bret's work so I'll let him speak to it.

Thanks,
Michael

On 4/10/2020 7:24 PM, Yao, Jiewen wrote:
> Hi Michael
> Thanks for the work.
>
> I remember the feedback before that I have concern on having an API to *DisableVariablePolicy*, and I prefer we have a way to disable the *DisableVariablePolicy*.
>
> May I know how that is addressed in this patch?
>
> Thank you
> Yao Jiewen
>
>
>
>
>> -----Original Message-----
>> From: michael.kubacki@outlook.com<mailto:michael.kubacki@outlook.com> <michael.kubacki@outlook.com<mailto:michael.kubacki@outlook.com>>
>> Sent: Saturday, April 11, 2020 2:36 AM
>> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
>> Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Zhang, Chao B
>> <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A
>> <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>
>> Subject: [PATCH v1 0/9] Add the VariablePolicy feature
>>
>> From: Michael Kubacki <michael.kubacki@microsoft.com<mailto:michael.kubacki@microsoft.com>>
>>
>> REF:https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2522&amp;data=02%7C01%7CBret.Barkelew%40microsoft.com%7Ce2e70011eb234e05925108d7dfce776d%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637223950173802418&amp;sdata=qALCkFg05umllXL46sG5nAMmst99oyLYbyGSsqEWYtY%3D&amp;reserved=0
>>
>> The 9 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.
>>
>> The 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://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F53712&amp;data=02%7C01%7CBret.Barkelew%40microsoft.com%7Ce2e70011eb234e05925108d7dfce776d%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637223950173802418&amp;sdata=HupEk9iq0qxeXA5NYCNFoUV0uXa%2BvqYV81UX76bH9eQ%3D&amp;reserved=0
>> (the code branches shared in that discussion are now out of date, but the
>> whitepapers and discussion are relevant).
>>
>> On a separate note, shallow threading might not work on this patch series
>> due to changes made by the SMTP server. Please bear with me while I am
>> investigating if this can be changed.
>>
>> Cc: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
>> Cc: Chao Zhang <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
>> Cc: Jian J Wang <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>
>> Cc: Hao A Wu <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
>> Cc: Liming Gao <liming.gao@intel.com<mailto:liming.gao@intel.com>>
>> Signed-off-by: Bret Barkelew <brbarkel@microsoft.com<mailto:brbarkel@microsoft.com>>
>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com<mailto:michael.kubacki@microsoft.com>>
>>
>> Bret Barkelew (9):
>>    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/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
>> |  211 ++
>>   MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c
>> |  396 ++++
>>   MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c                               |
>> 773 +++++++
>>
>> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicy
>> UnitTest.c   | 2285 ++++++++++++++++++++
>>   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
>> |   51 +
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLock.c
>> |   71 +
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c
>> |  445 ++++
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
>> |   15 +
>>   SecurityPkg/Library/AuthVariableLib/AuthService.c                                        |   22
>> +-
>>   MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h                                            |
>> 43 +
>>   MdeModulePkg/Include/Library/VariablePolicyHelperLib.h                                   |
>> 164 ++
>>   MdeModulePkg/Include/Library/VariablePolicyLib.h                                         |  206
>> ++
>>   MdeModulePkg/Include/Protocol/VariablePolicy.h                                           |  156
>> ++
>>   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                             |
>> 38 +
>>   MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni
>> |   12 +
>>
>> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicy
>> UnitTest.inf |   41 +
>>   MdeModulePkg/MdeModulePkg.dec                                                            |   17 +-
>>   MdeModulePkg/MdeModulePkg.dsc                                                            |    7 +
>>   MdeModulePkg/Test/MdeModulePkgHostTest.dsc                                               |
>> 8 +
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
>> |    5 +
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
>> |    4 +
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
>> |    8 +
>>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
>> |    4 +
>>   SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf                                  |    2
>> +
>>   31 files changed, 5172 insertions(+), 77 deletions(-)
>>   create mode 100644
>> MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
>>   create mode 100644
>> MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c
>>   create mode 100644
>> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
>>   create mode 100644
>> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicy
>> UnitTest.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/VariablePolicyUnitTest/VariablePolicy
>> UnitTest.inf
>>
>> --
>> 2.16.3.windows.1
>


[-- Attachment #2: Type: text/html, Size: 19097 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [edk2-devel] [PATCH v1 0/9] Add the VariablePolicy feature
  2020-04-10 18:36 [PATCH v1 0/9] Add the VariablePolicy feature Michael Kubacki
  2020-04-11  2:24 ` Yao, Jiewen
@ 2020-04-14  0:41 ` Nate DeSimone
  1 sibling, 0 replies; 6+ messages in thread
From: Nate DeSimone @ 2020-04-14  0:41 UTC (permalink / raw)
  To: devel@edk2.groups.io, michael.kubacki@outlook.com
  Cc: Yao, Jiewen, Zhang, Chao B, Wang, Jian J, Wu, Hao A, Gao, Liming

Hi Michael and Bret,

First of all, Great Work!

One thing I noticed is that this change will require 2 new LibraryClasses to be added to every platform .dsc file. However, these changes have not been made to OvmfPkg, EmulatorPkg, RaspberryPi, or any of the MinPlatform *OpenBoardPkgs in edk2-platforms. When one adds changes that require platform updates like this, it is generally expected that the change author provide those updates at the same time for the platforms that are in TianoCore project.

For an example of this, see Pankaj's patch series from a month ago: https://edk2.groups.io/g/devel/message/54678

Also, looking at your code, I highly doubt you tested this with the Runtime DXE version of UEFI variable services. There are a non-zero number of platforms that use the Runtime DXE version of UEFI variable services, so this patch series will need to be updated and tested in Runtime DXE mode.

Also, looking at your code, I highly doubt you tested with standalone MM either. We are trying to move from the legacy DXE+SMM to pure SMM/MM only drivers as much as possible as the mixing of DXE + SMM in a single driver has been a historical source of bugs/security vulnerabilities since you can only safely use DXE/UEFI services in the entry point. Also, there are some platforms now that only work with standalone MM, for example a lot of ARM designs have moved to using standalone MM + ARM trust zone to implement UEFI secure boot authenticated variables.

I also have the following comments:

PATCH #2:

1) VariablePolicyLib is defined as a BASE library, but I highly doubt this will actually work as a proper BASE library. For example, you have a lot of mutable global variables at the top of VariablePolicyLib.c, but nothing in the implementation I see handles the EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE EFI event so this code won't work with Runtime DXE. Runtime DXE support is required for this code since some platforms use the Runtime DXE version of UEFI variable services.

PATCH #4:

1) Commented out code in VarCheckPolicyLibMmiHandler():
// VAR_CHECK_POLICY_COMM_DUMP_PARAMS         *DumpParams;     // Not yet implemented.

Please don't check in commented out code.

2) VarCheckPolicyLib.inf - You define this LibraryClass as supporting the following module types: DXE_RUNTIME_DRIVER DXE_SMM_DRIVER. This is a problem because you do not support the standalone MM drivers, only the legacy DXE+SMM driver type. While most platforms work OK with the legacy DXE+SMM drivers, standalone MM is used by several platform designs, ARM trust zone for example. Please update.

Thanks,
Nate

On 4/10/20, 11:36 AM, "devel@edk2.groups.io on behalf of Michael Kubacki" <devel@edk2.groups.io on behalf of michael.kubacki@outlook.com> wrote:

    From: Michael Kubacki <michael.kubacki@microsoft.com>
    
    REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2522
    
    The 9 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.
    
    The 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).
    
    On a separate note, shallow threading might not work on this patch series
    due to changes made by the SMTP server. Please bear with me while I am
    investigating if this can be changed.
    
    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>
    Signed-off-by: Bret Barkelew <brbarkel@microsoft.com>
    Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
    
    Bret Barkelew (9):
      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/Library/VarCheckPolicyLib/VarCheckPolicyLib.c                               |  211 ++
     MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c                   |  396 ++++
     MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c                               |  773 +++++++
     MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.c   | 2285 ++++++++++++++++++++
     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                                 |   51 +
     MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLock.c                    |   71 +
     MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c                        |  445 ++++
     MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c                       |   15 +
     SecurityPkg/Library/AuthVariableLib/AuthService.c                                        |   22 +-
     MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h                                            |   43 +
     MdeModulePkg/Include/Library/VariablePolicyHelperLib.h                                   |  164 ++
     MdeModulePkg/Include/Library/VariablePolicyLib.h                                         |  206 ++
     MdeModulePkg/Include/Protocol/VariablePolicy.h                                           |  156 ++
     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                             |   38 +
     MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni                             |   12 +
     MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.inf |   41 +
     MdeModulePkg/MdeModulePkg.dec                                                            |   17 +-
     MdeModulePkg/MdeModulePkg.dsc                                                            |    7 +
     MdeModulePkg/Test/MdeModulePkgHostTest.dsc                                               |    8 +
     MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf                        |    5 +
     MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf                               |    4 +
     MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf                     |    8 +
     MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf                      |    4 +
     SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf                                  |    2 +
     31 files changed, 5172 insertions(+), 77 deletions(-)
     create mode 100644 MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
     create mode 100644 MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c
     create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
     create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.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/VariablePolicyUnitTest/VariablePolicyUnitTest.inf
    
    -- 
    2.16.3.windows.1
    
    
    
    
    


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-04-15  2:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-10 18:36 [PATCH v1 0/9] Add the VariablePolicy feature Michael Kubacki
2020-04-11  2:24 ` Yao, Jiewen
2020-04-13 17:16   ` Michael Kubacki
2020-04-14  5:24     ` [EXTERNAL] " Bret Barkelew
2020-04-15  2:59       ` Yao, Jiewen
2020-04-14  0:41 ` [edk2-devel] " Nate DeSimone

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox