回复: [edk2-devel] [PATCH v3 8/8] MdeModulePkg: Use SecureBootVariableLib in PlatformVarCleanupLib.

[gaoliming]

Grzegorz:
  MdeModulePkg is generic base package. It should not depend on SecurityPkg.

  I agree CreateTimeBasedPayload() is the generic API. It can be shared in
the different modules. 
  I propose to add it into MdeModulePkg AuthVariableLib.

Thanks
Liming
> -----邮件原件-----
> 发件人: devel at edk2.groups.io <devel at edk2.groups.io> 代表 Grzegorz
> Bernacki
> 发送时间: 2021年6月14日 17:43
> 收件人: devel at edk2.groups.io
> 抄送: leif at nuviainc.com; ardb+tianocore at kernel.org;
> Samer.El-Haj-Mahmoud at arm.com; sunny.Wang at arm.com;
> mw at semihalf.com; upstream at semihalf.com; jiewen.yao at intel.com;
> jian.j.wang at intel.com; min.m.xu at intel.com; lersek at redhat.com;
> sami.mujawar at arm.com; afish at apple.com; ray.ni at intel.com;
> jordan.l.justen at intel.com; rebecca at bsdio.com; grehan at freebsd.org;
> thomas.abraham at arm.com; chasel.chiu at intel.com;
> nathaniel.l.desimone at intel.com; gaoliming at byosoft.com.cn;
> eric.dong at intel.com; michael.d.kinney at intel.com; zailiang.sun at intel.com;
> yi.qian at intel.com; graeme at nuviainc.com; rad at semihalf.com; pete at akeo.ie;
> Grzegorz Bernacki <gjb at semihalf.com>
> 主题: [edk2-devel] [PATCH v3 8/8] MdeModulePkg: Use
> SecureBootVariableLib in PlatformVarCleanupLib.
> 
> This commits removes CreateTimeBasedPayload() function from
> PlatformVarCleanupLib and uses exactly the same function from
> SecureBootVariableLib.
> 
> Signed-off-by: Grzegorz Bernacki <gjb at semihalf.com>
> ---
>  MdeModulePkg/Library/PlatformVarCleanupLib/PlatformVarCleanupLib.inf |
> 2 +
>  MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanup.h
> |  1 +
>  MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c
> | 84 --------------------
>  3 files changed, 3 insertions(+), 84 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Library/PlatformVarCleanupLib/PlatformVarCleanupLib.inf
> b/MdeModulePkg/Library/PlatformVarCleanupLib/PlatformVarCleanupLib.inf
> index 8d5db826a0..493d03e1d8 100644
> ---
> a/MdeModulePkg/Library/PlatformVarCleanupLib/PlatformVarCleanupLib.inf
> +++
> b/MdeModulePkg/Library/PlatformVarCleanupLib/PlatformVarCleanupLib.inf
> @@ -34,6 +34,7 @@
>  [Packages]
>    MdePkg/MdePkg.dec
>    MdeModulePkg/MdeModulePkg.dec
> +  SecurityPkg/SecurityPkg.dec
> 
>  [LibraryClasses]
>    UefiBootServicesTableLib
> @@ -44,6 +45,7 @@
>    PrintLib
>    MemoryAllocationLib
>    HiiLib
> +  SecureBootVariableLib
> 
>  [Guids]
>    gEfiIfrTianoGuid                  ## SOMETIMES_PRODUCES   ##
> GUID
> diff --git a/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanup.h
> b/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanup.h
> index c809a7086b..94fbc7d2a4 100644
> --- a/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanup.h
> +++ b/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanup.h
> @@ -18,6 +18,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/HiiLib.h>
>  #include <Library/PlatformVarCleanupLib.h>
> +#include <Library/SecureBootVariableLib.h>
> 
>  #include <Protocol/Variable.h>
>  #include <Protocol/VarCheck.h>
> diff --git
> a/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c
> b/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c
> index 3875d614bb..204f1e00ad 100644
> --- a/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c
> +++ b/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c
> @@ -319,90 +319,6 @@ DestroyUserVariableNode (
>    }
>  }
> 
> -/**
> -  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 after using it.
> -
> -  @retval EFI_SUCCESS               Create time based payload
> successfully.
> -  @retval EFI_OUT_OF_RESOURCES      There are not enough memory
> resourses 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;
> -  }
> -
> -  //
> -  // At user physical presence, 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;
> -}
> -
>  /**
>    Create a counter based data payload by concatenating the
> EFI_VARIABLE_AUTHENTICATION
>    descriptor with the input data. NO authentication is required in this
> function.
> --
> 2.25.1
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76639): https://edk2.groups.io/g/devel/message/76639
Mute This Topic: https://groups.io/mt/83596600/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-