From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.31; helo=mga06.intel.com; envelope-from=chao.b.zhang@intel.com; receiver=edk2-devel@lists.01.org Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id F0162202E6145 for ; Mon, 16 Oct 2017 07:20:56 -0700 (PDT) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga104.jf.intel.com with ESMTP; 16 Oct 2017 07:24:31 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,387,1503385200"; d="scan'208";a="910406594" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by FMSMGA003.fm.intel.com with ESMTP; 16 Oct 2017 07:24:30 -0700 Received: from fmsmsx112.amr.corp.intel.com (10.18.116.6) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 16 Oct 2017 07:24:30 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by FMSMSX112.amr.corp.intel.com (10.18.116.6) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 16 Oct 2017 07:24:30 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.175]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.213]) with mapi id 14.03.0319.002; Mon, 16 Oct 2017 22:24:28 +0800 From: "Zhang, Chao B" To: "Long, Qin" , "James.Bottomley@HansenPartnership.com" CC: "edk2-devel@lists.01.org" Thread-Topic: [PATCH] SecurityPkg/Pkcs7Verify: Add the comments to address security problem Thread-Index: AQHTQvgKkECY7AMamkOYB7GsZtU4+KLmjegw Date: Mon, 16 Oct 2017 14:24:28 +0000 Message-ID: References: <20171012011811.2140-1-qin.long@intel.com> In-Reply-To: <20171012011811.2140-1-qin.long@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] SecurityPkg/Pkcs7Verify: Add the comments to address security problem X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 Oct 2017 14:20:57 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Qin: The bellowing checking log is a little confusing to me.=20 The specific problem is that if the supplied hash is a different alg= orithm from the blacklist hash, the hash will be approved even if it should= have been denied. How about changing it to=20 The backlist hash check may result in false negative given hashes f= rom other different algorithms. =20 Others are good to me.=20 Reviewed-by : Chao Zhang -----Original Message----- From: Long, Qin=20 Sent: Thursday, October 12, 2017 9:18 AM To: Zhang, Chao B ; James.Bottomley@HansenPartnersh= ip.com Cc: edk2-devel@lists.01.org; Long, Qin Subject: [PATCH] SecurityPkg/Pkcs7Verify: Add the comments to address secur= ity problem Add the comments to address security problems in the Pkcs7Verify Protocol p= er UEFI 2.7 updates. The Pkcs7Verifier function VerifySignature() has problematic use cases wher= e it might be used to unwittingly bypass security checks. The specific pro= blem is that if the supplied hash is a different algorithm from the blackli= st hash, the hash will be approved even if it should have been denied. The = added comments place a strong warning about the problem. It is possible to use the protocol reliably, either by agreeing a hash to u= se for all time (like sha256) or by looping over all supported hashes when = using the protocol. Cc: Chao Zhang Cc: James Bottomley Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Qin Long --- MdePkg/Include/Protocol/Pkcs7Verify.h | 10 +++++++++- SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c | 8 ++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/MdePkg/Include/Protocol/Pkcs7Verify.h b/MdePkg/Include/Protoco= l/Pkcs7Verify.h index ca5ec75910..eaeda48300 100644 --- a/MdePkg/Include/Protocol/Pkcs7Verify.h +++ b/MdePkg/Include/Protocol/Pkcs7Verify.h @@ -6,7 +6,7 @@ PKCS#7 is a general-purpose cryptographic standard (defined by RFC2315, available at http://tools.ietf.org/html/rfc2315). =20 -Copyright (c) 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made availabl= e under the terms and conditions of the BSD License that accompanies this = distribution. The full text of the license may be found at @@ -140,6 +140,14 @@ EFI_STAT= US verifies the signature of the content is valid and signing certificate w= as not revoked and is contained within a list of trusted signers. =20 + Note: because this function uses hashes and the specification contains a= variety of + hash choices, you should be aware that the check against the Revok= edDb list + will improperly succeed if the signature is revoked using a differ= ent hash + algorithm. For this reason, you should either cycle through all U= EFI supported + hashes to see if one is forbidden, or rely on a single hash choice= only if the + UEFI signature authority only signs and revokes with a single hash= (at time + of writing, this hash choice is SHA256). + @param[in] This Pointer to EFI_PKCS7_VERIFY_PROTOCOL= instance. @param[in] Signature Points to buffer containing ASN.1 DE= R-encoded PKCS detached signature. diff --git a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c b/Secu= rityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c index 0da549a6bd..ac83e6d5c2 100644 --- a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c +++ b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c @@ -1321,6 +1321,14 @@ _Exit: verifies the signature of the content is valid and signing certificate w= as not revoked and is contained within a list of trusted signers. =20 + Note: because this function uses hashes and the specification contains a= variety of + hash choices, you should be aware that the check against the Revok= edDb list + will improperly succeed if the signature is revoked using a differ= ent hash + algorithm. For this reason, you should either cycle through all U= EFI supported + hashes to see if one is forbidden, or rely on a single hash choice= only if the + UEFI signature authority only signs and revokes with a single hash= (at time + of writing, this hash choice is SHA256). + @param[in] This Pointer to EFI_PKCS7_VERIFY_PROTOCOL= instance. @param[in] Signature Points to buffer containing ASN.1 DE= R-encoded PKCS detached signature. -- 2.14.1.windows.1