public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3 0/2] Enhancement and Fixes to BaseHashApiLib
@ 2020-02-14 23:20 Sukerkar, Amol N
  2020-02-14 23:20 ` [PATCH v3 1/2] CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with TPM 2.0 Implementation Sukerkar, Amol N
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Sukerkar, Amol N @ 2020-02-14 23:20 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 | 121 ++++++--------------
 CryptoPkg/CryptoPkg.dec                           |  18 ++-
 CryptoPkg/CryptoPkg.uni                           |  12 +-
 CryptoPkg/Include/Library/HashApiLib.h            |  16 +--
 4 files changed, 52 insertions(+), 115 deletions(-)

-- 
2.16.2.windows.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/2] CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with TPM 2.0 Implementation
  2020-02-14 23:20 [PATCH v3 0/2] Enhancement and Fixes to BaseHashApiLib Sukerkar, Amol N
@ 2020-02-14 23:20 ` Sukerkar, Amol N
  2020-02-14 23:21 ` [PATCH v3 2/2] CryptoPkg/BaseHashApiLib: Change PcdHashApiLibPolicy type to FixedAtBuild Sukerkar, Amol N
  2020-02-15  0:04 ` [edk2-devel] [PATCH v3 0/2] Enhancement and Fixes to BaseHashApiLib Michael D Kinney
  2 siblings, 0 replies; 6+ messages in thread
From: Sukerkar, Amol N @ 2020-02-14 23:20 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
    
    v3
    - Fixed #ifdef for HashApiLib.h
    - Changed location of IndustryStandard/Tpm20.h from
      HashApiLib.h to BaseHashApiLib.c
    - Changed @ValidRange to @ValidList in CryptoPkg.dec
    - Aligned hash algorithm definitions to match Tpm20.h
      in CryptoPkg.dec and CryptoPkg.uni

 CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c | 121 ++++++--------------
 CryptoPkg/CryptoPkg.dec                           |  16 ++-
 CryptoPkg/CryptoPkg.uni                           |  12 +-
 CryptoPkg/Include/Library/HashApiLib.h            |  16 +--
 4 files changed, 51 insertions(+), 114 deletions(-)

diff --git a/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c b/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c
index 277ef9f0b421..df0b294860dc 100644
--- a/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c
+++ b/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c
@@ -12,6 +12,7 @@
 **/
 
 #include <Base.h>
+#include <IndustryStandard/Tpm20.h>
 #include <Library/BaseLib.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/MemoryAllocationLib.h>
@@ -31,32 +32,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 +74,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 +118,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 +164,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 +208,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 +254,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..25ffeb499cae 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>
+  #     0x0001    - HASH_ALG_SHA1.<BR>
+  #     0x0002    - HASH_ALG_SHA256.<BR>
+  #     0x0004    - HASH_ALG_SHA384.<BR>
+  #     0x0008    - HASH_ALG_SHA512.<BR>
+  #     0x0010    - HASH_ALG_SM3_256.<BR>
   # @Prompt Set policy for hashing unsigned image for Secure Boot.
-  # @ValidRange 0x80000001 | 0x00000001 - 0x00000007
-  gEfiCryptoPkgTokenSpaceGuid.PcdHashApiLibPolicy|0x04|UINT8|0x00000001
+  # @ValidList 0x80000001 | 0x0001, 0x0002, 0x0004, 0x0008, 0x0010
+  gEfiCryptoPkgTokenSpaceGuid.PcdHashApiLibPolicy|0x0002|UINT16|0x00000001
 
 [UserExtensions.TianoCore."ExtraFiles"]
   CryptoPkgExtra.uni
diff --git a/CryptoPkg/CryptoPkg.uni b/CryptoPkg/CryptoPkg.uni
index 2222762f42ee..295027d67692 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>"
+                                                                                        "0x0001  -  HASH_ALG_SHA1.<BR>\n"
+                                                                                        "0x0002  -  HASH_ALG_SHA256.<BR>\n"
+                                                                                        "0x0004  -  HASH_ALG_SHA384.<BR>\n"
+                                                                                        "0x0008  -  HASH_ALG_SHA512.<BR>\n"
+                                                                                        "0x0010  -  HASH_ALG_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..17250505fda1 100644
--- a/CryptoPkg/Include/Library/HashApiLib.h
+++ b/CryptoPkg/Include/Library/HashApiLib.h
@@ -9,23 +9,11 @@
 
 **/
 
-#ifndef __BASEHASHAPILIB_H_
-#define __BASEHASHAPILIB_H_
+#ifndef __HASH_API_LIB_H_
+#define __HASH_API_LIB_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] 6+ messages in thread

