On 4/27/19 2:53 AM, Laszlo Ersek wrote: > "EnrollDefaultKeys.c" defines three structure types: SINGLE_HEADER, > REPEATING_HEADER, and SETTINGS. The definitions are scattered over the C > file, and lack high-level summary comments. > > Extract the structures to "EnrollDefaultKeys.h", and add the missing > comments. > > 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> > Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747 > Signed-off-by: Laszlo Ersek <ler...@redhat.com> > --- > OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf | 1 + > OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.h | 121 ++++++++++++++++++++ > OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c | 101 +--------------- > 3 files changed, 124 insertions(+), 99 deletions(-) > > diff --git a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf > b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf > index 3a215df50863..9f315a8e6d90 100644 > --- a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf > +++ b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf > @@ -11,16 +11,17 @@ [Defines] > BASE_NAME = EnrollDefaultKeys > FILE_GUID = A0BAA8A3-041D-48A8-BC87-C36D121B5E3D > MODULE_TYPE = UEFI_APPLICATION > VERSION_STRING = 0.1 > ENTRY_POINT = ShellCEntryLib > > [Sources] > EnrollDefaultKeys.c > + EnrollDefaultKeys.h > > [Packages] > MdeModulePkg/MdeModulePkg.dec > MdePkg/MdePkg.dec > SecurityPkg/SecurityPkg.dec > ShellPkg/ShellPkg.dec > > [Guids] > diff --git a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.h > b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.h > new file mode 100644 > index 000000000000..9bcd87ff4f44 > --- /dev/null > +++ b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.h > @@ -0,0 +1,121 @@ > +/** @file > + Type definitions for the EnrollDefaultKeys application. > + > + Copyright (C) 2014-2019, Red Hat, Inc. > + > + SPDX-License-Identifier: BSD-2-Clause-Patent > +**/ > + > +#ifndef ENROLL_DEFAULT_KEYS_H_ > +#define ENROLL_DEFAULT_KEYS_H_ > + > +#include <Uefi/UefiBaseType.h> > + > +// > +// Convenience structure types for constructing "signature lists" for > +// authenticated UEFI variables. > +// > +// The most important thing about the variable payload is that it is a list > of > +// lists, where the element size of any given *inner* list is constant. > +// > +// Since X509 certificates vary in size, each of our *inner* lists will > contain > +// one element only (one X.509 certificate). This is explicitly mentioned in > +// the UEFI specification, in "28.4.1 Signature Database", in a Note. > +// > +// The list structure looks as follows: > +// > +// struct EFI_VARIABLE_AUTHENTICATION_2 { | > +// struct EFI_TIME { | > +// UINT16 Year; | > +// UINT8 Month; | > +// UINT8 Day; | > +// UINT8 Hour; | > +// UINT8 Minute; | > +// UINT8 Second; | > +// UINT8 Pad1; | > +// UINT32 Nanosecond; | > +// INT16 TimeZone; | > +// UINT8 Daylight; | > +// UINT8 Pad2; | > +// } TimeStamp; | > +// | > +// struct WIN_CERTIFICATE_UEFI_GUID { | | > +// struct WIN_CERTIFICATE { | | > +// UINT32 dwLength; ----------------------------------------+ | > +// UINT16 wRevision; | | > +// UINT16 wCertificateType; | | > +// } Hdr; | +- > DataSize > +// | | > +// EFI_GUID CertType; | | > +// UINT8 CertData[1] = { <--- "struct hack" | | > +// struct EFI_SIGNATURE_LIST { | | | > +// EFI_GUID SignatureType; | | | > +// UINT32 SignatureListSize; -------------------------+ | | > +// UINT32 SignatureHeaderSize; | | | > +// UINT32 SignatureSize; ---------------------------+ | | | > +// UINT8 SignatureHeader[SignatureHeaderSize]; | | | | > +// v | | | > +// struct EFI_SIGNATURE_DATA { | | | | > +// EFI_GUID SignatureOwner; | | | | > +// UINT8 SignatureData[1] = { <--- "struct hack" | | | | > +// X.509 payload | | | | > +// } | | | | > +// } Signatures[]; | | | > +// } SigLists[]; | | > +// }; | | > +// } AuthInfo; | | > +// }; | > +// > +// Given that the "struct hack" invokes undefined behavior (which is why C99 > +// introduced the flexible array member), and because subtracting those pesky > +// sizes of 1 is annoying, and because the format is fully specified in the > +// UEFI specification, we'll introduce two matching convenience structures > that > +// are customized for our X.509 purposes. > +// > +#pragma pack (1) > +typedef struct { > + EFI_TIME TimeStamp; > + > + // > + // dwLength covers data below > + // > + UINT32 dwLength; > + UINT16 wRevision; > + UINT16 wCertificateType; > + EFI_GUID CertType; > +} SINGLE_HEADER; > + > +typedef struct { > + // > + // SignatureListSize covers data below > + // > + EFI_GUID SignatureType; > + UINT32 SignatureListSize; > + UINT32 SignatureHeaderSize; // constant 0 > + UINT32 SignatureSize; > + > + // > + // SignatureSize covers data below > + // > + EFI_GUID SignatureOwner; > + > + // > + // X.509 certificate follows > + // > +} REPEATING_HEADER; > +#pragma pack () > + > + > +// > +// A structure that collects the values of UEFI variables related to Secure > +// Boot. > +// > +typedef struct { > + UINT8 SetupMode; > + UINT8 SecureBoot; > + UINT8 SecureBootEnable; > + UINT8 CustomMode; > + UINT8 VendorKeys; > +} SETTINGS; > + > +#endif /* ENROLL_DEFAULT_KEYS_H_ */ > diff --git a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c > b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c > index 671efef8d6ad..fefea6638887 100644 > --- a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c > +++ b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c > @@ -10,16 +10,18 @@ > #include <Guid/ImageAuthentication.h> // > EFI_IMAGE_SECURITY_DATABASE > #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" > + > // > // We'll use the certificate below as both Platform Key and as first Key > // Exchange Key. > // > // "Red Hat Secure Boot (PK/KEK key 1)/emailAddress=secal...@redhat.com" > // SHA1: fd:fc:7f:3c:7e:f3:e0:57:76:ad:d7:98:78:21:6c:9b:e0:e1:95:97 > // > STATIC CONST UINT8 mRedHatPkKek1[] = { > @@ -538,107 +540,16 @@ STATIC CONST UINT8 mSha256OfDevNull[] = { > // EFI_SIGNATURE_DATA.SignatureData, and not the organization that issued > // EFI_SIGNATURE_DATA.SignatureData. > // > STATIC CONST EFI_GUID mMicrosoftOwnerGuid = { > 0x77fa9abd, 0x0359, 0x4d32, > { 0xbd, 0x60, 0x28, 0xf4, 0xe7, 0x8f, 0x78, 0x4b }, > }; > > -// > -// The most important thing about the variable payload is that it is a list > of > -// lists, where the element size of any given *inner* list is constant. > -// > -// Since X509 certificates vary in size, each of our *inner* lists will > contain > -// one element only (one X.509 certificate). This is explicitly mentioned in > -// the UEFI specification, in "28.4.1 Signature Database", in a Note. > -// > -// The list structure looks as follows: > -// > -// struct EFI_VARIABLE_AUTHENTICATION_2 { | > -// struct EFI_TIME { | > -// UINT16 Year; | > -// UINT8 Month; | > -// UINT8 Day; | > -// UINT8 Hour; | > -// UINT8 Minute; | > -// UINT8 Second; | > -// UINT8 Pad1; | > -// UINT32 Nanosecond; | > -// INT16 TimeZone; | > -// UINT8 Daylight; | > -// UINT8 Pad2; | > -// } TimeStamp; | > -// | > -// struct WIN_CERTIFICATE_UEFI_GUID { | | > -// struct WIN_CERTIFICATE { | | > -// UINT32 dwLength; ----------------------------------------+ | > -// UINT16 wRevision; | | > -// UINT16 wCertificateType; | | > -// } Hdr; | +- > DataSize > -// | | > -// EFI_GUID CertType; | | > -// UINT8 CertData[1] = { <--- "struct hack" | | > -// struct EFI_SIGNATURE_LIST { | | | > -// EFI_GUID SignatureType; | | | > -// UINT32 SignatureListSize; -------------------------+ | | > -// UINT32 SignatureHeaderSize; | | | > -// UINT32 SignatureSize; ---------------------------+ | | | > -// UINT8 SignatureHeader[SignatureHeaderSize]; | | | | > -// v | | | > -// struct EFI_SIGNATURE_DATA { | | | | > -// EFI_GUID SignatureOwner; | | | | > -// UINT8 SignatureData[1] = { <--- "struct hack" | | | | > -// X.509 payload | | | | > -// } | | | | > -// } Signatures[]; | | | > -// } SigLists[]; | | > -// }; | | > -// } AuthInfo; | | > -// }; | > -// > -// Given that the "struct hack" invokes undefined behavior (which is why C99 > -// introduced the flexible array member), and because subtracting those pesky > -// sizes of 1 is annoying, and because the format is fully specified in the > -// UEFI specification, we'll introduce two matching convenience structures > that > -// are customized for our X.509 purposes. > -// > -#pragma pack (1) > -typedef struct { > - EFI_TIME TimeStamp; > - > - // > - // dwLength covers data below > - // > - UINT32 dwLength; > - UINT16 wRevision; > - UINT16 wCertificateType; > - EFI_GUID CertType; > -} SINGLE_HEADER; > - > -typedef struct { > - // > - // SignatureListSize covers data below > - // > - EFI_GUID SignatureType; > - UINT32 SignatureListSize; > - UINT32 SignatureHeaderSize; // constant 0 > - UINT32 SignatureSize; > - > - // > - // SignatureSize covers data below > - // > - EFI_GUID SignatureOwner; > - > - // > - // X.509 certificate follows > - // > -} REPEATING_HEADER; > -#pragma pack () > - > /** > Enroll a set of certificates in a global variable, overwriting it. > > The variable will be rewritten with NV+BS+RT+AT attributes. > > @param[in] VariableName The name of the variable to overwrite. > > @param[in] VendorGuid The namespace (ie. vendor GUID) of the variable to > @@ -839,24 +750,16 @@ GetExact ( > AsciiPrint ("error: GetVariable(\"%s\", %g): expected size 0x%Lx, " > "got 0x%Lx\n", VariableName, VendorGuid, (UINT64)DataSize, > (UINT64)Size); > return EFI_PROTOCOL_ERROR; > } > > return EFI_SUCCESS; > } > > -typedef struct { > - UINT8 SetupMode; > - UINT8 SecureBoot; > - UINT8 SecureBootEnable; > - UINT8 CustomMode; > - UINT8 VendorKeys; > -} SETTINGS; > - > STATIC > EFI_STATUS > GetSettings ( > OUT SETTINGS *Settings > ) > { > EFI_STATUS Status; > >
Reviewed-by: Philippe Mathieu-Daude <phi...@redhat.com> -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#39767): https://edk2.groups.io/g/devel/message/39767 Mute This Topic: https://groups.io/mt/31359379/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
