[RFC] [staging/CryptoLibrary] Openssl1.1 replacement proposal

[RFC] [staging/CryptoLibrary] Openssl1.1 replacement proposal

[Yao, Jiewen]

[-- Attachment #1: Type: text/plain, Size: 3943 bytes --]

Hello
I would like to give a proposal for the openssl 1.1 replacement in EDKII community.

[Problem Statement]
Openssl 1.1 is about to EOL at September, 2023. We need find replacement.

Bugzilla:
1. Openssl3.0: https://bugzilla.tianocore.org/show_bug.cgi?id=3466
2. MbedTls: https://bugzilla.tianocore.org/show_bug.cgi?id=4177

[POC result]
1. The natural successor is Openssl 3.0.
However, it brings size issue according to the POC evaluation - https://edk2.groups.io/g/devel/topic/87479913.
The concern is that: The size of flash is fixed. If we naïve replace it directly, the existing platforms may be broken immediately.

2. The alternative is MbedTls, which is much smaller.
However, there is feature gap, e.g. PKCS7 is not there.

3. At same time, we evaluated other crypto library.
A. Intel-IPP: https://software.intel.com/en-us/intel-ipp , https://github.com/intel/ipp-crypto (Apache license) - CON: Only crypto primitive, no certificate, no TLS
B. Libsodium: https://doc.libsodium.org/, https://github.com/jedisct1/libsodium (ISC license) - CON: Only crypto primitive, no certificate, no TLS
C: BoringSSL: https://github.com/google/boringssl (ISC license) - CON: Google only project. It says: "We don't recommend that third parties depend upon it."
D: WolfSSL: https://www.wolfssl.com/, https://github.com/wolfSSL/wolfssl (GPL license) - CON: GPL License issue.
E: BearSSL: https://bearssl.org/ (MIT license) - CON: Current version is 0.6. It is now considered beta-quality software.

[Proposal]
1. Let's put *Openssl 3.0 POC* to *edk2-staging*, and continue the research on how to reduce the size.
It is possible that we may need add MACRO to Openssl 3.0 to reduce the size. We can do POC and submit to openssl community.

2. Let's put *MbedTls POC* to *edk2-staging*, and continue the research on how to add missing features.

3. If 1 or 2 can success, we can replace openssl 1.1 with one crypto lib.
If both 1 and 2 fail, we may use *dual-crypto module*. For example: mbedtls for PEI and openssl3.0 for DXE.
The source code size will become larger, more time to download the tree.

4. We need control the quality of Openssl 1.1 replacement to avoid regression.
I propose the check-in criteria below:
A) Size delta < +10%
B) No API change.
C) All existing crypto usages in EDKII are covered, including:
UEFI Secure Boot - image verification/auth variable update (PKCS7)
TCG Trusted Boot (SHA2/SM3)
UEFI/PI FMP Capsule (PKCS7/PKCS1-v1.5-RSA)
HTTPs Boot (TLS 1.2/1.3)
BIOS Password (PKCS5-PBKDF)
iSCSI (SHA2)
HDD Password (SHA2)
Hash2Dxe (SHA2)
Pkcs7Verify (PKCS7)
D) All crypto APIs in EDKII are covered besides above use case, including:
SHA3-ParallelHash
HMAC/HKDF
ECC
E) All crypto test need pass.

Please review and provide your feedback.


[Misc]
I would like to clarify my position on the original openssl 3.0 path, to avoid the misunderstanding:
A) I never expressed a desire to retain the OpenSSL 1.1. (I am sorry, if I did not say that clearly.)
B) I raised the concern that *current OpenSSL 3.0 patch requires more improvement*, because it will increase the size and break the existing platform.
It has no difference with other comment I give in the EDKII. If we want to check in, we need resolve the concern and ensure it can still work with existing platform.


Last but not least, I would like to suggest *again* that we maintain *a civil and healthy community environment*. (Thanks, Ard)
NOTE: All EDKII maintainers are doing volunteer work here. We are using development mailing list. Let's focus on technical topic.
If anyone has any constructive technical idea, feel free to express your opinion, and we are happy to listen and discuss.
Any collaboration, patch submission, and validation are more than welcome.

