-----Original Message-----
From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N
Sent: Thursday, November 21, 2019 4:45 PM
To: rfc@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...>
Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...>
Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2

Thanks, Nate and Mike!

I am going through the code and comments and will respond shortly.

In the meantime, here is the GitHub link to my PoC for the community to look at and comment: https://github.com/ansukerk/edk2.

Best regards,
Amol

-----Original Message-----
From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Michael D Kinney
Sent: Thursday, November 21, 2019 4:37 PM
To: rfc@edk2.groups.io; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...>
Cc: Agrawal, Sachin <sachin.agrawal@...>
Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2

Nate and Amol,

There is some work already started by Sean and Matt that implements a PEIM, DXE Driver, and SMM Driver to produce Protocol/PPI that wraps the BaseCryptLib services. This content broken out into its own package is available here:

https://github.com/microsoft/mu_plus/tree/dev/201908/SharedCryptoPkg

I have ported and simplified this content into a proposed set of patches to the CryptoPkg. It uses a structured PCD to configure the services mapped into the Protocols/PPIs and avoids the issue Nate notes below with protocols and PPIs including all of the BaseCryptLib services. The structured PCD allows families of crypto services or individual services within a family to be enabled/disabled.

https://github.com/mdkinney/edk2/tree/CryptoPkg_PPI_Protocol_Proposal_V5

For example, the DSC file PCD statements to enable the SHA1 family and
SHA256 family of hash services with the HashAll service disabled is:

[PcdsFixedAtBuild]
gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha1.Family | PCD_CRYPTO_SERVICE_ENABLE_FAMILY
gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha1.Services.HashAll | FALSE
gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha256.Family | PCD_CRYPTO_SERVICE_ENABLE_FAMILY
gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha256.Services.HashAll | FALSE

Please take a look at this proposal and let me know if this can be used to address.

https://bugzilla.tianocore.org/show_bug.cgi?id=2151

There is currently a limitation in the structured PCD feature that does not allow the structured PCD field values to be set in the scope of a module in a <PcdsFixedAtBuild> section. To work around this limitation, the CryptoPkg DSC file has a define called CRYPTO_SERVICES that can be set to ALL, NONE, MIN_PEI, or MIN_DXE_MIN_SMM. The default is ALL. Building with each of these values will build the modules with different sets of enabled services that matches the services enabled using multiple modules in the work from Sean and Matt. If this limitation is addressed in BaseTools, then CryptoPkg could remove the CRYPTO_SERVIES define and all !if statements.

Thanks,

Mike

-----Original Message-----
From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Nate
DeSimone
Sent: Wednesday, November 20, 2019 10:31 PM
To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>
Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J
<jian.j.wang@...>
Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2

Hi Amol,

With regard to verifying code hashes, it makes much more sense to me
to have a GUID defined that represents the hash function to use versus
a PCD. The reason for this is the method for signing Firmware Volumes
typically involves using nested FVs where the GUID for the nested FV
type indicates the algorithm needed to "extract" the nested FV.
Extraction in this context is used to verify code hashes as well as
decompress compressed code. The "extractor" is usually a statically
linked library attached to either SignedSectionExtractPei and/or
DxeIpl.

For example, it is very possible for a platform to use more than one
hashing algorithm, I can think of more than one closed source platform
at Intel that actually does this.

Now, one thing that I think your document implies is that we should
have better APIs for crypto operations in general. If that was the
intent then I totally agree with you. There are several reasons that I
think this should be done. First, the API for OpenSSL has a tendency
to change often enough to make integrating new versions into older
codebases a headache, putting an abstraction layer on it would be
helpful. Second, the method used to consume OpenSSL in edk2 causes
some technical issues. Specifically, OpenSSL is statically linked and
it defines a crazy number of global variables, enough so that on a
debug build I have noticed roughly a 20KB increase in binary size even
if you don't call a single function in OpenSSL. Wrapping the crypto
operations in a PPI/Protocol so that only 1 driver needs to link to
OpenSSL would probably help reduce binary size for most
implementations. Your current document does not define a PPI/Protocol,
that is something I would recommend.

One issue that prior attempts of wrapping OpenSSL in a PPI/Protocol
have run into is that all the crypto functions need to be linked, even
the ones that a platform does not use, which results in a net negative
binary size impact. Accordingly, one would likely need to break the
crypto functionality into multiple PPIs/Protocols so that only the
functions a platform needs are actually included. I think some
experiments need to be done to figure out the most optimal division of
crypto APIs.

Hope that helps,
Nate

-----Original Message-----
From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar,
Amol N
Sent: Wednesday, November 20, 2019 8:28 AM
To: rfc@edk2.groups.io
Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J
<jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>;
Sukerkar, Amol N <amol.n.sukerkar@...>
Subject: [edk2-rfc] Unified API for Hashing Algorithms in EDK2
Importance: High

Hello,

Currently the UEFI drivers using the SHA/SM3 hashing algorithms use
hard-coded API to calculate the hash, such as, sha_256(...), etc.
Since SHA384 and/or SM3 are being increasingly adopted, it becomes
cumbersome to modify the driver with SHA384 or SM3 calls for each
application.

To better achieve this, we are proposing a unified API which can be
used by UEFI drivers that provides the drivers with flexibility to use
the hashing algorithm they desired or the strongest hashing algorithm
the system supports (with openssl). Attached is the design proposal
for the same and we request feedback from the community before we
begin the process of making the changes to EDK2 repo.

Alternatively, the design document is also attached to Bugzilla,
https://bugzilla.tianocore.org/show_bug.cgi?id=2151.
You can also provide the feedback in the Bugzilla.

Thanks,
Amol