* [PATCH v3 2/2] CryptoPkg/BaseHashApiLib: Change PcdHashApiLibPolicy type to FixedAtBuild
  2020-02-14 23:20 [PATCH v3 0/2] Enhancement and Fixes to BaseHashApiLib Sukerkar, Amol N
  2020-02-14 23:20 ` [PATCH v3 1/2] CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with TPM 2.0 Implementation Sukerkar, Amol N
@ 2020-02-14 23:21 ` Sukerkar, Amol N
  2020-02-15  0:04 ` [edk2-devel] [PATCH v3 0/2] Enhancement and Fixes to BaseHashApiLib Michael D Kinney
  2 siblings, 0 replies; 6+ messages in thread
From: Sukerkar, Amol N @ 2020-02-14 23:21 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 25ffeb499cae..01e9dbf1141f 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] 6+ messages in thread

* Re: [edk2-devel] [PATCH v3 0/2] Enhancement and Fixes to BaseHashApiLib
  2020-02-14 23:20 [PATCH v3 0/2] Enhancement and Fixes to BaseHashApiLib Sukerkar, Amol N
  2020-02-14 23:20 ` [PATCH v3 1/2] CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with TPM 2.0 Implementation Sukerkar, Amol N
  2020-02-14 23:21 ` [PATCH v3 2/2] CryptoPkg/BaseHashApiLib: Change PcdHashApiLibPolicy type to FixedAtBuild Sukerkar, Amol N
@ 2020-02-15  0:04 ` Michael D Kinney
  2020-02-15  0:11   ` Sukerkar, Amol N
  2 siblings, 1 reply; 6+ messages in thread
From: Michael D Kinney @ 2020-02-15  0:04 UTC (permalink / raw)
  To: devel@edk2.groups.io, Sukerkar, Amol N, Kinney, Michael D
  Cc: Yao, Jiewen, Wang, Jian J, Agrawal, Sachin, Gao, Liming

Amol,

Thanks for the quick update.

I see you changed the PCD to type UINT16.  I think this was
based on Jiewen feedback to use a set of TPM specific 
algorithms defines that were UINT16.

However, the HASH define values being used are 32-bits
and everywhere else that those defines values are used,
they are also 32-bits.

I recommend we keep the type UINT32, and use 8 digit
hex values in all references to the values and valid
value checks.

Thanks,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On
> Behalf Of Sukerkar, Amol N
> Sent: Friday, February 14, 2020 3:21 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>; Gao, Liming
> <liming.gao@intel.com>
> Subject: [edk2-devel] [PATCH v3 0/2] Enhancement and
> Fixes to BaseHashApiLib
> 
> 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 |
> 121 ++++++--------------
>  CryptoPkg/CryptoPkg.dec                           |
> 18 ++-
>  CryptoPkg/CryptoPkg.uni                           |
> 12 +-
>  CryptoPkg/Include/Library/HashApiLib.h            |
> 16 +--
>  4 files changed, 52 insertions(+), 115 deletions(-)
> 
> --
> 2.16.2.windows.1
> 
> 
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [edk2-devel] [PATCH v3 0/2] Enhancement and Fixes to BaseHashApiLib
  2020-02-15  0:04 ` [edk2-devel] [PATCH v3 0/2] Enhancement and Fixes to BaseHashApiLib Michael D Kinney
@ 2020-02-15  0:11   ` Sukerkar, Amol N
  2020-02-15  0:13     ` Michael D Kinney
  0 siblings, 1 reply; 6+ messages in thread
From: Sukerkar, Amol N @ 2020-02-15  0:11 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,

Although the values were listed in 8-digit hex format, the PCD, PcdHashApiLibPolicy itself was of type UINT8 earlier that was changed to UINT16. I agree changing the type to UINT32 will align with all the other PCDs implementing TCG spec (and using Tpm20.h defines). Can you confirm my understanding is accurate?

Thanks,
Amol

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com> 
Sent: Friday, February 14, 2020 5:04 PM
To: devel@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@intel.com>; 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: [edk2-devel] [PATCH v3 0/2] Enhancement and Fixes to BaseHashApiLib

Amol,

Thanks for the quick update.

I see you changed the PCD to type UINT16.  I think this was based on Jiewen feedback to use a set of TPM specific algorithms defines that were UINT16.

However, the HASH define values being used are 32-bits and everywhere else that those defines values are used, they are also 32-bits.