This is not a small work. We need all EDK-II community people's help to deal with this OpenSSL 1.1 EOL.

Thank you
Yao, Jiewen


[-- Attachment #2: Type: text/html, Size: 9423 bytes --]

Re: [edk2-devel] [RFC] [staging/CryptoLibrary] Openssl1.1 replacement proposal

[Marvin Häuser]

[-- Attachment #1: Type: text/plain, Size: 2015 bytes --]

Hi Jiewen,

Thanks for the proposal!

Regarding OpenSSL vs mbedtls, maybe a little fun fact: For a non-commercial project, we moved away from OpenSSL also due to size constraints. For various reasons, we forked off existing crypto code and kept our changes downstream (I do *not* recommend this and this will be taken care of in the future, but remember this is non-commercial and non-mission-critical). When I was optimizing the code to be tailored to our RSA usage, I noticed that OpenSSL does a whole load of complicated stuff in the BigNum space [1], when really due to the specific guarantees and requirements of RSA with Montgomery Arithmetics, it can all be done in the machine integer space. In fact, I was questioning my Maths for a bit, wondering why OpenSSL never implemented any tailored shortcut for this.

When I was looking at mbedtls today, I was surprised that not only did they optimize the logic to run solely in the machine integer space like I did, they also have some very smart optimization to shrink the code to nothing but a bare minimum [2]. Just look at the difference in mere code size. I *severely* dislike the absence of any sort of explanation or intuition for the code, let alone a proof of correctness. But just from the code design perspective, this is exactly what you want in the firmware space, in my opinion. I haven't checked other things in detail, but most things look rather sane to me.

I wish I knew why OpenSSL seems to prefer many implementations of the same algorithms in ASM, which has performance benefits but is an absolute pain to review and maintain, but lets such obvious "macro optimizations", that improve code performance, size, and readability (if documented correctly) at once, slide.

Best regards,
Marvin

[1] https://github.com/openssl/openssl/blob/b85a1f13d6d99ef96cc81d17d5ff991e1294a21b/crypto/bn/bn_gcd.c#L197-L512

[2] https://github.com/Mbed-TLS/mbedtls/blob/10ada3501975e7abab25a7fa28e9e8e0f6b4259f/library/bignum_core.c#L454-L465

[-- Attachment #2: Type: text/html, Size: 2337 bytes --]

Re: [edk2-devel] [RFC] [staging/CryptoLibrary] Openssl1.1 replacement proposal

[Gerd Hoffmann]

  Hi,

> 3. If 1 or 2 can success, we can replace openssl 1.1 with one crypto lib.
> If both 1 and 2 fail, we may use *dual-crypto module*. For example: mbedtls for PEI and openssl3.0 for DXE.
> The source code size will become larger, more time to download the tree.

Suggestions how to do that best, ideally without duplicating CryptoPkg
for that?

A while back I've tried to add openssl-3 in parallel to openssl-11,
with the idea to allow projects picking the one or the other, and quicky
ran into problems because apparently libraries can't add include
directories.  Only packages can do that (see Includes.Common.Private in
CryptoPkg/CryptoPkg.dec which adds Library/OpensslLib/openssl/include).

take care,
  Gerd

Re: [edk2-devel] [RFC] [staging/CryptoLibrary] Openssl1.1 replacement proposal

[Yao, Jiewen]

If you are asking how to do that best *at this moment*, I suggest we create a branch in https://github.com/tianocore/edk2-staging and continue the research work. Before September 2023, we need community's help to resolve openssl-3 size issue, before check in.

If you are asking how to do that best after September 2023, we have no choice but put to edk2 main branch. We have to remove openssl-11.

If we have either openssl-30 and mbedtls work (size/feature), we can replace openssl-11 with either openssl-30 or mbedtls.

Worst case, if we have to support dual-crypto module, I think to:
1) replace openssl-11 with openssl-30 directly.
2) add mbedtls as another cryptolib instance.

Thank you
Yao, Jiewen

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd
> Hoffmann
> Sent: Wednesday, February 8, 2023 7:45 PM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: Re: [edk2-devel] [RFC] [staging/CryptoLibrary] Openssl1.1 replacement
> proposal
> 
>   Hi,
> 
> > 3. If 1 or 2 can success, we can replace openssl 1.1 with one crypto lib.
> > If both 1 and 2 fail, we may use *dual-crypto module*. For example: mbedtls
> for PEI and openssl3.0 for DXE.
> > The source code size will become larger, more time to download the tree.
> 
> Suggestions how to do that best, ideally without duplicating CryptoPkg
> for that?
> 
> A while back I've tried to add openssl-3 in parallel to openssl-11,
> with the idea to allow projects picking the one or the other, and quicky
> ran into problems because apparently libraries can't add include
> directories.  Only packages can do that (see Includes.Common.Private in
> CryptoPkg/CryptoPkg.dec which adds Library/OpensslLib/openssl/include).
> 
> take care,
>   Gerd
> 
> 
> 
> 
> 

Re: [edk2-devel] [RFC] [staging/CryptoLibrary] Openssl1.1 replacement proposal

[Yao, Jiewen]

Hi All
I have created staging branch - https://github.com/tianocore/edk2-staging/tree/OpenSSL11_EOL based upon latest trunk today.

Let's use this branch to collaborate the work on openssl 1.1 deprecation and continue improving, before we can merge back to trunk.

The process is defined at https://github.com/tianocore/edk2-staging/.

Feature missing or size increasing won't be a blocking issue for this staging branch.

Any feedback is welcome.

Hi Gerd
If you don't mind, please submit your latest openssl-3.0 patch to the staging for broader evaluation and improvement.

Thank you
Yao, Jiewen

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
> Sent: Thursday, February 9, 2023 11:21 AM
> To: devel@edk2.groups.io; kraxel@redhat.com
> Subject: Re: [edk2-devel] [RFC] [staging/CryptoLibrary] Openssl1.1 replacement
> proposal
> 
> If you are asking how to do that best *at this moment*, I suggest we create a
> branch in https://github.com/tianocore/edk2-staging and continue the research
> work. Before September 2023, we need community's help to resolve openssl-3
> size issue, before check in.
> 
> If you are asking how to do that best after September 2023, we have no choice
> but put to edk2 main branch. We have to remove openssl-11.
> 
> If we have either openssl-30 and mbedtls work (size/feature), we can replace
> openssl-11 with either openssl-30 or mbedtls.
> 
> Worst case, if we have to support dual-crypto module, I think to:
> 1) replace openssl-11 with openssl-30 directly.
> 2) add mbedtls as another cryptolib instance.
> 
> Thank you
> Yao, Jiewen
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd
> > Hoffmann
> > Sent: Wednesday, February 8, 2023 7:45 PM
> > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> > Subject: Re: [edk2-devel] [RFC] [staging/CryptoLibrary] Openssl1.1
> replacement
> > proposal
> >
> >   Hi,
> >
> > > 3. If 1 or 2 can success, we can replace openssl 1.1 with one crypto lib.
> > > If both 1 and 2 fail, we may use *dual-crypto module*. For example: mbedtls
> > for PEI and openssl3.0 for DXE.
> > > The source code size will become larger, more time to download the tree.
> >
> > Suggestions how to do that best, ideally without duplicating CryptoPkg
> > for that?
> >
> > A while back I've tried to add openssl-3 in parallel to openssl-11,
> > with the idea to allow projects picking the one or the other, and quicky
> > ran into problems because apparently libraries can't add include
> > directories.  Only packages can do that (see Includes.Common.Private in
> > CryptoPkg/CryptoPkg.dec which adds Library/OpensslLib/openssl/include).
> >
> > take care,
> >   Gerd
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 

