Re: [PATCH v5 10/10] SecurityPkg: Add option to reset secure boot keys.

["Yao, Jiewen" <jiewen.yao@intel.com>]

Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

> -----Original Message-----
> From: Grzegorz Bernacki <gjb@semihalf.com>
> Sent: Thursday, July 1, 2021 5:18 PM
> To: devel@edk2.groups.io
> Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Samer.El-Haj-
> Mahmoud@arm.com; sunny.Wang@arm.com; mw@semihalf.com;
> upstream@semihalf.com; Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Xu, Min M <min.m.xu@intel.com>;
> lersek@redhat.com; sami.mujawar@arm.com; afish@apple.com; Ni, Ray
> <ray.ni@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
> rebecca@bsdio.com; grehan@freebsd.org; thomas.abraham@arm.com; Chiu,
> Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; gaoliming@byosoft.com.cn; Dong, Eric
> <eric.dong@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Sun,
> Zailiang <zailiang.sun@intel.com>; Qian, Yi <yi.qian@intel.com>;
> graeme@nuviainc.com; rad@semihalf.com; pete@akeo.ie; Grzegorz Bernacki
> <gjb@semihalf.com>; Sunny Wang <sunny.wang@arm.com>
> Subject: [PATCH v5 10/10] 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>
> Reviewed-by: Sunny Wang <sunny.wang@arm.com>
> Reviewed-by: Pete Batard <pete@akeo.ie>
> Tested-by: Pete Batard <pete@akeo.ie> on Raspberry Pi 4
> ---
> 
> SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDx
> e.inf     |   1 +
> 
> SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigNv
> Data.h    |   2 +
> 
> SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr
> |   6 +
> 
> SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigIm
> pl.c      | 154 ++++++++++++++++++++
> 
> SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigStri
> ngs.uni |   4 +
>  5 files changed, 167 insertions(+)
> 
> diff --git
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig
> Dxe.inf
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig
> Dxe.inf
> index 30d9cd8025..bd8d256dde 100644
> ---
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig
> Dxe.inf
> +++
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig
> Dxe.inf
> @@ -109,6 +109,7 @@
>  [Protocols]
>    gEfiHiiConfigAccessProtocolGuid               ## PRODUCES
>    gEfiDevicePathProtocolGuid                    ## PRODUCES
> +  gEfiHiiPopupProtocolGuid
> 
>  [Depex]
>    gEfiHiiConfigRoutingProtocolGuid  AND
> diff --git
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig
> NvData.h
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig
> NvData.h
> index 6e54a4b0f2..4ecc25efc3 100644
> ---
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig
> NvData.h
> +++
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig
> NvData.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/SecureBootConfigI
> mpl.c
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigI
> mpl.c
> index 67e5e594ed..47f281873b 100644
> ---
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigI
> mpl.c
> +++
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigI
> mpl.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/SecureBootConfigS
> trings.uni
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigS
> trings.uni
> index ac783453cc..0d01701de7 100644
> ---
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigS
> trings.uni
> +++
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigS
> trings.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