Re: [PATCH v2 6/6] SecurityPkg: Add option to reset secure boot keys.

["Sunny Wang" <Sunny.Wang@arm.com>]

Internally reviewed this patch before sending the edk2 mailing list and Greg already addressed all my comments, so It looks good to me.
Reviewed-by: Sunny Wang <sunny.wang@arm.com>

-----Original Message-----
From: Grzegorz Bernacki <gjb@semihalf.com>
Sent: Tuesday, June 1, 2021 9:12 PM
To: devel@edk2.groups.io
Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang <Sunny.Wang@arm.com>; mw@semihalf.com; upstream@semihalf.com; jiewen.yao@intel.com; jian.j.wang@intel.com; min.m.xu@intel.com; lersek@redhat.com; Grzegorz Bernacki <gjb@semihalf.com>
Subject: [PATCH v2 6/6] SecurityPkg: Add option to reset secure boot keys.

This commit add option which allows reset content of Secure Boot
keys and databases to default variables.

Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com>
---
 SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf     |   1 +
 SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigNvData.h    |   2 +
 SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr        |   6 +
 SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c      | 154 ++++++++++++++++++++
 SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigStrings.uni |   4 +
 5 files changed, 167 insertions(+)

diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
index 30d9cd8025..bd8d256dde 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
@@ -109,6 +109,7 @@
 [Protocols]
   gEfiHiiConfigAccessProtocolGuid               ## PRODUCES
   gEfiDevicePathProtocolGuid                    ## PRODUCES
+  gEfiHiiPopupProtocolGuid

 [Depex]
   gEfiHiiConfigRoutingProtocolGuid  AND
diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigNvData.h b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigNvData.h
index 6e54a4b0f2..4ecc25efc3 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigNvData.h
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigNvData.h
@@ -54,6 +54,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent

 #define KEY_VALUE_FROM_DBX_TO_LIST_FORM       0x100f

+#define KEY_SECURE_BOOT_RESET_TO_DEFAULT      0x1010
+
 #define KEY_SECURE_BOOT_OPTION                0x1100
 #define KEY_SECURE_BOOT_PK_OPTION             0x1101
 #define KEY_SECURE_BOOT_KEK_OPTION            0x1102
diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr
index fa7e11848c..e4560c592c 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr
@@ -69,6 +69,12 @@ formset
     endif;
     endif;

+    text
+      help   = STRING_TOKEN(STR_SECURE_RESET_TO_DEFAULTS_HELP),
+      text   = STRING_TOKEN(STR_SECURE_RESET_TO_DEFAULTS),
+      flags  = INTERACTIVE,
+      key    = KEY_SECURE_BOOT_RESET_TO_DEFAULT;
+
   endform;

   //
diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
index 67e5e594ed..47f281873b 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
@@ -8,6 +8,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 **/

 #include "SecureBootConfigImpl.h"
+#include <Protocol/HiiPopup.h>
 #include <Library/BaseCryptLib.h>
 #include <Library/SecureBootVariableLib.h>

@@ -4154,6 +4155,132 @@ ON_EXIT:
   return Status;
 }

