Hi Mike We are using submodule for mbedtls in this patch. Copying source code is not preferred.
I think we are discussing multiple ways to layout the *mbedtls crypto wrapper*. See following 4 options. Thank you Yao, Jiewen > -----Original Message----- > From: Kinney, Michael D <michael.d.kin...@intel.com> > Sent: Thursday, August 31, 2023 11:45 PM > To: Leif Lindholm <quic_llind...@quicinc.com>; Yao, Jiewen > <jiewen....@intel.com>; devel@edk2.groups.io; spbro...@outlook.com; Hou, > Wenxing <wenxing....@intel.com> > Cc: af...@apple.com; Kinney, Michael D <michael.d.kin...@intel.com> > Subject: RE: [edk2-devel] [edk2/add_mbedtls PATCH 0/9] *** Add > HMAC/HKDF/RSA/HASH features based on Mbedtls *** > > I have not looked at the Mbedtls patches in detail yet, but I > am curious if it is possible to add the mbedtls based library > instances of the edk2 crypto libraries to the existing > CryptoPkg and pull the mbedtls sources into the CryptoPkg using > a submodule just like openssl? This way, platforms can choose > either openssl or mbedtls library instances from CryptoPkg and > all INFs would continue to only list CryptoPkg.dec. > > I think use of submodules makes the most sense for content that > edk2 consumes as read-only and edk2 makes decisions to jump from > one validated release to the next validated release of the submodule > content. > > In general, we do not want to copy source from a different project > into TianoCore repos because of the overhead to keep them in sync. > An exception to this is something like cmocka where this was done > for CI stability issues and the copy in TianoCore is an automated > sync of the upstream repo. > > Thanks, > > Mike > > > > -----Original Message----- > > From: Leif Lindholm <quic_llind...@quicinc.com> > > Sent: Thursday, August 31, 2023 4:15 AM > > To: Yao, Jiewen <jiewen....@intel.com>; devel@edk2.groups.io; > > spbro...@outlook.com; Hou, Wenxing <wenxing....@intel.com> > > Cc: af...@apple.com; Kinney, Michael D <michael.d.kin...@intel.com> > > Subject: Re: [edk2-devel] [edk2/add_mbedtls PATCH 0/9] *** Add > > HMAC/HKDF/RSA/HASH features based on Mbedtls *** > > > > Like Sean, I'm very positive to the work - and I'm excited about the > > opportunity to formalise the abstractions. > > > > But Sean is also asking to import the mbedTLS code outright instead of > > using submodules, which adds an additional dimension to the matrix below. > > > > I'm not too concerned over the infrastructure change as such, but I > > would prefer to not move the dial even further in the direction of > > "upstream is a swarm of repositories". This adds complexity for new > > developers. And submodules are way easier for upstream to track external > > projects through. At the cost of complicating the maintenance process > > for released products. Which isn't great. > > > > Am I kicking the can too far down the road if I suggest we do some > > brainstorming around ways to achieve this with the least amount of > > friction for everyone at the plugfest in October? > > > > Regards, > > > > Leif > > > > On 2023-08-31 03:34, Yao, Jiewen wrote: > > > Hi Sean/Andrew/Leif/Mike > > > Now, I think we actually have multiple options to handle this: > > > > > > 1) CryptoPkg in edk2 repo (add MbedTls to existing CryptoPkg) > > > > > > 2) CryptoPkg in edk2 repo + a new MbedTlsCryptoPkg in edk2 repo > > > > > > 3) CryptoPkg in edk2 repo + MbedTlsCryptoPkg in a new repo > > > > > > 4) Move CryptoPkg from edk2 repo to OpensslCryptoPkg in a new repo + > > MbedTlsCryptoPkg in another new repo > > > > > > > > > > > > Current patch is for option 1). > > > Sean's proposal is for option 4). > > > > > > I feel 4) is very aggressive. My worry is that it will involve many > > infrastructure change such as CI, and all edk2 platforms. > > > > > > What about 2) or 3) ? > > > > > > Thank you > > > Yao, Jiewen > > > > > > > > >> -----Original Message----- > > >> From: Yao, Jiewen > > >> Sent: Thursday, August 31, 2023 8:10 AM > > >> To: devel@edk2.groups.io; spbro...@outlook.com; Hou, Wenxing > > >> <wenxing....@intel.com> > > >> Cc: af...@apple.com; quic_llind...@quicinc.com; Kinney, Michael D > > >> <michael.d.kin...@intel.com> > > >> Subject: RE: [edk2-devel] [edk2/add_mbedtls PATCH 0/9] *** Add > > >> HMAC/HKDF/RSA/HASH features based on Mbedtls *** > > >> > > >> Hi Sean > > >> Thanks for the feedback. Personally, I don't have strong opinion on this. > > >> > > >> Since this is a big change, I would like to have Steward member's > > >> opinion. > > >> > > >> Hi Andrew/Leif/Mike > > >> What do you think? > > >> > > >> Thank you > > >> Yao, Jiewen > > >> > > >> > > >>> -----Original Message----- > > >>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sean > > >>> Sent: Thursday, August 31, 2023 2:57 AM > > >>> To: devel@edk2.groups.io; Hou, Wenxing <wenxing....@intel.com> > > >>> Subject: Re: [edk2-devel] [edk2/add_mbedtls PATCH 0/9] *** Add > > >>> HMAC/HKDF/RSA/HASH features based on Mbedtls *** > > >>> > > >>> I appreciate and really like this work to enable mbedtls but I don't > > >>> like the idea of adding another submodule to edk2. > > >>> > > >>> For a long time there has been discussion about formalizing the > > >>> abstraction of the edk2 crypto api so that it would be practical to > > >>> implement edk2's crypto using various libraries. I propose we remove > > >>> openssl from the edk2 CryptoPkg and into the OpenSslCryptoPkg in another > > >>> new tianocore repository dedicated to OpenSsl. MbedTls could then be > > >>> checked into the MbedTlsCryptoPkg and added to another new repository. > > >>> This would also have the benefit of breaking the tight coupling of edk2 > > >>> stable tags from the crypto used in the code base (crypto has more > > >>> widely tracked vulnerabilities). > > >>> > > >>> Happy to discuss more if others have different ideas. > > >>> > > >>> Thanks > > >>> > > >>> Sean > > >>> > > >>> > > >>> > > >>> On 8/30/2023 12:52 AM, Wenxing Hou wrote: > > >>>> *** Add BaseCryptLibMbedTls for CryptoPkg, which can be an alternative > > to > > >>> OpenSSL in some scenarios. There are four features in the patch: > > >>> HMAC/HKDF/RSA/HASH.*** > > >>>> > > >>>> Wenxing Hou (9): > > >>>> CryptoPkg: Add mbedtls submodule for EDKII > > >>>> CryptoPkg: Add mbedtls_config and MbedTlsLib.inf > > >>>> CryptoPkg: Add HMAC functions based on Mbedtls > > >>>> CryptoPkg: Add HKDF functions based on Mbedtls > > >>>> CryptoPkg: Add RSA functions based on Mbedtls > > >>>> CryptoPkg: Add all .inf files for BaseCryptLibMbedTls > > >>>> CryptoPkg: Add Null functions for building pass > > >>>> CryptoPkg: Add MD5/SHA1/SHA2 functions based on Mbedtls > > >>>> CryptoPkg: Add Mbedtls submodule in CI > > >>>> > > >>>> .gitmodules | 3 + > > >>>> .pytool/CISettings.py | 2 + > > >>>> CryptoPkg/CryptoPkg.ci.yaml | 66 +- > > >>>> CryptoPkg/CryptoPkg.dec | 4 + > > >>>> CryptoPkg/CryptoPkgMbedTls.dsc | 280 ++ > > >>>> .../BaseCryptLibMbedTls/BaseCryptLib.inf | 81 + > > >>>> .../BaseCryptLibMbedTls/Bn/CryptBnNull.c | 520 +++ > > >>>> .../Cipher/CryptAeadAesGcmNull.c | 100 + > > >>>> .../BaseCryptLibMbedTls/Cipher/CryptAesNull.c | 159 + > > >>>> .../BaseCryptLibMbedTls/Hash/CryptMd5.c | 234 + > > >>>> .../BaseCryptLibMbedTls/Hash/CryptMd5Null.c | 163 + > > >>>> .../Hash/CryptParallelHashNull.c | 40 + > > >>>> .../BaseCryptLibMbedTls/Hash/CryptSha1.c | 234 + > > >>>> .../BaseCryptLibMbedTls/Hash/CryptSha1Null.c | 166 + > > >>>> .../BaseCryptLibMbedTls/Hash/CryptSha256.c | 227 + > > >>>> .../Hash/CryptSha256Null.c | 162 + > > >>>> .../BaseCryptLibMbedTls/Hash/CryptSha512.c | 447 ++ > > >>>> .../Hash/CryptSha512Null.c | 275 ++ > > >>>> .../BaseCryptLibMbedTls/Hash/CryptSm3Null.c | 164 + > > >>>> .../BaseCryptLibMbedTls/Hmac/CryptHmac.c | 620 +++ > > >>>> .../BaseCryptLibMbedTls/Hmac/CryptHmacNull.c | 359 ++ > > >>>> .../BaseCryptLibMbedTls/InternalCryptLib.h | 44 + > > >>>> .../BaseCryptLibMbedTls/Kdf/CryptHkdf.c | 372 ++ > > >>>> .../BaseCryptLibMbedTls/Kdf/CryptHkdfNull.c | 192 + > > >>>> .../BaseCryptLibMbedTls/PeiCryptLib.inf | 101 + > > >>>> .../BaseCryptLibMbedTls/PeiCryptLib.uni | 25 + > > >>>> .../BaseCryptLibMbedTls/Pem/CryptPemNull.c | 69 + > > >>>> .../Pk/CryptAuthenticodeNull.c | 45 + > > >>>> .../BaseCryptLibMbedTls/Pk/CryptDhNull.c | 150 + > > >>>> .../BaseCryptLibMbedTls/Pk/CryptEcNull.c | 578 +++ > > >>>> .../Pk/CryptPkcs1OaepNull.c | 51 + > > >>>> .../Pk/CryptPkcs5Pbkdf2Null.c | 48 + > > >>>> .../Pk/CryptPkcs7Internal.h | 83 + > > >>>> .../Pk/CryptPkcs7SignNull.c | 53 + > > >>>> .../Pk/CryptPkcs7VerifyEkuNull.c | 152 + > > >>>> .../Pk/CryptPkcs7VerifyEkuRuntime.c | 56 + > > >>>> .../Pk/CryptPkcs7VerifyNull.c | 163 + > > >>>> .../Pk/CryptPkcs7VerifyRuntime.c | 38 + > > >>>> .../BaseCryptLibMbedTls/Pk/CryptRsaBasic.c | 268 ++ > > >>>> .../Pk/CryptRsaBasicNull.c | 121 + > > >>>> .../BaseCryptLibMbedTls/Pk/CryptRsaExt.c | 337 ++ > > >>>> .../BaseCryptLibMbedTls/Pk/CryptRsaExtNull.c | 117 + > > >>>> .../BaseCryptLibMbedTls/Pk/CryptRsaPss.c | 164 + > > >>>> .../BaseCryptLibMbedTls/Pk/CryptRsaPssNull.c | 46 + > > >>>> .../BaseCryptLibMbedTls/Pk/CryptRsaPssSign.c | 231 + > > >>>> .../Pk/CryptRsaPssSignNull.c | 60 + > > >>>> .../BaseCryptLibMbedTls/Pk/CryptTsNull.c | 42 + > > >>>> .../BaseCryptLibMbedTls/Pk/CryptX509Null.c | 753 ++++ > > >>>> .../BaseCryptLibMbedTls/Rand/CryptRandNull.c | 56 + > > >>>> .../BaseCryptLibMbedTls/RuntimeCryptLib.inf | 92 + > > >>>> .../BaseCryptLibMbedTls/RuntimeCryptLib.uni | 22 + > > >>>> .../BaseCryptLibMbedTls/SecCryptLib.inf | 84 + > > >>>> .../BaseCryptLibMbedTls/SecCryptLib.uni | 17 + > > >>>> .../BaseCryptLibMbedTls/SmmCryptLib.inf | 92 + > > >>>> .../BaseCryptLibMbedTls/SmmCryptLib.uni | 22 + > > >>>> .../SysCall/ConstantTimeClock.c | 75 + > > >>>> .../BaseCryptLibMbedTls/SysCall/CrtWrapper.c | 58 + > > >>>> .../SysCall/RuntimeMemAllocation.c | 462 ++ > > >>>> .../SysCall/TimerWrapper.c | 198 + > > >>>> .../BaseCryptLibMbedTls/TestBaseCryptLib.inf | 78 + > > >>>> CryptoPkg/Library/MbedTlsLib/CrtWrapper.c | 96 + > > >>>> CryptoPkg/Library/MbedTlsLib/EcSm2Null.c | 495 +++ > > >>>> .../Include/mbedtls/mbedtls_config.h | 3823 > > +++++++++++++++++ > > >>>> CryptoPkg/Library/MbedTlsLib/MbedTlsLib.inf | 173 + > > >>>> .../Library/MbedTlsLib/MbedTlsLibFull.inf | 177 + > > >>>> CryptoPkg/Library/MbedTlsLib/mbedtls | 1 + > > >>>> 66 files changed, 14683 insertions(+), 3 deletions(-) > > >>>> create mode 100644 CryptoPkg/CryptoPkgMbedTls.dsc > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/BaseCryptLib.inf > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Bn/CryptBnNull.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Cipher/CryptAeadAesGcmNull.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Cipher/CryptAesNull.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Hash/CryptMd5.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Hash/CryptMd5Null.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Hash/CryptParallelHashNull.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Hash/CryptSha1.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Hash/CryptSha1Null.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Hash/CryptSha256.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Hash/CryptSha256Null.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Hash/CryptSha512.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Hash/CryptSha512Null.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Hash/CryptSm3Null.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Hmac/CryptHmac.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Hmac/CryptHmacNull.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/InternalCryptLib.h > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Kdf/CryptHkdf.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Kdf/CryptHkdfNull.c > > >>>> create mode 100644 > > >> CryptoPkg/Library/BaseCryptLibMbedTls/PeiCryptLib.inf > > >>>> create mode 100644 > > >> CryptoPkg/Library/BaseCryptLibMbedTls/PeiCryptLib.uni > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Pem/CryptPemNull.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptAuthenticodeNull.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptDhNull.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptEcNull.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs1OaepNull.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs5Pbkdf2Null.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs7Internal.h > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs7SignNull.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs7VerifyEkuNull.c > > >>>> create mode 100644 > > >>> > CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs7VerifyEkuRuntime.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs7VerifyNull.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs7VerifyRuntime.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptRsaBasic.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptRsaBasicNull.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptRsaExt.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptRsaExtNull.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptRsaPss.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptRsaPssNull.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptRsaPssSign.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptRsaPssSignNull.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptTsNull.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptX509Null.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/Rand/CryptRandNull.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/RuntimeCryptLib.inf > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/RuntimeCryptLib.uni > > >>>> create mode 100644 > > >> CryptoPkg/Library/BaseCryptLibMbedTls/SecCryptLib.inf > > >>>> create mode 100644 > > >> CryptoPkg/Library/BaseCryptLibMbedTls/SecCryptLib.uni > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/SmmCryptLib.inf > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/SmmCryptLib.uni > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/SysCall/ConstantTimeClock.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/SysCall/CrtWrapper.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/SysCall/RuntimeMemAllocation.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/SysCall/TimerWrapper.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/BaseCryptLibMbedTls/TestBaseCryptLib.inf > > >>>> create mode 100644 CryptoPkg/Library/MbedTlsLib/CrtWrapper.c > > >>>> create mode 100644 CryptoPkg/Library/MbedTlsLib/EcSm2Null.c > > >>>> create mode 100644 > > >>> CryptoPkg/Library/MbedTlsLib/Include/mbedtls/mbedtls_config.h > > >>>> create mode 100644 CryptoPkg/Library/MbedTlsLib/MbedTlsLib.inf > > >>>> create mode 100644 CryptoPkg/Library/MbedTlsLib/MbedTlsLibFull.inf > > >>>> create mode 160000 CryptoPkg/Library/MbedTlsLib/mbedtls > > >>>> > > >>> > > >>> > > >>> > > >>> > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#108198): https://edk2.groups.io/g/devel/message/108198 Mute This Topic: https://groups.io/mt/101048094/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
