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

["Sukerkar, Amol N" <amol.n.sukerkar@intel.com>]

Hi Mike,

I had an offline conversation with Jian and, now, I agree on the point that bitmap will not optimize the API. I will work on changing back to switch..case.

Thanks,
Amol

-----Original Message-----
From: Sukerkar, Amol N <amol.n.sukerkar@intel.com> 
Sent: Wednesday, January 15, 2020 9:26 PM
To: Wang, Jian J <jian.j.wang@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
Cc: Yao, Jiewen <jiewen.yao@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

Also, I don't have a definitive way of checking the optimization. I have only been checking size of the FVs. Can you suggest how to? Use of UefiTool does not help or at least, I don't know how to properly use it.

Thanks,
Amol

-----Original Message-----
From: Wang, Jian J <jian.j.wang@intel.com>
Sent: Wednesday, January 15, 2020 7:15 PM
To: Kinney, Michael D <michael.d.kinney@intel.com>; Sukerkar, Amol N <amol.n.sukerkar@intel.com>; devel@edk2.groups.io
Cc: Yao, Jiewen <jiewen.yao@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

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
> >
> >