-----Original Message-----
From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sean via Groups.Io
Sent: Monday, November 25, 2019 1:24 PM
To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; 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

Amol,

I am interested to hear more about the actual use cases. I like the idea of abstracting the API so that a calling driver doesn't have to change when the hashing algorithm requirements change but am not convinced that this will actually be helpful (or at least worth the introduction of more complexity in security sensitive code).

For the TPM and measured boot I agree. Random driver needs to extend a measurement then it can use the existing hashlib and it will be hashed and extended per platform policy. This is because the random driver doesn't need to understand the result. Only the TPM needs to and that is handled within the extend operation.

For the other drivers that I have seen using hashing, they need to understand the result and make driver specific decisions based on the result. For example if I am verifying the OBB or some FV then I must make sure the known good and computed match. This driver must locate the known good and thus this process generally must know the format of the result. For this I think it is best to know what algorithm I am using and I don't see a strong use case to change algorithm to an abstracted algorithm.

Do you have a set of example drivers/use cases that could use abstracted hash support?

Thanks
Sean



-----Original Message-----
From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N via Groups.Io
Sent: Monday, November 25, 2019 12:08 PM
To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; 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: [EXTERNAL] Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2

Hi Mike and Nate,

With our implementation we are trying to address the following in EDKII:
1. A common Hashing API for UEFI drivers to consume instead of the current API that directly calls into the specific hashing algorithm, for instance, SHA1_Init, SHA256_Update, etc. Due to a stronger hashing algorithm requirement, certain drivers need to be upgraded to SHA384 from SHA256 and the unified API is proposed, so, while upgrading we remove the hard-coded dependency on each hashing algorithm API.

2. The size issue as a result of statically linking openssl/cryptolib API to the consuming driver. There are two ways we tried to address this issue, although, we didn't explicitly defined a PPI to address it.
a. Introduced a fixed PCD per hashing algorithm to include/exclude a particular algorithm from BIOS.
b. Use a register mechanism to link the instance of a particular hashing algorithm to Unified API library. This is configurable using PCD.

Note that 2.a and 2.b implementation is very similar to HashLibBaseCryptoRouter used for TPM2 operations.

From the size and static linking perspective, I believe we can have a discussion between 2.a and 2.b with Mike's implementation(https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_PPI_Protocol_Proposal_V5&;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&amp;sdata=2acK0dmJZ85aZ3LvOOqD3e3w44VEM5dW8TEhs9USyDM%3D&amp;reserved=0) below.

We still need a common hashing API so the hashing algorithm strength for a particular driver can be increased without having to modify the consumer driver code.

Comments are welcome.

Thanks,
Amol

-----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://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fansukerk%2Fedk2&;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&amp;sdata=keRdfU%2BxffkBsyvYHwvSOIA8ryexOigiKhR%2B%2F1q7DjE%3D&amp;reserved=0.

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://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2Fmu_plus%2Ftree%2Fdev%2F201908%2FSharedCryptoPkg&;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&amp;sdata=88TRhUqsVrj7sx8P4hjbdLy2%2BDKnrBoi7HAI3GUhpPo%3D&amp;reserved=0

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://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_PPI_Protocol_Proposal_V5&;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&amp;sdata=2acK0dmJZ85aZ3LvOOqD3e3w44VEM5dW8TEhs9USyDM%3D&amp;reserved=0

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://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&amp;sdata=oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&amp;reserved=0

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://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&amp;sdata=oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&amp;reserved=0.
You can also provide the feedback in the Bugzilla.

Thanks,
Amol