-----Original Message-----
From: Sukerkar, Amol N <amol.n.sukerkar@...>
Sent: Tuesday, November 26, 2019 5:53 AM
To: rfc@edk2.groups.io; sean.brogan@...; 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

Hi Sean,

A few examples that we want to target are
- HDD Password verification
- Capsule Verification (Platform recovery)
- Bios Guard Platform Data Table
- More importantly, Boot Guard Event Log driver that has a bunch of redundant
code today calculating various hashes. I don't think this is open source though.

Currently, the hash verification mechanism in the above features is hard-coded
to SHA256 which we will need to change to SHA384 to support a more robust
hashing mechanism. This is a good opportunity to replace the existing SHA256
API with an abstracted unified API. And if necessary, the abstracted unified API
still supports driver specific hashing algorithm if needed (as opposed to the
strongest hashing algorithm supported by the system).

We are not targeting verification of OBB or for that matter any PE/COFF image
with this mechanism, mainly because it has it's own wrapper and verification
mechanism in DxeImageVerificationLib.

Thanks,
Amol

-----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&amp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110f
db2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1
%7C0%7C637103092953247213&amp;sdata=2acK0dmJZ85aZ3LvOOqD3e3w44V
EM5dW8TEhs9USyDM%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.c
om%2Fansukerk%2Fedk2&amp;data=02%7C01%7Csean.brogan%40microsoft.c
om%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd
011db47%7C1%7C0%7C637103092953247213&amp;sdata=keRdfU%2BxffkBsyv
YHwvSOIA8ryexOigiKhR%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.c
om%2Fmicrosoft%2Fmu_plus%2Ftree%2Fdev%2F201908%2FSharedCryptoPkg&
amp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb2a91f4e8c2
acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637
103092953247213&amp;sdata=88TRhUqsVrj7sx8P4hjbdLy2%2BDKnrBoi7HAI3G
UhpPo%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.c
om%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_PPI_Protocol_Proposal_V5&a
mp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb2a91f4e8c2a
cb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6371
03092953247213&amp;sdata=2acK0dmJZ85aZ3LvOOqD3e3w44VEM5dW8TEhs9
USyDM%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.Has
hAll | 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&amp;data=02%7C01%7Csean.b
rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988b
f86f141af91ab2d7cd011db47%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&amp;data=02%7C01%7Csean.b
rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988b
f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&amp;sdata
=oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&amp;reserved=0.

You can also provide the feedback in the Bugzilla.

Thanks,
Amol