[edk2-devel] [PATCH v2 00/11] Enhance Secure Boot Variable Libraries
Yao, Jiewen
jiewen.yao at intel.com
Thu Jun 30 00:19:37 UTC 2022
Sounds great.
1) I assume that if it is accepted by project MU, then it must be reviewed and tested. Please add required tag in next patch set.
2) I suggest just use one of: a) all zero, b) initial data Jan/1/1970, c) current data.
With above change, reviewed-by: Jiewen Yao <Jiewen.yao at intel.com>
Thank you
Yao, Jiewen
From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Kun Qin
Sent: Thursday, June 30, 2022 2:07 AM
To: devel at edk2.groups.io; Yao, Jiewen <jiewen.yao at intel.com>
Cc: Wang, Jian J <jian.j.wang at intel.com>; Xu, Min M <min.m.xu at intel.com>; Sean Brogan <sean.brogan at microsoft.com>; Ard Biesheuvel <ardb+tianocore at kernel.org>; Justen, Jordan L <jordan.l.justen at intel.com>; Gerd Hoffmann <kraxel at redhat.com>; Rebecca Cran <rebecca at bsdio.com>; Peter Grehan <grehan at freebsd.org>; Boeuf, Sebastien <sebastien.boeuf at intel.com>; Andrew Fish <afish at apple.com>; Ni, Ray <ray.ni at intel.com>
Subject: Re: [edk2-devel] [PATCH v2 00/11] Enhance Secure Boot Variable Libraries
Hi Jiewen,
Thanks for reading through these patches.
For #1, yes, we implemented these changes in project MU and validated them on both our virtual platform (https://github.com/microsoft/mu_tiano_platforms) and other proprietary hardware platforms. I will leave the acked-by or tested-by to others on the MU teams.
For #2, this is just an arbitrary timestamp from a previous date so that we can create time based payload without hard dependency on time protocol (which could result in potential executing sequence issue). I will update comment to avoid confusion in the next round of patch. But should you have other suggestions to improve this, please let me know.
Regards,
Kun
On 6/29/2022 1:50 AM, Yao, Jiewen wrote:
Hi Kun
Thank you to make the redesign.
Overall the patch set looks good to me. Some questions:
1. Is that from project MU? If so, I would like to see acked-by or tested-by from project MU owner. That can give me more confidence to accept it. 😊
1. Is below data from some document? If so, would please add URL? Also, why do we have to use this timestamp? What if a different timestamp is used?
+// MS Default Time-Based Payload Creation Date
+// This is the date that is used when creating SecureBoot default variables.
+// NOTE: This is a placeholder date that doesn't correspond to anything else.
+//
+EFI_TIME mDefaultPayloadTimestamp = {
+ 15, // Year (2015)
+ 8, // Month (Aug)
+ 28, // Day (28)
+ 0, // Hour
+ 0, // Minute
+ 0, // Second
+ 0, // Pad1
+ 0, // Nanosecond
+ 0, // Timezone (Dummy value)
+ 0, // Daylight (Dummy value)
+ 0 // Pad2
+};
From: Kun Qin <kuqin12 at gmail.com><mailto:kuqin12 at gmail.com>
Sent: Wednesday, June 29, 2022 5:19 AM
To: edk2-devel-groups-io <devel at edk2.groups.io><mailto:devel at edk2.groups.io>; kuqin12 at gmail.com<mailto:kuqin12 at gmail.com>
Cc: Yao, Jiewen <jiewen.yao at intel.com><mailto:jiewen.yao at intel.com>; Wang, Jian J <jian.j.wang at intel.com><mailto:jian.j.wang at intel.com>; Xu, Min M <min.m.xu at intel.com><mailto:min.m.xu at intel.com>; Sean Brogan <sean.brogan at microsoft.com><mailto:sean.brogan at microsoft.com>; Ard Biesheuvel <ardb+tianocore at kernel.org><mailto:ardb+tianocore at kernel.org>; Justen, Jordan L <jordan.l.justen at intel.com><mailto:jordan.l.justen at intel.com>; Gerd Hoffmann <kraxel at redhat.com><mailto:kraxel at redhat.com>; Rebecca Cran <rebecca at bsdio.com><mailto:rebecca at bsdio.com>; Peter Grehan <grehan at freebsd.org><mailto:grehan at freebsd.org>; Boeuf, Sebastien <sebastien.boeuf at intel.com><mailto:sebastien.boeuf at intel.com>; Andrew Fish <afish at apple.com><mailto:afish at apple.com>; Ni, Ray <ray.ni at intel.com><mailto:ray.ni at intel.com>
Subject: Re: [edk2-devel] [PATCH v2 00/11] Enhance Secure Boot Variable Libraries
Hi SecurityPkg maintainers & reviewers,
I posted this patch series a while back intending to generalize the usage of a few interfaces from secure boot libraries. Could you please help reviewing them and provide feedback? Any input is appreciated.
Regards,
Kun
On Mon, Jun 13, 2022 at 1:39 PM Kun Qin via groups.io<http://groups.io> <kuqin12=gmail.com at groups.io<mailto:gmail.com at groups.io>> wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3909
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3910
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3911
This is a revamp of a previously submitted patch series based on top of
master branch: https://edk2.groups.io/g/devel/message/89507. No changes
added.
Current SecureBootVariableLib provide great support for deleting secure
boot related variables, creating time-based payloads.
However, for secure boot enrollment, the SecureBootVariableProvisionLib
interfaces always assume the changes from variable storage, limiting the
usage, requiring existing platforms to change key initialization process
to adapt to the new methods, as well as bringing in extra dependencies
such as FV protocol, time protocols.
This patch series proposes to update the implementation for Secure Boot
Variable libraries and their consumers to better support the related
variables operations.
Patch v2 branch: https://github.com/kuqin12/edk2/tree/secure_boot_enhance_v2
Cc: Jiewen Yao <jiewen.yao at intel.com<mailto:jiewen.yao at intel.com>>
Cc: Jian J Wang <jian.j.wang at intel.com<mailto:jian.j.wang at intel.com>>
Cc: Min Xu <min.m.xu at intel.com<mailto:min.m.xu at intel.com>>
Cc: Sean Brogan <sean.brogan at microsoft.com<mailto:sean.brogan at microsoft.com>>
Cc: Ard Biesheuvel <ardb+tianocore at kernel.org<mailto:ardb%2Btianocore at kernel.org>>
Cc: Jordan Justen <jordan.l.justen at intel.com<mailto:jordan.l.justen at intel.com>>
Cc: Gerd Hoffmann <kraxel at redhat.com<mailto:kraxel at redhat.com>>
Cc: Rebecca Cran <rebecca at bsdio.com<mailto:rebecca at bsdio.com>>
Cc: Peter Grehan <grehan at freebsd.org<mailto:grehan at freebsd.org>>
Cc: Sebastien Boeuf <sebastien.boeuf at intel.com<mailto:sebastien.boeuf at intel.com>>
Cc: Andrew Fish <afish at apple.com<mailto:afish at apple.com>>
Cc: Ray Ni <ray.ni at intel.com<mailto:ray.ni at intel.com>>
Kun Qin (8):
SecurityPkg: UefiSecureBoot: Definitions of cert and payload
structures
SecurityPkg: PlatformPKProtectionLib: Added PK protection interface
SecurityPkg: SecureBootVariableLib: Updated time based payload creator
SecurityPkg: SecureBootVariableProvisionLib: Updated implementation
SecurityPkg: Secure Boot Drivers: Added common header files
SecurityPkg: SecureBootConfigDxe: Updated invocation pattern
OvmfPkg: Pipeline: Resolve SecureBootVariableLib dependency
EmulatorPkg: Pipeline: Resolve SecureBootVariableLib dependency
kuqin (3):
SecurityPkg: SecureBootVariableLib: Updated signature list creator
SecurityPkg: SecureBootVariableLib: Added newly supported interfaces
SecurityPkg: SecureBootVariableLib: Added unit tests
SecurityPkg/EnrollFromDefaultKeysApp/EnrollFromDefaultKeysApp.c | 1 +
SecurityPkg/Library/PlatformPKProtectionLibVarPolicy/PlatformPKProtectionLibVarPolicy.c | 51 +
SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c | 486 ++++-
SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockPlatformPKProtectionLib.c | 36 +
SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockUefiLib.c | 201 ++
SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockUefiRuntimeServicesTableLib.c | 13 +
SecurityPkg/Library/SecureBootVariableLib/UnitTest/SecureBootVariableLibUnitTest.c | 2037 ++++++++++++++++++++
SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.c | 145 +-
SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c | 128 +-
SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootDefaultKeysDxe.c | 1 +
EmulatorPkg/EmulatorPkg.dsc | 1 +
OvmfPkg/Bhyve/BhyveX64.dsc | 1 +
OvmfPkg/CloudHv/CloudHvX64.dsc | 1 +
OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
SecurityPkg/Include/Library/PlatformPKProtectionLib.h | 31 +
SecurityPkg/Include/Library/SecureBootVariableLib.h | 103 +-
SecurityPkg/Include/UefiSecureBoot.h | 94 +
SecurityPkg/Library/PlatformPKProtectionLibVarPolicy/PlatformPKProtectionLibVarPolicy.inf | 36 +
SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.inf | 14 +-
SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockPlatformPKProtectionLib.inf | 33 +
SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockUefiLib.inf | 45 +
SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockUefiRuntimeServicesTableLib.inf | 25 +
SecurityPkg/Library/SecureBootVariableLib/UnitTest/SecureBootVariableLibUnitTest.inf | 36 +
SecurityPkg/SecurityPkg.ci.yaml | 11 +
SecurityPkg/SecurityPkg.dec | 5 +
SecurityPkg/SecurityPkg.dsc | 2 +
SecurityPkg/Test/SecurityPkgHostTest.dsc | 38 +
SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf | 1 +
31 files changed, 3468 insertions(+), 112 deletions(-)
create mode 100644 SecurityPkg/Library/PlatformPKProtectionLibVarPolicy/PlatformPKProtectionLibVarPolicy.c
create mode 100644 SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockPlatformPKProtectionLib.c
create mode 100644 SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockUefiLib.c
create mode 100644 SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockUefiRuntimeServicesTableLib.c
create mode 100644 SecurityPkg/Library/SecureBootVariableLib/UnitTest/SecureBootVariableLibUnitTest.c
create mode 100644 SecurityPkg/Include/Library/PlatformPKProtectionLib.h
create mode 100644 SecurityPkg/Include/UefiSecureBoot.h
create mode 100644 SecurityPkg/Library/PlatformPKProtectionLibVarPolicy/PlatformPKProtectionLibVarPolicy.inf
create mode 100644 SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockPlatformPKProtectionLib.inf
create mode 100644 SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockUefiLib.inf
create mode 100644 SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockUefiRuntimeServicesTableLib.inf
create mode 100644 SecurityPkg/Library/SecureBootVariableLib/UnitTest/SecureBootVariableLibUnitTest.inf
create mode 100644 SecurityPkg/Test/SecurityPkgHostTest.dsc
--
2.35.1.windows.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#90920): https://edk2.groups.io/g/devel/message/90920
Mute This Topic: https://groups.io/mt/91735867/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20220630/6af3abe5/attachment-0001.htm>
More information about the edk2-devel-archive
mailing list