From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qk1-f180.google.com (mail-qk1-f180.google.com [209.85.222.180]) by mx.groups.io with SMTP id smtpd.web12.5058.1624003411868948139 for ; Fri, 18 Jun 2021 01:03:32 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@semihalf-com.20150623.gappssmtp.com header.s=20150623 header.b=1H0k9nUN; spf=none, err=SPF record not found (domain: semihalf.com, ip: 209.85.222.180, mailfrom: gjb@semihalf.com) Received: by mail-qk1-f180.google.com with SMTP id f70so8856237qke.13 for ; Fri, 18 Jun 2021 01:03:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=5jOPtLuN0vTH35ThJ2hq4V4UNbsl9urQZSIaQJ3ktB8=; b=1H0k9nUNY6jtKHKKObMdjV988U76d9K45CAkvCBA4UY0DGxsXWV+vzBMCf9n00YBmZ XqIlb7WL5ksWFxgEcv2hwglW9X+EmxHqGCTWp76TC/MlA+7Z0rAQNHjSUyYWTI6D9JnM Up4HsuVDKAOBk07gx5hOsL7WkMNFkeHYddQgqgaG4SsovD6fyEYUJ3j9u51UwQwy8Sfm +3Gny9H33QeIHLJQHZP1Xyt9z4ILk1IxDKcZe2XSoWNv4k7lE3jBCke5+e3tCV+NIR2j Y2BJD3ixcUERtQpf/mVUlufw1L0/M45sDZ7nhdlV8Gi2iaQUzFcSAwgvrSleIh/jim1d 09cg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=5jOPtLuN0vTH35ThJ2hq4V4UNbsl9urQZSIaQJ3ktB8=; b=pvSAKWiz8vZouspD+VoUXg+GDY4pzMcVURLaOhZjK87ed52bezPSIaDOm5Fimk1s6B Rnc7uj2lY6xBBlJsVXu6Ze89EU/T/poG1M6W8L4Mg8b4PvgtJOsi6L+T5JrMLDyMOjAQ 3q+10Eg862ydpmlUhGzjwTK7qsNY/M3kOtKarnp4pE7fShpav2PxgeOEnWnxgmKNILCL L4ls1FQJFCKfmG1cmnVaO0HfS5DE9+aueo5/iMhv8Voyo503OQhVb5e9e1XMd12n3PO7 dtqzlxPlKcxijVb8g+u15faG8o00jtO+qVfURyUB/JYVft/kf6eQmG5wWEGz00Rs/10x UQ2g== X-Gm-Message-State: AOAM532qVlxU9d+8m3e9zr25jO50JTqx0rocji2qMA9WfV3DOMNM1cFn lHZEgIAwdFDm3ta9AjpMd6XCjIY0feW+GsxTHCp8EAi2tbgJ9FEv X-Google-Smtp-Source: ABdhPJx9+HXCN2l3gN9Sz2zHzi3yagJ0vZqfYfRSJf19X4vEkufjT+Kgxmuc0xaSYUeOu/RWGEUVFiU8yp9tPaA2xaY= X-Received: by 2002:a37:bf81:: with SMTP id p123mr8416795qkf.40.1624003410678; Fri, 18 Jun 2021 01:03:30 -0700 (PDT) MIME-Version: 1.0 References: <20210614094308.2314345-1-gjb@semihalf.com> <20210614094308.2314345-11-gjb@semihalf.com> <005b01d76321$4a8500a0$df8f01e0$@byosoft.com.cn> In-Reply-To: <005b01d76321$4a8500a0$df8f01e0$@byosoft.com.cn> From: "Grzegorz Bernacki" Date: Fri, 18 Jun 2021 10:03:19 +0200 Message-ID: Subject: Re: [edk2-devel] [PATCH v3 8/8] MdeModulePkg: Use SecureBootVariableLib in PlatformVarCleanupLib. To: devel@edk2.groups.io, gaoliming@byosoft.com.cn Cc: leif@nuviainc.com, ardb+tianocore@kernel.org, Samer El-Haj-Mahmoud , Sunny Wang , Marcin Wojtas , upstream@semihalf.com, "Yao, Jiewen" , "Wang, Jian J" , "Xu, Min M" , Laszlo Ersek , sami.mujawar@arm.com, afish@apple.com, ray.ni@intel.com, jordan.l.justen@intel.com, rebecca@bsdio.com, grehan@freebsd.org, thomas.abraham@arm.com, chasel.chiu@intel.com, nathaniel.l.desimone@intel.com, eric.dong@intel.com, michael.d.kinney@intel.com, zailiang.sun@intel.com, yi.qian@intel.com, graeme@nuviainc.com, =?UTF-8?Q?Rados=C5=82aw_Biernacki?= , Pete Batard Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi, Thanks for the comment, I will move that function to AuthVariableLib. greg czw., 17 cze 2021 o 04:35 gaoliming napisa=C5= =82(a): > > Grzegorz: > MdeModulePkg is generic base package. It should not depend on Security= Pkg. > > 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 > > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > > =E5=8F=91=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io =E4=BB=A3=E8=A1=A8 Grzegorz > > Bernacki > > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2021=E5=B9=B46=E6=9C=8814=E6=97= =A5 17:43 > > =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io > > =E6=8A=84=E9=80=81: leif@nuviainc.com; ardb+tianocore@kernel.org; > > Samer.El-Haj-Mahmoud@arm.com; sunny.Wang@arm.com; > > mw@semihalf.com; upstream@semihalf.com; jiewen.yao@intel.com; > > jian.j.wang@intel.com; min.m.xu@intel.com; lersek@redhat.com; > > sami.mujawar@arm.com; afish@apple.com; ray.ni@intel.com; > > jordan.l.justen@intel.com; rebecca@bsdio.com; grehan@freebsd.org; > > thomas.abraham@arm.com; chasel.chiu@intel.com; > > nathaniel.l.desimone@intel.com; gaoliming@byosoft.com.cn; > > eric.dong@intel.com; michael.d.kinney@intel.com; zailiang.sun@intel.co= m; > > yi.qian@intel.com; graeme@nuviainc.com; rad@semihalf.com; pete@akeo.ie= ; > > Grzegorz Bernacki > > =E4=B8=BB=E9=A2=98: [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 > > --- > > 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 > > #include > > #include > > +#include > > > > #include > > #include > > 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 th= is > > 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 f= ree > > 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 =3D=3D NULL || DataSize =3D=3D NULL) { > > - return EFI_INVALID_PARAMETER; > > - } > > - > > - // > > - // At user physical presence, the variable does not need to be sign= ed > but > > the > > - // parameters to the SetVariable() call still need to be prepared a= s > > authenticated > > - // variable. So we create EFI_VARIABLE_AUTHENTICATED_2 descriptor > > without certificate > > - // data in it. > > - // > > - Payload =3D *Data; > > - PayloadSize =3D *DataSize; > > - > > - DescriptorSize =3D OFFSET_OF (EFI_VARIABLE_AUTHENTICATION_2, > > AuthInfo) + OFFSET_OF (WIN_CERTIFICATE_UEFI_GUID, CertData); > > - NewData =3D (UINT8 *) AllocateZeroPool (DescriptorSize + PayloadSiz= e); > > - if (NewData =3D=3D NULL) { > > - return EFI_OUT_OF_RESOURCES; > > - } > > - > > - if ((Payload !=3D NULL) && (PayloadSize !=3D 0)) { > > - CopyMem (NewData + DescriptorSize, Payload, PayloadSize); > > - } > > - > > - DescriptorData =3D (EFI_VARIABLE_AUTHENTICATION_2 *) (NewData); > > - > > - ZeroMem (&Time, sizeof (EFI_TIME)); > > - Status =3D gRT->GetTime (&Time, NULL); > > - if (EFI_ERROR (Status)) { > > - FreePool (NewData); > > - return Status; > > - } > > - Time.Pad1 =3D 0; > > - Time.Nanosecond =3D 0; > > - Time.TimeZone =3D 0; > > - Time.Daylight =3D 0; > > - Time.Pad2 =3D 0; > > - CopyMem (&DescriptorData->TimeStamp, &Time, sizeof (EFI_TIME)); > > - > > - DescriptorData->AuthInfo.Hdr.dwLength =3D OFFSET_OF > > (WIN_CERTIFICATE_UEFI_GUID, CertData); > > - DescriptorData->AuthInfo.Hdr.wRevision =3D 0x0200; > > - DescriptorData->AuthInfo.Hdr.wCertificateType =3D > > WIN_CERT_TYPE_EFI_GUID; > > - CopyGuid (&DescriptorData->AuthInfo.CertType, &gEfiCertPkcs7Guid); > > - > > - if (Payload !=3D NULL) { > > - FreePool (Payload); > > - } > > - > > - *DataSize =3D DescriptorSize + PayloadSize; > > - *Data =3D 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 th= is > > function. > > -- > > 2.25.1 > > > > > > > > > > > > > > > >=20 > >