Re: [edk2-devel] [RFC] [staging/CryptoLibrary] Openssl1.1 replacement proposal

[Yao, Jiewen]

Hello
We have created initial POC version CryptoPkg upgrade.

OpenSSL 3.0 POC: https://github.com/tianocore/edk2-staging/blob/OpenSSL11_EOL/CryptoPkg/Readme-OpenSSL3.0.md
The size is reduced a lots. But it still exceeds some platforms.

MbedTls 3.0 POC: https://github.com/tianocore/edk2-staging/blob/OpenSSL11_EOL/CryptoPkg/ReadmeMbedtls.md
The feature is not complete yet, especially SHA3 support is missing, which is required for ParallelHash.

You may try to use it to see if there is any gap.
Also, please let us know if anyone has good idea.

Thank you
Yao, Jiewen

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao,
> Jiewen
> Sent: Saturday, February 11, 2023 10:20 AM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>;
> kraxel@redhat.com
> Subject: Re: [edk2-devel] [RFC] [staging/CryptoLibrary] Openssl1.1
> replacement proposal
> 
> Hi All
> I have created staging branch - https://github.com/tianocore/edk2-
> staging/tree/OpenSSL11_EOL based upon latest trunk today.
> 
> Let's use this branch to collaborate the work on openssl 1.1 deprecation and
> continue improving, before we can merge back to trunk.
> 
> The process is defined at https://github.com/tianocore/edk2-staging/.
> 
> Feature missing or size increasing won't be a blocking issue for this staging
> branch.
> 
> Any feedback is welcome.
> 
> Hi Gerd
> If you don't mind, please submit your latest openssl-3.0 patch to the staging
> for broader evaluation and improvement.
> 
> Thank you
> Yao, Jiewen
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao,
> Jiewen
> > Sent: Thursday, February 9, 2023 11:21 AM
> > To: devel@edk2.groups.io; kraxel@redhat.com
> > Subject: Re: [edk2-devel] [RFC] [staging/CryptoLibrary] Openssl1.1
> replacement
> > proposal
> >
> > If you are asking how to do that best *at this moment*, I suggest we
> create a
> > branch in https://github.com/tianocore/edk2-staging and continue the
> research
> > work. Before September 2023, we need community's help to resolve
> openssl-3
> > size issue, before check in.
> >
> > If you are asking how to do that best after September 2023, we have no
> choice
> > but put to edk2 main branch. We have to remove openssl-11.
> >
> > If we have either openssl-30 and mbedtls work (size/feature), we can
> replace
> > openssl-11 with either openssl-30 or mbedtls.
> >
> > Worst case, if we have to support dual-crypto module, I think to:
> > 1) replace openssl-11 with openssl-30 directly.
> > 2) add mbedtls as another cryptolib instance.
> >
> > Thank you
> > Yao, Jiewen
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd
> > > Hoffmann
> > > Sent: Wednesday, February 8, 2023 7:45 PM
> > > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> > > Subject: Re: [edk2-devel] [RFC] [staging/CryptoLibrary] Openssl1.1
> > replacement
> > > proposal
> > >
> > >   Hi,
> > >
> > > > 3. If 1 or 2 can success, we can replace openssl 1.1 with one crypto lib.
> > > > If both 1 and 2 fail, we may use *dual-crypto module*. For example:
> mbedtls
> > > for PEI and openssl3.0 for DXE.
> > > > The source code size will become larger, more time to download the
> tree.
> > >
> > > Suggestions how to do that best, ideally without duplicating CryptoPkg
> > > for that?
> > >
> > > A while back I've tried to add openssl-3 in parallel to openssl-11,
> > > with the idea to allow projects picking the one or the other, and quicky
> > > ran into problems because apparently libraries can't add include
> > > directories.  Only packages can do that (see Includes.Common.Private in
> > > CryptoPkg/CryptoPkg.dec which adds
> Library/OpensslLib/openssl/include).
> > >
> > > take care,
> > >   Gerd
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 

