Hi,
AuthVariableLib must support DXE_DRIVER and UEFI_APPLICATION. Adding
it leads to errors for libraries which are used by AuthVariableLib and
which does not support DXE_DRIVER or UEFI_APPLICATION. This changes
causes a lot of minor changes in another libraries.
The whole problem exists because I wanted to remove a duplicate of
CreateTimeBasedPayload() from PlatformVarCleanupLib. I just leave it
there. This module is not used anyway. I think it can be safely
removed, but I don't want this change to be a part of this patchset.
thanks,
greg
śr., 23 cze 2021 o 05:33 Sunny Wang <sunny.w...@arm.com> napisał(a):
>
> Hi Greg,
> Why can't we make AuthVariableLib support DXE_DRIVER?
>
> Best Regards,
> Sunny Wang
>
> -----Original Message-----
> From: Grzegorz Bernacki <g...@semihalf.com>
> Sent: Monday, June 21, 2021 5:59 PM
> To: devel@edk2.groups.io; Grzegorz Bernacki <g...@semihalf.com>
> Cc: gaolim...@byosoft.com.cn; l...@nuviainc.com; ardb+tianoc...@kernel.org;
> Samer El-Haj-Mahmoud <samer.el-haj-mahm...@arm.com>; Sunny Wang
> <sunny.w...@arm.com>; Marcin Wojtas <m...@semihalf.com>;
> upstr...@semihalf.com; Yao, Jiewen <jiewen....@intel.com>; Wang, Jian J
> <jian.j.w...@intel.com>; Xu, Min M <min.m...@intel.com>; Laszlo Ersek
> <ler...@redhat.com>; Sami Mujawar <sami.muja...@arm.com>; af...@apple.com;
> ray...@intel.com; jordan.l.jus...@intel.com; rebe...@bsdio.com;
> gre...@freebsd.org; Thomas Abraham <thomas.abra...@arm.com>;
> chasel.c...@intel.com; nathaniel.l.desim...@intel.com; eric.d...@intel.com;
> michael.d.kin...@intel.com; zailiang....@intel.com; yi.q...@intel.com;
> gra...@nuviainc.com; Radosław Biernacki <r...@semihalf.com>; Pete Batard
> <p...@akeo.ie>
> Subject: Re: [edk2-devel] [PATCH v3 8/8] MdeModulePkg: Use
> SecureBootVariableLib in PlatformVarCleanupLib.
>
> 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....@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 <gaolim...@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@edk2.groups.io <devel@edk2.groups.io> 代表 Grzegorz
> > > > Bernacki
> > > > 发送时间: 2021年6月14日 17:43
> > > > 收件人: devel@edk2.groups.io
> > > > 抄送: l...@nuviainc.com; ardb+tianoc...@kernel.org;
> > > > samer.el-haj-mahm...@arm.com; sunny.w...@arm.com;
> > > > m...@semihalf.com; upstr...@semihalf.com; jiewen....@intel.com;
> > > > jian.j.w...@intel.com; min.m...@intel.com; ler...@redhat.com;
> > > > sami.muja...@arm.com; af...@apple.com; ray...@intel.com;
> > > > jordan.l.jus...@intel.com; rebe...@bsdio.com; gre...@freebsd.org;
> > > > thomas.abra...@arm.com; chasel.c...@intel.com;
> > > > nathaniel.l.desim...@intel.com; gaolim...@byosoft.com.cn;
> > > > eric.d...@intel.com; michael.d.kin...@intel.com; zailiang....@intel.com;
> > > > yi.q...@intel.com; gra...@nuviainc.com; r...@semihalf.com; p...@akeo.ie;
> > > > Grzegorz Bernacki <g...@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 <g...@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
> > > >
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
> 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.
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76927): https://edk2.groups.io/g/devel/message/76927
Mute This Topic: https://groups.io/mt/83684788/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
