CryptoPkg/CryptoPkg.ci.yaml | 11 +-
CryptoPkg/CryptoPkg.dec | 42 +-
CryptoPkg/CryptoPkg.dsc | 460 +++++++++---
.../Pcd/PcdCryptoServiceFamilyEnable.h | 122 +--
.../Library/BaseCryptLib/BaseCryptLib.inf | 10 +-
.../Library/BaseCryptLib/BaseCryptLib.uni | 2 -
.../Library/BaseCryptLib/Hmac/CryptHmac.c | 7 +
.../Library/BaseCryptLib/Kdf/CryptHkdf.c | 5 +-
.../Library/BaseCryptLib/PeiCryptLib.inf | 8 +-
.../Library/BaseCryptLib/PeiCryptLib.uni | 2 -
.../BaseCryptLib/Pk/CryptAuthenticode.c | 2 +-
.../BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c | 3 +-
.../BaseCryptLib/Pk/CryptPkcs7VerifyEku.c | 3 +
CryptoPkg/Library/BaseCryptLib/Pk/CryptTs.c | 44 +-
.../Library/BaseCryptLib/RuntimeCryptLib.inf | 9 +-
.../Library/BaseCryptLib/RuntimeCryptLib.uni | 2 -
.../Library/BaseCryptLib/SecCryptLib.inf | 13 +-
.../{SmmCryptLib.uni => SecCryptLib.uni} | 11 +-
.../Library/BaseCryptLib/SmmCryptLib.inf | 12 -
.../Library/BaseCryptLib/SmmCryptLib.uni | 2 -
.../BaseCryptLib/UnitTestHostBaseCryptLib.inf | 22 +-
.../Library/Include/openssl/opensslconf.h | 328 +++++++-
.../Include/openssl/opensslconf_generated.h | 333 ---------
CryptoPkg/Library/OpensslLib/EcSm2Null.c | 291 ++++++++
CryptoPkg/Library/OpensslLib/OpensslLib.inf | 133 ++--
CryptoPkg/Library/OpensslLib/OpensslLib.uni | 10 +-
...nsslLibIa32Gcc.inf => OpensslLibAccel.inf} | 189 +++--
.../Library/OpensslLib/OpensslLibAccel.uni | 14 +
.../OpensslLib/OpensslLibConstructor.c | 6 +-
.../Library/OpensslLib/OpensslLibCrypto.inf | 185 +++--
.../Library/OpensslLib/OpensslLibCrypto.uni | 11 +-
.../{OpensslLib.inf => OpensslLibFull.inf} | 143 ++--
.../{OpensslLib.uni => OpensslLibFull.uni} | 10 +-
...sslLibIa32.inf => OpensslLibFullAccel.inf} | 192 +++--
.../OpensslLib/OpensslLibFullAccel.uni | 14 +
.../Library/OpensslLib/OpensslLibX64.inf | 706 ------------------
.../Library/OpensslLib/OpensslLibX64Gcc.inf | 706 ------------------
CryptoPkg/Library/OpensslLib/SslNull.c | 405 ++++++++++
.../SysCall/inet_pton.c | 0
CryptoPkg/Library/TlsLib/TlsConfig.c | 2 +-
CryptoPkg/Library/TlsLib/TlsLib.inf | 12 +-
CryptoPkg/Private/Library/IntrinsicLib.h | 16 +
CryptoPkg/Private/Library/OpensslLib.h | 14 +
CryptoPkg/Readme.md | 498 ++++++++++++
CryptoPkg/Test/CryptoPkgHostUnitTest.dsc | 17 +-
.../UnitTest/Library/BaseCryptLib/HmacTests.c | 17 +-
.../UnitTest/Library/BaseCryptLib/TSTests.c | 2 +-
.../TestBaseCryptLibHostAccel.inf | 55 ++
48 files changed, 2667 insertions(+), 2434 deletions(-)
copy CryptoPkg/Library/BaseCryptLib/{SmmCryptLib.uni => SecCryptLib.uni} (74%)
delete mode 100644 CryptoPkg/Library/Include/openssl/opensslconf_generated.h
create mode 100644 CryptoPkg/Library/OpensslLib/EcSm2Null.c
rename CryptoPkg/Library/OpensslLib/{OpensslLibIa32Gcc.inf => OpensslLibAccel.inf} (79%)
create mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibAccel.uni
copy CryptoPkg/Library/OpensslLib/{OpensslLib.inf => OpensslLibFull.inf} (80%)
copy CryptoPkg/Library/OpensslLib/{OpensslLib.uni => OpensslLibFull.uni} (56%)
rename CryptoPkg/Library/OpensslLib/{OpensslLibIa32.inf => OpensslLibFullAccel.inf} (79%)
create mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.uni
delete mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibX64.inf
delete mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibX64Gcc.inf
create mode 100644 CryptoPkg/Library/OpensslLib/SslNull.c
rename CryptoPkg/Library/{BaseCryptLib => TlsLib}/SysCall/inet_pton.c (100%)
create mode 100644 CryptoPkg/Private/Library/IntrinsicLib.h
create mode 100644 CryptoPkg/Private/Library/OpensslLib.h
create mode 100644 CryptoPkg/Readme.md
create mode 100644 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibHostAccel.inf
The recent addition of the Ecliptic Curve (EC) feature and the performance
optimization features increased the complexity for platforms to integrate
and enable these features. This series simplifies the platform configuration
as much as possible and improves the ability to manage the the size impact
of cryptographic services in each FW phase. A Readme.md is also added that
provides an overview of the CryptoPkg design and features along with platform
integration recommendations.
This series also addresses private library class declarations missing from
CryptoPkg.dec and library instances not producing all the APIs defined
by the library classes. A review of the CryptoPkg EDK II meta data files identified
a number of additional cleanups. The CryptoPkg.dsc file was also updated to
improve CI coverage for future CryptoPkg changes and identified some
unit test bug fixes.
PR: https://github.com/tianocore/edk2/pull/3443
Branch: https://github.com/mdkinney/edk2/tree/CryptoPkg_RemoveEcPcd_MergeOptimizedOpensslLibs
Readme: https://github.com/mdkinney/edk2/blob/CryptoPkg_RemoveEcPcd_MergeOptimizedOpensslLibs/CryptoPkg/Readme.md
Change Summary
==============
* Document disabled/deprecated cryptographic services
* Add missing UNI files in BaseCryptLib
* Update BaseCryptLib internal functions to be STATIC and remove EFIAPI
* Add GLOBAL_REMOVE_IF_UNREFERENCED to BaseCryptLib global variables
* Fix BaseCryptLib unit tests
* Cleanup BaseCryptLib and TlsLib INF files and
* Move SysCall/inet_pton.c from BaseCryptLib to TlsLib that uses it.
* Merge 4 performance optimized INFs into OpensslLib*Accel.inf
* Remove use of PcdOpensslEcEnabled and use OpensslLibFull*.inf instead
* Add OpensslLib and IntrinsicLib to CryptoPkg.dec as private library classes
* Update all OpensslLib instances to always produce all APIs in OpensslLib class
* Move PrintLib dependency from OpensslLib INF files to BaseCryptLib INF files
* Update CryptoPkg.dsc files to provide full CI test coverage across all the
supported combinations of OpensslLib, BaseCryptLib, and TlsLib instances.
* Remove PACKAGE profile from CryptoPkg.dsc and add TARGET_UNIT_TESTS
profile. Adding TARGET_UNIT_TESTS profile is required to prevent a few unit
test artifacts being included in non unit test builds of components.
* Add CryptoPkg Readme.md with overview and platform integration details.
* Update host-based unit tests to always use OpensslLibFull.inf and add unit
test coverage for OpensslLibFullAccel.inf.
* Add Readme.md with CryptoPkg overview and platform integration
documentation
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Xiaoyu Lu <xiaoyu1.lu@intel.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Christopher Zurcher <christopher.zurcher@microsoft.com>
Cc: Rebecca Cran <quic_rcran@quicinc.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Michael D Kinney (12):
CryptoPkg: Document and disable deprecated crypto services
CryptoPkg/Library/BaseCryptLib: Add missing UNI file and fix format
CryptoPkg/Library/BaseCryptLib: Update internal functions/variables
CryptoPkg/Test/UnitTest/Library/BaseCryptLib: Unit test fixes
CryptoPkg/Library: Cleanup BaseCryptLib and TlsLib
CryptoPkg/Library/OpensslLib: Combine all performance optimized INFs
CryptoPkg/Library/OpensslLib: Produce consistent set of APIs
CryptoPkg/Library/OpensslLib: Remove PrintLib from INF files
CryptoPkg: Remove PcdOpensslEcEnabled from CryptoPkg.dec
CryptoPkg: Update DSC to improve CI test coverage
CryptoPkg: Fixed host-based unit tests
CryptoPkg: Add Readme.md
CryptoPkg/CryptoPkg.ci.yaml | 11 +-
CryptoPkg/CryptoPkg.dec | 42 +-
CryptoPkg/CryptoPkg.dsc | 460 +++++++++---
.../Pcd/PcdCryptoServiceFamilyEnable.h | 122 +--
.../Library/BaseCryptLib/BaseCryptLib.inf | 10 +-
.../Library/BaseCryptLib/BaseCryptLib.uni | 2 -
.../Library/BaseCryptLib/Hmac/CryptHmac.c | 7 +
.../Library/BaseCryptLib/Kdf/CryptHkdf.c | 5 +-
.../Library/BaseCryptLib/PeiCryptLib.inf | 8 +-
.../Library/BaseCryptLib/PeiCryptLib.uni | 2 -
.../BaseCryptLib/Pk/CryptAuthenticode.c | 2 +-
.../BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c | 3 +-
.../BaseCryptLib/Pk/CryptPkcs7VerifyEku.c | 3 +
CryptoPkg/Library/BaseCryptLib/Pk/CryptTs.c | 44 +-
.../Library/BaseCryptLib/RuntimeCryptLib.inf | 9 +-
.../Library/BaseCryptLib/RuntimeCryptLib.uni | 2 -
.../Library/BaseCryptLib/SecCryptLib.inf | 13 +-
.../{SmmCryptLib.uni => SecCryptLib.uni} | 11 +-
.../Library/BaseCryptLib/SmmCryptLib.inf | 12 -
.../Library/BaseCryptLib/SmmCryptLib.uni | 2 -
.../BaseCryptLib/UnitTestHostBaseCryptLib.inf | 22 +-
.../Library/Include/openssl/opensslconf.h | 328 +++++++-
.../Include/openssl/opensslconf_generated.h | 333 ---------
CryptoPkg/Library/OpensslLib/EcSm2Null.c | 291 ++++++++
CryptoPkg/Library/OpensslLib/OpensslLib.inf | 133 ++--
CryptoPkg/Library/OpensslLib/OpensslLib.uni | 10 +-
...nsslLibIa32Gcc.inf => OpensslLibAccel.inf} | 189 +++--
.../Library/OpensslLib/OpensslLibAccel.uni | 14 +
.../OpensslLib/OpensslLibConstructor.c | 6 +-
.../Library/OpensslLib/OpensslLibCrypto.inf | 185 +++--
.../Library/OpensslLib/OpensslLibCrypto.uni | 11 +-
.../{OpensslLib.inf => OpensslLibFull.inf} | 143 ++--
.../{OpensslLib.uni => OpensslLibFull.uni} | 10 +-
...sslLibIa32.inf => OpensslLibFullAccel.inf} | 192 +++--
.../OpensslLib/OpensslLibFullAccel.uni | 14 +
.../Library/OpensslLib/OpensslLibX64.inf | 706 ------------------
.../Library/OpensslLib/OpensslLibX64Gcc.inf | 706 ------------------
CryptoPkg/Library/OpensslLib/SslNull.c | 405 ++++++++++
.../SysCall/inet_pton.c | 0
CryptoPkg/Library/TlsLib/TlsConfig.c | 2 +-
CryptoPkg/Library/TlsLib/TlsLib.inf | 12 +-
CryptoPkg/Private/Library/IntrinsicLib.h | 16 +
CryptoPkg/Private/Library/OpensslLib.h | 14 +
CryptoPkg/Readme.md | 498 ++++++++++++
CryptoPkg/Test/CryptoPkgHostUnitTest.dsc | 17 +-
.../UnitTest/Library/BaseCryptLib/HmacTests.c | 17 +-
.../UnitTest/Library/BaseCryptLib/TSTests.c | 2 +-
.../TestBaseCryptLibHostAccel.inf | 55 ++
48 files changed, 2667 insertions(+), 2434 deletions(-)
copy CryptoPkg/Library/BaseCryptLib/{SmmCryptLib.uni => SecCryptLib.uni} (74%)
delete mode 100644 CryptoPkg/Library/Include/openssl/opensslconf_generated.h
create mode 100644 CryptoPkg/Library/OpensslLib/EcSm2Null.c
rename CryptoPkg/Library/OpensslLib/{OpensslLibIa32Gcc.inf => OpensslLibAccel.inf} (79%)
create mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibAccel.uni
copy CryptoPkg/Library/OpensslLib/{OpensslLib.inf => OpensslLibFull.inf} (80%)
copy CryptoPkg/Library/OpensslLib/{OpensslLib.uni => OpensslLibFull.uni} (56%)
rename CryptoPkg/Library/OpensslLib/{OpensslLibIa32.inf => OpensslLibFullAccel.inf} (79%)
create mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.uni
delete mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibX64.inf
delete mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibX64Gcc.inf
create mode 100644 CryptoPkg/Library/OpensslLib/SslNull.c
rename CryptoPkg/Library/{BaseCryptLib => TlsLib}/SysCall/inet_pton.c (100%)
create mode 100644 CryptoPkg/Private/Library/IntrinsicLib.h
create mode 100644 CryptoPkg/Private/Library/OpensslLib.h
create mode 100644 CryptoPkg/Readme.md
create mode 100644 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibHostAccel.inf
--
2.37.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#94993): https://edk2.groups.io/g/devel/message/94993
Mute This Topic: https://groups.io/mt/94260718/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Thank you Mike.
1) I like the idea to combine multiple OpensslLibIA32/X64.inf into one OpensslLibAccel.inf.
Also the cleanup looks good to me.
2) I also like the summary in readme in https://github.com/mdkinney/edk2/tree/CryptoPkg_RemoveEcPcd_MergeOptimizedOpensslLibs/CryptoPkg
I notice some algorithms are listed Y(Deprecated) but N(Don't Use), such as Tdes, Arc4, Aes.Ecb*.
But I don't see the use case for those algorithms and I suggest a Y(Deprecated) have Y(Don't Use).
3) About PcdOpensslEcEnabled
I notice it is used in existing code - https://github.com/mdkinney/edk2/blob/CryptoPkg_RemoveEcPcd_MergeOptimizedOpensslLibs/CryptoPkg/Library/TlsLib/TlsConfig.c#L1139
Is this right way?
Thank you
Yao, Jiewen
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Tuesday, October 11, 2022 11:04 PM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Lu, Xiaoyu1 <xiaoyu1.lu@intel.com>; Jiang,
> Guomin <guomin.jiang@intel.com>; Zurcher, Christopher
> <christopher.zurcher@microsoft.com>; Rebecca Cran
> <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> Subject: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt
> OpensslLibs
>
> The recent addition of the Ecliptic Curve (EC) feature and the performance
> optimization features increased the complexity for platforms to integrate
> and enable these features. This series simplifies the platform configuration
> as much as possible and improves the ability to manage the the size impact
> of cryptographic services in each FW phase. A Readme.md is also added
> that
> provides an overview of the CryptoPkg design and features along with
> platform
> integration recommendations.
>
> This series also addresses private library class declarations missing from
> CryptoPkg.dec and library instances not producing all the APIs defined
> by the library classes. A review of the CryptoPkg EDK II meta data files
> identified
> a number of additional cleanups. The CryptoPkg.dsc file was also updated to
> improve CI coverage for future CryptoPkg changes and identified some
> unit test bug fixes.
>
> PR: https://github.com/tianocore/edk2/pull/3443
> Branch:
> https://github.com/mdkinney/edk2/tree/CryptoPkg_RemoveEcPcd_Merge
> OptimizedOpensslLibs
> Readme:
> https://github.com/mdkinney/edk2/blob/CryptoPkg_RemoveEcPcd_Merge
> OptimizedOpensslLibs/CryptoPkg/Readme.md
>
> Change Summary
> ==============
> * Document disabled/deprecated cryptographic services
> * Add missing UNI files in BaseCryptLib
> * Update BaseCryptLib internal functions to be STATIC and remove EFIAPI
> * Add GLOBAL_REMOVE_IF_UNREFERENCED to BaseCryptLib global
> variables
> * Fix BaseCryptLib unit tests
> * Cleanup BaseCryptLib and TlsLib INF files and
> * Move SysCall/inet_pton.c from BaseCryptLib to TlsLib that uses it.
> * Merge 4 performance optimized INFs into OpensslLib*Accel.inf
> * Remove use of PcdOpensslEcEnabled and use OpensslLibFull*.inf instead
> * Add OpensslLib and IntrinsicLib to CryptoPkg.dec as private library classes
> * Update all OpensslLib instances to always produce all APIs in OpensslLib
> class
> * Move PrintLib dependency from OpensslLib INF files to BaseCryptLib INF
> files
> * Update CryptoPkg.dsc files to provide full CI test coverage across all the
> supported combinations of OpensslLib, BaseCryptLib, and TlsLib instances.
> * Remove PACKAGE profile from CryptoPkg.dsc and add
> TARGET_UNIT_TESTS
> profile. Adding TARGET_UNIT_TESTS profile is required to prevent a few
> unit
> test artifacts being included in non unit test builds of components.
> * Add CryptoPkg Readme.md with overview and platform integration
> details.
> * Update host-based unit tests to always use OpensslLibFull.inf and add
> unit
> test coverage for OpensslLibFullAccel.inf.
> * Add Readme.md with CryptoPkg overview and platform integration
> documentation
>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Xiaoyu Lu <xiaoyu1.lu@intel.com>
> Cc: Guomin Jiang <guomin.jiang@intel.com>
> Cc: Christopher Zurcher <christopher.zurcher@microsoft.com>
> Cc: Rebecca Cran <quic_rcran@quicinc.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
>
> Michael D Kinney (12):
> CryptoPkg: Document and disable deprecated crypto services
> CryptoPkg/Library/BaseCryptLib: Add missing UNI file and fix format
> CryptoPkg/Library/BaseCryptLib: Update internal functions/variables
> CryptoPkg/Test/UnitTest/Library/BaseCryptLib: Unit test fixes
> CryptoPkg/Library: Cleanup BaseCryptLib and TlsLib
> CryptoPkg/Library/OpensslLib: Combine all performance optimized INFs
> CryptoPkg/Library/OpensslLib: Produce consistent set of APIs
> CryptoPkg/Library/OpensslLib: Remove PrintLib from INF files
> CryptoPkg: Remove PcdOpensslEcEnabled from CryptoPkg.dec
> CryptoPkg: Update DSC to improve CI test coverage
> CryptoPkg: Fixed host-based unit tests
> CryptoPkg: Add Readme.md
>
> CryptoPkg/CryptoPkg.ci.yaml | 11 +-
> CryptoPkg/CryptoPkg.dec | 42 +-
> CryptoPkg/CryptoPkg.dsc | 460 +++++++++---
> .../Pcd/PcdCryptoServiceFamilyEnable.h | 122 +--
> .../Library/BaseCryptLib/BaseCryptLib.inf | 10 +-
> .../Library/BaseCryptLib/BaseCryptLib.uni | 2 -
> .../Library/BaseCryptLib/Hmac/CryptHmac.c | 7 +
> .../Library/BaseCryptLib/Kdf/CryptHkdf.c | 5 +-
> .../Library/BaseCryptLib/PeiCryptLib.inf | 8 +-
> .../Library/BaseCryptLib/PeiCryptLib.uni | 2 -
> .../BaseCryptLib/Pk/CryptAuthenticode.c | 2 +-
> .../BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c | 3 +-
> .../BaseCryptLib/Pk/CryptPkcs7VerifyEku.c | 3 +
> CryptoPkg/Library/BaseCryptLib/Pk/CryptTs.c | 44 +-
> .../Library/BaseCryptLib/RuntimeCryptLib.inf | 9 +-
> .../Library/BaseCryptLib/RuntimeCryptLib.uni | 2 -
> .../Library/BaseCryptLib/SecCryptLib.inf | 13 +-
> .../{SmmCryptLib.uni => SecCryptLib.uni} | 11 +-
> .../Library/BaseCryptLib/SmmCryptLib.inf | 12 -
> .../Library/BaseCryptLib/SmmCryptLib.uni | 2 -
> .../BaseCryptLib/UnitTestHostBaseCryptLib.inf | 22 +-
> .../Library/Include/openssl/opensslconf.h | 328 +++++++-
> .../Include/openssl/opensslconf_generated.h | 333 ---------
> CryptoPkg/Library/OpensslLib/EcSm2Null.c | 291 ++++++++
> CryptoPkg/Library/OpensslLib/OpensslLib.inf | 133 ++--
> CryptoPkg/Library/OpensslLib/OpensslLib.uni | 10 +-
> ...nsslLibIa32Gcc.inf => OpensslLibAccel.inf} | 189 +++--
> .../Library/OpensslLib/OpensslLibAccel.uni | 14 +
> .../OpensslLib/OpensslLibConstructor.c | 6 +-
> .../Library/OpensslLib/OpensslLibCrypto.inf | 185 +++--
> .../Library/OpensslLib/OpensslLibCrypto.uni | 11 +-
> .../{OpensslLib.inf => OpensslLibFull.inf} | 143 ++--
> .../{OpensslLib.uni => OpensslLibFull.uni} | 10 +-
> ...sslLibIa32.inf => OpensslLibFullAccel.inf} | 192 +++--
> .../OpensslLib/OpensslLibFullAccel.uni | 14 +
> .../Library/OpensslLib/OpensslLibX64.inf | 706 ------------------
> .../Library/OpensslLib/OpensslLibX64Gcc.inf | 706 ------------------
> CryptoPkg/Library/OpensslLib/SslNull.c | 405 ++++++++++
> .../SysCall/inet_pton.c | 0
> CryptoPkg/Library/TlsLib/TlsConfig.c | 2 +-
> CryptoPkg/Library/TlsLib/TlsLib.inf | 12 +-
> CryptoPkg/Private/Library/IntrinsicLib.h | 16 +
> CryptoPkg/Private/Library/OpensslLib.h | 14 +
> CryptoPkg/Readme.md | 498 ++++++++++++
> CryptoPkg/Test/CryptoPkgHostUnitTest.dsc | 17 +-
> .../UnitTest/Library/BaseCryptLib/HmacTests.c | 17 +-
> .../UnitTest/Library/BaseCryptLib/TSTests.c | 2 +-
> .../TestBaseCryptLibHostAccel.inf | 55 ++
> 48 files changed, 2667 insertions(+), 2434 deletions(-)
> copy CryptoPkg/Library/BaseCryptLib/{SmmCryptLib.uni =>
> SecCryptLib.uni} (74%)
> delete mode 100644
> CryptoPkg/Library/Include/openssl/opensslconf_generated.h
> create mode 100644 CryptoPkg/Library/OpensslLib/EcSm2Null.c
> rename CryptoPkg/Library/OpensslLib/{OpensslLibIa32Gcc.inf =>
> OpensslLibAccel.inf} (79%)
> create mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibAccel.uni
> copy CryptoPkg/Library/OpensslLib/{OpensslLib.inf => OpensslLibFull.inf}
> (80%)
> copy CryptoPkg/Library/OpensslLib/{OpensslLib.uni => OpensslLibFull.uni}
> (56%)
> rename CryptoPkg/Library/OpensslLib/{OpensslLibIa32.inf =>
> OpensslLibFullAccel.inf} (79%)
> create mode 100644
> CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.uni
> delete mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibX64.inf
> delete mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibX64Gcc.inf
> create mode 100644 CryptoPkg/Library/OpensslLib/SslNull.c
> rename CryptoPkg/Library/{BaseCryptLib => TlsLib}/SysCall/inet_pton.c
> (100%)
> create mode 100644 CryptoPkg/Private/Library/IntrinsicLib.h
> create mode 100644 CryptoPkg/Private/Library/OpensslLib.h
> create mode 100644 CryptoPkg/Readme.md
> create mode 100644
> CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibHostAccel.i
> nf
>
> --
> 2.37.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95025): https://edk2.groups.io/g/devel/message/95025
Mute This Topic: https://groups.io/mt/94260718/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi Jiewen,
Comments below.
Mike
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Tuesday, October 11, 2022 6:09 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1 <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>; Zurcher,
> Christopher <christopher.zurcher@microsoft.com>; Rebecca Cran <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> Subject: RE: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt OpensslLibs
>
> Thank you Mike.
>
> 1) I like the idea to combine multiple OpensslLibIA32/X64.inf into one OpensslLibAccel.inf.
> Also the cleanup looks good to me.
>
> 2) I also like the summary in readme in
> https://github.com/mdkinney/edk2/tree/CryptoPkg_RemoveEcPcd_MergeOptimizedOpensslLibs/CryptoPkg
> I notice some algorithms are listed Y(Deprecated) but N(Don't Use), such as Tdes, Arc4, Aes.Ecb*.
> But I don’t see the use case for those algorithms and I suggest a Y(Deprecated) have Y(Don't Use).
Good catch. I will fix.
>
> 3) About PcdOpensslEcEnabled
> I notice it is used in existing code -
> https://github.com/mdkinney/edk2/blob/CryptoPkg_RemoveEcPcd_MergeOptimizedOpensslLibs/CryptoPkg/Library/TlsLib/TlsConfig.c#L11
> 39
> Is this right way?
This was added since I started this work and was added back in by rebase. I will fix.
We can just remove the check for the PCD. If the OpensslLib instance does not include
SSL services, then the Null SSL services are present and the call to SSL_ctrl() will
return 0 which will force TlsSetEcCurve() to return EFI_UNSUPPORTED. It will also
ASSERT() informing the developer that a call to a service that depends on SSL was made
without SSL services available.
long
SSL_ctrl (
SSL *ssl,
int cmd,
long larg,
void *parg
)
{
ASSERT (FALSE);
return 0;
}
Likewise, if the OpensslLib instance does not support EC services, then the Null
EC services will be included and the call to EC_KEY_new_by_curve_name() will
return NULL which will force TlsSetEcCurve() to return EFI_UNSUPPORTED. It will also
ASSERT() informing the developer that a call to a service that depends on EC was made
without EC services available.
EC_KEY *
EC_KEY_new_by_curve_name (
int nid
)
{
ASSERT (FALSE);
return NULL;
}
>
> Thank you
> Yao, Jiewen
>
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > Sent: Tuesday, October 11, 2022 11:04 PM
> > To: devel@edk2.groups.io
> > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> > <jian.j.wang@intel.com>; Lu, Xiaoyu1 <xiaoyu1.lu@intel.com>; Jiang,
> > Guomin <guomin.jiang@intel.com>; Zurcher, Christopher
> > <christopher.zurcher@microsoft.com>; Rebecca Cran
> > <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> > Subject: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt
> > OpensslLibs
> >
> > The recent addition of the Ecliptic Curve (EC) feature and the performance
> > optimization features increased the complexity for platforms to integrate
> > and enable these features. This series simplifies the platform configuration
> > as much as possible and improves the ability to manage the the size impact
> > of cryptographic services in each FW phase. A Readme.md is also added
> > that
> > provides an overview of the CryptoPkg design and features along with
> > platform
> > integration recommendations.
> >
> > This series also addresses private library class declarations missing from
> > CryptoPkg.dec and library instances not producing all the APIs defined
> > by the library classes. A review of the CryptoPkg EDK II meta data files
> > identified
> > a number of additional cleanups. The CryptoPkg.dsc file was also updated to
> > improve CI coverage for future CryptoPkg changes and identified some
> > unit test bug fixes.
> >
> > PR: https://github.com/tianocore/edk2/pull/3443
> > Branch:
> > https://github.com/mdkinney/edk2/tree/CryptoPkg_RemoveEcPcd_Merge
> > OptimizedOpensslLibs
> > Readme:
> > https://github.com/mdkinney/edk2/blob/CryptoPkg_RemoveEcPcd_Merge
> > OptimizedOpensslLibs/CryptoPkg/Readme.md
> >
> > Change Summary
> > ==============
> > * Document disabled/deprecated cryptographic services
> > * Add missing UNI files in BaseCryptLib
> > * Update BaseCryptLib internal functions to be STATIC and remove EFIAPI
> > * Add GLOBAL_REMOVE_IF_UNREFERENCED to BaseCryptLib global
> > variables
> > * Fix BaseCryptLib unit tests
> > * Cleanup BaseCryptLib and TlsLib INF files and
> > * Move SysCall/inet_pton.c from BaseCryptLib to TlsLib that uses it.
> > * Merge 4 performance optimized INFs into OpensslLib*Accel.inf
> > * Remove use of PcdOpensslEcEnabled and use OpensslLibFull*.inf instead
> > * Add OpensslLib and IntrinsicLib to CryptoPkg.dec as private library classes
> > * Update all OpensslLib instances to always produce all APIs in OpensslLib
> > class
> > * Move PrintLib dependency from OpensslLib INF files to BaseCryptLib INF
> > files
> > * Update CryptoPkg.dsc files to provide full CI test coverage across all the
> > supported combinations of OpensslLib, BaseCryptLib, and TlsLib instances.
> > * Remove PACKAGE profile from CryptoPkg.dsc and add
> > TARGET_UNIT_TESTS
> > profile. Adding TARGET_UNIT_TESTS profile is required to prevent a few
> > unit
> > test artifacts being included in non unit test builds of components.
> > * Add CryptoPkg Readme.md with overview and platform integration
> > details.
> > * Update host-based unit tests to always use OpensslLibFull.inf and add
> > unit
> > test coverage for OpensslLibFullAccel.inf.
> > * Add Readme.md with CryptoPkg overview and platform integration
> > documentation
> >
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Xiaoyu Lu <xiaoyu1.lu@intel.com>
> > Cc: Guomin Jiang <guomin.jiang@intel.com>
> > Cc: Christopher Zurcher <christopher.zurcher@microsoft.com>
> > Cc: Rebecca Cran <quic_rcran@quicinc.com>
> > Cc: Ard Biesheuvel <ardb@kernel.org>
> > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> >
> > Michael D Kinney (12):
> > CryptoPkg: Document and disable deprecated crypto services
> > CryptoPkg/Library/BaseCryptLib: Add missing UNI file and fix format
> > CryptoPkg/Library/BaseCryptLib: Update internal functions/variables
> > CryptoPkg/Test/UnitTest/Library/BaseCryptLib: Unit test fixes
> > CryptoPkg/Library: Cleanup BaseCryptLib and TlsLib
> > CryptoPkg/Library/OpensslLib: Combine all performance optimized INFs
> > CryptoPkg/Library/OpensslLib: Produce consistent set of APIs
> > CryptoPkg/Library/OpensslLib: Remove PrintLib from INF files
> > CryptoPkg: Remove PcdOpensslEcEnabled from CryptoPkg.dec
> > CryptoPkg: Update DSC to improve CI test coverage
> > CryptoPkg: Fixed host-based unit tests
> > CryptoPkg: Add Readme.md
> >
> > CryptoPkg/CryptoPkg.ci.yaml | 11 +-
> > CryptoPkg/CryptoPkg.dec | 42 +-
> > CryptoPkg/CryptoPkg.dsc | 460 +++++++++---
> > .../Pcd/PcdCryptoServiceFamilyEnable.h | 122 +--
> > .../Library/BaseCryptLib/BaseCryptLib.inf | 10 +-
> > .../Library/BaseCryptLib/BaseCryptLib.uni | 2 -
> > .../Library/BaseCryptLib/Hmac/CryptHmac.c | 7 +
> > .../Library/BaseCryptLib/Kdf/CryptHkdf.c | 5 +-
> > .../Library/BaseCryptLib/PeiCryptLib.inf | 8 +-
> > .../Library/BaseCryptLib/PeiCryptLib.uni | 2 -
> > .../BaseCryptLib/Pk/CryptAuthenticode.c | 2 +-
> > .../BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c | 3 +-
> > .../BaseCryptLib/Pk/CryptPkcs7VerifyEku.c | 3 +
> > CryptoPkg/Library/BaseCryptLib/Pk/CryptTs.c | 44 +-
> > .../Library/BaseCryptLib/RuntimeCryptLib.inf | 9 +-
> > .../Library/BaseCryptLib/RuntimeCryptLib.uni | 2 -
> > .../Library/BaseCryptLib/SecCryptLib.inf | 13 +-
> > .../{SmmCryptLib.uni => SecCryptLib.uni} | 11 +-
> > .../Library/BaseCryptLib/SmmCryptLib.inf | 12 -
> > .../Library/BaseCryptLib/SmmCryptLib.uni | 2 -
> > .../BaseCryptLib/UnitTestHostBaseCryptLib.inf | 22 +-
> > .../Library/Include/openssl/opensslconf.h | 328 +++++++-
> > .../Include/openssl/opensslconf_generated.h | 333 ---------
> > CryptoPkg/Library/OpensslLib/EcSm2Null.c | 291 ++++++++
> > CryptoPkg/Library/OpensslLib/OpensslLib.inf | 133 ++--
> > CryptoPkg/Library/OpensslLib/OpensslLib.uni | 10 +-
> > ...nsslLibIa32Gcc.inf => OpensslLibAccel.inf} | 189 +++--
> > .../Library/OpensslLib/OpensslLibAccel.uni | 14 +
> > .../OpensslLib/OpensslLibConstructor.c | 6 +-
> > .../Library/OpensslLib/OpensslLibCrypto.inf | 185 +++--
> > .../Library/OpensslLib/OpensslLibCrypto.uni | 11 +-
> > .../{OpensslLib.inf => OpensslLibFull.inf} | 143 ++--
> > .../{OpensslLib.uni => OpensslLibFull.uni} | 10 +-
> > ...sslLibIa32.inf => OpensslLibFullAccel.inf} | 192 +++--
> > .../OpensslLib/OpensslLibFullAccel.uni | 14 +
> > .../Library/OpensslLib/OpensslLibX64.inf | 706 ------------------
> > .../Library/OpensslLib/OpensslLibX64Gcc.inf | 706 ------------------
> > CryptoPkg/Library/OpensslLib/SslNull.c | 405 ++++++++++
> > .../SysCall/inet_pton.c | 0
> > CryptoPkg/Library/TlsLib/TlsConfig.c | 2 +-
> > CryptoPkg/Library/TlsLib/TlsLib.inf | 12 +-
> > CryptoPkg/Private/Library/IntrinsicLib.h | 16 +
> > CryptoPkg/Private/Library/OpensslLib.h | 14 +
> > CryptoPkg/Readme.md | 498 ++++++++++++
> > CryptoPkg/Test/CryptoPkgHostUnitTest.dsc | 17 +-
> > .../UnitTest/Library/BaseCryptLib/HmacTests.c | 17 +-
> > .../UnitTest/Library/BaseCryptLib/TSTests.c | 2 +-
> > .../TestBaseCryptLibHostAccel.inf | 55 ++
> > 48 files changed, 2667 insertions(+), 2434 deletions(-)
> > copy CryptoPkg/Library/BaseCryptLib/{SmmCryptLib.uni =>
> > SecCryptLib.uni} (74%)
> > delete mode 100644
> > CryptoPkg/Library/Include/openssl/opensslconf_generated.h
> > create mode 100644 CryptoPkg/Library/OpensslLib/EcSm2Null.c
> > rename CryptoPkg/Library/OpensslLib/{OpensslLibIa32Gcc.inf =>
> > OpensslLibAccel.inf} (79%)
> > create mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibAccel.uni
> > copy CryptoPkg/Library/OpensslLib/{OpensslLib.inf => OpensslLibFull.inf}
> > (80%)
> > copy CryptoPkg/Library/OpensslLib/{OpensslLib.uni => OpensslLibFull.uni}
> > (56%)
> > rename CryptoPkg/Library/OpensslLib/{OpensslLibIa32.inf =>
> > OpensslLibFullAccel.inf} (79%)
> > create mode 100644
> > CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.uni
> > delete mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibX64.inf
> > delete mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibX64Gcc.inf
> > create mode 100644 CryptoPkg/Library/OpensslLib/SslNull.c
> > rename CryptoPkg/Library/{BaseCryptLib => TlsLib}/SysCall/inet_pton.c
> > (100%)
> > create mode 100644 CryptoPkg/Private/Library/IntrinsicLib.h
> > create mode 100644 CryptoPkg/Private/Library/OpensslLib.h
> > create mode 100644 CryptoPkg/Readme.md
> > create mode 100644
> > CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibHostAccel.i
> > nf
> >
> > --
> > 2.37.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95028): https://edk2.groups.io/g/devel/message/95028
Mute This Topic: https://groups.io/mt/94260718/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi Mike
For PcdOpensslEcEnabled, I am seeing other usage. For example:
https://github.com/qizhangz/edk2/blob/EC_Upstream/CryptoPkg/Library/BaseCryptLib/Pem/CryptPem.c#L156
I think we may need BKM on "how to disable EC".
Thank you
Yao, Jiewen
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Wednesday, October 12, 2022 9:25 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1
> <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
> Zurcher, Christopher <christopher.zurcher@microsoft.com>; Rebecca Cran
> <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> Subject: RE: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt
> OpensslLibs
>
> Hi Jiewen,
>
> Comments below.
>
> Mike
>
> > -----Original Message-----
> > From: Yao, Jiewen <jiewen.yao@intel.com>
> > Sent: Tuesday, October 11, 2022 6:09 PM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1
> <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
> Zurcher,
> > Christopher <christopher.zurcher@microsoft.com>; Rebecca Cran
> <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> > Subject: RE: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt
> OpensslLibs
> >
> > Thank you Mike.
> >
> > 1) I like the idea to combine multiple OpensslLibIA32/X64.inf into one
> OpensslLibAccel.inf.
> > Also the cleanup looks good to me.
> >
> > 2) I also like the summary in readme in
> >
> https://github.com/mdkinney/edk2/tree/CryptoPkg_RemoveEcPcd_Merge
> OptimizedOpensslLibs/CryptoPkg
> > I notice some algorithms are listed Y(Deprecated) but N(Don't Use), such
> as Tdes, Arc4, Aes.Ecb*.
> > But I don’t see the use case for those algorithms and I suggest a
> Y(Deprecated) have Y(Don't Use).
>
> Good catch. I will fix.
>
> >
> > 3) About PcdOpensslEcEnabled
> > I notice it is used in existing code -
> >
> https://github.com/mdkinney/edk2/blob/CryptoPkg_RemoveEcPcd_Merge
> OptimizedOpensslLibs/CryptoPkg/Library/TlsLib/TlsConfig.c#L11
> > 39
> > Is this right way?
>
> This was added since I started this work and was added back in by rebase. I
> will fix.
> We can just remove the check for the PCD. If the OpensslLib instance does
> not include
> SSL services, then the Null SSL services are present and the call to SSL_ctrl()
> will
> return 0 which will force TlsSetEcCurve() to return EFI_UNSUPPORTED. It
> will also
> ASSERT() informing the developer that a call to a service that depends on
> SSL was made
> without SSL services available.
>
> long
> SSL_ctrl (
> SSL *ssl,
> int cmd,
> long larg,
> void *parg
> )
> {
> ASSERT (FALSE);
> return 0;
> }
>
> Likewise, if the OpensslLib instance does not support EC services, then the
> Null
> EC services will be included and the call to EC_KEY_new_by_curve_name()
> will
> return NULL which will force TlsSetEcCurve() to return EFI_UNSUPPORTED. It
> will also
> ASSERT() informing the developer that a call to a service that depends on EC
> was made
> without EC services available.
>
> EC_KEY *
> EC_KEY_new_by_curve_name (
> int nid
> )
> {
> ASSERT (FALSE);
> return NULL;
> }
>
> >
> > Thank you
> > Yao, Jiewen
> >
> > > -----Original Message-----
> > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > Sent: Tuesday, October 11, 2022 11:04 PM
> > > To: devel@edk2.groups.io
> > > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> > > <jian.j.wang@intel.com>; Lu, Xiaoyu1 <xiaoyu1.lu@intel.com>; Jiang,
> > > Guomin <guomin.jiang@intel.com>; Zurcher, Christopher
> > > <christopher.zurcher@microsoft.com>; Rebecca Cran
> > > <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> > > Subject: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt
> > > OpensslLibs
> > >
> > > The recent addition of the Ecliptic Curve (EC) feature and the
> performance
> > > optimization features increased the complexity for platforms to
> integrate
> > > and enable these features. This series simplifies the platform
> configuration
> > > as much as possible and improves the ability to manage the the size
> impact
> > > of cryptographic services in each FW phase. A Readme.md is also added
> > > that
> > > provides an overview of the CryptoPkg design and features along with
> > > platform
> > > integration recommendations.
> > >
> > > This series also addresses private library class declarations missing from
> > > CryptoPkg.dec and library instances not producing all the APIs defined
> > > by the library classes. A review of the CryptoPkg EDK II meta data files
> > > identified
> > > a number of additional cleanups. The CryptoPkg.dsc file was also
> updated to
> > > improve CI coverage for future CryptoPkg changes and identified some
> > > unit test bug fixes.
> > >
> > > PR: https://github.com/tianocore/edk2/pull/3443
> > > Branch:
> > >
> https://github.com/mdkinney/edk2/tree/CryptoPkg_RemoveEcPcd_Merge
> > > OptimizedOpensslLibs
> > > Readme:
> > >
> https://github.com/mdkinney/edk2/blob/CryptoPkg_RemoveEcPcd_Merge
> > > OptimizedOpensslLibs/CryptoPkg/Readme.md
> > >
> > > Change Summary
> > > ==============
> > > * Document disabled/deprecated cryptographic services
> > > * Add missing UNI files in BaseCryptLib
> > > * Update BaseCryptLib internal functions to be STATIC and remove
> EFIAPI
> > > * Add GLOBAL_REMOVE_IF_UNREFERENCED to BaseCryptLib global
> > > variables
> > > * Fix BaseCryptLib unit tests
> > > * Cleanup BaseCryptLib and TlsLib INF files and
> > > * Move SysCall/inet_pton.c from BaseCryptLib to TlsLib that uses it.
> > > * Merge 4 performance optimized INFs into OpensslLib*Accel.inf
> > > * Remove use of PcdOpensslEcEnabled and use OpensslLibFull*.inf
> instead
> > > * Add OpensslLib and IntrinsicLib to CryptoPkg.dec as private library
> classes
> > > * Update all OpensslLib instances to always produce all APIs in
> OpensslLib
> > > class
> > > * Move PrintLib dependency from OpensslLib INF files to BaseCryptLib
> INF
> > > files
> > > * Update CryptoPkg.dsc files to provide full CI test coverage across all
> the
> > > supported combinations of OpensslLib, BaseCryptLib, and TlsLib
> instances.
> > > * Remove PACKAGE profile from CryptoPkg.dsc and add
> > > TARGET_UNIT_TESTS
> > > profile. Adding TARGET_UNIT_TESTS profile is required to prevent a
> few
> > > unit
> > > test artifacts being included in non unit test builds of components.
> > > * Add CryptoPkg Readme.md with overview and platform integration
> > > details.
> > > * Update host-based unit tests to always use OpensslLibFull.inf and add
> > > unit
> > > test coverage for OpensslLibFullAccel.inf.
> > > * Add Readme.md with CryptoPkg overview and platform integration
> > > documentation
> > >
> > > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > Cc: Xiaoyu Lu <xiaoyu1.lu@intel.com>
> > > Cc: Guomin Jiang <guomin.jiang@intel.com>
> > > Cc: Christopher Zurcher <christopher.zurcher@microsoft.com>
> > > Cc: Rebecca Cran <quic_rcran@quicinc.com>
> > > Cc: Ard Biesheuvel <ardb@kernel.org>
> > > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> > >
> > > Michael D Kinney (12):
> > > CryptoPkg: Document and disable deprecated crypto services
> > > CryptoPkg/Library/BaseCryptLib: Add missing UNI file and fix format
> > > CryptoPkg/Library/BaseCryptLib: Update internal functions/variables
> > > CryptoPkg/Test/UnitTest/Library/BaseCryptLib: Unit test fixes
> > > CryptoPkg/Library: Cleanup BaseCryptLib and TlsLib
> > > CryptoPkg/Library/OpensslLib: Combine all performance optimized
> INFs
> > > CryptoPkg/Library/OpensslLib: Produce consistent set of APIs
> > > CryptoPkg/Library/OpensslLib: Remove PrintLib from INF files
> > > CryptoPkg: Remove PcdOpensslEcEnabled from CryptoPkg.dec
> > > CryptoPkg: Update DSC to improve CI test coverage
> > > CryptoPkg: Fixed host-based unit tests
> > > CryptoPkg: Add Readme.md
> > >
> > > CryptoPkg/CryptoPkg.ci.yaml | 11 +-
> > > CryptoPkg/CryptoPkg.dec | 42 +-
> > > CryptoPkg/CryptoPkg.dsc | 460 +++++++++---
> > > .../Pcd/PcdCryptoServiceFamilyEnable.h | 122 +--
> > > .../Library/BaseCryptLib/BaseCryptLib.inf | 10 +-
> > > .../Library/BaseCryptLib/BaseCryptLib.uni | 2 -
> > > .../Library/BaseCryptLib/Hmac/CryptHmac.c | 7 +
> > > .../Library/BaseCryptLib/Kdf/CryptHkdf.c | 5 +-
> > > .../Library/BaseCryptLib/PeiCryptLib.inf | 8 +-
> > > .../Library/BaseCryptLib/PeiCryptLib.uni | 2 -
> > > .../BaseCryptLib/Pk/CryptAuthenticode.c | 2 +-
> > > .../BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c | 3 +-
> > > .../BaseCryptLib/Pk/CryptPkcs7VerifyEku.c | 3 +
> > > CryptoPkg/Library/BaseCryptLib/Pk/CryptTs.c | 44 +-
> > > .../Library/BaseCryptLib/RuntimeCryptLib.inf | 9 +-
> > > .../Library/BaseCryptLib/RuntimeCryptLib.uni | 2 -
> > > .../Library/BaseCryptLib/SecCryptLib.inf | 13 +-
> > > .../{SmmCryptLib.uni => SecCryptLib.uni} | 11 +-
> > > .../Library/BaseCryptLib/SmmCryptLib.inf | 12 -
> > > .../Library/BaseCryptLib/SmmCryptLib.uni | 2 -
> > > .../BaseCryptLib/UnitTestHostBaseCryptLib.inf | 22 +-
> > > .../Library/Include/openssl/opensslconf.h | 328 +++++++-
> > > .../Include/openssl/opensslconf_generated.h | 333 ---------
> > > CryptoPkg/Library/OpensslLib/EcSm2Null.c | 291 ++++++++
> > > CryptoPkg/Library/OpensslLib/OpensslLib.inf | 133 ++--
> > > CryptoPkg/Library/OpensslLib/OpensslLib.uni | 10 +-
> > > ...nsslLibIa32Gcc.inf => OpensslLibAccel.inf} | 189 +++--
> > > .../Library/OpensslLib/OpensslLibAccel.uni | 14 +
> > > .../OpensslLib/OpensslLibConstructor.c | 6 +-
> > > .../Library/OpensslLib/OpensslLibCrypto.inf | 185 +++--
> > > .../Library/OpensslLib/OpensslLibCrypto.uni | 11 +-
> > > .../{OpensslLib.inf => OpensslLibFull.inf} | 143 ++--
> > > .../{OpensslLib.uni => OpensslLibFull.uni} | 10 +-
> > > ...sslLibIa32.inf => OpensslLibFullAccel.inf} | 192 +++--
> > > .../OpensslLib/OpensslLibFullAccel.uni | 14 +
> > > .../Library/OpensslLib/OpensslLibX64.inf | 706 ------------------
> > > .../Library/OpensslLib/OpensslLibX64Gcc.inf | 706 ------------------
> > > CryptoPkg/Library/OpensslLib/SslNull.c | 405 ++++++++++
> > > .../SysCall/inet_pton.c | 0
> > > CryptoPkg/Library/TlsLib/TlsConfig.c | 2 +-
> > > CryptoPkg/Library/TlsLib/TlsLib.inf | 12 +-
> > > CryptoPkg/Private/Library/IntrinsicLib.h | 16 +
> > > CryptoPkg/Private/Library/OpensslLib.h | 14 +
> > > CryptoPkg/Readme.md | 498 ++++++++++++
> > > CryptoPkg/Test/CryptoPkgHostUnitTest.dsc | 17 +-
> > > .../UnitTest/Library/BaseCryptLib/HmacTests.c | 17 +-
> > > .../UnitTest/Library/BaseCryptLib/TSTests.c | 2 +-
> > > .../TestBaseCryptLibHostAccel.inf | 55 ++
> > > 48 files changed, 2667 insertions(+), 2434 deletions(-)
> > > copy CryptoPkg/Library/BaseCryptLib/{SmmCryptLib.uni =>
> > > SecCryptLib.uni} (74%)
> > > delete mode 100644
> > > CryptoPkg/Library/Include/openssl/opensslconf_generated.h
> > > create mode 100644 CryptoPkg/Library/OpensslLib/EcSm2Null.c
> > > rename CryptoPkg/Library/OpensslLib/{OpensslLibIa32Gcc.inf =>
> > > OpensslLibAccel.inf} (79%)
> > > create mode 100644
> CryptoPkg/Library/OpensslLib/OpensslLibAccel.uni
> > > copy CryptoPkg/Library/OpensslLib/{OpensslLib.inf =>
> OpensslLibFull.inf}
> > > (80%)
> > > copy CryptoPkg/Library/OpensslLib/{OpensslLib.uni =>
> OpensslLibFull.uni}
> > > (56%)
> > > rename CryptoPkg/Library/OpensslLib/{OpensslLibIa32.inf =>
> > > OpensslLibFullAccel.inf} (79%)
> > > create mode 100644
> > > CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.uni
> > > delete mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibX64.inf
> > > delete mode 100644
> CryptoPkg/Library/OpensslLib/OpensslLibX64Gcc.inf
> > > create mode 100644 CryptoPkg/Library/OpensslLib/SslNull.c
> > > rename CryptoPkg/Library/{BaseCryptLib => TlsLib}/SysCall/inet_pton.c
> > > (100%)
> > > create mode 100644 CryptoPkg/Private/Library/IntrinsicLib.h
> > > create mode 100644 CryptoPkg/Private/Library/OpensslLib.h
> > > create mode 100644 CryptoPkg/Readme.md
> > > create mode 100644
> > >
> CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibHostAccel.i
> > > nf
> > >
> > > --
> > > 2.37.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95034): https://edk2.groups.io/g/devel/message/95034
Mute This Topic: https://groups.io/mt/94260718/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Jiewen,
The BKM is to just call the EC services from the layers above OpensslLib.
The EC APIs will always be present. Either real EC service or Null EC service.
This way we can remove all the #ifdef on EC PCD.
Platform developers select the OpensslLib instance with EC (OpensslLibFull.inf) or
without EC (OpensslLib.inf).
Mike
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Tuesday, October 11, 2022 6:37 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1 <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>; Zurcher,
> Christopher <christopher.zurcher@microsoft.com>; Rebecca Cran <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> Subject: RE: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt OpensslLibs
>
> Hi Mike
> For PcdOpensslEcEnabled, I am seeing other usage. For example:
> https://github.com/qizhangz/edk2/blob/EC_Upstream/CryptoPkg/Library/BaseCryptLib/Pem/CryptPem.c#L156
>
> I think we may need BKM on "how to disable EC".
>
> Thank you
> Yao, Jiewen
>
>
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > Sent: Wednesday, October 12, 2022 9:25 AM
> > To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; Kinney,
> > Michael D <michael.d.kinney@intel.com>
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1
> > <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
> > Zurcher, Christopher <christopher.zurcher@microsoft.com>; Rebecca Cran
> > <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> > Subject: RE: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt
> > OpensslLibs
> >
> > Hi Jiewen,
> >
> > Comments below.
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: Yao, Jiewen <jiewen.yao@intel.com>
> > > Sent: Tuesday, October 11, 2022 6:09 PM
> > > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > devel@edk2.groups.io
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1
> > <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
> > Zurcher,
> > > Christopher <christopher.zurcher@microsoft.com>; Rebecca Cran
> > <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> > > Subject: RE: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt
> > OpensslLibs
> > >
> > > Thank you Mike.
> > >
> > > 1) I like the idea to combine multiple OpensslLibIA32/X64.inf into one
> > OpensslLibAccel.inf.
> > > Also the cleanup looks good to me.
> > >
> > > 2) I also like the summary in readme in
> > >
> > https://github.com/mdkinney/edk2/tree/CryptoPkg_RemoveEcPcd_Merge
> > OptimizedOpensslLibs/CryptoPkg
> > > I notice some algorithms are listed Y(Deprecated) but N(Don't Use), such
> > as Tdes, Arc4, Aes.Ecb*.
> > > But I don’t see the use case for those algorithms and I suggest a
> > Y(Deprecated) have Y(Don't Use).
> >
> > Good catch. I will fix.
> >
> > >
> > > 3) About PcdOpensslEcEnabled
> > > I notice it is used in existing code -
> > >
> > https://github.com/mdkinney/edk2/blob/CryptoPkg_RemoveEcPcd_Merge
> > OptimizedOpensslLibs/CryptoPkg/Library/TlsLib/TlsConfig.c#L11
> > > 39
> > > Is this right way?
> >
> > This was added since I started this work and was added back in by rebase. I
> > will fix.
> > We can just remove the check for the PCD. If the OpensslLib instance does
> > not include
> > SSL services, then the Null SSL services are present and the call to SSL_ctrl()
> > will
> > return 0 which will force TlsSetEcCurve() to return EFI_UNSUPPORTED. It
> > will also
> > ASSERT() informing the developer that a call to a service that depends on
> > SSL was made
> > without SSL services available.
> >
> > long
> > SSL_ctrl (
> > SSL *ssl,
> > int cmd,
> > long larg,
> > void *parg
> > )
> > {
> > ASSERT (FALSE);
> > return 0;
> > }
> >
> > Likewise, if the OpensslLib instance does not support EC services, then the
> > Null
> > EC services will be included and the call to EC_KEY_new_by_curve_name()
> > will
> > return NULL which will force TlsSetEcCurve() to return EFI_UNSUPPORTED. It
> > will also
> > ASSERT() informing the developer that a call to a service that depends on EC
> > was made
> > without EC services available.
> >
> > EC_KEY *
> > EC_KEY_new_by_curve_name (
> > int nid
> > )
> > {
> > ASSERT (FALSE);
> > return NULL;
> > }
> >
> > >
> > > Thank you
> > > Yao, Jiewen
> > >
> > > > -----Original Message-----
> > > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > > Sent: Tuesday, October 11, 2022 11:04 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> > > > <jian.j.wang@intel.com>; Lu, Xiaoyu1 <xiaoyu1.lu@intel.com>; Jiang,
> > > > Guomin <guomin.jiang@intel.com>; Zurcher, Christopher
> > > > <christopher.zurcher@microsoft.com>; Rebecca Cran
> > > > <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> > > > Subject: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt
> > > > OpensslLibs
> > > >
> > > > The recent addition of the Ecliptic Curve (EC) feature and the
> > performance
> > > > optimization features increased the complexity for platforms to
> > integrate
> > > > and enable these features. This series simplifies the platform
> > configuration
> > > > as much as possible and improves the ability to manage the the size
> > impact
> > > > of cryptographic services in each FW phase. A Readme.md is also added
> > > > that
> > > > provides an overview of the CryptoPkg design and features along with
> > > > platform
> > > > integration recommendations.
> > > >
> > > > This series also addresses private library class declarations missing from
> > > > CryptoPkg.dec and library instances not producing all the APIs defined
> > > > by the library classes. A review of the CryptoPkg EDK II meta data files
> > > > identified
> > > > a number of additional cleanups. The CryptoPkg.dsc file was also
> > updated to
> > > > improve CI coverage for future CryptoPkg changes and identified some
> > > > unit test bug fixes.
> > > >
> > > > PR: https://github.com/tianocore/edk2/pull/3443
> > > > Branch:
> > > >
> > https://github.com/mdkinney/edk2/tree/CryptoPkg_RemoveEcPcd_Merge
> > > > OptimizedOpensslLibs
> > > > Readme:
> > > >
> > https://github.com/mdkinney/edk2/blob/CryptoPkg_RemoveEcPcd_Merge
> > > > OptimizedOpensslLibs/CryptoPkg/Readme.md
> > > >
> > > > Change Summary
> > > > ==============
> > > > * Document disabled/deprecated cryptographic services
> > > > * Add missing UNI files in BaseCryptLib
> > > > * Update BaseCryptLib internal functions to be STATIC and remove
> > EFIAPI
> > > > * Add GLOBAL_REMOVE_IF_UNREFERENCED to BaseCryptLib global
> > > > variables
> > > > * Fix BaseCryptLib unit tests
> > > > * Cleanup BaseCryptLib and TlsLib INF files and
> > > > * Move SysCall/inet_pton.c from BaseCryptLib to TlsLib that uses it.
> > > > * Merge 4 performance optimized INFs into OpensslLib*Accel.inf
> > > > * Remove use of PcdOpensslEcEnabled and use OpensslLibFull*.inf
> > instead
> > > > * Add OpensslLib and IntrinsicLib to CryptoPkg.dec as private library
> > classes
> > > > * Update all OpensslLib instances to always produce all APIs in
> > OpensslLib
> > > > class
> > > > * Move PrintLib dependency from OpensslLib INF files to BaseCryptLib
> > INF
> > > > files
> > > > * Update CryptoPkg.dsc files to provide full CI test coverage across all
> > the
> > > > supported combinations of OpensslLib, BaseCryptLib, and TlsLib
> > instances.
> > > > * Remove PACKAGE profile from CryptoPkg.dsc and add
> > > > TARGET_UNIT_TESTS
> > > > profile. Adding TARGET_UNIT_TESTS profile is required to prevent a
> > few
> > > > unit
> > > > test artifacts being included in non unit test builds of components.
> > > > * Add CryptoPkg Readme.md with overview and platform integration
> > > > details.
> > > > * Update host-based unit tests to always use OpensslLibFull.inf and add
> > > > unit
> > > > test coverage for OpensslLibFullAccel.inf.
> > > > * Add Readme.md with CryptoPkg overview and platform integration
> > > > documentation
> > > >
> > > > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > > Cc: Xiaoyu Lu <xiaoyu1.lu@intel.com>
> > > > Cc: Guomin Jiang <guomin.jiang@intel.com>
> > > > Cc: Christopher Zurcher <christopher.zurcher@microsoft.com>
> > > > Cc: Rebecca Cran <quic_rcran@quicinc.com>
> > > > Cc: Ard Biesheuvel <ardb@kernel.org>
> > > > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> > > >
> > > > Michael D Kinney (12):
> > > > CryptoPkg: Document and disable deprecated crypto services
> > > > CryptoPkg/Library/BaseCryptLib: Add missing UNI file and fix format
> > > > CryptoPkg/Library/BaseCryptLib: Update internal functions/variables
> > > > CryptoPkg/Test/UnitTest/Library/BaseCryptLib: Unit test fixes
> > > > CryptoPkg/Library: Cleanup BaseCryptLib and TlsLib
> > > > CryptoPkg/Library/OpensslLib: Combine all performance optimized
> > INFs
> > > > CryptoPkg/Library/OpensslLib: Produce consistent set of APIs
> > > > CryptoPkg/Library/OpensslLib: Remove PrintLib from INF files
> > > > CryptoPkg: Remove PcdOpensslEcEnabled from CryptoPkg.dec
> > > > CryptoPkg: Update DSC to improve CI test coverage
> > > > CryptoPkg: Fixed host-based unit tests
> > > > CryptoPkg: Add Readme.md
> > > >
> > > > CryptoPkg/CryptoPkg.ci.yaml | 11 +-
> > > > CryptoPkg/CryptoPkg.dec | 42 +-
> > > > CryptoPkg/CryptoPkg.dsc | 460 +++++++++---
> > > > .../Pcd/PcdCryptoServiceFamilyEnable.h | 122 +--
> > > > .../Library/BaseCryptLib/BaseCryptLib.inf | 10 +-
> > > > .../Library/BaseCryptLib/BaseCryptLib.uni | 2 -
> > > > .../Library/BaseCryptLib/Hmac/CryptHmac.c | 7 +
> > > > .../Library/BaseCryptLib/Kdf/CryptHkdf.c | 5 +-
> > > > .../Library/BaseCryptLib/PeiCryptLib.inf | 8 +-
> > > > .../Library/BaseCryptLib/PeiCryptLib.uni | 2 -
> > > > .../BaseCryptLib/Pk/CryptAuthenticode.c | 2 +-
> > > > .../BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c | 3 +-
> > > > .../BaseCryptLib/Pk/CryptPkcs7VerifyEku.c | 3 +
> > > > CryptoPkg/Library/BaseCryptLib/Pk/CryptTs.c | 44 +-
> > > > .../Library/BaseCryptLib/RuntimeCryptLib.inf | 9 +-
> > > > .../Library/BaseCryptLib/RuntimeCryptLib.uni | 2 -
> > > > .../Library/BaseCryptLib/SecCryptLib.inf | 13 +-
> > > > .../{SmmCryptLib.uni => SecCryptLib.uni} | 11 +-
> > > > .../Library/BaseCryptLib/SmmCryptLib.inf | 12 -
> > > > .../Library/BaseCryptLib/SmmCryptLib.uni | 2 -
> > > > .../BaseCryptLib/UnitTestHostBaseCryptLib.inf | 22 +-
> > > > .../Library/Include/openssl/opensslconf.h | 328 +++++++-
> > > > .../Include/openssl/opensslconf_generated.h | 333 ---------
> > > > CryptoPkg/Library/OpensslLib/EcSm2Null.c | 291 ++++++++
> > > > CryptoPkg/Library/OpensslLib/OpensslLib.inf | 133 ++--
> > > > CryptoPkg/Library/OpensslLib/OpensslLib.uni | 10 +-
> > > > ...nsslLibIa32Gcc.inf => OpensslLibAccel.inf} | 189 +++--
> > > > .../Library/OpensslLib/OpensslLibAccel.uni | 14 +
> > > > .../OpensslLib/OpensslLibConstructor.c | 6 +-
> > > > .../Library/OpensslLib/OpensslLibCrypto.inf | 185 +++--
> > > > .../Library/OpensslLib/OpensslLibCrypto.uni | 11 +-
> > > > .../{OpensslLib.inf => OpensslLibFull.inf} | 143 ++--
> > > > .../{OpensslLib.uni => OpensslLibFull.uni} | 10 +-
> > > > ...sslLibIa32.inf => OpensslLibFullAccel.inf} | 192 +++--
> > > > .../OpensslLib/OpensslLibFullAccel.uni | 14 +
> > > > .../Library/OpensslLib/OpensslLibX64.inf | 706 ------------------
> > > > .../Library/OpensslLib/OpensslLibX64Gcc.inf | 706 ------------------
> > > > CryptoPkg/Library/OpensslLib/SslNull.c | 405 ++++++++++
> > > > .../SysCall/inet_pton.c | 0
> > > > CryptoPkg/Library/TlsLib/TlsConfig.c | 2 +-
> > > > CryptoPkg/Library/TlsLib/TlsLib.inf | 12 +-
> > > > CryptoPkg/Private/Library/IntrinsicLib.h | 16 +
> > > > CryptoPkg/Private/Library/OpensslLib.h | 14 +
> > > > CryptoPkg/Readme.md | 498 ++++++++++++
> > > > CryptoPkg/Test/CryptoPkgHostUnitTest.dsc | 17 +-
> > > > .../UnitTest/Library/BaseCryptLib/HmacTests.c | 17 +-
> > > > .../UnitTest/Library/BaseCryptLib/TSTests.c | 2 +-
> > > > .../TestBaseCryptLibHostAccel.inf | 55 ++
> > > > 48 files changed, 2667 insertions(+), 2434 deletions(-)
> > > > copy CryptoPkg/Library/BaseCryptLib/{SmmCryptLib.uni =>
> > > > SecCryptLib.uni} (74%)
> > > > delete mode 100644
> > > > CryptoPkg/Library/Include/openssl/opensslconf_generated.h
> > > > create mode 100644 CryptoPkg/Library/OpensslLib/EcSm2Null.c
> > > > rename CryptoPkg/Library/OpensslLib/{OpensslLibIa32Gcc.inf =>
> > > > OpensslLibAccel.inf} (79%)
> > > > create mode 100644
> > CryptoPkg/Library/OpensslLib/OpensslLibAccel.uni
> > > > copy CryptoPkg/Library/OpensslLib/{OpensslLib.inf =>
> > OpensslLibFull.inf}
> > > > (80%)
> > > > copy CryptoPkg/Library/OpensslLib/{OpensslLib.uni =>
> > OpensslLibFull.uni}
> > > > (56%)
> > > > rename CryptoPkg/Library/OpensslLib/{OpensslLibIa32.inf =>
> > > > OpensslLibFullAccel.inf} (79%)
> > > > create mode 100644
> > > > CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.uni
> > > > delete mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibX64.inf
> > > > delete mode 100644
> > CryptoPkg/Library/OpensslLib/OpensslLibX64Gcc.inf
> > > > create mode 100644 CryptoPkg/Library/OpensslLib/SslNull.c
> > > > rename CryptoPkg/Library/{BaseCryptLib => TlsLib}/SysCall/inet_pton.c
> > > > (100%)
> > > > create mode 100644 CryptoPkg/Private/Library/IntrinsicLib.h
> > > > create mode 100644 CryptoPkg/Private/Library/OpensslLib.h
> > > > create mode 100644 CryptoPkg/Readme.md
> > > > create mode 100644
> > > >
> > CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibHostAccel.i
> > > > nf
> > > >
> > > > --
> > > > 2.37.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95035): https://edk2.groups.io/g/devel/message/95035
Mute This Topic: https://groups.io/mt/94260718/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
OK. That means we need manual write a wrapper for all EC APIs in Openssl Lib for internal usage.
More precisely, a wrapper for the delta between OpensslLib and OpensslLibFull, right?
There will be size difference between those two solutions, because the code other than EC will be include in the function.
Why not use NO_EC macro from openssl directly?
Thank you
Yao Jiewen
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Wednesday, October 12, 2022 9:55 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1
> <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
> Zurcher, Christopher <christopher.zurcher@microsoft.com>; Rebecca Cran
> <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> Subject: RE: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt
> OpensslLibs
>
> Jiewen,
>
> The BKM is to just call the EC services from the layers above OpensslLib.
> The EC APIs will always be present. Either real EC service or Null EC service.
> This way we can remove all the #ifdef on EC PCD.
>
> Platform developers select the OpensslLib instance with EC
> (OpensslLibFull.inf) or
> without EC (OpensslLib.inf).
>
> Mike
>
> > -----Original Message-----
> > From: Yao, Jiewen <jiewen.yao@intel.com>
> > Sent: Tuesday, October 11, 2022 6:37 PM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1
> <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
> Zurcher,
> > Christopher <christopher.zurcher@microsoft.com>; Rebecca Cran
> <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> > Subject: RE: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt
> OpensslLibs
> >
> > Hi Mike
> > For PcdOpensslEcEnabled, I am seeing other usage. For example:
> >
> https://github.com/qizhangz/edk2/blob/EC_Upstream/CryptoPkg/Library/
> BaseCryptLib/Pem/CryptPem.c#L156
> >
> > I think we may need BKM on "how to disable EC".
> >
> > Thank you
> > Yao, Jiewen
> >
> >
> > > -----Original Message-----
> > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > Sent: Wednesday, October 12, 2022 9:25 AM
> > > To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; Kinney,
> > > Michael D <michael.d.kinney@intel.com>
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1
> > > <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
> > > Zurcher, Christopher <christopher.zurcher@microsoft.com>; Rebecca
> Cran
> > > <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> > > Subject: RE: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf
> opt
> > > OpensslLibs
> > >
> > > Hi Jiewen,
> > >
> > > Comments below.
> > >
> > > Mike
> > >
> > > > -----Original Message-----
> > > > From: Yao, Jiewen <jiewen.yao@intel.com>
> > > > Sent: Tuesday, October 11, 2022 6:09 PM
> > > > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > > devel@edk2.groups.io
> > > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1
> > > <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
> > > Zurcher,
> > > > Christopher <christopher.zurcher@microsoft.com>; Rebecca Cran
> > > <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> > > > Subject: RE: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf
> opt
> > > OpensslLibs
> > > >
> > > > Thank you Mike.
> > > >
> > > > 1) I like the idea to combine multiple OpensslLibIA32/X64.inf into one
> > > OpensslLibAccel.inf.
> > > > Also the cleanup looks good to me.
> > > >
> > > > 2) I also like the summary in readme in
> > > >
> > >
> https://github.com/mdkinney/edk2/tree/CryptoPkg_RemoveEcPcd_Merge
> > > OptimizedOpensslLibs/CryptoPkg
> > > > I notice some algorithms are listed Y(Deprecated) but N(Don't Use),
> such
> > > as Tdes, Arc4, Aes.Ecb*.
> > > > But I don’t see the use case for those algorithms and I suggest a
> > > Y(Deprecated) have Y(Don't Use).
> > >
> > > Good catch. I will fix.
> > >
> > > >
> > > > 3) About PcdOpensslEcEnabled
> > > > I notice it is used in existing code -
> > > >
> > >
> https://github.com/mdkinney/edk2/blob/CryptoPkg_RemoveEcPcd_Merge
> > > OptimizedOpensslLibs/CryptoPkg/Library/TlsLib/TlsConfig.c#L11
> > > > 39
> > > > Is this right way?
> > >
> > > This was added since I started this work and was added back in by
> rebase. I
> > > will fix.
> > > We can just remove the check for the PCD. If the OpensslLib instance
> does
> > > not include
> > > SSL services, then the Null SSL services are present and the call to
> SSL_ctrl()
> > > will
> > > return 0 which will force TlsSetEcCurve() to return EFI_UNSUPPORTED.
> It
> > > will also
> > > ASSERT() informing the developer that a call to a service that depends
> on
> > > SSL was made
> > > without SSL services available.
> > >
> > > long
> > > SSL_ctrl (
> > > SSL *ssl,
> > > int cmd,
> > > long larg,
> > > void *parg
> > > )
> > > {
> > > ASSERT (FALSE);
> > > return 0;
> > > }
> > >
> > > Likewise, if the OpensslLib instance does not support EC services, then
> the
> > > Null
> > > EC services will be included and the call to
> EC_KEY_new_by_curve_name()
> > > will
> > > return NULL which will force TlsSetEcCurve() to return
> EFI_UNSUPPORTED. It
> > > will also
> > > ASSERT() informing the developer that a call to a service that depends
> on EC
> > > was made
> > > without EC services available.
> > >
> > > EC_KEY *
> > > EC_KEY_new_by_curve_name (
> > > int nid
> > > )
> > > {
> > > ASSERT (FALSE);
> > > return NULL;
> > > }
> > >
> > > >
> > > > Thank you
> > > > Yao, Jiewen
> > > >
> > > > > -----Original Message-----
> > > > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > > > Sent: Tuesday, October 11, 2022 11:04 PM
> > > > > To: devel@edk2.groups.io
> > > > > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> > > > > <jian.j.wang@intel.com>; Lu, Xiaoyu1 <xiaoyu1.lu@intel.com>; Jiang,
> > > > > Guomin <guomin.jiang@intel.com>; Zurcher, Christopher
> > > > > <christopher.zurcher@microsoft.com>; Rebecca Cran
> > > > > <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> > > > > Subject: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf
> opt
> > > > > OpensslLibs
> > > > >
> > > > > The recent addition of the Ecliptic Curve (EC) feature and the
> > > performance
> > > > > optimization features increased the complexity for platforms to
> > > integrate
> > > > > and enable these features. This series simplifies the platform
> > > configuration
> > > > > as much as possible and improves the ability to manage the the size
> > > impact
> > > > > of cryptographic services in each FW phase. A Readme.md is also
> added
> > > > > that
> > > > > provides an overview of the CryptoPkg design and features along
> with
> > > > > platform
> > > > > integration recommendations.
> > > > >
> > > > > This series also addresses private library class declarations missing
> from
> > > > > CryptoPkg.dec and library instances not producing all the APIs
> defined
> > > > > by the library classes. A review of the CryptoPkg EDK II meta data
> files
> > > > > identified
> > > > > a number of additional cleanups. The CryptoPkg.dsc file was also
> > > updated to
> > > > > improve CI coverage for future CryptoPkg changes and identified
> some
> > > > > unit test bug fixes.
> > > > >
> > > > > PR: https://github.com/tianocore/edk2/pull/3443
> > > > > Branch:
> > > > >
> > >
> https://github.com/mdkinney/edk2/tree/CryptoPkg_RemoveEcPcd_Merge
> > > > > OptimizedOpensslLibs
> > > > > Readme:
> > > > >
> > >
> https://github.com/mdkinney/edk2/blob/CryptoPkg_RemoveEcPcd_Merge
> > > > > OptimizedOpensslLibs/CryptoPkg/Readme.md
> > > > >
> > > > > Change Summary
> > > > > ==============
> > > > > * Document disabled/deprecated cryptographic services
> > > > > * Add missing UNI files in BaseCryptLib
> > > > > * Update BaseCryptLib internal functions to be STATIC and remove
> > > EFIAPI
> > > > > * Add GLOBAL_REMOVE_IF_UNREFERENCED to BaseCryptLib global
> > > > > variables
> > > > > * Fix BaseCryptLib unit tests
> > > > > * Cleanup BaseCryptLib and TlsLib INF files and
> > > > > * Move SysCall/inet_pton.c from BaseCryptLib to TlsLib that uses it.
> > > > > * Merge 4 performance optimized INFs into OpensslLib*Accel.inf
> > > > > * Remove use of PcdOpensslEcEnabled and use OpensslLibFull*.inf
> > > instead
> > > > > * Add OpensslLib and IntrinsicLib to CryptoPkg.dec as private library
> > > classes
> > > > > * Update all OpensslLib instances to always produce all APIs in
> > > OpensslLib
> > > > > class
> > > > > * Move PrintLib dependency from OpensslLib INF files to
> BaseCryptLib
> > > INF
> > > > > files
> > > > > * Update CryptoPkg.dsc files to provide full CI test coverage across
> all
> > > the
> > > > > supported combinations of OpensslLib, BaseCryptLib, and TlsLib
> > > instances.
> > > > > * Remove PACKAGE profile from CryptoPkg.dsc and add
> > > > > TARGET_UNIT_TESTS
> > > > > profile. Adding TARGET_UNIT_TESTS profile is required to prevent
> a
> > > few
> > > > > unit
> > > > > test artifacts being included in non unit test builds of components.
> > > > > * Add CryptoPkg Readme.md with overview and platform
> integration
> > > > > details.
> > > > > * Update host-based unit tests to always use OpensslLibFull.inf and
> add
> > > > > unit
> > > > > test coverage for OpensslLibFullAccel.inf.
> > > > > * Add Readme.md with CryptoPkg overview and platform
> integration
> > > > > documentation
> > > > >
> > > > > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > > > Cc: Xiaoyu Lu <xiaoyu1.lu@intel.com>
> > > > > Cc: Guomin Jiang <guomin.jiang@intel.com>
> > > > > Cc: Christopher Zurcher <christopher.zurcher@microsoft.com>
> > > > > Cc: Rebecca Cran <quic_rcran@quicinc.com>
> > > > > Cc: Ard Biesheuvel <ardb@kernel.org>
> > > > > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> > > > >
> > > > > Michael D Kinney (12):
> > > > > CryptoPkg: Document and disable deprecated crypto services
> > > > > CryptoPkg/Library/BaseCryptLib: Add missing UNI file and fix
> format
> > > > > CryptoPkg/Library/BaseCryptLib: Update internal
> functions/variables
> > > > > CryptoPkg/Test/UnitTest/Library/BaseCryptLib: Unit test fixes
> > > > > CryptoPkg/Library: Cleanup BaseCryptLib and TlsLib
> > > > > CryptoPkg/Library/OpensslLib: Combine all performance optimized
> > > INFs
> > > > > CryptoPkg/Library/OpensslLib: Produce consistent set of APIs
> > > > > CryptoPkg/Library/OpensslLib: Remove PrintLib from INF files
> > > > > CryptoPkg: Remove PcdOpensslEcEnabled from CryptoPkg.dec
> > > > > CryptoPkg: Update DSC to improve CI test coverage
> > > > > CryptoPkg: Fixed host-based unit tests
> > > > > CryptoPkg: Add Readme.md
> > > > >
> > > > > CryptoPkg/CryptoPkg.ci.yaml | 11 +-
> > > > > CryptoPkg/CryptoPkg.dec | 42 +-
> > > > > CryptoPkg/CryptoPkg.dsc | 460 +++++++++---
> > > > > .../Pcd/PcdCryptoServiceFamilyEnable.h | 122 +--
> > > > > .../Library/BaseCryptLib/BaseCryptLib.inf | 10 +-
> > > > > .../Library/BaseCryptLib/BaseCryptLib.uni | 2 -
> > > > > .../Library/BaseCryptLib/Hmac/CryptHmac.c | 7 +
> > > > > .../Library/BaseCryptLib/Kdf/CryptHkdf.c | 5 +-
> > > > > .../Library/BaseCryptLib/PeiCryptLib.inf | 8 +-
> > > > > .../Library/BaseCryptLib/PeiCryptLib.uni | 2 -
> > > > > .../BaseCryptLib/Pk/CryptAuthenticode.c | 2 +-
> > > > > .../BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c | 3 +-
> > > > > .../BaseCryptLib/Pk/CryptPkcs7VerifyEku.c | 3 +
> > > > > CryptoPkg/Library/BaseCryptLib/Pk/CryptTs.c | 44 +-
> > > > > .../Library/BaseCryptLib/RuntimeCryptLib.inf | 9 +-
> > > > > .../Library/BaseCryptLib/RuntimeCryptLib.uni | 2 -
> > > > > .../Library/BaseCryptLib/SecCryptLib.inf | 13 +-
> > > > > .../{SmmCryptLib.uni => SecCryptLib.uni} | 11 +-
> > > > > .../Library/BaseCryptLib/SmmCryptLib.inf | 12 -
> > > > > .../Library/BaseCryptLib/SmmCryptLib.uni | 2 -
> > > > > .../BaseCryptLib/UnitTestHostBaseCryptLib.inf | 22 +-
> > > > > .../Library/Include/openssl/opensslconf.h | 328 +++++++-
> > > > > .../Include/openssl/opensslconf_generated.h | 333 ---------
> > > > > CryptoPkg/Library/OpensslLib/EcSm2Null.c | 291 ++++++++
> > > > > CryptoPkg/Library/OpensslLib/OpensslLib.inf | 133 ++--
> > > > > CryptoPkg/Library/OpensslLib/OpensslLib.uni | 10 +-
> > > > > ...nsslLibIa32Gcc.inf => OpensslLibAccel.inf} | 189 +++--
> > > > > .../Library/OpensslLib/OpensslLibAccel.uni | 14 +
> > > > > .../OpensslLib/OpensslLibConstructor.c | 6 +-
> > > > > .../Library/OpensslLib/OpensslLibCrypto.inf | 185 +++--
> > > > > .../Library/OpensslLib/OpensslLibCrypto.uni | 11 +-
> > > > > .../{OpensslLib.inf => OpensslLibFull.inf} | 143 ++--
> > > > > .../{OpensslLib.uni => OpensslLibFull.uni} | 10 +-
> > > > > ...sslLibIa32.inf => OpensslLibFullAccel.inf} | 192 +++--
> > > > > .../OpensslLib/OpensslLibFullAccel.uni | 14 +
> > > > > .../Library/OpensslLib/OpensslLibX64.inf | 706 ------------------
> > > > > .../Library/OpensslLib/OpensslLibX64Gcc.inf | 706 ------------------
> > > > > CryptoPkg/Library/OpensslLib/SslNull.c | 405 ++++++++++
> > > > > .../SysCall/inet_pton.c | 0
> > > > > CryptoPkg/Library/TlsLib/TlsConfig.c | 2 +-
> > > > > CryptoPkg/Library/TlsLib/TlsLib.inf | 12 +-
> > > > > CryptoPkg/Private/Library/IntrinsicLib.h | 16 +
> > > > > CryptoPkg/Private/Library/OpensslLib.h | 14 +
> > > > > CryptoPkg/Readme.md | 498 ++++++++++++
> > > > > CryptoPkg/Test/CryptoPkgHostUnitTest.dsc | 17 +-
> > > > > .../UnitTest/Library/BaseCryptLib/HmacTests.c | 17 +-
> > > > > .../UnitTest/Library/BaseCryptLib/TSTests.c | 2 +-
> > > > > .../TestBaseCryptLibHostAccel.inf | 55 ++
> > > > > 48 files changed, 2667 insertions(+), 2434 deletions(-)
> > > > > copy CryptoPkg/Library/BaseCryptLib/{SmmCryptLib.uni =>
> > > > > SecCryptLib.uni} (74%)
> > > > > delete mode 100644
> > > > > CryptoPkg/Library/Include/openssl/opensslconf_generated.h
> > > > > create mode 100644 CryptoPkg/Library/OpensslLib/EcSm2Null.c
> > > > > rename CryptoPkg/Library/OpensslLib/{OpensslLibIa32Gcc.inf =>
> > > > > OpensslLibAccel.inf} (79%)
> > > > > create mode 100644
> > > CryptoPkg/Library/OpensslLib/OpensslLibAccel.uni
> > > > > copy CryptoPkg/Library/OpensslLib/{OpensslLib.inf =>
> > > OpensslLibFull.inf}
> > > > > (80%)
> > > > > copy CryptoPkg/Library/OpensslLib/{OpensslLib.uni =>
> > > OpensslLibFull.uni}
> > > > > (56%)
> > > > > rename CryptoPkg/Library/OpensslLib/{OpensslLibIa32.inf =>
> > > > > OpensslLibFullAccel.inf} (79%)
> > > > > create mode 100644
> > > > > CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.uni
> > > > > delete mode 100644
> CryptoPkg/Library/OpensslLib/OpensslLibX64.inf
> > > > > delete mode 100644
> > > CryptoPkg/Library/OpensslLib/OpensslLibX64Gcc.inf
> > > > > create mode 100644 CryptoPkg/Library/OpensslLib/SslNull.c
> > > > > rename CryptoPkg/Library/{BaseCryptLib =>
> TlsLib}/SysCall/inet_pton.c
> > > > > (100%)
> > > > > create mode 100644 CryptoPkg/Private/Library/IntrinsicLib.h
> > > > > create mode 100644 CryptoPkg/Private/Library/OpensslLib.h
> > > > > create mode 100644 CryptoPkg/Readme.md
> > > > > create mode 100644
> > > > >
> > >
> CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibHostAccel.i
> > > > > nf
> > > > >
> > > > > --
> > > > > 2.37.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95036): https://edk2.groups.io/g/devel/message/95036
Mute This Topic: https://groups.io/mt/94260718/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi Jiewen,
comments below.
Mike
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Tuesday, October 11, 2022 7:07 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1 <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>; Zurcher,
> Christopher <christopher.zurcher@microsoft.com>; Rebecca Cran <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> Subject: RE: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt OpensslLibs
>
> OK. That means we need manual write a wrapper for all EC APIs in Openssl Lib for internal usage.
> More precisely, a wrapper for the delta between OpensslLib and OpensslLibFull, right?
Yes. This these are implemented in:
https://github.com/mdkinney/edk2/blob/CryptoPkg_RemoveEcPcd_MergeOptimizedOpensslLibs/CryptoPkg/Library/OpensslLib/SslNull.c
https://github.com/mdkinney/edk2/blob/CryptoPkg_RemoveEcPcd_MergeOptimizedOpensslLibs/CryptoPkg/Library/OpensslLib/EcSm2Null.c
New instances are easy to find because you will get a link failure in CI for missing symbols if
BaseCryptLib or TlsLib uses a new API that is only present in OpensslLibFull.inf. When that occurs
add the Null version of API that always returns a failure condition.
The current implementation before this PR is using OpensslLib class, but the OpensslLib instances
are inconsistent in the APIs produced by the lib instances. This is what caused the introduction
of #if for EC feature into the layers above OpensslLib. This means the EDK II rules for
lib class/lib instance were not followed, and the workaround was to use #if which are also not allowed.
The correct fix is to follow EDK II lib class/lib instance rules. All lib instances of the same
lib class always produce the same APIs, and then all components that use the lib class can depend
on all the services being present without link failures.
>
> There will be size difference between those two solutions, because the code other than EC will be include in the function.
Yes,
>
> Why not use NO_EC macro from openssl directly?
Because that hides all the EC related types required to implement the Null EC APIs
and the EC types required in the layer above OpensslLib that calls EC services.
They will not compile without the EC types being defined.
>
> Thank you
> Yao Jiewen
>
>
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > Sent: Wednesday, October 12, 2022 9:55 AM
> > To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; Kinney,
> > Michael D <michael.d.kinney@intel.com>
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1
> > <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
> > Zurcher, Christopher <christopher.zurcher@microsoft.com>; Rebecca Cran
> > <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> > Subject: RE: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt
> > OpensslLibs
> >
> > Jiewen,
> >
> > The BKM is to just call the EC services from the layers above OpensslLib.
> > The EC APIs will always be present. Either real EC service or Null EC service.
> > This way we can remove all the #ifdef on EC PCD.
> >
> > Platform developers select the OpensslLib instance with EC
> > (OpensslLibFull.inf) or
> > without EC (OpensslLib.inf).
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: Yao, Jiewen <jiewen.yao@intel.com>
> > > Sent: Tuesday, October 11, 2022 6:37 PM
> > > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > devel@edk2.groups.io
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1
> > <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
> > Zurcher,
> > > Christopher <christopher.zurcher@microsoft.com>; Rebecca Cran
> > <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> > > Subject: RE: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt
> > OpensslLibs
> > >
> > > Hi Mike
> > > For PcdOpensslEcEnabled, I am seeing other usage. For example:
> > >
> > https://github.com/qizhangz/edk2/blob/EC_Upstream/CryptoPkg/Library/
> > BaseCryptLib/Pem/CryptPem.c#L156
> > >
> > > I think we may need BKM on "how to disable EC".
> > >
> > > Thank you
> > > Yao, Jiewen
> > >
> > >
> > > > -----Original Message-----
> > > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > > Sent: Wednesday, October 12, 2022 9:25 AM
> > > > To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; Kinney,
> > > > Michael D <michael.d.kinney@intel.com>
> > > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1
> > > > <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
> > > > Zurcher, Christopher <christopher.zurcher@microsoft.com>; Rebecca
> > Cran
> > > > <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> > > > Subject: RE: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf
> > opt
> > > > OpensslLibs
> > > >
> > > > Hi Jiewen,
> > > >
> > > > Comments below.
> > > >
> > > > Mike
> > > >
> > > > > -----Original Message-----
> > > > > From: Yao, Jiewen <jiewen.yao@intel.com>
> > > > > Sent: Tuesday, October 11, 2022 6:09 PM
> > > > > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > > > devel@edk2.groups.io
> > > > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1
> > > > <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
> > > > Zurcher,
> > > > > Christopher <christopher.zurcher@microsoft.com>; Rebecca Cran
> > > > <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> > > > > Subject: RE: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf
> > opt
> > > > OpensslLibs
> > > > >
> > > > > Thank you Mike.
> > > > >
> > > > > 1) I like the idea to combine multiple OpensslLibIA32/X64.inf into one
> > > > OpensslLibAccel.inf.
> > > > > Also the cleanup looks good to me.
> > > > >
> > > > > 2) I also like the summary in readme in
> > > > >
> > > >
> > https://github.com/mdkinney/edk2/tree/CryptoPkg_RemoveEcPcd_Merge
> > > > OptimizedOpensslLibs/CryptoPkg
> > > > > I notice some algorithms are listed Y(Deprecated) but N(Don't Use),
> > such
> > > > as Tdes, Arc4, Aes.Ecb*.
> > > > > But I don’t see the use case for those algorithms and I suggest a
> > > > Y(Deprecated) have Y(Don't Use).
> > > >
> > > > Good catch. I will fix.
> > > >
> > > > >
> > > > > 3) About PcdOpensslEcEnabled
> > > > > I notice it is used in existing code -
> > > > >
> > > >
> > https://github.com/mdkinney/edk2/blob/CryptoPkg_RemoveEcPcd_Merge
> > > > OptimizedOpensslLibs/CryptoPkg/Library/TlsLib/TlsConfig.c#L11
> > > > > 39
> > > > > Is this right way?
> > > >
> > > > This was added since I started this work and was added back in by
> > rebase. I
> > > > will fix.
> > > > We can just remove the check for the PCD. If the OpensslLib instance
> > does
> > > > not include
> > > > SSL services, then the Null SSL services are present and the call to
> > SSL_ctrl()
> > > > will
> > > > return 0 which will force TlsSetEcCurve() to return EFI_UNSUPPORTED.
> > It
> > > > will also
> > > > ASSERT() informing the developer that a call to a service that depends
> > on
> > > > SSL was made
> > > > without SSL services available.
> > > >
> > > > long
> > > > SSL_ctrl (
> > > > SSL *ssl,
> > > > int cmd,
> > > > long larg,
> > > > void *parg
> > > > )
> > > > {
> > > > ASSERT (FALSE);
> > > > return 0;
> > > > }
> > > >
> > > > Likewise, if the OpensslLib instance does not support EC services, then
> > the
> > > > Null
> > > > EC services will be included and the call to
> > EC_KEY_new_by_curve_name()
> > > > will
> > > > return NULL which will force TlsSetEcCurve() to return
> > EFI_UNSUPPORTED. It
> > > > will also
> > > > ASSERT() informing the developer that a call to a service that depends
> > on EC
> > > > was made
> > > > without EC services available.
> > > >
> > > > EC_KEY *
> > > > EC_KEY_new_by_curve_name (
> > > > int nid
> > > > )
> > > > {
> > > > ASSERT (FALSE);
> > > > return NULL;
> > > > }
> > > >
> > > > >
> > > > > Thank you
> > > > > Yao, Jiewen
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > > > > Sent: Tuesday, October 11, 2022 11:04 PM
> > > > > > To: devel@edk2.groups.io
> > > > > > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> > > > > > <jian.j.wang@intel.com>; Lu, Xiaoyu1 <xiaoyu1.lu@intel.com>; Jiang,
> > > > > > Guomin <guomin.jiang@intel.com>; Zurcher, Christopher
> > > > > > <christopher.zurcher@microsoft.com>; Rebecca Cran
> > > > > > <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> > > > > > Subject: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf
> > opt
> > > > > > OpensslLibs
> > > > > >
> > > > > > The recent addition of the Ecliptic Curve (EC) feature and the
> > > > performance
> > > > > > optimization features increased the complexity for platforms to
> > > > integrate
> > > > > > and enable these features. This series simplifies the platform
> > > > configuration
> > > > > > as much as possible and improves the ability to manage the the size
> > > > impact
> > > > > > of cryptographic services in each FW phase. A Readme.md is also
> > added
> > > > > > that
> > > > > > provides an overview of the CryptoPkg design and features along
> > with
> > > > > > platform
> > > > > > integration recommendations.
> > > > > >
> > > > > > This series also addresses private library class declarations missing
> > from
> > > > > > CryptoPkg.dec and library instances not producing all the APIs
> > defined
> > > > > > by the library classes. A review of the CryptoPkg EDK II meta data
> > files
> > > > > > identified
> > > > > > a number of additional cleanups. The CryptoPkg.dsc file was also
> > > > updated to
> > > > > > improve CI coverage for future CryptoPkg changes and identified
> > some
> > > > > > unit test bug fixes.
> > > > > >
> > > > > > PR: https://github.com/tianocore/edk2/pull/3443
> > > > > > Branch:
> > > > > >
> > > >
> > https://github.com/mdkinney/edk2/tree/CryptoPkg_RemoveEcPcd_Merge
> > > > > > OptimizedOpensslLibs
> > > > > > Readme:
> > > > > >
> > > >
> > https://github.com/mdkinney/edk2/blob/CryptoPkg_RemoveEcPcd_Merge
> > > > > > OptimizedOpensslLibs/CryptoPkg/Readme.md
> > > > > >
> > > > > > Change Summary
> > > > > > ==============
> > > > > > * Document disabled/deprecated cryptographic services
> > > > > > * Add missing UNI files in BaseCryptLib
> > > > > > * Update BaseCryptLib internal functions to be STATIC and remove
> > > > EFIAPI
> > > > > > * Add GLOBAL_REMOVE_IF_UNREFERENCED to BaseCryptLib global
> > > > > > variables
> > > > > > * Fix BaseCryptLib unit tests
> > > > > > * Cleanup BaseCryptLib and TlsLib INF files and
> > > > > > * Move SysCall/inet_pton.c from BaseCryptLib to TlsLib that uses it.
> > > > > > * Merge 4 performance optimized INFs into OpensslLib*Accel.inf
> > > > > > * Remove use of PcdOpensslEcEnabled and use OpensslLibFull*.inf
> > > > instead
> > > > > > * Add OpensslLib and IntrinsicLib to CryptoPkg.dec as private library
> > > > classes
> > > > > > * Update all OpensslLib instances to always produce all APIs in
> > > > OpensslLib
> > > > > > class
> > > > > > * Move PrintLib dependency from OpensslLib INF files to
> > BaseCryptLib
> > > > INF
> > > > > > files
> > > > > > * Update CryptoPkg.dsc files to provide full CI test coverage across
> > all
> > > > the
> > > > > > supported combinations of OpensslLib, BaseCryptLib, and TlsLib
> > > > instances.
> > > > > > * Remove PACKAGE profile from CryptoPkg.dsc and add
> > > > > > TARGET_UNIT_TESTS
> > > > > > profile. Adding TARGET_UNIT_TESTS profile is required to prevent
> > a
> > > > few
> > > > > > unit
> > > > > > test artifacts being included in non unit test builds of components.
> > > > > > * Add CryptoPkg Readme.md with overview and platform
> > integration
> > > > > > details.
> > > > > > * Update host-based unit tests to always use OpensslLibFull.inf and
> > add
> > > > > > unit
> > > > > > test coverage for OpensslLibFullAccel.inf.
> > > > > > * Add Readme.md with CryptoPkg overview and platform
> > integration
> > > > > > documentation
> > > > > >
> > > > > > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > > > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > > > > Cc: Xiaoyu Lu <xiaoyu1.lu@intel.com>
> > > > > > Cc: Guomin Jiang <guomin.jiang@intel.com>
> > > > > > Cc: Christopher Zurcher <christopher.zurcher@microsoft.com>
> > > > > > Cc: Rebecca Cran <quic_rcran@quicinc.com>
> > > > > > Cc: Ard Biesheuvel <ardb@kernel.org>
> > > > > > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> > > > > >
> > > > > > Michael D Kinney (12):
> > > > > > CryptoPkg: Document and disable deprecated crypto services
> > > > > > CryptoPkg/Library/BaseCryptLib: Add missing UNI file and fix
> > format
> > > > > > CryptoPkg/Library/BaseCryptLib: Update internal
> > functions/variables
> > > > > > CryptoPkg/Test/UnitTest/Library/BaseCryptLib: Unit test fixes
> > > > > > CryptoPkg/Library: Cleanup BaseCryptLib and TlsLib
> > > > > > CryptoPkg/Library/OpensslLib: Combine all performance optimized
> > > > INFs
> > > > > > CryptoPkg/Library/OpensslLib: Produce consistent set of APIs
> > > > > > CryptoPkg/Library/OpensslLib: Remove PrintLib from INF files
> > > > > > CryptoPkg: Remove PcdOpensslEcEnabled from CryptoPkg.dec
> > > > > > CryptoPkg: Update DSC to improve CI test coverage
> > > > > > CryptoPkg: Fixed host-based unit tests
> > > > > > CryptoPkg: Add Readme.md
> > > > > >
> > > > > > CryptoPkg/CryptoPkg.ci.yaml | 11 +-
> > > > > > CryptoPkg/CryptoPkg.dec | 42 +-
> > > > > > CryptoPkg/CryptoPkg.dsc | 460 +++++++++---
> > > > > > .../Pcd/PcdCryptoServiceFamilyEnable.h | 122 +--
> > > > > > .../Library/BaseCryptLib/BaseCryptLib.inf | 10 +-
> > > > > > .../Library/BaseCryptLib/BaseCryptLib.uni | 2 -
> > > > > > .../Library/BaseCryptLib/Hmac/CryptHmac.c | 7 +
> > > > > > .../Library/BaseCryptLib/Kdf/CryptHkdf.c | 5 +-
> > > > > > .../Library/BaseCryptLib/PeiCryptLib.inf | 8 +-
> > > > > > .../Library/BaseCryptLib/PeiCryptLib.uni | 2 -
> > > > > > .../BaseCryptLib/Pk/CryptAuthenticode.c | 2 +-
> > > > > > .../BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c | 3 +-
> > > > > > .../BaseCryptLib/Pk/CryptPkcs7VerifyEku.c | 3 +
> > > > > > CryptoPkg/Library/BaseCryptLib/Pk/CryptTs.c | 44 +-
> > > > > > .../Library/BaseCryptLib/RuntimeCryptLib.inf | 9 +-
> > > > > > .../Library/BaseCryptLib/RuntimeCryptLib.uni | 2 -
> > > > > > .../Library/BaseCryptLib/SecCryptLib.inf | 13 +-
> > > > > > .../{SmmCryptLib.uni => SecCryptLib.uni} | 11 +-
> > > > > > .../Library/BaseCryptLib/SmmCryptLib.inf | 12 -
> > > > > > .../Library/BaseCryptLib/SmmCryptLib.uni | 2 -
> > > > > > .../BaseCryptLib/UnitTestHostBaseCryptLib.inf | 22 +-
> > > > > > .../Library/Include/openssl/opensslconf.h | 328 +++++++-
> > > > > > .../Include/openssl/opensslconf_generated.h | 333 ---------
> > > > > > CryptoPkg/Library/OpensslLib/EcSm2Null.c | 291 ++++++++
> > > > > > CryptoPkg/Library/OpensslLib/OpensslLib.inf | 133 ++--
> > > > > > CryptoPkg/Library/OpensslLib/OpensslLib.uni | 10 +-
> > > > > > ...nsslLibIa32Gcc.inf => OpensslLibAccel.inf} | 189 +++--
> > > > > > .../Library/OpensslLib/OpensslLibAccel.uni | 14 +
> > > > > > .../OpensslLib/OpensslLibConstructor.c | 6 +-
> > > > > > .../Library/OpensslLib/OpensslLibCrypto.inf | 185 +++--
> > > > > > .../Library/OpensslLib/OpensslLibCrypto.uni | 11 +-
> > > > > > .../{OpensslLib.inf => OpensslLibFull.inf} | 143 ++--
> > > > > > .../{OpensslLib.uni => OpensslLibFull.uni} | 10 +-
> > > > > > ...sslLibIa32.inf => OpensslLibFullAccel.inf} | 192 +++--
> > > > > > .../OpensslLib/OpensslLibFullAccel.uni | 14 +
> > > > > > .../Library/OpensslLib/OpensslLibX64.inf | 706 ------------------
> > > > > > .../Library/OpensslLib/OpensslLibX64Gcc.inf | 706 ------------------
> > > > > > CryptoPkg/Library/OpensslLib/SslNull.c | 405 ++++++++++
> > > > > > .../SysCall/inet_pton.c | 0
> > > > > > CryptoPkg/Library/TlsLib/TlsConfig.c | 2 +-
> > > > > > CryptoPkg/Library/TlsLib/TlsLib.inf | 12 +-
> > > > > > CryptoPkg/Private/Library/IntrinsicLib.h | 16 +
> > > > > > CryptoPkg/Private/Library/OpensslLib.h | 14 +
> > > > > > CryptoPkg/Readme.md | 498 ++++++++++++
> > > > > > CryptoPkg/Test/CryptoPkgHostUnitTest.dsc | 17 +-
> > > > > > .../UnitTest/Library/BaseCryptLib/HmacTests.c | 17 +-
> > > > > > .../UnitTest/Library/BaseCryptLib/TSTests.c | 2 +-
> > > > > > .../TestBaseCryptLibHostAccel.inf | 55 ++
> > > > > > 48 files changed, 2667 insertions(+), 2434 deletions(-)
> > > > > > copy CryptoPkg/Library/BaseCryptLib/{SmmCryptLib.uni =>
> > > > > > SecCryptLib.uni} (74%)
> > > > > > delete mode 100644
> > > > > > CryptoPkg/Library/Include/openssl/opensslconf_generated.h
> > > > > > create mode 100644 CryptoPkg/Library/OpensslLib/EcSm2Null.c
> > > > > > rename CryptoPkg/Library/OpensslLib/{OpensslLibIa32Gcc.inf =>
> > > > > > OpensslLibAccel.inf} (79%)
> > > > > > create mode 100644
> > > > CryptoPkg/Library/OpensslLib/OpensslLibAccel.uni
> > > > > > copy CryptoPkg/Library/OpensslLib/{OpensslLib.inf =>
> > > > OpensslLibFull.inf}
> > > > > > (80%)
> > > > > > copy CryptoPkg/Library/OpensslLib/{OpensslLib.uni =>
> > > > OpensslLibFull.uni}
> > > > > > (56%)
> > > > > > rename CryptoPkg/Library/OpensslLib/{OpensslLibIa32.inf =>
> > > > > > OpensslLibFullAccel.inf} (79%)
> > > > > > create mode 100644
> > > > > > CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.uni
> > > > > > delete mode 100644
> > CryptoPkg/Library/OpensslLib/OpensslLibX64.inf
> > > > > > delete mode 100644
> > > > CryptoPkg/Library/OpensslLib/OpensslLibX64Gcc.inf
> > > > > > create mode 100644 CryptoPkg/Library/OpensslLib/SslNull.c
> > > > > > rename CryptoPkg/Library/{BaseCryptLib =>
> > TlsLib}/SysCall/inet_pton.c
> > > > > > (100%)
> > > > > > create mode 100644 CryptoPkg/Private/Library/IntrinsicLib.h
> > > > > > create mode 100644 CryptoPkg/Private/Library/OpensslLib.h
> > > > > > create mode 100644 CryptoPkg/Readme.md
> > > > > > create mode 100644
> > > > > >
> > > >
> > CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibHostAccel.i
> > > > > > nf
> > > > > >
> > > > > > --
> > > > > > 2.37.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95038): https://edk2.groups.io/g/devel/message/95038
Mute This Topic: https://groups.io/mt/94260718/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Thanks Mike. Agree with you.
For the series, Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
Just FYI: I have merged rest pending CryptoPkg patches.
Please rebase when you submit next version.
Thank you
Yao Jiewen
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Wednesday, October 12, 2022 10:23 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1
> <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
> Zurcher, Christopher <christopher.zurcher@microsoft.com>; Rebecca Cran
> <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> Subject: RE: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt
> OpensslLibs
>
> Hi Jiewen,
>
> comments below.
>
> Mike
>
> > -----Original Message-----
> > From: Yao, Jiewen <jiewen.yao@intel.com>
> > Sent: Tuesday, October 11, 2022 7:07 PM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1
> <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
> Zurcher,
> > Christopher <christopher.zurcher@microsoft.com>; Rebecca Cran
> <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> > Subject: RE: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt
> OpensslLibs
> >
> > OK. That means we need manual write a wrapper for all EC APIs in Openssl
> Lib for internal usage.
> > More precisely, a wrapper for the delta between OpensslLib and
> OpensslLibFull, right?
>
> Yes. This these are implemented in:
>
> https://github.com/mdkinney/edk2/blob/CryptoPkg_RemoveEcPcd_Merge
> OptimizedOpensslLibs/CryptoPkg/Library/OpensslLib/SslNull.c
> https://github.com/mdkinney/edk2/blob/CryptoPkg_RemoveEcPcd_Merge
> OptimizedOpensslLibs/CryptoPkg/Library/OpensslLib/EcSm2Null.c
>
> New instances are easy to find because you will get a link failure in CI for
> missing symbols if
> BaseCryptLib or TlsLib uses a new API that is only present in
> OpensslLibFull.inf. When that occurs
> add the Null version of API that always returns a failure condition.
>
> The current implementation before this PR is using OpensslLib class, but the
> OpensslLib instances
> are inconsistent in the APIs produced by the lib instances. This is what
> caused the introduction
> of #if for EC feature into the layers above OpensslLib. This means the EDK II
> rules for
> lib class/lib instance were not followed, and the workaround was to use #if
> which are also not allowed.
>
> The correct fix is to follow EDK II lib class/lib instance rules. All lib instances
> of the same
> lib class always produce the same APIs, and then all components that use
> the lib class can depend
> on all the services being present without link failures.
>
> >
> > There will be size difference between those two solutions, because the
> code other than EC will be include in the function.
>
> Yes,
>
> >
> > Why not use NO_EC macro from openssl directly?
>
> Because that hides all the EC related types required to implement the Null
> EC APIs
> and the EC types required in the layer above OpensslLib that calls EC
> services.
> They will not compile without the EC types being defined.
>
> >
> > Thank you
> > Yao Jiewen
> >
> >
> > > -----Original Message-----
> > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > Sent: Wednesday, October 12, 2022 9:55 AM
> > > To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; Kinney,
> > > Michael D <michael.d.kinney@intel.com>
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1
> > > <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
> > > Zurcher, Christopher <christopher.zurcher@microsoft.com>; Rebecca
> Cran
> > > <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> > > Subject: RE: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf
> opt
> > > OpensslLibs
> > >
> > > Jiewen,
> > >
> > > The BKM is to just call the EC services from the layers above OpensslLib.
> > > The EC APIs will always be present. Either real EC service or Null EC
> service.
> > > This way we can remove all the #ifdef on EC PCD.
> > >
> > > Platform developers select the OpensslLib instance with EC
> > > (OpensslLibFull.inf) or
> > > without EC (OpensslLib.inf).
> > >
> > > Mike
> > >
> > > > -----Original Message-----
> > > > From: Yao, Jiewen <jiewen.yao@intel.com>
> > > > Sent: Tuesday, October 11, 2022 6:37 PM
> > > > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > > devel@edk2.groups.io
> > > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1
> > > <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
> > > Zurcher,
> > > > Christopher <christopher.zurcher@microsoft.com>; Rebecca Cran
> > > <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> > > > Subject: RE: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf
> opt
> > > OpensslLibs
> > > >
> > > > Hi Mike
> > > > For PcdOpensslEcEnabled, I am seeing other usage. For example:
> > > >
> > >
> https://github.com/qizhangz/edk2/blob/EC_Upstream/CryptoPkg/Library/
> > > BaseCryptLib/Pem/CryptPem.c#L156
> > > >
> > > > I think we may need BKM on "how to disable EC".
> > > >
> > > > Thank you
> > > > Yao, Jiewen
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > > > Sent: Wednesday, October 12, 2022 9:25 AM
> > > > > To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io;
> Kinney,
> > > > > Michael D <michael.d.kinney@intel.com>
> > > > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1
> > > > > <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
> > > > > Zurcher, Christopher <christopher.zurcher@microsoft.com>;
> Rebecca
> > > Cran
> > > > > <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> > > > > Subject: RE: [Patch 00/12] CryptoPkg: Remove EC PCD and merge
> perf
> > > opt
> > > > > OpensslLibs
> > > > >
> > > > > Hi Jiewen,
> > > > >
> > > > > Comments below.
> > > > >
> > > > > Mike
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Yao, Jiewen <jiewen.yao@intel.com>
> > > > > > Sent: Tuesday, October 11, 2022 6:09 PM
> > > > > > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > > > > devel@edk2.groups.io
> > > > > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1
> > > > > <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
> > > > > Zurcher,
> > > > > > Christopher <christopher.zurcher@microsoft.com>; Rebecca Cran
> > > > > <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> > > > > > Subject: RE: [Patch 00/12] CryptoPkg: Remove EC PCD and merge
> perf
> > > opt
> > > > > OpensslLibs
> > > > > >
> > > > > > Thank you Mike.
> > > > > >
> > > > > > 1) I like the idea to combine multiple OpensslLibIA32/X64.inf into
> one
> > > > > OpensslLibAccel.inf.
> > > > > > Also the cleanup looks good to me.
> > > > > >
> > > > > > 2) I also like the summary in readme in
> > > > > >
> > > > >
> > >
> https://github.com/mdkinney/edk2/tree/CryptoPkg_RemoveEcPcd_Merge
> > > > > OptimizedOpensslLibs/CryptoPkg
> > > > > > I notice some algorithms are listed Y(Deprecated) but N(Don't Use),
> > > such
> > > > > as Tdes, Arc4, Aes.Ecb*.
> > > > > > But I don’t see the use case for those algorithms and I suggest a
> > > > > Y(Deprecated) have Y(Don't Use).
> > > > >
> > > > > Good catch. I will fix.
> > > > >
> > > > > >
> > > > > > 3) About PcdOpensslEcEnabled
> > > > > > I notice it is used in existing code -
> > > > > >
> > > > >
> > >
> https://github.com/mdkinney/edk2/blob/CryptoPkg_RemoveEcPcd_Merge
> > > > > OptimizedOpensslLibs/CryptoPkg/Library/TlsLib/TlsConfig.c#L11
> > > > > > 39
> > > > > > Is this right way?
> > > > >
> > > > > This was added since I started this work and was added back in by
> > > rebase. I
> > > > > will fix.
> > > > > We can just remove the check for the PCD. If the OpensslLib
> instance
> > > does
> > > > > not include
> > > > > SSL services, then the Null SSL services are present and the call to
> > > SSL_ctrl()
> > > > > will
> > > > > return 0 which will force TlsSetEcCurve() to return
> EFI_UNSUPPORTED.
> > > It
> > > > > will also
> > > > > ASSERT() informing the developer that a call to a service that
> depends
> > > on
> > > > > SSL was made
> > > > > without SSL services available.
> > > > >
> > > > > long
> > > > > SSL_ctrl (
> > > > > SSL *ssl,
> > > > > int cmd,
> > > > > long larg,
> > > > > void *parg
> > > > > )
> > > > > {
> > > > > ASSERT (FALSE);
> > > > > return 0;
> > > > > }
> > > > >
> > > > > Likewise, if the OpensslLib instance does not support EC services,
> then
> > > the
> > > > > Null
> > > > > EC services will be included and the call to
> > > EC_KEY_new_by_curve_name()
> > > > > will
> > > > > return NULL which will force TlsSetEcCurve() to return
> > > EFI_UNSUPPORTED. It
> > > > > will also
> > > > > ASSERT() informing the developer that a call to a service that
> depends
> > > on EC
> > > > > was made
> > > > > without EC services available.
> > > > >
> > > > > EC_KEY *
> > > > > EC_KEY_new_by_curve_name (
> > > > > int nid
> > > > > )
> > > > > {
> > > > > ASSERT (FALSE);
> > > > > return NULL;
> > > > > }
> > > > >
> > > > > >
> > > > > > Thank you
> > > > > > Yao, Jiewen
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > > > > > Sent: Tuesday, October 11, 2022 11:04 PM
> > > > > > > To: devel@edk2.groups.io
> > > > > > > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> > > > > > > <jian.j.wang@intel.com>; Lu, Xiaoyu1 <xiaoyu1.lu@intel.com>;
> Jiang,
> > > > > > > Guomin <guomin.jiang@intel.com>; Zurcher, Christopher
> > > > > > > <christopher.zurcher@microsoft.com>; Rebecca Cran
> > > > > > > <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>
> > > > > > > Subject: [Patch 00/12] CryptoPkg: Remove EC PCD and merge
> perf
> > > opt
> > > > > > > OpensslLibs
> > > > > > >
> > > > > > > The recent addition of the Ecliptic Curve (EC) feature and the
> > > > > performance
> > > > > > > optimization features increased the complexity for platforms to
> > > > > integrate
> > > > > > > and enable these features. This series simplifies the platform
> > > > > configuration
> > > > > > > as much as possible and improves the ability to manage the the
> size
> > > > > impact
> > > > > > > of cryptographic services in each FW phase. A Readme.md is also
> > > added
> > > > > > > that
> > > > > > > provides an overview of the CryptoPkg design and features along
> > > with
> > > > > > > platform
> > > > > > > integration recommendations.
> > > > > > >
> > > > > > > This series also addresses private library class declarations
> missing
> > > from
> > > > > > > CryptoPkg.dec and library instances not producing all the APIs
> > > defined
> > > > > > > by the library classes. A review of the CryptoPkg EDK II meta
> data
> > > files
> > > > > > > identified
> > > > > > > a number of additional cleanups. The CryptoPkg.dsc file was also
> > > > > updated to
> > > > > > > improve CI coverage for future CryptoPkg changes and identified
> > > some
> > > > > > > unit test bug fixes.
> > > > > > >
> > > > > > > PR: https://github.com/tianocore/edk2/pull/3443
> > > > > > > Branch:
> > > > > > >
> > > > >
> > >
> https://github.com/mdkinney/edk2/tree/CryptoPkg_RemoveEcPcd_Merge
> > > > > > > OptimizedOpensslLibs
> > > > > > > Readme:
> > > > > > >
> > > > >
> > >
> https://github.com/mdkinney/edk2/blob/CryptoPkg_RemoveEcPcd_Merge
> > > > > > > OptimizedOpensslLibs/CryptoPkg/Readme.md
> > > > > > >
> > > > > > > Change Summary
> > > > > > > ==============
> > > > > > > * Document disabled/deprecated cryptographic services
> > > > > > > * Add missing UNI files in BaseCryptLib
> > > > > > > * Update BaseCryptLib internal functions to be STATIC and
> remove
> > > > > EFIAPI
> > > > > > > * Add GLOBAL_REMOVE_IF_UNREFERENCED to BaseCryptLib
> global
> > > > > > > variables
> > > > > > > * Fix BaseCryptLib unit tests
> > > > > > > * Cleanup BaseCryptLib and TlsLib INF files and
> > > > > > > * Move SysCall/inet_pton.c from BaseCryptLib to TlsLib that
> uses it.
> > > > > > > * Merge 4 performance optimized INFs into OpensslLib*Accel.inf
> > > > > > > * Remove use of PcdOpensslEcEnabled and use
> OpensslLibFull*.inf
> > > > > instead
> > > > > > > * Add OpensslLib and IntrinsicLib to CryptoPkg.dec as private
> library
> > > > > classes
> > > > > > > * Update all OpensslLib instances to always produce all APIs in
> > > > > OpensslLib
> > > > > > > class
> > > > > > > * Move PrintLib dependency from OpensslLib INF files to
> > > BaseCryptLib
> > > > > INF
> > > > > > > files
> > > > > > > * Update CryptoPkg.dsc files to provide full CI test coverage
> across
> > > all
> > > > > the
> > > > > > > supported combinations of OpensslLib, BaseCryptLib, and
> TlsLib
> > > > > instances.
> > > > > > > * Remove PACKAGE profile from CryptoPkg.dsc and add
> > > > > > > TARGET_UNIT_TESTS
> > > > > > > profile. Adding TARGET_UNIT_TESTS profile is required to
> prevent
> > > a
> > > > > few
> > > > > > > unit
> > > > > > > test artifacts being included in non unit test builds of
> components.
> > > > > > > * Add CryptoPkg Readme.md with overview and platform
> > > integration
> > > > > > > details.
> > > > > > > * Update host-based unit tests to always use OpensslLibFull.inf
> and
> > > add
> > > > > > > unit
> > > > > > > test coverage for OpensslLibFullAccel.inf.
> > > > > > > * Add Readme.md with CryptoPkg overview and platform
> > > integration
> > > > > > > documentation
> > > > > > >
> > > > > > > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > > > > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > > > > > Cc: Xiaoyu Lu <xiaoyu1.lu@intel.com>
> > > > > > > Cc: Guomin Jiang <guomin.jiang@intel.com>
> > > > > > > Cc: Christopher Zurcher <christopher.zurcher@microsoft.com>
> > > > > > > Cc: Rebecca Cran <quic_rcran@quicinc.com>
> > > > > > > Cc: Ard Biesheuvel <ardb@kernel.org>
> > > > > > > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> > > > > > >
> > > > > > > Michael D Kinney (12):
> > > > > > > CryptoPkg: Document and disable deprecated crypto services
> > > > > > > CryptoPkg/Library/BaseCryptLib: Add missing UNI file and fix
> > > format
> > > > > > > CryptoPkg/Library/BaseCryptLib: Update internal
> > > functions/variables
> > > > > > > CryptoPkg/Test/UnitTest/Library/BaseCryptLib: Unit test fixes
> > > > > > > CryptoPkg/Library: Cleanup BaseCryptLib and TlsLib
> > > > > > > CryptoPkg/Library/OpensslLib: Combine all performance
> optimized
> > > > > INFs
> > > > > > > CryptoPkg/Library/OpensslLib: Produce consistent set of APIs
> > > > > > > CryptoPkg/Library/OpensslLib: Remove PrintLib from INF files
> > > > > > > CryptoPkg: Remove PcdOpensslEcEnabled from CryptoPkg.dec
> > > > > > > CryptoPkg: Update DSC to improve CI test coverage
> > > > > > > CryptoPkg: Fixed host-based unit tests
> > > > > > > CryptoPkg: Add Readme.md
> > > > > > >
> > > > > > > CryptoPkg/CryptoPkg.ci.yaml | 11 +-
> > > > > > > CryptoPkg/CryptoPkg.dec | 42 +-
> > > > > > > CryptoPkg/CryptoPkg.dsc | 460 +++++++++---
> > > > > > > .../Pcd/PcdCryptoServiceFamilyEnable.h | 122 +--
> > > > > > > .../Library/BaseCryptLib/BaseCryptLib.inf | 10 +-
> > > > > > > .../Library/BaseCryptLib/BaseCryptLib.uni | 2 -
> > > > > > > .../Library/BaseCryptLib/Hmac/CryptHmac.c | 7 +
> > > > > > > .../Library/BaseCryptLib/Kdf/CryptHkdf.c | 5 +-
> > > > > > > .../Library/BaseCryptLib/PeiCryptLib.inf | 8 +-
> > > > > > > .../Library/BaseCryptLib/PeiCryptLib.uni | 2 -
> > > > > > > .../BaseCryptLib/Pk/CryptAuthenticode.c | 2 +-
> > > > > > > .../BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c | 3 +-
> > > > > > > .../BaseCryptLib/Pk/CryptPkcs7VerifyEku.c | 3 +
> > > > > > > CryptoPkg/Library/BaseCryptLib/Pk/CryptTs.c | 44 +-
> > > > > > > .../Library/BaseCryptLib/RuntimeCryptLib.inf | 9 +-
> > > > > > > .../Library/BaseCryptLib/RuntimeCryptLib.uni | 2 -
> > > > > > > .../Library/BaseCryptLib/SecCryptLib.inf | 13 +-
> > > > > > > .../{SmmCryptLib.uni => SecCryptLib.uni} | 11 +-
> > > > > > > .../Library/BaseCryptLib/SmmCryptLib.inf | 12 -
> > > > > > > .../Library/BaseCryptLib/SmmCryptLib.uni | 2 -
> > > > > > > .../BaseCryptLib/UnitTestHostBaseCryptLib.inf | 22 +-
> > > > > > > .../Library/Include/openssl/opensslconf.h | 328 +++++++-
> > > > > > > .../Include/openssl/opensslconf_generated.h | 333 ---------
> > > > > > > CryptoPkg/Library/OpensslLib/EcSm2Null.c | 291 ++++++++
> > > > > > > CryptoPkg/Library/OpensslLib/OpensslLib.inf | 133 ++--
> > > > > > > CryptoPkg/Library/OpensslLib/OpensslLib.uni | 10 +-
> > > > > > > ...nsslLibIa32Gcc.inf => OpensslLibAccel.inf} | 189 +++--
> > > > > > > .../Library/OpensslLib/OpensslLibAccel.uni | 14 +
> > > > > > > .../OpensslLib/OpensslLibConstructor.c | 6 +-
> > > > > > > .../Library/OpensslLib/OpensslLibCrypto.inf | 185 +++--
> > > > > > > .../Library/OpensslLib/OpensslLibCrypto.uni | 11 +-
> > > > > > > .../{OpensslLib.inf => OpensslLibFull.inf} | 143 ++--
> > > > > > > .../{OpensslLib.uni => OpensslLibFull.uni} | 10 +-
> > > > > > > ...sslLibIa32.inf => OpensslLibFullAccel.inf} | 192 +++--
> > > > > > > .../OpensslLib/OpensslLibFullAccel.uni | 14 +
> > > > > > > .../Library/OpensslLib/OpensslLibX64.inf | 706 ------------------
> > > > > > > .../Library/OpensslLib/OpensslLibX64Gcc.inf | 706 -----------------
> -
> > > > > > > CryptoPkg/Library/OpensslLib/SslNull.c | 405 ++++++++++
> > > > > > > .../SysCall/inet_pton.c | 0
> > > > > > > CryptoPkg/Library/TlsLib/TlsConfig.c | 2 +-
> > > > > > > CryptoPkg/Library/TlsLib/TlsLib.inf | 12 +-
> > > > > > > CryptoPkg/Private/Library/IntrinsicLib.h | 16 +
> > > > > > > CryptoPkg/Private/Library/OpensslLib.h | 14 +
> > > > > > > CryptoPkg/Readme.md | 498 ++++++++++++
> > > > > > > CryptoPkg/Test/CryptoPkgHostUnitTest.dsc | 17 +-
> > > > > > > .../UnitTest/Library/BaseCryptLib/HmacTests.c | 17 +-
> > > > > > > .../UnitTest/Library/BaseCryptLib/TSTests.c | 2 +-
> > > > > > > .../TestBaseCryptLibHostAccel.inf | 55 ++
> > > > > > > 48 files changed, 2667 insertions(+), 2434 deletions(-)
> > > > > > > copy CryptoPkg/Library/BaseCryptLib/{SmmCryptLib.uni =>
> > > > > > > SecCryptLib.uni} (74%)
> > > > > > > delete mode 100644
> > > > > > > CryptoPkg/Library/Include/openssl/opensslconf_generated.h
> > > > > > > create mode 100644
> CryptoPkg/Library/OpensslLib/EcSm2Null.c
> > > > > > > rename CryptoPkg/Library/OpensslLib/{OpensslLibIa32Gcc.inf =>
> > > > > > > OpensslLibAccel.inf} (79%)
> > > > > > > create mode 100644
> > > > > CryptoPkg/Library/OpensslLib/OpensslLibAccel.uni
> > > > > > > copy CryptoPkg/Library/OpensslLib/{OpensslLib.inf =>
> > > > > OpensslLibFull.inf}
> > > > > > > (80%)
> > > > > > > copy CryptoPkg/Library/OpensslLib/{OpensslLib.uni =>
> > > > > OpensslLibFull.uni}
> > > > > > > (56%)
> > > > > > > rename CryptoPkg/Library/OpensslLib/{OpensslLibIa32.inf =>
> > > > > > > OpensslLibFullAccel.inf} (79%)
> > > > > > > create mode 100644
> > > > > > > CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.uni
> > > > > > > delete mode 100644
> > > CryptoPkg/Library/OpensslLib/OpensslLibX64.inf
> > > > > > > delete mode 100644
> > > > > CryptoPkg/Library/OpensslLib/OpensslLibX64Gcc.inf
> > > > > > > create mode 100644 CryptoPkg/Library/OpensslLib/SslNull.c
> > > > > > > rename CryptoPkg/Library/{BaseCryptLib =>
> > > TlsLib}/SysCall/inet_pton.c
> > > > > > > (100%)
> > > > > > > create mode 100644 CryptoPkg/Private/Library/IntrinsicLib.h
> > > > > > > create mode 100644 CryptoPkg/Private/Library/OpensslLib.h
> > > > > > > create mode 100644 CryptoPkg/Readme.md
> > > > > > > create mode 100644
> > > > > > >
> > > > >
> > >
> CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibHostAccel.i
> > > > > > > nf
> > > > > > >
> > > > > > > --
> > > > > > > 2.37.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95059): https://edk2.groups.io/g/devel/message/95059
Mute This Topic: https://groups.io/mt/94260718/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.