On 5/6/24 15:27, Tobin Feldman-Fitzthum wrote:
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 <to...@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.
+
Probably shouldn't specify this here as the VerifyBlob() that is not in
AmdSev will not enter a dead loop.
Thanks,
Tom
@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;
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119398): https://edk2.groups.io/g/devel/message/119398
Mute This Topic: https://groups.io/mt/105977015/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
