-----Original Message-----
From: Wang, Jian J <jian.j.wang@...>
Sent: Monday, November 25, 2019 9:09 PM
To: Sukerkar, Amol N <amol.n.sukerkar@...>;
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@...>; Lu, XiaoyuX
<xiaoyux.lu@...>
Cc: Agrawal, Sachin <sachin.agrawal@...>
Subject: RE: [edk2-rfc] Unified API for Hashing
Algorithms in EDK2

Hi Sean and Amol,

I believe the OBB and FV/PE image verification can
still benefit from the unified API, if we add one more
parameter to HashApiIinit() to force using a hash
algorithm or add a PCD to choose one or link only
required HashApiInstanceXxx.

(Just for example)
HashApiInit (
IN EFI_GUID *HashAlg,
OUT HASH_HANDLE *HashHandle
)

In this way, we can avoid hard-code hash API in the
verification module. Although it's slow, I think the
OBB verification or FV/PE image signature algorithm is
still subject to change per security requirement
change. During a transition period, we don't have to
add a new code to handle a different algorithm in
verification module. Instead we just need to link a
different hash lib or set PCD to use a different
algorithm. I think this makes things simpler and easier
to change.

The BaseCryptLib implemented in PPI/Protocol is a good
idea to save image size. I think the Unified hash API
(BaseHashLib) can make use of it to save extra hash
algorithm library instances (HashApiInstanceSha1,
Sha256,...).
The only problem is that the structured PCD, like Mike
mentioned before, doesn't support module override,
which prevents different modules from using different
algorithms and then cannot save module size utter
mostly.


Regards,
Jian

-----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.outlo
ok.com/?url=htt

ps%3A

%2F%2Fgithub.com%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_P
PI_Protocol_

Proposal_V5&amp;data=02%7C01%7Csean.brogan%40microsoft.
com%7Cd110f

db2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd
011db47%7C1

%7C0%7C637103092953247213&amp;sdata=2acK0dmJZ85aZ3LvOOq
D3e3w44V

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=htt
ps%3A%2F%2Fgith

ub.c

om%2Fansukerk%2Fedk2&amp;data=02%7C01%7Csean.brogan%40m
icrosoft.c

om%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141a
f91ab2d7cd

011db47%7C1%7C0%7C637103092953247213&amp;sdata=keRdfU%2
BxffkBsyv

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=htt
ps%3A%2F%2Fgith

ub.c

om%2Fmicrosoft%2Fmu_plus%2Ftree%2Fdev%2F201908%2FShared
CryptoPkg&

amp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fd
b2a91f4e8c2

acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%
7C0%7C637

103092953247213&amp;sdata=88TRhUqsVrj7sx8P4hjbdLy2%2BDK
nrBoi7HAI3G

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=htt
ps%3A%2F%2Fgith

ub.c

om%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_PPI_Protocol_Pr
oposal_V5&a

mp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb
2a91f4e8c2a

cb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7
C0%7C6371

03092953247213&amp;sdata=2acK0dmJZ85aZ3LvOOqD3e3w44VEM5
dW8TEhs9

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.PcdCryptoServiceFamilyEnabl
e.Sha1.Family

| PCD_CRYPTO_SERVICE_ENABLE_FAMILY

gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl
e.Sha1.Services.Has

hAll | FALSE

gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl
e.Sha256.Family

| PCD_CRYPTO_SERVICE_ENABLE_FAMILY

gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl
e.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=htt
ps%3A%2F%2Fbugzilla.

tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&amp;data=02%7C
01%7Csean.b

rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e3339
0%7C72f988b

f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&
amp;sdata

=oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&amp;r
eserved=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=htt
ps%3A%2F%2Fbugzilla.

tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&amp;data=02%7C
01%7Csean.b

rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e3339
0%7C72f988b

f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&
amp;sdata

=oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&amp;r
eserved=0.

You can also provide the feedback in the Bugzilla.

Thanks,
Amol