The breaking change would be in the future to move/make these structural
changes (package /repo) in the future after checking in this patch.
Thanks
Sean
On 8/31/2023 11:45 AM, Michael D Kinney wrote:
How is the current patch set a breaking change?
Mike
*From:* Sean Brogan <spbro...@outlook.com>
*Sent:* Thursday, August 31, 2023 10:52 AM
*To:* devel@edk2.groups.io; Kinney, Michael D
<michael.d.kin...@intel.com>; Yao, Jiewen <jiewen....@intel.com>; Leif
Lindholm <quic_llind...@quicinc.com>; Hou, Wenxing <wenxing....@intel.com>
*Cc:* af...@apple.com
*Subject:* Re: [edk2-devel] [edk2/add_mbedtls PATCH 0/9] *** Add
HMAC/HKDF/RSA/HASH features based on Mbedtls ***
replying to the whole chain.
I am not encouraging importing the source directly but still trying to
isolate the "wrapper code" and the external mbedtls submodule management.
I am advocating that the underlying crypto implementation is 100%
"hidden" from public include/dependency and the rest of the edk2
tree. I am advocating that crypto "releases" are in essence
independent of Edk2 stable tags (obviously a stable tag would still
have reference to version tested at that time) because crypto needs to
be updated more quickly and regularly and should have very minimal
breaking dependencies.
Regarding Jiewen's options for layout my proposal would be more like
option 5. :)
Tianocore/Edk2: CryptoPkg
* Header files for the crypto api of edk2(protocol, ppi, pcd,
library definitions).
* Modules that are underlying crypto library independent.
* Null libraries that support compile test of edk2 CI
Tianocore/MbedTlsRepo: MbedTlsCryptoPkg
* No public header files for consumption outside package.
* Wrapper code and modules to support edk2 crypto using mbedtls library.
* submodule tracking info for mbedtls upstream project
* tests
* release notes
* Security vulnerability management for mbedtls based work
Tianocore/OpenSslRepo: OpenSslCryptoPkg
* No public header files for consumption outside package.
* Wrapper code and modules to support edk2 crypto using openssl library.
* submodule tracking info for openssl upstream project
* tests
* release notes
* Security vulnerability management for OpenSSL based work
I hope that helps explain.
Regarding checking in and then moving later...well i am skeptical.
This is a breaking change and once dependencies are formed, edk2 has
historically had challenges in making these kinds of changes.
Thanks for consideration.
Sean
On 8/31/2023 10:24 AM, Michael D Kinney wrote:
Jiewen,
Thanks. Option #1 makes more sense if it is the Mbedtls
wrapper code.
I prefer Option #1.
Breaking out into multiple repos also makes it hard to align
Releases across multiple repos. We already have this as an
unsolved problem for edk2-platforms repo, and I am not interested
in adding more repos until we have a complete solution to do
edk2-platforms releases in place.
Mike
-----Original Message-----
From: Yao, Jiewen<jiewen....@intel.com> <mailto:jiewen....@intel.com>
Sent: Thursday, August 31, 2023 9:07 AM
To: Kinney, Michael D<michael.d.kin...@intel.com>
<mailto:michael.d.kin...@intel.com>; Leif Lindholm
<quic_llind...@quicinc.com>
<mailto:quic_llind...@quicinc.com>;devel@edk2.groups.io;spbro...@outlook.com;
Hou, Wenxing<wenxing....@intel.com> <mailto:wenxing....@intel.com>
Cc:af...@apple.com
Subject: RE: [edk2-devel] [edk2/add_mbedtls PATCH 0/9] *** Add
HMAC/HKDF/RSA/HASH features based on Mbedtls ***
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>
<mailto:michael.d.kin...@intel.com>
Sent: Thursday, August 31, 2023 11:45 PM
To: Leif Lindholm<quic_llind...@quicinc.com>
<mailto:quic_llind...@quicinc.com>; Yao, Jiewen
<jiewen....@intel.com>
<mailto:jiewen....@intel.com>;devel@edk2.groups.io;spbro...@outlook.com;
Hou,
Wenxing<wenxing....@intel.com> <mailto:wenxing....@intel.com>
Cc:af...@apple.com; Kinney, Michael D<michael.d.kin...@intel.com>
<mailto: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>
<mailto:quic_llind...@quicinc.com>
Sent: Thursday, August 31, 2023 4:15 AM
To: Yao, Jiewen<jiewen....@intel.com>
<mailto:jiewen....@intel.com>;devel@edk2.groups.io;
spbro...@outlook.com; Hou, Wenxing<wenxing....@intel.com>
<mailto:wenxing....@intel.com>
Cc:af...@apple.com; Kinney, Michael D<michael.d.kin...@intel.com>
<mailto: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> <mailto:wenxing....@intel.com>
Cc:af...@apple.com;quic_llind...@quicinc.com; Kinney,
Michael D
<michael.d.kin...@intel.com>
<mailto: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>
<mailto: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> <mailto: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 (#108207): https://edk2.groups.io/g/devel/message/108207
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]
-=-=-=-=-=-=-=-=-=-=-=-
