public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [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

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

* 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

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