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

Grzegorz Bernacki gjb at semihalf.com
Mon Jun 21 09:58:58 UTC 2021


Hi,

I moved CreateTimeBasedPayload() to AuthVariableLib, but then I cannot
use it in SecureBootConfigDxe, cause AuthVariableLib does not support
DXE_DRIVER.
So:
- having that function only in AuthVariableLib does not work
- having that function only in SecureBootVariableLib, causes a lot of
changes in platform DSCs files and also causes MdeModulePkg to be
depended on SecurityPkg

Right now I tend to roll back the changes related to
CreateTimeBasedPayload() and just let the modules to have its own copy
of that function. What do you think?
thanks,
greg

pt., 18 cze 2021 o 10:03 Grzegorz Bernacki via groups.io
<gjb=semihalf.com at groups.io> napisał(a):
>
> Hi,
>
> Thanks for the comment, I will move that function to AuthVariableLib.
> greg
>
> czw., 17 cze 2021 o 04:35 gaoliming <gaoliming at byosoft.com.cn> napisał(a):
> >
> > 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 (#76800): https://edk2.groups.io/g/devel/message/76800
Mute This Topic: https://groups.io/mt/83684788/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-






More information about the edk2-devel-archive mailing list