Re: [edk2-devel] [PATCH 11/16] OvmfPkg/EnrollDefaultKeys: extract MICROSOFT_VENDOR_GUID

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 4/27/19 2:53 AM, Laszlo Ersek wrote:
> The GUID
> 
>   77FA9ABD-0359-4D32-BD60-28F4E78F784B
> 
> is specified in MSDN, at
> <https://msdn.microsoft.com/en-us/ie/dn932805(v=vs.94)>, therefore it
> deserves an entry in the package DEC file, and a header file under
> "Include/Guid".
> 
> (Arguably, this GUID declaration / definition could even live under
> SecurityPkg, but the edk2 tradition has been to hoist GUIDs,
> protocols/PPIs, and lib classes from OvmfPkg to a core package only when
> dependent C code is added to the core package.)
> 
> 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>
> Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  OvmfPkg/OvmfPkg.dec                             |  1 +
>  OvmfPkg/Include/Guid/MicrosoftVendor.h          | 55 ++++++++++++++++++++
>  OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf |  2 +
>  OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.h   |  2 -
>  OvmfPkg/EnrollDefaultKeys/AuthData.c            | 28 ----------
>  OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c   |  7 +--
>  6 files changed, 62 insertions(+), 33 deletions(-)
> 
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index cc2a4909afd4..922e061cc85c 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -72,16 +72,17 @@ [LibraryClasses]
>  [Guids]
>    gUefiOvmfPkgTokenSpaceGuid          = {0x93bb96af, 0xb9f2, 0x4eb8, {0x94, 0x62, 0xe0, 0xba, 0x74, 0x56, 0x42, 0x36}}
>    gEfiXenInfoGuid                     = {0xd3b46f3b, 0xd441, 0x1244, {0x9a, 0x12, 0x0, 0x12, 0x27, 0x3f, 0xc1, 0x4d}}
>    gOvmfPlatformConfigGuid             = {0x7235c51c, 0x0c80, 0x4cab, {0x87, 0xac, 0x3b, 0x08, 0x4a, 0x63, 0x04, 0xb1}}
>    gVirtioMmioTransportGuid            = {0x837dca9e, 0xe874, 0x4d82, {0xb2, 0x9a, 0x23, 0xfe, 0x0e, 0x23, 0xd1, 0xe2}}
>    gQemuRamfbGuid                      = {0x557423a1, 0x63ab, 0x406c, {0xbe, 0x7e, 0x91, 0xcd, 0xbc, 0x08, 0xc4, 0x57}}
>    gXenBusRootDeviceGuid               = {0xa732241f, 0x383d, 0x4d9c, {0x8a, 0xe1, 0x8e, 0x09, 0x83, 0x75, 0x89, 0xd7}}
>    gRootBridgesConnectedEventGroupGuid = {0x24a2d66f, 0xeedd, 0x4086, {0x90, 0x42, 0xf2, 0x6e, 0x47, 0x97, 0xee, 0x69}}
> +  gMicrosoftVendorGuid                = {0x77fa9abd, 0x0359, 0x4d32, {0xbd, 0x60, 0x28, 0xf4, 0xe7, 0x8f, 0x78, 0x4b}}
>  
>  [Protocols]
>    gVirtioDeviceProtocolGuid           = {0xfa920010, 0x6785, 0x4941, {0xb6, 0xec, 0x49, 0x8c, 0x57, 0x9f, 0x16, 0x0a}}
>    gXenBusProtocolGuid                 = {0x3d3ca290, 0xb9a5, 0x11e3, {0xb7, 0x5d, 0xb8, 0xac, 0x6f, 0x7d, 0x65, 0xe6}}
>    gXenIoProtocolGuid                  = {0x6efac84f, 0x0ab0, 0x4747, {0x81, 0xbe, 0x85, 0x55, 0x62, 0x59, 0x04, 0x49}}
>    gIoMmuAbsentProtocolGuid            = {0xf8775d50, 0x8abd, 0x4adf, {0x92, 0xac, 0x85, 0x3e, 0x51, 0xf6, 0xc8, 0xdc}}
>    gEfiLegacy8259ProtocolGuid          = {0x38321dba, 0x4fe0, 0x4e17, {0x8a, 0xec, 0x41, 0x30, 0x55, 0xea, 0xed, 0xc1}}
>  
> diff --git a/OvmfPkg/Include/Guid/MicrosoftVendor.h b/OvmfPkg/Include/Guid/MicrosoftVendor.h
> new file mode 100644
> index 000000000000..db7a326c3194
> --- /dev/null
> +++ b/OvmfPkg/Include/Guid/MicrosoftVendor.h
> @@ -0,0 +1,55 @@
> +/** @file
> +  Declare the GUID that is expected:
> +
> +  - as EFI_SIGNATURE_DATA.SignatureOwner GUID in association with X509 and
> +    RSA2048 Secure Boot certificates issued by/for Microsoft,
> +
> +  - as UEFI variable vendor GUID in association with (unspecified)
> +    Microsoft-owned variables.
> +
> +  Copyright (C) 2014-2019, Red Hat, Inc.
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +  @par Specification Reference:
> +  - MSDN: System.Fundamentals.Firmware at
> +    <https://msdn.microsoft.com/en-us/ie/dn932805(v=vs.94)>.
> +**/
> +
> +#ifndef MICROSOFT_VENDOR_H_
> +#define MICROSOFT_VENDOR_H_
> +
> +#include <Uefi/UefiBaseType.h>
> +
> +//
> +// The following test cases of the Secure Boot Logo Test in the Microsoft
> +// Hardware Certification Kit:
> +//
> +// - Microsoft.UefiSecureBootLogo.Tests.OutOfBoxVerifyMicrosoftKEKpresent
> +// - Microsoft.UefiSecureBootLogo.Tests.OutOfBoxConfirmMicrosoftSignatureInDB
> +//
> +// expect the EFI_SIGNATURE_DATA.SignatureOwner GUID to be
> +// 77FA9ABD-0359-4D32-BD60-28F4E78F784B, when the
> +// EFI_SIGNATURE_DATA.SignatureData field carries any of the following X509
> +// certificates:
> +//
> +// - "Microsoft Corporation KEK CA 2011" (in KEK)
> +// - "Microsoft Windows Production PCA 2011" (in db)
> +// - "Microsoft Corporation UEFI CA 2011" (in db)
> +//
> +// This is despite the fact that the UEFI specification requires
> +// EFI_SIGNATURE_DATA.SignatureOwner to reflect the agent (i.e., OS,
> +// application or driver) that enrolled and therefore owns
> +// EFI_SIGNATURE_DATA.SignatureData, and not the organization that issued
> +// EFI_SIGNATURE_DATA.SignatureData.
> +//
> +#define MICROSOFT_VENDOR_GUID                           \
> +  { 0x77fa9abd,                                         \
> +    0x0359,                                             \
> +    0x4d32,                                             \
> +    { 0xbd, 0x60, 0x28, 0xf4, 0xe7, 0x8f, 0x78, 0x4b }, \
> +  }
> +
> +extern EFI_GUID gMicrosoftVendorGuid;
> +
> +#endif /* MICROSOFT_VENDOR_H_ */
> diff --git a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf
> index 3f093c768585..28db52586a9b 100644
> --- a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf
> +++ b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf
> @@ -17,27 +17,29 @@ [Defines]
>  [Sources]
>    AuthData.c
>    EnrollDefaultKeys.c
>    EnrollDefaultKeys.h
>  
>  [Packages]
>    MdeModulePkg/MdeModulePkg.dec
>    MdePkg/MdePkg.dec
> +  OvmfPkg/OvmfPkg.dec
>    SecurityPkg/SecurityPkg.dec
>    ShellPkg/ShellPkg.dec
>  
>  [Guids]
>    gEfiCertPkcs7Guid
>    gEfiCertSha256Guid
>    gEfiCertX509Guid
>    gEfiCustomModeEnableGuid
>    gEfiGlobalVariableGuid
>    gEfiImageSecurityDatabaseGuid
>    gEfiSecureBootEnableDisableGuid
> +  gMicrosoftVendorGuid
>  
>  [LibraryClasses]
>    BaseMemoryLib
>    DebugLib
>    MemoryAllocationLib
>    ShellCEntryLib
>    UefiLib
>    UefiRuntimeServicesTableLib
> diff --git a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.h b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.h
> index 07f4aa04e469..e3a7e43da4e3 100644
> --- a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.h
> +++ b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.h
> @@ -133,11 +133,9 @@ extern CONST UINT8 mMicrosoftPca[];
>  extern CONST UINTN mSizeOfMicrosoftPca;
>  
>  extern CONST UINT8 mMicrosoftUefiCa[];
>  extern CONST UINTN mSizeOfMicrosoftUefiCa;
>  
>  extern CONST UINT8 mSha256OfDevNull[];
>  extern CONST UINTN mSizeOfSha256OfDevNull;
>  
> -extern CONST EFI_GUID mMicrosoftOwnerGuid;
> -
>  #endif /* ENROLL_DEFAULT_KEYS_H_ */
> diff --git a/OvmfPkg/EnrollDefaultKeys/AuthData.c b/OvmfPkg/EnrollDefaultKeys/AuthData.c
> index e0a543785fb5..9a96dcc440b3 100644
> --- a/OvmfPkg/EnrollDefaultKeys/AuthData.c
> +++ b/OvmfPkg/EnrollDefaultKeys/AuthData.c
> @@ -518,36 +518,8 @@ CONST UINTN mSizeOfMicrosoftUefiCa = sizeof mMicrosoftUefiCa;
>  //
>  CONST UINT8 mSha256OfDevNull[] = {
>    0xe3, 0xb0, 0xc4, 0x42, 0x98, 0xfc, 0x1c, 0x14, 0x9a, 0xfb, 0xf4, 0xc8, 0x99,
>    0x6f, 0xb9, 0x24, 0x27, 0xae, 0x41, 0xe4, 0x64, 0x9b, 0x93, 0x4c, 0xa4, 0x95,
>    0x99, 0x1b, 0x78, 0x52, 0xb8, 0x55
>  };
>  
>  CONST UINTN mSizeOfSha256OfDevNull = sizeof mSha256OfDevNull;
> -
> -
> -//
> -// The following test cases of the Secure Boot Logo Test in the Microsoft
> -// Hardware Certification Kit:
> -//
> -// - Microsoft.UefiSecureBootLogo.Tests.OutOfBoxVerifyMicrosoftKEKpresent
> -// - Microsoft.UefiSecureBootLogo.Tests.OutOfBoxConfirmMicrosoftSignatureInDB
> -//
> -// expect the EFI_SIGNATURE_DATA.SignatureOwner GUID to be
> -// 77FA9ABD-0359-4D32-BD60-28F4E78F784B, when the
> -// EFI_SIGNATURE_DATA.SignatureData field carries any of the following X509
> -// certificates:
> -//
> -// - "Microsoft Corporation KEK CA 2011" (in KEK)
> -// - "Microsoft Windows Production PCA 2011" (in db)
> -// - "Microsoft Corporation UEFI CA 2011" (in db)
> -//
> -// This is despite the fact that the UEFI specification requires
> -// EFI_SIGNATURE_DATA.SignatureOwner to reflect the agent (i.e., OS,
> -// application or driver) that enrolled and therefore owns
> -// EFI_SIGNATURE_DATA.SignatureData, and not the organization that issued
> -// EFI_SIGNATURE_DATA.SignatureData.
> -//
> -CONST EFI_GUID mMicrosoftOwnerGuid = {
> -  0x77fa9abd, 0x0359, 0x4d32,
> -  { 0xbd, 0x60, 0x28, 0xf4, 0xe7, 0x8f, 0x78, 0x4b },
> -};
> diff --git a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c
> index 528718b15ae9..e4f6a50e008b 100644
> --- a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c
> +++ b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c
> @@ -3,16 +3,17 @@
>  
>    Copyright (C) 2014-2019, Red Hat, Inc.
>  
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>  **/
>  #include <Guid/AuthenticatedVariableFormat.h>    // gEfiCustomModeEnableGuid
>  #include <Guid/GlobalVariable.h>                 // EFI_SETUP_MODE_NAME
>  #include <Guid/ImageAuthentication.h>            // EFI_IMAGE_SECURITY_DATABASE
> +#include <Guid/MicrosoftVendor.h>                // gMicrosoftVendorGuid
>  #include <Library/BaseMemoryLib.h>               // CopyGuid()
>  #include <Library/DebugLib.h>                    // ASSERT()
>  #include <Library/MemoryAllocationLib.h>         // FreePool()
>  #include <Library/ShellCEntryLib.h>              // ShellAppMain()
>  #include <Library/UefiLib.h>                     // AsciiPrint()
>  #include <Library/UefiRuntimeServicesTableLib.h> // gRT
>  
>  #include "EnrollDefaultKeys.h"
> @@ -310,18 +311,18 @@ ShellAppMain (
>        return 1;
>      }
>    }
>  
>    Status = EnrollListOfCerts (
>               EFI_IMAGE_SECURITY_DATABASE,
>               &gEfiImageSecurityDatabaseGuid,
>               &gEfiCertX509Guid,
> -             mMicrosoftPca,    mSizeOfMicrosoftPca,    &mMicrosoftOwnerGuid,
> -             mMicrosoftUefiCa, mSizeOfMicrosoftUefiCa, &mMicrosoftOwnerGuid,
> +             mMicrosoftPca,    mSizeOfMicrosoftPca,    &gMicrosoftVendorGuid,
> +             mMicrosoftUefiCa, mSizeOfMicrosoftUefiCa, &gMicrosoftVendorGuid,
>               NULL);
>    if (EFI_ERROR (Status)) {
>      return 1;
>    }
>  
>    Status = EnrollListOfCerts (
>               EFI_IMAGE_SECURITY_DATABASE1,
>               &gEfiImageSecurityDatabaseGuid,
> @@ -332,17 +333,17 @@ ShellAppMain (
>      return 1;
>    }
>  
>    Status = EnrollListOfCerts (
>               EFI_KEY_EXCHANGE_KEY_NAME,
>               &gEfiGlobalVariableGuid,
>               &gEfiCertX509Guid,
>               mRedHatPkKek1, mSizeOfRedHatPkKek1, &gEfiCallerIdGuid,
> -             mMicrosoftKek, mSizeOfMicrosoftKek, &mMicrosoftOwnerGuid,
> +             mMicrosoftKek, mSizeOfMicrosoftKek, &gMicrosoftVendorGuid,
>               NULL);
>    if (EFI_ERROR (Status)) {
>      return 1;
>    }
>  
>    Status = EnrollListOfCerts (
>               EFI_PLATFORM_KEY_NAME,
>               &gEfiGlobalVariableGuid,
> 

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>