Re: [edk2-devel] [PATCH v2 2/2] CryptoPkg/OpensslLib: Commit the auto-generated assembly files for X64

["Laszlo Ersek" <lersek@redhat.com>]

Hi,

On 08/18/20 23:33, Sean wrote:
> Mike,
> 
> I am not technically a basetool maintainer but as an active user/dev in
> basetools, i would be opposed to bringing in perl as an edk2 dependency.
> Also introducing another language is counter to the goal of aligning on
> python and improving the python used within edk2.  From my perspective
> the openssl config case isn't strong enough to counter the above goal.
> In fact as you know we are trying to change the paradigm for
> Crypto/OpenSSL with the Crypto Driver
> (https://github.com/tianocore/edk2/tree/master/CryptoPkg/Driver) and
> BaseCryptLibOnProtocolPpi
> (https://github.com/tianocore/edk2/tree/master/CryptoPkg/Library/BaseCryptLibOnProtocolPpi)
> work so that everyday development doesn't need to compile openssl in
> their edk2 builds.

Here I'd only like to comment on this one aspect (= build OpenSSL as a
statically linked library vs. as a crypto service driver / PEIM).

Recently I tried to evaluate the crypto driver for OVMF. I started with
the PEI phase. The configuration interface (the PCD) is baroque, *BUT*
that is a direct consequence of OpenSSL offering a huge range of
interfaces. So no complaints about the config interface.

I also reviewed the CryptoPkg.dsc "pre-sets" (i.e., CRYPTO_SERVICES
being one of PACKAGE / ALL / NONE / MIN_PEI / MIN_DXE_MIN_SMM). I had
two problems:

- These pre-sets are supremely suitable for a platform that is composed
of multiple build runs; that is, build the crypto PEIM, build the DXE /
SMM protocol drivers, package up the resultant binaries, and *then*
build the actual platforms (which will then include the crypto service
drivers in *binary* form). On the other hand, the pre-sets are not
useful to a platform that is supposed to be built in a single-shot.
Importantly, I'm not saying that the pre-sets are *detrimental* to such
platforms -- they aren't. It's just that the pre-sets target a different
use case.

- The other problem I had was the one that we had discussed when the
crypto service driver was being introduced. Namely, selecting the
OpenSSL interfaces (interface families) that the platform actually consumes.

Now, I carefully tracked down the modules in OVMF that needed crypto
support, by *not* resolving SmmCryptLib, RuntimeCryptLib, TlsLib in
general [LibraryClasses] sections in the OVMF DSC files. Then I re-added
those lib-class resolutions as module-scoped <LibraryClasses> overrides
to the actual modules that needed them.

However, I didn't know how to even *begin* evaluating the specific "API
needs" of the modules identified thusly. On a Windows or Linux OS, when
you have a dynamically linked executable, and it doesn't find a symbol
in a shared library, you get a nice error message, and the application
doesn't start. On the other hand, if a crypto protocol call fails in SMM
because we missed a feature bit in the config PCD, the results are
somewhat less user-friendly.

The expression "minimum required services" in CryptoPkg.dsc seems
relevant, but it didn't convince me that it would cover everything
needed by -- for example -- VariableSmm, VariableRuntimeDxe, and TlsDxe.

So, given that I couldn't construct a "tight profile", I started my
investigation (for OVMF's PEI phase) by including the crypto service
PEIM with *all* interfaces enabled.

This would be restricted to "TPM_ENABLE", because only that is when
OVMF's PEI phase needs crypto -- due to including the various TPM1 and
TPM2 PEIMs.

So basically this check would replace the statically linked -- and
accordingly trimmed! -- "thick" OpenSSL library copies in the TPM1/PTM2
PEIMs, with the thin wrapper lib
(BaseCryptLibOnProtocolPpi/PeiCryptLib.inf) *plus* the full-blown crypto
service PEIM.

The result was a *violent* size explosion in PEIFV; at least in the
NOOPT build. Before:

> PEIFV [64%Full] 917504 total, 592456 used, 325048 free

after:

>   the required fv image size 0x132968 exceeds the set fv image size
> 0xe0000

The PEIFV footprint more than doubled, from 592,456 bytes to 1,255,784
bytes.

I gave up there. Until the "crypto profile" construction is not
automated for platforms, somehow, I don't know how I could maintain OVMF
consuming the crypto service PEIMs/drivers.

(I wonder if we should maintain a "required crypto services" bitmap for
each individual PEIM / DXE / SMM driver inside edk2. And then, when a
platform includes any one such PEIM or driver, they'd know to "OR" the
bitmap for that particular module into their platform PCD setting.)

> So I support leaving it as is which means if you have to change
> something in openssl config you deal with it and a special one off.

(OK, I guess I can comment on this too, after all.)

I agree.

While perl is readily available on Linux build hosts, I remember:
- accommodating the python3 BaseTools requirements on my RHEL7 laptop,
- (almost) bumping the NASM version so we could compile the VMGEXIT
instruction for IA32,
- the python virtual environment discussions for running CI locally.

So I agree that new build dependencies should be avoided as much as
possible.

Thanks
Laszlo