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. 😊 > > 2. 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 > *Sent:* Wednesday, June 29, 2022 5:19 AM > *To:* edk2-devel-groups-io ; kuqin12@gmail.com > *Cc:* Yao, Jiewen ; Wang, Jian J > ; Xu, Min M ; Sean Brogan > ; Ard Biesheuvel > ; Justen, Jordan L > ; Gerd Hoffmann ; > Rebecca Cran ; Peter Grehan ; > Boeuf, Sebastien ; Andrew Fish > ; Ni, Ray > *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 > 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 > Cc: Jian J Wang > Cc: Min Xu > Cc: Sean Brogan > Cc: Ard Biesheuvel > > Cc: Jordan Justen > Cc: Gerd Hoffmann > Cc: Rebecca Cran > Cc: Peter Grehan > Cc: Sebastien Boeuf > Cc: Andrew Fish > Cc: Ray Ni > > 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 > > > > > >