public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Min Xu" <min.m.xu@intel.com>
To: devel@edk2.groups.io
Cc: Min M Xu <min.m.xu@intel.com>,
	Erdem Aktas <erdemaktas@google.com>,
	James Bottomley <jejb@linux.ibm.com>,
	Jiewen Yao <jiewen.yao@intel.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	Tom Lendacky <thomas.lendacky@amd.com>
Subject: [PATCH 1/3] OvmfPkg: Move TdxValidateCfv from PeilessStartupLib to PlatformInitLib
Date: Sat, 18 Jun 2022 10:32:01 +0800	[thread overview]
Message-ID: <3b3dce962500443d1281db91fd5e0ba2619aaf15.1655518585.git.min.m.xu@intel.com> (raw)
In-Reply-To: <cover.1655518585.git.min.m.xu@intel.com>

From: Min M Xu <min.m.xu@intel.com>

TdxValidateCfv validates the integrity of Configuration FV (CFV). It was
implemented in PeilessStartupLib which is included in IntelTdxX64.

In OvmfPkgX64 we should validate CFV as well. So it is moved from
PeilessStartupLib to PlatformInitLib so that it can be called in both
OvmfPkgX64 and IntelTdxX64.

Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/Include/Library/PlatformInitLib.h     |  17 ++
 OvmfPkg/Library/PeilessStartupLib/IntelTdx.c  | 153 ------------------
 .../PeilessStartupInternal.h                  |  17 --
 OvmfPkg/Library/PlatformInitLib/IntelTdx.c    | 153 ++++++++++++++++++
 4 files changed, 170 insertions(+), 170 deletions(-)

diff --git a/OvmfPkg/Include/Library/PlatformInitLib.h b/OvmfPkg/Include/Library/PlatformInitLib.h
index 2987a367cc9c..a3acfb1fb196 100644
--- a/OvmfPkg/Include/Library/PlatformInitLib.h
+++ b/OvmfPkg/Include/Library/PlatformInitLib.h
@@ -234,4 +234,21 @@ PlatformTdxPublishRamRegions (
   VOID
   );
 
+/**
+  Check the integrity of CFV data.
+
+  @param[in] TdxCfvBase - A pointer to CFV header
+  @param[in] TdxCfvSize - CFV data size
+
+  @retval  TRUE   - The CFV data is valid.
+  @retval  FALSE  - The CFV data is invalid.
+
+**/
+BOOLEAN
+EFIAPI
+TdxValidateCfv (
+  IN UINT8   *TdxCfvBase,
+  IN UINT32  TdxCfvSize
+  );
+
 #endif // PLATFORM_INIT_LIB_H_
diff --git a/OvmfPkg/Library/PeilessStartupLib/IntelTdx.c b/OvmfPkg/Library/PeilessStartupLib/IntelTdx.c
index 484fd21057c8..216c413caad5 100644
--- a/OvmfPkg/Library/PeilessStartupLib/IntelTdx.c
+++ b/OvmfPkg/Library/PeilessStartupLib/IntelTdx.c
@@ -7,8 +7,6 @@
 #include <Library/BaseLib.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/DebugLib.h>
-#include <Guid/VariableFormat.h>
-#include <Guid/SystemNvDataGuid.h>
 #include <IndustryStandard/Tpm20.h>
 #include <IndustryStandard/UefiTcgPlatform.h>
 #include <Library/HobLib.h>
