[PATCH v4 10/11] OvmfPkg: add BlobVerifierLibSevHashes


Dov Murik
 

Add an implementation for BlobVerifierLib that locates the SEV hashes
table and verifies that the calculated hashes of the kernel, initrd, and
cmdline blobs indeed match the expected hashes stated in the hashes
table.

If there's a missing hash or a hash mismatch then EFI_ACCESS_DENIED is
returned which will cause a failure to load a kernel image.

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=3D3457
Co-developed-by: James Bottomley <jejb@linux.ibm.com>
Signed-off-by: James Bottomley <jejb@linux.ibm.com>
Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
---
OvmfPkg/Library/BlobVerifierLib/BlobVerifierLibSevHashes.inf | 37 ++++
OvmfPkg/Library/BlobVerifierLib/BlobVerifierSevHashes.c | 199 +++++++=
+++++++++++++
2 files changed, 236 insertions(+)

diff --git a/OvmfPkg/Library/BlobVerifierLib/BlobVerifierLibSevHashes.inf b=
/OvmfPkg/Library/BlobVerifierLib/BlobVerifierLibSevHashes.inf
new file mode 100644
index 000000000000..76ca0b8154ce
--- /dev/null
+++ b/OvmfPkg/Library/BlobVerifierLib/BlobVerifierLibSevHashes.inf
@@ -0,0 +1,37 @@
+## @file=0D
+#=0D
+# Blob verifier library that uses SEV hashes table. The hashes table hol=
ds the=0D
+# allowed hashes of the kernel, initrd, and cmdline blobs.=0D
+#=0D
+# Copyright (C) 2021, IBM Corp=0D
+#=0D
+# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
+#=0D
+##=0D
+=0D
+[Defines]=0D
+ INF_VERSION =3D 1.29=0D
+ BASE_NAME =3D BlobVerifierLibSevHashes=0D
+ FILE_GUID =3D 59e713b5-eff3-46a7-8d8b-46f4c004ad7b=
=0D
+ MODULE_TYPE =3D BASE=0D
+ VERSION_STRING =3D 1.0=0D
+ LIBRARY_CLASS =3D BlobVerifierLib=0D
+ CONSTRUCTOR =3D BlobVerifierLibSevHashesConstructor=0D
+=0D
+[Sources]=0D
+ BlobVerifierSevHashes.c=0D
+=0D
+[Packages]=0D
+ CryptoPkg/CryptoPkg.dec=0D
+ MdePkg/MdePkg.dec=0D
+ OvmfPkg/OvmfPkg.dec=0D
+=0D
+[LibraryClasses]=0D
+ BaseCryptLib=0D
+ BaseMemoryLib=0D
+ DebugLib=0D
+ PcdLib=0D
+=0D
+[FixedPcd]=0D
+ gUefiOvmfPkgTokenSpaceGuid.PcdQemuHashTableBase=0D
+ gUefiOvmfPkgTokenSpaceGuid.PcdQemuHashTableSize=0D
diff --git a/OvmfPkg/Library/BlobVerifierLib/BlobVerifierSevHashes.c b/Ovmf=
Pkg/Library/BlobVerifierLib/BlobVerifierSevHashes.c
new file mode 100644
index 000000000000..372ae6f469e7
--- /dev/null
+++ b/OvmfPkg/Library/BlobVerifierLib/BlobVerifierSevHashes.c
@@ -0,0 +1,199 @@
+/** @file=0D
+=0D
+ Blob verifier library that uses SEV hashes table. The hashes table hold=
s the=0D
+ allowed hashes of the kernel, initrd, and cmdline blobs.=0D
+=0D
+ Copyright (C) 2021, IBM Corporation=0D
+=0D
+ SPDX-License-Identifier: BSD-2-Clause-Patent=0D
+**/=0D
+=0D
+#include <Library/BaseCryptLib.h>=0D
+#include <Library/BaseLib.h>=0D
+#include <Library/BaseMemoryLib.h>=0D
+#include <Library/DebugLib.h>=0D
+#include <Library/BlobVerifierLib.h>=0D
+=0D
+/**=0D
+ The SEV Hashes table must be in encrypted memory and has the table=0D
+ and its entries described by=0D
+=0D
+ <GUID>|UINT16 <len>|<data>=0D
+=0D
+ With the whole table GUID being 9438d606-4f22-4cc9-b479-a793d411fd21=0D
+=0D
+ The current possible table entries are for the kernel, the initrd=0D
+ and the cmdline:=0D
+=0D
+ 4de79437-abd2-427f-b835-d5b172d2045b kernel=0D
+ 44baf731-3a2f-4bd7-9af1-41e29169781d initrd=0D
+ 97d02dd8-bd20-4c94-aa78-e7714d36ab2a cmdline=0D
+=0D
+ The size of the entry is used to identify the hash, but the=0D
+ expectation is that it will be 32 bytes of SHA-256.=0D
+**/=0D
+=0D
+#define SEV_HASH_TABLE_GUID \=0D
+ (GUID) { 0x9438d606, 0x4f22, 0x4cc9, { 0xb4, 0x79, 0xa7, 0x93, 0xd4, 0x1=
1, 0xfd, 0x21 } }=0D
+#define SEV_KERNEL_HASH_GUID \=0D
+ (GUID) { 0x4de79437, 0xabd2, 0x427f, { 0xb8, 0x35, 0xd5, 0xb1, 0x72, 0xd=
2, 0x04, 0x5b } }=0D
+#define SEV_INITRD_HASH_GUID \=0D
+ (GUID) { 0x44baf731, 0x3a2f, 0x4bd7, { 0x9a, 0xf1, 0x41, 0xe2, 0x91, 0x6=
9, 0x78, 0x1d } }=0D
+#define SEV_CMDLINE_HASH_GUID \=0D
+ (GUID) { 0x97d02dd8, 0xbd20, 0x4c94, { 0xaa, 0x78, 0xe7, 0x71, 0x4d, 0x3=
6, 0xab, 0x2a } }=0D
+=0D
+STATIC CONST EFI_GUID mSevKernelHashGuid =3D SEV_KERNEL_HASH_GUID;=0D
+STATIC CONST EFI_GUID mSevInitrdHashGuid =3D SEV_INITRD_HASH_GUID;=0D
+STATIC CONST EFI_GUID mSevCmdlineHashGuid =3D SEV_CMDLINE_HASH_GUID;=0D
+=0D
+#pragma pack (1)=0D
+typedef struct {=0D
+ GUID Guid;=0D
+ UINT16 Len;=0D
+ UINT8 Data[];=0D
+} HASH_TABLE;=0D
+#pragma pack ()=0D
+=0D
+STATIC HASH_TABLE *mHashesTable;=0D
+STATIC UINT16 mHashesTableSize;=0D
+=0D
+STATIC=0D
+CONST GUID*=0D
+FindBlobEntryGuid (=0D
+ IN CONST CHAR16 *BlobName=0D
+ )=0D
+{=0D
+ if (StrCmp (BlobName, L"kernel") =3D=3D 0) {=0D
+ return &mSevKernelHashGuid;=0D
+ } else if (StrCmp (BlobName, L"initrd") =3D=3D 0) {=0D
+ return &mSevInitrdHashGuid;=0D
+ } else if (StrCmp (BlobName, L"cmdline") =3D=3D 0) {=0D
+ return &mSevCmdlineHashGuid;=0D
+ } else {=0D
+ return NULL;=0D
+ }=0D
+}=0D
+=0D
+/**=0D
+ Verify blob from an external source.=0D
+=0D
+ @param[in] BlobName The name of the blob=0D
+ @param[in] Buf The data of the blob=0D
+ @param[in] BufSize The size of the blob in bytes=0D
+=0D
+ @retval EFI_SUCCESS The blob was verified successfully.=0D
+ @retval EFI_ACCESS_DENIED The blob could not be verified, and theref=
ore=0D
+ should be considered non-secure.=0D
+**/=0D
+EFI_STATUS=0D
+EFIAPI=0D
+VerifyBlob (=0D
+ IN CONST CHAR16 *BlobName,=0D
+ IN CONST VOID *Buf,=0D
+ IN UINT32 BufSize=0D
+ )=0D
+{=0D
+ CONST GUID *Guid;=0D
+ INT32 Remaining;=0D
+ HASH_TABLE *Entry;=0D
+=0D
+ if (mHashesTable =3D=3D NULL || mHashesTableSize =3D=3D 0) {=0D
+ DEBUG ((DEBUG_ERROR,=0D
+ "%a: Verifier called but no hashes table discoverd in MEMFD\n",=0D
+ __FUNCTION__));=0D
+ return EFI_ACCESS_DENIED;=0D
+ }=0D
+=0D
+ Guid =3D FindBlobEntryGuid (BlobName);=0D
+ if (Guid =3D=3D NULL) {=0D
+ DEBUG ((DEBUG_ERROR, "%a: Unknown blob name \"%s\"\n", __FUNCTION__,=0D
+ BlobName));=0D
+ return EFI_ACCESS_DENIED;=0D
+ }=0D
+=0D
+ //=0D
+ // Remaining is INT32 to catch underflow in case Entry->Len has a=0D
+ // very high UINT16 value=0D
+ //=0D
+ for (Entry =3D mHashesTable, Remaining =3D mHashesTableSize;=0D
+ Remaining >=3D sizeof *Entry && Remaining >=3D Entry->Len;=0D
+ Remaining -=3D Entry->Len,=0D
+ Entry =3D (HASH_TABLE *)((UINT8 *)Entry + Entry->Len)) {=0D
+ UINTN EntrySize;=0D
+ EFI_STATUS Status;=0D
+ UINT8 Hash[SHA256_DIGEST_SIZE];=0D
+=0D
+ if (!CompareGuid (&Entry->Guid, Guid)) {=0D
+ continue;=0D
+ }=0D
+=0D
+ DEBUG ((DEBUG_INFO, "%a: Found GUID %g in table\n", __FUNCTION__, Guid=
));=0D
+=0D
+ EntrySize =3D Entry->Len - sizeof Entry->Guid - sizeof Entry->Len;=0D
+ if (EntrySize !=3D SHA256_DIGEST_SIZE) {=0D
+ DEBUG ((DEBUG_ERROR, "%a: Hash has the wrong size %d !=3D %d\n",=0D
+ __FUNCTION__, EntrySize, SHA256_DIGEST_SIZE));=0D
+ return EFI_ACCESS_DENIED;=0D
+ }=0D
+=0D
+ //=0D
+ // Calculate the buffer's hash and verify that it is identical to the=
=0D
+ // expected hash table entry=0D
+ //=0D
+ Sha256HashAll (Buf, BufSize, Hash);=0D
+=0D
+ if (CompareMem (Entry->Data, Hash, EntrySize) =3D=3D 0) {=0D
+ Status =3D EFI_SUCCESS;=0D
+ DEBUG ((DEBUG_INFO, "%a: Hash comparison succeeded for \"%s\"\n",=0D
+ __FUNCTION__, BlobName));=0D
+ } else {=0D
+ Status =3D EFI_ACCESS_DENIED;=0D
+ DEBUG ((DEBUG_ERROR, "%a: Hash comparison failed for \"%s\"\n",=0D
+ __FUNCTION__, BlobName));=0D
+ }=0D
+ return Status;=0D
+ }=0D
+=0D
+ DEBUG ((DEBUG_ERROR, "%a: Hash GUID %g not found in table\n", __FUNCTION=
__,=0D
+ Guid));=0D
+ return EFI_ACCESS_DENIED;=0D
+}=0D
+=0D
+/**=0D
+ Locate the SEV hashes table.=0D
+=0D
+ This function always returns success, even if the table can't be found. =
The=0D
+ subsequent VerifyBlob calls will fail if no table was found.=0D
+=0D
+ @retval RETURN_SUCCESS The hashes table is set up correctly, or there =
is no=0D
+ hashes table=0D
+**/=0D
+RETURN_STATUS=0D
+EFIAPI=0D
+BlobVerifierLibSevHashesConstructor (=0D
+ VOID=0D
+ )=0D
+{=0D
+ HASH_TABLE *Ptr =3D (void *)(UINTN)FixedPcdGet64 (PcdQemuHashTableBase);=
=0D
+ UINT32 Size =3D FixedPcdGet32 (PcdQemuHashTableSize);=0D
+=0D
+ mHashesTable =3D NULL;=0D
+ mHashesTableSize =3D 0;=0D
+=0D
+ if (Ptr =3D=3D NULL || Size < sizeof *Ptr ||=0D
+ !CompareGuid (&Ptr->Guid, &SEV_HASH_TABLE_GUID) ||=0D
+ Ptr->Len < sizeof *Ptr || Ptr->Len > Size) {=0D
+ return RETURN_SUCCESS;=0D
+ }=0D
+=0D
+ DEBUG ((DEBUG_INFO, "%a: Found injected hashes table in secure location\=
n",=0D
+ __FUNCTION__));=0D
+=0D
+ mHashesTable =3D (HASH_TABLE *)Ptr->Data;=0D
+ mHashesTableSize =3D Ptr->Len - sizeof Ptr->Guid - sizeof Ptr->Len;=0D
+=0D
+ DEBUG ((DEBUG_VERBOSE, "%a: mHashesTable=3D0x%p, Size=3D%u\n", __FUNCTIO=
N__,=0D
+ mHashesTable, mHashesTableSize));=0D
+=0D
+ return RETURN_SUCCESS;=0D
+}=0D
--=20
2.25.1


