Re: [edk2-devel] [PATCH 0/1] CryptoPkg/OpensslLib: Add native instruction support for IA32 and X64

["Ard Biesheuvel" <ard.biesheuvel@linaro.org>]

On Thu, 26 Mar 2020 at 02:04, Zurcher, Christopher J
<christopher.j.zurcher@intel.com> wrote:
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard Biesheuvel
> > Sent: Wednesday, March 25, 2020 11:40
> > To: Zurcher, Christopher J <christopher.j.zurcher@intel.com>
> > Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Wang, Jian J
> > <jian.j.wang@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Eugene Cohen
> > <eugene@hp.com>
> > Subject: Re: [edk2-devel] [PATCH 0/1] CryptoPkg/OpensslLib: Add native
> > instruction support for IA32 and X64
> >
> > On Tue, 17 Mar 2020 at 11:26, Christopher J Zurcher
> > <christopher.j.zurcher@intel.com> wrote:
> > >
> > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2507
> > >
> > > This patch adds support for building the native instruction algorithms for
> > > IA32 and X64 versions of OpensslLib. The process_files.pl script was
> > modified
> > > to parse the .asm file targets from the OpenSSL build config data struct,
> > and
> > > generate the necessary assembly files for the EDK2 build environment.
> > >
> > > For the X64 variant, OpenSSL includes calls to a Windows error handling
> > API,
> > > and that function has been stubbed out in ApiHooks.c.
> > >
> > > For all variants, a constructor was added to call the required CPUID
> > function
> > > within OpenSSL to facilitate processor capability checks in the native
> > > algorithms.
> > >
> > > Additional native architecture variants should be simple to add by
> > following
> > > the changes made for these two architectures.
> > >
> > > The OpenSSL assembly files are traditionally generated at build time using
> > a
> > > perl script. To avoid that burden on EDK2 users, these end-result assembly
> > > files are generated during the configuration steps performed by the package
> > > maintainer (through process_files.pl). The perl generator scripts inside
> > > OpenSSL do not parse file comments as they are only meant to create
> > > intermediate build files, so process_files.pl contains additional hooks to
> > > preserve the copyright headers as well as clean up tabs and line endings to
> > > comply with EDK2 coding standards. The resulting file headers align with
> > > the generated .h files which are already included in the EDK2 repository.
> > >
> > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
> > > Cc: Eugene Cohen <eugene@hp.com>
> > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > >
> > > Christopher J Zurcher (1):
> > >   CryptoPkg/OpensslLib: Add native instruction support for IA32 and X64
> > >
> >
> > Hello Christopher,
> >
> > It would be helpful to understand the purpose of all this. Also, I
> > think we could be more picky about which algorithms we enable - DES
> > and MD5 don't seem highly useful, and even if they were, what do we
> > gain by using a faster (or smaller?) implementation?
> >
>
> The selection of algorithms comes from the default OpenSSL assembly targets; this combination is validated and widely used, and I don't know all the consequences of picking and choosing which ones to include. If necessary I could look into reducing the list.
>
> The primary driver for this change is enabling the Full Flash Update (FFU) OS provisioning flow for Windows as described here:
> https://docs.microsoft.com/en-us/windows-hardware/manufacture/desktop/wim-vs-ffu-image-file-formats
> This item under "Reliability" is what we are speeding up: "Includes a catalog and hash table to validate a signature upfront before flashing onto a device. The hash table is generated during capture, and validated when applying the image."
> This provisioning flow can be performed within the UEFI environment, and the native algorithms allow significant time savings in a factory setting (minutes per device).
>
> We also had a BZ which requested these changes and the specific need was not provided. Maybe David @HP can provide further insight?
> https://bugzilla.tianocore.org/show_bug.cgi?id=2507
>
> There have been additional platform-specific benefits identified, for example speeding up HMAC authentication of communication with other HW/FW components.
>

OK, so in summary, there is one particular hash that you need to speed
up, and this is why you enable every single asm implementation in
OpenSSL for X86. I'm not sure I am convinced that this is justified.

OpenSSL can easily deal with having just a couple of these accelerated
implementations enabled. To me, it seems like a more sensible approach
to only enable algorithms based on special instructions (such as
AES-NI and SHA-NI), which typically give a speedup in the 10x-20x
range, and to leave the other ones disabled. E.g., accelerated
montgomery multiplication for RSA is really not worth it, since the
speedup is around 50% and RSA is hardly a bottleneck in UEFI. Same
applies to deprecated ciphers such as DES or MD5 - they are not based
on special instructions, and shouldn't be used in the first place.