Re: [edk2-devel] [PATCH 00/16] OvmfPkg, ArmVirtPkg: upstream the EnrollDefaultKeys app

["Ard Biesheuvel" <ard.biesheuvel@linaro.org>]

On Tue, 30 Apr 2019 at 14:32, Laszlo Ersek <lersek@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.perard@citrix.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Julien Grall <julien.grall@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:

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)