From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mx.groups.io with SMTP id smtpd.web11.1042.1588044457731838955 for ; Mon, 27 Apr 2020 20:27:37 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.31, mailfrom: jian.j.wang@intel.com) IronPort-SDR: mO3YJtw46wGlLRM3wteCdks4q1K4ObDIjP6D51ho88RkSI8xHHj8+h72lT24IpCw2eJrjRQ+PM GTVlgXIAZtFg== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Apr 2020 20:27:36 -0700 IronPort-SDR: /SdXe77OIBzblVxr367MbCvNS7tN/obGRIAca6UpmH1mOpyatIVspNxp4rxveTwHPm1dhOV18h uZvH2TQ1eElA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,326,1583222400"; d="scan'208";a="247591112" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga007.fm.intel.com with ESMTP; 27 Apr 2020 20:27:35 -0700 Received: from fmsmsx114.amr.corp.intel.com (10.18.116.8) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 27 Apr 2020 20:27:29 -0700 Received: from shsmsx106.ccr.corp.intel.com (10.239.4.159) by FMSMSX114.amr.corp.intel.com (10.18.116.8) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 27 Apr 2020 20:27:29 -0700 Received: from shsmsx107.ccr.corp.intel.com ([169.254.9.191]) by SHSMSX106.ccr.corp.intel.com ([169.254.10.89]) with mapi id 14.03.0439.000; Tue, 28 Apr 2020 11:27:26 +0800 From: "Wang, Jian J" To: "devel@edk2.groups.io" , "Jiang, Guomin" CC: "Lu, XiaoyuX" Subject: Re: [edk2-devel] [PATCH v5] CryptoPkg/Pkcs7: Extend support for other OID types Thread-Topic: [edk2-devel] [PATCH v5] CryptoPkg/Pkcs7: Extend support for other OID types Thread-Index: AQHWHPb6p8/ftlTpokShlNzrEJ45OaiN3JHw Date: Tue, 28 Apr 2020 03:27:25 +0000 Message-ID: References: <20200428004953.83-1-guomin.jiang@intel.com> In-Reply-To: <20200428004953.83-1-guomin.jiang@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: jian.j.wang@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Guomin, Sorry I didn't check carefully on the return type of function Pkcs7TypeIsOt= her(). I think BOOLEAN should be more appropriate. INTN looks strange, although it works. It's actually used to check if the PKCS7 is data or not. Son in this= function TRUE or FALSE should be returned instead of 1 or 0. If using BOOLEAN, you can also use this function directly in following exp= ression, without comparing to '1', which looks strange to me too. + if (Pkcs7TypeIsOther(P7) && P7->d.other && In addition, 'P7->d.other' should use explicit check against NULL. With above addressed, Reviewed-by: Jian J Wang Regards, Jian > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Guomin > Jiang > Sent: Tuesday, April 28, 2020 8:50 AM > To: devel@edk2.groups.io > Cc: Wang, Jian J ; Lu, XiaoyuX > Subject: [edk2-devel] [PATCH v5] CryptoPkg/Pkcs7: Extend support for othe= r OID > types >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2539 >=20 > Microsoft signtool supports creation of attached P7's with any OID payloa= d > via the "/p7co" parameter. It is necessary to check the data before get > the string. >=20 > Cc: Jian J Wang > Cc: Xiaoyu Lu > Signed-off-by: Guomin Jiang > --- > .../BaseCryptLib/Pk/CryptPkcs7VerifyBase.c | 66 ++++++++++++++++++- > 1 file changed, 65 insertions(+), 1 deletion(-) >=20 > diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyBase.c > b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyBase.c > index 313f459b11..1b23ec99af 100644 > --- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyBase.c > +++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyBase.c > @@ -13,6 +13,66 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #include >=20 > #include >=20 >=20 >=20 > +/** >=20 > + Check the contents of PKCS7 is not data. >=20 > + >=20 > + It is copied from PKCS7_type_is_other() in pk7_doit.c. >=20 > + >=20 > + @param[in] P7 Pointer to the location at which the PKCS7 is located. >=20 > + >=20 > + @return INTN The content type. >=20 > +**/ >=20 > +STATIC >=20 > +INTN >=20 > +Pkcs7TypeIsOther ( >=20 > + IN PKCS7 *P7 >=20 > + ) >=20 > +{ >=20 > + INTN Others =3D 1; >=20 > + INTN Nid =3D OBJ_obj2nid (P7->type); >=20 > + >=20 > + switch (Nid) { >=20 > + case NID_pkcs7_data: >=20 > + case NID_pkcs7_signed: >=20 > + case NID_pkcs7_enveloped: >=20 > + case NID_pkcs7_signedAndEnveloped: >=20 > + case NID_pkcs7_encrypted: >=20 > + Others =3D 0; >=20 > + break; >=20 > + default: >=20 > + Others =3D 1; >=20 > + } >=20 > + >=20 > + return Others; >=20 > +} >=20 > + >=20 > +/** >=20 > + Get the ASN.1 string for the PKCS7. >=20 > + >=20 > + It is copied from PKCS7_get_octet_string() in pk7_doit.c. >=20 > + >=20 > + @param[in] P7 Pointer to the location at which the PKCS7 is located. >=20 > + >=20 > + @return ASN1_OCTET_STRING ASN.1 string. >=20 > +**/ >=20 > +STATIC >=20 > +ASN1_OCTET_STRING* >=20 > +Pkcs7GetOctetString ( >=20 > + IN PKCS7 *P7 >=20 > + ) >=20 > +{ >=20 > + if (PKCS7_type_is_data (P7)) { >=20 > + return P7->d.data; >=20 > + } >=20 > + >=20 > + if ((Pkcs7TypeIsOther(P7) =3D=3D 1) && P7->d.other && >=20 > + (P7->d.other->type =3D=3D V_ASN1_OCTET_STRING)) { >=20 > + return P7->d.other->value.octet_string; >=20 > + } >=20 > + >=20 > + return NULL; >=20 > +} >=20 > + >=20 > /** >=20 > Extracts the attached content from a PKCS#7 signed data if existed. Th= e input > signed >=20 > data could be wrapped in a ContentInfo structure. >=20 > @@ -98,7 +158,11 @@ Pkcs7GetAttachedContent ( > // >=20 > // Retrieve the attached content in PKCS7 signedData >=20 > // >=20 > - OctStr =3D Pkcs7->d.sign->contents->d.data; >=20 > + OctStr =3D Pkcs7GetOctetString (Pkcs7->d.sign->contents); >=20 > + if (OctStr =3D=3D NULL) { >=20 > + goto _Exit; >=20 > + } >=20 > + >=20 > if ((OctStr->length > 0) && (OctStr->data !=3D NULL)) { >=20 > *ContentSize =3D OctStr->length; >=20 > *Content =3D AllocatePool (*ContentSize); >=20 > -- > 2.25.1.windows.1 >=20 >=20 > -=3D-=3D-=3D-=3D-=3D-=3D > Groups.io Links: You receive all messages sent to this group. >=20 > View/Reply Online (#58197): https://edk2.groups.io/g/devel/message/58197 > Mute This Topic: https://groups.io/mt/73318347/1768734 > Group Owner: devel+owner@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub [jian.j.wang@intel.com= ] > -=3D-=3D-=3D-=3D-=3D-=3D