From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout02.posteo.de (mout02.posteo.de [185.67.36.66]) by mx.groups.io with SMTP id smtpd.web09.18981.1628487724824411335 for ; Sun, 08 Aug 2021 22:42:05 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@posteo.de header.s=2017 header.b=H7/SH9XO; spf=pass (domain: posteo.de, ip: 185.67.36.66, mailfrom: mhaeuser@posteo.de) Received: from submission (posteo.de [89.146.220.130]) by mout02.posteo.de (Postfix) with ESMTPS id 07473240105 for ; Mon, 9 Aug 2021 07:42:02 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017; t=1628487723; bh=r8hDje6ptQRo3t9h346wEj1SYHIQ8UnEKxgMdmTc2PE=; h=Subject:To:Cc:From:Date:From; b=H7/SH9XOHQxN1ICTRBOTYs+KgoLxFngFhOvd75Qt9zEETMnrKfze5oxRrXREDP+uM 3LAX+tBChwxPXvMunYZRf88dIEo9JEXTkJ6bgIKxvlyoS6+a/52oNm05XZXZACsrwk 6Dn6NDcRUj5zTFB34WfkViM885YMh/yVGZxSz1MsTqts4OtvH3kFHD4Xyy6VFOXEn9 TafFQpCGSzKcRYCegMtWGLpuYv/Niv7BE58URCsDb64TrYi2fa9eyXsxivN/PTHpY1 uSmzuDHZ2ONXfzqwwEr0m81ze2od5mD5clFsQ7Az0v5ejfRACQBhhdUa7Ru6RytRBG AipXIdOOd/lZg== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4GjlMZ085Gz9rxK; Mon, 9 Aug 2021 07:42:02 +0200 (CEST) Subject: Re: [edk2-devel] [PATCH] SecurityPkg/DxeImageVerificationLib: Always lookup SHA-256 hash in dbx To: devel@edk2.groups.io, jiewen.yao@intel.com Cc: "Wang, Jian J" , "Xu, Min M" , Vitaly Cheptsov References: <5df11a13422732b9c03c120775a2b4dd0a49182f.1628444003.git.mhaeuser@posteo.de> <6810bb96b0c7ef377680112f48bac9cd0a964a52.1628353537.git.mhaeuser@posteo.de> From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= Message-ID: <320346fe-0083-45d7-ac8c-1dac7bec2a32@posteo.de> Date: Mon, 9 Aug 2021 05:42:01 +0000 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: quoted-printable Hey Jiewen, Right, I meant to ask about this and forgot (sorry, I sent out a bit=20 less than 30 patches yesterday :) ). Why do we record and potentially defer the loading of images that are=20 distrusted by dbx? I would expect any image explicitly distrusted (not just untrusted) to=20 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 =3D=3D EFI_IMAGE_EXECUTION_AUTH_SIG_FOU= ND" 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=C3=A4user >> Sent: Monday, August 9, 2021 3:40 AM >> To: devel@edk2.groups.io >> Cc: Yao, Jiewen ; Wang, Jian J ; >> Xu, Min M ; Vitaly Cheptsov >> Subject: [PATCH] SecurityPkg/DxeImageVerificationLib: Always lookup SHA-= 256 >> hash in dbx >> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3461 >> >> 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 >> Cc: Jian J Wang >> Cc: Min Xu >> Cc: Vitaly Cheptsov >> Signed-off-by: Marvin H=C3=A4user >> --- >> 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/DxeImageVerificationLi= b.c >> +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLi= b.c >> @@ -1803,34 +1803,36 @@ DxeImageVerificationHandler ( >> } >> >> } >> >> >> >> + // >> >> + // The SHA256 hash value of the image must not be reflected in the se= curity >> data base "dbx". >> >> + // >> >> + if (!HashPeImage (HASHALG_SHA256)) { >> >> + DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Failed to hash this i= mage >> using %s.\n", mHashTypeStr)); >> >> + goto Failed; >> >> + } >> >> + >> >> + DbStatus =3D 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 =3D=3D NULL || SecDataDir->Size =3D=3D 0) { >> >> // >> >> - // This image is not signed. The SHA256 hash value of the image mus= t 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 mus= t 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 =3D 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 =3D IsSignatureFoundInDatabase ( >> >> EFI_IMAGE_SECURITY_DATABASE, >> >> mImageDigest, >> >> @@ -1932,20 +1934,6 @@ DxeImageVerificationHandler ( >> // >> >> // Check the image's hash value. >> >> // >> >> - DbStatus =3D IsSignatureFoundInDatabase ( >> >> - EFI_IMAGE_SECURITY_DATABASE1, >> >> - mImageDigest, >> >> - &mCertType, >> >> - mImageDigestSize, >> >> - &IsFound >> >> - ); >> >> - if (EFI_ERROR (DbStatus) || IsFound) { >> >> - Action =3D 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 =3D FALSE; >> >> - break; >> >> - } >> >> - >> >> if (!IsVerified) { >> >> DbStatus =3D IsSignatureFoundInDatabase ( >> >> EFI_IMAGE_SECURITY_DATABASE, >> >> -- >> 2.31.1 > > >=20 > >