Re: Unified API for Hashing Algorithms in EDK2


Michael D Kinney
 

As Sean points out, different components in a platform
may have different hash requirements.

If we want to go down the path where we have a few modules
that produce Protocols/PPIs for the BaseCryptLib so all
modules can share the same crypto services, then each
module need to decide which specific hash algorithm it
requires.

Instead of defining another protocol/PPI for a unified
hash service, we could consider a couple of other
options:

1) Each module that uses hash services could use
a FixedAtBuild PCD to decide which hash algorithm
it should use. Based on this PCD value, different
APIs in BaseCryptLib can be called. Since it is
a FixedAtBuild PCD, the compiler/linker optimizations
will only include the one hash algorithm selected.
by using BaseCryptLib, this is compatible with the
proposal to wrap BaseCryptLib in Protocols/PPIs.

2) If we have many modules that need to make a hash
algorithm selection, then define a new library
class with the unified hash APIs, and the
library instance uses a FixedAtBuild PCD to configure
the hash algorithm used. This is also compatible
with the proposal to wrap BaseCryptLib in Protocols/PPIs
and minimizes the sources in modules that need
a design that supports flexible hash algorithm selection.
If new hash algorithm is added, then only the lib
instance and PCD need to be updated.

Each module sets the FixedAtBuild PCD value in the scope
of the module in the platform DSC file. If most or all
modules in a platform need the same hash algorithm, then
the FixedAtBuild PCD can be set at the platform scope in
the platform DSC file.

Best regards,

Mike

-----Original Message-----
From: Wang, Jian J <jian.j.wang@intel.com>
Sent: Monday, November 25, 2019 9:09 PM
To: Sukerkar, Amol N <amol.n.sukerkar@intel.com>;
rfc@edk2.groups.io; sean.brogan@microsoft.com; Kinney,
Michael D <michael.d.kinney@intel.com>; Desimone,
Nathaniel L <nathaniel.l.desimone@intel.com>; Matthew
Carlson <macarl@microsoft.com>; Gao, Liming
<liming.gao@intel.com>; Feng, Bob C
<bob.c.feng@intel.com>; Lu, XiaoyuX
<xiaoyux.lu@intel.com>
Cc: Agrawal, Sachin <sachin.agrawal@intel.com>
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@intel.com>
Sent: Tuesday, November 26, 2019 5:53 AM
To: rfc@edk2.groups.io; sean.brogan@microsoft.com;
Kinney, Michael D
<michael.d.kinney@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desimone@intel.com>; Matthew Carlson
<macarl@microsoft.com>; Gao, Liming
<liming.gao@intel.com>; Feng, Bob
C <bob.c.feng@intel.com>; Wang, Jian J
<jian.j.wang@intel.com>; Lu,
XiaoyuX <xiaoyux.lu@intel.com>
Cc: Agrawal, Sachin <sachin.agrawal@intel.com>
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@intel.com>;
Kinney, Michael D <michael.d.kinney@intel.com>;
Desimone, Nathaniel L
<nathaniel.l.desimone@intel.com>; Matthew Carlson
<macarl@microsoft.com>; Gao, Liming
<liming.gao@intel.com>; Feng, Bob
C <bob.c.feng@intel.com>; Wang, Jian J
<jian.j.wang@intel.com>; Lu,
XiaoyuX <xiaoyux.lu@intel.com>
Cc: Agrawal, Sachin <sachin.agrawal@intel.com>
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@intel.com>;
Kinney, Michael D <michael.d.kinney@intel.com>;
Desimone, Nathaniel L
<nathaniel.l.desimone@intel.com>; Sean Brogan
<sean.brogan@microsoft.com>; Matthew Carlson
<macarl@microsoft.com>;
Gao, Liming <liming.gao@intel.com>; Feng, Bob C
<bob.c.feng@intel.com>; Wang, Jian J
<jian.j.wang@intel.com>; Lu,
XiaoyuX <xiaoyux.lu@intel.com>
Cc: Agrawal, Sachin <sachin.agrawal@intel.com>;
Sukerkar, Amol N
<amol.n.sukerkar@intel.com>
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@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desimone@intel.com>;
'sean.brogan@microsoft.com'
<sean.brogan@microsoft.com>; Matthew Carlson
<macarl@microsoft.com>;
Gao, Liming <liming.gao@intel.com>; Feng, Bob C
<bob.c.feng@intel.com>; Wang, Jian J
<jian.j.wang@intel.com>; Lu,
XiaoyuX <xiaoyux.lu@intel.com>
Cc: Agrawal, Sachin <sachin.agrawal@intel.com>;
Sukerkar, Amol N
<amol.n.sukerkar@intel.com>
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@intel.com>; Sukerkar, Amol N
<amol.n.sukerkar@intel.com>; Kinney, Michael D
<michael.d.kinney@intel.com>;
'sean.brogan@microsoft.com'
<sean.brogan@microsoft.com>; Matthew Carlson
<macarl@microsoft.com>;
Gao, Liming <liming.gao@intel.com>; Feng, Bob C
<bob.c.feng@intel.com>; Wang, Jian J
<jian.j.wang@intel.com>; Lu,
XiaoyuX <xiaoyux.lu@intel.com>
Cc: Agrawal, Sachin <sachin.agrawal@intel.com>
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@intel.com>
Cc: Agrawal, Sachin <sachin.agrawal@intel.com>;
Wang, Jian J
<jian.j.wang@intel.com>
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@intel.com>;
Wang, Jian J
<jian.j.wang@intel.com>; Sukerkar, Amol N
<amol.n.sukerkar@intel.com>; Sukerkar, Amol N
<amol.n.sukerkar@intel.com>
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















Join rfc@edk2.groups.io to automatically receive all group messages.