From nobody Tue Nov 26 13:37:14 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) client-ip=66.175.222.12; envelope-from=bounce+27952+53875+1787277+3901457@groups.io; helo=web01.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+53875+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=intel.com Received: from web01.groups.io (web01.groups.io [66.175.222.12]) by mx.zohomail.com with SMTPS id 1580998784643585.9656377912642; Thu, 6 Feb 2020 06:19:44 -0800 (PST) Return-Path: X-Received: by 127.0.0.2 with SMTP id zOoOYY1788612xecHUtUoKO8; Thu, 06 Feb 2020 06:19:44 -0800 X-Received: from mga02.intel.com (mga02.intel.com []) by mx.groups.io with SMTP id smtpd.web11.12696.1580998775768725774 for ; Thu, 06 Feb 2020 06:19:43 -0800 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Feb 2020 06:19:43 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,409,1574150400"; d="scan'208";a="226160707" X-Received: from shwdeopensfp777.ccr.corp.intel.com ([10.239.158.78]) by fmsmga008.fm.intel.com with ESMTP; 06 Feb 2020 06:19:42 -0800 From: "Wang, Jian J" To: devel@edk2.groups.io Cc: Jiewen Yao , Chao Zhang Subject: [edk2-devel] [PATCH 9/9] SecurityPkg/DxeImageVerificationLib: Differentiate error and search result in IsSignatureFoundInDatabase(CVE-2019-14575) Date: Thu, 6 Feb 2020 22:19:33 +0800 Message-Id: <20200206141933.356-10-jian.j.wang@intel.com> In-Reply-To: <20200206141933.356-1-jian.j.wang@intel.com> References: <20200206141933.356-1-jian.j.wang@intel.com> MIME-Version: 1.0 Precedence: Bulk List-Unsubscribe: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,jian.j.wang@intel.com X-Gm-Message-State: DDfIkJCrw7LldOJ2Hb6HrwGrx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1580998784; bh=xOOK52d+JiJ9tgH2mr3QWsL+NQjSAqK70IR4L/Q2/10=; h=Cc:Date:From:Reply-To:Subject:To; b=rw8vJ36OWUAPVpD26y7yrCbzRousRd5MsdMyURu1nb+pxYpHhPYZNeFatyQsKlWHntp +lQ229rsCwMqm/WQQF/reT8gC4m8ZzofRLLWNaO6EyVgMzxa2CQ4ZcdTGmxRlheXF6QJ3 w1Xj7QlSupjUZ1TtcrnoZ3ckcdv1jXoYJQg= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1608 To avoid false-negative issue in check hash against dbx, both error condition (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 happening, and enforce a more secure operation to prevent secure boot from being 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 calculation error or certificate retrieval failure. Cc: Jiewen Yao Cc: Chao Zhang Signed-off-by: Jian J Wang Reviewed-by: Jiewen Yao --- .../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/DxeImageVerificationLib.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 =20 - @return TRUE Found the signature in the variable data= base. - @return FALSE Not found the signature in the variable = database. + @retval EFI_SUCCESS Finished the search without any error. + @retval Others Error occurred in the search of database. =20 **/ -BOOLEAN +EFI_STATUS IsSignatureFoundInDatabase ( - IN CHAR16 *VariableName, - IN UINT8 *Signature, - IN EFI_GUID *CertType, - IN UINTN SignatureSize + IN CHAR16 *VariableName, + IN UINT8 *Signature, + IN EFI_GUID *CertType, + IN UINTN SignatureSize, + OUT BOOLEAN *IsFound ) { EFI_STATUS Status; @@ -975,22 +977,28 @@ IsSignatureFoundInDatabase ( UINT8 *Data; UINTN Index; UINTN CertCount; - BOOLEAN IsFound; =20 // // Read signature database variable. // - IsFound =3D FALSE; + *IsFound =3D FALSE; Data =3D NULL; DataSize =3D 0; Status =3D gRT->GetVariable (VariableName, &gEfiImageSecurityDatabase= Guid, NULL, &DataSize, NULL); if (Status !=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; } =20 Data =3D (UINT8 *) AllocateZeroPool (DataSize); if (Data =3D=3D NULL) { - return FALSE; + return EFI_OUT_OF_RESOURCES; } =20 Status =3D gRT->GetVariable (VariableName, &gEfiImageSecurityDatabaseGui= d, 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 vali= date image should be measured // @@ -1023,7 +1031,7 @@ IsSignatureFoundInDatabase ( Cert =3D (EFI_SIGNATURE_DATA *) ((UINT8 *) Cert + CertList->Signat= ureSize); } =20 - if (IsFound) { + if (*IsFound) { break; } } @@ -1037,7 +1045,7 @@ Done: FreePool (Data); } =20 - return IsFound; + return Status; } =20 /** @@ -1642,6 +1650,8 @@ DxeImageVerificationHandler ( CHAR16 *NameStr; RETURN_STATUS PeCoffStatus; EFI_STATUS HashStatus; + EFI_STATUS DbStatus; + BOOLEAN IsFound; =20 SignatureList =3D NULL; SignatureListSize =3D 0; @@ -1650,7 +1660,7 @@ DxeImageVerificationHandler ( PkcsCertData =3D NULL; Action =3D EFI_IMAGE_EXECUTION_AUTH_UNTESTED; IsVerified =3D FALSE; - + IsFound =3D FALSE; =20 // // Check the image type and get policy setting. @@ -1792,7 +1802,14 @@ DxeImageVerificationHandler ( goto Failed; } =20 - if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE1, mImageDi= gest, &mCertType, mImageDigestSize)) { + DbStatus =3D IsSignatureFoundInDatabase ( + EFI_IMAGE_SECURITY_DATABASE1, + mImageDigest, + &mCertType, + mImageDigestSize, + &IsFound + ); + if (EFI_ERROR (DbStatus) || IsFound) { // // Image Hash is in forbidden database (DBX). // @@ -1800,7 +1817,14 @@ DxeImageVerificationHandler ( goto Failed; } =20 - if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE, mImageDig= est, &mCertType, mImageDigestSize)) { + DbStatus =3D IsSignatureFoundInDatabase ( + EFI_IMAGE_SECURITY_DATABASE, + mImageDigest, + &mCertType, + mImageDigestSize, + &IsFound + ); + if (!EFI_ERROR (DbStatus) && IsFound) { // // Image Hash is in allowed database (DB). // @@ -1888,14 +1912,29 @@ DxeImageVerificationHandler ( // // Check the image's hash value. // - if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE1, mImageDi= gest, &mCertType, mImageDigestSize)) { + 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) { - if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE, mImageD= igest, &mCertType, mImageDigestSize)) { + DbStatus =3D IsSignatureFoundInDatabase ( + EFI_IMAGE_SECURITY_DATABASE, + mImageDigest, + &mCertType, + mImageDigestSize, + &IsFound + ); + if (!EFI_ERROR (DbStatus) && IsFound) { IsVerified =3D TRUE; } else { DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image 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-=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/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-