On 04/30/19 21:42, Ard Biesheuvel wrote:
> On Tue, 30 Apr 2019 at 14:32, Laszlo Ersek <ler...@redhat.com> wrote:
>>
>> On 04/27/19 02:53, Laszlo Ersek wrote:
>>> Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747
>>> Repo:     https://github.com/lersek/edk2.git
>>> Branch:   enroll_bz_1747
>>>
>>> Please see the goal / use case in the BZ.
>>>
>>> Anatomy of the series:
>>>
>>> - Patch 01 adds the application as-is from RHEL, as the starting point
>>>   for upstreaming (preserves continuity).
>>>
>>> - Patches 02 through 13 clean up various coding style warts, and add
>>>   documentation, without functional changes.
>>>
>>> - Patches 14 through 16 replace the hard-coded Red Hat certificate
>>>   (enrolled as PK and 1st KEK) with a certificate read dynamically from
>>>   SMBIOS (enrolled the same way), originating from the VMM.
>>>
>>> I've successfully re-run the Secure Boot Logo Test in Windows HCK, after
>>> enabling SB in the VM-under-test with this application. I'll attach the
>>> test log in a separate email (sent in response to this one).
>>>
>>> Cc: Anthony Perard <anthony.per...@citrix.com>
>>> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
>>> Cc: Jordan Justen <jordan.l.jus...@intel.com>
>>> Cc: Julien Grall <julien.gr...@arm.com>
>>>
>>> Thanks,
>>> Laszlo
>>>
>>> Laszlo Ersek (16):
>>>   OvmfPkg: introduce EnrollDefaultKeys application
>>>   OvmfPkg/EnrollDefaultKeys: update @file comment blocks
>>>   OvmfPkg/EnrollDefaultKeys: refresh INF file
>>>   ArmVirtPkg: build EnrollDefaultKeys.efi
>>>   OvmfPkg/EnrollDefaultKeys: clean up minor whitespace wart
>>>   OvmfPkg/EnrollDefaultKeys: clean up global variable name prefixes
>>>   OvmfPkg/EnrollDefaultKeys: clean up acronym capitalization in
>>>     identifiers
>>>   OvmfPkg/EnrollDefaultKeys: remove unneeded EFIAPI call. conv.
>>>     specifiers
>>>   OvmfPkg/EnrollDefaultKeys: extract typedefs to a header file
>>>   OvmfPkg/EnrollDefaultKeys: split out certificate and signature
>>>     constants
>>>   OvmfPkg/EnrollDefaultKeys: extract MICROSOFT_VENDOR_GUID
>>>   OvmfPkg/EnrollDefaultKeys: describe functions with leading comment
>>>     blocks
>>>   OvmfPkg/EnrollDefaultKeys: document the steps of the entry point
>>>     function
>>>   OvmfPkg: introduce OVMF_PK_KEK1_APP_PREFIX_GUID
>>>   OvmfPkg/EnrollDefaultKeys: enroll PK/KEK1 from the Type 11 SMBIOS
>>>     table
>>>   OvmfPkg/EnrollDefaultKeys: remove Red Hat's hard-coded PK/KEK1
>>>
>>>  ArmVirtPkg/ArmVirt.dsc.inc                      |   1 +
>>>  ArmVirtPkg/ArmVirtQemu.dsc                      |   1 +
>>>  ArmVirtPkg/ArmVirtQemuKernel.dsc                |   1 +
>>>  OvmfPkg/EnrollDefaultKeys/AuthData.c            | 440 ++++++++++++
>>>  OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c   | 706 ++++++++++++++++++++
>>>  OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.h   | 138 ++++
>>>  OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf |  52 ++
>>>  OvmfPkg/Include/Guid/MicrosoftVendor.h          |  55 ++
>>>  OvmfPkg/Include/Guid/OvmfPkKek1AppPrefix.h      |  45 ++
>>>  OvmfPkg/OvmfPkg.dec                             |   2 +
>>>  OvmfPkg/OvmfPkgIa32.dsc                         |   2 +
>>>  OvmfPkg/OvmfPkgIa32X64.dsc                      |   2 +
>>>  OvmfPkg/OvmfPkgX64.dsc                          |   2 +
>>>  13 files changed, 1447 insertions(+)
>>>  create mode 100644 OvmfPkg/EnrollDefaultKeys/AuthData.c
>>>  create mode 100644 OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c
>>>  create mode 100644 OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.h
>>>  create mode 100644 OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf
>>>  create mode 100644 OvmfPkg/Include/Guid/MicrosoftVendor.h
>>>  create mode 100644 OvmfPkg/Include/Guid/OvmfPkKek1AppPrefix.h
>>>
>>
>> Thank you all for the quick reviews; the series has been pushed as
>> commit range 137cbff041fc..9fb2ce2f465d.
>>
> 
> Unfortunately, it seems we are hitting another potential false
> positive with GCC48:

yep, false positive :/

> OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c: In function ‘ShellAppMain’:
> OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c:631:10: error:
> ‘SizeOfPkKek1’ may be used uninitialized in this function
> [-Werror=maybe-uninitialized]
>    Status = EnrollListOfCerts (
>           ^
> OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c:703:12: error: ‘PkKek1’
> may be used uninitialized in this function
> [-Werror=maybe-uninitialized]
>    FreePool (PkKek1);
> 
> Given the history, I wouldn't mind disabling this warning for GCC48
> altogether (assuming it doesn't trigger on other compilers - my CI job
> tries GCC48 first IIRC)
> 

Suppressing this warning just for GCC48, without introducing partial
overlap between BaseTools and the following three INF files:
- CryptoPkg/Library/OpensslLib/OpensslLib.inf
- CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
- MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf

doesn't look like an attractive task.

On the flipside, we have a dedicated kludge at least in
"ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c".

I guess I'll try to send a patch soon, for "EnrollDefaultKeys.c". Sorry
about the breakage. :(

Thanks,
Laszlo

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#39875): https://edk2.groups.io/g/devel/message/39875
Mute This Topic: https://groups.io/mt/31359367/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to