SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 60 ++++++++------------ 1 file changed, 24 insertions(+), 36 deletions(-)
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3461
The UEFI specification prohibits loading any UEFI image of which a
matching SHA-256 hash is contained in "dbx" (UEFI 2.9, 32.5.3.3
"Authorization Process", 3.A). Currently, this is only explicitly
checked when the image is unsigned and otherwise the hash algorithms
of the certificates are used.
Align with the UEFI specification by specifically looking up the
SHA-256 hash of the image in "dbx".
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
---
SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 60 ++++++++------------
1 file changed, 24 insertions(+), 36 deletions(-)
diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index c48861cd6496..1f9bb33e86c3 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -1803,34 +1803,36 @@ DxeImageVerificationHandler (
}
}
+ //
+ // The SHA256 hash value of the image must not be reflected in the security data base "dbx".
+ //
+ if (!HashPeImage (HASHALG_SHA256)) {
+ DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Failed to hash this image using %s.\n", mHashTypeStr));
+ goto Failed;
+ }
+
+ DbStatus = IsSignatureFoundInDatabase (
+ EFI_IMAGE_SECURITY_DATABASE1,
+ mImageDigest,
+ &mCertType,
+ mImageDigestSize,
+ &IsFound
+ );
+ if (EFI_ERROR (DbStatus) || IsFound) {
+ //
+ // Image Hash is in forbidden database (DBX).
+ //
+ DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is not signed and %s hash of image is forbidden by DBX.\n", mHashTypeStr));
+ goto Failed;
+ }
+
//
// Start Image Validation.
//
if (SecDataDir == NULL || SecDataDir->Size == 0) {
//
- // This image is not signed. The SHA256 hash value of the image must match a record in the security database "db",
- // and not be reflected in the security data base "dbx".
+ // This image is not signed. The SHA256 hash value of the image must match a record in the security database "db".
//
- if (!HashPeImage (HASHALG_SHA256)) {
- DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Failed to hash this image using %s.\n", mHashTypeStr));
- goto Failed;
- }
-
- DbStatus = IsSignatureFoundInDatabase (
- EFI_IMAGE_SECURITY_DATABASE1,
- mImageDigest,
- &mCertType,
- mImageDigestSize,
- &IsFound
- );
- if (EFI_ERROR (DbStatus) || IsFound) {
- //
- // Image Hash is in forbidden database (DBX).
- //
- DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is not signed and %s hash of image is forbidden by DBX.\n", mHashTypeStr));
- goto Failed;
- }
-
DbStatus = IsSignatureFoundInDatabase (
EFI_IMAGE_SECURITY_DATABASE,
mImageDigest,
@@ -1932,20 +1934,6 @@ DxeImageVerificationHandler (
//
// Check the image's hash value.
//
- DbStatus = IsSignatureFoundInDatabase (
- EFI_IMAGE_SECURITY_DATABASE1,
- mImageDigest,
- &mCertType,
- mImageDigestSize,
- &IsFound
- );
- if (EFI_ERROR (DbStatus) || IsFound) {
- Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND;
- DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed but %s hash of image is found in DBX.\n", mHashTypeStr));
- IsVerified = FALSE;
- break;
- }
-
if (!IsVerified) {
DbStatus = IsSignatureFoundInDatabase (
EFI_IMAGE_SECURITY_DATABASE,
--
2.31.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78889): https://edk2.groups.io/g/devel/message/78889
Mute This Topic: https://groups.io/mt/84754063/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On August 9, 2021 3:40 AM, Marvin Häuser wrote: > Subject: [PATCH] SecurityPkg/DxeImageVerificationLib: Always lookup SHA-256 > hash in dbx > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3461 > > The UEFI specification prohibits loading any UEFI image of which a matching > SHA-256 hash is contained in "dbx" (UEFI 2.9, 32.5.3.3 "Authorization Process", > 3.A). Currently, this is only explicitly checked when the image is unsigned and > otherwise the hash algorithms of the certificates are used. > > Align with the UEFI specification by specifically looking up the > SHA-256 hash of the image in "dbx". > > Cc: Jiewen Yao <jiewen.yao@intel.com> > Cc: Jian J Wang <jian.j.wang@intel.com> > Cc: Min Xu <min.m.xu@intel.com> > Cc: Vitaly Cheptsov <vit9696@protonmail.com> > Signed-off-by: Marvin Häuser <mhaeuser@posteo.de> > --- It seems there are 3 patches sent from Marvin Häuser and I suppose they're in one patch-set, right? Please follow the link below to send out patch-set for review. https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers For example, if there are 3 commits in one patch-set, then the subject of the commits looks like: [PATCH 0/4] This is the cover letter [PATCH 1/4] This is patch 1 Otherwise the reviewers are confused by the patches. Thanks! Xu, Min -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#78909): https://edk2.groups.io/g/devel/message/78909 Mute This Topic: https://groups.io/mt/84754063/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Good day, I just woke up to this mess, yes. I actually did follow that guide, just around 3 years ago I believe, so let me check where things went wrong since then... The patch you quoted was a standalone patch. However, for some reason, none of the other patch series had indices appended. I'm sure I can get that fixed shortly, but what to do then, re-send the entire bulk? I don't want to spam the list, maybe it is smarter to group them by some overview mail this one time? Sorry for the disruption! Best regards, Marvin On 09/08/2021 02:02, Min Xu wrote: > On August 9, 2021 3:40 AM, Marvin Häuser wrote: >> Subject: [PATCH] SecurityPkg/DxeImageVerificationLib: Always lookup SHA-256 >> hash in dbx >> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3461 >> >> The UEFI specification prohibits loading any UEFI image of which a matching >> SHA-256 hash is contained in "dbx" (UEFI 2.9, 32.5.3.3 "Authorization Process", >> 3.A). Currently, this is only explicitly checked when the image is unsigned and >> otherwise the hash algorithms of the certificates are used. >> >> Align with the UEFI specification by specifically looking up the >> SHA-256 hash of the image in "dbx". >> >> Cc: Jiewen Yao <jiewen.yao@intel.com> >> Cc: Jian J Wang <jian.j.wang@intel.com> >> Cc: Min Xu <min.m.xu@intel.com> >> Cc: Vitaly Cheptsov <vit9696@protonmail.com> >> Signed-off-by: Marvin Häuser <mhaeuser@posteo.de> >> --- > It seems there are 3 patches sent from Marvin Häuser and I suppose they're in one patch-set, right? Please follow the link below to send out patch-set for review. > https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers > For example, if there are 3 commits in one patch-set, then the subject of the commits looks like: > [PATCH 0/4] This is the cover letter > [PATCH 1/4] This is patch 1 > > Otherwise the reviewers are confused by the patches. > > Thanks! > Xu, Min > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#78917): https://edk2.groups.io/g/devel/message/78917 Mute This Topic: https://groups.io/mt/84754063/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi Marvin
With this patch, the path "Action == EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND" no longer exists.
Do you think we should remove EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND as well?
Thank you
Yao Jiewen
> -----Original Message-----
> From: Marvin Häuser <mhaeuser@posteo.de>
> Sent: Monday, August 9, 2021 3:40 AM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Xu, Min M <min.m.xu@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> Subject: [PATCH] SecurityPkg/DxeImageVerificationLib: Always lookup SHA-256
> hash in dbx
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3461
>
> The UEFI specification prohibits loading any UEFI image of which a
> matching SHA-256 hash is contained in "dbx" (UEFI 2.9, 32.5.3.3
> "Authorization Process", 3.A). Currently, this is only explicitly
> checked when the image is unsigned and otherwise the hash algorithms
> of the certificates are used.
>
> Align with the UEFI specification by specifically looking up the
> SHA-256 hash of the image in "dbx".
>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Min Xu <min.m.xu@intel.com>
> Cc: Vitaly Cheptsov <vit9696@protonmail.com>
> Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
> ---
> SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 60
> ++++++++------------
> 1 file changed, 24 insertions(+), 36 deletions(-)
>
> diff --git
> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> index c48861cd6496..1f9bb33e86c3 100644
> --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> @@ -1803,34 +1803,36 @@ DxeImageVerificationHandler (
> }
>
> }
>
>
>
> + //
>
> + // The SHA256 hash value of the image must not be reflected in the security
> data base "dbx".
>
> + //
>
> + if (!HashPeImage (HASHALG_SHA256)) {
>
> + DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Failed to hash this image
> using %s.\n", mHashTypeStr));
>
> + goto Failed;
>
> + }
>
> +
>
> + DbStatus = IsSignatureFoundInDatabase (
>
> + EFI_IMAGE_SECURITY_DATABASE1,
>
> + mImageDigest,
>
> + &mCertType,
>
> + mImageDigestSize,
>
> + &IsFound
>
> + );
>
> + if (EFI_ERROR (DbStatus) || IsFound) {
>
> + //
>
> + // Image Hash is in forbidden database (DBX).
>
> + //
>
> + DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is not signed
> and %s hash of image is forbidden by DBX.\n", mHashTypeStr));
>
> + goto Failed;
>
> + }
>
> +
>
> //
>
> // Start Image Validation.
>
> //
>
> if (SecDataDir == NULL || SecDataDir->Size == 0) {
>
> //
>
> - // This image is not signed. The SHA256 hash value of the image must match
> a record in the security database "db",
>
> - // and not be reflected in the security data base "dbx".
>
> + // This image is not signed. The SHA256 hash value of the image must match
> a record in the security database "db".
>
> //
>
> - if (!HashPeImage (HASHALG_SHA256)) {
>
> - DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Failed to hash this image
> using %s.\n", mHashTypeStr));
>
> - goto Failed;
>
> - }
>
> -
>
> - DbStatus = IsSignatureFoundInDatabase (
>
> - EFI_IMAGE_SECURITY_DATABASE1,
>
> - mImageDigest,
>
> - &mCertType,
>
> - mImageDigestSize,
>
> - &IsFound
>
> - );
>
> - if (EFI_ERROR (DbStatus) || IsFound) {
>
> - //
>
> - // Image Hash is in forbidden database (DBX).
>
> - //
>
> - DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is not signed
> and %s hash of image is forbidden by DBX.\n", mHashTypeStr));
>
> - goto Failed;
>
> - }
>
> -
>
> DbStatus = IsSignatureFoundInDatabase (
>
> EFI_IMAGE_SECURITY_DATABASE,
>
> mImageDigest,
>
> @@ -1932,20 +1934,6 @@ DxeImageVerificationHandler (
> //
>
> // Check the image's hash value.
>
> //
>
> - DbStatus = IsSignatureFoundInDatabase (
>
> - EFI_IMAGE_SECURITY_DATABASE1,
>
> - mImageDigest,
>
> - &mCertType,
>
> - mImageDigestSize,
>
> - &IsFound
>
> - );
>
> - if (EFI_ERROR (DbStatus) || IsFound) {
>
> - Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND;
>
> - DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed but %s
> hash of image is found in DBX.\n", mHashTypeStr));
>
> - IsVerified = FALSE;
>
> - break;
>
> - }
>
> -
>
> if (!IsVerified) {
>
> DbStatus = IsSignatureFoundInDatabase (
>
> EFI_IMAGE_SECURITY_DATABASE,
>
> --
> 2.31.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78914): https://edk2.groups.io/g/devel/message/78914
Mute This Topic: https://groups.io/mt/84754063/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hey Jiewen,
Right, I meant to ask about this and forgot (sorry, I sent out a bit
less than 30 patches yesterday :) ).
Why do we record and potentially defer the loading of images that are
distrusted by dbx?
I would expect any image explicitly distrusted (not just untrusted) to
be rejected and unloaded immediately.
Sorry if I got wrong what is happening!
Best regards,
Marvin
On 09/08/2021 04:48, Yao, Jiewen wrote:
> Hi Marvin
> With this patch, the path "Action == EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND" no longer exists.
>
> Do you think we should remove EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND as well?
>
>
>
> Thank you
> Yao Jiewen
>
>
>> -----Original Message-----
>> From: Marvin Häuser <mhaeuser@posteo.de>
>> Sent: Monday, August 9, 2021 3:40 AM
>> To: devel@edk2.groups.io
>> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
>> Xu, Min M <min.m.xu@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
>> Subject: [PATCH] SecurityPkg/DxeImageVerificationLib: Always lookup SHA-256
>> hash in dbx
>>
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3461
>>
>> The UEFI specification prohibits loading any UEFI image of which a
>> matching SHA-256 hash is contained in "dbx" (UEFI 2.9, 32.5.3.3
>> "Authorization Process", 3.A). Currently, this is only explicitly
>> checked when the image is unsigned and otherwise the hash algorithms
>> of the certificates are used.
>>
>> Align with the UEFI specification by specifically looking up the
>> SHA-256 hash of the image in "dbx".
>>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Cc: Jian J Wang <jian.j.wang@intel.com>
>> Cc: Min Xu <min.m.xu@intel.com>
>> Cc: Vitaly Cheptsov <vit9696@protonmail.com>
>> Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
>> ---
>> SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 60
>> ++++++++------------
>> 1 file changed, 24 insertions(+), 36 deletions(-)
>>
>> diff --git
>> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
>> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
>> index c48861cd6496..1f9bb33e86c3 100644
>> --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
>> +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
>> @@ -1803,34 +1803,36 @@ DxeImageVerificationHandler (
>> }
>>
>> }
>>
>>
>>
>> + //
>>
>> + // The SHA256 hash value of the image must not be reflected in the security
>> data base "dbx".
>>
>> + //
>>
>> + if (!HashPeImage (HASHALG_SHA256)) {
>>
>> + DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Failed to hash this image
>> using %s.\n", mHashTypeStr));
>>
>> + goto Failed;
>>
>> + }
>>
>> +
>>
>> + DbStatus = IsSignatureFoundInDatabase (
>>
>> + EFI_IMAGE_SECURITY_DATABASE1,
>>
>> + mImageDigest,
>>
>> + &mCertType,
>>
>> + mImageDigestSize,
>>
>> + &IsFound
>>
>> + );
>>
>> + if (EFI_ERROR (DbStatus) || IsFound) {
>>
>> + //
>>
>> + // Image Hash is in forbidden database (DBX).
>>
>> + //
>>
>> + DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is not signed
>> and %s hash of image is forbidden by DBX.\n", mHashTypeStr));
>>
>> + goto Failed;
>>
>> + }
>>
>> +
>>
>> //
>>
>> // Start Image Validation.
>>
>> //
>>
>> if (SecDataDir == NULL || SecDataDir->Size == 0) {
>>
>> //
>>
>> - // This image is not signed. The SHA256 hash value of the image must match
>> a record in the security database "db",
>>
>> - // and not be reflected in the security data base "dbx".
>>
>> + // This image is not signed. The SHA256 hash value of the image must match
>> a record in the security database "db".
>>
>> //
>>
>> - if (!HashPeImage (HASHALG_SHA256)) {
>>
>> - DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Failed to hash this image
>> using %s.\n", mHashTypeStr));
>>
>> - goto Failed;
>>
>> - }
>>
>> -
>>
>> - DbStatus = IsSignatureFoundInDatabase (
>>
>> - EFI_IMAGE_SECURITY_DATABASE1,
>>
>> - mImageDigest,
>>
>> - &mCertType,
>>
>> - mImageDigestSize,
>>
>> - &IsFound
>>
>> - );
>>
>> - if (EFI_ERROR (DbStatus) || IsFound) {
>>
>> - //
>>
>> - // Image Hash is in forbidden database (DBX).
>>
>> - //
>>
>> - DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is not signed
>> and %s hash of image is forbidden by DBX.\n", mHashTypeStr));
>>
>> - goto Failed;
>>
>> - }
>>
>> -
>>
>> DbStatus = IsSignatureFoundInDatabase (
>>
>> EFI_IMAGE_SECURITY_DATABASE,
>>
>> mImageDigest,
>>
>> @@ -1932,20 +1934,6 @@ DxeImageVerificationHandler (
>> //
>>
>> // Check the image's hash value.
>>
>> //
>>
>> - DbStatus = IsSignatureFoundInDatabase (
>>
>> - EFI_IMAGE_SECURITY_DATABASE1,
>>
>> - mImageDigest,
>>
>> - &mCertType,
>>
>> - mImageDigestSize,
>>
>> - &IsFound
>>
>> - );
>>
>> - if (EFI_ERROR (DbStatus) || IsFound) {
>>
>> - Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND;
>>
>> - DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed but %s
>> hash of image is found in DBX.\n", mHashTypeStr));
>>
>> - IsVerified = FALSE;
>>
>> - break;
>>
>> - }
>>
>> -
>>
>> if (!IsVerified) {
>>
>> DbStatus = IsSignatureFoundInDatabase (
>>
>> EFI_IMAGE_SECURITY_DATABASE,
>>
>> --
>> 2.31.1
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78919): https://edk2.groups.io/g/devel/message/78919
Mute This Topic: https://groups.io/mt/84754063/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.