@@ -37,157 +35,6 @@ typedef struct {
 
 #pragma pack()
 
-/**
-  Check padding data all bit should be 1.
-
-  @param[in] Buffer     - A pointer to buffer header
-  @param[in] BufferSize - Buffer size
-
-  @retval  TRUE   - The padding data is valid.
-  @retval  TRUE  - The padding data is invalid.
-
-**/
-BOOLEAN
-CheckPaddingData (
-  IN UINT8   *Buffer,
-  IN UINT32  BufferSize
-  )
-{
-  UINT32  index;
-
-  for (index = 0; index < BufferSize; index++) {
-    if (Buffer[index] != 0xFF) {
-      return FALSE;
-    }
-  }
-
-  return TRUE;
-}
-
-/**
-  Check the integrity of CFV data.
-
-  @param[in] TdxCfvBase - A pointer to CFV header
-  @param[in] TdxCfvSize - CFV data size
-
-  @retval  TRUE   - The CFV data is valid.
-  @retval  FALSE  - The CFV data is invalid.
-
-**/
-BOOLEAN
-EFIAPI
-TdxValidateCfv (
-  IN UINT8   *TdxCfvBase,
-  IN UINT32  TdxCfvSize
-  )
-{
-  UINT16                         Checksum;
-  UINTN                          VariableBase;
-  UINT32                         VariableOffset;
-  UINT32                         VariableOffsetBeforeAlign;
-  EFI_FIRMWARE_VOLUME_HEADER     *CfvFvHeader;
-  VARIABLE_STORE_HEADER          *CfvVariableStoreHeader;
-  AUTHENTICATED_VARIABLE_HEADER  *VariableHeader;
-
-  static EFI_GUID  FvHdrGUID       = EFI_SYSTEM_NV_DATA_FV_GUID;
-  static EFI_GUID  VarStoreHdrGUID = EFI_AUTHENTICATED_VARIABLE_GUID;
-
-  VariableOffset = 0;
-
-  if (TdxCfvBase == NULL) {
-    DEBUG ((DEBUG_ERROR, "TDX CFV: CFV pointer is NULL\n"));
-    return FALSE;
-  }
-
-  //
-  // Verify the header zerovetor, filesystemguid,
-  // revision, signature, attributes, fvlength, checksum
-  // HeaderLength cannot be an odd number
-  //
-  CfvFvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)TdxCfvBase;
-
-  if ((!IsZeroBuffer (CfvFvHeader->ZeroVector, 16)) ||
-      (!CompareGuid (&FvHdrGUID, &CfvFvHeader->FileSystemGuid)) ||
-      (CfvFvHeader->Signature != EFI_FVH_SIGNATURE) ||
-      (CfvFvHeader->Attributes != 0x4feff) ||
-      (CfvFvHeader->Revision != EFI_FVH_REVISION) ||
-      (CfvFvHeader->FvLength != TdxCfvSize)
-      )
-  {
-    DEBUG ((DEBUG_ERROR, "TDX CFV: Basic FV headers were invalid\n"));
-    return FALSE;
-  }
-
-  //
-  // Verify the header checksum
-  //
-  Checksum = CalculateSum16 ((VOID *)CfvFvHeader, CfvFvHeader->HeaderLength);
-
-  if (Checksum != 0) {
-    DEBUG ((DEBUG_ERROR, "TDX CFV: FV checksum was invalid\n"));
-    return FALSE;
-  }
-
-  //
-  // Verify the header signature, size, format, state
-  //
-  CfvVariableStoreHeader = (VARIABLE_STORE_HEADER *)(TdxCfvBase + CfvFvHeader->HeaderLength);
-  if ((!CompareGuid (&VarStoreHdrGUID, &CfvVariableStoreHeader->Signature)) ||
-      (CfvVariableStoreHeader->Format != VARIABLE_STORE_FORMATTED) ||
-      (CfvVariableStoreHeader->State != VARIABLE_STORE_HEALTHY) ||
-      (CfvVariableStoreHeader->Size > (CfvFvHeader->FvLength - CfvFvHeader->HeaderLength)) ||
-      (CfvVariableStoreHeader->Size < sizeof (VARIABLE_STORE_HEADER))
-      )
-  {
-    DEBUG ((DEBUG_ERROR, "TDX CFV: Variable Store header was invalid\n"));
-    return FALSE;
-  }
-
-  //
-  // Verify the header startId, state
-  // Verify data to the end
-  //
-  VariableBase = (UINTN)TdxCfvBase + CfvFvHeader->HeaderLength + sizeof (VARIABLE_STORE_HEADER);
-  while (VariableOffset  < (CfvVariableStoreHeader->Size - sizeof (VARIABLE_STORE_HEADER))) {
-    VariableHeader = (AUTHENTICATED_VARIABLE_HEADER *)(VariableBase + VariableOffset);
-    if (VariableHeader->StartId != VARIABLE_DATA) {
-      if (!CheckPaddingData ((UINT8 *)VariableHeader, CfvVariableStoreHeader->Size - sizeof (VARIABLE_STORE_HEADER) - VariableOffset)) {
-        DEBUG ((DEBUG_ERROR, "TDX CFV: Variable header was invalid\n"));
-        return FALSE;
-      }
-
-      VariableOffset = CfvVariableStoreHeader->Size - sizeof (VARIABLE_STORE_HEADER);
-    } else {
-      if (!((VariableHeader->State == VAR_IN_DELETED_TRANSITION) ||
-            (VariableHeader->State == VAR_DELETED) ||
-            (VariableHeader->State == VAR_HEADER_VALID_ONLY) ||
-            (VariableHeader->State == VAR_ADDED)))
-      {
-        DEBUG ((DEBUG_ERROR, "TDX CFV: Variable header was invalid\n"));
-        return FALSE;
-      }
-
-      VariableOffset += sizeof (AUTHENTICATED_VARIABLE_HEADER) + VariableHeader->NameSize + VariableHeader->DataSize;
-      // Verify VariableOffset should be less than or equal CfvVariableStoreHeader->Size - sizeof(VARIABLE_STORE_HEADER)
-      if (VariableOffset > (CfvVariableStoreHeader->Size - sizeof (VARIABLE_STORE_HEADER))) {
-        DEBUG ((DEBUG_ERROR, "TDX CFV: Variable header was invalid\n"));
-        return FALSE;
-      }
-
-      VariableOffsetBeforeAlign = VariableOffset;
-      // 4 byte align
-      VariableOffset = (VariableOffset  + 3) & (UINTN)(~3);
-
-      if (!CheckPaddingData ((UINT8 *)(VariableBase + VariableOffsetBeforeAlign), VariableOffset - VariableOffsetBeforeAlign)) {
-        DEBUG ((DEBUG_ERROR, "TDX CFV: Variable header was invalid\n"));
-        return FALSE;
-      }
-    }
-  }
-
-  return TRUE;
-}
-
 /**
   Measure the Hoblist passed from the VMM.
 
diff --git a/OvmfPkg/Library/PeilessStartupLib/PeilessStartupInternal.h b/OvmfPkg/Library/PeilessStartupLib/PeilessStartupInternal.h
index 74b5f46552c2..09cac3e26c67 100644
--- a/OvmfPkg/Library/PeilessStartupLib/PeilessStartupInternal.h
+++ b/OvmfPkg/Library/PeilessStartupLib/PeilessStartupInternal.h
@@ -52,23 +52,6 @@ EFIAPI
 ConstructSecHobList (
   );
 
-/**
-  Check the integrity of CFV data.
-
-  @param[in] TdxCfvBase - A pointer to CFV header
-  @param[in] TdxCfvSize - CFV data size
-
-  @retval  TRUE   - The CFV data is valid.
-  @retval  FALSE  - The CFV data is invalid.
-
-**/
-BOOLEAN
-EFIAPI
-TdxValidateCfv (
-  IN UINT8   *TdxCfvBase,
-  IN UINT32  TdxCfvSize
-  );
-
 /**
   Measure the Hoblist passed from the VMM.
 
diff --git a/OvmfPkg/Library/PlatformInitLib/IntelTdx.c b/OvmfPkg/Library/PlatformInitLib/IntelTdx.c
index c6d7c8bb6e0e..626a670c9c64 100644
--- a/OvmfPkg/Library/PlatformInitLib/IntelTdx.c
+++ b/OvmfPkg/Library/PlatformInitLib/IntelTdx.c
@@ -20,6 +20,8 @@
 #include <Library/PeiServicesLib.h>
 #include <Library/TdxLib.h>
 #include <Library/SynchronizationLib.h>
+#include <Guid/SystemNvDataGuid.h>
+#include <Guid/VariableFormat.h>
 #include <WorkArea.h>
 #include <ConfidentialComputingGuestAttr.h>
 
@@ -561,3 +563,154 @@ PlatformTdxPublishRamRegions (
       );
   }
 }
+
+/**
+  Check padding data all bit should be 1.
+
+  @param[in] Buffer     - A pointer to buffer header
+  @param[in] BufferSize - Buffer size
+
+  @retval  TRUE   - The padding data is valid.
+  @retval  TRUE  - The padding data is invalid.
+
+**/
+BOOLEAN
+CheckPaddingData (
+  IN UINT8   *Buffer,
+  IN UINT32  BufferSize
+  )
+{
+  UINT32  index;
+
+  for (index = 0; index < BufferSize; index++) {
+    if (Buffer[index] != 0xFF) {
+      return FALSE;
+    }
+  }
+
+  return TRUE;
+}
+
+/**
+  Check the integrity of CFV data.
+
+  @param[in] TdxCfvBase - A pointer to CFV header
+  @param[in] TdxCfvSize - CFV data size
+
+  @retval  TRUE   - The CFV data is valid.
+  @retval  FALSE  - The CFV data is invalid.
+
+**/
+BOOLEAN
+EFIAPI
+TdxValidateCfv (
+  IN UINT8   *TdxCfvBase,
+  IN UINT32  TdxCfvSize
+  )
+{
+  UINT16                         Checksum;
+  UINTN                          VariableBase;
+  UINT32                         VariableOffset;
+  UINT32                         VariableOffsetBeforeAlign;
+  EFI_FIRMWARE_VOLUME_HEADER     *CfvFvHeader;
+  VARIABLE_STORE_HEADER          *CfvVariableStoreHeader;
+  AUTHENTICATED_VARIABLE_HEADER  *VariableHeader;
+
+  static EFI_GUID  FvHdrGUID       = EFI_SYSTEM_NV_DATA_FV_GUID;
+  static EFI_GUID  VarStoreHdrGUID = EFI_AUTHENTICATED_VARIABLE_GUID;
+
+  VariableOffset = 0;
+
+  if (TdxCfvBase == NULL) {
+    DEBUG ((DEBUG_ERROR, "TDX CFV: CFV pointer is NULL\n"));
+    return FALSE;
+  }
+
+  //
+  // Verify the header zerovetor, filesystemguid,
+  // revision, signature, attributes, fvlength, checksum
+  // HeaderLength cannot be an odd number
+  //
+  CfvFvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)TdxCfvBase;
+
+  if ((!IsZeroBuffer (CfvFvHeader->ZeroVector, 16)) ||
+      (!CompareGuid (&FvHdrGUID, &CfvFvHeader->FileSystemGuid)) ||
+      (CfvFvHeader->Signature != EFI_FVH_SIGNATURE) ||
+      (CfvFvHeader->Attributes != 0x4feff) ||
+      (CfvFvHeader->Revision != EFI_FVH_REVISION) ||
+      (CfvFvHeader->FvLength != TdxCfvSize)
+      )
+  {
+    DEBUG ((DEBUG_ERROR, "TDX CFV: Basic FV headers were invalid\n"));
+    return FALSE;
+  }
+
+  //
+  // Verify the header checksum
+  //
+  Checksum = CalculateSum16 ((VOID *)CfvFvHeader, CfvFvHeader->HeaderLength);
+
+  if (Checksum != 0) {
+    DEBUG ((DEBUG_ERROR, "TDX CFV: FV checksum was invalid\n"));
+    return FALSE;
+  }
+
+  //
+  // Verify the header signature, size, format, state
+  //
+  CfvVariableStoreHeader = (VARIABLE_STORE_HEADER *)(TdxCfvBase + CfvFvHeader->HeaderLength);
+  if ((!CompareGuid (&VarStoreHdrGUID, &CfvVariableStoreHeader->Signature)) ||
+      (CfvVariableStoreHeader->Format != VARIABLE_STORE_FORMATTED) ||
+      (CfvVariableStoreHeader->State != VARIABLE_STORE_HEALTHY) ||
+      (CfvVariableStoreHeader->Size > (CfvFvHeader->FvLength - CfvFvHeader->HeaderLength)) ||
+      (CfvVariableStoreHeader->Size < sizeof (VARIABLE_STORE_HEADER))
+      )
+  {
+    DEBUG ((DEBUG_ERROR, "TDX CFV: Variable Store header was invalid\n"));
+    return FALSE;
+  }
+
+  //
+  // Verify the header startId, state
+  // Verify data to the end
+  //
+  VariableBase = (UINTN)TdxCfvBase + CfvFvHeader->HeaderLength + sizeof (VARIABLE_STORE_HEADER);
+  while (VariableOffset  < (CfvVariableStoreHeader->Size - sizeof (VARIABLE_STORE_HEADER))) {
+    VariableHeader = (AUTHENTICATED_VARIABLE_HEADER *)(VariableBase + VariableOffset);
+    if (VariableHeader->StartId != VARIABLE_DATA) {
+      if (!CheckPaddingData ((UINT8 *)VariableHeader, CfvVariableStoreHeader->Size - sizeof (VARIABLE_STORE_HEADER) - VariableOffset)) {
+        DEBUG ((DEBUG_ERROR, "TDX CFV: Variable header was invalid\n"));
+        return FALSE;
+      }
+
+      VariableOffset = CfvVariableStoreHeader->Size - sizeof (VARIABLE_STORE_HEADER);
+    } else {
+      if (!((VariableHeader->State == VAR_IN_DELETED_TRANSITION) ||
+            (VariableHeader->State == VAR_DELETED) ||
+            (VariableHeader->State == VAR_HEADER_VALID_ONLY) ||
+            (VariableHeader->State == VAR_ADDED)))
+      {
+        DEBUG ((DEBUG_ERROR, "TDX CFV: Variable header was invalid\n"));
+        return FALSE;
+      }
+
+      VariableOffset += sizeof (AUTHENTICATED_VARIABLE_HEADER) + VariableHeader->NameSize + VariableHeader->DataSize;
+      // Verify VariableOffset should be less than or equal CfvVariableStoreHeader->Size - sizeof(VARIABLE_STORE_HEADER)
+      if (VariableOffset > (CfvVariableStoreHeader->Size - sizeof (VARIABLE_STORE_HEADER))) {
+        DEBUG ((DEBUG_ERROR, "TDX CFV: Variable header was invalid\n"));
+        return FALSE;
+      }
+
+      VariableOffsetBeforeAlign = VariableOffset;
+      // 4 byte align
+      VariableOffset = (VariableOffset  + 3) & (UINTN)(~3);
+
+      if (!CheckPaddingData ((UINT8 *)(VariableBase + VariableOffsetBeforeAlign), VariableOffset - VariableOffsetBeforeAlign)) {
+        DEBUG ((DEBUG_ERROR, "TDX CFV: Variable header was invalid\n"));
+        return FALSE;
+      }
+    }
+  }
+
+  return TRUE;
+}
-- 
2.29.2.windows.2


  reply	other threads:[~2022-06-18  2:32 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-18  2:32 [PATCH 0/3] Enable Secure-Boot in Tdx guest Min Xu
2022-06-18  2:32 ` Min Xu [this message]
2022-06-18  2:32 ` [PATCH 2/3] OvmfPkg: Validate Cfv integrity " Min Xu
2022-06-18  2:32 ` [PATCH 3/3] OvmfPkg: Initialize NvVarStore with Configuration FV in Td guest Min Xu
2022-06-20 11:01   ` Gerd Hoffmann
2022-06-22  2:02     ` Min Xu
2022-06-22  7:01       ` Gerd Hoffmann
2022-06-22  8:14         ` Min Xu
2022-06-22  9:22           ` Gerd Hoffmann
2022-06-23  0:40             ` Min Xu
2022-06-23  7:23               ` Gerd Hoffmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3b3dce962500443d1281db91fd5e0ba2619aaf15.1655518585.git.min.m.xu@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox