[PATCH 09/16] OvmfPkg/EnrollDefaultKeys: extract typedefs to a header file

["Laszlo Ersek" <lersek@redhat.com>]

"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.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/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=secalert@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