public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Kun Qin" <kuqin12@gmail.com>
To: devel@edk2.groups.io
Cc: Jiewen Yao <jiewen.yao@intel.com>,
	Jian J Wang <jian.j.wang@intel.com>, Min Xu <min.m.xu@intel.com>
Subject: [PATCH v1 08/11] SecurityPkg: SecureBootConfigDxe: Updated invocation pattern
Date: Wed,  4 May 2022 11:04:34 -0700	[thread overview]
Message-ID: <20220504180438.1321-9-kuqin12@gmail.com> (raw)
In-Reply-To: <20220504180438.1321-1-kuqin12@gmail.com>

From: Kun Qin <kuqin@microsoft.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3909

This change is in pair with the previous SecureBootVariableLib change,
which updated the interface of `CreateTimeBasedPayload`.

This change added a helper function to query the current time through
Real Time Clock protocol. This function is used when needing to format
an authenticated variable payload.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Min Xu <min.m.xu@intel.com>

Signed-off-by: Kun Qin <kun.qin@microsoft.com>
---
 SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c  | 127 ++++++++++++++++++--
 SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf |   1 +
 2 files changed, 119 insertions(+), 9 deletions(-)

diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
index a13c349a0f89..4299a6b5e56d 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
@@ -10,6 +10,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include "SecureBootConfigImpl.h"
 #include <UefiSecureBoot.h>
 #include <Protocol/HiiPopup.h>
+#include <Protocol/RealTimeClock.h>
 #include <Library/BaseCryptLib.h>
 #include <Library/SecureBootVariableLib.h>
 #include <Library/SecureBootVariableProvisionLib.h>
@@ -136,6 +137,51 @@ CloseEnrolledFile (
   FileContext->FileType = UNKNOWN_FILE_TYPE;
 }
 
