[edk2-devel] [PATCH 2/2] AmdSev: Halt on failed blob allocation

[Tobin Feldman-Fitzthum <tobin@linux.ibm.com>]

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>
---
 .../BlobVerifierSevHashes.c                     | 17 ++++++++++++++++-
 OvmfPkg/Include/Library/BlobVerifierLib.h       | 14 ++++++++++----
 .../BlobVerifierLibNull/BlobVerifierNull.c      | 13 ++++++++-----
 .../QemuKernelLoaderFsDxe.c                     |  9 ++++-----
 4 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/OvmfPkg/AmdSev/BlobVerifierLibSevHashes/BlobVerifierSevHashes.c b/OvmfPkg/AmdSev/BlobVerifierLibSevHashes/BlobVerifierSevHashes.c
index ee8bca509a..c550518d73 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..efe26734b1 100644
--- a/OvmfPkg/Include/Library/BlobVerifierLib.h
+++ b/OvmfPkg/Include/Library/BlobVerifierLib.h
@@ -19,20 +19,26 @@
 /**
   Verify blob from an external source.
 
+  If a non-secure configuration is detected this function will enter a
+  dead loop to prevent a boot.
+
   @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;
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118663): https://edk2.groups.io/g/devel/message/118663
Mute This Topic: https://groups.io/mt/105977015/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-