Re: [PATCH v2 04/11] OvmfPkg: add library class BlobVerifierLib with null implementation

["Lendacky, Thomas" <thomas.lendacky@amd.com>]

On 7/6/21 3:54 AM, Dov Murik wrote:
> BlobVerifierLib will be used to verify blobs fetching them from QEMU's
> firmware config (fw_cfg) in platforms that enable such verification.
> 
> The null implementation NullBlobVerifierLib treats all blobs as valid.
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ashish Kalra <ashish.kalra@amd.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Min Xu <min.m.xu@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3457
> Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
> ---
>  OvmfPkg/OvmfPkg.dec                                     |  3 ++
>  OvmfPkg/Library/BlobVerifierLib/NullBlobVerifierLib.inf | 27 ++++++++++++++
>  OvmfPkg/Include/Library/BlobVerifierLib.h               | 38 ++++++++++++++++++++
>  OvmfPkg/Library/BlobVerifierLib/NullBlobVerifier.c      | 34 ++++++++++++++++++
>  4 files changed, 102 insertions(+)
> 
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index 6ae733f6e39f..f82228d69cc2 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -23,6 +23,9 @@ [LibraryClasses]
>    ##  @libraryclass  Access bhyve's firmware control interface.
>    BhyveFwCtlLib|Include/Library/BhyveFwCtlLib.h
>  
> +  ##  @libraryclass  Verify blobs read from the VMM
> +  BlobVerifierLib|Include/Library/BlobVerifierLib.h
> +
>    ##  @libraryclass  Loads and boots a Linux kernel image
>    #
>    LoadLinuxLib|Include/Library/LoadLinuxLib.h
> diff --git a/OvmfPkg/Library/BlobVerifierLib/NullBlobVerifierLib.inf b/OvmfPkg/Library/BlobVerifierLib/NullBlobVerifierLib.inf
> new file mode 100644
> index 000000000000..c8942ad05d96
> --- /dev/null
> +++ b/OvmfPkg/Library/BlobVerifierLib/NullBlobVerifierLib.inf
> @@ -0,0 +1,27 @@
> +## @file
> +#
> +#  Null implementation of the blob verifier library.
> +#
> +#  Copyright (C) 2021, IBM Corp
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005

You can specify the INF_VERSION using x.y format now, and I believe the
latest is 1.29.

> +  BASE_NAME                      = NullBlobVerifierLib

Typically, the NULL libraries would be named BlobVerifierLibNull.

> +  FILE_GUID                      = b1b5533e-e01a-43bb-9e54-414f00ca036e
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = BlobVerifierLib
> +
> +[Sources]
> +  NullBlobVerifier.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  OvmfPkg/OvmfPkg.dec
> +
> +[LibraryClasses]
> +  DebugLib

Is this library (and associated include below) needed?

> diff --git a/OvmfPkg/Include/Library/BlobVerifierLib.h b/OvmfPkg/Include/Library/BlobVerifierLib.h
> new file mode 100644
> index 000000000000..667024766681
> --- /dev/null
> +++ b/OvmfPkg/Include/Library/BlobVerifierLib.h
> @@ -0,0 +1,38 @@
> +/** @file
> +
> +  Blob verification library
> +
> +  This library class allows verifiying whether blobs from external sources
> +  (such as QEMU's firmware config) are trusted.
> +
> +  Copyright (C) 2021, IBM Corporation
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#ifndef BLOB_VERIFIER_LIB_H__
> +#define BLOB_VERIFIER_LIB_H__
> +
> +#include <Uefi/UefiBaseType.h>
> +#include <Base.h>
> +
> +/**
> +  Verify blob from an external source.
> +
> +  @param BlobName               The name of the blob

I believe this is supposed to be @param[in]

> +  @param Buf                    The data of the blob
> +  @param BufSize                The size of the blob in bytes
> +
> +  @retval EFI_SUCCESS           The blob was verified successfully.
> +  @retval EFI_ACCESS_DENIED     The blob could not be verified, and therefore
> +                                should be considered non-secure.
> +**/
> +EFI_STATUS
> +EFIAPI
> +VerifyBlob (
> +  IN  CONST CHAR16    *BlobName,
> +  IN  CONST VOID      *Buf,
> +  UINT32              BufSize

Missing "IN" here (same below for these).

Thanks,
Tom

> +  );
> +
> +#endif
> diff --git a/OvmfPkg/Library/BlobVerifierLib/NullBlobVerifier.c b/OvmfPkg/Library/BlobVerifierLib/NullBlobVerifier.c
> new file mode 100644
> index 000000000000..7b31b6ec767d
> --- /dev/null
> +++ b/OvmfPkg/Library/BlobVerifierLib/NullBlobVerifier.c
> @@ -0,0 +1,34 @@
> +/** @file
> +
> +  Null implementation of the blob verifier library.
> +
> +  Copyright (C) 2021, IBM Corporation
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/BlobVerifierLib.h>
> +
> +/**
> +  Verify blob from an external source.
> +
> +  @param BlobName               The name of the blob
> +  @param Buf                    The data of the blob
> +  @param BufSize                The size of the blob in bytes
> +
> +  @retval EFI_SUCCESS           The blob was verified successfully.
> +  @retval EFI_ACCESS_DENIED     The blob could not be verified, and therefore
> +                                should be considered non-secure.
> +**/
> +EFI_STATUS
> +EFIAPI
> +VerifyBlob (
> +  IN  CONST CHAR16    *BlobName,
> +  IN  CONST VOID      *Buf,
> +  UINT32              BufSize
> +  )
> +{
> +  return EFI_SUCCESS;
> +}
>