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


Michael D Kinney
 

Gerd,

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

Mike

-----Original Message-----
From: Yao, Jiewen <jiewen.yao@...>
Sent: Tuesday, January 18, 2022 3:12 AM
To: kraxel@...; devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@...>; Wang, Jian J <jian.j.wang@...>; Jiang, Guomin <guomin.jiang@...>;
Pawel Polawski <ppolawsk@...>; Lu, XiaoyuX <xiaoyux.lu@...>
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@... <kraxel@...>
Sent: Monday, January 17, 2022 7:46 PM
To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@...>
Cc: Kinney, Michael D <michael.d.kinney@...>; Wang, Jian J
<jian.j.wang@...>; Jiang, Guomin <guomin.jiang@...>; Pawel
Polawski <ppolawsk@...>; Lu, XiaoyuX <xiaoyux.lu@...>
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

Join devel@edk2.groups.io to automatically receive all group messages.