From 10b4bb8d6d0c515ed9663691aea3684be8f7b0fc Mon Sep 17 00:00:00 2001
From: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
Date: Mon, 29 Apr 2024 20:07:19 +0000
Subject: AmdSev: Halt on failed blob allocation

A malicious host may be able to undermine the fw_cfg
interface such that loading a blob fails.

In this case rather than continuing to the next boot
option, the blob verifier should halt.

For non-confidential guests, the error should be non-fatal.

Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
---
 .../BlobVerifierLibSevHashes/BlobVerifierSevHashes.c    | 17 ++++++++++++++++-
 OvmfPkg/Include/Library/BlobVerifierLib.h               | 11 +++++++----
 OvmfPkg/Library/BlobVerifierLibNull/BlobVerifierNull.c  | 13 ++++++++-----
 OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c   |  9 ++++-----
 4 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/OvmfPkg/AmdSev/BlobVerifierLibSevHashes/BlobVerifierSevHashes.c b/OvmfPkg/AmdSev/BlobVerifierLibSevHashes/BlobVerifierSevHashes.c
index 37c38e9e98..bc2d5daadc 100644
--- a/OvmfPkg/AmdSev/BlobVerifierLibSevHashes/BlobVerifierSevHashes.c
+++ b/OvmfPkg/AmdSev/BlobVerifierLibSevHashes/BlobVerifierSevHashes.c
@@ -83,6 +83,7 @@ FindBlobEntryGuid (
   @param[in] BlobName           The name of the blob
   @param[in] Buf                The data of the blob
   @param[in] BufSize            The size of the blob in bytes
+  @param[in] FetchStatus        The status of the previous blob fetch
 
   @retval EFI_SUCCESS           The blob was verified successfully or was not
                                 found in the hash table.
@@ -94,13 +95,27 @@ EFIAPI
 VerifyBlob (
   IN  CONST CHAR16  *BlobName,
   IN  CONST VOID    *Buf,
-  IN  UINT32        BufSize
+  IN  UINT32        BufSize,
+  IN  EFI_STATUS    FetchStatus
   )
 {
   CONST GUID  *Guid;
   INT32       Remaining;
   HASH_TABLE  *Entry;
 
+  // Enter a dead loop if the fetching of this blob
+  // failed. This prevents a malicious host from
+  // circumventing the following checks.
+  if (EFI_ERROR (FetchStatus)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: Fetching blob failed.\n",
+      __func__
+      ));
+
+    CpuDeadLoop ();
+  }
+
   if ((mHashesTable == NULL) || (mHashesTableSize == 0)) {
     DEBUG ((
       DEBUG_WARN,
diff --git a/OvmfPkg/Include/Library/BlobVerifierLib.h b/OvmfPkg/Include/Library/BlobVerifierLib.h
index 7e1af27574..09af1b77de 100644
--- a/OvmfPkg/Include/Library/BlobVerifierLib.h
+++ b/OvmfPkg/Include/Library/BlobVerifierLib.h
@@ -22,17 +22,20 @@
   @param[in] BlobName           The name of the blob
   @param[in] Buf                The data of the blob
   @param[in] BufSize            The size of the blob in bytes
+  @param[in] FetchStatus        The status of fetching this blob
 
-  @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.
+  @retval EFI_SUCCESS           The blob was verified successfully or was not
+                                found in the hash table.
+  @retval EFI_ACCESS_DENIED     Kernel hashes not supported but the boot can
+                                continue safely.
 **/
 EFI_STATUS
 EFIAPI
 VerifyBlob (
   IN  CONST CHAR16  *BlobName,
   IN  CONST VOID    *Buf,
-  IN  UINT32        BufSize
+  IN  UINT32        BufSize,
+  IN  EFI_STATUS    FetchStatus
   );
 
 #endif
diff --git a/OvmfPkg/Library/BlobVerifierLibNull/BlobVerifierNull.c b/OvmfPkg/Library/BlobVerifierLibNull/BlobVerifierNull.c
index e817c3cc95..db5320571c 100644
--- a/OvmfPkg/Library/BlobVerifierLibNull/BlobVerifierNull.c
+++ b/OvmfPkg/Library/BlobVerifierLibNull/BlobVerifierNull.c
@@ -16,18 +16,21 @@
   @param[in] BlobName           The name of the blob
   @param[in] Buf                The data of the blob
   @param[in] BufSize            The size of the blob in bytes
+  @param[in] FetchStatus        The status of the fetch of this blob
 
-  @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.
+  @retval EFI_SUCCESS           The blob was verified successfully or was not
+                                found in the hash table.
+  @retval EFI_ACCESS_DENIED     Kernel hashes not supported but the boot can
+                                continue safely.
 **/
 EFI_STATUS
 EFIAPI
 VerifyBlob (
   IN  CONST CHAR16  *BlobName,
   IN  CONST VOID    *Buf,
-  IN  UINT32        BufSize
+  IN  UINT32        BufSize,
+  IN  EFI_STATUS    FetchStatus
   )
 {
-  return EFI_SUCCESS;
+  return FetchStatus;
 }
diff --git a/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c b/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c
index 3c12085f6c..cf58c97cd2 100644
--- a/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c
+++ b/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c
@@ -1042,6 +1042,7 @@ QemuKernelLoaderFsDxeEntrypoint (
   KERNEL_BLOB  *CurrentBlob;
   KERNEL_BLOB  *KernelBlob;
   EFI_STATUS   Status;
+  EFI_STATUS   FetchStatus;
   EFI_HANDLE   FileSystemHandle;
   EFI_HANDLE   InitrdLoadFile2Handle;
 
@@ -1060,15 +1061,13 @@ QemuKernelLoaderFsDxeEntrypoint (
   //
   for (BlobType = 0; BlobType < KernelBlobTypeMax; ++BlobType) {
     CurrentBlob = &mKernelBlob[BlobType];
-    Status      = FetchBlob (CurrentBlob);
-    if (EFI_ERROR (Status)) {
-      goto FreeBlobs;
-    }
+    FetchStatus = FetchBlob (CurrentBlob);
 
     Status = VerifyBlob (
                CurrentBlob->Name,
                CurrentBlob->Data,
-               CurrentBlob->Size
+               CurrentBlob->Size,
+               FetchStatus
                );
     if (EFI_ERROR (Status)) {
       goto FreeBlobs;
-- 
cgit