Hi Greg,

On 04/28/21 12:50, Grzegorz Bernacki wrote:

Hi,

Thanks a lot for reviewing my previous document. Please see below link to
updated design:

https://drive.google.com/file/d/11yNJJ2x8WXYZRg1nZWrr4C4Hq89Izn1D/view?usp=sharing

Changes:
- remove unnecessary description of default variables
- add macros to specify default keys files
- add fdf include file
- add UI to reset the content of the keys to default values

the master format for this document seems to be LaTeX (from the "pdf
properties" window in evince), which is fantastic: can you please post a
diff between the v1 and v2 LaTeX sources?

I can only deal with incremental reviews for v2 and later postings. Most
WYSIWYG document editors nowadays support one form of change tracking or
another. Working with a plaintext source format such as LaTeX is always
best, of course. My only request is then, "diffs please".

The rendered format is absolutely welcome in addition to the diffs, of
course.

Thanks
Laszlo

Please let me know if you have any questions/comments/concerns.
thanks,
greg

pon., 19 kwi 2021 o 11:07 Grzegorz Bernacki <gjb@semihalf.com>

napisał(a):

Hi Laszlo,

Thanks a lot for your review. I will wait some time for more comments
and I will send an updated design.
thanks,
greg

pt., 16 kwi 2021 o 14:16 Laszlo Ersek <lersek@redhat.com> napisał(a):

On 04/16/21 14:10, Laszlo Ersek wrote:

On 04/15/21 13:59, Grzegorz Bernacki wrote:

Hello,

I would like to ask you to review the proposal for new application

for

enrolling Secure Boot keys. The application uses content of

PKDefault,

KEKDefault, dbDefault, dbtDefault and dbxDefault to set Secure Boot
key. Default variables are initialized based on keys embedded in

flash

binary file. Please find details in following document:

https://drive.google.com/file/d/1rC6BjRWKODdXPdAa5cIeBG8_ye2L8aSZ/view?usp=sharing

- A new platform DXE driver (which need not even stay resident), for
locating some FFS files and their sections, and for copying their
contents into PKDefault and friends, seems like a good / useful idea

to me.

- Regarding the following sentence:

However there is no need to keep these variables as non-volatile in
order to avoid unnecessary bloating of the consumed non-volatile
storage space.

This is valid, but redundant (or at least somewhat redundantly
expressed), IMO. The UEFI spec already marks PKDefault as "BS, RT"
(section 3.3).

- The setting of PK from PKDefault (and similarly for the other
variables) is indeed a separate action. The UEFI spec says this is
primarily an OS action (again section 3.3), but I agree having a
dedicated UEFI app for this may be useful.

- Regarding the following sentence:

In case when default variables are already initialized a warning

message will be printed and variables will not be changed.

A warning is possible perhaps via status code reporting, or DEBUG. A
platform DXE driver is not supposed to write to the console.

- The idea as a whole looks good to me. I agree it should be testable
with OVMF, provided the FDF file(s) are modified as needed; however, I
wouldn't like to have that in the FDF file(s) permanently -- or at

least

it should be gated with a new build time feature test macro.

The

SECTION RAW = ...

statements in the FDF files could perhaps refer to macros as well (for
the pathnames). So a user building a platform with this feature

enabled

would have to pass a -D flag (boolean) for enabling the feature in
general (enabling the necessary lines in both the DSC and the FDF
file(s)), and then further -D flags for specifying the pathnames.

You could perhaps introduce DSC and FDF snippets (include files) too,
under SecurityPkg, so that any platform utilizing this feature do so
through identical macro names.

Thanks
Laszlo