* [PATCH v6 0/2] CryptoPkg/HashApiLib: Implement Unified Hash Calculation API
@ 2020-01-28 18:03 Sukerkar, Amol N
2020-01-28 18:03 ` [PATCH v6 1/2] CryptoPkg: Add CryptoPkg Token Space GUID Sukerkar, Amol N
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Sukerkar, Amol N @ 2020-01-28 18:03 UTC (permalink / raw)
To: devel
Cc: michael.d.kinney, jiewen.yao, jian.j.wang, sachin.agrawal,
srinivas.musti, subash.lakkimsetti
Currently, the UEFI drivers using the SHA/SM3 hashing algorithms use hard-coded
API to calculate the hash, for instance, sha_256(...), etc. Since SHA384 and/or
SM3_256 are being increasingly adopted for robustness, it becomes cumbersome to
modify each driver that calls into hash calculating API.
To better achieve this, we are proposing a Unified API, which can be used by UEFI
drivers, that provides the drivers with flexibility to use the desired hashing
algorithm based on the required robnustness.
Alternatively, the design document is also attached to Bugzilla,
https://bugzilla.tianocore.org/show_bug.cgi?id=2151.
Sukerkar, Amol N (2):
CryptoPkg: Add CryptoPkg Token Space GUID
CryptoPkg/HashApiLib: Implement Unified Hash Calculation API
CryptoPkg/Library/HashApiLib/HashApiLib.c | 333 ++++++++++++++++++++
CryptoPkg/CryptoPkg.dec | 27 +-
CryptoPkg/CryptoPkg.dsc | 7 +-
CryptoPkg/CryptoPkg.uni | 17 +
CryptoPkg/Include/Library/HashApiLib.h | 122 +++++++
CryptoPkg/Library/HashApiLib/HashApiLib.inf | 45 +++
CryptoPkg/Library/HashApiLib/HashApiLib.uni | 17 +
7 files changed, 566 insertions(+), 2 deletions(-)
create mode 100644 CryptoPkg/Library/HashApiLib/HashApiLib.c
create mode 100644 CryptoPkg/Include/Library/HashApiLib.h
create mode 100644 CryptoPkg/Library/HashApiLib/HashApiLib.inf
create mode 100644 CryptoPkg/Library/HashApiLib/HashApiLib.uni
--
2.16.2.windows.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v6 1/2] CryptoPkg: Add CryptoPkg Token Space GUID
2020-01-28 18:03 [PATCH v6 0/2] CryptoPkg/HashApiLib: Implement Unified Hash Calculation API Sukerkar, Amol N
@ 2020-01-28 18:03 ` Sukerkar, Amol N
2020-01-28 18:03 ` [PATCH v6 2/2] CryptoPkg/HashApiLib: Implement Unified Hash Calculation API Sukerkar, Amol N
2020-01-29 20:10 ` [PATCH v6 0/2] " Michael D Kinney
2 siblings, 0 replies; 7+ messages in thread
From: Sukerkar, Amol N @ 2020-01-28 18:03 UTC (permalink / raw)
To: devel
Cc: michael.d.kinney, jiewen.yao, jian.j.wang, sachin.agrawal,
srinivas.musti, subash.lakkimsetti
Added CryptoPkg Token Space GUID to be able to define PCDs.
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: Sukerkar, Amol N <amol.n.sukerkar@intel.com>
---
Notes:
v6
- removed file CryptoPkgTokenSpace.h
CryptoPkg/CryptoPkg.dec | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/CryptoPkg/CryptoPkg.dec b/CryptoPkg/CryptoPkg.dec
index 08bedd57daad..5986a988f790 100644
--- a/CryptoPkg/CryptoPkg.dec
+++ b/CryptoPkg/CryptoPkg.dec
@@ -4,7 +4,7 @@
# This Package provides cryptographic-related libraries for UEFI security modules.
# It also provides a test application to test libraries.
#
-# Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
@@ -33,5 +33,9 @@ [LibraryClasses]
##
TlsLib|Include/Library/TlsLib.h
+[Guids]
+ ## Security package token space guid.
+ gEfiCryptoPkgTokenSpaceGuid = { 0xd3fb176, 0x9569, 0x4d51, { 0xa3, 0xef, 0x7d, 0x61, 0xc6, 0x4f, 0xea, 0xba }}
+
[UserExtensions.TianoCore."ExtraFiles"]
CryptoPkgExtra.uni
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v6 2/2] CryptoPkg/HashApiLib: Implement Unified Hash Calculation API
2020-01-28 18:03 [PATCH v6 0/2] CryptoPkg/HashApiLib: Implement Unified Hash Calculation API Sukerkar, Amol N
2020-01-28 18:03 ` [PATCH v6 1/2] CryptoPkg: Add CryptoPkg Token Space GUID Sukerkar, Amol N
@ 2020-01-28 18:03 ` Sukerkar, Amol N
2020-01-29 20:10 ` [PATCH v6 0/2] " Michael D Kinney
2 siblings, 0 replies; 7+ messages in thread
From: Sukerkar, Amol N @ 2020-01-28 18:03 UTC (permalink / raw)
To: devel
Cc: michael.d.kinney, jiewen.yao, jian.j.wang, sachin.agrawal,
srinivas.musti, subash.lakkimsetti
This commit introduces a Unified Hash API to calculate hash using a
hashing algorithm specified by the PCD, PcdHashApiLibPolicy. This library
interfaces with the various hashing API, such as, MD4, MD5, SHA1, SHA256,
SHA512 and SM3_256 implemented in BaseCryptLib. The user can calculate
the desired hash by setting PcdHashApiLibPolicy to appropriate value.
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: Sukerkar, Amol N <amol.n.sukerkar@intel.com>
---
Notes:
v6
- Changed the PCD name to PcdHashApiLibPolicy
- Changed the lib nabme to HashApiLib
- Changes lib MODULE_TYPE to BASE
CryptoPkg/Library/HashApiLib/HashApiLib.c | 333 ++++++++++++++++++++
CryptoPkg/CryptoPkg.dec | 21 ++
CryptoPkg/CryptoPkg.dsc | 7 +-
CryptoPkg/CryptoPkg.uni | 17 +
CryptoPkg/Include/Library/HashApiLib.h | 122 +++++++
CryptoPkg/Library/HashApiLib/HashApiLib.inf | 45 +++
CryptoPkg/Library/HashApiLib/HashApiLib.uni | 17 +
7 files changed, 561 insertions(+), 1 deletion(-)
diff --git a/CryptoPkg/Library/HashApiLib/HashApiLib.c b/CryptoPkg/Library/HashApiLib/HashApiLib.c
new file mode 100644
index 000000000000..0f5b594fb7c0
--- /dev/null
+++ b/CryptoPkg/Library/HashApiLib/HashApiLib.c
@@ -0,0 +1,333 @@
+/** @file
+ Unified Hash API Implementation
+
+ This file implements the Unified Hash API.
+
+ This API, when called, will calculate the Hash using the
+ hashing algorithm specified by PcdHashApiLibPolicy.
+
+ Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Base.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/BaseCryptLib.h>
+#include <Library/DebugLib.h>
+#include <Library/PcdLib.h>
+#include <Library/HashApiLib.h>
+
+/**
+ Retrieves the size, in bytes, of the context buffer required for hash operations.
+
+ @return The size, in bytes, of the context buffer required for hash operations.
+
+**/
+UINTN
+EFIAPI
+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:
+ return Sha1GetContextSize ();
+ break;
+
+ case HASH_API_ALGO_SHA256:
+ return Sha256GetContextSize ();
+ break;
+
+ case HASH_API_ALGO_SHA384:
+ return Sha384GetContextSize ();
+ break;
+
+ case HASH_API_ALGO_SHA512:
+ return Sha512GetContextSize ();
+ break;
+
+ case HASH_API_ALGO_SM3_256:
+ return Sm3GetContextSize ();
+ break;
+
+ default:
+ ASSERT (FALSE);
+ return 0;
+ break;
+ }
+}
+
+/**
+ Init hash sequence.
+
+ @param[out] HashContext Hash context.
+
+ @retval TRUE Hash start and HashHandle returned.
+ @retval FALSE Hash Init unsuccessful.
+**/
+BOOLEAN
+EFIAPI
+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:
+ return Sha1Init (HashContext);
+ break;
+
+ case HASH_API_ALGO_SHA256:
+ return Sha256Init (HashContext);
+ break;
+
+ case HASH_API_ALGO_SHA384:
+ return Sha384Init (HashContext);
+ break;
+
+ case HASH_API_ALGO_SHA512:
+ return Sha512Init (HashContext);
+ break;
+
+ case HASH_API_ALGO_SM3_256:
+ return Sm3Init (HashContext);
+ break;
+
+ default:
+ ASSERT (FALSE);
+ return FALSE;
+ break;
+ }
+}
+
+/**
+ Makes a copy of an existing hash context.
+
+ @param[in] HashContext Hash context.
+ @param[out] NewHashContext New copy of hash context.
+
+ @retval TRUE Hash context copy succeeded.
+ @retval FALSE Hash context copy failed.
+
+**/
+BOOLEAN
+EFIAPI
+HashApiDuplicate (
+ IN HASH_API_CONTEXT *HashContext,
+ OUT VOID *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:
+ return Sha1Duplicate (HashContext, NewHashContext);
+ break;
+
+ case HASH_API_ALGO_SHA256:
+ return Sha256Duplicate (HashContext, NewHashContext);
+ break;
+
+ case HASH_API_ALGO_SHA384:
+ return Sha384Duplicate (HashContext, NewHashContext);
+ break;
+
+ case HASH_API_ALGO_SHA512:
+ return Sha512Duplicate (HashContext, NewHashContext);
+ break;
+
+ case HASH_API_ALGO_SM3_256:
+ return Sm3Duplicate (HashContext, NewHashContext);
+ break;
+
+ default:
+ ASSERT (FALSE);
+ return FALSE;
+ break;
+ }
+}
+
+/**
+ Update hash data.
+
+ @param[in] HashContext Hash context.
+ @param[in] DataToHash Data to be hashed.
+ @param[in] DataToHashLen Data size.
+
+ @retval TRUE Hash updated.
+ @retval FALSE Hash updated unsuccessful.
+**/
+BOOLEAN
+EFIAPI
+HashApiUpdate (
+ IN HASH_API_CONTEXT *HashContext,
+ IN VOID *DataToHash,
+ 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:
+ return Sha1Update (HashContext, DataToHash, DataToHashLen);
+ break;
+
+ case HASH_API_ALGO_SHA256:
+ return Sha256Update (HashContext, DataToHash, DataToHashLen);
+ break;
+
+ case HASH_API_ALGO_SHA384:
+ return Sha384Update (HashContext, DataToHash, DataToHashLen);
+ break;
+
+ case HASH_API_ALGO_SHA512:
+ return Sha512Update (HashContext, DataToHash, DataToHashLen);
+ break;
+
+ case HASH_API_ALGO_SM3_256:
+ return Sm3Update (HashContext, DataToHash, DataToHashLen);
+ break;
+
+ default:
+ ASSERT (FALSE);
+ return FALSE;
+ break;
+ }
+}
+
+/**
+ Hash complete.
+
+ @param[in] HashContext Hash context.
+ @param[out] Digest Hash Digest.
+
+ @retval TRUE Hash complete and Digest is returned.
+ @retval FALSE Hash complete unsuccessful.
+**/
+BOOLEAN
+EFIAPI
+HashApiFinal (
+ IN HASH_API_CONTEXT *HashContext,
+ 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:
+ return Sha1Final (HashContext, Digest);
+ break;
+
+ case HASH_API_ALGO_SHA256:
+ return Sha256Final (HashContext, Digest);
+ break;
+
+ case HASH_API_ALGO_SHA384:
+ return Sha384Final (HashContext, Digest);
+ break;
+
+ case HASH_API_ALGO_SHA512:
+ return Sha512Final (HashContext, Digest);
+ break;
+
+ case HASH_API_ALGO_SM3_256:
+ return Sm3Final (HashContext, Digest);
+ break;
+
+ default:
+ ASSERT (FALSE);
+ return FALSE;
+ break;
+ }
+}
+
+/**
+ Computes hash message digest of a input data buffer.
+
+ @param[in] DataToHash Data to be hashed.
+ @param[in] DataToHashLen Data size.
+ @param[out] Digest Hash Digest.
+
+ @retval TRUE Hash digest computation succeeded.
+ @retval FALSE Hash digest computation failed.
+
+**/
+BOOLEAN
+EFIAPI
+HashApiHashAll (
+ IN CONST VOID *DataToHash,
+ IN UINTN DataToHashLen,
+ 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:
+ return Sha1HashAll (DataToHash, DataToHashLen, Digest);
+ break;
+
+ case HASH_API_ALGO_SHA256:
+ return Sha256HashAll (DataToHash, DataToHashLen, Digest);
+ break;
+
+ case HASH_API_ALGO_SHA384:
+ return Sha384HashAll (DataToHash, DataToHashLen, Digest);
+ break;
+
+ case HASH_API_ALGO_SHA512:
+ return Sha512HashAll (DataToHash, DataToHashLen, Digest);
+ break;
+
+ case HASH_API_ALGO_SM3_256:
+ return Sm3HashAll (DataToHash, DataToHashLen, Digest);
+ break;
+
+ default:
+ ASSERT (FALSE);
+ return FALSE;
+ break;
+ }
+}
diff --git a/CryptoPkg/CryptoPkg.dec b/CryptoPkg/CryptoPkg.dec
index 5986a988f790..bf0a408099db 100644
--- a/CryptoPkg/CryptoPkg.dec
+++ b/CryptoPkg/CryptoPkg.dec
@@ -33,9 +33,30 @@ [LibraryClasses]
##
TlsLib|Include/Library/TlsLib.h
+ ## @libraryclass Provides Unified API for different hash implementations.
+ #
+ HashApiLib|Include/Library/HashApiLib.h
+
[Guids]
## Security package token space guid.
gEfiCryptoPkgTokenSpaceGuid = { 0xd3fb176, 0x9569, 0x4d51, { 0xa3, 0xef, 0x7d, 0x61, 0xc6, 0x4f, 0xea, 0xba }}
+[PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
+ ## This PCD indicates the HASH algorithm to verify unsigned PE/COFF image
+ # Based on the value set, the required algorithm is chosen to verify
+ # the unsigned image during Secure Boot.<BR>
+ # The hashing algorithm selected must match the hashing algorithm used to
+ # hash the image to be added to DB using tools such as KeyEnroll.<BR>
+ # 0x00000001 - MD4.<BR>
+ # 0x00000002 - MD5.<BR>
+ # 0x00000003 - SHA1.<BR>
+ # 0x00000004 - SHA256.<BR>
+ # 0x00000005 - SHA384.<BR>
+ # 0x00000006 - SHA512.<BR>
+ # 0x00000007 - SM3_256.<BR>
+ # @Prompt Set policy for hashing unsigned image for Secure Boot.
+ # @ValidRange 0x80000001 | 0x00000001 - 0x00000007
+ gEfiCryptoPkgTokenSpaceGuid.PcdHashApiLibPolicy|0x04|UINT8|0x00000001
+
[UserExtensions.TianoCore."ExtraFiles"]
CryptoPkgExtra.uni
diff --git a/CryptoPkg/CryptoPkg.dsc b/CryptoPkg/CryptoPkg.dsc
index ec43c1f0a47e..1a9e70e5bf12 100644
--- a/CryptoPkg/CryptoPkg.dsc
+++ b/CryptoPkg/CryptoPkg.dsc
@@ -1,7 +1,7 @@
## @file
# Cryptographic Library Package for UEFI Security Implementation.
#
-# Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
@@ -62,15 +62,19 @@ [LibraryClasses.ARM]
[LibraryClasses.common.PEIM]
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
+ HashApiLib|CryptoPkg/Library/HashApiLib/HashApiLib.inf
[LibraryClasses.common.DXE_DRIVER]
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
+ HashApiLib|CryptoPkg/Library/HashApiLib/HashApiLib.inf
[LibraryClasses.common.DXE_RUNTIME_DRIVER]
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
+ HashApiLib|CryptoPkg/Library/HashApiLib/HashApiLib.inf
[LibraryClasses.common.DXE_SMM_DRIVER]
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
+ HashApiLib|CryptoPkg/Library/HashApiLib/HashApiLib.inf
[LibraryClasses.common.UEFI_DRIVER]
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
@@ -120,6 +124,7 @@ [Components]
CryptoPkg/Library/TlsLibNull/TlsLibNull.inf
CryptoPkg/Library/OpensslLib/OpensslLib.inf
CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
+ CryptoPkg/Library/HashApiLib/HashApiLib.inf
[Components.IA32, Components.X64]
CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
diff --git a/CryptoPkg/CryptoPkg.uni b/CryptoPkg/CryptoPkg.uni
index beb0036ef583..8da39fa0f613 100644
--- a/CryptoPkg/CryptoPkg.uni
+++ b/CryptoPkg/CryptoPkg.uni
@@ -17,3 +17,20 @@
+#string STR_gEfiCryptoPkgTokenSpaceGuid_PcdHashApiLibPolicy_PROMPT #language en-US "HASH algorithm to verify unsigned PE/COFF image"
+
+#string STR_gEfiCryptoPkgTokenSpaceGuid_PcdHashApiLibPolicy_HELP #language en-US "This PCD indicates the HASH algorithm to verify unsigned PE/COFF image.<BR><BR>\n"
+ "Based on the value set, the required algorithm is chosen to verify\n"
+ "the unsigned image during Secure Boot.<BR>\n"
+ "The hashing algorithm selected must match the hashing algorithm used to\n"
+ "hash the image to be added to DB using tools such as KeyEnroll.<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>"
+
+
+
diff --git a/CryptoPkg/Include/Library/HashApiLib.h b/CryptoPkg/Include/Library/HashApiLib.h
new file mode 100644
index 000000000000..354b0c02b6ab
--- /dev/null
+++ b/CryptoPkg/Include/Library/HashApiLib.h
@@ -0,0 +1,122 @@
+/** @file
+ Unified Hash API Defines
+
+ This API when called will calculate the Hash using the
+ hashing algorithm specified by PcdHashApiLibPolicy.
+
+ Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef __HASHAPILIB_H_
+#define __HASHAPILIB_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 of the context buffer required for hash operations.
+
+ @return The size of the context buffer required for hash operations (in bytes).
+**/
+UINTN
+EFIAPI
+HashApiGetContextSize (
+ VOID
+);
+
+/**
+ Init hash sequence.
+
+ @param[out] HashContext Hash context.
+
+ @retval TRUE Hash start and HashHandle returned.
+ @retval FALSE Hash Init unsuccessful.
+**/
+BOOLEAN
+EFIAPI
+HashApiInit (
+ OUT HASH_API_CONTEXT *HashContext
+);
+
+/**
+ Makes a copy of an existing hash context.
+
+ @param[in] HashContext Hash context.
+ @param[out] NewHashContext New copy of hash context.
+
+ @retval TRUE Hash context copy succeeded.
+ @retval FALSE Hash context copy failed.
+**/
+BOOLEAN
+EFIAPI
+HashApiDuplicate (
+ IN HASH_API_CONTEXT *HashContext,
+ OUT VOID *NewHashContext
+);
+
+/**
+ Update hash data.
+
+ @param[in] HashContext Hash context.
+ @param[in] DataToHash Data to be hashed.
+ @param[in] DataToHashLen Data size.
+
+ @retval TRUE Hash updated.
+ @retval FALSE Hash updated unsuccessful.
+**/
+BOOLEAN
+EFIAPI
+HashApiUpdate (
+ IN HASH_API_CONTEXT *HashContext,
+ IN VOID *DataToHash,
+ IN UINTN DataToHashLen
+);
+
+/**
+ Hash complete.
+
+ @param[in] HashContext Hash context.
+ @param[out] Digest Hash Digest.
+
+ @retval TRUE Hash complete and Digest is returned.
+ @retval FALSE Hash complete unsuccessful.
+**/
+BOOLEAN
+EFIAPI
+HashApiFinal (
+ IN HASH_API_CONTEXT *HashContext,
+ OUT UINT8 *Digest
+);
+
+/**
+ Computes hash message digest of a input data buffer.
+
+ @param[in] DataToHash Data to be hashed.
+ @param[in] DataToHashLen Data size.
+ @param[out] Digest Hash Digest.
+
+ @retval TRUE Hash digest computation succeeded.
+ @retval FALSE Hash digest computation failed.
+**/
+BOOLEAN
+EFIAPI
+HashApiHashAll (
+ IN CONST VOID *DataToHash,
+ IN UINTN DataToHashLen,
+ OUT UINT8 *Digest
+);
+
+#endif
diff --git a/CryptoPkg/Library/HashApiLib/HashApiLib.inf b/CryptoPkg/Library/HashApiLib/HashApiLib.inf
new file mode 100644
index 000000000000..1eeda4dd55e6
--- /dev/null
+++ b/CryptoPkg/Library/HashApiLib/HashApiLib.inf
@@ -0,0 +1,45 @@
+## @file
+# Provides Unified API for Hash Calculation
+#
+# This library is HashApiLib. It will redirect hash request to each
+# individual hash API, such as SHA1, SHA256, SHA384, SM3 based on
+# hashing algorithm specified by PcdHashApiLibPolicy.
+#
+# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = HashApiLib
+ MODULE_UNI_FILE = HashApiLib.uni
+ FILE_GUID = DDCBCFBA-8EEB-488a-96D6-097831A6E50B
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = HashApiLib
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = IA32 X64
+#
+
+[Sources]
+ HashApiLib.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ CryptoPkg/CryptoPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+ BaseLib
+ BaseMemoryLib
+ DebugLib
+ MemoryAllocationLib
+ BaseCryptLib
+ PcdLib
+
+[Pcd]
+ gEfiCryptoPkgTokenSpaceGuid.PcdHashApiLibPolicy ## CONSUMES
diff --git a/CryptoPkg/Library/HashApiLib/HashApiLib.uni b/CryptoPkg/Library/HashApiLib/HashApiLib.uni
new file mode 100644
index 000000000000..2e09642a3197
--- /dev/null
+++ b/CryptoPkg/Library/HashApiLib/HashApiLib.uni
@@ -0,0 +1,17 @@
+// /** @file
+// Provides Unified API for Hash Calculation
+//
+// This library is HashApiLib. It will redirect hash request to each
+// individual hash API, such as SHA1, SHA256, SHA384, SM3 based on
+// hashing algorithm specified by PcdHashApiLibPolicy.
+//
+// Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+//
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+// **/
+
+
+#string STR_MODULE_ABSTRACT #language en-US "Provides hash service by specified hash handler"
+
+#string STR_MODULE_DESCRIPTION #language en-US "This library is Unified Hash API. It will redirect hash request to the hash handler specified by PcdHashApiLibPolicy."
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v6 0/2] CryptoPkg/HashApiLib: Implement Unified Hash Calculation API
2020-01-28 18:03 [PATCH v6 0/2] CryptoPkg/HashApiLib: Implement Unified Hash Calculation API Sukerkar, Amol N
2020-01-28 18:03 ` [PATCH v6 1/2] CryptoPkg: Add CryptoPkg Token Space GUID Sukerkar, Amol N
2020-01-28 18:03 ` [PATCH v6 2/2] CryptoPkg/HashApiLib: Implement Unified Hash Calculation API Sukerkar, Amol N
@ 2020-01-29 20:10 ` Michael D Kinney
2020-01-29 23:09 ` Sukerkar, Amol N
2 siblings, 1 reply; 7+ messages in thread
From: Michael D Kinney @ 2020-01-29 20:10 UTC (permalink / raw)
To: Sukerkar, Amol N, devel@edk2.groups.io, Kinney, Michael D
Cc: Yao, Jiewen, Wang, Jian J, Agrawal, Sachin, Musti, Srinivas,
Lakkimsetti, Subash
Amol,
1) Typo in CryptoPkg.dec. Should be Crypto Package, not Security package.
[Guids]
## Security package token space guid.
2) CryptoPkg.dec/uni. I see the default value for PcdHashApiLibPolicy
is 0x04. This is documented to be SHA256. The DEC/UNI file
descriptions of this PCD should state that the default policy is
SHA256. This makes it clear to platform developers that maintain
DSC files what the default policy is.
3) CryptoPkg.dsc: The same HashApiLib instance is used for all module types
so a single mapping can be moved to [LibraryClasses] section and the
DSC file and removed from the [LibraryClass.common.<ModuleType>] sections.
4) The name of the HashApiLib instance should be "BaseHashApiLib" and the
should be in the CryptoPkg/Library/BaseHashApiLib directory with
files BashHashApiLib.inf, BaseHashApiLib.c, and BaseHashApiLib.uni.
BASE_NAME in BaseHashApiLib.iunf should also be BaseHashApiLib.
5) In order to be consistent with other EDK II context typedefs, I recommend
typedef VOID *HASH_API_CONTEXT;
Also update APIs to use HashContext instead of *HashContext.
6) HashApiDuplicate() - The NewHashContext parameter should be type
HASH_API_CONTEXT.
7) HashApiLib.inf - I think you can remove MdeModulePkg.dec from [Packages]
Thanks,
Mike
> -----Original Message-----
> From: Sukerkar, Amol N <amol.n.sukerkar@intel.com>
> Sent: Tuesday, January 28, 2020 10:04 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>; Musti, Srinivas
> <srinivas.musti@intel.com>; Lakkimsetti, Subash
> <subash.lakkimsetti@intel.com>
> Subject: [PATCH v6 0/2] CryptoPkg/HashApiLib: Implement
> Unified Hash Calculation API
>
> Currently, the UEFI drivers using the SHA/SM3 hashing
> algorithms use hard-coded
> API to calculate the hash, for instance, sha_256(...),
> etc. Since SHA384 and/or
> SM3_256 are being increasingly adopted for robustness,
> it becomes cumbersome to
> modify each driver that calls into hash calculating API.
>
> To better achieve this, we are proposing a Unified API,
> which can be used by UEFI
> drivers, that provides the drivers with flexibility to
> use the desired hashing
> algorithm based on the required robnustness.
>
> Alternatively, the design document is also attached to
> Bugzilla,
> https://bugzilla.tianocore.org/show_bug.cgi?id=2151.
>
> Sukerkar, Amol N (2):
> CryptoPkg: Add CryptoPkg Token Space GUID
> CryptoPkg/HashApiLib: Implement Unified Hash
> Calculation API
>
> CryptoPkg/Library/HashApiLib/HashApiLib.c | 333
> ++++++++++++++++++++
> CryptoPkg/CryptoPkg.dec | 27 +-
> CryptoPkg/CryptoPkg.dsc | 7 +-
> CryptoPkg/CryptoPkg.uni | 17 +
> CryptoPkg/Include/Library/HashApiLib.h | 122
> +++++++
> CryptoPkg/Library/HashApiLib/HashApiLib.inf | 45 +++
> CryptoPkg/Library/HashApiLib/HashApiLib.uni | 17 +
> 7 files changed, 566 insertions(+), 2 deletions(-)
> create mode 100644
> CryptoPkg/Library/HashApiLib/HashApiLib.c
> create mode 100644
> CryptoPkg/Include/Library/HashApiLib.h
> create mode 100644
> CryptoPkg/Library/HashApiLib/HashApiLib.inf
> create mode 100644
> CryptoPkg/Library/HashApiLib/HashApiLib.uni
>
> --
> 2.16.2.windows.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v6 0/2] CryptoPkg/HashApiLib: Implement Unified Hash Calculation API
2020-01-29 20:10 ` [PATCH v6 0/2] " Michael D Kinney
@ 2020-01-29 23:09 ` Sukerkar, Amol N
2020-01-29 23:16 ` Michael D Kinney
0 siblings, 1 reply; 7+ messages in thread
From: Sukerkar, Amol N @ 2020-01-29 23:09 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io
Cc: Yao, Jiewen, Wang, Jian J, Agrawal, Sachin, Musti, Srinivas,
Lakkimsetti, Subash, Sukerkar, Amol N
Hi Mike,
Question about point 4. Could you help me clear the confusion?
4) The name of the HashApiLib instance should be "BaseHashApiLib" and the
should be in the CryptoPkg/Library/BaseHashApiLib directory with
files BashHashApiLib.inf, BaseHashApiLib.c, and BaseHashApiLib.uni.
BASE_NAME in BaseHashApiLib.iunf should also be BaseHashApiLib.
Perhaps I am not very clear but it appears you are contradicting your earlier feedback:
I have received feedback before against the use of the term "Base" in the name of a library class. It causes confusion because the term "Base" usually applies to the library implementation to describe the module type compatibility of the lib instance. Take BaseCryptLib class as an example. There are instances of this library that are specific to PEI, DXE, SMM, and Runtime. If we look at the entire edk2 repo, there are only 3 lib classes that start with the term "Base":
BaseLib - OK. Single instance of type BASE.
BaseMemoryLib - Confusing. BASE, PEI, DXE versions
BaseCryptLib - Confusing. PEI, DXE, SMM, RT versions
I also see the BaseHashLib service names use "HashApi".
In order to address both the use of the term "Base" and The inconsistency between the lib class name and the lib service names, I recommend the lib class be changed from "BaseHashLib" to "HashApiLib" along with a few other name changes to scope defines and types to the HashApiLib:
BaseHashLib -> HashApiLib
HASH_HANDLE -> HASH_API_CONTEXT
HASH_INVALID -> HASH_API_ALGORITHM_INVALID
HASH_MD4 -> HASH_API_ALGORITHM_MD4
HASH_MD5 -> HASH_API_ALGORITHM_MD5
HASH_SHA1 -> HASH_API_ALGORITHM_SHA1
HASH_SHA256 -> HASH_API_ALGORITHM_SHA256
HASH_SHA384 -> HASH_API_ALGORITHM_SHA384
HASH_SHA512 -> HASH_API_ALGORITHM_SHA512
HASH_SM3_256 -> HASH_API_ALGORITHM_SM3_256
HASH_MAX -> Remove. Not used.
Some file name a directory names changes would also be required to follow this same pattern.
Thanks,
Amol
-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Wednesday, January 29, 2020 1:10 PM
To: Sukerkar, Amol N <amol.n.sukerkar@intel.com>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Agrawal, Sachin <sachin.agrawal@intel.com>; Musti, Srinivas <srinivas.musti@intel.com>; Lakkimsetti, Subash <subash.lakkimsetti@intel.com>
Subject: RE: [PATCH v6 0/2] CryptoPkg/HashApiLib: Implement Unified Hash Calculation API
Amol,
1) Typo in CryptoPkg.dec. Should be Crypto Package, not Security package.
[Guids]
## Security package token space guid.
2) CryptoPkg.dec/uni. I see the default value for PcdHashApiLibPolicy
is 0x04. This is documented to be SHA256. The DEC/UNI file
descriptions of this PCD should state that the default policy is
SHA256. This makes it clear to platform developers that maintain
DSC files what the default policy is.
3) CryptoPkg.dsc: The same HashApiLib instance is used for all module types
so a single mapping can be moved to [LibraryClasses] section and the
DSC file and removed from the [LibraryClass.common.<ModuleType>] sections.
4) The name of the HashApiLib instance should be "BaseHashApiLib" and the
should be in the CryptoPkg/Library/BaseHashApiLib directory with
files BashHashApiLib.inf, BaseHashApiLib.c, and BaseHashApiLib.uni.
BASE_NAME in BaseHashApiLib.iunf should also be BaseHashApiLib.
5) In order to be consistent with other EDK II context typedefs, I recommend
typedef VOID *HASH_API_CONTEXT;
Also update APIs to use HashContext instead of *HashContext.
6) HashApiDuplicate() - The NewHashContext parameter should be type
HASH_API_CONTEXT.
7) HashApiLib.inf - I think you can remove MdeModulePkg.dec from [Packages]
Thanks,
Mike
> -----Original Message-----
> From: Sukerkar, Amol N <amol.n.sukerkar@intel.com>
> Sent: Tuesday, January 28, 2020 10:04 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>; Musti, Srinivas
> <srinivas.musti@intel.com>; Lakkimsetti, Subash
> <subash.lakkimsetti@intel.com>
> Subject: [PATCH v6 0/2] CryptoPkg/HashApiLib: Implement Unified Hash
> Calculation API
>
> Currently, the UEFI drivers using the SHA/SM3 hashing algorithms use
> hard-coded API to calculate the hash, for instance, sha_256(...), etc.
> Since SHA384 and/or
> SM3_256 are being increasingly adopted for robustness, it becomes
> cumbersome to modify each driver that calls into hash calculating API.
>
> To better achieve this, we are proposing a Unified API, which can be
> used by UEFI drivers, that provides the drivers with flexibility to
> use the desired hashing algorithm based on the required robnustness.
>
> Alternatively, the design document is also attached to Bugzilla,
> https://bugzilla.tianocore.org/show_bug.cgi?id=2151.
>
> Sukerkar, Amol N (2):
> CryptoPkg: Add CryptoPkg Token Space GUID
> CryptoPkg/HashApiLib: Implement Unified Hash Calculation API
>
> CryptoPkg/Library/HashApiLib/HashApiLib.c | 333
> ++++++++++++++++++++
> CryptoPkg/CryptoPkg.dec | 27 +-
> CryptoPkg/CryptoPkg.dsc | 7 +-
> CryptoPkg/CryptoPkg.uni | 17 +
> CryptoPkg/Include/Library/HashApiLib.h | 122
> +++++++
> CryptoPkg/Library/HashApiLib/HashApiLib.inf | 45 +++
> CryptoPkg/Library/HashApiLib/HashApiLib.uni | 17 +
> 7 files changed, 566 insertions(+), 2 deletions(-) create mode
> 100644 CryptoPkg/Library/HashApiLib/HashApiLib.c
> create mode 100644
> CryptoPkg/Include/Library/HashApiLib.h
> create mode 100644
> CryptoPkg/Library/HashApiLib/HashApiLib.inf
> create mode 100644
> CryptoPkg/Library/HashApiLib/HashApiLib.uni
>
> --
> 2.16.2.windows.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v6 0/2] CryptoPkg/HashApiLib: Implement Unified Hash Calculation API
2020-01-29 23:09 ` Sukerkar, Amol N
@ 2020-01-29 23:16 ` Michael D Kinney
2020-01-29 23:23 ` Sukerkar, Amol N
0 siblings, 1 reply; 7+ messages in thread
From: Michael D Kinney @ 2020-01-29 23:16 UTC (permalink / raw)
To: Sukerkar, Amol N, devel@edk2.groups.io, Kinney, Michael D
Cc: Yao, Jiewen, Wang, Jian J, Agrawal, Sachin, Musti, Srinivas,
Lakkimsetti, Subash
Hi Amol,
The feedback I received was for the use of the term
"Base" in library class names. You did the correct
change by changing the class name from "BaseHashLib"
to "HashApiLib".
It is correct to use the term "Base" in the name of
a library instance if the library as implemented is
compatible with all module types. The one implementation
of the HashApiLib class is compatible with all module
types, so the library instance name should be "BaseHashApiLib".
Mike
> -----Original Message-----
> From: Sukerkar, Amol N <amol.n.sukerkar@intel.com>
> Sent: Wednesday, January 29, 2020 3:10 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Agrawal, Sachin
> <sachin.agrawal@intel.com>; Musti, Srinivas
> <srinivas.musti@intel.com>; Lakkimsetti, Subash
> <subash.lakkimsetti@intel.com>; Sukerkar, Amol N
> <amol.n.sukerkar@intel.com>
> Subject: RE: [PATCH v6 0/2] CryptoPkg/HashApiLib:
> Implement Unified Hash Calculation API
>
> Hi Mike,
>
> Question about point 4. Could you help me clear the
> confusion?
>
> 4) The name of the HashApiLib instance should be
> "BaseHashApiLib" and the
> should be in the CryptoPkg/Library/BaseHashApiLib
> directory with
> files BashHashApiLib.inf, BaseHashApiLib.c, and
> BaseHashApiLib.uni.
> BASE_NAME in BaseHashApiLib.iunf should also be
> BaseHashApiLib.
>
> Perhaps I am not very clear but it appears you are
> contradicting your earlier feedback:
>
> I have received feedback before against the use of the
> term "Base" in the name of a library class. It causes
> confusion because the term "Base" usually applies to
> the library implementation to describe the module type
> compatibility of the lib instance. Take BaseCryptLib
> class as an example. There are instances of this
> library that are specific to PEI, DXE, SMM, and
> Runtime. If we look at the entire edk2 repo, there are
> only 3 lib classes that start with the term "Base":
>
> BaseLib - OK. Single instance of type BASE.
> BaseMemoryLib - Confusing. BASE, PEI, DXE versions
> BaseCryptLib - Confusing. PEI, DXE, SMM, RT
> versions
>
> I also see the BaseHashLib service names use "HashApi".
> In order to address both the use of the term "Base" and
> The inconsistency between the lib class name and the
> lib service names, I recommend the lib class be changed
> from "BaseHashLib" to "HashApiLib" along with a few
> other name changes to scope defines and types to the
> HashApiLib:
>
> BaseHashLib -> HashApiLib
> HASH_HANDLE -> HASH_API_CONTEXT
> HASH_INVALID -> HASH_API_ALGORITHM_INVALID
> HASH_MD4 -> HASH_API_ALGORITHM_MD4
> HASH_MD5 -> HASH_API_ALGORITHM_MD5
> HASH_SHA1 -> HASH_API_ALGORITHM_SHA1
> HASH_SHA256 -> HASH_API_ALGORITHM_SHA256
> HASH_SHA384 -> HASH_API_ALGORITHM_SHA384
> HASH_SHA512 -> HASH_API_ALGORITHM_SHA512
> HASH_SM3_256 -> HASH_API_ALGORITHM_SM3_256
> HASH_MAX -> Remove. Not used.
>
> Some file name a directory names changes would also be
> required to follow this same pattern.
>
> Thanks,
> Amol
>
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Wednesday, January 29, 2020 1:10 PM
> To: Sukerkar, Amol N <amol.n.sukerkar@intel.com>;
> devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Agrawal, Sachin
> <sachin.agrawal@intel.com>; Musti, Srinivas
> <srinivas.musti@intel.com>; Lakkimsetti, Subash
> <subash.lakkimsetti@intel.com>
> Subject: RE: [PATCH v6 0/2] CryptoPkg/HashApiLib:
> Implement Unified Hash Calculation API
>
> Amol,
>
>
> 1) Typo in CryptoPkg.dec. Should be Crypto Package,
> not Security package.
>
> [Guids]
> ## Security package token space guid.
>
> 2) CryptoPkg.dec/uni. I see the default value for
> PcdHashApiLibPolicy
> is 0x04. This is documented to be SHA256. The
> DEC/UNI file
> descriptions of this PCD should state that the
> default policy is
> SHA256. This makes it clear to platform developers
> that maintain
> DSC files what the default policy is.
>
> 3) CryptoPkg.dsc: The same HashApiLib instance is used
> for all module types
> so a single mapping can be moved to [LibraryClasses]
> section and the
> DSC file and removed from the
> [LibraryClass.common.<ModuleType>] sections.
>
> 4) The name of the HashApiLib instance should be
> "BaseHashApiLib" and the
> should be in the CryptoPkg/Library/BaseHashApiLib
> directory with
> files BashHashApiLib.inf, BaseHashApiLib.c, and
> BaseHashApiLib.uni.
> BASE_NAME in BaseHashApiLib.iunf should also be
> BaseHashApiLib.
>
> 5) In order to be consistent with other EDK II context
> typedefs, I recommend
>
> typedef VOID *HASH_API_CONTEXT;
>
> Also update APIs to use HashContext instead of
> *HashContext.
>
> 6) HashApiDuplicate() - The NewHashContext parameter
> should be type
> HASH_API_CONTEXT.
>
> 7) HashApiLib.inf - I think you can remove
> MdeModulePkg.dec from [Packages]
>
> Thanks,
>
> Mike
>
> > -----Original Message-----
> > From: Sukerkar, Amol N <amol.n.sukerkar@intel.com>
> > Sent: Tuesday, January 28, 2020 10:04 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>; Musti, Srinivas
> > <srinivas.musti@intel.com>; Lakkimsetti, Subash
> > <subash.lakkimsetti@intel.com>
> > Subject: [PATCH v6 0/2] CryptoPkg/HashApiLib:
> Implement Unified Hash
> > Calculation API
> >
> > Currently, the UEFI drivers using the SHA/SM3 hashing
> algorithms use
> > hard-coded API to calculate the hash, for instance,
> sha_256(...), etc.
> > Since SHA384 and/or
> > SM3_256 are being increasingly adopted for
> robustness, it becomes
> > cumbersome to modify each driver that calls into hash
> calculating API.
> >
> > To better achieve this, we are proposing a Unified
> API, which can be
> > used by UEFI drivers, that provides the drivers with
> flexibility to
> > use the desired hashing algorithm based on the
> required robnustness.
> >
> > Alternatively, the design document is also attached
> to Bugzilla,
> > https://bugzilla.tianocore.org/show_bug.cgi?id=2151.
> >
> > Sukerkar, Amol N (2):
> > CryptoPkg: Add CryptoPkg Token Space GUID
> > CryptoPkg/HashApiLib: Implement Unified Hash
> Calculation API
> >
> > CryptoPkg/Library/HashApiLib/HashApiLib.c | 333
> > ++++++++++++++++++++
> > CryptoPkg/CryptoPkg.dec | 27 +-
> > CryptoPkg/CryptoPkg.dsc | 7 +-
> > CryptoPkg/CryptoPkg.uni | 17 +
> > CryptoPkg/Include/Library/HashApiLib.h | 122
> > +++++++
> > CryptoPkg/Library/HashApiLib/HashApiLib.inf | 45
> +++
> > CryptoPkg/Library/HashApiLib/HashApiLib.uni | 17 +
> > 7 files changed, 566 insertions(+), 2 deletions(-)
> create mode
> > 100644 CryptoPkg/Library/HashApiLib/HashApiLib.c
> > create mode 100644
> > CryptoPkg/Include/Library/HashApiLib.h
> > create mode 100644
> > CryptoPkg/Library/HashApiLib/HashApiLib.inf
> > create mode 100644
> > CryptoPkg/Library/HashApiLib/HashApiLib.uni
> >
> > --
> > 2.16.2.windows.1
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v6 0/2] CryptoPkg/HashApiLib: Implement Unified Hash Calculation API
2020-01-29 23:16 ` Michael D Kinney
@ 2020-01-29 23:23 ` Sukerkar, Amol N
0 siblings, 0 replies; 7+ messages in thread
From: Sukerkar, Amol N @ 2020-01-29 23:23 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io
Cc: Yao, Jiewen, Wang, Jian J, Agrawal, Sachin, Musti, Srinivas,
Lakkimsetti, Subash, Sukerkar, Amol N
Ah, OK! It is clear to me now. Thanks, Mike!
~ Amol
-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Wednesday, January 29, 2020 4:16 PM
To: Sukerkar, Amol N <amol.n.sukerkar@intel.com>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Agrawal, Sachin <sachin.agrawal@intel.com>; Musti, Srinivas <srinivas.musti@intel.com>; Lakkimsetti, Subash <subash.lakkimsetti@intel.com>
Subject: RE: [PATCH v6 0/2] CryptoPkg/HashApiLib: Implement Unified Hash Calculation API
Hi Amol,
The feedback I received was for the use of the term "Base" in library class names. You did the correct change by changing the class name from "BaseHashLib"
to "HashApiLib".
It is correct to use the term "Base" in the name of a library instance if the library as implemented is compatible with all module types. The one implementation of the HashApiLib class is compatible with all module types, so the library instance name should be "BaseHashApiLib".
Mike
> -----Original Message-----
> From: Sukerkar, Amol N <amol.n.sukerkar@intel.com>
> Sent: Wednesday, January 29, 2020 3:10 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Agrawal, Sachin <sachin.agrawal@intel.com>;
> Musti, Srinivas <srinivas.musti@intel.com>; Lakkimsetti, Subash
> <subash.lakkimsetti@intel.com>; Sukerkar, Amol N
> <amol.n.sukerkar@intel.com>
> Subject: RE: [PATCH v6 0/2] CryptoPkg/HashApiLib:
> Implement Unified Hash Calculation API
>
> Hi Mike,
>
> Question about point 4. Could you help me clear the confusion?
>
> 4) The name of the HashApiLib instance should be "BaseHashApiLib" and
> the
> should be in the CryptoPkg/Library/BaseHashApiLib directory with
> files BashHashApiLib.inf, BaseHashApiLib.c, and BaseHashApiLib.uni.
> BASE_NAME in BaseHashApiLib.iunf should also be BaseHashApiLib.
>
> Perhaps I am not very clear but it appears you are contradicting your
> earlier feedback:
>
> I have received feedback before against the use of the term "Base" in
> the name of a library class. It causes confusion because the term
> "Base" usually applies to the library implementation to describe the
> module type compatibility of the lib instance. Take BaseCryptLib
> class as an example. There are instances of this library that are
> specific to PEI, DXE, SMM, and Runtime. If we look at the entire edk2
> repo, there are only 3 lib classes that start with the term "Base":
>
> BaseLib - OK. Single instance of type BASE.
> BaseMemoryLib - Confusing. BASE, PEI, DXE versions
> BaseCryptLib - Confusing. PEI, DXE, SMM, RT
> versions
>
> I also see the BaseHashLib service names use "HashApi".
> In order to address both the use of the term "Base" and The
> inconsistency between the lib class name and the lib service names, I
> recommend the lib class be changed from "BaseHashLib" to "HashApiLib"
> along with a few other name changes to scope defines and types to the
> HashApiLib:
>
> BaseHashLib -> HashApiLib
> HASH_HANDLE -> HASH_API_CONTEXT
> HASH_INVALID -> HASH_API_ALGORITHM_INVALID
> HASH_MD4 -> HASH_API_ALGORITHM_MD4
> HASH_MD5 -> HASH_API_ALGORITHM_MD5
> HASH_SHA1 -> HASH_API_ALGORITHM_SHA1
> HASH_SHA256 -> HASH_API_ALGORITHM_SHA256
> HASH_SHA384 -> HASH_API_ALGORITHM_SHA384
> HASH_SHA512 -> HASH_API_ALGORITHM_SHA512
> HASH_SM3_256 -> HASH_API_ALGORITHM_SM3_256
> HASH_MAX -> Remove. Not used.
>
> Some file name a directory names changes would also be required to
> follow this same pattern.
>
> Thanks,
> Amol
>
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Wednesday, January 29, 2020 1:10 PM
> To: Sukerkar, Amol N <amol.n.sukerkar@intel.com>;
> devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Agrawal, Sachin <sachin.agrawal@intel.com>;
> Musti, Srinivas <srinivas.musti@intel.com>; Lakkimsetti, Subash
> <subash.lakkimsetti@intel.com>
> Subject: RE: [PATCH v6 0/2] CryptoPkg/HashApiLib:
> Implement Unified Hash Calculation API
>
> Amol,
>
>
> 1) Typo in CryptoPkg.dec. Should be Crypto Package, not Security
> package.
>
> [Guids]
> ## Security package token space guid.
>
> 2) CryptoPkg.dec/uni. I see the default value for PcdHashApiLibPolicy
> is 0x04. This is documented to be SHA256. The DEC/UNI file
> descriptions of this PCD should state that the default policy is
> SHA256. This makes it clear to platform developers that maintain
> DSC files what the default policy is.
>
> 3) CryptoPkg.dsc: The same HashApiLib instance is used for all module
> types
> so a single mapping can be moved to [LibraryClasses] section and
> the
> DSC file and removed from the
> [LibraryClass.common.<ModuleType>] sections.
>
> 4) The name of the HashApiLib instance should be "BaseHashApiLib" and
> the
> should be in the CryptoPkg/Library/BaseHashApiLib directory with
> files BashHashApiLib.inf, BaseHashApiLib.c, and BaseHashApiLib.uni.
> BASE_NAME in BaseHashApiLib.iunf should also be BaseHashApiLib.
>
> 5) In order to be consistent with other EDK II context typedefs, I
> recommend
>
> typedef VOID *HASH_API_CONTEXT;
>
> Also update APIs to use HashContext instead of *HashContext.
>
> 6) HashApiDuplicate() - The NewHashContext parameter should be type
> HASH_API_CONTEXT.
>
> 7) HashApiLib.inf - I think you can remove MdeModulePkg.dec from
> [Packages]
>
> Thanks,
>
> Mike
>
> > -----Original Message-----
> > From: Sukerkar, Amol N <amol.n.sukerkar@intel.com>
> > Sent: Tuesday, January 28, 2020 10:04 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>; Musti, Srinivas
> > <srinivas.musti@intel.com>; Lakkimsetti, Subash
> > <subash.lakkimsetti@intel.com>
> > Subject: [PATCH v6 0/2] CryptoPkg/HashApiLib:
> Implement Unified Hash
> > Calculation API
> >
> > Currently, the UEFI drivers using the SHA/SM3 hashing
> algorithms use
> > hard-coded API to calculate the hash, for instance,
> sha_256(...), etc.
> > Since SHA384 and/or
> > SM3_256 are being increasingly adopted for
> robustness, it becomes
> > cumbersome to modify each driver that calls into hash
> calculating API.
> >
> > To better achieve this, we are proposing a Unified
> API, which can be
> > used by UEFI drivers, that provides the drivers with
> flexibility to
> > use the desired hashing algorithm based on the
> required robnustness.
> >
> > Alternatively, the design document is also attached
> to Bugzilla,
> > https://bugzilla.tianocore.org/show_bug.cgi?id=2151.
> >
> > Sukerkar, Amol N (2):
> > CryptoPkg: Add CryptoPkg Token Space GUID
> > CryptoPkg/HashApiLib: Implement Unified Hash
> Calculation API
> >
> > CryptoPkg/Library/HashApiLib/HashApiLib.c | 333
> > ++++++++++++++++++++
> > CryptoPkg/CryptoPkg.dec | 27 +-
> > CryptoPkg/CryptoPkg.dsc | 7 +-
> > CryptoPkg/CryptoPkg.uni | 17 +
> > CryptoPkg/Include/Library/HashApiLib.h | 122
> > +++++++
> > CryptoPkg/Library/HashApiLib/HashApiLib.inf | 45
> +++
> > CryptoPkg/Library/HashApiLib/HashApiLib.uni | 17 +
> > 7 files changed, 566 insertions(+), 2 deletions(-)
> create mode
> > 100644 CryptoPkg/Library/HashApiLib/HashApiLib.c
> > create mode 100644
> > CryptoPkg/Include/Library/HashApiLib.h
> > create mode 100644
> > CryptoPkg/Library/HashApiLib/HashApiLib.inf
> > create mode 100644
> > CryptoPkg/Library/HashApiLib/HashApiLib.uni
> >
> > --
> > 2.16.2.windows.1
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-01-29 23:23 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-01-28 18:03 [PATCH v6 0/2] CryptoPkg/HashApiLib: Implement Unified Hash Calculation API Sukerkar, Amol N
2020-01-28 18:03 ` [PATCH v6 1/2] CryptoPkg: Add CryptoPkg Token Space GUID Sukerkar, Amol N
2020-01-28 18:03 ` [PATCH v6 2/2] CryptoPkg/HashApiLib: Implement Unified Hash Calculation API Sukerkar, Amol N
2020-01-29 20:10 ` [PATCH v6 0/2] " Michael D Kinney
2020-01-29 23:09 ` Sukerkar, Amol N
2020-01-29 23:16 ` Michael D Kinney
2020-01-29 23:23 ` Sukerkar, Amol N
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox