* [PATCH v2 0/2] Enhancement and Fixes to BaseHashApiLib @ 2020-02-14 18:05 Sukerkar, Amol N 2020-02-14 18:05 ` [PATCH v2 1/2] CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with TPM 2.0 Implementation Sukerkar, Amol N 2020-02-14 18:05 ` [PATCH v2 2/2] CryptoPkg/BaseHashApiLib: Change PcdHashApiLibPolicy type to FixedAtBuild Sukerkar, Amol N 0 siblings, 2 replies; 7+ messages in thread From: Sukerkar, Amol N @ 2020-02-14 18:05 UTC (permalink / raw) To: devel; +Cc: michael.d.kinney, jiewen.yao, jian.j.wang, sachin.agrawal, liming.gao This patch implements the fixes and enhancement to BaseHashApiLib in the following manner: - Remove reference to MD4 and MD5 hashing algorithms as they are deprecated; - Align the enumeration for hashing algorithmswith the one used in TPM 2.0 implementation defined in IndustryStandard/Tpm20.h; - Change the type of PcdHashApiLibPolicy to PcdsFixedAtBuild to optimize away the unused hashing algorithms for a particular instance of HashApiLib. More information can be found at Bugzilla ticket, https://bugzilla.tianocore.org/show_bug.cgi?id=2511. Amol N Sukerkar (2): CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with TPM 2.0 Implementation CryptoPkg/BaseHashApiLib: Change PcdHashApiLibPolicy type to FixedAtBuild CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c | 120 ++++++-------------- CryptoPkg/CryptoPkg.dec | 18 ++- CryptoPkg/CryptoPkg.uni | 12 +- CryptoPkg/Include/Library/HashApiLib.h | 14 +-- 4 files changed, 51 insertions(+), 113 deletions(-) -- 2.16.2.windows.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with TPM 2.0 Implementation 2020-02-14 18:05 [PATCH v2 0/2] Enhancement and Fixes to BaseHashApiLib Sukerkar, Amol N @ 2020-02-14 18:05 ` Sukerkar, Amol N 2020-02-14 22:29 ` Michael D Kinney 2020-02-14 18:05 ` [PATCH v2 2/2] CryptoPkg/BaseHashApiLib: Change PcdHashApiLibPolicy type to FixedAtBuild Sukerkar, Amol N 1 sibling, 1 reply; 7+ messages in thread From: Sukerkar, Amol N @ 2020-02-14 18:05 UTC (permalink / raw) To: devel; +Cc: michael.d.kinney, jiewen.yao, jian.j.wang, sachin.agrawal, liming.gao Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2511 This commit aligns the baseHashApiLib with TPM 2.0 Implementation as follows: - Remove reference to MD4 and MD5 algorithms as they are deprecated - Align the enumerations for hashing algoerithms with the one used in TPM 2.0 implementation defined in IndustryStandard/Tpm20.h. Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Signed-off-by: Amol N Sukerkar <amol.n.sukerkar@intel.com> --- Notes: v2 - Fixed closed parentheses in commit message CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c | 120 ++++++-------------- CryptoPkg/CryptoPkg.dec | 16 ++- CryptoPkg/CryptoPkg.uni | 12 +- CryptoPkg/Include/Library/HashApiLib.h | 14 +-- 4 files changed, 50 insertions(+), 112 deletions(-) diff --git a/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c b/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c index 277ef9f0b421..b87a82b06ce1 100644 --- a/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c +++ b/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c @@ -31,32 +31,24 @@ HashApiGetContextSize ( VOID ) { - switch (PcdGet8 (PcdHashApiLibPolicy)) { - case HASH_API_ALGO_MD4: - return Md4GetContextSize (); - break; - - case HASH_API_ALGO_MD5: - return Md5GetContextSize (); - break; - - case HASH_API_ALGO_SHA1: + switch (PcdGet16 (PcdHashApiLibPolicy)) { + case HASH_ALG_SHA1: return Sha1GetContextSize (); break; - case HASH_API_ALGO_SHA256: + case HASH_ALG_SHA256: return Sha256GetContextSize (); break; - case HASH_API_ALGO_SHA384: + case HASH_ALG_SHA384: return Sha384GetContextSize (); break; - case HASH_API_ALGO_SHA512: + case HASH_ALG_SHA512: return Sha512GetContextSize (); break; - case HASH_API_ALGO_SM3_256: + case HASH_ALG_SM3_256: return Sm3GetContextSize (); break; @@ -81,32 +73,24 @@ HashApiInit ( OUT HASH_API_CONTEXT HashContext ) { - switch (PcdGet8 (PcdHashApiLibPolicy)) { - case HASH_API_ALGO_MD4: - return Md4Init (HashContext); - break; - - case HASH_API_ALGO_MD5: - return Md5Init (HashContext); - break; - - case HASH_API_ALGO_SHA1: + switch (PcdGet16 (PcdHashApiLibPolicy)) { + case HASH_ALG_SHA1: return Sha1Init (HashContext); break; - case HASH_API_ALGO_SHA256: + case HASH_ALG_SHA256: return Sha256Init (HashContext); break; - case HASH_API_ALGO_SHA384: + case HASH_ALG_SHA384: return Sha384Init (HashContext); break; - case HASH_API_ALGO_SHA512: + case HASH_ALG_SHA512: return Sha512Init (HashContext); break; - case HASH_API_ALGO_SM3_256: + case HASH_ALG_SM3_256: return Sm3Init (HashContext); break; @@ -133,32 +117,24 @@ HashApiDuplicate ( OUT HASH_API_CONTEXT NewHashContext ) { - switch (PcdGet8 (PcdHashApiLibPolicy)) { - case HASH_API_ALGO_MD4: - return Md4Duplicate (HashContext, NewHashContext); - break; - - case HASH_API_ALGO_MD5: - return Md5Duplicate (HashContext, NewHashContext); - break; - - case HASH_API_ALGO_SHA1: + switch (PcdGet16 (PcdHashApiLibPolicy)) { + case HASH_ALG_SHA1: return Sha1Duplicate (HashContext, NewHashContext); break; - case HASH_API_ALGO_SHA256: + case HASH_ALG_SHA256: return Sha256Duplicate (HashContext, NewHashContext); break; - case HASH_API_ALGO_SHA384: + case HASH_ALG_SHA384: return Sha384Duplicate (HashContext, NewHashContext); break; - case HASH_API_ALGO_SHA512: + case HASH_ALG_SHA512: return Sha512Duplicate (HashContext, NewHashContext); break; - case HASH_API_ALGO_SM3_256: + case HASH_ALG_SM3_256: return Sm3Duplicate (HashContext, NewHashContext); break; @@ -187,32 +163,24 @@ HashApiUpdate ( IN UINTN DataToHashLen ) { - switch (PcdGet8 (PcdHashApiLibPolicy)) { - case HASH_API_ALGO_MD4: - return Md4Update (HashContext, DataToHash, DataToHashLen); - break; - - case HASH_API_ALGO_MD5: - return Md5Update (HashContext, DataToHash, DataToHashLen); - break; - - case HASH_API_ALGO_SHA1: + switch (PcdGet16 (PcdHashApiLibPolicy)) { + case HASH_ALG_SHA1: return Sha1Update (HashContext, DataToHash, DataToHashLen); break; - case HASH_API_ALGO_SHA256: + case HASH_ALG_SHA256: return Sha256Update (HashContext, DataToHash, DataToHashLen); break; - case HASH_API_ALGO_SHA384: + case HASH_ALG_SHA384: return Sha384Update (HashContext, DataToHash, DataToHashLen); break; - case HASH_API_ALGO_SHA512: + case HASH_ALG_SHA512: return Sha512Update (HashContext, DataToHash, DataToHashLen); break; - case HASH_API_ALGO_SM3_256: + case HASH_ALG_SM3_256: return Sm3Update (HashContext, DataToHash, DataToHashLen); break; @@ -239,32 +207,24 @@ HashApiFinal ( OUT UINT8 *Digest ) { - switch (PcdGet8 (PcdHashApiLibPolicy)) { - case HASH_API_ALGO_MD4: - return Md4Final (HashContext, Digest); - break; - - case HASH_API_ALGO_MD5: - return Md5Final (HashContext, Digest); - break; - - case HASH_API_ALGO_SHA1: + switch (PcdGet16 (PcdHashApiLibPolicy)) { + case HASH_ALG_SHA1: return Sha1Final (HashContext, Digest); break; - case HASH_API_ALGO_SHA256: + case HASH_ALG_SHA256: return Sha256Final (HashContext, Digest); break; - case HASH_API_ALGO_SHA384: + case HASH_ALG_SHA384: return Sha384Final (HashContext, Digest); break; - case HASH_API_ALGO_SHA512: + case HASH_ALG_SHA512: return Sha512Final (HashContext, Digest); break; - case HASH_API_ALGO_SM3_256: + case HASH_ALG_SM3_256: return Sm3Final (HashContext, Digest); break; @@ -293,32 +253,24 @@ HashApiHashAll ( OUT UINT8 *Digest ) { - switch (PcdGet8 (PcdHashApiLibPolicy)) { - case HASH_API_ALGO_MD4: - return Md4HashAll (DataToHash, DataToHashLen, Digest); - break; - - case HASH_API_ALGO_MD5: - return Md5HashAll (DataToHash, DataToHashLen, Digest); - break; - - case HASH_API_ALGO_SHA1: + switch (PcdGet16 (PcdHashApiLibPolicy)) { + case HASH_ALG_SHA1: return Sha1HashAll (DataToHash, DataToHashLen, Digest); break; - case HASH_API_ALGO_SHA256: + case HASH_ALG_SHA256: return Sha256HashAll (DataToHash, DataToHashLen, Digest); break; - case HASH_API_ALGO_SHA384: + case HASH_ALG_SHA384: return Sha384HashAll (DataToHash, DataToHashLen, Digest); break; - case HASH_API_ALGO_SHA512: + case HASH_ALG_SHA512: return Sha512HashAll (DataToHash, DataToHashLen, Digest); break; - case HASH_API_ALGO_SM3_256: + case HASH_ALG_SM3_256: return Sm3HashAll (DataToHash, DataToHashLen, Digest); break; diff --git a/CryptoPkg/CryptoPkg.dec b/CryptoPkg/CryptoPkg.dec index 8bd63a76dd22..f185bcc82515 100644 --- a/CryptoPkg/CryptoPkg.dec +++ b/CryptoPkg/CryptoPkg.dec @@ -74,16 +74,14 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] # Based on the value set, the required algorithm is chosen to calculate # the hash of data.<BR> # The default hashing algorithm for BaseHashApiLib is set to SHA256.<BR> - # 0x00000001 - MD4.<BR> - # 0x00000002 - MD5.<BR> - # 0x00000003 - SHA1.<BR> - # 0x00000004 - SHA256.<BR> - # 0x00000005 - SHA384.<BR> - # 0x00000006 - SHA512.<BR> - # 0x00000007 - SM3_256.<BR> + # 0x00000001 - SHA1.<BR> + # 0x00000002 - SHA256.<BR> + # 0x00000004 - SHA384.<BR> + # 0x00000008 - SHA512.<BR> + # 0x00000010 - SM3_256.<BR> # @Prompt Set policy for hashing unsigned image for Secure Boot. - # @ValidRange 0x80000001 | 0x00000001 - 0x00000007 - gEfiCryptoPkgTokenSpaceGuid.PcdHashApiLibPolicy|0x04|UINT8|0x00000001 + # @ValidRange 0x80000001 | 0x00000001 - 0x00000010 + gEfiCryptoPkgTokenSpaceGuid.PcdHashApiLibPolicy|0x02|UINT16|0x00000001 [UserExtensions.TianoCore."ExtraFiles"] CryptoPkgExtra.uni diff --git a/CryptoPkg/CryptoPkg.uni b/CryptoPkg/CryptoPkg.uni index 2222762f42ee..7e97ac7af8b7 100644 --- a/CryptoPkg/CryptoPkg.uni +++ b/CryptoPkg/CryptoPkg.uni @@ -21,13 +21,11 @@ "Based on the value set, the required algorithm is chosen to calculate\n" "the hash of data.<BR>\n" "The default hashing algorithm for BaseHashApiLib is set to SHA256.<BR>\n" - "0x00000001 - MD4.<BR>\n" - "0x00000002 - MD5.<BR>\n" - "0x00000003 - SHA1.<BR>\n" - "0x00000004 - SHA256.<BR>\n" - "0x00000005 - SHA384.<BR>\n" - "0x00000006 - SHA512.<BR>\n" - "0x00000007 - SM3.<BR>" + "0x00000001 - SHA1.<BR>\n" + "0x00000002 - SHA256.<BR>\n" + "0x00000004 - SHA384.<BR>\n" + "0x00000008 - SHA512.<BR>\n" + "0x00000010 - SM3.<BR>" #string STR_gEfiCryptoPkgTokenSpaceGuid_PcdCryptoServiceFamilyEnable_PROMPT #language en-US "Enable/Disable EDK II Crypto Protocol/PPI services" diff --git a/CryptoPkg/Include/Library/HashApiLib.h b/CryptoPkg/Include/Library/HashApiLib.h index 22068e5a1756..b8b52ae15bd9 100644 --- a/CryptoPkg/Include/Library/HashApiLib.h +++ b/CryptoPkg/Include/Library/HashApiLib.h @@ -12,20 +12,10 @@ #ifndef __BASEHASHAPILIB_H_ #define __BASEHASHAPILIB_H_ +#include <IndustryStandard/Tpm20.h> + typedef VOID *HASH_API_CONTEXT; -// -// Hash Algorithms -// -#define HASH_API_ALGO_INVALID 0x00000000 -#define HASH_API_ALGO_MD4 0x00000001 -#define HASH_API_ALGO_MD5 0x00000002 -#define HASH_API_ALGO_SHA1 0x00000003 -#define HASH_API_ALGO_SHA256 0x00000004 -#define HASH_API_ALGO_SHA384 0x00000005 -#define HASH_API_ALGO_SHA512 0x00000006 -#define HASH_API_ALGO_SM3_256 0x00000007 - /** Retrieves the size, in bytes, of the context buffer required for hash operations. -- 2.16.2.windows.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with TPM 2.0 Implementation 2020-02-14 18:05 ` [PATCH v2 1/2] CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with TPM 2.0 Implementation Sukerkar, Amol N @ 2020-02-14 22:29 ` Michael D Kinney 2020-02-14 22:38 ` Michael D Kinney 0 siblings, 1 reply; 7+ messages in thread From: Michael D Kinney @ 2020-02-14 22:29 UTC (permalink / raw) To: Sukerkar, Amol N, devel@edk2.groups.io, Kinney, Michael D Cc: Yao, Jiewen, Wang, Jian J, Agrawal, Sachin, Gao, Liming Amol, Comments included below. Mike > -----Original Message----- > From: Sukerkar, Amol N <amol.n.sukerkar@intel.com> > Sent: Friday, February 14, 2020 10:06 AM > 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>; Gao, Liming > <liming.gao@intel.com> > Subject: [PATCH v2 1/2] CryptoPkg/BaseHashApiLib: Align > BaseHashApiLib with TPM 2.0 Implementation > > Ref: > https://bugzilla.tianocore.org/show_bug.cgi?id=2511 > > This commit aligns the baseHashApiLib with TPM 2.0 > Implementation > as follows: > - Remove reference to MD4 and MD5 algorithms as they > are deprecated > - Align the enumerations for hashing algoerithms with > the one used > in TPM 2.0 implementation defined in > IndustryStandard/Tpm20.h. > > Cc: Jiewen Yao <jiewen.yao@intel.com> > Cc: Jian J Wang <jian.j.wang@intel.com> > Cc: Michael D Kinney <michael.d.kinney@intel.com> > Signed-off-by: Amol N Sukerkar > <amol.n.sukerkar@intel.com> > --- > > Notes: > v2 > - Fixed closed parentheses in commit message > > CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c | > 120 ++++++-------------- > CryptoPkg/CryptoPkg.dec | > 16 ++- > CryptoPkg/CryptoPkg.uni | > 12 +- > CryptoPkg/Include/Library/HashApiLib.h | > 14 +-- > 4 files changed, 50 insertions(+), 112 deletions(-) > > diff --git > a/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c > b/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c > index 277ef9f0b421..b87a82b06ce1 100644 > --- a/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c > +++ b/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c > @@ -31,32 +31,24 @@ HashApiGetContextSize ( > VOID > ) > { > - switch (PcdGet8 (PcdHashApiLibPolicy)) { > - case HASH_API_ALGO_MD4: > - return Md4GetContextSize (); > - break; > - > - case HASH_API_ALGO_MD5: > - return Md5GetContextSize (); > - break; > - > - case HASH_API_ALGO_SHA1: > + switch (PcdGet16 (PcdHashApiLibPolicy)) { > + case HASH_ALG_SHA1: > return Sha1GetContextSize (); > break; > > - case HASH_API_ALGO_SHA256: > + case HASH_ALG_SHA256: > return Sha256GetContextSize (); > break; > > - case HASH_API_ALGO_SHA384: > + case HASH_ALG_SHA384: > return Sha384GetContextSize (); > break; > > - case HASH_API_ALGO_SHA512: > + case HASH_ALG_SHA512: > return Sha512GetContextSize (); > break; > > - case HASH_API_ALGO_SM3_256: > + case HASH_ALG_SM3_256: > return Sm3GetContextSize (); > break; > > @@ -81,32 +73,24 @@ HashApiInit ( > OUT HASH_API_CONTEXT HashContext > ) > { > - switch (PcdGet8 (PcdHashApiLibPolicy)) { > - case HASH_API_ALGO_MD4: > - return Md4Init (HashContext); > - break; > - > - case HASH_API_ALGO_MD5: > - return Md5Init (HashContext); > - break; > - > - case HASH_API_ALGO_SHA1: > + switch (PcdGet16 (PcdHashApiLibPolicy)) { > + case HASH_ALG_SHA1: > return Sha1Init (HashContext); > break; > > - case HASH_API_ALGO_SHA256: > + case HASH_ALG_SHA256: > return Sha256Init (HashContext); > break; > > - case HASH_API_ALGO_SHA384: > + case HASH_ALG_SHA384: > return Sha384Init (HashContext); > break; > > - case HASH_API_ALGO_SHA512: > + case HASH_ALG_SHA512: > return Sha512Init (HashContext); > break; > > - case HASH_API_ALGO_SM3_256: > + case HASH_ALG_SM3_256: > return Sm3Init (HashContext); > break; > > @@ -133,32 +117,24 @@ HashApiDuplicate ( > OUT HASH_API_CONTEXT NewHashContext > ) > { > - switch (PcdGet8 (PcdHashApiLibPolicy)) { > - case HASH_API_ALGO_MD4: > - return Md4Duplicate (HashContext, > NewHashContext); > - break; > - > - case HASH_API_ALGO_MD5: > - return Md5Duplicate (HashContext, > NewHashContext); > - break; > - > - case HASH_API_ALGO_SHA1: > + switch (PcdGet16 (PcdHashApiLibPolicy)) { > + case HASH_ALG_SHA1: > return Sha1Duplicate (HashContext, > NewHashContext); > break; > > - case HASH_API_ALGO_SHA256: > + case HASH_ALG_SHA256: > return Sha256Duplicate (HashContext, > NewHashContext); > break; > > - case HASH_API_ALGO_SHA384: > + case HASH_ALG_SHA384: > return Sha384Duplicate (HashContext, > NewHashContext); > break; > > - case HASH_API_ALGO_SHA512: > + case HASH_ALG_SHA512: > return Sha512Duplicate (HashContext, > NewHashContext); > break; > > - case HASH_API_ALGO_SM3_256: > + case HASH_ALG_SM3_256: > return Sm3Duplicate (HashContext, > NewHashContext); > break; > > @@ -187,32 +163,24 @@ HashApiUpdate ( > IN UINTN DataToHashLen > ) > { > - switch (PcdGet8 (PcdHashApiLibPolicy)) { > - case HASH_API_ALGO_MD4: > - return Md4Update (HashContext, DataToHash, > DataToHashLen); > - break; > - > - case HASH_API_ALGO_MD5: > - return Md5Update (HashContext, DataToHash, > DataToHashLen); > - break; > - > - case HASH_API_ALGO_SHA1: > + switch (PcdGet16 (PcdHashApiLibPolicy)) { > + case HASH_ALG_SHA1: > return Sha1Update (HashContext, DataToHash, > DataToHashLen); > break; > > - case HASH_API_ALGO_SHA256: > + case HASH_ALG_SHA256: > return Sha256Update (HashContext, DataToHash, > DataToHashLen); > break; > > - case HASH_API_ALGO_SHA384: > + case HASH_ALG_SHA384: > return Sha384Update (HashContext, DataToHash, > DataToHashLen); > break; > > - case HASH_API_ALGO_SHA512: > + case HASH_ALG_SHA512: > return Sha512Update (HashContext, DataToHash, > DataToHashLen); > break; > > - case HASH_API_ALGO_SM3_256: > + case HASH_ALG_SM3_256: > return Sm3Update (HashContext, DataToHash, > DataToHashLen); > break; > > @@ -239,32 +207,24 @@ HashApiFinal ( > OUT UINT8 *Digest > ) > { > - switch (PcdGet8 (PcdHashApiLibPolicy)) { > - case HASH_API_ALGO_MD4: > - return Md4Final (HashContext, Digest); > - break; > - > - case HASH_API_ALGO_MD5: > - return Md5Final (HashContext, Digest); > - break; > - > - case HASH_API_ALGO_SHA1: > + switch (PcdGet16 (PcdHashApiLibPolicy)) { > + case HASH_ALG_SHA1: > return Sha1Final (HashContext, Digest); > break; > > - case HASH_API_ALGO_SHA256: > + case HASH_ALG_SHA256: > return Sha256Final (HashContext, Digest); > break; > > - case HASH_API_ALGO_SHA384: > + case HASH_ALG_SHA384: > return Sha384Final (HashContext, Digest); > break; > > - case HASH_API_ALGO_SHA512: > + case HASH_ALG_SHA512: > return Sha512Final (HashContext, Digest); > break; > > - case HASH_API_ALGO_SM3_256: > + case HASH_ALG_SM3_256: > return Sm3Final (HashContext, Digest); > break; > > @@ -293,32 +253,24 @@ HashApiHashAll ( > OUT UINT8 *Digest > ) > { > - switch (PcdGet8 (PcdHashApiLibPolicy)) { > - case HASH_API_ALGO_MD4: > - return Md4HashAll (DataToHash, DataToHashLen, > Digest); > - break; > - > - case HASH_API_ALGO_MD5: > - return Md5HashAll (DataToHash, DataToHashLen, > Digest); > - break; > - > - case HASH_API_ALGO_SHA1: > + switch (PcdGet16 (PcdHashApiLibPolicy)) { > + case HASH_ALG_SHA1: > return Sha1HashAll (DataToHash, DataToHashLen, > Digest); > break; > > - case HASH_API_ALGO_SHA256: > + case HASH_ALG_SHA256: > return Sha256HashAll (DataToHash, DataToHashLen, > Digest); > break; > > - case HASH_API_ALGO_SHA384: > + case HASH_ALG_SHA384: > return Sha384HashAll (DataToHash, DataToHashLen, > Digest); > break; > > - case HASH_API_ALGO_SHA512: > + case HASH_ALG_SHA512: > return Sha512HashAll (DataToHash, DataToHashLen, > Digest); > break; > > - case HASH_API_ALGO_SM3_256: > + case HASH_ALG_SM3_256: > return Sm3HashAll (DataToHash, DataToHashLen, > Digest); > break; > > diff --git a/CryptoPkg/CryptoPkg.dec > b/CryptoPkg/CryptoPkg.dec > index 8bd63a76dd22..f185bcc82515 100644 > --- a/CryptoPkg/CryptoPkg.dec > +++ b/CryptoPkg/CryptoPkg.dec > @@ -74,16 +74,14 @@ [PcdsFixedAtBuild, > PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] > # Based on the value set, the required algorithm is > chosen to calculate > # the hash of data.<BR> > # The default hashing algorithm for BaseHashApiLib > is set to SHA256.<BR> > - # 0x00000001 - MD4.<BR> > - # 0x00000002 - MD5.<BR> > - # 0x00000003 - SHA1.<BR> > - # 0x00000004 - SHA256.<BR> > - # 0x00000005 - SHA384.<BR> > - # 0x00000006 - SHA512.<BR> > - # 0x00000007 - SM3_256.<BR> > + # 0x00000001 - SHA1.<BR> > + # 0x00000002 - SHA256.<BR> > + # 0x00000004 - SHA384.<BR> > + # 0x00000008 - SHA512.<BR> > + # 0x00000010 - SM3_256.<BR> Update the names to match the define names in Tpm20.h such as HASH_ALG_SHA256. > # @Prompt Set policy for hashing unsigned image for > Secure Boot. > - # @ValidRange 0x80000001 | 0x00000001 - 0x00000007 > - > gEfiCryptoPkgTokenSpaceGuid.PcdHashApiLibPolicy|0x04|UI > NT8|0x00000001 > + # @ValidRange 0x80000001 | 0x00000001 - 0x00000010 Using ValidRange is not correct because not all the values from 0x0000001 - 0x00000010 are valid. Should change to @ValidList. # @ValidList 0x80000001 | 0x00000001, 0x00000002, 0x00000004, 0x00000008, 0x00000010 > + > gEfiCryptoPkgTokenSpaceGuid.PcdHashApiLibPolicy|0x02|UI > NT16|0x00000001 > > [UserExtensions.TianoCore."ExtraFiles"] > CryptoPkgExtra.uni > diff --git a/CryptoPkg/CryptoPkg.uni > b/CryptoPkg/CryptoPkg.uni > index 2222762f42ee..7e97ac7af8b7 100644 > --- a/CryptoPkg/CryptoPkg.uni > +++ b/CryptoPkg/CryptoPkg.uni > @@ -21,13 +21,11 @@ > > "Based on the value set, the required algorithm is > chosen to calculate\n" > > "the hash of data.<BR>\n" > > "The default hashing algorithm for BaseHashApiLib is > set to SHA256.<BR>\n" > - > "0x00000001 - MD4.<BR>\n" > - > "0x00000002 - MD5.<BR>\n" > - > "0x00000003 - SHA1.<BR>\n" > - > "0x00000004 - SHA256.<BR>\n" > - > "0x00000005 - SHA384.<BR>\n" > - > "0x00000006 - SHA512.<BR>\n" > - > "0x00000007 - SM3.<BR>" > + > "0x00000001 - SHA1.<BR>\n" > + > "0x00000002 - SHA256.<BR>\n" > + > "0x00000004 - SHA384.<BR>\n" > + > "0x00000008 - SHA512.<BR>\n" > + > "0x00000010 - SM3.<BR>" Update the names to match the define names in Tpm20.h such as HASH_ALG_SHA256. > > #string > STR_gEfiCryptoPkgTokenSpaceGuid_PcdCryptoServiceFamilyE > nable_PROMPT #language en-US "Enable/Disable EDK II > Crypto Protocol/PPI services" > > diff --git a/CryptoPkg/Include/Library/HashApiLib.h > b/CryptoPkg/Include/Library/HashApiLib.h > index 22068e5a1756..b8b52ae15bd9 100644 > --- a/CryptoPkg/Include/Library/HashApiLib.h > +++ b/CryptoPkg/Include/Library/HashApiLib.h > @@ -12,20 +12,10 @@ > #ifndef __BASEHASHAPILIB_H_ > #define __BASEHASHAPILIB_H_ This define name does not match the pattern for other includes and BASE should not be used here. Please change to: #ifndef __HASH_API_LIB_H_ #define __HASH_API_LIB_H_ > > +#include <IndustryStandard/Tpm20.h> > + > typedef VOID *HASH_API_CONTEXT; > > -// > -// Hash Algorithms > -// > -#define HASH_API_ALGO_INVALID 0x00000000 > -#define HASH_API_ALGO_MD4 0x00000001 > -#define HASH_API_ALGO_MD5 0x00000002 > -#define HASH_API_ALGO_SHA1 0x00000003 > -#define HASH_API_ALGO_SHA256 0x00000004 > -#define HASH_API_ALGO_SHA384 0x00000005 > -#define HASH_API_ALGO_SHA512 0x00000006 > -#define HASH_API_ALGO_SM3_256 0x00000007 > - > /** > Retrieves the size, in bytes, of the context buffer > required for hash operations. > > -- > 2.16.2.windows.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with TPM 2.0 Implementation 2020-02-14 22:29 ` Michael D Kinney @ 2020-02-14 22:38 ` Michael D Kinney 2020-02-14 23:21 ` Sukerkar, Amol N 0 siblings, 1 reply; 7+ messages in thread From: Michael D Kinney @ 2020-02-14 22:38 UTC (permalink / raw) To: Sukerkar, Amol N, devel@edk2.groups.io, Kinney, Michael D Cc: Yao, Jiewen, Wang, Jian J, Agrawal, Sachin, Gao, Liming Amol, One additional comment. You added #include <IndustryStandard/Tpm20.h> to the HashApiLib.h class. The public APIs do not depend on any definitions from <IndustryStandard/Tpm20.h> so this line should ne removed from CryptoPkg/Include/Library/HashApiLib.h. Instead, #include <IndustryStandard/Tpm20.h> should be added to the BaseHashApiLib implementation in the file CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c. Thanks, Mike > -----Original Message----- > From: Kinney, Michael D <michael.d.kinney@intel.com> > Sent: Friday, February 14, 2020 2:30 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>; Gao, Liming > <liming.gao@intel.com> > Subject: RE: [PATCH v2 1/2] CryptoPkg/BaseHashApiLib: > Align BaseHashApiLib with TPM 2.0 Implementation > > Amol, > > Comments included below. > > Mike > > > -----Original Message----- > > From: Sukerkar, Amol N <amol.n.sukerkar@intel.com> > > Sent: Friday, February 14, 2020 10:06 AM > > 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>; Gao, Liming > > <liming.gao@intel.com> > > Subject: [PATCH v2 1/2] CryptoPkg/BaseHashApiLib: > Align > > BaseHashApiLib with TPM 2.0 Implementation > > > > Ref: > > https://bugzilla.tianocore.org/show_bug.cgi?id=2511 > > > > This commit aligns the baseHashApiLib with TPM 2.0 > > Implementation > > as follows: > > - Remove reference to MD4 and MD5 algorithms as they > > are deprecated > > - Align the enumerations for hashing algoerithms with > > the one used > > in TPM 2.0 implementation defined in > > IndustryStandard/Tpm20.h. > > > > Cc: Jiewen Yao <jiewen.yao@intel.com> > > Cc: Jian J Wang <jian.j.wang@intel.com> > > Cc: Michael D Kinney <michael.d.kinney@intel.com> > > Signed-off-by: Amol N Sukerkar > > <amol.n.sukerkar@intel.com> > > --- > > > > Notes: > > v2 > > - Fixed closed parentheses in commit message > > > > CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c | > > 120 ++++++-------------- > > CryptoPkg/CryptoPkg.dec | > > 16 ++- > > CryptoPkg/CryptoPkg.uni | > > 12 +- > > CryptoPkg/Include/Library/HashApiLib.h | > > 14 +-- > > 4 files changed, 50 insertions(+), 112 deletions(-) > > > > diff --git > > a/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c > > b/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c > > index 277ef9f0b421..b87a82b06ce1 100644 > > --- > a/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c > > +++ > b/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c > > @@ -31,32 +31,24 @@ HashApiGetContextSize ( > > VOID > > ) > > { > > - switch (PcdGet8 (PcdHashApiLibPolicy)) { > > - case HASH_API_ALGO_MD4: > > - return Md4GetContextSize (); > > - break; > > - > > - case HASH_API_ALGO_MD5: > > - return Md5GetContextSize (); > > - break; > > - > > - case HASH_API_ALGO_SHA1: > > + switch (PcdGet16 (PcdHashApiLibPolicy)) { > > + case HASH_ALG_SHA1: > > return Sha1GetContextSize (); > > break; > > > > - case HASH_API_ALGO_SHA256: > > + case HASH_ALG_SHA256: > > return Sha256GetContextSize (); > > break; > > > > - case HASH_API_ALGO_SHA384: > > + case HASH_ALG_SHA384: > > return Sha384GetContextSize (); > > break; > > > > - case HASH_API_ALGO_SHA512: > > + case HASH_ALG_SHA512: > > return Sha512GetContextSize (); > > break; > > > > - case HASH_API_ALGO_SM3_256: > > + case HASH_ALG_SM3_256: > > return Sm3GetContextSize (); > > break; > > > > @@ -81,32 +73,24 @@ HashApiInit ( > > OUT HASH_API_CONTEXT HashContext > > ) > > { > > - switch (PcdGet8 (PcdHashApiLibPolicy)) { > > - case HASH_API_ALGO_MD4: > > - return Md4Init (HashContext); > > - break; > > - > > - case HASH_API_ALGO_MD5: > > - return Md5Init (HashContext); > > - break; > > - > > - case HASH_API_ALGO_SHA1: > > + switch (PcdGet16 (PcdHashApiLibPolicy)) { > > + case HASH_ALG_SHA1: > > return Sha1Init (HashContext); > > break; > > > > - case HASH_API_ALGO_SHA256: > > + case HASH_ALG_SHA256: > > return Sha256Init (HashContext); > > break; > > > > - case HASH_API_ALGO_SHA384: > > + case HASH_ALG_SHA384: > > return Sha384Init (HashContext); > > break; > > > > - case HASH_API_ALGO_SHA512: > > + case HASH_ALG_SHA512: > > return Sha512Init (HashContext); > > break; > > > > - case HASH_API_ALGO_SM3_256: > > + case HASH_ALG_SM3_256: > > return Sm3Init (HashContext); > > break; > > > > @@ -133,32 +117,24 @@ HashApiDuplicate ( > > OUT HASH_API_CONTEXT NewHashContext > > ) > > { > > - switch (PcdGet8 (PcdHashApiLibPolicy)) { > > - case HASH_API_ALGO_MD4: > > - return Md4Duplicate (HashContext, > > NewHashContext); > > - break; > > - > > - case HASH_API_ALGO_MD5: > > - return Md5Duplicate (HashContext, > > NewHashContext); > > - break; > > - > > - case HASH_API_ALGO_SHA1: > > + switch (PcdGet16 (PcdHashApiLibPolicy)) { > > + case HASH_ALG_SHA1: > > return Sha1Duplicate (HashContext, > > NewHashContext); > > break; > > > > - case HASH_API_ALGO_SHA256: > > + case HASH_ALG_SHA256: > > return Sha256Duplicate (HashContext, > > NewHashContext); > > break; > > > > - case HASH_API_ALGO_SHA384: > > + case HASH_ALG_SHA384: > > return Sha384Duplicate (HashContext, > > NewHashContext); > > break; > > > > - case HASH_API_ALGO_SHA512: > > + case HASH_ALG_SHA512: > > return Sha512Duplicate (HashContext, > > NewHashContext); > > break; > > > > - case HASH_API_ALGO_SM3_256: > > + case HASH_ALG_SM3_256: > > return Sm3Duplicate (HashContext, > > NewHashContext); > > break; > > > > @@ -187,32 +163,24 @@ HashApiUpdate ( > > IN UINTN DataToHashLen > > ) > > { > > - switch (PcdGet8 (PcdHashApiLibPolicy)) { > > - case HASH_API_ALGO_MD4: > > - return Md4Update (HashContext, DataToHash, > > DataToHashLen); > > - break; > > - > > - case HASH_API_ALGO_MD5: > > - return Md5Update (HashContext, DataToHash, > > DataToHashLen); > > - break; > > - > > - case HASH_API_ALGO_SHA1: > > + switch (PcdGet16 (PcdHashApiLibPolicy)) { > > + case HASH_ALG_SHA1: > > return Sha1Update (HashContext, DataToHash, > > DataToHashLen); > > break; > > > > - case HASH_API_ALGO_SHA256: > > + case HASH_ALG_SHA256: > > return Sha256Update (HashContext, DataToHash, > > DataToHashLen); > > break; > > > > - case HASH_API_ALGO_SHA384: > > + case HASH_ALG_SHA384: > > return Sha384Update (HashContext, DataToHash, > > DataToHashLen); > > break; > > > > - case HASH_API_ALGO_SHA512: > > + case HASH_ALG_SHA512: > > return Sha512Update (HashContext, DataToHash, > > DataToHashLen); > > break; > > > > - case HASH_API_ALGO_SM3_256: > > + case HASH_ALG_SM3_256: > > return Sm3Update (HashContext, DataToHash, > > DataToHashLen); > > break; > > > > @@ -239,32 +207,24 @@ HashApiFinal ( > > OUT UINT8 *Digest > > ) > > { > > - switch (PcdGet8 (PcdHashApiLibPolicy)) { > > - case HASH_API_ALGO_MD4: > > - return Md4Final (HashContext, Digest); > > - break; > > - > > - case HASH_API_ALGO_MD5: > > - return Md5Final (HashContext, Digest); > > - break; > > - > > - case HASH_API_ALGO_SHA1: > > + switch (PcdGet16 (PcdHashApiLibPolicy)) { > > + case HASH_ALG_SHA1: > > return Sha1Final (HashContext, Digest); > > break; > > > > - case HASH_API_ALGO_SHA256: > > + case HASH_ALG_SHA256: > > return Sha256Final (HashContext, Digest); > > break; > > > > - case HASH_API_ALGO_SHA384: > > + case HASH_ALG_SHA384: > > return Sha384Final (HashContext, Digest); > > break; > > > > - case HASH_API_ALGO_SHA512: > > + case HASH_ALG_SHA512: > > return Sha512Final (HashContext, Digest); > > break; > > > > - case HASH_API_ALGO_SM3_256: > > + case HASH_ALG_SM3_256: > > return Sm3Final (HashContext, Digest); > > break; > > > > @@ -293,32 +253,24 @@ HashApiHashAll ( > > OUT UINT8 *Digest > > ) > > { > > - switch (PcdGet8 (PcdHashApiLibPolicy)) { > > - case HASH_API_ALGO_MD4: > > - return Md4HashAll (DataToHash, DataToHashLen, > > Digest); > > - break; > > - > > - case HASH_API_ALGO_MD5: > > - return Md5HashAll (DataToHash, DataToHashLen, > > Digest); > > - break; > > - > > - case HASH_API_ALGO_SHA1: > > + switch (PcdGet16 (PcdHashApiLibPolicy)) { > > + case HASH_ALG_SHA1: > > return Sha1HashAll (DataToHash, DataToHashLen, > > Digest); > > break; > > > > - case HASH_API_ALGO_SHA256: > > + case HASH_ALG_SHA256: > > return Sha256HashAll (DataToHash, > DataToHashLen, > > Digest); > > break; > > > > - case HASH_API_ALGO_SHA384: > > + case HASH_ALG_SHA384: > > return Sha384HashAll (DataToHash, > DataToHashLen, > > Digest); > > break; > > > > - case HASH_API_ALGO_SHA512: > > + case HASH_ALG_SHA512: > > return Sha512HashAll (DataToHash, > DataToHashLen, > > Digest); > > break; > > > > - case HASH_API_ALGO_SM3_256: > > + case HASH_ALG_SM3_256: > > return Sm3HashAll (DataToHash, DataToHashLen, > > Digest); > > break; > > > > diff --git a/CryptoPkg/CryptoPkg.dec > > b/CryptoPkg/CryptoPkg.dec > > index 8bd63a76dd22..f185bcc82515 100644 > > --- a/CryptoPkg/CryptoPkg.dec > > +++ b/CryptoPkg/CryptoPkg.dec > > @@ -74,16 +74,14 @@ [PcdsFixedAtBuild, > > PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] > > # Based on the value set, the required algorithm > is > > chosen to calculate > > # the hash of data.<BR> > > # The default hashing algorithm for > BaseHashApiLib > > is set to SHA256.<BR> > > - # 0x00000001 - MD4.<BR> > > - # 0x00000002 - MD5.<BR> > > - # 0x00000003 - SHA1.<BR> > > - # 0x00000004 - SHA256.<BR> > > - # 0x00000005 - SHA384.<BR> > > - # 0x00000006 - SHA512.<BR> > > - # 0x00000007 - SM3_256.<BR> > > + # 0x00000001 - SHA1.<BR> > > + # 0x00000002 - SHA256.<BR> > > + # 0x00000004 - SHA384.<BR> > > + # 0x00000008 - SHA512.<BR> > > + # 0x00000010 - SM3_256.<BR> > > Update the names to match the define names in Tpm20.h > such as HASH_ALG_SHA256. > > > # @Prompt Set policy for hashing unsigned image > for > > Secure Boot. > > - # @ValidRange 0x80000001 | 0x00000001 - 0x00000007 > > - > > > gEfiCryptoPkgTokenSpaceGuid.PcdHashApiLibPolicy|0x04|UI > > NT8|0x00000001 > > + # @ValidRange 0x80000001 | 0x00000001 - 0x00000010 > > Using ValidRange is not correct because not all the > values from 0x0000001 - 0x00000010 are valid. Should > change to @ValidList. > > # @ValidList 0x80000001 | 0x00000001, 0x00000002, > 0x00000004, 0x00000008, 0x00000010 > > > + > > > gEfiCryptoPkgTokenSpaceGuid.PcdHashApiLibPolicy|0x02|UI > > NT16|0x00000001 > > > > [UserExtensions.TianoCore."ExtraFiles"] > > CryptoPkgExtra.uni > > diff --git a/CryptoPkg/CryptoPkg.uni > > b/CryptoPkg/CryptoPkg.uni > > index 2222762f42ee..7e97ac7af8b7 100644 > > --- a/CryptoPkg/CryptoPkg.uni > > +++ b/CryptoPkg/CryptoPkg.uni > > @@ -21,13 +21,11 @@ > > > > "Based on the value set, the required algorithm is > > chosen to calculate\n" > > > > "the hash of data.<BR>\n" > > > > "The default hashing algorithm for BaseHashApiLib is > > set to SHA256.<BR>\n" > > - > > "0x00000001 - MD4.<BR>\n" > > - > > "0x00000002 - MD5.<BR>\n" > > - > > "0x00000003 - SHA1.<BR>\n" > > - > > "0x00000004 - SHA256.<BR>\n" > > - > > "0x00000005 - SHA384.<BR>\n" > > - > > "0x00000006 - SHA512.<BR>\n" > > - > > "0x00000007 - SM3.<BR>" > > + > > "0x00000001 - SHA1.<BR>\n" > > + > > "0x00000002 - SHA256.<BR>\n" > > + > > "0x00000004 - SHA384.<BR>\n" > > + > > "0x00000008 - SHA512.<BR>\n" > > + > > "0x00000010 - SM3.<BR>" > > Update the names to match the define names in Tpm20.h > such as HASH_ALG_SHA256. > > > > > #string > > > STR_gEfiCryptoPkgTokenSpaceGuid_PcdCryptoServiceFamilyE > > nable_PROMPT #language en-US "Enable/Disable EDK II > > Crypto Protocol/PPI services" > > > > diff --git a/CryptoPkg/Include/Library/HashApiLib.h > > b/CryptoPkg/Include/Library/HashApiLib.h > > index 22068e5a1756..b8b52ae15bd9 100644 > > --- a/CryptoPkg/Include/Library/HashApiLib.h > > +++ b/CryptoPkg/Include/Library/HashApiLib.h > > @@ -12,20 +12,10 @@ > > #ifndef __BASEHASHAPILIB_H_ > > #define __BASEHASHAPILIB_H_ > > This define name does not match the pattern for > other includes and BASE should not be used here. > Please change to: > > #ifndef __HASH_API_LIB_H_ > #define __HASH_API_LIB_H_ > > > > > +#include <IndustryStandard/Tpm20.h> > > + > > typedef VOID *HASH_API_CONTEXT; > > > > -// > > -// Hash Algorithms > > -// > > -#define HASH_API_ALGO_INVALID 0x00000000 > > -#define HASH_API_ALGO_MD4 0x00000001 > > -#define HASH_API_ALGO_MD5 0x00000002 > > -#define HASH_API_ALGO_SHA1 0x00000003 > > -#define HASH_API_ALGO_SHA256 0x00000004 > > -#define HASH_API_ALGO_SHA384 0x00000005 > > -#define HASH_API_ALGO_SHA512 0x00000006 > > -#define HASH_API_ALGO_SM3_256 0x00000007 > > - > > /** > > Retrieves the size, in bytes, of the context > buffer > > required for hash operations. > > > > -- > > 2.16.2.windows.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with TPM 2.0 Implementation 2020-02-14 22:38 ` Michael D Kinney @ 2020-02-14 23:21 ` Sukerkar, Amol N 0 siblings, 0 replies; 7+ messages in thread From: Sukerkar, Amol N @ 2020-02-14 23:21 UTC (permalink / raw) To: Kinney, Michael D, devel@edk2.groups.io Cc: Yao, Jiewen, Wang, Jian J, Agrawal, Sachin, Gao, Liming, Sukerkar, Amol N Hi Mike, Addressed all the comments and sent newer version of patch. Thanks, Amol -----Original Message----- From: Kinney, Michael D <michael.d.kinney@intel.com> Sent: Friday, February 14, 2020 3:38 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>; Gao, Liming <liming.gao@intel.com> Subject: RE: [PATCH v2 1/2] CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with TPM 2.0 Implementation Amol, One additional comment. You added #include <IndustryStandard/Tpm20.h> to the HashApiLib.h class. The public APIs do not depend on any definitions from <IndustryStandard/Tpm20.h> so this line should ne removed from CryptoPkg/Include/Library/HashApiLib.h. Instead, #include <IndustryStandard/Tpm20.h> should be added to the BaseHashApiLib implementation in the file CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c. Thanks, Mike > -----Original Message----- > From: Kinney, Michael D <michael.d.kinney@intel.com> > Sent: Friday, February 14, 2020 2:30 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>; > Gao, Liming <liming.gao@intel.com> > Subject: RE: [PATCH v2 1/2] CryptoPkg/BaseHashApiLib: > Align BaseHashApiLib with TPM 2.0 Implementation > > Amol, > > Comments included below. > > Mike > > > -----Original Message----- > > From: Sukerkar, Amol N <amol.n.sukerkar@intel.com> > > Sent: Friday, February 14, 2020 10:06 AM > > 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>; Gao, Liming > > <liming.gao@intel.com> > > Subject: [PATCH v2 1/2] CryptoPkg/BaseHashApiLib: > Align > > BaseHashApiLib with TPM 2.0 Implementation > > > > Ref: > > https://bugzilla.tianocore.org/show_bug.cgi?id=2511 > > > > This commit aligns the baseHashApiLib with TPM 2.0 Implementation as > > follows: > > - Remove reference to MD4 and MD5 algorithms as they are deprecated > > - Align the enumerations for hashing algoerithms with the one used > > in TPM 2.0 implementation defined in IndustryStandard/Tpm20.h. > > > > Cc: Jiewen Yao <jiewen.yao@intel.com> > > Cc: Jian J Wang <jian.j.wang@intel.com> > > Cc: Michael D Kinney <michael.d.kinney@intel.com> > > Signed-off-by: Amol N Sukerkar > > <amol.n.sukerkar@intel.com> > > --- > > > > Notes: > > v2 > > - Fixed closed parentheses in commit message > > > > CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c | > > 120 ++++++-------------- > > CryptoPkg/CryptoPkg.dec | > > 16 ++- > > CryptoPkg/CryptoPkg.uni | > > 12 +- > > CryptoPkg/Include/Library/HashApiLib.h | > > 14 +-- > > 4 files changed, 50 insertions(+), 112 deletions(-) > > > > diff --git > > a/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c > > b/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c > > index 277ef9f0b421..b87a82b06ce1 100644 > > --- > a/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c > > +++ > b/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c > > @@ -31,32 +31,24 @@ HashApiGetContextSize ( > > VOID > > ) > > { > > - switch (PcdGet8 (PcdHashApiLibPolicy)) { > > - case HASH_API_ALGO_MD4: > > - return Md4GetContextSize (); > > - break; > > - > > - case HASH_API_ALGO_MD5: > > - return Md5GetContextSize (); > > - break; > > - > > - case HASH_API_ALGO_SHA1: > > + switch (PcdGet16 (PcdHashApiLibPolicy)) { > > + case HASH_ALG_SHA1: > > return Sha1GetContextSize (); > > break; > > > > - case HASH_API_ALGO_SHA256: > > + case HASH_ALG_SHA256: > > return Sha256GetContextSize (); > > break; > > > > - case HASH_API_ALGO_SHA384: > > + case HASH_ALG_SHA384: > > return Sha384GetContextSize (); > > break; > > > > - case HASH_API_ALGO_SHA512: > > + case HASH_ALG_SHA512: > > return Sha512GetContextSize (); > > break; > > > > - case HASH_API_ALGO_SM3_256: > > + case HASH_ALG_SM3_256: > > return Sm3GetContextSize (); > > break; > > > > @@ -81,32 +73,24 @@ HashApiInit ( > > OUT HASH_API_CONTEXT HashContext > > ) > > { > > - switch (PcdGet8 (PcdHashApiLibPolicy)) { > > - case HASH_API_ALGO_MD4: > > - return Md4Init (HashContext); > > - break; > > - > > - case HASH_API_ALGO_MD5: > > - return Md5Init (HashContext); > > - break; > > - > > - case HASH_API_ALGO_SHA1: > > + switch (PcdGet16 (PcdHashApiLibPolicy)) { > > + case HASH_ALG_SHA1: > > return Sha1Init (HashContext); > > break; > > > > - case HASH_API_ALGO_SHA256: > > + case HASH_ALG_SHA256: > > return Sha256Init (HashContext); > > break; > > > > - case HASH_API_ALGO_SHA384: > > + case HASH_ALG_SHA384: > > return Sha384Init (HashContext); > > break; > > > > - case HASH_API_ALGO_SHA512: > > + case HASH_ALG_SHA512: > > return Sha512Init (HashContext); > > break; > > > > - case HASH_API_ALGO_SM3_256: > > + case HASH_ALG_SM3_256: > > return Sm3Init (HashContext); > > break; > > > > @@ -133,32 +117,24 @@ HashApiDuplicate ( > > OUT HASH_API_CONTEXT NewHashContext > > ) > > { > > - switch (PcdGet8 (PcdHashApiLibPolicy)) { > > - case HASH_API_ALGO_MD4: > > - return Md4Duplicate (HashContext, > > NewHashContext); > > - break; > > - > > - case HASH_API_ALGO_MD5: > > - return Md5Duplicate (HashContext, > > NewHashContext); > > - break; > > - > > - case HASH_API_ALGO_SHA1: > > + switch (PcdGet16 (PcdHashApiLibPolicy)) { > > + case HASH_ALG_SHA1: > > return Sha1Duplicate (HashContext, NewHashContext); > > break; > > > > - case HASH_API_ALGO_SHA256: > > + case HASH_ALG_SHA256: > > return Sha256Duplicate (HashContext, NewHashContext); > > break; > > > > - case HASH_API_ALGO_SHA384: > > + case HASH_ALG_SHA384: > > return Sha384Duplicate (HashContext, NewHashContext); > > break; > > > > - case HASH_API_ALGO_SHA512: > > + case HASH_ALG_SHA512: > > return Sha512Duplicate (HashContext, NewHashContext); > > break; > > > > - case HASH_API_ALGO_SM3_256: > > + case HASH_ALG_SM3_256: > > return Sm3Duplicate (HashContext, NewHashContext); > > break; > > > > @@ -187,32 +163,24 @@ HashApiUpdate ( > > IN UINTN DataToHashLen > > ) > > { > > - switch (PcdGet8 (PcdHashApiLibPolicy)) { > > - case HASH_API_ALGO_MD4: > > - return Md4Update (HashContext, DataToHash, > > DataToHashLen); > > - break; > > - > > - case HASH_API_ALGO_MD5: > > - return Md5Update (HashContext, DataToHash, > > DataToHashLen); > > - break; > > - > > - case HASH_API_ALGO_SHA1: > > + switch (PcdGet16 (PcdHashApiLibPolicy)) { > > + case HASH_ALG_SHA1: > > return Sha1Update (HashContext, DataToHash, DataToHashLen); > > break; > > > > - case HASH_API_ALGO_SHA256: > > + case HASH_ALG_SHA256: > > return Sha256Update (HashContext, DataToHash, DataToHashLen); > > break; > > > > - case HASH_API_ALGO_SHA384: > > + case HASH_ALG_SHA384: > > return Sha384Update (HashContext, DataToHash, DataToHashLen); > > break; > > > > - case HASH_API_ALGO_SHA512: > > + case HASH_ALG_SHA512: > > return Sha512Update (HashContext, DataToHash, DataToHashLen); > > break; > > > > - case HASH_API_ALGO_SM3_256: > > + case HASH_ALG_SM3_256: > > return Sm3Update (HashContext, DataToHash, DataToHashLen); > > break; > > > > @@ -239,32 +207,24 @@ HashApiFinal ( > > OUT UINT8 *Digest > > ) > > { > > - switch (PcdGet8 (PcdHashApiLibPolicy)) { > > - case HASH_API_ALGO_MD4: > > - return Md4Final (HashContext, Digest); > > - break; > > - > > - case HASH_API_ALGO_MD5: > > - return Md5Final (HashContext, Digest); > > - break; > > - > > - case HASH_API_ALGO_SHA1: > > + switch (PcdGet16 (PcdHashApiLibPolicy)) { > > + case HASH_ALG_SHA1: > > return Sha1Final (HashContext, Digest); > > break; > > > > - case HASH_API_ALGO_SHA256: > > + case HASH_ALG_SHA256: > > return Sha256Final (HashContext, Digest); > > break; > > > > - case HASH_API_ALGO_SHA384: > > + case HASH_ALG_SHA384: > > return Sha384Final (HashContext, Digest); > > break; > > > > - case HASH_API_ALGO_SHA512: > > + case HASH_ALG_SHA512: > > return Sha512Final (HashContext, Digest); > > break; > > > > - case HASH_API_ALGO_SM3_256: > > + case HASH_ALG_SM3_256: > > return Sm3Final (HashContext, Digest); > > break; > > > > @@ -293,32 +253,24 @@ HashApiHashAll ( > > OUT UINT8 *Digest > > ) > > { > > - switch (PcdGet8 (PcdHashApiLibPolicy)) { > > - case HASH_API_ALGO_MD4: > > - return Md4HashAll (DataToHash, DataToHashLen, > > Digest); > > - break; > > - > > - case HASH_API_ALGO_MD5: > > - return Md5HashAll (DataToHash, DataToHashLen, > > Digest); > > - break; > > - > > - case HASH_API_ALGO_SHA1: > > + switch (PcdGet16 (PcdHashApiLibPolicy)) { > > + case HASH_ALG_SHA1: > > return Sha1HashAll (DataToHash, DataToHashLen, Digest); > > break; > > > > - case HASH_API_ALGO_SHA256: > > + case HASH_ALG_SHA256: > > return Sha256HashAll (DataToHash, > DataToHashLen, > > Digest); > > break; > > > > - case HASH_API_ALGO_SHA384: > > + case HASH_ALG_SHA384: > > return Sha384HashAll (DataToHash, > DataToHashLen, > > Digest); > > break; > > > > - case HASH_API_ALGO_SHA512: > > + case HASH_ALG_SHA512: > > return Sha512HashAll (DataToHash, > DataToHashLen, > > Digest); > > break; > > > > - case HASH_API_ALGO_SM3_256: > > + case HASH_ALG_SM3_256: > > return Sm3HashAll (DataToHash, DataToHashLen, Digest); > > break; > > > > diff --git a/CryptoPkg/CryptoPkg.dec b/CryptoPkg/CryptoPkg.dec index > > 8bd63a76dd22..f185bcc82515 100644 > > --- a/CryptoPkg/CryptoPkg.dec > > +++ b/CryptoPkg/CryptoPkg.dec > > @@ -74,16 +74,14 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, > > PcdsDynamic, PcdsDynamicEx] > > # Based on the value set, the required algorithm > is > > chosen to calculate > > # the hash of data.<BR> > > # The default hashing algorithm for > BaseHashApiLib > > is set to SHA256.<BR> > > - # 0x00000001 - MD4.<BR> > > - # 0x00000002 - MD5.<BR> > > - # 0x00000003 - SHA1.<BR> > > - # 0x00000004 - SHA256.<BR> > > - # 0x00000005 - SHA384.<BR> > > - # 0x00000006 - SHA512.<BR> > > - # 0x00000007 - SM3_256.<BR> > > + # 0x00000001 - SHA1.<BR> > > + # 0x00000002 - SHA256.<BR> > > + # 0x00000004 - SHA384.<BR> > > + # 0x00000008 - SHA512.<BR> > > + # 0x00000010 - SM3_256.<BR> > > Update the names to match the define names in Tpm20.h such as > HASH_ALG_SHA256. > > > # @Prompt Set policy for hashing unsigned image > for > > Secure Boot. > > - # @ValidRange 0x80000001 | 0x00000001 - 0x00000007 > > - > > > gEfiCryptoPkgTokenSpaceGuid.PcdHashApiLibPolicy|0x04|UI > > NT8|0x00000001 > > + # @ValidRange 0x80000001 | 0x00000001 - 0x00000010 > > Using ValidRange is not correct because not all the values from > 0x0000001 - 0x00000010 are valid. Should change to @ValidList. > > # @ValidList 0x80000001 | 0x00000001, 0x00000002, 0x00000004, > 0x00000008, 0x00000010 > > > + > > > gEfiCryptoPkgTokenSpaceGuid.PcdHashApiLibPolicy|0x02|UI > > NT16|0x00000001 > > > > [UserExtensions.TianoCore."ExtraFiles"] > > CryptoPkgExtra.uni > > diff --git a/CryptoPkg/CryptoPkg.uni b/CryptoPkg/CryptoPkg.uni index > > 2222762f42ee..7e97ac7af8b7 100644 > > --- a/CryptoPkg/CryptoPkg.uni > > +++ b/CryptoPkg/CryptoPkg.uni > > @@ -21,13 +21,11 @@ > > > > "Based on the value set, the required algorithm is chosen to > > calculate\n" > > > > "the hash of data.<BR>\n" > > > > "The default hashing algorithm for BaseHashApiLib is set to > > SHA256.<BR>\n" > > - > > "0x00000001 - MD4.<BR>\n" > > - > > "0x00000002 - MD5.<BR>\n" > > - > > "0x00000003 - SHA1.<BR>\n" > > - > > "0x00000004 - SHA256.<BR>\n" > > - > > "0x00000005 - SHA384.<BR>\n" > > - > > "0x00000006 - SHA512.<BR>\n" > > - > > "0x00000007 - SM3.<BR>" > > + > > "0x00000001 - SHA1.<BR>\n" > > + > > "0x00000002 - SHA256.<BR>\n" > > + > > "0x00000004 - SHA384.<BR>\n" > > + > > "0x00000008 - SHA512.<BR>\n" > > + > > "0x00000010 - SM3.<BR>" > > Update the names to match the define names in Tpm20.h such as > HASH_ALG_SHA256. > > > > > #string > > > STR_gEfiCryptoPkgTokenSpaceGuid_PcdCryptoServiceFamilyE > > nable_PROMPT #language en-US "Enable/Disable EDK II Crypto > > Protocol/PPI services" > > > > diff --git a/CryptoPkg/Include/Library/HashApiLib.h > > b/CryptoPkg/Include/Library/HashApiLib.h > > index 22068e5a1756..b8b52ae15bd9 100644 > > --- a/CryptoPkg/Include/Library/HashApiLib.h > > +++ b/CryptoPkg/Include/Library/HashApiLib.h > > @@ -12,20 +12,10 @@ > > #ifndef __BASEHASHAPILIB_H_ > > #define __BASEHASHAPILIB_H_ > > This define name does not match the pattern for other includes and > BASE should not be used here. > Please change to: > > #ifndef __HASH_API_LIB_H_ > #define __HASH_API_LIB_H_ > > > > > +#include <IndustryStandard/Tpm20.h> > > + > > typedef VOID *HASH_API_CONTEXT; > > > > -// > > -// Hash Algorithms > > -// > > -#define HASH_API_ALGO_INVALID 0x00000000 > > -#define HASH_API_ALGO_MD4 0x00000001 > > -#define HASH_API_ALGO_MD5 0x00000002 > > -#define HASH_API_ALGO_SHA1 0x00000003 > > -#define HASH_API_ALGO_SHA256 0x00000004 > > -#define HASH_API_ALGO_SHA384 0x00000005 > > -#define HASH_API_ALGO_SHA512 0x00000006 > > -#define HASH_API_ALGO_SM3_256 0x00000007 > > - > > /** > > Retrieves the size, in bytes, of the context > buffer > > required for hash operations. > > > > -- > > 2.16.2.windows.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] CryptoPkg/BaseHashApiLib: Change PcdHashApiLibPolicy type to FixedAtBuild 2020-02-14 18:05 [PATCH v2 0/2] Enhancement and Fixes to BaseHashApiLib Sukerkar, Amol N 2020-02-14 18:05 ` [PATCH v2 1/2] CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with TPM 2.0 Implementation Sukerkar, Amol N @ 2020-02-14 18:05 ` Sukerkar, Amol N 2020-02-14 22:29 ` Michael D Kinney 1 sibling, 1 reply; 7+ messages in thread From: Sukerkar, Amol N @ 2020-02-14 18:05 UTC (permalink / raw) To: devel; +Cc: michael.d.kinney, jiewen.yao, jian.j.wang, sachin.agrawal, liming.gao Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2511 This commit changes the PCD PcdHashApiLibPolicy to the type PcdsFixedAtBuild so as to be able to optimize away the unused hashing algorithms in HashApiLib instance used by a driver. Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Signed-off-by: Amol N Sukerkar <amol.n.sukerkar@intel.com> --- Notes: v2 - Fixed closed parantheses in the commit message CryptoPkg/CryptoPkg.dec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CryptoPkg/CryptoPkg.dec b/CryptoPkg/CryptoPkg.dec index f185bcc82515..c23af7b87d25 100644 --- a/CryptoPkg/CryptoPkg.dec +++ b/CryptoPkg/CryptoPkg.dec @@ -69,7 +69,7 @@ [PcdsFixedAtBuild] Pcd/PcdCryptoServiceFamilyEnable.h } -[PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] +[PcdsFixedAtBuild] ## This PCD indicates the HASH algorithm to calculate hash of data # Based on the value set, the required algorithm is chosen to calculate # the hash of data.<BR> -- 2.16.2.windows.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] CryptoPkg/BaseHashApiLib: Change PcdHashApiLibPolicy type to FixedAtBuild 2020-02-14 18:05 ` [PATCH v2 2/2] CryptoPkg/BaseHashApiLib: Change PcdHashApiLibPolicy type to FixedAtBuild Sukerkar, Amol N @ 2020-02-14 22:29 ` Michael D Kinney 0 siblings, 0 replies; 7+ messages in thread From: Michael D Kinney @ 2020-02-14 22:29 UTC (permalink / raw) To: Sukerkar, Amol N, devel@edk2.groups.io Cc: Yao, Jiewen, Wang, Jian J, Agrawal, Sachin, Gao, Liming Amol, Comments included below. Mike > -----Original Message----- > From: Sukerkar, Amol N <amol.n.sukerkar@intel.com> > Sent: Friday, February 14, 2020 10:06 AM > 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>; Gao, Liming > <liming.gao@intel.com> > Subject: [PATCH v2 2/2] CryptoPkg/BaseHashApiLib: > Change PcdHashApiLibPolicy type to FixedAtBuild > > Ref: > https://bugzilla.tianocore.org/show_bug.cgi?id=2511 > > This commit changes the PCD PcdHashApiLibPolicy to the > type > PcdsFixedAtBuild so as to be able to optimize away the > unused hashing > algorithms in HashApiLib instance used by a driver. > > Cc: Jiewen Yao <jiewen.yao@intel.com> > Cc: Jian J Wang <jian.j.wang@intel.com> > Cc: Michael D Kinney <michael.d.kinney@intel.com> > Signed-off-by: Amol N Sukerkar > <amol.n.sukerkar@intel.com> > --- > > Notes: > v2 > - Fixed closed parantheses in the commit message > > CryptoPkg/CryptoPkg.dec | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/CryptoPkg/CryptoPkg.dec > b/CryptoPkg/CryptoPkg.dec > index f185bcc82515..c23af7b87d25 100644 > --- a/CryptoPkg/CryptoPkg.dec > +++ b/CryptoPkg/CryptoPkg.dec > @@ -69,7 +69,7 @@ [PcdsFixedAtBuild] > Pcd/PcdCryptoServiceFamilyEnable.h > } > > -[PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, > PcdsDynamicEx] > +[PcdsFixedAtBuild] This line can be removed. The PCD above is already in a [PcdsFixedAtBuild] section. > ## This PCD indicates the HASH algorithm to > calculate hash of data > # Based on the value set, the required algorithm is > chosen to calculate > # the hash of data.<BR> > -- > 2.16.2.windows.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-02-14 23:22 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-02-14 18:05 [PATCH v2 0/2] Enhancement and Fixes to BaseHashApiLib Sukerkar, Amol N 2020-02-14 18:05 ` [PATCH v2 1/2] CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with TPM 2.0 Implementation Sukerkar, Amol N 2020-02-14 22:29 ` Michael D Kinney 2020-02-14 22:38 ` Michael D Kinney 2020-02-14 23:21 ` Sukerkar, Amol N 2020-02-14 18:05 ` [PATCH v2 2/2] CryptoPkg/BaseHashApiLib: Change PcdHashApiLibPolicy type to FixedAtBuild Sukerkar, Amol N 2020-02-14 22:29 ` Michael D Kinney
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox