* [edk2-devel] [PATCH 0/2] AmdSev: Harden SEV Kernel hashes verifier
@ 2024-05-06 20:27 Tobin Feldman-Fitzthum
2024-05-06 20:27 ` [edk2-devel] [PATCH 1/2] AmdSev: Rework Blob Verifier Tobin Feldman-Fitzthum
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Tobin Feldman-Fitzthum @ 2024-05-06 20:27 UTC (permalink / raw)
To: devel
Cc: dov.murik, james.bottomley, thomas.lendacky, tobin,
Tobin Feldman-Fitzthum
The AmdSev package has a so-called BlobVerifier, which
is meant to extend the TCB of a confidential guest
(SEV or SNP) to include components provided via fw_cfg
such as initrd, kernel, kernel params.
This series fixes a few implementation errors in the
blob verifier. One common theme is that the verifier
currently fails to halt the boot when an invalid blob
is detected. This can lead to a confidential guest
having a launch measurement that does not reflect the
guest TCB.
This series could also help us move towards consolidating
the AmdSev package back into the OvmfPkg although more
discussion will be needed on this.
Thank you for Ryan Savino at AMD for pointing out
some of these issues.
Tobin Feldman-Fitzthum (2):
AmdSev: Rework Blob Verifier
AmdSev: Halt on failed blob allocation
.../BlobVerifierSevHashes.c | 56 ++++++++++++++++---
OvmfPkg/Include/Library/BlobVerifierLib.h | 14 +++--
.../BlobVerifierLibNull/BlobVerifierNull.c | 13 +++--
.../QemuKernelLoaderFsDxe.c | 9 ++-
4 files changed, 69 insertions(+), 23 deletions(-)
--
2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118661): https://edk2.groups.io/g/devel/message/118661
Mute This Topic: https://groups.io/mt/105977013/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 9+ messages in thread
* [edk2-devel] [PATCH 1/2] AmdSev: Rework Blob Verifier
2024-05-06 20:27 [edk2-devel] [PATCH 0/2] AmdSev: Harden SEV Kernel hashes verifier Tobin Feldman-Fitzthum
@ 2024-05-06 20:27 ` Tobin Feldman-Fitzthum
2024-05-30 15:46 ` Lendacky, Thomas via groups.io
2024-05-06 20:27 ` [edk2-devel] [PATCH 2/2] AmdSev: Halt on failed blob allocation Tobin Feldman-Fitzthum
2024-06-26 8:08 ` [edk2-devel] [PATCH 0/2] AmdSev: Harden SEV Kernel hashes verifier Aithal, Srikanth via groups.io
2 siblings, 1 reply; 9+ messages in thread
From: Tobin Feldman-Fitzthum @ 2024-05-06 20:27 UTC (permalink / raw)
To: devel
Cc: dov.murik, james.bottomley, thomas.lendacky, tobin,
Tobin Feldman-Fitzthum
The Blob Verifier checks boot artifacts against a hash table
injected by the hypervisor and measured by hardware.
Update the Blob Verifier to enter a dead loop if the artifacts
do not match.
Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
---
.../BlobVerifierSevHashes.c | 39 +++++++++++++++----
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/OvmfPkg/AmdSev/BlobVerifierLibSevHashes/BlobVerifierSevHashes.c b/OvmfPkg/AmdSev/BlobVerifierLibSevHashes/BlobVerifierSevHashes.c
index 2e58794c3c..ee8bca509a 100644
--- a/OvmfPkg/AmdSev/BlobVerifierLibSevHashes/BlobVerifierSevHashes.c
+++ b/OvmfPkg/AmdSev/BlobVerifierLibSevHashes/BlobVerifierSevHashes.c
@@ -77,13 +77,17 @@ FindBlobEntryGuid (
/**
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
- @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
@@ -99,8 +103,8 @@ VerifyBlob (
if ((mHashesTable == NULL) || (mHashesTableSize == 0)) {
DEBUG ((
- DEBUG_ERROR,
- "%a: Verifier called but no hashes table discoverd in MEMFD\n",
+ DEBUG_WARN,
+ "%a: No hashes table discovered in MEMFD\n",
__func__
));
return EFI_ACCESS_DENIED;
@@ -114,7 +118,8 @@ VerifyBlob (
__func__,
BlobName
));
- return EFI_ACCESS_DENIED;
+
+ CpuDeadLoop ();
}
//
@@ -136,10 +141,22 @@ VerifyBlob (
DEBUG ((DEBUG_INFO, "%a: Found GUID %g in table\n", __func__, Guid));
+ if (BufSize == 0) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Blob Specified in Hash Table was not Provided",
+ __func__,
+ EntrySize,
+ SHA256_DIGEST_SIZE
+ ));
+
+ CpuDeadLoop ();
+ }
+
EntrySize = Entry->Len - sizeof Entry->Guid - sizeof Entry->Len;
if (EntrySize != SHA256_DIGEST_SIZE) {
DEBUG ((
- DEBUG_ERROR,
+ DEBUG_WARN,
"%a: Hash has the wrong size %d != %d\n",
__func__,
EntrySize,
@@ -170,18 +187,24 @@ VerifyBlob (
__func__,
BlobName
));
+
+ CpuDeadLoop ();
}
return Status;
}
+ //
+ // If the GUID is not in the hash table, execution can still continue.
+ // This blob will not be measured, but at least one blob must be.
+ //
DEBUG ((
DEBUG_ERROR,
"%a: Hash GUID %g not found in table\n",
__func__,
Guid
));
- return EFI_ACCESS_DENIED;
+ return EFI_SUCCESS;
}
/**
--
2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118662): https://edk2.groups.io/g/devel/message/118662
Mute This Topic: https://groups.io/mt/105977014/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [edk2-devel] [PATCH 2/2] AmdSev: Halt on failed blob allocation
2024-05-06 20:27 [edk2-devel] [PATCH 0/2] AmdSev: Harden SEV Kernel hashes verifier Tobin Feldman-Fitzthum
2024-05-06 20:27 ` [edk2-devel] [PATCH 1/2] AmdSev: Rework Blob Verifier Tobin Feldman-Fitzthum
@ 2024-05-06 20:27 ` Tobin Feldman-Fitzthum
2024-05-30 15:51 ` Lendacky, Thomas via groups.io
2024-06-26 8:08 ` [edk2-devel] [PATCH 0/2] AmdSev: Harden SEV Kernel hashes verifier Aithal, Srikanth via groups.io
2 siblings, 1 reply; 9+ messages in thread
From: Tobin Feldman-Fitzthum @ 2024-05-06 20:27 UTC (permalink / raw)
To: devel
Cc: dov.murik, james.bottomley, thomas.lendacky, tobin,
Tobin Feldman-Fitzthum
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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH 1/2] AmdSev: Rework Blob Verifier
2024-05-06 20:27 ` [edk2-devel] [PATCH 1/2] AmdSev: Rework Blob Verifier Tobin Feldman-Fitzthum
@ 2024-05-30 15:46 ` Lendacky, Thomas via groups.io
0 siblings, 0 replies; 9+ messages in thread
From: Lendacky, Thomas via groups.io @ 2024-05-30 15:46 UTC (permalink / raw)
To: Tobin Feldman-Fitzthum, devel, Gerd Hoffmann, Ard Biesheuvel
Cc: dov.murik, james.bottomley, tobin
On 5/6/24 15:27, Tobin Feldman-Fitzthum wrote:
> The Blob Verifier checks boot artifacts against a hash table
> injected by the hypervisor and measured by hardware.
>
> Update the Blob Verifier to enter a dead loop if the artifacts
> do not match.
There are some changes to messages from ERROR to WARN and the return is
kept as EFI_ACCESS_DENIED, so it would be best to describe in the commit
message what situations result in EFI_ACCES_DENIED vs dead loop.
>
> Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
> ---
> .../BlobVerifierSevHashes.c | 39 +++++++++++++++----
> 1 file changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/OvmfPkg/AmdSev/BlobVerifierLibSevHashes/BlobVerifierSevHashes.c b/OvmfPkg/AmdSev/BlobVerifierLibSevHashes/BlobVerifierSevHashes.c
> index 2e58794c3c..ee8bca509a 100644
> --- a/OvmfPkg/AmdSev/BlobVerifierLibSevHashes/BlobVerifierSevHashes.c
> +++ b/OvmfPkg/AmdSev/BlobVerifierLibSevHashes/BlobVerifierSevHashes.c
> @@ -77,13 +77,17 @@ FindBlobEntryGuid (
> /**
> 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
>
> - @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
> @@ -99,8 +103,8 @@ VerifyBlob (
>
> if ((mHashesTable == NULL) || (mHashesTableSize == 0)) {
> DEBUG ((
> - DEBUG_ERROR,
> - "%a: Verifier called but no hashes table discoverd in MEMFD\n",
> + DEBUG_WARN,
> + "%a: No hashes table discovered in MEMFD\n",
Technically, this should really just change the ERROR to WARN without
changing the message text.
> __func__
> ));
> return EFI_ACCESS_DENIED;
> @@ -114,7 +118,8 @@ VerifyBlob (
> __func__,
> BlobName
> ));
> - return EFI_ACCESS_DENIED;
> +
> + CpuDeadLoop ();
> }
>
> //
> @@ -136,10 +141,22 @@ VerifyBlob (
>
> DEBUG ((DEBUG_INFO, "%a: Found GUID %g in table\n", __func__, Guid));
>
> + if (BufSize == 0) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: Blob Specified in Hash Table was not Provided",
> + __func__,
> + EntrySize,
> + SHA256_DIGEST_SIZE
Looks like there's no substitution spots in the message for these last
two parameters.
Thanks,
Tom
> + ));
> +
> + CpuDeadLoop ();
> + }
> +
> EntrySize = Entry->Len - sizeof Entry->Guid - sizeof Entry->Len;
> if (EntrySize != SHA256_DIGEST_SIZE) {
> DEBUG ((
> - DEBUG_ERROR,
> + DEBUG_WARN,
> "%a: Hash has the wrong size %d != %d\n",
> __func__,
> EntrySize,
> @@ -170,18 +187,24 @@ VerifyBlob (
> __func__,
> BlobName
> ));
> +
> + CpuDeadLoop ();
> }
>
> return Status;
> }
>
> + //
> + // If the GUID is not in the hash table, execution can still continue.
> + // This blob will not be measured, but at least one blob must be.
> + //
> DEBUG ((
> DEBUG_ERROR,
> "%a: Hash GUID %g not found in table\n",
> __func__,
> Guid
> ));
> - return EFI_ACCESS_DENIED;
> + return EFI_SUCCESS;
> }
>
> /**
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119397): https://edk2.groups.io/g/devel/message/119397
Mute This Topic: https://groups.io/mt/105977014/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH 2/2] AmdSev: Halt on failed blob allocation
2024-05-06 20:27 ` [edk2-devel] [PATCH 2/2] AmdSev: Halt on failed blob allocation Tobin Feldman-Fitzthum
@ 2024-05-30 15:51 ` Lendacky, Thomas via groups.io
0 siblings, 0 replies; 9+ messages in thread
From: Lendacky, Thomas via groups.io @ 2024-05-30 15:51 UTC (permalink / raw)
To: Tobin Feldman-Fitzthum, devel, Gerd Hoffmann, Ard Biesheuvel
Cc: dov.murik, james.bottomley, tobin
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 <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.
> +
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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH 0/2] AmdSev: Harden SEV Kernel hashes verifier
2024-05-06 20:27 [edk2-devel] [PATCH 0/2] AmdSev: Harden SEV Kernel hashes verifier Tobin Feldman-Fitzthum
2024-05-06 20:27 ` [edk2-devel] [PATCH 1/2] AmdSev: Rework Blob Verifier Tobin Feldman-Fitzthum
2024-05-06 20:27 ` [edk2-devel] [PATCH 2/2] AmdSev: Halt on failed blob allocation Tobin Feldman-Fitzthum
@ 2024-06-26 8:08 ` Aithal, Srikanth via groups.io
2024-06-26 13:58 ` Tobin Feldman-Fitzthum
2 siblings, 1 reply; 9+ messages in thread
From: Aithal, Srikanth via groups.io @ 2024-06-26 8:08 UTC (permalink / raw)
To: devel, tobin; +Cc: dov.murik, james.bottomley, thomas.lendacky, tobin
[-- Attachment #1: Type: text/plain, Size: 3219 bytes --]
Hello,
SEV/SEVES guest boot fails with AMDSEV OVMF package built using upstream
edk2 master [commit head: 2fbaaa96d11ad61a9133df1728e3fe965d1457a5].
SEV/SEVES guest boot with AMDSEV package gets stuck at below point:
Plain Text
|2024-06-26 04:38:02: FetchBlob: loading 14332416 bytes for "kernel"
2024-06-26 04:38:02: Select Item: 0x18 2024-06-26 04:38:02: Select Item:
0x11 2024-06-26 04:38:02: VerifyBlob: Found GUID
4DE79437-ABD2-427F-B835-D5B172D2045B in table 2024-06-26 04:38:02:
VerifyBlob: Hash comparison succeeded for "kernel" 2024-06-26 04:38:02:
Select Item: 0xB 2024-06-26 04:38:02: VerifyBlob: Found GUID
44BAF731-3A2F-4BD7-9AF1-41E29169781D in table 2024-06-26 04:38:02:
VerifyBlob: Blob Specified in Hash Table was not Provided --> Hung here |
This was working until yesterday [commit head: be38c01], where we can
see boot was proceeding
Plain Text
|2024-06-25 03:13:23: VerifyBlob: Found GUID
4DE79437-ABD2-427F-B835-D5B172D2045B in table 2024-06-25 03:13:23:
VerifyBlob: Hash comparison succeeded for "kernel" 2024-06-25 03:13:23:
Select Item: 0xB 2024-06-25 03:13:23: VerifyBlob: Found GUID
44BAF731-3A2F-4BD7-9AF1-41E29169781D in table 2024-06-25 03:13:23:
VerifyBlob: Hash comparison succeeded for "initrd" 2024-06-25 03:13:23:
Select Item: 0x14 2024-06-25 03:13:23: FetchBlob: loading 120 bytes for
"cmdline" 2024-06-25 03:13:23: Select Item: 0x15 2024-06-25 03:13:23:
VerifyBlob: Found GUID 97D02DD8-BD20-4C94-AA78-E7714D36AB2A in table
2024-06-25 03:13:23: VerifyBlob: Hash comparison succeeded for "cmdline"|
After this patch got merged the regression is seen.
Thanks,
Srikanth Aithal
On 5/7/2024 1:57 AM, Tobin Feldman-Fitzthum via groups.io wrote:
> The AmdSev package has a so-called BlobVerifier, which
> is meant to extend the TCB of a confidential guest
> (SEV or SNP) to include components provided via fw_cfg
> such as initrd, kernel, kernel params.
>
> This series fixes a few implementation errors in the
> blob verifier. One common theme is that the verifier
> currently fails to halt the boot when an invalid blob
> is detected. This can lead to a confidential guest
> having a launch measurement that does not reflect the
> guest TCB.
>
> This series could also help us move towards consolidating
> the AmdSev package back into the OvmfPkg although more
> discussion will be needed on this.
>
> Thank you for Ryan Savino at AMD for pointing out
> some of these issues.
>
> Tobin Feldman-Fitzthum (2):
> AmdSev: Rework Blob Verifier
> AmdSev: Halt on failed blob allocation
>
> .../BlobVerifierSevHashes.c | 56 ++++++++++++++++---
> OvmfPkg/Include/Library/BlobVerifierLib.h | 14 +++--
> .../BlobVerifierLibNull/BlobVerifierNull.c | 13 +++--
> .../QemuKernelLoaderFsDxe.c | 9 ++-
> 4 files changed, 69 insertions(+), 23 deletions(-)
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119717): https://edk2.groups.io/g/devel/message/119717
Mute This Topic: https://groups.io/mt/105977013/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #2: Type: text/html, Size: 6450 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH 0/2] AmdSev: Harden SEV Kernel hashes verifier
2024-06-26 8:08 ` [edk2-devel] [PATCH 0/2] AmdSev: Harden SEV Kernel hashes verifier Aithal, Srikanth via groups.io
@ 2024-06-26 13:58 ` Tobin Feldman-Fitzthum
2024-06-26 14:33 ` Aithal, Srikanth via groups.io
0 siblings, 1 reply; 9+ messages in thread
From: Tobin Feldman-Fitzthum @ 2024-06-26 13:58 UTC (permalink / raw)
To: Aithal, Srikanth, devel
Cc: dov.murik, james.bottomley, thomas.lendacky, tobin
On 6/26/24 4:08 AM, Aithal, Srikanth wrote:
> Hello, SEV/SEVES guest boot fails with AMDSEV OVMF package built using
> upstream edk2 master [commit head:
> 2fbaaa96d11ad61a9133df1728e3fe965d1457a5]. SEV/SEVES guest boot with
> AMDSEV package gets stuck at below point: Plain Text 2024-06-26 04: 38:
> 02:
>
>
> Hello,
>
> SEV/SEVES guest boot fails with AMDSEV OVMF package built using upstream
> edk2 master [commit head: 2fbaaa96d11ad61a9133df1728e3fe965d1457a5].
>
> SEV/SEVES guest boot with AMDSEV package gets stuck at below point:
>
> Plain Text
>
> |2024-06-26 04:38:02: FetchBlob: loading 14332416 bytes for "kernel"
> 2024-06-26 04:38:02: Select Item: 0x18 2024-06-26 04:38:02: Select Item:
> 0x11 2024-06-26 04:38:02: VerifyBlob: Found GUID
> 4DE79437-ABD2-427F-B835-D5B172D2045B in table 2024-06-26 04:38:02:
> VerifyBlob: Hash comparison succeeded for "kernel" 2024-06-26 04:38:02:
> Select Item: 0xB 2024-06-26 04:38:02: VerifyBlob: Found GUID
> 44BAF731-3A2F-4BD7-9AF1-41E29169781D in table 2024-06-26 04:38:02:
> VerifyBlob: Blob Specified in Hash Table was not Provided --> Hung here |
>
Can you send qemu command that produces this case?
It looks like what is happening is that the initrd is specified in the
hash table, but is not provided to the guest.
You might try with and without the -initrd option and compare.
It's possible that when -initrd is not provided, QEMU will still put
something in the table, which this patch might not account for.
-Tobin
> This was working until yesterday [commit head: be38c01], where we can
> see boot was proceeding
>
> Plain Text
>
> |2024-06-25 03:13:23: VerifyBlob: Found GUID
> 4DE79437-ABD2-427F-B835-D5B172D2045B in table 2024-06-25 03:13:23:
> VerifyBlob: Hash comparison succeeded for "kernel" 2024-06-25 03:13:23:
> Select Item: 0xB 2024-06-25 03:13:23: VerifyBlob: Found GUID
> 44BAF731-3A2F-4BD7-9AF1-41E29169781D in table 2024-06-25 03:13:23:
> VerifyBlob: Hash comparison succeeded for "initrd" 2024-06-25 03:13:23:
> Select Item: 0x14 2024-06-25 03:13:23: FetchBlob: loading 120 bytes for
> "cmdline" 2024-06-25 03:13:23: Select Item: 0x15 2024-06-25 03:13:23:
> VerifyBlob: Found GUID 97D02DD8-BD20-4C94-AA78-E7714D36AB2A in table
> 2024-06-25 03:13:23: VerifyBlob: Hash comparison succeeded for "cmdline"|
>
>
> After this patch got merged the regression is seen.
>
> Thanks,
>
> Srikanth Aithal
>
>
> On 5/7/2024 1:57 AM, Tobin Feldman-Fitzthum via groups.io wrote:
>> The AmdSev package has a so-called BlobVerifier, which
>> is meant to extend the TCB of a confidential guest
>> (SEV or SNP) to include components provided via fw_cfg
>> such as initrd, kernel, kernel params.
>>
>> This series fixes a few implementation errors in the
>> blob verifier. One common theme is that the verifier
>> currently fails to halt the boot when an invalid blob
>> is detected. This can lead to a confidential guest
>> having a launch measurement that does not reflect the
>> guest TCB.
>>
>> This series could also help us move towards consolidating
>> the AmdSev package back into the OvmfPkg although more
>> discussion will be needed on this.
>>
>> Thank you for Ryan Savino at AMD for pointing out
>> some of these issues.
>>
>> Tobin Feldman-Fitzthum (2):
>> AmdSev: Rework Blob Verifier
>> AmdSev: Halt on failed blob allocation
>>
>> .../BlobVerifierSevHashes.c | 56 ++++++++++++++++---
>> OvmfPkg/Include/Library/BlobVerifierLib.h | 14 +++--
>> .../BlobVerifierLibNull/BlobVerifierNull.c | 13 +++--
>> .../QemuKernelLoaderFsDxe.c | 9 ++-
>> 4 files changed, 69 insertions(+), 23 deletions(-)
>>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119720): https://edk2.groups.io/g/devel/message/119720
Mute This Topic: https://groups.io/mt/105977013/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH 0/2] AmdSev: Harden SEV Kernel hashes verifier
2024-06-26 13:58 ` Tobin Feldman-Fitzthum
@ 2024-06-26 14:33 ` Aithal, Srikanth via groups.io
2024-06-26 17:14 ` Tobin Feldman-Fitzthum
0 siblings, 1 reply; 9+ messages in thread
From: Aithal, Srikanth via groups.io @ 2024-06-26 14:33 UTC (permalink / raw)
To: Tobin Feldman-Fitzthum, devel
Cc: dov.murik, james.bottomley, thomas.lendacky, tobin
On 6/26/2024 7:28 PM, Tobin Feldman-Fitzthum wrote:
>
>
> On 6/26/24 4:08 AM, Aithal, Srikanth wrote:
>> Hello, SEV/SEVES guest boot fails with AMDSEV OVMF package built
>> using upstream edk2 master [commit head:
>> 2fbaaa96d11ad61a9133df1728e3fe965d1457a5]. SEV/SEVES guest boot with
>> AMDSEV package gets stuck at below point: Plain Text 2024-06-26
>> 04: 38: 02:
>>
>>
>> Hello,
>>
>> SEV/SEVES guest boot fails with AMDSEV OVMF package built using
>> upstream edk2 master [commit
>> head: 2fbaaa96d11ad61a9133df1728e3fe965d1457a5].
>>
>> SEV/SEVES guest boot with AMDSEV package gets stuck at below point:
>>
>> Plain Text
>>
>> |2024-06-26 04:38:02: FetchBlob: loading 14332416 bytes for "kernel"
>> 2024-06-26 04:38:02: Select Item: 0x18 2024-06-26 04:38:02: Select
>> Item: 0x11 2024-06-26 04:38:02: VerifyBlob: Found GUID
>> 4DE79437-ABD2-427F-B835-D5B172D2045B in table 2024-06-26 04:38:02:
>> VerifyBlob: Hash comparison succeeded for "kernel" 2024-06-26
>> 04:38:02: Select Item: 0xB 2024-06-26 04:38:02: VerifyBlob: Found
>> GUID 44BAF731-3A2F-4BD7-9AF1-41E29169781D in table 2024-06-26
>> 04:38:02: VerifyBlob: Blob Specified in Hash Table was not Provided
>> --> Hung here |
>>
> Can you send qemu command that produces this case?
> It looks like what is happening is that the initrd is specified in the
> hash table, but is not provided to the guest.
> You might try with and without the -initrd option and compare.
> It's possible that when -initrd is not provided, QEMU will still put
> something in the table, which this patch might not account for.
>
> -Tobin
my qemu command line to recreate the issue:
qemu-system-x86_64 \
-machine q35,confidential-guest-support=sev0,vmport=off \
-object sev-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,kernel-hashes=on \
-name guest=vm,debug-threads=on \
-drive
if=pflash,format=raw,unit=0,file=/VT_BUILD/OVMF_AmdSev/OVMF.fd,readonly \
-cpu host \
-m 4096 \
-kernel '/VT_BUILD/usr/local/bzImage' \
-append 'root=/dev/sda rw console=ttyS0,115200n8
earlyprintk=ttyS0,115200 net.ifnames=0 biosdevname=0 movable_node
swiotlb=65536' \
-smp 1,cores=1,threads=1,dies=1,sockets=1 \
-drive
file=/VT_BUILD/images/22.04-server.qcow2,index=0,media=disk,format=qcow2 \
--enable-kvm \
--nographic
Yes, if I pass -initrd explicitly the Hash verify step passes properly
FetchBlob: loading 14332416 bytes for "kernel"
Select Item: 0x18
Select Item: 0x11
VerifyBlob: Found GUID 4DE79437-ABD2-427F-B835-D5B172D2045B in table
VerifyBlob: Hash comparison succeeded for "kernel"
Select Item: 0xB
FetchBlob: loading 75379943 bytes for "initrd"
Select Item: 0x12
VerifyBlob: Found GUID 44BAF731-3A2F-4BD7-9AF1-41E29169781D in table
VerifyBlob: Hash comparison succeeded for "initrd"
Select Item: 0x14
FetchBlob: loading 120 bytes for "cmdline"
Select Item: 0x15
VerifyBlob: Found GUID 97D02DD8-BD20-4C94-AA78-E7714D36AB2A in table
VerifyBlob: Hash comparison succeeded for "cmdline"
>> This was working until yesterday [commit head: be38c01], where we can
>> see boot was proceeding
>>
>> Plain Text
>>
>> |2024-06-25 03:13:23: VerifyBlob: Found GUID
>> 4DE79437-ABD2-427F-B835-D5B172D2045B in table 2024-06-25 03:13:23:
>> VerifyBlob: Hash comparison succeeded for "kernel" 2024-06-25
>> 03:13:23: Select Item: 0xB 2024-06-25 03:13:23: VerifyBlob: Found
>> GUID 44BAF731-3A2F-4BD7-9AF1-41E29169781D in table 2024-06-25
>> 03:13:23: VerifyBlob: Hash comparison succeeded for "initrd"
>> 2024-06-25 03:13:23: Select Item: 0x14 2024-06-25 03:13:23:
>> FetchBlob: loading 120 bytes for "cmdline" 2024-06-25 03:13:23:
>> Select Item: 0x15 2024-06-25 03:13:23: VerifyBlob: Found GUID
>> 97D02DD8-BD20-4C94-AA78-E7714D36AB2A in table 2024-06-25 03:13:23:
>> VerifyBlob: Hash comparison succeeded for "cmdline"|
>>
>>
>> After this patch got merged the regression is seen.
>>
>> Thanks,
>>
>> Srikanth Aithal
>>
>>
>> On 5/7/2024 1:57 AM, Tobin Feldman-Fitzthum via groups.io wrote:
>>> The AmdSev package has a so-called BlobVerifier, which
>>> is meant to extend the TCB of a confidential guest
>>> (SEV or SNP) to include components provided via fw_cfg
>>> such as initrd, kernel, kernel params.
>>>
>>> This series fixes a few implementation errors in the
>>> blob verifier. One common theme is that the verifier
>>> currently fails to halt the boot when an invalid blob
>>> is detected. This can lead to a confidential guest
>>> having a launch measurement that does not reflect the
>>> guest TCB.
>>>
>>> This series could also help us move towards consolidating
>>> the AmdSev package back into the OvmfPkg although more
>>> discussion will be needed on this.
>>>
>>> Thank you for Ryan Savino at AMD for pointing out
>>> some of these issues.
>>>
>>> Tobin Feldman-Fitzthum (2):
>>> AmdSev: Rework Blob Verifier
>>> AmdSev: Halt on failed blob allocation
>>>
>>> .../BlobVerifierSevHashes.c | 56
>>> ++++++++++++++++---
>>> OvmfPkg/Include/Library/BlobVerifierLib.h | 14 +++--
>>> .../BlobVerifierLibNull/BlobVerifierNull.c | 13 +++--
>>> .../QemuKernelLoaderFsDxe.c | 9 ++-
>>> 4 files changed, 69 insertions(+), 23 deletions(-)
>>>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119718): https://edk2.groups.io/g/devel/message/119718
Mute This Topic: https://groups.io/mt/105977013/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH 0/2] AmdSev: Harden SEV Kernel hashes verifier
2024-06-26 14:33 ` Aithal, Srikanth via groups.io
@ 2024-06-26 17:14 ` Tobin Feldman-Fitzthum
0 siblings, 0 replies; 9+ messages in thread
From: Tobin Feldman-Fitzthum @ 2024-06-26 17:14 UTC (permalink / raw)
To: Aithal, Srikanth, devel
Cc: dov.murik, james.bottomley, thomas.lendacky, tobin
On 6/26/24 10:33 AM, Aithal, Srikanth wrote:
>
> On 6/26/2024 7:28 PM, Tobin Feldman-Fitzthum wrote:
>>
>>
>> On 6/26/24 4:08 AM, Aithal, Srikanth wrote:
>>> Hello, SEV/SEVES guest boot fails with AMDSEV OVMF package built
>>> using upstream edk2 master [commit head:
>>> 2fbaaa96d11ad61a9133df1728e3fe965d1457a5]. SEV/SEVES guest boot with
>>> AMDSEV package gets stuck at below point: Plain Text 2024-06-26 04:
>>> 38: 02:
>>>
>>>
>>> Hello,
>>>
>>> SEV/SEVES guest boot fails with AMDSEV OVMF package built using
>>> upstream edk2 master [commit
>>> head: 2fbaaa96d11ad61a9133df1728e3fe965d1457a5].
>>>
>>> SEV/SEVES guest boot with AMDSEV package gets stuck at below point:
>>>
>>> Plain Text
>>>
>>> |2024-06-26 04:38:02: FetchBlob: loading 14332416 bytes for "kernel"
>>> 2024-06-26 04:38:02: Select Item: 0x18 2024-06-26 04:38:02: Select
>>> Item: 0x11 2024-06-26 04:38:02: VerifyBlob: Found GUID
>>> 4DE79437-ABD2-427F-B835-D5B172D2045B in table 2024-06-26 04:38:02:
>>> VerifyBlob: Hash comparison succeeded for "kernel" 2024-06-26
>>> 04:38:02: Select Item: 0xB 2024-06-26 04:38:02: VerifyBlob: Found
>>> GUID 44BAF731-3A2F-4BD7-9AF1-41E29169781D in table 2024-06-26
>>> 04:38:02: VerifyBlob: Blob Specified in Hash Table was not Provided
>>> --> Hung here |
>>>
>> Can you send qemu command that produces this case?
>> It looks like what is happening is that the initrd is specified in the
>> hash table, but is not provided to the guest.
>> You might try with and without the -initrd option and compare.
>> It's possible that when -initrd is not provided, QEMU will still put
>> something in the table, which this patch might not account for.
>>
>> -Tobin
> my qemu command line to recreate the issue:
>
> qemu-system-x86_64 \
> -machine q35,confidential-guest-support=sev0,vmport=off \
> -object sev-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,kernel-hashes=on \
> -name guest=vm,debug-threads=on \
> -drive
> if=pflash,format=raw,unit=0,file=/VT_BUILD/OVMF_AmdSev/OVMF.fd,readonly \
> -cpu host \
> -m 4096 \
> -kernel '/VT_BUILD/usr/local/bzImage' \
> -append 'root=/dev/sda rw console=ttyS0,115200n8
> earlyprintk=ttyS0,115200 net.ifnames=0 biosdevname=0 movable_node
> swiotlb=65536' \
> -smp 1,cores=1,threads=1,dies=1,sockets=1 \
> -drive
> file=/VT_BUILD/images/22.04-server.qcow2,index=0,media=disk,format=qcow2 \
> --enable-kvm \
> --nographic
>
>
> Yes, if I pass -initrd explicitly the Hash verify step passes properly
>
> FetchBlob: loading 14332416 bytes for "kernel"
> Select Item: 0x18
> Select Item: 0x11
> VerifyBlob: Found GUID 4DE79437-ABD2-427F-B835-D5B172D2045B in table
> VerifyBlob: Hash comparison succeeded for "kernel"
> Select Item: 0xB
> FetchBlob: loading 75379943 bytes for "initrd"
> Select Item: 0x12
> VerifyBlob: Found GUID 44BAF731-3A2F-4BD7-9AF1-41E29169781D in table
> VerifyBlob: Hash comparison succeeded for "initrd"
> Select Item: 0x14
> FetchBlob: loading 120 bytes for "cmdline"
> Select Item: 0x15
> VerifyBlob: Found GUID 97D02DD8-BD20-4C94-AA78-E7714D36AB2A in table
> VerifyBlob: Hash comparison succeeded for "cmdline"
>
Ok, I'll submit a little tweak this afternoon that should fix your case.
-Tobin
>>> This was working until yesterday [commit head: be38c01], where we can
>>> see boot was proceeding
>>>
>>> Plain Text
>>>
>>> |2024-06-25 03:13:23: VerifyBlob: Found GUID
>>> 4DE79437-ABD2-427F-B835-D5B172D2045B in table 2024-06-25 03:13:23:
>>> VerifyBlob: Hash comparison succeeded for "kernel" 2024-06-25
>>> 03:13:23: Select Item: 0xB 2024-06-25 03:13:23: VerifyBlob: Found
>>> GUID 44BAF731-3A2F-4BD7-9AF1-41E29169781D in table 2024-06-25
>>> 03:13:23: VerifyBlob: Hash comparison succeeded for "initrd"
>>> 2024-06-25 03:13:23: Select Item: 0x14 2024-06-25 03:13:23:
>>> FetchBlob: loading 120 bytes for "cmdline" 2024-06-25 03:13:23:
>>> Select Item: 0x15 2024-06-25 03:13:23: VerifyBlob: Found GUID
>>> 97D02DD8-BD20-4C94-AA78-E7714D36AB2A in table 2024-06-25 03:13:23:
>>> VerifyBlob: Hash comparison succeeded for "cmdline"|
>>>
>>>
>>> After this patch got merged the regression is seen.
>>>
>>> Thanks,
>>>
>>> Srikanth Aithal
>>>
>>>
>>> On 5/7/2024 1:57 AM, Tobin Feldman-Fitzthum via groups.io wrote:
>>>> The AmdSev package has a so-called BlobVerifier, which
>>>> is meant to extend the TCB of a confidential guest
>>>> (SEV or SNP) to include components provided via fw_cfg
>>>> such as initrd, kernel, kernel params.
>>>>
>>>> This series fixes a few implementation errors in the
>>>> blob verifier. One common theme is that the verifier
>>>> currently fails to halt the boot when an invalid blob
>>>> is detected. This can lead to a confidential guest
>>>> having a launch measurement that does not reflect the
>>>> guest TCB.
>>>>
>>>> This series could also help us move towards consolidating
>>>> the AmdSev package back into the OvmfPkg although more
>>>> discussion will be needed on this.
>>>>
>>>> Thank you for Ryan Savino at AMD for pointing out
>>>> some of these issues.
>>>>
>>>> Tobin Feldman-Fitzthum (2):
>>>> AmdSev: Rework Blob Verifier
>>>> AmdSev: Halt on failed blob allocation
>>>>
>>>> .../BlobVerifierSevHashes.c | 56
>>>> ++++++++++++++++---
>>>> OvmfPkg/Include/Library/BlobVerifierLib.h | 14 +++--
>>>> .../BlobVerifierLibNull/BlobVerifierNull.c | 13 +++--
>>>> .../QemuKernelLoaderFsDxe.c | 9 ++-
>>>> 4 files changed, 69 insertions(+), 23 deletions(-)
>>>>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119725): https://edk2.groups.io/g/devel/message/119725
Mute This Topic: https://groups.io/mt/105977013/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-06-27 22:42 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-06 20:27 [edk2-devel] [PATCH 0/2] AmdSev: Harden SEV Kernel hashes verifier Tobin Feldman-Fitzthum
2024-05-06 20:27 ` [edk2-devel] [PATCH 1/2] AmdSev: Rework Blob Verifier Tobin Feldman-Fitzthum
2024-05-30 15:46 ` Lendacky, Thomas via groups.io
2024-05-06 20:27 ` [edk2-devel] [PATCH 2/2] AmdSev: Halt on failed blob allocation Tobin Feldman-Fitzthum
2024-05-30 15:51 ` Lendacky, Thomas via groups.io
2024-06-26 8:08 ` [edk2-devel] [PATCH 0/2] AmdSev: Harden SEV Kernel hashes verifier Aithal, Srikanth via groups.io
2024-06-26 13:58 ` Tobin Feldman-Fitzthum
2024-06-26 14:33 ` Aithal, Srikanth via groups.io
2024-06-26 17:14 ` Tobin Feldman-Fitzthum
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox