Re: [PATCH v5 05/10] SecurityPkg: Remove duplicated functions from SecureBootConfigDxe.

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

Signed-off-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
> <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang <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
> <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 <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>
> Subject: [PATCH v5 05/10] SecurityPkg: Remove duplicated functions from
> SecureBootConfigDxe.
> 
> This commit removes functions which were added
> to SecureBootVariableLib. It also adds dependecy
> on that library.
> 
> Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com>
> ---
> 
> SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDx
> e.inf |   1 +
> 
> SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigIm
> pl.c  | 189 +-------------------
>  2 files changed, 2 insertions(+), 188 deletions(-)
> 
> diff --git
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig
> Dxe.inf
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig
> Dxe.inf
> index 573efa6379..30d9cd8025 100644
> ---
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig
> Dxe.inf
> +++
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig
> Dxe.inf
> @@ -54,6 +54,7 @@
>    DevicePathLib
>    FileExplorerLib
>    PeCoffLib
> +  SecureBootVariableLib
> 
>  [Guids]
>    ## SOMETIMES_CONSUMES      ## Variable:L"CustomMode"
> diff --git
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigI
> mpl.c
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigI
> mpl.c
> index e82bfe7757..67e5e594ed 100644
> ---
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigI
> mpl.c
> +++
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigI
> mpl.c
> @@ -9,6 +9,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  #include "SecureBootConfigImpl.h"
>  #include <Library/BaseCryptLib.h>
> +#include <Library/SecureBootVariableLib.h>
> 
>  CHAR16              mSecureBootStorageName[] =
> L"SECUREBOOT_CONFIGURATION";
> 
> @@ -237,168 +238,6 @@ SaveSecureBootVariable (
>    return Status;
>  }
> 
> -/**
> -  Create a time based data payload by concatenating the
> EFI_VARIABLE_AUTHENTICATION_2
> -  descriptor with the input data. NO authentication is required in this function.
> -
> -  @param[in, out]   DataSize       On input, the size of Data buffer in bytes.
> -                                   On output, the size of data returned in Data
> -                                   buffer in bytes.
> -  @param[in, out]   Data           On input, Pointer to data buffer to be wrapped or
> -                                   pointer to NULL to wrap an empty payload.
> -                                   On output, Pointer to the new payload date buffer allocated
> from pool,
> -                                   it's caller's responsibility to free the memory when finish
> using it.
> -
> -  @retval EFI_SUCCESS              Create time based payload successfully.
> -  @retval EFI_OUT_OF_RESOURCES     There are not enough memory resources
> to create time based payload.
> -  @retval EFI_INVALID_PARAMETER    The parameter is invalid.
> -  @retval Others                   Unexpected error happens.
> -
> -**/
> -EFI_STATUS
> -CreateTimeBasedPayload (
> -  IN OUT UINTN            *DataSize,
> -  IN OUT UINT8            **Data
> -  )
> -{
> -  EFI_STATUS                       Status;
> -  UINT8                            *NewData;
> -  UINT8                            *Payload;
> -  UINTN                            PayloadSize;
> -  EFI_VARIABLE_AUTHENTICATION_2    *DescriptorData;
> -  UINTN                            DescriptorSize;
> -  EFI_TIME                         Time;
> -
> -  if (Data == NULL || DataSize == NULL) {
> -    return EFI_INVALID_PARAMETER;
> -  }
> -
> -  //
> -  // In Setup mode or Custom mode, the variable does not need to be signed but
> the
> -  // parameters to the SetVariable() call still need to be prepared as
> authenticated
> -  // variable. So we create EFI_VARIABLE_AUTHENTICATED_2 descriptor
> without certificate
> -  // data in it.
> -  //
> -  Payload     = *Data;
> -  PayloadSize = *DataSize;
> -
> -  DescriptorSize    = OFFSET_OF (EFI_VARIABLE_AUTHENTICATION_2, AuthInfo)
> + OFFSET_OF (WIN_CERTIFICATE_UEFI_GUID, CertData);
> -  NewData = (UINT8*) AllocateZeroPool (DescriptorSize + PayloadSize);
> -  if (NewData == NULL) {
> -    return EFI_OUT_OF_RESOURCES;
> -  }
> -
> -  if ((Payload != NULL) && (PayloadSize != 0)) {
> -    CopyMem (NewData + DescriptorSize, Payload, PayloadSize);
> -  }
> -
> -  DescriptorData = (EFI_VARIABLE_AUTHENTICATION_2 *) (NewData);
> -
> -  ZeroMem (&Time, sizeof (EFI_TIME));
> -  Status = gRT->GetTime (&Time, NULL);
> -  if (EFI_ERROR (Status)) {
> -    FreePool(NewData);
> -    return Status;
> -  }
> -  Time.Pad1       = 0;
> -  Time.Nanosecond = 0;
> -  Time.TimeZone   = 0;
> -  Time.Daylight   = 0;
> -  Time.Pad2       = 0;
> -  CopyMem (&DescriptorData->TimeStamp, &Time, sizeof (EFI_TIME));
> -
> -  DescriptorData->AuthInfo.Hdr.dwLength         = OFFSET_OF
> (WIN_CERTIFICATE_UEFI_GUID, CertData);
> -  DescriptorData->AuthInfo.Hdr.wRevision        = 0x0200;
> -  DescriptorData->AuthInfo.Hdr.wCertificateType = WIN_CERT_TYPE_EFI_GUID;
> -  CopyGuid (&DescriptorData->AuthInfo.CertType, &gEfiCertPkcs7Guid);
> -
> -  if (Payload != NULL) {
> -    FreePool(Payload);
> -  }
> -
> -  *DataSize = DescriptorSize + PayloadSize;
> -  *Data     = NewData;
> -  return EFI_SUCCESS;
> -}
> -
> -/**
> -  Internal helper function to delete a Variable given its name and GUID, NO
> authentication
> -  required.
> -
> -  @param[in]      VariableName            Name of the Variable.
> -  @param[in]      VendorGuid              GUID of the Variable.
> -
> -  @retval EFI_SUCCESS              Variable deleted successfully.
> -  @retval Others                   The driver failed to start the device.
> -
> -**/
> -EFI_STATUS
> -DeleteVariable (
> -  IN  CHAR16                    *VariableName,
> -  IN  EFI_GUID                  *VendorGuid
> -  )
> -{
> -  EFI_STATUS              Status;
> -  VOID*                   Variable;
> -  UINT8                   *Data;
> -  UINTN                   DataSize;
> -  UINT32                  Attr;
> -
> -  GetVariable2 (VariableName, VendorGuid, &Variable, NULL);
> -  if (Variable == NULL) {
> -    return EFI_SUCCESS;
> -  }
> -  FreePool (Variable);
> -
> -  Data     = NULL;
> -  DataSize = 0;
> -  Attr     = EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS |
> EFI_VARIABLE_BOOTSERVICE_ACCESS
> -             | EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
> -
> -  Status = CreateTimeBasedPayload (&DataSize, &Data);
> -  if (EFI_ERROR (Status)) {
> -    DEBUG ((EFI_D_ERROR, "Fail to create time-based data payload: %r", Status));
> -    return Status;
> -  }
> -
> -  Status = gRT->SetVariable (
> -                  VariableName,
> -                  VendorGuid,
> -                  Attr,
> -                  DataSize,
> -                  Data
> -                  );
> -  if (Data != NULL) {
> -    FreePool (Data);
> -  }
> -  return Status;
> -}
> -
> -/**
> -
> -  Set the platform secure boot mode into "Custom" or "Standard" mode.
> -
> -  @param[in]   SecureBootMode        New secure boot mode:
> STANDARD_SECURE_BOOT_MODE or
> -                                     CUSTOM_SECURE_BOOT_MODE.
> -
> -  @return EFI_SUCCESS                The platform has switched to the special mode
> successfully.
> -  @return other                      Fail to operate the secure boot mode.
> -
> -**/
> -EFI_STATUS
> -SetSecureBootMode (
> -  IN     UINT8         SecureBootMode
> -  )
> -{
> -  return gRT->SetVariable (
> -                EFI_CUSTOM_MODE_NAME,
> -                &gEfiCustomModeEnableGuid,
> -                EFI_VARIABLE_NON_VOLATILE |
> EFI_VARIABLE_BOOTSERVICE_ACCESS,
> -                sizeof (UINT8),
> -                &SecureBootMode
> -                );
> -}
> -
>  /**
>    This code checks if the encode type and key strength of X.509
>    certificate is qualified.
> @@ -646,32 +485,6 @@ ON_EXIT:
>    return Status;
>  }
> 
> -/**
> -  Remove the PK variable.
> -
> -  @retval EFI_SUCCESS    Delete PK successfully.
> -  @retval Others         Could not allow to delete PK.
> -
> -**/
> -EFI_STATUS
> -DeletePlatformKey (
> -  VOID
> -)
> -{
> -  EFI_STATUS Status;
> -
> -  Status = SetSecureBootMode(CUSTOM_SECURE_BOOT_MODE);
> -  if (EFI_ERROR (Status)) {
> -    return Status;
> -  }
> -
> -  Status = DeleteVariable (
> -             EFI_PLATFORM_KEY_NAME,
> -             &gEfiGlobalVariableGuid
> -             );
> -  return Status;
> -}
> -
>  /**
>    Enroll a new KEK item from public key storing file (*.pbk).
> 
> --
> 2.25.1