Re: [edk2-devel] [RFC] [staging/CryptoLibrary] Openssl1.1 replacement proposal

[Gerd Hoffmann]

On Fri, Mar 10, 2023 at 12:28:54PM +0000, Yao, Jiewen wrote:
> Hello
> We have created initial POC version CryptoPkg upgrade.
> 
> OpenSSL 3.0 POC: https://github.com/tianocore/edk2-staging/blob/OpenSSL11_EOL/CryptoPkg/Readme-OpenSSL3.0.md
> The size is reduced a lots. But it still exceeds some platforms.

I've already mentioned the branch in the cover letter of the openssl
hash series (https://edk2.groups.io/g/devel/message/100123), but
apparently it went unnoticed, there are lots of commits from my old
branch in there ...

Anyway, my latest branch (just rebased to master) is here:

https://github.com/kraxel/edk2/commits/openssl3

Doesn't (yet) pass CI, most failures are on IA32 due to missing
compiler intrinsics.

I've put the configuration system upside-down, replaced the
process_files.pl script with python.  All generated files are
placed in a new 'openssl-gen' subdirectory, no matter whenever
they are header files, C files or asm files.

Some code changes are needed for openssl 3.0, those are mostly
unchanged when comparing to my ~1y old branch.  Exceptions are
some EC-related changes.

Acceleration support has been expanded to also cover AARCH64
with GCC5.

The old openssl-1.1 apparently tries to avoid adding support
for avx for asm acceleration, by taking care that nasm is not
in the path.  That trick will surely will not work with
openssl-3.0 as openssl has learned to generate avx instructions
for other assemblers meanwhile.

Is there some specific reason for that?
Compatibility with toolchains without avx support?
Or is firmware not allowed to use avx instructions?

In case of the latter we probably have to add a 'no-avx' config option
to upstream openssl, similiar to the 'no-sse2' option which already
exists.

take care,
  Gerd

Re: [edk2-devel] [RFC] [staging/CryptoLibrary] Openssl1.1 replacement proposal

[Yao, Jiewen]

Hi Gerd
I have asked in my previous email in Feb 11, 20 - https://edk2.groups.io/g/devel/message/100040
> Hi Gerd
> If you don't mind, please submit your latest openssl-3.0 patch to the staging for broader evaluation and improvement.
Unfortunately, I do not see any response or action.

The staging tree is for POC purpose, there is no requirement to pass CI. Please don't worry about that at this moment.

With that clarification, please allow me to ask again, would you please to submit your latest work to staging, if you see something is missing?

All in all, I do hope we can work on the same tree to keep improving.

Thank you
Yao, Jiewen


> -----Original Message-----
> From: kraxel@redhat.com <kraxel@redhat.com>
> Sent: Friday, March 10, 2023 11:50 PM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: Re: [edk2-devel] [RFC] [staging/CryptoLibrary] Openssl1.1
> replacement proposal
> 
> On Fri, Mar 10, 2023 at 12:28:54PM +0000, Yao, Jiewen wrote:
> > Hello
> > We have created initial POC version CryptoPkg upgrade.
> >
> > OpenSSL 3.0 POC: https://github.com/tianocore/edk2-
> staging/blob/OpenSSL11_EOL/CryptoPkg/Readme-OpenSSL3.0.md
> > The size is reduced a lots. But it still exceeds some platforms.
> 
> I've already mentioned the branch in the cover letter of the openssl
> hash series (https://edk2.groups.io/g/devel/message/100123), but
> apparently it went unnoticed, there are lots of commits from my old
> branch in there ...
> 
> Anyway, my latest branch (just rebased to master) is here:
> 
> https://github.com/kraxel/edk2/commits/openssl3
> 
> Doesn't (yet) pass CI, most failures are on IA32 due to missing
> compiler intrinsics.
> 
> I've put the configuration system upside-down, replaced the
> process_files.pl script with python.  All generated files are
> placed in a new 'openssl-gen' subdirectory, no matter whenever
> they are header files, C files or asm files.
> 
> Some code changes are needed for openssl 3.0, those are mostly
> unchanged when comparing to my ~1y old branch.  Exceptions are
> some EC-related changes.
> 
> Acceleration support has been expanded to also cover AARCH64
> with GCC5.
> 
> The old openssl-1.1 apparently tries to avoid adding support
> for avx for asm acceleration, by taking care that nasm is not
> in the path.  That trick will surely will not work with
> openssl-3.0 as openssl has learned to generate avx instructions
> for other assemblers meanwhile.
> 
> Is there some specific reason for that?
> Compatibility with toolchains without avx support?
> Or is firmware not allowed to use avx instructions?
> 
> In case of the latter we probably have to add a 'no-avx' config option
> to upstream openssl, similiar to the 'no-sse2' option which already
> exists.
> 
> take care,
>   Gerd