Re: [edk2-devel] [PATCH 15/16] OvmfPkg/EnrollDefaultKeys: enroll PK/KEK1 from the Type 11 SMBIOS table

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

On 4/27/19 2:53 AM, Laszlo Ersek wrote:
> Disconnect the certificate that is enrolled as both Platform Key and first
> Key Exchange Key from Red Hat: expect the hypervisor to specify it, as
> part of SMBIOS.
> 
> Example usage with QEMU:
> 
> * Generate self-signed X509 certificate:
> 
>   openssl req \
>     -x509 \
>     -newkey rsa:2048 \
>     -outform PEM \
>     -keyout PkKek1.private.key \
>     -out PkKek1.pem
> 
>   (where "PEM" simply means "DER + base64 + header + footer").
> 
> * Strip the header, footer, and newline characters; prepend the
>   application prefix:
> 
>   sed \
>     -e 's/^-----BEGIN CERTIFICATE-----$/4e32566d-8e9e-4f52-81d3-5bb9715f9727:/' \
>     -e '/^-----END CERTIFICATE-----$/d' \
>     PkKek1.pem \
>   | tr -d '\n' \
>   > PkKek1.oemstr
> 
> * Pass the certificate to EnrollDefaultKeys with the following QEMU
>   option:
> 
>   -smbios type=11,value="$(< PkKek1.oemstr)"
> 
>   (Note: for the above option to work correctly, a QEMU version is needed
>   that includes commit 950c4e6c94b1 ("opts: don't silently truncate long
>   option values", 2018-05-09). The first upstream release with that commit
>   was v3.0.0.
> 
>   Once <https://bugs.launchpad.net/qemu/+bug/1826200> is fixed, QEMU will
>   learn to read the file directly; passing the blob on the command will be
>   necessary no more.)
> 
> 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 |   7 +
>  OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c   | 223 ++++++++++++++++++--
>  2 files changed, 217 insertions(+), 13 deletions(-)
> 
> diff --git a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf
> index 28db52586a9b..184f7972d52d 100644
> --- a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf
> +++ b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf
> @@ -30,16 +30,23 @@ [Guids]
>    gEfiCertPkcs7Guid
>    gEfiCertSha256Guid
>    gEfiCertX509Guid
>    gEfiCustomModeEnableGuid
>    gEfiGlobalVariableGuid
>    gEfiImageSecurityDatabaseGuid
>    gEfiSecureBootEnableDisableGuid
>    gMicrosoftVendorGuid
> +  gOvmfPkKek1AppPrefixGuid
> +
> +[Protocols]
> +  gEfiSmbiosProtocolGuid ## CONSUMES
>  
>  [LibraryClasses]
> +  BaseLib
>    BaseMemoryLib
>    DebugLib
>    MemoryAllocationLib
> +  PrintLib
>    ShellCEntryLib
> +  UefiBootServicesTableLib
>    UefiLib
>    UefiRuntimeServicesTableLib
> diff --git a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c
> index 9c4a0f06fb4d..b7b2e424c59e 100644
> --- a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c
> +++ b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c
> @@ -4,26 +4,201 @@
>    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 <Guid/OvmfPkKek1AppPrefix.h>            // gOvmfPkKek1AppPrefixGuid
> +#include <IndustryStandard/SmBios.h>             // SMBIOS_HANDLE_PI_RESERVED
> +#include <Library/BaseLib.h>                     // GUID_STRING_LENGTH
>  #include <Library/BaseMemoryLib.h>               // CopyGuid()
>  #include <Library/DebugLib.h>                    // ASSERT()
>  #include <Library/MemoryAllocationLib.h>         // FreePool()
> +#include <Library/PrintLib.h>                    // AsciiSPrint()
>  #include <Library/ShellCEntryLib.h>              // ShellAppMain()
> +#include <Library/UefiBootServicesTableLib.h>    // gBS
>  #include <Library/UefiLib.h>                     // AsciiPrint()
>  #include <Library/UefiRuntimeServicesTableLib.h> // gRT
> +#include <Protocol/Smbios.h>                     // EFI_SMBIOS_PROTOCOL
>  
>  #include "EnrollDefaultKeys.h"
>  
>  
> +/**
> +  Fetch the X509 certificate (to be used as Platform Key and first Key Exchange
> +  Key) from SMBIOS.
> +
> +  @param[out] PkKek1        The X509 certificate in DER encoding from the
> +                            hypervisor, to be enrolled as PK and first KEK
> +                            entry. On success, the caller is responsible for
> +                            releasing PkKek1 with FreePool().
> +
> +  @param[out] SizeOfPkKek1  The size of PkKek1 in bytes.
> +
> +  @retval EFI_SUCCESS           PkKek1 and SizeOfPkKek1 have been set
> +                                successfully.
> +
> +  @retval EFI_NOT_FOUND         An OEM String matching
> +                                OVMF_PK_KEK1_APP_PREFIX_GUID has not been
> +                                found.
> +
> +  @retval EFI_PROTOCOL_ERROR    In the OEM String matching
> +                                OVMF_PK_KEK1_APP_PREFIX_GUID, the certificate
> +                                is empty, or it has invalid base64 encoding.
> +
> +  @retval EFI_OUT_OF_RESOURCES  Memory allocation failed.
> +
> +  @return                       Error codes from gBS->LocateProtocol().
> +**/
> +STATIC
> +EFI_STATUS
> +GetPkKek1 (
> +  OUT UINT8 **PkKek1,
> +  OUT UINTN *SizeOfPkKek1
> +  )
> +{
> +  CONST CHAR8             *Base64Cert;
> +  CHAR8                   OvmfPkKek1AppPrefix[GUID_STRING_LENGTH + 1 + 1];
> +  EFI_STATUS              Status;
> +  EFI_SMBIOS_PROTOCOL     *Smbios;
> +  EFI_SMBIOS_HANDLE       Handle;
> +  EFI_SMBIOS_TYPE         Type;
> +  EFI_SMBIOS_TABLE_HEADER *Header;
> +  SMBIOS_TABLE_TYPE11     *OemStringsTable;
> +  UINTN                   Base64CertLen;
> +  UINTN                   DecodedCertSize;
> +  UINT8                   *DecodedCert;
> +
> +  Base64Cert = NULL;
> +
> +  //
> +  // Format the application prefix, for OEM String matching.
> +  //
> +  AsciiSPrint (OvmfPkKek1AppPrefix, sizeof OvmfPkKek1AppPrefix, "%g:",
> +    &gOvmfPkKek1AppPrefixGuid);
> +
> +  //
> +  // Scan all "OEM Strings" tables.
> +  //
> +  Status = gBS->LocateProtocol (&gEfiSmbiosProtocolGuid, NULL,
> +                  (VOID **)&Smbios);
> +  if (EFI_ERROR (Status)) {
> +    AsciiPrint ("error: failed to locate EFI_SMBIOS_PROTOCOL: %r\n", Status);
> +    return Status;
> +  }
> +
> +  Handle = SMBIOS_HANDLE_PI_RESERVED;
> +  Type = SMBIOS_TYPE_OEM_STRINGS;
> +  for (Status = Smbios->GetNext (Smbios, &Handle, &Type, &Header, NULL);
> +       !EFI_ERROR (Status);
> +       Status = Smbios->GetNext (Smbios, &Handle, &Type, &Header, NULL)) {
> +    CONST CHAR8 *OemString;
> +    UINTN       Idx;
> +
> +    if (Header->Length < sizeof *OemStringsTable) {
> +      //
> +      // Malformed table header, skip to next.
> +      //
> +      continue;
> +    }
> +    OemStringsTable = (SMBIOS_TABLE_TYPE11 *)Header;
> +
> +    //
> +    // Scan all strings in the unformatted area of the current "OEM Strings"
> +    // table.
> +    //
> +    OemString = (CONST CHAR8 *)(OemStringsTable + 1);
> +    for (Idx = 0; Idx < OemStringsTable->StringCount; ++Idx) {
> +      CHAR8 CandidatePrefix[sizeof OvmfPkKek1AppPrefix];
> +
> +      //
> +      // NUL-terminate the candidate prefix for case-insensitive comparison.
> +      //
> +      AsciiStrnCpyS (CandidatePrefix, sizeof CandidatePrefix, OemString,
> +        GUID_STRING_LENGTH + 1);
> +      if (AsciiStriCmp (OvmfPkKek1AppPrefix, CandidatePrefix) == 0) {
> +        //
> +        // The current string matches the prefix.
> +        //
> +        Base64Cert = OemString + GUID_STRING_LENGTH + 1;
> +        break;
> +      }
> +      OemString += AsciiStrSize (OemString);
> +    }
> +
> +    if (Idx < OemStringsTable->StringCount) {
> +      //
> +      // The current table has a matching string.
> +      //
> +      break;
> +    }
> +  }
> +
> +  if (EFI_ERROR (Status)) {
> +    //
> +    // No table with a matching string has been found.
> +    //
> +    AsciiPrint ("error: OEM String with app prefix %g not found: %r\n",
> +      &gOvmfPkKek1AppPrefixGuid, Status);
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  ASSERT (Base64Cert != NULL);
> +  Base64CertLen = AsciiStrLen (Base64Cert);
> +
> +  //
> +  // Verify the base64 encoding, and determine the decoded size.
> +  //
> +  DecodedCertSize = 0;
> +  Status = Base64Decode (Base64Cert, Base64CertLen, NULL, &DecodedCertSize);
> +  switch (Status) {
> +  case EFI_BUFFER_TOO_SMALL:
> +    if (DecodedCertSize > 0) {
> +      break;
> +    }
> +    //
> +    // Fall through: the above Base64Decode() call is ill-specified in BaseLib
> +    // if Source decodes to zero bytes (for example if it consists of ignored
> +    // whitespace only).

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

> +    //
> +  case EFI_SUCCESS:
> +    AsciiPrint ("error: empty certificate after app prefix %g\n",
> +      &gOvmfPkKek1AppPrefixGuid);
> +    return EFI_PROTOCOL_ERROR;
> +  default:
> +    AsciiPrint ("error: invalid base64 string after app prefix %g\n",
> +      &gOvmfPkKek1AppPrefixGuid);
> +    return EFI_PROTOCOL_ERROR;
> +  }
> +
> +  //
> +  // Allocate the output buffer.
> +  //
> +  DecodedCert = AllocatePool (DecodedCertSize);
> +  if (DecodedCert == NULL) {
> +    AsciiPrint ("error: failed to allocate memory\n");
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  //
> +  // Decoding will succeed at this point.
> +  //
> +  Status = Base64Decode (Base64Cert, Base64CertLen, DecodedCert,
> +             &DecodedCertSize);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  *PkKek1 = DecodedCert;
> +  *SizeOfPkKek1 = DecodedCertSize;
> +  return EFI_SUCCESS;
> +}
> +
> +
>  /**
>    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
> @@ -353,116 +528,133 @@ PrintSettings (
>  **/
>  INTN
>  EFIAPI
>  ShellAppMain (
>    IN UINTN  Argc,
>    IN CHAR16 **Argv
>    )
>  {
> +  INTN       RetVal;
>    EFI_STATUS Status;
>    SETTINGS   Settings;
> +  UINT8      *PkKek1;
> +  UINTN      SizeOfPkKek1;
> +
> +  //
> +  // Prepare for failure.
> +  //
> +  RetVal = 1;
>  
>    //
>    // If we're not in Setup Mode, we can't do anything.
>    //
>    Status = GetSettings (&Settings);
>    if (EFI_ERROR (Status)) {
> -    return 1;
> +    return RetVal;
>    }
>    PrintSettings (&Settings);
>  
>    if (Settings.SetupMode != 1) {
>      AsciiPrint ("error: already in User Mode\n");
> -    return 1;
> +    return RetVal;
> +  }
> +
> +  //
> +  // Fetch the X509 certificate (to be used as Platform Key and first Key
> +  // Exchange Key) from SMBIOS.
> +  //
> +  Status = GetPkKek1 (&PkKek1, &SizeOfPkKek1);
> +  if (EFI_ERROR (Status)) {
> +    return RetVal;
>    }
>  
>    //
>    // Enter Custom Mode so we can enroll PK, KEK, db, and dbx without signature
>    // checks on those variable writes.
>    //
>    if (Settings.CustomMode != CUSTOM_SECURE_BOOT_MODE) {
>      Settings.CustomMode = CUSTOM_SECURE_BOOT_MODE;
>      Status = gRT->SetVariable (EFI_CUSTOM_MODE_NAME, &gEfiCustomModeEnableGuid,
>                      (EFI_VARIABLE_NON_VOLATILE |
>                       EFI_VARIABLE_BOOTSERVICE_ACCESS),
>                      sizeof Settings.CustomMode, &Settings.CustomMode);
>      if (EFI_ERROR (Status)) {
>        AsciiPrint ("error: SetVariable(\"%s\", %g): %r\n", EFI_CUSTOM_MODE_NAME,
>          &gEfiCustomModeEnableGuid, Status);
> -      return 1;
> +      goto FreePkKek1;
>      }
>    }
>  
>    //
>    // Enroll db.
>    //
>    Status = EnrollListOfCerts (
>               EFI_IMAGE_SECURITY_DATABASE,
>               &gEfiImageSecurityDatabaseGuid,
>               &gEfiCertX509Guid,
>               mMicrosoftPca,    mSizeOfMicrosoftPca,    &gMicrosoftVendorGuid,
>               mMicrosoftUefiCa, mSizeOfMicrosoftUefiCa, &gMicrosoftVendorGuid,
>               NULL);
>    if (EFI_ERROR (Status)) {
> -    return 1;
> +    goto FreePkKek1;
>    }
>  
>    //
>    // Enroll dbx.
>    //
>    Status = EnrollListOfCerts (
>               EFI_IMAGE_SECURITY_DATABASE1,
>               &gEfiImageSecurityDatabaseGuid,
>               &gEfiCertSha256Guid,
>               mSha256OfDevNull, mSizeOfSha256OfDevNull, &gEfiCallerIdGuid,
>               NULL);
>    if (EFI_ERROR (Status)) {
> -    return 1;
> +    goto FreePkKek1;
>    }
>  
>    //
>    // Enroll KEK.
>    //
>    Status = EnrollListOfCerts (
>               EFI_KEY_EXCHANGE_KEY_NAME,
>               &gEfiGlobalVariableGuid,
>               &gEfiCertX509Guid,
> -             mRedHatPkKek1, mSizeOfRedHatPkKek1, &gEfiCallerIdGuid,
> +             PkKek1,        SizeOfPkKek1,        &gEfiCallerIdGuid,
>               mMicrosoftKek, mSizeOfMicrosoftKek, &gMicrosoftVendorGuid,
>               NULL);
>    if (EFI_ERROR (Status)) {
> -    return 1;
> +    goto FreePkKek1;
>    }
>  
>    //
>    // Enroll PK, leaving Setup Mode (entering User Mode) at once.
>    //
>    Status = EnrollListOfCerts (
>               EFI_PLATFORM_KEY_NAME,
>               &gEfiGlobalVariableGuid,
>               &gEfiCertX509Guid,
> -             mRedHatPkKek1, mSizeOfRedHatPkKek1, &gEfiGlobalVariableGuid,
> +             PkKek1, SizeOfPkKek1, &gEfiGlobalVariableGuid,
>               NULL);
>    if (EFI_ERROR (Status)) {
> -    return 1;
> +    goto FreePkKek1;
>    }
>  
>    //
>    // Leave Custom Mode, so that updates to PK, KEK, db, and dbx require valid
>    // signatures.
>    //
>    Settings.CustomMode = STANDARD_SECURE_BOOT_MODE;
>    Status = gRT->SetVariable (EFI_CUSTOM_MODE_NAME, &gEfiCustomModeEnableGuid,
>                    EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,
>                    sizeof Settings.CustomMode, &Settings.CustomMode);
>    if (EFI_ERROR (Status)) {
>      AsciiPrint ("error: SetVariable(\"%s\", %g): %r\n", EFI_CUSTOM_MODE_NAME,
>        &gEfiCustomModeEnableGuid, Status);
> -    return 1;
> +    goto FreePkKek1;
>    }
>  
>    //
>    // Final sanity check:
>    //
>    //                                 [SetupMode]
>    //                        (read-only, standardized by UEFI)
>    //                                /                \_
> @@ -488,22 +680,27 @@ ShellAppMain (
>    //                                /                           \_
>    //                          0, default                         1
>    //                              /                               \_
>    //                      PK, KEK, db, dbx                PK, KEK, db, dbx
>    //                    updates are verified          updates are not verified
>    //
>    Status = GetSettings (&Settings);
>    if (EFI_ERROR (Status)) {
> -    return 1;
> +    goto FreePkKek1;
>    }
>    PrintSettings (&Settings);
>  
>    if (Settings.SetupMode != 0 || Settings.SecureBoot != 1 ||
>        Settings.SecureBootEnable != 1 || Settings.CustomMode != 0 ||
>        Settings.VendorKeys != 0) {
>      AsciiPrint ("error: unexpected\n");
> -    return 1;
> +    goto FreePkKek1;
>    }
>  
>    AsciiPrint ("info: success\n");
> -  return 0;
> +  RetVal = 0;
> +
> +FreePkKek1:
> +  FreePool (PkKek1);
> +
> +  return RetVal;
>  }
>