Re: [PATCH v3 0/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API

["Michael D Kinney" <michael.d.kinney@intel.com>]

Amol,

Add a PCD to CryptoPkg.

There are other CryptoPkg extensions I am working on 
that will also define a PCD.

When you build your code and disassemble, are all the
hash algorithms included even through a module only 
needs one?  The design I have in mind allows unused 
hash services to always be optimized away.

Mike

> -----Original Message-----
> From: Sukerkar, Amol N <amol.n.sukerkar@intel.com>
> Sent: Wednesday, January 15, 2020 4:48 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Agrawal, Sachin
> <sachin.agrawal@intel.com>; Musti, Srinivas
> <srinivas.musti@intel.com>; Lakkimsetti, Subash
> <subash.lakkimsetti@intel.com>; Sukerkar, Amol N
> <amol.n.sukerkar@intel.com>
> Subject: RE: [PATCH v3 0/1] SecurityPkg/BaseHashLib:
> Implement Unified Hash Calculation API
> 
> Hi Mike,
> 
> This design does not implement any registration. The
> hashing algorithm is selected from the array index
> specified by PcdSystemHashPolicy value, just like
> switch..case, based on recommendation by Jian. Are you
> referring to the document attached to Bugzilla ticket
> mentioned below? I plan to update it as soon as we
> agree on the final design. Apologies if it was
> misleading today.
> 
> The reason this lib was added to SecurityPkg and not
> CryptoPkg was done because the decision to choose
> hashing algorithm is based on PCD, PcdSystemHashPolicy.
> CryptoPkg only provides API for accessing specific
> hashing algorithm and there is no mechanism to choose,
> as there is no precedent to using a PCD in CryptoPkg
> and it does not look like that needs to change. On the
> other hand, we actually do have API support in
> SecurityPkg (HashInstanceLib). Our design provides
> similar API support, although, it is much simpler and
> does not involve registration as in HashInstanceLib. Do
> you still think this lib should be implemented in
> CryptoPkg? If yes, how do you propose the user choose
> the desired hashing mechanism?
> 
> Thanks,
> Amol
> 
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Wednesday, January 15, 2020 4:52 PM
> To: Sukerkar, Amol N <amol.n.sukerkar@intel.com>;
> devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Agrawal, Sachin
> <sachin.agrawal@intel.com>; Musti, Srinivas
> <srinivas.musti@intel.com>; Lakkimsetti, Subash
> <subash.lakkimsetti@intel.com>
> Subject: RE: [PATCH v3 0/1] SecurityPkg/BaseHashLib:
> Implement Unified Hash Calculation API
> 
> Amol,
> 
> I still think the handle based registration is too
> complex for this feature.
> 
> I recommend a simpler lib design and add it to
> CryptoPkg instead of SecurityPkg.  Providing a
> different method to access the hashing functions in
> BaseCryptLib is not a Security feature, it is a Crypto
> feature.
> 
> Thanks,
> 
> Mike
> 
> 
> > -----Original Message-----
> > From: Sukerkar, Amol N <amol.n.sukerkar@intel.com>
> > Sent: Wednesday, January 15, 2020 2:57 PM
> > To: devel@edk2.groups.io
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> Yao, Jiewen
> > <jiewen.yao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Agrawal,
> > Sachin <sachin.agrawal@intel.com>; Musti, Srinivas
> > <srinivas.musti@intel.com>; Lakkimsetti, Subash
> > <subash.lakkimsetti@intel.com>
> > Subject: [PATCH v3 0/1] SecurityPkg/BaseHashLib:
> > Implement Unified Hash Calculation API
> >
> > Currently, the UEFI drivers using the SHA/SM3 hashing
> algorithms use
> > hard-coded API to calculate the hash, for instance,
> sha_256(...), etc.
> > Since SHA384 and/or
> > SM3_256 are being increasingly adopted for
> robustness, it becomes
> > cumbersome to modify each driver that calls into hash
> calculating API.
> >
> > 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 desired hashing algorithm based on the
> required robnustness.
> >
> > Alternatively, the design document is also attached
> to Bugzilla,
> > https://bugzilla.tianocore.org/show_bug.cgi?id=2151.
> >
> > Sukerkar, Amol N (1):
> >   SecurityPkg/BaseHashLib: Implement Unified Hash
> Calculation API
> >
> >  SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c
> |
> > 151 ++++++++++++++++++++
> >  SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c
> |
> > 100 +++++++++++++
> >  SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c
> |
> > 103 +++++++++++++
> >  SecurityPkg/Include/Library/BaseHashLib.h
> |
> > 85 +++++++++++
> >  SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h
> |
> > 141 ++++++++++++++++++
> >  SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf
> |
> > 46 ++++++
> >  SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni
> |
> > 17 +++
> >  SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf
> |
> > 51 +++++++
> >  SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni
> |
> > 16 +++
> >  SecurityPkg/SecurityPkg.dec
> |
> > 23 ++-
> >  SecurityPkg/SecurityPkg.dsc
> |
> > 10 +-
> >  SecurityPkg/SecurityPkg.uni
> |
> > 15 +-
> >  12 files changed, 755 insertions(+), 3 deletions(-)
> create mode
> > 100644
> SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c
> >  create mode 100644
> > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c
> >  create mode 100644
> > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c
> >  create mode 100644
> > SecurityPkg/Include/Library/BaseHashLib.h
> >  create mode 100644
> > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h
> >  create mode 100644
> > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf
> >  create mode 100644
> > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni
> >  create mode 100644
> > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf
> >  create mode 100644
> > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni
> >
> > --
> > 2.16.2.windows.1
> 
>