From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mx.groups.io with SMTP id smtpd.web12.3751.1581584560926607918 for ; Thu, 13 Feb 2020 01:02:41 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.126, mailfrom: chao.b.zhang@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Feb 2020 01:02:39 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,436,1574150400"; d="scan'208";a="267005912" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga002.fm.intel.com with ESMTP; 13 Feb 2020 01:02:39 -0800 Received: from shsmsx601.ccr.corp.intel.com (10.109.6.141) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 13 Feb 2020 01:02:39 -0800 Received: from shsmsx602.ccr.corp.intel.com (10.109.6.142) by SHSMSX601.ccr.corp.intel.com (10.109.6.141) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Thu, 13 Feb 2020 17:02:37 +0800 Received: from shsmsx602.ccr.corp.intel.com ([10.109.6.142]) by SHSMSX602.ccr.corp.intel.com ([10.109.6.142]) with mapi id 15.01.1713.004; Thu, 13 Feb 2020 17:02:37 +0800 From: "Zhang, Chao B" To: "devel@edk2.groups.io" , "Wang, Jian J" CC: "Yao, Jiewen" Subject: Re: [edk2-devel] [PATCH 9/9] SecurityPkg/DxeImageVerificationLib: Differentiate error and search result in IsSignatureFoundInDatabase(CVE-2019-14575) Thread-Topic: [edk2-devel] [PATCH 9/9] SecurityPkg/DxeImageVerificationLib: Differentiate error and search result in IsSignatureFoundInDatabase(CVE-2019-14575) Thread-Index: AQHV3Ph+AVI6DAUv0kGlOrzfatdrX6gY3qJw Date: Thu, 13 Feb 2020 09:02:37 +0000 Message-ID: <713ae40e107f48409602006ff45e1b4a@intel.com> References: <20200206141933.356-1-jian.j.wang@intel.com> <20200206141933.356-10-jian.j.wang@intel.com> In-Reply-To: <20200206141933.356-10-jian.j.wang@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNGJmNmE4YWYtOWEzZi00ZDI3LThiYjgtZDkyMGI0MzEzMDI0IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoidUFrYllmcXlNTU1wNVp3N1hOaXdqZzJhTndTTG91aThtNlhEYTllRHg1Sit5MEpJNUdTMGdPcWNuWkN6VXlMZSJ9 dlp-version: 11.2.0.6 dlp-product: dlpe-windows x-ctpclassification: CTP_NT dlp-reaction: no-action x-originating-ip: [10.239.127.36] MIME-Version: 1.0 Return-Path: chao.b.zhang@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Ack-by : Chao Zhang -----Original Message----- From: devel@edk2.groups.io On Behalf Of Wang, Jian J Sent: Thursday, February 6, 2020 10:20 PM To: devel@edk2.groups.io Cc: Yao, Jiewen ; Zhang, Chao B Subject: [edk2-devel] [PATCH 9/9] SecurityPkg/DxeImageVerificationLib: Diff= erentiate error and search result in IsSignatureFoundInDatabase(CVE-2019-14= 575) REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1608 To avoid false-negative issue in check hash against dbx, both error conditi= on (as return value) and check result (as out parameter) of IsSignatureFoundInDatabase() are added. So the caller of this function will= know exactly if a failure is caused by a black list hit or other error hap= pening, and enforce a more secure operation to prevent secure boot from bei= ng bypassed. For a white list check (db), there's no such necessity. All intermediate results inside this function will be checked and returned = immediately upon any failure or error, like out-of-resource, hash calculati= on error or certificate retrieval failure. Cc: Jiewen Yao Cc: Chao Zhang Signed-off-by: Jian J Wang --- .../DxeImageVerificationLib.c | 77 ++++++++++++++----- 1 file changed, 58 insertions(+), 19 deletions(-) diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificati= onLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL= ib.c index 5b7a67f811..8e599ca0be 100644 --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLi +++ b.c @@ -955,17 +955,19 @@ Done: @param[in] Signature Pointer to signature that is searched fo= r. @param[in] CertType Pointer to hash algorithm. @param[in= ] SignatureSize Size of Signature.+ @param[out] IsFound = Search result. Only valid if EFI_SUCCESS returned - @return TRUE = Found the signature in the variable database.- @return FALSE = Not found the signature in the variable database.+ @retval= EFI_SUCCESS Finished the search without any error.+ @retval O= thers Error occurred in the search of database. **/-BOOLE= AN+EFI_STATUS IsSignatureFoundInDatabase (- IN CHAR16 *Variabl= eName,- IN UINT8 *Signature,- IN EFI_GUID *CertTyp= e,- IN UINTN SignatureSize+ IN CHAR16 *VariableN= ame,+ IN UINT8 *Signature,+ IN EFI_GUID *CertType,= + IN UINTN SignatureSize,+ OUT BOOLEAN *IsFound = ) { EFI_STATUS Status;@@ -975,22 +977,28 @@ IsSignatureFoundInDa= tabase ( UINT8 *Data; UINTN Index; UINTN = CertCount;- BOOLEAN IsFound; // // Read signature d= atabase variable. //- IsFound =3D FALSE;+ *IsFound =3D FALSE; Data= =3D NULL; DataSize =3D 0; Status =3D gRT->GetVariable (Variab= leName, &gEfiImageSecurityDatabaseGuid, NULL, &DataSize, NULL); if (Statu= s !=3D EFI_BUFFER_TOO_SMALL) {- return FALSE;+ if (Status =3D=3D EFI_= NOT_FOUND) {+ //+ // No database, no need to search.+ //+ = Status =3D EFI_SUCCESS;+ }++ return Status; } Data =3D (UINT8= *) AllocateZeroPool (DataSize); if (Data =3D=3D NULL) {- return FALSE= ;+ return EFI_OUT_OF_RESOURCES; } Status =3D gRT->GetVariable (Vari= ableName, &gEfiImageSecurityDatabaseGuid, NULL, &DataSize, Data);@@ -1010,7= +1018,7 @@ IsSignatureFoundInDatabase ( // // Find the signature in database. //- = IsFound =3D TRUE;+ *IsFound =3D TRUE; // = // Entries in UEFI_IMAGE_SECURITY_DATABASE that are used to validate imag= e should be measured //@@ -1023,7 +1031,7 @@ IsSignatureFoundInDa= tabase ( Cert =3D (EFI_SIGNATURE_DATA *) ((UINT8 *) Cert + CertList->Signat= ureSize); } - if (IsFound) {+ if (*IsFound) { break= ; } }@@ -1037,7 +1045,7 @@ Done: FreePool (Data); } - return IsFound;+ return Status; } /**@@ -164= 2,6 +1650,8 @@ DxeImageVerificationHandler ( CHAR16 *NameStr; RETURN_STATUS = PeCoffStatus; EFI_STATUS HashStat= us;+ EFI_STATUS DbStatus;+ BOOLEAN = IsFound; SignatureList =3D NULL; SignatureListSiz= e =3D 0;@@ -1650,7 +1660,7 @@ DxeImageVerificationHandler ( PkcsCertData =3D NULL; Action =3D EFI_IMAGE_EXECUTION_= AUTH_UNTESTED; IsVerified =3D FALSE;-+ IsFound =3D FALS= E; // // Check the image type and get policy setting.@@ -1792,7 +1802,= 14 @@ DxeImageVerificationHandler ( goto Failed; } - if (IsSignatureFoundInDatabase (EFI_IMAGE_SE= CURITY_DATABASE1, mImageDigest, &mCertType, mImageDigestSize)) {+ DbStat= us =3D IsSignatureFoundInDatabase (+ EFI_IMAGE_SECURITY_DAT= ABASE1,+ mImageDigest,+ &mCertType,+ = mImageDigestSize,+ &IsFound+ );+ = if (EFI_ERROR (DbStatus) || IsFound) { // // Image Hash is i= n forbidden database (DBX). //@@ -1800,7 +1817,14 @@ DxeImageVerifica= tionHandler ( goto Failed; } - if (IsSignatureFoundInDatabase (EFI_IMAGE_SE= CURITY_DATABASE, mImageDigest, &mCertType, mImageDigestSize)) {+ DbStatu= s =3D IsSignatureFoundInDatabase (+ EFI_IMAGE_SECURITY_DATA= BASE,+ mImageDigest,+ &mCertType,+ = mImageDigestSize,+ &IsFound+ );+ = if (!EFI_ERROR (DbStatus) && IsFound) { // // Image Hash is in= allowed database (DB). //@@ -1888,14 +1912,29 @@ DxeImageVerificatio= nHandler ( // // Check the image's hash value. //- if (IsSignatureFoun= dInDatabase (EFI_IMAGE_SECURITY_DATABASE1, mImageDigest, &mCertType, mImage= DigestSize)) {+ DbStatus =3D IsSignatureFoundInDatabase (+ = EFI_IMAGE_SECURITY_DATABASE1,+ mImageDigest,+ = &mCertType,+ mImageDigestSize,+ &IsFo= und+ );+ if (EFI_ERROR (DbStatus) || IsFound) { Ac= tion =3D EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND; DEBUG ((DEBUG_INFO, "Dxe= ImageVerificationLib: Image is signed but %s hash of image is found in DBX.= \n", mHashTypeStr)); IsVerified =3D FALSE; break; }+ if= (!IsVerified) {- if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_D= ATABASE, mImageDigest, &mCertType, mImageDigestSize)) {+ DbStatus =3D = IsSignatureFoundInDatabase (+ EFI_IMAGE_SECURITY_DATABASE= ,+ mImageDigest,+ &mCertType,+ = mImageDigestSize,+ &IsFound+ = );+ if (!EFI_ERROR (DbStatus) && IsFound) { IsVerified =3D TRU= E; } else { DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Ima= ge is signed but signature is not allowed by DB and %s hash of image is not= found in DB/DBX.\n", mHashTypeStr));--=20 2.24.0.windows.2 -=3D-=3D-=3D-=3D-=3D-=3D Groups.io Links: You receive all messages sent to this group. View/Reply Online (#53875): https://edk2.groups.io/g/devel/message/53875 Mute This Topic: https://groups.io/mt/71023427/1790519 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [chao.b.zhang@intel.com]= -=3D-=3D-=3D-=3D-=3D-=3D