Comments below: > -----Original Message----- > From: Zurcher, Christopher J <christopher.j.zurc...@intel.com> > Sent: Tuesday, September 15, 2020 10:54 AM > To: Yao, Jiewen <jiewen....@intel.com>; devel@edk2.groups.io > Cc: Laszlo Ersek <ler...@redhat.com>; Wang, Jian J <jian.j.w...@intel.com>; > Lu, XiaoyuX <xiaoyux...@intel.com> > Subject: RE: [PATCH v2 0/3] CryptoPkg/BaseCryptLib: Add EVP (Envelope) Digest > interface > > Replies inline > > > -----Original Message----- > > From: Yao, Jiewen <jiewen....@intel.com> > > Sent: Monday, September 14, 2020 18:22 > > To: Zurcher, Christopher J <christopher.j.zurc...@intel.com>; > > devel@edk2.groups.io > > Cc: Laszlo Ersek <ler...@redhat.com>; Wang, Jian J <jian.j.w...@intel.com>; > > Lu, XiaoyuX <xiaoyux...@intel.com> > > Subject: RE: [PATCH v2 0/3] CryptoPkg/BaseCryptLib: Add EVP (Envelope) > Digest > > interface > > > > Hi Zurcher: > > Thanks for your work. > > 1) Please share with us what unit test you have done for all new APIs. > > I unit tested both the native and Crypto Service implementations through the > modified Hash2DxeCrypto protocol. > I tested the Init/Update/Final flow as well as the HashAll function. > > > > > 2) Please add comment on what is the valid DigestName in EvpMdInit(). > > Otherwise, people will have no idea on that. > > I will add valid options in a comment. > I have to send another patch anyway to add a file in my commit (missed the > second copy of CryptEvpMdNull.c in the NullLib folder). > > > > > 3) I assume the size will be unchanged if a module does not use the new > EVPMD > > API, such as UEFI secure boot, TCG trusted boot. Please double confirm if > > that is right understanding. > > Yes, if a module does not call the EVPMD API, it should not grow in size. > The Crypto Service build output CryptoDxe.efi grew less than 1% after enabling > the EvpMd function family through PcdCryptoServiceFamilyEnable. > I suspect this is because the HmacSha256 Family was already enabled, and > inside > OpenSSL the HMAC functions are wrappers for EVP functions. > So even with library-mode BaseCryptLib, any module that already calls the > HMAC functions should not see any size change by adding EVP. > > > > > Hi all: > > I would like collect feedback on below: > > -- "I replaced the MD5 and SHAx functions with EVP functions in > > Hash2DxeCrypto, and it grew from ~26k to ~253k." > > > > If there is negative size impact for the platform BIOS that is using > > Hash2DxeCrypto, please share with the community. > > The size change in Hash2DxeCrypto was seen while using the library-mode > BaseCryptLib implementation, not the Crypto Services driver. > We cannot move to OpenSSL 3 without replacing all low-level algorithm > functions with EVP calls, so platforms using Hash2DxeCrypto will have to eat > the > size increase eventually. > For platforms using Hash2DxeCrypto, moving to the Crypto Services model > should help offset this increase.
[Jiewen] I think we need evaluate the size impact to decide if/when/how to move to OpenSSL 3 later. We can cross the bridge when we come to it. > > Thanks, > Christopher Zurcher > > > > > Thank you > > Yao Jiewen > > > > > -----Original Message----- > > > From: Christopher J Zurcher <christopher.j.zurc...@intel.com> > > > Sent: Tuesday, September 15, 2020 8:58 AM > > > To: devel@edk2.groups.io > > > Cc: Laszlo Ersek <ler...@redhat.com>; Yao, Jiewen > <jiewen....@intel.com>; > > > Wang, Jian J <jian.j.w...@intel.com>; Lu, XiaoyuX <xiaoyux...@intel.com> > > > Subject: [PATCH v2 0/3] CryptoPkg/BaseCryptLib: Add EVP (Envelope) Digest > > > interface > > > > > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2545 > > > > > > V2 changes: > > > Added NullLib implementation > > > Added Crypto Service implementation > > > Rebased Hash2DxeCrypto to use EVP interface instead of low-level functions > > > Removed unnecessary casts > > > Added "HashAll" utility function > > > Merged "New" and "Init" functions as well as "Final" and "Free" functions > > > Retained "Init/Update/Final" naming instead of "New/Update/Free" as this > > > conforms with common usage > > > > > > Low-level interfaces to message digest (hash) functions have been > > deprecated > > > in OpenSSL 3. In order to upgrade to OpenSSL 3, all direct calls to > > > low-level functions (such as SHA256_Init() in CryptSha256.c) will need to > > > be replaced by EVP inteface calls. > > > > > > References: > > > https://www.openssl.org/docs/manmaster/man7/evp.html > > > https://www.openssl.org/docs/manmaster/man3/SHA256_Init.html > > > > > > Cc: Laszlo Ersek <ler...@redhat.com> > > > Cc: Jiewen Yao <jiewen....@intel.com> > > > Cc: Jian J Wang <jian.j.w...@intel.com> > > > Cc: Xiaoyu Lu <xiaoyux...@intel.com> > > > > > > Christopher J Zurcher (3): > > > CryptoPkg/BaseCryptLib: Add EVP (Envelope) Digest interface > > > CryptoPkg: Add EVP to Crypto Service driver interface > > > SecurityPkg/Hash2DxeCrypto: Rebase Hash2DxeCrypto onto the EVP > > > interface > > > > > > CryptoPkg/CryptoPkg.dsc | 3 + > > > CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf | 1 + > > > CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf | 1 + > > > CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf | 1 + > > > CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf | 1 + > > > CryptoPkg/Library/BaseCryptLibNull/BaseCryptLibNull.inf | 1 + > > > CryptoPkg/Include/Library/BaseCryptLib.h | 125 +++++++ > > > CryptoPkg/Include/Pcd/PcdCryptoServiceFamilyEnable.h | 10 + > > > CryptoPkg/Private/Protocol/Crypto.h | 127 +++++++ > > > SecurityPkg/Hash2DxeCrypto/Driver.h | 1 - > > > CryptoPkg/Driver/Crypto.c | 148 ++++++++- > > > CryptoPkg/Library/BaseCryptLib/Evp/CryptEvpMd.c | 253 > > ++++++++++++++ > > > CryptoPkg/Library/BaseCryptLib/Evp/CryptEvpMdNull.c | 124 +++++++ > > > CryptoPkg/Library/BaseCryptLibOnProtocolPpi/CryptLib.c | 140 ++++++++ > > > SecurityPkg/Hash2DxeCrypto/Hash2DxeCrypto.c | 345 > > > ++---------- > > -------- > > > 15 files changed, 965 insertions(+), 316 deletions(-) > > > create mode 100644 CryptoPkg/Library/BaseCryptLib/Evp/CryptEvpMd.c > > > create mode 100644 > CryptoPkg/Library/BaseCryptLib/Evp/CryptEvpMdNull.c > > > > > > -- > > > 2.28.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#65243): https://edk2.groups.io/g/devel/message/65243 Mute This Topic: https://groups.io/mt/76856051/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
