<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> </head> <body> Thanks, Jiewen. I will incorporate the change you suggested below and re-validate on the platforms we have, while I wait for the tags from MU repo owners. Regards, Kun <div class="moz-cite-prefix">On 6/29/2022 5:19 PM, Yao, Jiewen wrote: </div> <blockquote type="cite" cite="mid:MW4PR11MB5872218B292DAE6EDBD3B48D8CBA9@MW4PR11MB5872.namprd11.prod.outlook.com"> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)">  <style>@font-face {font-family:"Cambria Math"; panose-1:2 4 5 3 5 4 6 3 2 4;}@font-face {font-family:DengXian; panose-1:2 1 6 0 3 1 1 1 1 1;}@font-face {font-family:Calibri; panose-1:2 15 5 2 2 2 4 3 2 4;}@font-face {font-family:"Segoe UI Emoji"; panose-1:2 11 5 2 4 2 4 2 2 3;}@font-face {font-family:"\@DengXian"; panose-1:2 1 6 0 3 1 1 1 1 1;}p.MsoNormal, li.MsoNormal, div.MsoNormal {margin:0in; font-size:11.0pt; font-family:"Calibri",sans-serif;}a:link, span.MsoHyperlink {mso-style-priority:99; color:blue; text-decoration:underline;}p.MsoListParagraph, li.MsoListParagraph, div.MsoListParagraph {mso-style-priority:34; margin-top:0in; margin-right:0in; margin-bottom:0in; margin-left:.5in; font-size:11.0pt; font-family:"Calibri",sans-serif;}span.EmailStyle21 {mso-style-type:personal-reply; font-family:"Calibri",sans-serif; color:windowtext;}.MsoChpDefault {mso-style-type:export-only; font-size:10.0pt;}div.WordSection1 {page:WordSection1;}ol {margin-bottom:0in;}ul {margin-bottom:0in;}</style> <div class="WordSection1"> Sounds great.<o:p></o:p> <o:p> </o:p> 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.<o:p></o:p> <o:p> </o:p> 2) I suggest just use one of: a) all zero, b) initial data Jan/1/1970, c) current data.<o:p></o:p> <o:p> </o:p> With above change, reviewed-by: Jiewen Yao <a class="moz-txt-link-rfc2396E" href="mailto:Jiewen.yao@intel.com"><Jiewen.yao@intel.com></a><o:p></o:p> <o:p> </o:p> Thank you<o:p></o:p> Yao, Jiewen<o:p></o:p> <o:p> </o:p> <o:p> </o:p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> From: <a class="moz-txt-link-abbreviated" href="mailto:devel@edk2.groups.io">devel@edk2.groups.io</a> <a class="moz-txt-link-rfc2396E" href="mailto:devel@edk2.groups.io"><devel@edk2.groups.io></a> On Behalf Of Kun Qin Sent: Thursday, June 30, 2022 2:07 AM To: <a class="moz-txt-link-abbreviated" href="mailto:devel@edk2.groups.io">devel@edk2.groups.io</a>; Yao, Jiewen <a class="moz-txt-link-rfc2396E" href="mailto:jiewen.yao@intel.com"><jiewen.yao@intel.com></a> Cc: Wang, Jian J <a class="moz-txt-link-rfc2396E" href="mailto:jian.j.wang@intel.com"><jian.j.wang@intel.com></a>; Xu, Min M <a class="moz-txt-link-rfc2396E" href="mailto:min.m.xu@intel.com"><min.m.xu@intel.com></a>; Sean Brogan <a class="moz-txt-link-rfc2396E" href="mailto:sean.brogan@microsoft.com"><sean.brogan@microsoft.com></a>; Ard Biesheuvel <a class="moz-txt-link-rfc2396E" href="mailto:ardb+tianocore@kernel.org"><ardb+tianocore@kernel.org></a>; Justen, Jordan L <a class="moz-txt-link-rfc2396E" href="mailto:jordan.l.justen@intel.com"><jordan.l.justen@intel.com></a>; Gerd Hoffmann <a class="moz-txt-link-rfc2396E" href="mailto:kraxel@redhat.com"><kraxel@redhat.com></a>; Rebecca Cran <a class="moz-txt-link-rfc2396E" href="mailto:rebecca@bsdio.com"><rebecca@bsdio.com></a>; Peter Grehan <a class="moz-txt-link-rfc2396E" href="mailto:grehan@freebsd.org"><grehan@freebsd.org></a>; Boeuf, Sebastien <a class="moz-txt-link-rfc2396E" href="mailto:sebastien.boeuf@intel.com"><sebastien.boeuf@intel.com></a>; Andrew Fish <a class="moz-txt-link-rfc2396E" href="mailto:afish@apple.com"><afish@apple.com></a>; Ni, Ray <a class="moz-txt-link-rfc2396E" href="mailto:ray.ni@intel.com"><ray.ni@intel.com></a> Subject: Re: [edk2-devel] [PATCH v2 00/11] Enhance Secure Boot Variable Libraries<o:p></o:p> </div> </div> <o:p> </o:p> Hi Jiewen,<o:p></o:p> Thanks for reading through these patches.<o:p></o:p> For #1, yes, we implemented these changes in project MU and validated them on both our virtual platform (<a href="https://github.com/microsoft/mu_tiano_platforms" moz-do-not-send="true" class="moz-txt-link-freetext">https://github.com/microsoft/mu_tiano_platforms</a>) and other proprietary hardware platforms. I will leave the acked-by or tested-by to others on the MU teams.<o:p></o:p> 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.<o:p></o:p> Regards, Kun<o:p></o:p> <div> On 6/29/2022 1:50 AM, Yao, Jiewen wrote:<o:p></o:p> </div> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> Hi Kun<o:p></o:p> Thank you to make the redesign.<o:p></o:p> <o:p></o:p> Overall the patch set looks good to me. Some questions:<o:p></o:p> <o:p></o:p> <ol style="margin-top:0in" type="1" start="1"> <li class="MsoListParagraph" style="margin-left:0in;mso-list:l2 level1 lfo3">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. 😊<o:p></o:p></li> </ol> <o:p></o:p> <ol style="margin-top:0in" type="1" start="2"> <li class="MsoListParagraph" style="margin-left:0in;mso-list:l2 level1 lfo3">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? <o:p></o:p></li> </ol> <o:p></o:p> +// MS Default Time-Based Payload Creation Date<o:p></o:p> +// This is the date that is used when creating SecureBoot default variables.<o:p></o:p> +// NOTE: This is a placeholder date that doesn't correspond to anything else.<o:p></o:p> +//<o:p></o:p> +EFI_TIME mDefaultPayloadTimestamp = {<o:p></o:p> + 15, // Year (2015)<o:p></o:p> + 8, // Month (Aug)<o:p></o:p> + 28, // Day (28)<o:p></o:p> + 0, // Hour<o:p></o:p> + 0, // Minute<o:p></o:p> + 0, // Second<o:p></o:p> + 0, // Pad1<o:p></o:p> + 0, // Nanosecond<o:p></o:p> + 0, // Timezone (Dummy value)<o:p></o:p> + 0, // Daylight (Dummy value)<o:p></o:p> + 0 // Pad2<o:p></o:p> +};<o:p></o:p> <o:p></o:p> <o:p></o:p> <div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt"> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> From: Kun Qin <a href="mailto:kuqin12@gmail.com" moz-do-not-send="true"><kuqin12@gmail.com></a> Sent: Wednesday, June 29, 2022 5:19 AM To: edk2-devel-groups-io <a href="mailto:devel@edk2.groups.io" moz-do-not-send="true"><devel@edk2.groups.io></a>; <a href="mailto:kuqin12@gmail.com" moz-do-not-send="true" class="moz-txt-link-freetext">kuqin12@gmail.com</a> Cc: Yao, Jiewen <a href="mailto:jiewen.yao@intel.com" moz-do-not-send="true"><jiewen.yao@intel.com></a>; Wang, Jian J <a href="mailto:jian.j.wang@intel.com" moz-do-not-send="true"><jian.j.wang@intel.com></a>; Xu, Min M <a href="mailto:min.m.xu@intel.com" moz-do-not-send="true"> <min.m.xu@intel.com></a>; Sean Brogan <a href="mailto:sean.brogan@microsoft.com" moz-do-not-send="true"> <sean.brogan@microsoft.com></a>; Ard Biesheuvel <a href="mailto:ardb+tianocore@kernel.org" moz-do-not-send="true"> <ardb+tianocore@kernel.org></a>; Justen, Jordan L <a href="mailto:jordan.l.justen@intel.com" moz-do-not-send="true"> <jordan.l.justen@intel.com></a>; Gerd Hoffmann <a href="mailto:kraxel@redhat.com" moz-do-not-send="true"> <kraxel@redhat.com></a>; Rebecca Cran <a href="mailto:rebecca@bsdio.com" moz-do-not-send="true"><rebecca@bsdio.com></a>; Peter Grehan <a href="mailto:grehan@freebsd.org" moz-do-not-send="true"><grehan@freebsd.org></a>; Boeuf, Sebastien <a href="mailto:sebastien.boeuf@intel.com" moz-do-not-send="true"> <sebastien.boeuf@intel.com></a>; Andrew Fish <a href="mailto:afish@apple.com" moz-do-not-send="true"><afish@apple.com></a>; Ni, Ray <a href="mailto:ray.ni@intel.com" moz-do-not-send="true"><ray.ni@intel.com></a> Subject: Re: [edk2-devel] [PATCH v2 00/11] Enhance Secure Boot Variable Libraries<o:p></o:p> </div> </div> <o:p></o:p> <div> <div> Hi SecurityPkg maintainers & reviewers,<o:p></o:p> <div> <o:p></o:p> </div> <div> 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.<o:p></o:p> </div> <div> <o:p></o:p> </div> <div> Regards,<o:p></o:p> </div> <div> Kun<o:p></o:p> </div> </div> <o:p></o:p> <div> <div> On Mon, Jun 13, 2022 at 1:39 PM Kun Qin via <a href="http://groups.io" moz-do-not-send="true"> groups.io</a> <kuqin12=<a href="mailto:gmail.com@groups.io" moz-do-not-send="true" class="moz-txt-link-freetext">gmail.com@groups.io</a>> wrote:<o:p></o:p> </div> <blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt"> REF: <a href="https://bugzilla.tianocore.org/show_bug.cgi?id=3909" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext"> https://bugzilla.tianocore.org/show_bug.cgi?id=3909</a> REF: <a href="https://bugzilla.tianocore.org/show_bug.cgi?id=3910" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext"> https://bugzilla.tianocore.org/show_bug.cgi?id=3910</a> REF: <a href="https://bugzilla.tianocore.org/show_bug.cgi?id=3911" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext"> https://bugzilla.tianocore.org/show_bug.cgi?id=3911</a> This is a revamp of a previously submitted patch series based on top of master branch: <a href="https://edk2.groups.io/g/devel/message/89507" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext"> https://edk2.groups.io/g/devel/message/89507</a>. 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: <a href="https://github.com/kuqin12/edk2/tree/secure_boot_enhance_v2" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext"> https://github.com/kuqin12/edk2/tree/secure_boot_enhance_v2</a> Cc: Jiewen Yao <<a href="mailto:jiewen.yao@intel.com" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">jiewen.yao@intel.com</a>> Cc: Jian J Wang <<a href="mailto:jian.j.wang@intel.com" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">jian.j.wang@intel.com</a>> Cc: Min Xu <<a href="mailto:min.m.xu@intel.com" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">min.m.xu@intel.com</a>> Cc: Sean Brogan <<a href="mailto:sean.brogan@microsoft.com" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">sean.brogan@microsoft.com</a>> Cc: Ard Biesheuvel <<a href="mailto:ardb%2Btianocore@kernel.org" target="_blank" moz-do-not-send="true">ardb+tianocore@kernel.org</a>> Cc: Jordan Justen <<a href="mailto:jordan.l.justen@intel.com" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">jordan.l.justen@intel.com</a>> Cc: Gerd Hoffmann <<a href="mailto:kraxel@redhat.com" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">kraxel@redhat.com</a>> Cc: Rebecca Cran <<a href="mailto:rebecca@bsdio.com" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">rebecca@bsdio.com</a>> Cc: Peter Grehan <<a href="mailto:grehan@freebsd.org" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">grehan@freebsd.org</a>> Cc: Sebastien Boeuf <<a href="mailto:sebastien.boeuf@intel.com" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">sebastien.boeuf@intel.com</a>> Cc: Andrew Fish <<a href="mailto:afish@apple.com" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">afish@apple.com</a>> Cc: Ray Ni <<a href="mailto:ray.ni@intel.com" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">ray.ni@intel.com</a>> 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 <o:p></o:p> </blockquote> </div> </div> </div> </blockquote> <div> </div> </div> </blockquote> </body> </html> <div width="1" style="color:white;clear:both">_._,_._,_</div> <hr> Groups.io Links: You receive all messages sent to this group. <a target="_blank" href="https://edk2.groups.io/g/devel/message/90921">View/Reply Online (#90921)</a> | | <a target="_blank" href="https://groups.io/mt/91735867/1813853">Mute This Topic</a> | <a href="https://edk2.groups.io/g/devel/post">New Topic</a> <a href="https://edk2.groups.io/g/devel/editsub/1813853">Your Subscription</a> | <a href="mailto:devel+owner@edk2.groups.io">Contact Group Owner</a> | <a href="https://edk2.groups.io/g/devel/unsub">Unsubscribe</a> [edk2-devel-archive@redhat.com] <div width="1" style="color:white;clear:both">_._,_._,_</div>