[PATCH 1/1] SecurityPkg/DxeImageVerificationLib: Set Action for failed unsigned image

[PATCH 1/1] SecurityPkg/DxeImageVerificationLib: Set Action for failed unsigned image

[Joseph Hemann]

If the image is not signed and the hash of image is not found
in DB/DBX, then the EFI_IMAGE_INFO_ACTION of the load of said
image should be set to,
EFI_IMAGE_EXECUTION_AUTH_SIG_NOT_FOUND, rather then being left
unset as EFI_IMAGE_EXECUTION_AUTH_UNTESTED.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Min Xu <min.m.xu@intel.com>

Signed-off-by: Joseph Hemann <Joseph.Hemann@arm.com>
Change-Id: Ia432ebf4ec811e36d67b80bc438a6aff60bc9b67
---
 .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c    | 1 +
 1 file changed, 1 insertion(+)

diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 0a804af2162f..e5fae732bb1f 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -1848,6 +1848,7 @@ DxeImageVerificationHandler (
     //
     // Image Hash is not found in both forbidden and allowed database.
     //
+    Action = EFI_IMAGE_EXECUTION_AUTH_SIG_NOT_FOUND;
     DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is not signed and %s hash of image is not found in DB/DBX.\n", mHashTypeStr));
     goto Failed;
   }
-- 
2.17.1

Re: [edk2-devel] [PATCH 1/1] SecurityPkg/DxeImageVerificationLib: Set Action for failed unsigned image

[Samer El-Haj-Mahmoud]

Hi Jiewen, Jian, and Min,

Can you please review this patch? We have a corresponding UEFI Spec "code first" ECR (https://bugzilla.tianocore.org/show_bug.cgi?id=3561), and need to clarify a couple of cases in the code.

Thanks,
--Samer

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Joseph
> Hemann via groups.io
> Sent: Tuesday, October 12, 2021 12:57 PM
> To: devel@edk2.groups.io
> Cc: nd <nd@arm.com>; Joseph Hemann <Joseph.Hemann@arm.com>; Jiewen
> Yao <jiewen.yao@intel.com>; Jian J Wang <jian.j.wang@intel.com>; Min Xu
> <min.m.xu@intel.com>; Joseph Hemann <Joseph.Hemann@arm.com>
> Subject: [edk2-devel] [PATCH 1/1] SecurityPkg/DxeImageVerificationLib: Set
> Action for failed unsigned image
> 
> If the image is not signed and the hash of image is not found
> in DB/DBX, then the EFI_IMAGE_INFO_ACTION of the load of said
> image should be set to,
> EFI_IMAGE_EXECUTION_AUTH_SIG_NOT_FOUND, rather then being left
> unset as EFI_IMAGE_EXECUTION_AUTH_UNTESTED.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Min Xu <min.m.xu@intel.com>
> 
> Signed-off-by: Joseph Hemann <Joseph.Hemann@arm.com>
> Change-Id: Ia432ebf4ec811e36d67b80bc438a6aff60bc9b67
> ---
>  .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c    | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git
> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> index 0a804af2162f..e5fae732bb1f 100644
> --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> @@ -1848,6 +1848,7 @@ DxeImageVerificationHandler (
>      //
>      // Image Hash is not found in both forbidden and allowed database.
>      //
> +    Action = EFI_IMAGE_EXECUTION_AUTH_SIG_NOT_FOUND;
>      DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is not signed and %s
> hash of image is not found in DB/DBX.\n", mHashTypeStr));
>      goto Failed;
>    }
> --
> 2.17.1
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH 1/1] SecurityPkg/DxeImageVerificationLib: Set Action for failed unsigned image

[Yao, Jiewen]

Hi Samer
Thanks for the patch.

I overlook this one when I check the title, it is quite similar to previous one. The only difference is signed v.s. unsigned.

It seems make sense. But I have same feedback as previous one.

Would you please:
1) Fila a Bugzilla - https://bugzilla.tianocore.org/ for the issue.
The CodeFirst is only for doc, not for code.

If you need submit v2, I highly recommend you bind those two in one patch set. (Still 2 patches, but in one set.)

Thank you
Yao Jiewen

> -----Original Message-----
> From: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> Sent: Wednesday, October 27, 2021 5:09 AM
> To: devel@edk2.groups.io; Joseph Hemann <Joseph.Hemann@arm.com>
> Cc: nd <nd@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Xu, Min M <min.m.xu@intel.com>; Samer El-Haj-
> Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> Subject: RE: [edk2-devel] [PATCH 1/1] SecurityPkg/DxeImageVerificationLib: Set
> Action for failed unsigned image
> 
> Hi Jiewen, Jian, and Min,
> 
> Can you please review this patch? We have a corresponding UEFI Spec "code
> first" ECR (https://bugzilla.tianocore.org/show_bug.cgi?id=3561), and need to
> clarify a couple of cases in the code.
> 
> Thanks,
> --Samer
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Joseph
> > Hemann via groups.io
> > Sent: Tuesday, October 12, 2021 12:57 PM
> > To: devel@edk2.groups.io
> > Cc: nd <nd@arm.com>; Joseph Hemann <Joseph.Hemann@arm.com>; Jiewen
> > Yao <jiewen.yao@intel.com>; Jian J Wang <jian.j.wang@intel.com>; Min Xu
> > <min.m.xu@intel.com>; Joseph Hemann <Joseph.Hemann@arm.com>
> > Subject: [edk2-devel] [PATCH 1/1] SecurityPkg/DxeImageVerificationLib: Set
> > Action for failed unsigned image
> >
> > If the image is not signed and the hash of image is not found
> > in DB/DBX, then the EFI_IMAGE_INFO_ACTION of the load of said
> > image should be set to,
> > EFI_IMAGE_EXECUTION_AUTH_SIG_NOT_FOUND, rather then being left
> > unset as EFI_IMAGE_EXECUTION_AUTH_UNTESTED.
> >
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Min Xu <min.m.xu@intel.com>
> >
> > Signed-off-by: Joseph Hemann <Joseph.Hemann@arm.com>
> > Change-Id: Ia432ebf4ec811e36d67b80bc438a6aff60bc9b67
> > ---
> >  .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c    | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git
> > a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> > b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> > index 0a804af2162f..e5fae732bb1f 100644
> > --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> > +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> > @@ -1848,6 +1848,7 @@ DxeImageVerificationHandler (
> >      //
> >      // Image Hash is not found in both forbidden and allowed database.
> >      //
> > +    Action = EFI_IMAGE_EXECUTION_AUTH_SIG_NOT_FOUND;
> >      DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is not signed
> and %s
> > hash of image is not found in DB/DBX.\n", mHashTypeStr));
> >      goto Failed;
> >    }
> > --
> > 2.17.1
> >
> >
> >
> > 
> >