Dov Murik
 

Here's the diff from the v3 of this patch. It's supposed to catch
more cases of bad length fields overflowing the reserved MEMFD space or
the declared length of the table.



diff --git a/OvmfPkg/Library/BlobVerifierLib/BlobVerifierSevHashes.c
b/OvmfPkg/Library/BlobVerifierLib/BlobVerifierSevHashes.c
index 797d63d18067..372ae6f469e7 100644
--- a/OvmfPkg/Library/BlobVerifierLib/BlobVerifierSevHashes.c
+++ b/OvmfPkg/Library/BlobVerifierLib/BlobVerifierSevHashes.c
@@ -94,7 +94,7 @@ VerifyBlob (
)
{
CONST GUID *Guid;
- INT32 Len;
+ INT32 Remaining;
HASH_TABLE *Entry;

if (mHashesTable == NULL || mHashesTableSize == 0) {
@@ -111,9 +111,13 @@ VerifyBlob (
return EFI_ACCESS_DENIED;
}

- for (Entry = mHashesTable, Len = 0;
- Len < (INT32)mHashesTableSize;
- Len += Entry->Len,
+ //
+ // Remaining is INT32 to catch underflow in case Entry->Len has a
+ // very high UINT16 value
+ //
+ for (Entry = mHashesTable, Remaining = mHashesTableSize;
+ Remaining >= sizeof *Entry && Remaining >= Entry->Len;
+ Remaining -= Entry->Len,
Entry = (HASH_TABLE *)((UINT8 *)Entry + Entry->Len)) {
UINTN EntrySize;
EFI_STATUS Status;
@@ -125,7 +129,7 @@ VerifyBlob (

DEBUG ((DEBUG_INFO, "%a: Found GUID %g in table\n", __FUNCTION__,
Guid));

- EntrySize = Entry->Len - sizeof (Entry->Guid) - sizeof (Entry->Len);
+ EntrySize = Entry->Len - sizeof Entry->Guid - sizeof Entry->Len;
if (EntrySize != SHA256_DIGEST_SIZE) {
DEBUG ((DEBUG_ERROR, "%a: Hash has the wrong size %d != %d\n",
__FUNCTION__, EntrySize, SHA256_DIGEST_SIZE));
@@ -161,7 +165,8 @@ VerifyBlob (
This function always returns success, even if the table can't be
found. The
subsequent VerifyBlob calls will fail if no table was found.

- @retval RETURN_SUCCESS The verifier tables were set up correctly
+ @retval RETURN_SUCCESS The hashes table is set up correctly, or
there is no
+ hashes table
**/
RETURN_STATUS
EFIAPI
@@ -175,15 +180,9 @@ BlobVerifierLibSevHashesConstructor (
mHashesTable = NULL;
mHashesTableSize = 0;

- if (Ptr == NULL || Size == 0) {
- return RETURN_SUCCESS;
- }
-
- if (!CompareGuid (&Ptr->Guid, &SEV_HASH_TABLE_GUID)) {
- return RETURN_SUCCESS;
- }
-
- if (Ptr->Len < (sizeof Ptr->Guid + sizeof Ptr->Len)) {
+ if (Ptr == NULL || Size < sizeof *Ptr ||
+ !CompareGuid (&Ptr->Guid, &SEV_HASH_TABLE_GUID) ||
+ Ptr->Len < sizeof *Ptr || Ptr->Len > Size) {
return RETURN_SUCCESS;
}

On 22/07/2021 11:43, Dov Murik wrote:
Add an implementation for BlobVerifierLib that locates the SEV hashes
table and verifies that the calculated hashes of the kernel, initrd, and
cmdline blobs indeed match the expected hashes stated in the hashes
table.

If there's a missing hash or a hash mismatch then EFI_ACCESS_DENIED is
returned which will cause a failure to load a kernel image.

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
Co-developed-by: James Bottomley <jejb@linux.ibm.com>
Signed-off-by: James Bottomley <jejb@linux.ibm.com>
Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
---
OvmfPkg/Library/BlobVerifierLib/BlobVerifierLibSevHashes.inf | 37 ++++
OvmfPkg/Library/BlobVerifierLib/BlobVerifierSevHashes.c | 199 ++++++++++++++++++++
2 files changed, 236 insertions(+)

diff --git a/OvmfPkg/Library/BlobVerifierLib/BlobVerifierLibSevHashes.inf b/OvmfPkg/Library/BlobVerifierLib/BlobVerifierLibSevHashes.inf
new file mode 100644
index 000000000000..76ca0b8154ce
--- /dev/null
+++ b/OvmfPkg/Library/BlobVerifierLib/BlobVerifierLibSevHashes.inf
@@ -0,0 +1,37 @@
+## @file

+#

+# Blob verifier library that uses SEV hashes table. The hashes table holds the

+# allowed hashes of the kernel, initrd, and cmdline blobs.

+#

+# Copyright (C) 2021, IBM Corp

+#

+# SPDX-License-Identifier: BSD-2-Clause-Patent

+#

+##

+

+[Defines]

+ INF_VERSION = 1.29

+ BASE_NAME = BlobVerifierLibSevHashes

+ FILE_GUID = 59e713b5-eff3-46a7-8d8b-46f4c004ad7b

+ MODULE_TYPE = BASE

+ VERSION_STRING = 1.0

+ LIBRARY_CLASS = BlobVerifierLib

+ CONSTRUCTOR = BlobVerifierLibSevHashesConstructor

+

+[Sources]

+ BlobVerifierSevHashes.c

+

+[Packages]

+ CryptoPkg/CryptoPkg.dec

+ MdePkg/MdePkg.dec

+ OvmfPkg/OvmfPkg.dec

+

+[LibraryClasses]

+ BaseCryptLib

+ BaseMemoryLib

+ DebugLib

+ PcdLib

+

+[FixedPcd]

+ gUefiOvmfPkgTokenSpaceGuid.PcdQemuHashTableBase

+ gUefiOvmfPkgTokenSpaceGuid.PcdQemuHashTableSize

diff --git a/OvmfPkg/Library/BlobVerifierLib/BlobVerifierSevHashes.c b/OvmfPkg/Library/BlobVerifierLib/BlobVerifierSevHashes.c
new file mode 100644
index 000000000000..372ae6f469e7
--- /dev/null
+++ b/OvmfPkg/Library/BlobVerifierLib/BlobVerifierSevHashes.c
@@ -0,0 +1,199 @@
+/** @file

+

+ Blob verifier library that uses SEV hashes table. The hashes table holds the

+ allowed hashes of the kernel, initrd, and cmdline blobs.

+

+ Copyright (C) 2021, IBM Corporation

+

+ SPDX-License-Identifier: BSD-2-Clause-Patent

+**/

+

+#include <Library/BaseCryptLib.h>

+#include <Library/BaseLib.h>

+#include <Library/BaseMemoryLib.h>

+#include <Library/DebugLib.h>

+#include <Library/BlobVerifierLib.h>

+

+/**

+ The SEV Hashes table must be in encrypted memory and has the table

+ and its entries described by

+

+ <GUID>|UINT16 <len>|<data>

+

+ With the whole table GUID being 9438d606-4f22-4cc9-b479-a793d411fd21

+

+ The current possible table entries are for the kernel, the initrd

+ and the cmdline:

+

+ 4de79437-abd2-427f-b835-d5b172d2045b kernel

+ 44baf731-3a2f-4bd7-9af1-41e29169781d initrd

+ 97d02dd8-bd20-4c94-aa78-e7714d36ab2a cmdline

+

+ The size of the entry is used to identify the hash, but the

+ expectation is that it will be 32 bytes of SHA-256.

+**/

+

+#define SEV_HASH_TABLE_GUID \

+ (GUID) { 0x9438d606, 0x4f22, 0x4cc9, { 0xb4, 0x79, 0xa7, 0x93, 0xd4, 0x11, 0xfd, 0x21 } }

+#define SEV_KERNEL_HASH_GUID \

+ (GUID) { 0x4de79437, 0xabd2, 0x427f, { 0xb8, 0x35, 0xd5, 0xb1, 0x72, 0xd2, 0x04, 0x5b } }

+#define SEV_INITRD_HASH_GUID \

+ (GUID) { 0x44baf731, 0x3a2f, 0x4bd7, { 0x9a, 0xf1, 0x41, 0xe2, 0x91, 0x69, 0x78, 0x1d } }

+#define SEV_CMDLINE_HASH_GUID \

+ (GUID) { 0x97d02dd8, 0xbd20, 0x4c94, { 0xaa, 0x78, 0xe7, 0x71, 0x4d, 0x36, 0xab, 0x2a } }

+

+STATIC CONST EFI_GUID mSevKernelHashGuid = SEV_KERNEL_HASH_GUID;

+STATIC CONST EFI_GUID mSevInitrdHashGuid = SEV_INITRD_HASH_GUID;

+STATIC CONST EFI_GUID mSevCmdlineHashGuid = SEV_CMDLINE_HASH_GUID;

+

+#pragma pack (1)

+typedef struct {

+ GUID Guid;

+ UINT16 Len;

+ UINT8 Data[];

+} HASH_TABLE;

+#pragma pack ()

+

+STATIC HASH_TABLE *mHashesTable;

+STATIC UINT16 mHashesTableSize;

+

+STATIC

+CONST GUID*

+FindBlobEntryGuid (

+ IN CONST CHAR16 *BlobName

+ )

+{

+ if (StrCmp (BlobName, L"kernel") == 0) {

+ return &mSevKernelHashGuid;

+ } else if (StrCmp (BlobName, L"initrd") == 0) {

+ return &mSevInitrdHashGuid;

+ } else if (StrCmp (BlobName, L"cmdline") == 0) {

+ return &mSevCmdlineHashGuid;

+ } else {

+ return NULL;

+ }

+}

+

+/**

+ Verify blob from an external source.

+

+ @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.

+**/

+EFI_STATUS

+EFIAPI

+VerifyBlob (

+ IN CONST CHAR16 *BlobName,

+ IN CONST VOID *Buf,

+ IN UINT32 BufSize

+ )

+{

+ CONST GUID *Guid;

+ INT32 Remaining;

+ HASH_TABLE *Entry;

+

+ if (mHashesTable == NULL || mHashesTableSize == 0) {

+ DEBUG ((DEBUG_ERROR,

+ "%a: Verifier called but no hashes table discoverd in MEMFD\n",

+ __FUNCTION__));

+ return EFI_ACCESS_DENIED;

+ }

+

+ Guid = FindBlobEntryGuid (BlobName);

+ if (Guid == NULL) {

+ DEBUG ((DEBUG_ERROR, "%a: Unknown blob name \"%s\"\n", __FUNCTION__,

+ BlobName));

+ return EFI_ACCESS_DENIED;

+ }

+

+ //

+ // Remaining is INT32 to catch underflow in case Entry->Len has a

+ // very high UINT16 value

+ //

+ for (Entry = mHashesTable, Remaining = mHashesTableSize;

+ Remaining >= sizeof *Entry && Remaining >= Entry->Len;

+ Remaining -= Entry->Len,

+ Entry = (HASH_TABLE *)((UINT8 *)Entry + Entry->Len)) {

+ UINTN EntrySize;

+ EFI_STATUS Status;

+ UINT8 Hash[SHA256_DIGEST_SIZE];

+

+ if (!CompareGuid (&Entry->Guid, Guid)) {

+ continue;

+ }

+

+ DEBUG ((DEBUG_INFO, "%a: Found GUID %g in table\n", __FUNCTION__, Guid));

+

+ EntrySize = Entry->Len - sizeof Entry->Guid - sizeof Entry->Len;

+ if (EntrySize != SHA256_DIGEST_SIZE) {

+ DEBUG ((DEBUG_ERROR, "%a: Hash has the wrong size %d != %d\n",

+ __FUNCTION__, EntrySize, SHA256_DIGEST_SIZE));

+ return EFI_ACCESS_DENIED;

+ }

+

+ //

+ // Calculate the buffer's hash and verify that it is identical to the

+ // expected hash table entry

+ //

+ Sha256HashAll (Buf, BufSize, Hash);

+

+ if (CompareMem (Entry->Data, Hash, EntrySize) == 0) {

+ Status = EFI_SUCCESS;

+ DEBUG ((DEBUG_INFO, "%a: Hash comparison succeeded for \"%s\"\n",

+ __FUNCTION__, BlobName));

+ } else {

+ Status = EFI_ACCESS_DENIED;

+ DEBUG ((DEBUG_ERROR, "%a: Hash comparison failed for \"%s\"\n",

+ __FUNCTION__, BlobName));

+ }

+ return Status;

+ }

+

+ DEBUG ((DEBUG_ERROR, "%a: Hash GUID %g not found in table\n", __FUNCTION__,

+ Guid));

+ return EFI_ACCESS_DENIED;

+}

+

+/**

+ Locate the SEV hashes table.

+

+ This function always returns success, even if the table can't be found. The

+ subsequent VerifyBlob calls will fail if no table was found.

+

+ @retval RETURN_SUCCESS The hashes table is set up correctly, or there is no

+ hashes table

+**/

+RETURN_STATUS

+EFIAPI

+BlobVerifierLibSevHashesConstructor (

+ VOID

+ )

+{

+ HASH_TABLE *Ptr = (void *)(UINTN)FixedPcdGet64 (PcdQemuHashTableBase);

+ UINT32 Size = FixedPcdGet32 (PcdQemuHashTableSize);

+

+ mHashesTable = NULL;

+ mHashesTableSize = 0;

+

+ if (Ptr == NULL || Size < sizeof *Ptr ||

+ !CompareGuid (&Ptr->Guid, &SEV_HASH_TABLE_GUID) ||

+ Ptr->Len < sizeof *Ptr || Ptr->Len > Size) {

+ return RETURN_SUCCESS;

+ }

+

+ DEBUG ((DEBUG_INFO, "%a: Found injected hashes table in secure location\n",

+ __FUNCTION__));

+

+ mHashesTable = (HASH_TABLE *)Ptr->Data;

+ mHashesTableSize = Ptr->Len - sizeof Ptr->Guid - sizeof Ptr->Len;

+

+ DEBUG ((DEBUG_VERBOSE, "%a: mHashesTable=0x%p, Size=%u\n", __FUNCTION__,

+ mHashesTable, mHashesTableSize));

+

+ return RETURN_SUCCESS;

+}


Yao, Jiewen
 

Hi Dov
If this library is only needed by AmdSev build, I feel it should be in AmdSev dir, instead of Ovmf dir. (See my question in previous email).


Thank you
Yao Jiewen

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Dov Murik
Sent: Thursday, July 22, 2021 4:45 PM
To: devel@edk2.groups.io
Cc: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>; Tobin Feldman-Fitzthum
<tobin@ibm.com>; Jim Cadden <jcadden@ibm.com>; James Bottomley
<jejb@linux.ibm.com>; Hubertus Franke <frankeh@us.ibm.com>; Ard Biesheuvel
<ardb+tianocore@kernel.org>; Justen, Jordan L <jordan.l.justen@intel.com>;
Ashish Kalra <ashish.kalra@amd.com>; Brijesh Singh <brijesh.singh@amd.com>;
Erdem Aktas <erdemaktas@google.com>; Yao, Jiewen <jiewen.yao@intel.com>;
Xu, Min M <min.m.xu@intel.com>; Tom Lendacky
<thomas.lendacky@amd.com>; Dov Murik <dovmurik@linux.ibm.com>
Subject: Re: [edk2-devel] [PATCH v4 10/11] OvmfPkg: add
BlobVerifierLibSevHashes


Here's the diff from the v3 of this patch. It's supposed to catch
more cases of bad length fields overflowing the reserved MEMFD space or
the declared length of the table.



diff --git a/OvmfPkg/Library/BlobVerifierLib/BlobVerifierSevHashes.c
b/OvmfPkg/Library/BlobVerifierLib/BlobVerifierSevHashes.c
index 797d63d18067..372ae6f469e7 100644
--- a/OvmfPkg/Library/BlobVerifierLib/BlobVerifierSevHashes.c
+++ b/OvmfPkg/Library/BlobVerifierLib/BlobVerifierSevHashes.c
@@ -94,7 +94,7 @@ VerifyBlob (
)
{
CONST GUID *Guid;
- INT32 Len;
+ INT32 Remaining;
HASH_TABLE *Entry;

if (mHashesTable == NULL || mHashesTableSize == 0) {
@@ -111,9 +111,13 @@ VerifyBlob (
return EFI_ACCESS_DENIED;
}

- for (Entry = mHashesTable, Len = 0;
- Len < (INT32)mHashesTableSize;
- Len += Entry->Len,
+ //
+ // Remaining is INT32 to catch underflow in case Entry->Len has a
+ // very high UINT16 value
+ //
+ for (Entry = mHashesTable, Remaining = mHashesTableSize;
+ Remaining >= sizeof *Entry && Remaining >= Entry->Len;
+ Remaining -= Entry->Len,
Entry = (HASH_TABLE *)((UINT8 *)Entry + Entry->Len)) {
UINTN EntrySize;
EFI_STATUS Status;
@@ -125,7 +129,7 @@ VerifyBlob (

DEBUG ((DEBUG_INFO, "%a: Found GUID %g in table\n", __FUNCTION__,
Guid));

- EntrySize = Entry->Len - sizeof (Entry->Guid) - sizeof (Entry->Len);
+ EntrySize = Entry->Len - sizeof Entry->Guid - sizeof Entry->Len;
if (EntrySize != SHA256_DIGEST_SIZE) {
DEBUG ((DEBUG_ERROR, "%a: Hash has the wrong size %d != %d\n",
__FUNCTION__, EntrySize, SHA256_DIGEST_SIZE));
@@ -161,7 +165,8 @@ VerifyBlob (
This function always returns success, even if the table can't be
found. The
subsequent VerifyBlob calls will fail if no table was found.

- @retval RETURN_SUCCESS The verifier tables were set up correctly
+ @retval RETURN_SUCCESS The hashes table is set up correctly, or
there is no
+ hashes table
**/
RETURN_STATUS
EFIAPI
@@ -175,15 +180,9 @@ BlobVerifierLibSevHashesConstructor (
mHashesTable = NULL;
mHashesTableSize = 0;

- if (Ptr == NULL || Size == 0) {
- return RETURN_SUCCESS;
- }
-
- if (!CompareGuid (&Ptr->Guid, &SEV_HASH_TABLE_GUID)) {
- return RETURN_SUCCESS;
- }
-
- if (Ptr->Len < (sizeof Ptr->Guid + sizeof Ptr->Len)) {
+ if (Ptr == NULL || Size < sizeof *Ptr ||
+ !CompareGuid (&Ptr->Guid, &SEV_HASH_TABLE_GUID) ||
+ Ptr->Len < sizeof *Ptr || Ptr->Len > Size) {
return RETURN_SUCCESS;
}





On 22/07/2021 11:43, Dov Murik wrote:
Add an implementation for BlobVerifierLib that locates the SEV hashes
table and verifies that the calculated hashes of the kernel, initrd, and
cmdline blobs indeed match the expected hashes stated in the hashes
table.

If there's a missing hash or a hash mismatch then EFI_ACCESS_DENIED is
returned which will cause a failure to load a kernel image.

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
Co-developed-by: James Bottomley <jejb@linux.ibm.com>
Signed-off-by: James Bottomley <jejb@linux.ibm.com>
Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
---
OvmfPkg/Library/BlobVerifierLib/BlobVerifierLibSevHashes.inf | 37 ++++
OvmfPkg/Library/BlobVerifierLib/BlobVerifierSevHashes.c | 199
++++++++++++++++++++
2 files changed, 236 insertions(+)

diff --git a/OvmfPkg/Library/BlobVerifierLib/BlobVerifierLibSevHashes.inf
b/OvmfPkg/Library/BlobVerifierLib/BlobVerifierLibSevHashes.inf
new file mode 100644
index 000000000000..76ca0b8154ce
--- /dev/null
+++ b/OvmfPkg/Library/BlobVerifierLib/BlobVerifierLibSevHashes.inf
@@ -0,0 +1,37 @@
+## @file

+#

+# Blob verifier library that uses SEV hashes table. The hashes table holds the

+# allowed hashes of the kernel, initrd, and cmdline blobs.

+#

+# Copyright (C) 2021, IBM Corp

+#

+# SPDX-License-Identifier: BSD-2-Clause-Patent

+#

+##

+

+[Defines]

+ INF_VERSION = 1.29

+ BASE_NAME = BlobVerifierLibSevHashes

+ FILE_GUID = 59e713b5-eff3-46a7-8d8b-46f4c004ad7b

+ MODULE_TYPE = BASE

+ VERSION_STRING = 1.0

+ LIBRARY_CLASS = BlobVerifierLib

+ CONSTRUCTOR = BlobVerifierLibSevHashesConstructor

+

+[Sources]

+ BlobVerifierSevHashes.c

+

+[Packages]

+ CryptoPkg/CryptoPkg.dec

+ MdePkg/MdePkg.dec

+ OvmfPkg/OvmfPkg.dec

+

+[LibraryClasses]

+ BaseCryptLib

+ BaseMemoryLib

+ DebugLib

+ PcdLib

+

+[FixedPcd]

+ gUefiOvmfPkgTokenSpaceGuid.PcdQemuHashTableBase

+ gUefiOvmfPkgTokenSpaceGuid.PcdQemuHashTableSize

diff --git a/OvmfPkg/Library/BlobVerifierLib/BlobVerifierSevHashes.c
b/OvmfPkg/Library/BlobVerifierLib/BlobVerifierSevHashes.c
new file mode 100644
index 000000000000..372ae6f469e7
--- /dev/null
+++ b/OvmfPkg/Library/BlobVerifierLib/BlobVerifierSevHashes.c
@@ -0,0 +1,199 @@
+/** @file

+

+ Blob verifier library that uses SEV hashes table. The hashes table holds the

+ allowed hashes of the kernel, initrd, and cmdline blobs.

+

+ Copyright (C) 2021, IBM Corporation

+

+ SPDX-License-Identifier: BSD-2-Clause-Patent

+**/

+

+#include <Library/BaseCryptLib.h>

+#include <Library/BaseLib.h>

+#include <Library/BaseMemoryLib.h>

+#include <Library/DebugLib.h>

+#include <Library/BlobVerifierLib.h>

+

+/**

+ The SEV Hashes table must be in encrypted memory and has the table

+ and its entries described by

+

+ <GUID>|UINT16 <len>|<data>

+

+ With the whole table GUID being 9438d606-4f22-4cc9-b479-a793d411fd21

+

+ The current possible table entries are for the kernel, the initrd

+ and the cmdline:

+

+ 4de79437-abd2-427f-b835-d5b172d2045b kernel

+ 44baf731-3a2f-4bd7-9af1-41e29169781d initrd

+ 97d02dd8-bd20-4c94-aa78-e7714d36ab2a cmdline

+

+ The size of the entry is used to identify the hash, but the

+ expectation is that it will be 32 bytes of SHA-256.

+**/

+

+#define SEV_HASH_TABLE_GUID \

+ (GUID) { 0x9438d606, 0x4f22, 0x4cc9, { 0xb4, 0x79, 0xa7, 0x93, 0xd4, 0x11,
0xfd, 0x21 } }

+#define SEV_KERNEL_HASH_GUID \

+ (GUID) { 0x4de79437, 0xabd2, 0x427f, { 0xb8, 0x35, 0xd5, 0xb1, 0x72, 0xd2,
0x04, 0x5b } }

+#define SEV_INITRD_HASH_GUID \

+ (GUID) { 0x44baf731, 0x3a2f, 0x4bd7, { 0x9a, 0xf1, 0x41, 0xe2, 0x91, 0x69,
0x78, 0x1d } }

+#define SEV_CMDLINE_HASH_GUID \

+ (GUID) { 0x97d02dd8, 0xbd20, 0x4c94, { 0xaa, 0x78, 0xe7, 0x71, 0x4d, 0x36,
0xab, 0x2a } }

+

+STATIC CONST EFI_GUID mSevKernelHashGuid = SEV_KERNEL_HASH_GUID;

+STATIC CONST EFI_GUID mSevInitrdHashGuid = SEV_INITRD_HASH_GUID;

+STATIC CONST EFI_GUID mSevCmdlineHashGuid =
SEV_CMDLINE_HASH_GUID;

+

+#pragma pack (1)

+typedef struct {

+ GUID Guid;

+ UINT16 Len;

+ UINT8 Data[];

+} HASH_TABLE;

+#pragma pack ()

+

+STATIC HASH_TABLE *mHashesTable;

+STATIC UINT16 mHashesTableSize;

+

+STATIC

+CONST GUID*

+FindBlobEntryGuid (

+ IN CONST CHAR16 *BlobName

+ )

+{

+ if (StrCmp (BlobName, L"kernel") == 0) {

+ return &mSevKernelHashGuid;

+ } else if (StrCmp (BlobName, L"initrd") == 0) {

+ return &mSevInitrdHashGuid;

+ } else if (StrCmp (BlobName, L"cmdline") == 0) {

+ return &mSevCmdlineHashGuid;

+ } else {

+ return NULL;

+ }

+}

+

+/**

+ Verify blob from an external source.

+

+ @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.

+**/

+EFI_STATUS

+EFIAPI

+VerifyBlob (

+ IN CONST CHAR16 *BlobName,

+ IN CONST VOID *Buf,

+ IN UINT32 BufSize

+ )

+{

+ CONST GUID *Guid;

+ INT32 Remaining;

+ HASH_TABLE *Entry;

+

+ if (mHashesTable == NULL || mHashesTableSize == 0) {

+ DEBUG ((DEBUG_ERROR,

+ "%a: Verifier called but no hashes table discoverd in MEMFD\n",

+ __FUNCTION__));

+ return EFI_ACCESS_DENIED;

+ }

+

+ Guid = FindBlobEntryGuid (BlobName);

+ if (Guid == NULL) {

+ DEBUG ((DEBUG_ERROR, "%a: Unknown blob name \"%s\"\n",
__FUNCTION__,

+ BlobName));

+ return EFI_ACCESS_DENIED;

+ }

+

+ //

+ // Remaining is INT32 to catch underflow in case Entry->Len has a

+ // very high UINT16 value

+ //

+ for (Entry = mHashesTable, Remaining = mHashesTableSize;

+ Remaining >= sizeof *Entry && Remaining >= Entry->Len;

+ Remaining -= Entry->Len,

+ Entry = (HASH_TABLE *)((UINT8 *)Entry + Entry->Len)) {

+ UINTN EntrySize;

+ EFI_STATUS Status;

+ UINT8 Hash[SHA256_DIGEST_SIZE];

+

+ if (!CompareGuid (&Entry->Guid, Guid)) {

+ continue;

+ }

+

+ DEBUG ((DEBUG_INFO, "%a: Found GUID %g in table\n", __FUNCTION__,
Guid));

+

+ EntrySize = Entry->Len - sizeof Entry->Guid - sizeof Entry->Len;

+ if (EntrySize != SHA256_DIGEST_SIZE) {

+ DEBUG ((DEBUG_ERROR, "%a: Hash has the wrong size %d != %d\n",

+ __FUNCTION__, EntrySize, SHA256_DIGEST_SIZE));

+ return EFI_ACCESS_DENIED;

+ }

+

+ //

+ // Calculate the buffer's hash and verify that it is identical to the

+ // expected hash table entry

+ //

+ Sha256HashAll (Buf, BufSize, Hash);

+

+ if (CompareMem (Entry->Data, Hash, EntrySize) == 0) {

+ Status = EFI_SUCCESS;

+ DEBUG ((DEBUG_INFO, "%a: Hash comparison succeeded for \"%s\"\n",

+ __FUNCTION__, BlobName));

+ } else {

+ Status = EFI_ACCESS_DENIED;

+ DEBUG ((DEBUG_ERROR, "%a: Hash comparison failed for \"%s\"\n",

+ __FUNCTION__, BlobName));

+ }

+ return Status;

+ }

+

+ DEBUG ((DEBUG_ERROR, "%a: Hash GUID %g not found in table\n",
__FUNCTION__,

+ Guid));

+ return EFI_ACCESS_DENIED;

+}

+

+/**

+ Locate the SEV hashes table.

+

+ This function always returns success, even if the table can't be found. The

+ subsequent VerifyBlob calls will fail if no table was found.

+

+ @retval RETURN_SUCCESS The hashes table is set up correctly, or there is
no

+ hashes table

+**/

+RETURN_STATUS

+EFIAPI

+BlobVerifierLibSevHashesConstructor (

+ VOID

+ )

+{

+ HASH_TABLE *Ptr = (void *)(UINTN)FixedPcdGet64
(PcdQemuHashTableBase);

+ UINT32 Size = FixedPcdGet32 (PcdQemuHashTableSize);

+

+ mHashesTable = NULL;

+ mHashesTableSize = 0;

+

+ if (Ptr == NULL || Size < sizeof *Ptr ||

+ !CompareGuid (&Ptr->Guid, &SEV_HASH_TABLE_GUID) ||

+ Ptr->Len < sizeof *Ptr || Ptr->Len > Size) {

+ return RETURN_SUCCESS;

+ }

+

+ DEBUG ((DEBUG_INFO, "%a: Found injected hashes table in secure
location\n",

+ __FUNCTION__));

+

+ mHashesTable = (HASH_TABLE *)Ptr->Data;

+ mHashesTableSize = Ptr->Len - sizeof Ptr->Guid - sizeof Ptr->Len;

+

+ DEBUG ((DEBUG_VERBOSE, "%a: mHashesTable=0x%p, Size=%u\n",
__FUNCTION__,

+ mHashesTable, mHashesTableSize));

+

+ return RETURN_SUCCESS;

+}