Re: [edk2-devel] [PATCH 00/24] CryptoPkg/openssl: update openssl submodule to v3.0

["Michael D Kinney" <michael.d.kinney@intel.com>]

Gerd,

Thank you for the continued work on v3.0 support.  Comments below.

Mike

> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Tuesday, January 18, 2022 3:12 AM
> To: kraxel@redhat.com; devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
> Pawel Polawski <ppolawsk@redhat.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>
> Subject: RE: [edk2-devel] [PATCH 00/24] CryptoPkg/openssl: update openssl submodule to v3.0
> 
> Thank you!
> Good result. Comment below:
> 
> > -----Original Message-----
> > From: kraxel@redhat.com <kraxel@redhat.com>
> > Sent: Monday, January 17, 2022 7:46 PM
> > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Wang, Jian J
> > <jian.j.wang@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>; Pawel
> > Polawski <ppolawsk@redhat.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>
> > Subject: Re: [edk2-devel] [PATCH 00/24] CryptoPkg/openssl: update openssl
> > submodule to v3.0
> >
> >   Hi,
> >
> > I've continued working on this over the last weeks.  Time for a status
> > update.  All applies to the latest tree, sneak preview is here:
> > 	https://github.com/kraxel/edk2/commits/openssl3
> >
> > > Also, assuming you have done enough test, would you please provide:
> > > 1) size difference, Including PEI, SMM, DXE.
> >
> > No changes in SEC and PEI.
> [Jiewen] Do you mean the Crypto consumer in PEI has no size difference? Such as
> https://github.com/tianocore/edk2/tree/master/SecurityPkg/Tcg/Tcg2Pei ,
> https://github.com/tianocore/edk2/tree/master/SecurityPkg/FvReportPei ,
> https://github.com/tianocore/edk2/tree/master/SignedCapsulePkg/Universal/RecoveryModuleLoadPei linking
> https://github.com/tianocore/edk2/tree/master/SecurityPkg/Library/FmpAuthenticationLibRsa2048Sha256.
> 
> DXE:
> >
> > openssl 1.1
> > -  399582 SecureBootConfigDxe
> > -  472182 SecurityStubDxe
> > -  532626 VariableSmm
> > -  656382 TlsDxe
> >
> > openssl 3.0
> > +  809886 SecureBootConfigDxe
> > +  912310 SecurityStubDxe
> > +  970898 VariableSmm
> > + 1125758 TlsDxe
> >
> > Most of that seems to come from some openssl core changes (the new
> > 'provider' concept) and I don't see an easy way to cut that down.
> >
> > That is with the same feature set we have right now (i.e. no elliptic
> > curves and thus no TLS 1.3 support).
> [Jiewen] It almost doubles the size, which will becomes a big challenge for openssl3.0 adoption.
> 
> 
> >
> > > 2) performance difference, Including PEI, SMM, DXE.
> >
> > Suggestions how to measure that?
> [Jiewen] Please just write an app to call the crypto API, multiple times.
> https://github.com/tianocore/edk2/tree/master/CryptoPkg/Test/UnitTest/Library/BaseCryptLib
> I think we can focus on SHA256/RSA2048 + AES, which is used in secure boot, and HTTPS boot.
> 
> >
> > > 3) what unit test you have done (such as each crypto API)
> >
> > CryptoPkg/UnitTest passes.
> [Jiewen] Good enough.
> 
> >
> > > 4) what system test you have done (such as secure boot, trusted boot)
> >
> > Secure boot works.
> > TlsDxe (boot from https server) works.
> > TPM not tested yet.
> [Jiewen] Good enough. TPM only includes HASH. I am not too worry about that.
> 
> 
> >
> >
> > I still have a bunch of failures in CI, for some of them I'm not sure
> > how to handle them best:
> >
> > (1) 32-bit builds on windows fail:
> >
> > INFO - OpensslLibCrypto.lib(rsa_lib.obj) : error LNK2001: unresolved external
> > symbol __allmul
> > INFO - OpensslLibCrypto.lib(rsa_lib.obj) : error LNK2001: unresolved external
> > symbol __aulldiv
> > INFO - OpensslLibCrypto.lib(bio_print.obj) : error LNK2001: unresolved external
> > symbol __aulldvrm
> > INFO - OpensslLibCrypto.lib(bio_print.obj) : error LNK2001: unresolved external
> > symbol __ftol2_sse

We need to see if there are any OpenSSL config settings to completely remove use of
float/double types.  UEFI envs do not support float/double.  It is possible to 
use them in a UEFI App or other UEFI FW components, but the use of those need
to do extra work to disable interrupts and save/restore state.

> >
> > Those symbols look like they reference helper functions to do 64bit math
> > on 32bit architecture.  Any hints how to fix that?
> [Jiewen] Please add them to https://github.com/tianocore/edk2/tree/master/CryptoPkg/Library/IntrinsicLib
> 
> >
> >
> > (2) va_arg is not working with floats due to SEE being disabled:
> >
> > INFO -
> > /home/vsts/work/1/s/CryptoPkg/Library/OpensslLib/openssl/crypto/bio/bio_pri
> > nt.c:265:28: error: SSE register argument with SSE disabled
> > INFO -                      fvalue = va_arg(args, LDOUBLE);
> >
> > I can't see a way to fix that given that va_arg typically refers to a
> > compiler builtin so I don't think there is a way to declare that a
> > EFIAPI function to change the calling convention.  Not all builds fail
> > though, possibly because the compiler inlines with optimization turned
> > on.
> >
> > Suggestions anyone?
> [Jiewen] This seems infrastructure issue.
> Any suggestion, Mike ?

As mentioned above, it would be better if OpenSSL had a config setting to 
not use any float/double types.

> 
> 
> >
> >
> > (3) Some NOOPT builds are failing due to the size growing ...
> [Jiewen] Size becomes big challenge...
> Have you tried to use https://github.com/tianocore/edk2/tree/master/CryptoPkg/Driver solution?
> 
> 
> >
> >
> > take care,
> >   Gerd