Re: [edk2-devel] [PATCH v2 1/1] SecurityPkg/DxeImageVerificationLib: Check result of GetEfiGlobalVariable2

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

Merged https://github.com/tianocore/edk2/pull/4155


> -----Original Message-----
> From: Yao, Jiewen
> Sent: Tuesday, March 21, 2023 10:29 AM
> To: kraxel@redhat.com; devel@edk2.groups.io
> Cc: Pawel Polawski <ppolawsk@redhat.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Oliver Steffen <osteffen@redhat.com>; Xu, Min M
> <min.m.xu@intel.com>; Marvin Häuser <mhaeuser@posteo.de>;
> jmaloy@redhat.com
> Subject: RE: [edk2-devel] [PATCH v2 1/1]
> SecurityPkg/DxeImageVerificationLib: Check result of GetEfiGlobalVariable2
> 
> Sounds good. Thanks.
> 
> Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
> 
> > -----Original Message-----
> > From: kraxel@redhat.com <kraxel@redhat.com>
> > Sent: Monday, March 20, 2023 11:00 PM
> > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> > Cc: Pawel Polawski <ppolawsk@redhat.com>; Wang, Jian J
> > <jian.j.wang@intel.com>; Oliver Steffen <osteffen@redhat.com>; Xu, Min
> M
> > <min.m.xu@intel.com>; Marvin Häuser <mhaeuser@posteo.de>;
> > jmaloy@redhat.com
> > Subject: Re: [edk2-devel] [PATCH v2 1/1]
> > SecurityPkg/DxeImageVerificationLib: Check result of
> GetEfiGlobalVariable2
> >
> > On Mon, Mar 20, 2023 at 01:20:29PM +0000, Yao, Jiewen wrote:
> > > Would you please share with us what test has been done for this patch?
> >
> > Usual regression testing, including booting images with and without
> > secure boot.  Additionally checked images with the wrong signature
> > are rejected (try boot grub.efi directly instead of using the
> > shim.efi -> grub.efi chain).
> >
> > take care,
> >   Gerd
> >
> > >
> > > Thank you
> > > Yao, Jiewen
> > >
> > > > -----Original Message-----
> > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Gerd
> > > > Hoffmann
> > > > Sent: Monday, March 20, 2023 6:02 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Pawel Polawski <ppolawsk@redhat.com>; Wang, Jian J
> > > > <jian.j.wang@intel.com>; Oliver Steffen <osteffen@redhat.com>; Xu,
> > Min M
> > > > <min.m.xu@intel.com>; Marvin Häuser <mhaeuser@posteo.de>; Yao,
> > > > Jiewen <jiewen.yao@intel.com>; jmaloy@redhat.com
> > > > Subject: Re: [edk2-devel] [PATCH v2 1/1]
> > > > SecurityPkg/DxeImageVerificationLib: Check result of
> > GetEfiGlobalVariable2
> > > >
> > > > On Fri, Mar 03, 2023 at 11:35:53AM +0100, Gerd Hoffmann wrote:
> > > > > Call gRT->GetVariable() directly to read the SecureBoot variable.  It is
> > > > > one byte in size so we can easily place it on the stack instead of
> > > > > having GetEfiGlobalVariable2() allocate it for us, which avoids a few
> > > > > possible error cases.
> > > > >
> > > > > Skip secure boot checks if (and only if):
> > > > >
> > > > >  (a) the SecureBoot variable is not present (EFI_NOT_FOUND)
> according
> > to
> > > > >      the return value, or
> > > > >  (b) the SecureBoot variable was read successfully and is set to
> > > > >      SECURE_BOOT_MODE_DISABLE.
> > > > >
> > > > > Previously the code skipped the secure boot checks on *any*
> > > > > gRT->GetVariable() error (GetEfiGlobalVariable2 sets the variable
> > > > > value to NULL in that case) and also on memory allocation failures.
> > > > >
> > > > > Fixes: CVE-2019-14560
> > > > > Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=2167
> > > > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > > >
> > > > Ping.  Any comments on this patch?
> > > >
> > > > take care,
> > > >   Gerd
> > > >
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> > >
> > > 
> > >
> > >
> >
> > --