Hi, Jiewen Although the protocol is private, a corresponding BaseCryptoLib instance is not private, like PeiCryptLib.inf, RuntimeCryptLib, etc. These library instances will be static linked to the consumer driver, for example an iSCSI network driver. So actually it's not a "private" change inside CryptoPkg.
The goal to provide a driver version of crypto service is to support modulization FW update, which means the crypto driver may NOT be updated together with its consumer. A platform may choose to update the crypto service driver to a new version with this patch, then all the BaseCryptoLib consumers will be messed. Best Regards Siyuan > -----Original Message----- > From: Yao, Jiewen <jiewen....@intel.com> > Sent: 2020年3月27日 10:58 > To: devel@edk2.groups.io; Fu, Siyuan <siyuan...@intel.com>; Gao, Zhichao > <zhichao....@intel.com> > Cc: Wang, Jian J <jian.j.w...@intel.com>; Lu, XiaoyuX <xiaoyux...@intel.com>; > Maciej Rabeda <maciej.rab...@linux.intel.com>; Wu, Jiaxin > <jiaxin...@intel.com> > Subject: RE: [edk2-devel] [PATCH 0/8] CryptoPkg: Retire the deprecate function > > EDKII_CRYPTO_PROTOCOL is *private*. > https://github.com/tianocore/edk2/blob/master/CryptoPkg/Private/Protocol/C > rypto.h > > Why we cannot change? > > > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Siyuan, Fu > > Sent: Friday, March 27, 2020 10:47 AM > > To: Gao, Zhichao <zhichao....@intel.com>; devel@edk2.groups.io > > Cc: Wang, Jian J <jian.j.w...@intel.com>; Lu, XiaoyuX > <xiaoyux...@intel.com>; > > Maciej Rabeda <maciej.rab...@linux.intel.com>; Wu, Jiaxin > > <jiaxin...@intel.com> > > Subject: Re: [edk2-devel] [PATCH 0/8] CryptoPkg: Retire the deprecate > function > > > > Hi, Zhichao > > > > We should never move/delete a member field of a previous defined protocol > > Interface. Instead, these protocol APIs shall be kept and return an error > > code > > If the function is retired. Otherwise the consumer driver may call into an > > Incorrect function if it's build with different codebase/PCD settings with > > the > > Crypto PEI/DXE/SMM driver. > > This comment applies to all the EDKII_CRYPTO_PROTOCOL related changes in > > your patch set. > > > > Best Regards > > Siyuan > > > > > -----Original Message----- > > > From: Gao, Zhichao <zhichao....@intel.com> > > > Sent: 2020年3月27日 9:56 > > > To: devel@edk2.groups.io > > > Cc: Wang, Jian J <jian.j.w...@intel.com>; Lu, XiaoyuX > > <xiaoyux...@intel.com>; > > > Maciej Rabeda <maciej.rab...@linux.intel.com>; Wu, Jiaxin > > > <jiaxin...@intel.com>; Fu, Siyuan <siyuan...@intel.com> > > > Subject: [PATCH 0/8] CryptoPkg: Retire the deprecate function > > > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1682 > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1898 > > > > > > MD4, AR4, Tdes, Aes Ecb mode, MD5 and SHA1 is not secure any longer. > > > They are all deprecated. Edk2 would not support them any longer. > > > So remove them. > > > But uefi spec want to keep MD5 and SHA1 for backwards compatibility. > > > So add two pcds to control the MD5 and SHA1 enablement. Set the pcds > > > default value to false to indicate they are deprecated. > > > > > > NetWorkPkg's iSCSI driver would consume the MD5 function, so change > > > the md5 pcd to TURE when iSCSI is enabled. > > > > > > Cc: Jian J Wang <jian.j.w...@intel.com> > > > Cc: Xiaoyu Lu <xiaoyux...@intel.com> > > > Cc: Maciej Rabeda <maciej.rab...@linux.intel.com> > > > Cc: Jiaxin Wu <jiaxin...@intel.com> > > > Cc: Siyuan Fu <siyuan...@intel.com> > > > Signed-off-by: Zhichao Gao <zhichao....@intel.com> > > > > > > Zhichao Gao (8): > > > CryptoPkg/BaseCrpytLib: Retire MD4 algorithm > > > CryptoPkg/BaseCryptLib: Retire ARC4 algorithm > > > CryptoPkg/BaseCryptLib: Retire the Tdes algorithm > > > CryptoPkg/BaseCryptLib: Retire Aes Ecb mode algorithm > > > CryptoPkg/dec: Add pcds to avoid building the deprecated function > > > NetWorkPkg/Pcd.inc: Enable the MD5 for iSCSI > > > Crypto/BaseCryptLib: Using pcd to control MD5 enablement > > > CryptoPkg/BaseCryptLib: Use Pcd to control the SHA1 enablement > > > > > > CryptoPkg/CryptoPkg.dec | 11 + > > > CryptoPkg/CryptoPkg.uni | 11 + > > > CryptoPkg/Driver/Crypto.c | 634 +----------------- > > > CryptoPkg/Include/Library/BaseCryptLib.h | 548 --------------- > > > .../Library/BaseCryptLib/BaseCryptLib.inf | 9 +- > > > .../Library/BaseCryptLib/Cipher/CryptAes.c | 114 ---- > > > .../BaseCryptLib/Cipher/CryptAesNull.c | 52 -- > > > .../Library/BaseCryptLib/Cipher/CryptArc4.c | 205 ------ > > > .../BaseCryptLib/Cipher/CryptArc4Null.c | 124 ---- > > > .../Library/BaseCryptLib/Cipher/CryptTdes.c | 364 ---------- > > > .../BaseCryptLib/Cipher/CryptTdesNull.c | 160 ----- > > > .../Library/BaseCryptLib/Hash/CryptMd4.c | 223 ------ > > > .../Library/BaseCryptLib/Hash/CryptMd4Null.c | 143 ---- > > > .../Library/BaseCryptLib/Hash/CryptMd5.c | 5 +- > > > .../Library/BaseCryptLib/Hmac/CryptHmacMd5.c | 3 + > > > .../BaseCryptLib/Hmac/CryptHmacMd5Null.c | 3 + > > > .../Library/BaseCryptLib/Hmac/CryptHmacSha1.c | 3 + > > > .../BaseCryptLib/Hmac/CryptHmacSha1Null.c | 3 + > > > .../Library/BaseCryptLib/PeiCryptLib.inf | 13 +- > > > .../BaseCryptLib/Pk/CryptPkcs5Pbkdf2.c | 3 + > > > .../Library/BaseCryptLib/Pk/CryptRsaBasic.c | 5 + > > > .../Library/BaseCryptLib/Pk/CryptRsaExt.c | 5 + > > > .../Library/BaseCryptLib/RuntimeCryptLib.inf | 13 +- > > > .../Library/BaseCryptLib/SmmCryptLib.inf | 13 +- > > > .../BaseCryptLibNull/BaseCryptLibNull.inf | 3 - > > > .../BaseCryptLibNull/Cipher/CryptAesNull.c | 54 +- > > > .../BaseCryptLibNull/Cipher/CryptArc4Null.c | 124 ---- > > > .../BaseCryptLibNull/Cipher/CryptTdesNull.c | 160 ----- > > > .../BaseCryptLibNull/Hash/CryptMd4Null.c | 143 ---- > > > .../BaseCryptLibNull/Hash/CryptMd5Null.c | 3 + > > > .../BaseCryptLibNull/Hmac/CryptHmacMd5Null.c | 3 + > > > .../BaseCryptLibNull/Hmac/CryptHmacSha1Null.c | 4 +- > > > .../BaseCryptLibOnProtocolPpi/CryptLib.c | 604 +---------------- > > > .../Library/BaseHashApiLib/BaseHashApiLib.c | 12 + > > > .../Library/BaseHashApiLib/BaseHashApiLib.inf | 1 + > > > CryptoPkg/Private/Protocol/Crypto.h | 583 +--------------- > > > NetworkPkg/NetworkPcds.dsc.inc | 5 +- > > > 37 files changed, 145 insertions(+), 4221 deletions(-) > > > delete mode 100644 CryptoPkg/Library/BaseCryptLib/Cipher/CryptArc4.c > > > delete mode 100644 > CryptoPkg/Library/BaseCryptLib/Cipher/CryptArc4Null.c > > > delete mode 100644 CryptoPkg/Library/BaseCryptLib/Cipher/CryptTdes.c > > > delete mode 100644 > CryptoPkg/Library/BaseCryptLib/Cipher/CryptTdesNull.c > > > delete mode 100644 CryptoPkg/Library/BaseCryptLib/Hash/CryptMd4.c > > > delete mode 100644 CryptoPkg/Library/BaseCryptLib/Hash/CryptMd4Null.c > > > delete mode 100644 > > > CryptoPkg/Library/BaseCryptLibNull/Cipher/CryptArc4Null.c > > > delete mode 100644 > > > CryptoPkg/Library/BaseCryptLibNull/Cipher/CryptTdesNull.c > > > delete mode 100644 > > CryptoPkg/Library/BaseCryptLibNull/Hash/CryptMd4Null.c > > > > > > -- > > > 2.21.0.windows.1 > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#56466): https://edk2.groups.io/g/devel/message/56466 Mute This Topic: https://groups.io/mt/72579461/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-