+/**
+  Helper function to populate an EFI_TIME instance.
+
+  @param[in] Time   FileContext cached in SecureBootConfig driver
+
+**/
+STATIC
+EFI_STATUS
+GetCurrentTime (
+  IN EFI_TIME  *Time
+  )
+{
+  EFI_STATUS  Status;
+  VOID        *TestPointer;
+
+  if (Time == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  Status = gBS->LocateProtocol (&gEfiRealTimeClockArchProtocolGuid, NULL, &TestPointer);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  ZeroMem (Time, sizeof (EFI_TIME));
+  Status = gRT->GetTime (Time, NULL);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a(), GetTime() failed, status = '%r'\n",
+      __FUNCTION__,
+      Status
+      ));
+    return Status;
+  }
+
+  Time->Pad1       = 0;
+  Time->Nanosecond = 0;
+  Time->TimeZone   = 0;
+  Time->Daylight   = 0;
+  Time->Pad2       = 0;
+
+  return EFI_SUCCESS;
+}
+
 /**
   This code checks if the FileSuffix is one of the possible DER-encoded certificate suffix.
 
@@ -436,6 +482,7 @@ EnrollPlatformKey (
   UINT32              Attr;
   UINTN               DataSize;
   EFI_SIGNATURE_LIST  *PkCert;
+  EFI_TIME            Time;
 
   PkCert = NULL;
 
@@ -463,7 +510,13 @@ EnrollPlatformKey (
   Attr = EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS
          | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
   DataSize = PkCert->SignatureListSize;
-  Status   = CreateTimeBasedPayload (&DataSize, (UINT8 **)&PkCert);
+  Status   = GetCurrentTime (&Time);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Fail to fetch valid time data: %r", Status));
+    goto ON_EXIT;
+  }
+
+  Status = CreateTimeBasedPayload (&DataSize, (UINT8 **)&PkCert, &Time);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "Fail to create time-based data payload: %r", Status));
     goto ON_EXIT;
@@ -522,6 +575,7 @@ EnrollRsa2048ToKek (
   UINTN               KekSigListSize;
   UINT8               *KeyBuffer;
   UINTN               KeyLenInBytes;
+  EFI_TIME            Time;
 
   Attr           = 0;
   DataSize       = 0;
@@ -608,7 +662,13 @@ EnrollRsa2048ToKek (
   //
   Attr = EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS
          | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
-  Status = CreateTimeBasedPayload (&KekSigListSize, (UINT8 **)&KekSigList);
+  Status = GetCurrentTime (&Time);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Fail to fetch valid time data: %r", Status));
+    goto ON_EXIT;
+  }
+
+  Status = CreateTimeBasedPayload (&KekSigListSize, (UINT8 **)&KekSigList, &Time);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "Fail to create time-based data payload: %r", Status));
     goto ON_EXIT;
@@ -689,6 +749,7 @@ EnrollX509ToKek (
   UINTN               DataSize;
   UINTN               KekSigListSize;
   UINT32              Attr;
+  EFI_TIME            Time;
 
   X509Data       = NULL;
   X509DataSize   = 0;
@@ -735,7 +796,13 @@ EnrollX509ToKek (
   //
   Attr = EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS
          | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
-  Status = CreateTimeBasedPayload (&KekSigListSize, (UINT8 **)&KekSigList);
+  Status = GetCurrentTime (&Time);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Fail to fetch valid time data: %r", Status));
+    goto ON_EXIT;
+  }
+
+  Status = CreateTimeBasedPayload (&KekSigListSize, (UINT8 **)&KekSigList, &Time);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "Fail to create time-based data payload: %r", Status));
     goto ON_EXIT;
@@ -861,6 +928,7 @@ EnrollX509toSigDB (
   UINTN               DataSize;
   UINTN               SigDBSize;
   UINT32              Attr;
+  EFI_TIME            Time;
 
   X509DataSize  = 0;
   SigDBSize     = 0;
@@ -910,7 +978,13 @@ EnrollX509toSigDB (
   //
   Attr = EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS
          | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
-  Status = CreateTimeBasedPayload (&SigDBSize, (UINT8 **)&Data);
+  Status = GetCurrentTime (&Time);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Fail to fetch valid time data: %r", Status));
+    goto ON_EXIT;
+  }
+
+  Status = CreateTimeBasedPayload (&SigDBSize, (UINT8 **)&Data, &Time);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "Fail to create time-based data payload: %r", Status));
     goto ON_EXIT;
@@ -1321,6 +1395,7 @@ EnrollX509HashtoSigDB (
   UINT16              *FilePostFix;
   UINTN               NameLength;
   EFI_TIME            *Time;
+  EFI_TIME            NewTime;
 
   X509DataSize  = 0;
   DbSize        = 0;
@@ -1490,7 +1565,13 @@ EnrollX509HashtoSigDB (
     DataSize = DbSize;
   }
 
-  Status = CreateTimeBasedPayload (&DataSize, (UINT8 **)&Data);
+  Status = GetCurrentTime (&NewTime);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Fail to fetch valid time data: %r", Status));
+    goto ON_EXIT;
+  }
+
+  Status = CreateTimeBasedPayload (&DataSize, (UINT8 **)&Data, &NewTime);
   if (EFI_ERROR (Status)) {
     goto ON_EXIT;
   }
@@ -2169,6 +2250,7 @@ EnrollImageSignatureToSigDB (
   UINTN                      SigDBSize;
   UINT32                     Attr;
   WIN_CERTIFICATE_UEFI_GUID  *GuidCertData;
+  EFI_TIME                   Time;
 
   Data         = NULL;
   GuidCertData = NULL;
@@ -2267,7 +2349,13 @@ EnrollImageSignatureToSigDB (
 
   Attr = EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS
          | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
-  Status = CreateTimeBasedPayload (&SigDBSize, (UINT8 **)&Data);
+  Status = GetCurrentTime (&Time);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Fail to fetch valid time data: %r", Status));
+    goto ON_EXIT;
+  }
+
+  Status = CreateTimeBasedPayload (&SigDBSize, (UINT8 **)&Data, &Time);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "Fail to create time-based data payload: %r", Status));
     goto ON_EXIT;
@@ -2609,6 +2697,7 @@ DeleteKeyExchangeKey (
   UINT32              KekDataSize;
   UINTN               DeleteKekIndex;
   UINTN               GuidIndex;
+  EFI_TIME            Time;
 
   Data           = NULL;
   OldData        = NULL;
@@ -2727,7 +2816,13 @@ DeleteKeyExchangeKey (
 
   DataSize = Offset;
   if ((Attr & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) != 0) {
-    Status = CreateTimeBasedPayload (&DataSize, &OldData);
+    Status = GetCurrentTime (&Time);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR, "Fail to fetch valid time data: %r", Status));
+      goto ON_EXIT;
+    }
+
+    Status = CreateTimeBasedPayload (&DataSize, &OldData, &Time);
     if (EFI_ERROR (Status)) {
       DEBUG ((DEBUG_ERROR, "Fail to create time-based data payload: %r", Status));
       goto ON_EXIT;
@@ -2805,6 +2900,7 @@ DeleteSignature (
   BOOLEAN             IsItemFound;
   UINT32              ItemDataSize;
   UINTN               GuidIndex;
+  EFI_TIME            Time;
 
   Data     = NULL;
   OldData  = NULL;
@@ -2931,7 +3027,13 @@ DeleteSignature (
 
   DataSize = Offset;
   if ((Attr & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) != 0) {
-    Status = CreateTimeBasedPayload (&DataSize, &OldData);
+    Status = GetCurrentTime (&Time);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR, "Fail to fetch valid time data: %r", Status));
+      goto ON_EXIT;
+    }
+
+    Status = CreateTimeBasedPayload (&DataSize, &OldData, &Time);
     if (EFI_ERROR (Status)) {
       DEBUG ((DEBUG_ERROR, "Fail to create time-based data payload: %r", Status));
       goto ON_EXIT;
@@ -3000,6 +3102,7 @@ DeleteSignatureEx (
   UINTN               Offset;
   UINT8               *VariableData;
   UINT8               *NewVariableData;
+  EFI_TIME            Time;
 
   Status           = EFI_SUCCESS;
   VariableAttr     = 0;
@@ -3120,7 +3223,13 @@ DeleteSignatureEx (
   }
 
   if ((VariableAttr & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) != 0) {
-    Status = CreateTimeBasedPayload (&VariableDataSize, &NewVariableData);
+    Status = GetCurrentTime (&Time);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR, "Fail to fetch valid time data: %r", Status));
+      goto ON_EXIT;
+    }
+
+    Status = CreateTimeBasedPayload (&VariableDataSize, &NewVariableData, &Time);
     if (EFI_ERROR (Status)) {
       DEBUG ((DEBUG_ERROR, "Fail to create time-based data payload: %r", Status));
       goto ON_EXIT;
diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
index 420687a21141..1671d5be7ccd 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
@@ -111,6 +111,7 @@ [Protocols]
   gEfiHiiConfigAccessProtocolGuid               ## PRODUCES
   gEfiDevicePathProtocolGuid                    ## PRODUCES
   gEfiHiiPopupProtocolGuid
+  gEfiRealTimeClockArchProtocolGuid             ## CONSUMES
 
 [Depex]
   gEfiHiiConfigRoutingProtocolGuid  AND
-- 
2.34.1.windows.1


  parent reply	other threads:[~2022-05-04 18:04 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-04 18:04 [PATCH v1 00/11] Enhance Secure Boot Variable Libraries Kun Qin
2022-05-04 18:04 ` [PATCH v1 01/11] SecurityPkg: UefiSecureBoot: Definitions of cert and payload structures Kun Qin
2022-05-04 18:04 ` [PATCH v1 02/11] SecurityPkg: PlatformPKProtectionLib: Added PK protection interface Kun Qin
2022-05-04 18:04 ` [PATCH v1 03/11] SecurityPkg: SecureBootVariableLib: Updated time based payload creator Kun Qin
2022-05-04 18:04 ` [PATCH v1 04/11] SecurityPkg: SecureBootVariableLib: Updated signature list creator Kun Qin
2022-05-04 18:04 ` [PATCH v1 05/11] SecurityPkg: SecureBootVariableLib: Added newly supported interfaces Kun Qin
2022-05-04 18:04 ` [PATCH v1 06/11] SecurityPkg: SecureBootVariableProvisionLib: Updated implementation Kun Qin
2022-05-04 18:04 ` [PATCH v1 07/11] SecurityPkg: Secure Boot Drivers: Added common header files Kun Qin
2022-05-04 18:04 ` Kun Qin [this message]
2022-05-04 18:04 ` [PATCH v1 09/11] SecurityPkg: SecureBootVariableLib: Added unit tests Kun Qin
2022-05-04 18:04 ` [PATCH v1 10/11] OvmfPkg: Pipeline: Resolve SecureBootVariableLib dependency Kun Qin
2022-05-04 18:04 ` [PATCH v1 11/11] EmulatorPkg: " Kun Qin

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=20220504180438.1321-9-kuqin12@gmail.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