+/**
+  This function reinitializes Secure Boot variables with default values.
+
+  @retval   EFI_SUCCESS           Success to update the signature list page
+  @retval   others                Fail to delete or enroll signature data.
+**/
+
+STATIC EFI_STATUS
+EFIAPI
+KeyEnrollReset (
+  VOID
+  )
+{
+  EFI_STATUS  Status;
+  UINT8       SetupMode;
+
+  Status = EFI_SUCCESS;
+
+  Status = SetSecureBootMode (CUSTOM_SECURE_BOOT_MODE);
+  if (EFI_ERROR(Status)) {
+    return Status;
+  }
+
+  // Clear all the keys and databases
+  Status = DeleteDb ();
+  if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
+    DEBUG ((DEBUG_ERROR, "Fail to clear DB: %r\n", Status));
+    return Status;
+  }
+
+  Status = DeleteDbx ();
+  if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
+    DEBUG ((DEBUG_ERROR, "Fail to clear DBX: %r\n", Status));
+    return Status;
+  }
+
+  Status = DeleteDbt ();
+  if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
+    DEBUG ((DEBUG_ERROR, "Fail to clear DBT: %r\n", Status));
+    return Status;
+  }
+
+  Status = DeleteKEK ();
+  if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
+    DEBUG ((DEBUG_ERROR, "Fail to clear KEK: %r\n", Status));
+    return Status;
+  }
+
+  Status = DeletePlatformKey ();
+  if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
+    DEBUG ((DEBUG_ERROR, "Fail to clear PK: %r\n", Status));
+    return Status;
+  }
+
+  // After PK clear, Setup Mode shall be enabled
+  Status = GetSetupMode (&SetupMode);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Cannot get SetupMode variable: %r\n",
+      Status));
+    return Status;
+  }
+
+  if (SetupMode == USER_MODE) {
+    DEBUG((DEBUG_INFO, "Skipped - USER_MODE\n"));
+    return EFI_SUCCESS;
+  }
+
+  Status = SetSecureBootMode (CUSTOM_SECURE_BOOT_MODE);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Cannot set CUSTOM_SECURE_BOOT_MODE: %r\n",
+      Status));
+    return EFI_SUCCESS;
+  }
+
+  // Enroll all the keys from default variables
+  Status = EnrollDbFromDefault ();
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Cannot enroll db: %r\n", Status));
+    goto error;
+  }
+
+  Status = EnrollDbxFromDefault ();
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Cannot enroll dbx: %r\n", Status));
+  }
+
+  Status = EnrollDbtFromDefault ();
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Cannot enroll dbt: %r\n", Status));
+  }
+
+  Status = EnrollKEKFromDefault ();
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Cannot enroll KEK: %r\n", Status));
+    goto cleardbs;
+  }
+
+  Status = EnrollPKFromDefault ();
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Cannot enroll PK: %r\n", Status));
+    goto clearKEK;
+  }
+
+  Status = SetSecureBootMode (STANDARD_SECURE_BOOT_MODE);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Cannot set CustomMode to STANDARD_SECURE_BOOT_MODE\n"
+      "Please do it manually, otherwise system can be easily compromised\n"));
+  }
+
+  return Status;
+
+clearKEK:
+  DeleteKEK ();
+
+cleardbs:
+  DeleteDbt ();
+  DeleteDbx ();
+  DeleteDb ();
+
+error:
+  if (SetSecureBootMode (STANDARD_SECURE_BOOT_MODE) != EFI_SUCCESS) {
+    DEBUG ((DEBUG_ERROR, "Cannot set mode to Secure: %r\n", Status));
+  }
+  return Status;
+}
+
 /**
   This function is called to provide results data to the driver.

@@ -4205,6 +4332,8 @@ SecureBootCallback (
   SECUREBOOT_CONFIG_PRIVATE_DATA  *PrivateData;
   BOOLEAN                         GetBrowserDataResult;
   ENROLL_KEY_ERROR                EnrollKeyErrorCode;
+  EFI_HII_POPUP_PROTOCOL          *HiiPopup;
+  EFI_HII_POPUP_SELECTION         UserSelection;

   Status             = EFI_SUCCESS;
   SecureBootEnable   = NULL;
@@ -4755,6 +4884,31 @@ SecureBootCallback (
         FreePool (SetupMode);
       }
       break;
+    case KEY_SECURE_BOOT_RESET_TO_DEFAULT:
+    {
+      Status = gBS->LocateProtocol (&gEfiHiiPopupProtocolGuid, NULL, (VOID **) &HiiPopup);
+      if (EFI_ERROR (Status)) {
+        return Status;
+      }
+      Status = HiiPopup->CreatePopup (
+                           HiiPopup,
+                           EfiHiiPopupStyleInfo,
+                           EfiHiiPopupTypeYesNo,
+                           Private->HiiHandle,
+                           STRING_TOKEN (STR_RESET_TO_DEFAULTS_POPUP),
+                           &UserSelection
+                           );
+      if (UserSelection == EfiHiiPopupSelectionYes) {
+        Status = KeyEnrollReset ();
+      }
+      //
+      // Update secure boot strings after key reset
+      //
+      if (Status == EFI_SUCCESS) {
+        Status = UpdateSecureBootString (Private);
+        SecureBootExtractConfigFromVariable (Private, IfrNvData);
+      }
+    }
     default:
       break;
     }
diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigStrings.uni b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigStrings.uni
index ac783453cc..0d01701de7 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigStrings.uni
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigStrings.uni
@@ -21,6 +21,10 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #string STR_SECURE_BOOT_PROMPT             #language en-US "Attempt Secure Boot"
 #string STR_SECURE_BOOT_HELP               #language en-US "Enable/Disable the Secure Boot feature after platform reset"

+#string STR_SECURE_RESET_TO_DEFAULTS_HELP  #language en-US "Enroll keys with data from default variables"
+#string STR_SECURE_RESET_TO_DEFAULTS       #language en-US "Reset Secure Boot Keys"
+#string STR_RESET_TO_DEFAULTS_POPUP        #language en-US "Secure Boot Keys & databases will be initialized from defaults.\n Are you sure?"
+
 #string STR_SECURE_BOOT_ENROLL_SIGNATURE   #language en-US "Enroll Signature"
 #string STR_SECURE_BOOT_DELETE_SIGNATURE   #language en-US "Delete Signature"
 #string STR_SECURE_BOOT_DELETE_LIST_FORM   #language en-US "Delete Signature List Form"
--
2.25.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.