OvmfPkg, ArmVirtPkg: upstream the EnrollDefaultKeys app

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

Posted by Laszlo Ersek 6 years, 9 months ago

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

-- 
2.19.1.3.g30247aa5d201


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

View/Reply Online (#39675): https://edk2.groups.io/g/devel/message/39675
Mute This Topic: https://groups.io/mt/31359367/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Laszlo Ersek 6 years, 9 months ago

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).

Done.

Thanks
Laszlo

> 
> 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
> 


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

View/Reply Online (#39692): https://edk2.groups.io/g/devel/message/39692
Mute This Topic: https://groups.io/mt/31359367/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Ard Biesheuvel 6 years, 9 months ago

On Sat, 27 Apr 2019 at 02:53, Laszlo Ersek <lersek@redhat.com> 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
>

I'm still quite unhappy about the way we are buying into using the MS
certs for everything even on ARM systems (where vendors care about
other certifications beyond the Windows Logo tests) but nonetheless, I
see that this has value for ARM as well as x86, so

Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

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

View/Reply Online (#39693): https://edk2.groups.io/g/devel/message/39693
Mute This Topic: https://groups.io/mt/31359367/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Laszlo Ersek 6 years, 9 months ago

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.

Cheers!
Laszlo

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

View/Reply Online (#39848): https://edk2.groups.io/g/devel/message/39848
Mute This Topic: https://groups.io/mt/31359367/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Ard Biesheuvel 6 years, 9 months ago

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)

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

View/Reply Online (#39874): https://edk2.groups.io/g/devel/message/39874
Mute This Topic: https://groups.io/mt/31359367/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Laszlo Ersek 6 years, 9 months ago

On 04/30/19 21:42, Ard Biesheuvel wrote:
> 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:

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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Gary Lin 6 years, 9 months ago

On Sat, Apr 27, 2019 at 02:53:12AM +0200, 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).
> 
For the series, Reviewed-by: Gary Lin <glin@suse.com>

> 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
> 
> -- 
> 2.19.1.3.g30247aa5d201
> 
> 
> 
> 
> 

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

View/Reply Online (#39830): https://edk2.groups.io/g/devel/message/39830
Mute This Topic: https://groups.io/mt/31359367/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-