"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; -- 2.19.1.3.g30247aa5d201 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#39684): https://edk2.groups.io/g/devel/message/39684 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] -=-=-=-=-=-=-=-=-=-=-=-