I recommend we keep the type UINT32, and use 8 digit hex values in all references to the values and valid value checks.

Thanks,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of 
> Sukerkar, Amol N
> Sent: Friday, February 14, 2020 3:21 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>; Gao, Liming <liming.gao@intel.com>
> Subject: [edk2-devel] [PATCH v3 0/2] Enhancement and Fixes to 
> BaseHashApiLib
> 
> 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 |
> 121 ++++++--------------
>  CryptoPkg/CryptoPkg.dec                           |
> 18 ++-
>  CryptoPkg/CryptoPkg.uni                           |
> 12 +-
>  CryptoPkg/Include/Library/HashApiLib.h            |
> 16 +--
>  4 files changed, 52 insertions(+), 115 deletions(-)
> 
> --
> 2.16.2.windows.1
> 
> 
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [edk2-devel] [PATCH v3 0/2] Enhancement and Fixes to BaseHashApiLib
  2020-02-15  0:11   ` Sukerkar, Amol N
@ 2020-02-15  0:13     ` Michael D Kinney
  0 siblings, 0 replies; 6+ messages in thread
From: Michael D Kinney @ 2020-02-15  0:13 UTC (permalink / raw)
  To: Sukerkar, Amol N, devel@edk2.groups.io, Kinney, Michael D
  Cc: Yao, Jiewen, Wang, Jian J, Agrawal, Sachin, Gao, Liming

Yes.  That is correct.

Mike

> -----Original Message-----
> From: Sukerkar, Amol N <amol.n.sukerkar@intel.com>
> Sent: Friday, February 14, 2020 4:12 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>; Gao, Liming
> <liming.gao@intel.com>; Sukerkar, Amol N
> <amol.n.sukerkar@intel.com>
> Subject: RE: [edk2-devel] [PATCH v3 0/2] Enhancement
> and Fixes to BaseHashApiLib
> 
> Hi Mike,
> 
> Although the values were listed in 8-digit hex format,
> the PCD, PcdHashApiLibPolicy itself was of type UINT8
> earlier that was changed to UINT16. I agree changing
> the type to UINT32 will align with all the other PCDs
> implementing TCG spec (and using Tpm20.h defines). Can
> you confirm my understanding is accurate?
> 
> Thanks,
> Amol
> 
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Friday, February 14, 2020 5:04 PM
> To: devel@edk2.groups.io; Sukerkar, Amol N
> <amol.n.sukerkar@intel.com>; 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: [edk2-devel] [PATCH v3 0/2] Enhancement
> and Fixes to BaseHashApiLib
> 
> Amol,
> 
> Thanks for the quick update.
> 
> I see you changed the PCD to type UINT16.  I think this
> was based on Jiewen feedback to use a set of TPM
> specific algorithms defines that were UINT16.
> 
> However, the HASH define values being used are 32-bits
> and everywhere else that those defines values are used,
> they are also 32-bits.
> 
> I recommend we keep the type UINT32, and use 8 digit
> hex values in all references to the values and valid
> value checks.
> 
> Thanks,
> 
> Mike
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On
> Behalf Of
> > Sukerkar, Amol N
> > Sent: Friday, February 14, 2020 3:21 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>; Gao, Liming
> <liming.gao@intel.com>
> > Subject: [edk2-devel] [PATCH v3 0/2] Enhancement and
> Fixes to
> > BaseHashApiLib
> >
> > 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 |
> > 121 ++++++--------------
> >  CryptoPkg/CryptoPkg.dec                           |
> > 18 ++-
> >  CryptoPkg/CryptoPkg.uni                           |
> > 12 +-
> >  CryptoPkg/Include/Library/HashApiLib.h            |
> > 16 +--
> >  4 files changed, 52 insertions(+), 115 deletions(-)
> >
> > --
> > 2.16.2.windows.1
> >
> >
> > 
> 
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-02-15  0:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-02-14 23:20 [PATCH v3 0/2] Enhancement and Fixes to BaseHashApiLib Sukerkar, Amol N
2020-02-14 23:20 ` [PATCH v3 1/2] CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with TPM 2.0 Implementation Sukerkar, Amol N
2020-02-14 23:21 ` [PATCH v3 2/2] CryptoPkg/BaseHashApiLib: Change PcdHashApiLibPolicy type to FixedAtBuild Sukerkar, Amol N
2020-02-15  0:04 ` [edk2-devel] [PATCH v3 0/2] Enhancement and Fixes to BaseHashApiLib Michael D Kinney
2020-02-15  0:11   ` Sukerkar, Amol N
2020-02-15  0:13     ` 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