From: "Sean" <spbrogan@outlook.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
"jiewen.yao@intel.com" <jiewen.yao@intel.com>,
Jan Bobek <jbobek@nvidia.com>,
Sean Brogan <sean.brogan@microsoft.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Subject: Re: [edk2-devel] [PATCH v1 0/4] Don't require self-signed PK in setup mode
Date: Tue, 24 Jan 2023 21:51:37 -0800 [thread overview]
Message-ID: <BY3PR19MB49006A3AAB1E04381A5FC2BDC8CE9@BY3PR19MB4900.namprd19.prod.outlook.com> (raw)
In-Reply-To: <MW4PR11MB5872ADE7F30774B295F07EFA8CC89@MW4PR11MB5872.namprd11.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 3258 bytes --]
Jan,
From looking over the patch 1/4 email i have a concern.
In AuthService.c on the conditional change you made. Aren't we losing a
case where we are evaluating a nonPK payload signed by the PK? Given
the system is in SetupMode that means there is no PK but is this the
conditional path that is used when installing Secure boot keys in
reverse (DBX,DX,KEK,PK) order?
Is there testing you have done? This code should be pretty easy to do
host based unit testing on. Any chance you have authored that to
confirm use cases are not unexpectedly impacted? Example of host based
unit test of library is here:
edk2/SecurityPkg/Library/SecureBootVariableLib/UnitTest at master ·
tianocore/edk2 (github.com)
<https://github.com/tianocore/edk2/tree/master/SecurityPkg/Library/SecureBootVariableLib/UnitTest>
Thanks
Sean
On 1/22/2023 10:13 PM, Yao, Jiewen wrote:
> Hi Sean
> I would like to hear your feedback, since it is a little different from the original MSFT patch.
>
> Would you please take a look?
>
> Thank you
> Yao, Jiewen
>
>> -----Original Message-----
>> From: Jan Bobek<jbobek@nvidia.com>
>> Sent: Saturday, January 21, 2023 6:59 AM
>> To:devel@edk2.groups.io
>> Cc: Jan Bobek<jbobek@nvidia.com>; Laszlo Ersek<lersek@redhat.com>; Yao,
>> Jiewen<jiewen.yao@intel.com>
>> Subject: [PATCH v1 0/4] Don't require self-signed PK in setup mode
>>
>> Hi all,
>>
>> I'm sending out v1 of my patch series that addresses a UEFI spec
>> non-compliance when enrolling PK in setup mode. Additional info can be
>> found in bugzilla [1]; the changes are split into 4 patches as
>> suggested by Laszlo Ersek in comment #4.
>>
>> I've based my work on the patch by Matthew Carlson; I've credited him
>> with co-authorship of the first patch even though in the end I decided
>> to do the implementation a bit differently.
>>
>> Comments & reviews welcome!
>>
>> Cheers,
>> -Jan
>>
>> References:
>> 1.https://bugzilla.tianocore.org/show_bug.cgi?id=2506
>>
>> Jan Bobek (4):
>> SecurityPkg: limit verification of enrolled PK in setup mode
>> OvmfPkg: require self-signed PK when secure boot is enabled
>> ArmVirtPkg: require self-signed PK when secure boot is enabled
>> SecurityPkg: don't require PK to be self-signed by default
>>
>> SecurityPkg/SecurityPkg.dec | 7 +++++++
>> ArmVirtPkg/ArmVirtCloudHv.dsc | 4 ++++
>> ArmVirtPkg/ArmVirtQemu.dsc | 4 ++++
>> ArmVirtPkg/ArmVirtQemuKernel.dsc | 4 ++++
>> OvmfPkg/Bhyve/BhyveX64.dsc | 3 +++
>> OvmfPkg/CloudHv/CloudHvX64.dsc | 3 +++
>> OvmfPkg/IntelTdx/IntelTdxX64.dsc | 3 +++
>> OvmfPkg/Microvm/MicrovmX64.dsc | 3 +++
>> OvmfPkg/OvmfPkgIa32.dsc | 3 +++
>> OvmfPkg/OvmfPkgIa32X64.dsc | 3 +++
>> OvmfPkg/OvmfPkgX64.dsc | 3 +++
>> SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf | 3 +++
>> SecurityPkg/Library/AuthVariableLib/AuthService.c | 9 +++++++--
>> 13 files changed, 50 insertions(+), 2 deletions(-)
>>
>> --
>> 2.30.2
>
>
>
>
>
[-- Attachment #2: Type: text/html, Size: 4292 bytes --]
next prev parent reply other threads:[~2023-01-25 5:51 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-20 22:58 [PATCH v1 0/4] Don't require self-signed PK in setup mode Jan Bobek
2023-01-20 22:58 ` [PATCH v1 1/4] SecurityPkg: limit verification of enrolled " Jan Bobek
2023-01-20 22:58 ` [PATCH v1 2/4] OvmfPkg: require self-signed PK when secure boot is enabled Jan Bobek
2023-01-20 22:58 ` [PATCH v1 3/4] ArmVirtPkg: " Jan Bobek
2023-02-03 0:11 ` Yao, Jiewen
2023-02-03 10:49 ` Ard Biesheuvel
2023-02-03 11:14 ` Yao, Jiewen
2023-02-03 11:15 ` Ard Biesheuvel
2023-02-03 11:39 ` Gerd Hoffmann
2023-01-20 22:58 ` [PATCH v1 4/4] SecurityPkg: don't require PK to be self-signed by default Jan Bobek
2023-01-23 6:13 ` [PATCH v1 0/4] Don't require self-signed PK in setup mode Yao, Jiewen
2023-01-25 5:51 ` Sean [this message]
2023-01-25 21:38 ` [edk2-devel] " Jan Bobek
2023-01-27 21:28 ` Sean
2023-01-27 22:05 ` Jan Bobek
2023-01-28 2:37 ` Sean
2023-02-03 0:08 ` Yao, Jiewen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BY3PR19MB49006A3AAB1E04381A5FC2BDC8CE9@BY3PR19MB4900.namprd19.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox