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

["Wang, Jian J" <jian.j.wang@intel.com>]

Mike,

If I remember correctly, the optimization will be left to the PPI/Protocol version
of BaseCryptLib, which will be merged into edk2 code base from Mu project.

Regards,
Jian

> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Thursday, January 16, 2020 8:56 AM
> 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,
> 
> 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
> >
> >