From nobody Tue Nov 26 13:21:46 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+53872+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+53872+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 1580998781641226.4889816411545; Thu, 6 Feb 2020 06:19:41 -0800 (PST) Return-Path: X-Received: by 127.0.0.2 with SMTP id nLKIYY1788612xrKIqnvFxjh; Thu, 06 Feb 2020 06:19:41 -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:40 -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:40 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,409,1574150400"; d="scan'208";a="226160689" X-Received: from shwdeopensfp777.ccr.corp.intel.com ([10.239.158.78]) by fmsmga008.fm.intel.com with ESMTP; 06 Feb 2020 06:19:39 -0800 From: "Wang, Jian J" To: devel@edk2.groups.io Cc: Jiewen Yao , Chao Zhang , Laszlo Ersek Subject: [edk2-devel] [PATCH 6/9] SecurityPkg/DxeImageVerificationLib: Differentiate error and search result in IsCertHashFoundInDatabase(CVE-2019-14575) Date: Thu, 6 Feb 2020 22:19:30 +0800 Message-Id: <20200206141933.356-7-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: JWpB8Y2PBzX2AnK9boicXsM1x1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1580998781; bh=xXtbhP05GE3cP0UopnahDz0HgpRP0HhkqbnYAnLtVh4=; h=Cc:Date:From:Reply-To:Subject:To; b=Xc6PrkFtwT6mfpTeIWFkkb7ye6KSVv2KW5PBC9w72B6P8IpCGARZcUl5xD4sNHszjFS +hKVgg3vyPJ9n0riBsqc4ZK2nWyAiVPh4rQG8HAfFDGFcpXEyQnO2ZJtttcXOppuuzPbw bmme86QyUkeRibyrWefgpzXezbxWxuZw2I8= 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 IsCertHashFoundInDatabase() 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. Cc: Jiewen Yao Cc: Chao Zhang Signed-off-by: Jian J Wang Signed-off-by: Laszlo Ersek --- .../DxeImageVerificationLib.c | 68 +++++++++++-------- 1 file changed, 41 insertions(+), 27 deletions(-) diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificati= onLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL= ib.c index 8739d1fa29..a5dfee0f8e 100644 --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c @@ -822,22 +822,23 @@ AddImageExeInfo ( @param[in] SignatureList Pointer to the Signature List in forbidden= database. @param[in] SignatureListSize Size of Signature List. @param[out] RevocationTime Return the time that the certificate was r= evoked. + @param[out] IsFound Search result. Only valid if EFI_SUCCESS r= eturned. =20 - @return TRUE The certificate hash is found in the forbidden database. - @return FALSE The certificate hash is not found in the forbidden databa= se. + @retval EFI_SUCCESS Finished the search without any error. + @retval Others Error occurred in the search of database. =20 **/ -BOOLEAN +EFI_STATUS IsCertHashFoundInDatabase ( IN UINT8 *Certificate, IN UINTN CertSize, IN EFI_SIGNATURE_LIST *SignatureList, IN UINTN SignatureListSize, - OUT EFI_TIME *RevocationTime + OUT EFI_TIME *RevocationTime, + OUT BOOLEAN *IsFound ) { - BOOLEAN IsFound; - BOOLEAN Status; + EFI_STATUS Status; EFI_SIGNATURE_LIST *DbxList; UINTN DbxSize; EFI_SIGNATURE_DATA *CertHash; @@ -851,21 +852,22 @@ IsCertHashFoundInDatabase ( UINT8 *TBSCert; UINTN TBSCertSize; =20 - IsFound =3D FALSE; + Status =3D EFI_ABORTED; + *IsFound =3D FALSE; DbxList =3D SignatureList; DbxSize =3D SignatureListSize; HashCtx =3D NULL; HashAlg =3D HASHALG_MAX; =20 if ((RevocationTime =3D=3D NULL) || (DbxList =3D=3D NULL)) { - return FALSE; + return EFI_INVALID_PARAMETER; } =20 // // Retrieve the TBSCertificate from the X.509 Certificate. // if (!X509GetTBSCert (Certificate, CertSize, &TBSCert, &TBSCertSize)) { - return FALSE; + return Status; } =20 while ((DbxSize > 0) && (SignatureListSize >=3D DbxList->SignatureListSi= ze)) { @@ -895,16 +897,13 @@ IsCertHashFoundInDatabase ( if (HashCtx =3D=3D NULL) { goto Done; } - Status =3D mHash[HashAlg].HashInit (HashCtx); - if (!Status) { + if (!mHash[HashAlg].HashInit (HashCtx)) { goto Done; } - Status =3D mHash[HashAlg].HashUpdate (HashCtx, TBSCert, TBSCertSize); - if (!Status) { + if (!mHash[HashAlg].HashUpdate (HashCtx, TBSCert, TBSCertSize)) { goto Done; } - Status =3D mHash[HashAlg].HashFinal (HashCtx, CertDigest); - if (!Status) { + if (!mHash[HashAlg].HashFinal (HashCtx, CertDigest)) { goto Done; } =20 @@ -923,7 +922,8 @@ IsCertHashFoundInDatabase ( // // Hash of Certificate is found in forbidden database. // - IsFound =3D TRUE; + Status =3D EFI_SUCCESS; + *IsFound =3D TRUE; =20 // // Return the revocation time. @@ -938,12 +938,14 @@ IsCertHashFoundInDatabase ( DbxList =3D (EFI_SIGNATURE_LIST *) ((UINT8 *) DbxList + DbxList->Sign= atureListSize); } =20 + Status =3D EFI_SUCCESS; + Done: if (HashCtx !=3D NULL) { FreePool (HashCtx); } =20 - return IsFound; + return Status; } =20 /** @@ -1216,6 +1218,7 @@ IsForbiddenByDbx ( { EFI_STATUS Status; BOOLEAN IsForbidden; + BOOLEAN IsFound; UINT8 *Data; UINTN DataSize; EFI_SIGNATURE_LIST *CertList; @@ -1344,12 +1347,13 @@ IsForbiddenByDbx ( // CertPtr =3D CertPtr + sizeof (UINT32) + CertSize; =20 - if (IsCertHashFoundInDatabase (Cert, CertSize, (EFI_SIGNATURE_LIST *)D= ata, DataSize, &RevocationTime)) { + Status =3D IsCertHashFoundInDatabase (Cert, CertSize, (EFI_SIGNATURE_L= IST *)Data, DataSize, &RevocationTime, &IsFound); + if (EFI_ERROR (Status) || IsFound) { // // Check the timestamp signature and signing time to determine if th= e image can be trusted. // IsForbidden =3D TRUE; - if (PassTimestampCheck (AuthData, AuthDataSize, &RevocationTime)) { + if (!EFI_ERROR (Status) && PassTimestampCheck (AuthData, AuthDataSiz= e, &RevocationTime)) { IsForbidden =3D FALSE; // // Pass DBT check. Continue to check other certs in image signer's= cert list against DBX, DBT @@ -1392,6 +1396,7 @@ IsAllowedByDb ( { EFI_STATUS Status; BOOLEAN VerifyStatus; + BOOLEAN IsFound; EFI_SIGNATURE_LIST *CertList; EFI_SIGNATURE_DATA *CertData; UINTN DataSize; @@ -1495,17 +1500,26 @@ IsAllowedByDb ( // The image is signed and its signature is found in 'db'. // if (DbxData !=3D NULL) { + VerifyStatus =3D FALSE; // // Here We still need to check if this RootCert's Hash is revo= ked // - if (IsCertHashFoundInDatabase (RootCert, RootCertSize, (EFI_SI= GNATURE_LIST *)DbxData, DbxDataSize, &RevocationTime)) { - // - // Check the timestamp signature and signing time to determi= ne if the RootCert can be trusted. - // - VerifyStatus =3D PassTimestampCheck (AuthData, AuthDataSize,= &RevocationTime); - if (!VerifyStatus) { - DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is sig= ned and signature is accepted by DB, but its root cert failed the timestamp= check.\n")); - } + Status =3D IsCertHashFoundInDatabase (RootCert, RootCertSize, = (EFI_SIGNATURE_LIST *)DbxData, DbxDataSize, &RevocationTime, &IsFound); + if (EFI_ERROR (Status)) { + goto Done; + } + + if (!IsFound) { + VerifyStatus =3D TRUE; + goto Done; + } + + // + // Check the timestamp signature and signing time to determine= if the RootCert can be trusted. + // + VerifyStatus =3D PassTimestampCheck (AuthData, AuthDataSize, &= RevocationTime); + if (!VerifyStatus) { + DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signe= d and signature is accepted by DB, but its root cert failed the timestamp c= heck.\n")); } } =20 --=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 (#53872): https://edk2.groups.io/g/devel/message/53872 Mute This Topic: https://groups.io/mt/71023424/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-