* [PATCH v5 0/2] CryptoPkg/BaseHashLib: Implement Unified Hash Calculation API
@ 2020-01-23 16:46 Sukerkar, Amol N
2020-01-23 16:46 ` [PATCH v5 1/2] CryptoPkg: Add CryptoPkg Token Space GUID Sukerkar, Amol N
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Sukerkar, Amol N @ 2020-01-23 16:46 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/BaseHashLib: Implement Unified Hash Calculation API
CryptoPkg/Library/BaseHashLib/BaseHashLib.c | 228 ++++++++++++++++++++
CryptoPkg/CryptoPkg.dec | 28 ++-
CryptoPkg/CryptoPkg.dsc | 6 +-
CryptoPkg/CryptoPkg.uni | 17 ++
CryptoPkg/Include/Guid/CryptoPkgTokenSpace.h | 19 ++
CryptoPkg/Include/Library/BaseHashLib.h | 81 +++++++
CryptoPkg/Library/BaseHashLib/BaseHashLibDxe.inf | 44 ++++
CryptoPkg/Library/BaseHashLib/BaseHashLibDxe.uni | 17 ++
CryptoPkg/Library/BaseHashLib/BaseHashLibPei.inf | 45 ++++
CryptoPkg/Library/BaseHashLib/BaseHashLibPei.uni | 17 ++
10 files changed, 500 insertions(+), 2 deletions(-)
create mode 100644 CryptoPkg/Library/BaseHashLib/BaseHashLib.c
create mode 100644 CryptoPkg/Include/Guid/CryptoPkgTokenSpace.h
create mode 100644 CryptoPkg/Include/Library/BaseHashLib.h
create mode 100644 CryptoPkg/Library/BaseHashLib/BaseHashLibDxe.inf
create mode 100644 CryptoPkg/Library/BaseHashLib/BaseHashLibDxe.uni
create mode 100644 CryptoPkg/Library/BaseHashLib/BaseHashLibPei.inf
create mode 100644 CryptoPkg/Library/BaseHashLib/BaseHashLibPei.uni
--
2.16.2.windows.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v5 1/2] CryptoPkg: Add CryptoPkg Token Space GUID
2020-01-23 16:46 [PATCH v5 0/2] CryptoPkg/BaseHashLib: Implement Unified Hash Calculation API Sukerkar, Amol N
@ 2020-01-23 16:46 ` Sukerkar, Amol N
2020-01-23 16:46 ` [PATCH v5 2/2] CryptoPkg/BaseHashLib: Implement Unified Hash Calculation API Sukerkar, Amol N
2020-01-26 19:48 ` [PATCH v5 0/2] " Michael D Kinney
2 siblings, 0 replies; 5+ messages in thread
From: Sukerkar, Amol N @ 2020-01-23 16:46 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>
---
CryptoPkg/CryptoPkg.dec | 7 ++++++-
CryptoPkg/Include/Guid/CryptoPkgTokenSpace.h | 19 +++++++++++++++++++
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/CryptoPkg/CryptoPkg.dec b/CryptoPkg/CryptoPkg.dec
index 08bedd57daad..a548ec7ddc71 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,10 @@ [LibraryClasses]
##
TlsLib|Include/Library/TlsLib.h
+[Guids]
+ ## Security package token space guid.
+ # Include/Guid/CryptoPkgTokenSpace.h
+ gEfiCryptoPkgTokenSpaceGuid = { 0xd3fb176, 0x9569, 0x4d51, { 0xa3, 0xef, 0x7d, 0x61, 0xc6, 0x4f, 0xea, 0xba }}
+
[UserExtensions.TianoCore."ExtraFiles"]
CryptoPkgExtra.uni
diff --git a/CryptoPkg/Include/Guid/CryptoPkgTokenSpace.h b/CryptoPkg/Include/Guid/CryptoPkgTokenSpace.h
new file mode 100644
index 000000000000..1c9165080048
--- /dev/null
+++ b/CryptoPkg/Include/Guid/CryptoPkgTokenSpace.h
@@ -0,0 +1,19 @@
+/** @file
+ GUID for CryptoPkg PCD Token Space.
+
+Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _CRYPTOPKG_TOKEN_SPACE_GUID_H_
+#define _CRYPTOPKG_TOKEN_SPACE_GUID_H_
+
+#define CRYPTOPKG_TOKEN_SPACE_GUID \
+ { \
+ 0x7c4b0548, 0xd267, 0x451f, { 0xb5, 0x68, 0x58, 0x4f, 0x82, 0xb2, 0x1c, 0x89 } \
+ }
+
+extern EFI_GUID gEfiCryptoPkgTokenSpaceGuid;
+
+#endif
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v5 2/2] CryptoPkg/BaseHashLib: Implement Unified Hash Calculation API
2020-01-23 16:46 [PATCH v5 0/2] CryptoPkg/BaseHashLib: Implement Unified Hash Calculation API Sukerkar, Amol N
2020-01-23 16:46 ` [PATCH v5 1/2] CryptoPkg: Add CryptoPkg Token Space GUID Sukerkar, Amol N
@ 2020-01-23 16:46 ` Sukerkar, Amol N
2020-01-26 19:48 ` [PATCH v5 0/2] " Michael D Kinney
2 siblings, 0 replies; 5+ messages in thread
From: Sukerkar, Amol N @ 2020-01-23 16:46 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, PcdSystemHashPolicy. 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 PcdSystemHashPolicy 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>
---
CryptoPkg/Library/BaseHashLib/BaseHashLib.c | 228 ++++++++++++++++++++
CryptoPkg/CryptoPkg.dec | 21 ++
CryptoPkg/CryptoPkg.dsc | 6 +-
CryptoPkg/CryptoPkg.uni | 17 ++
CryptoPkg/Include/Library/BaseHashLib.h | 81 +++++++
CryptoPkg/Library/BaseHashLib/BaseHashLibDxe.inf | 44 ++++
CryptoPkg/Library/BaseHashLib/BaseHashLibDxe.uni | 17 ++
CryptoPkg/Library/BaseHashLib/BaseHashLibPei.inf | 45 ++++
CryptoPkg/Library/BaseHashLib/BaseHashLibPei.uni | 17 ++
9 files changed, 475 insertions(+), 1 deletion(-)
diff --git a/CryptoPkg/Library/BaseHashLib/BaseHashLib.c b/CryptoPkg/Library/BaseHashLib/BaseHashLib.c
new file mode 100644
index 000000000000..4cfea80d1df2
--- /dev/null
+++ b/CryptoPkg/Library/BaseHashLib/BaseHashLib.c
@@ -0,0 +1,228 @@
+/** @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 PcdSystemHashPolicy.
+
+ Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#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/BaseHashLib.h>
+
+/**
+ Init hash sequence.
+
+ @param HashHandle Hash handle.
+
+ @retval TRUE Hash start and HashHandle returned.
+ @retval FALSE Hash Init unsuccessful.
+**/
+BOOLEAN
+EFIAPI
+HashApiInit (
+ OUT HASH_HANDLE *HashHandle
+ )
+{
+ BOOLEAN Status;
+ VOID *HashCtx;
+ UINTN CtxSize;
+
+ switch (PcdGet8 (PcdSystemHashPolicy)) {
+ case HASH_MD4:
+ CtxSize = Md4GetContextSize ();
+ HashCtx = AllocatePool (CtxSize);
+ ASSERT (HashCtx != NULL);
+
+ Status = Md4Init (HashCtx);
+ break;
+
+ case HASH_MD5:
+ CtxSize = Md5GetContextSize ();
+ HashCtx = AllocatePool (CtxSize);
+ ASSERT (HashCtx != NULL);
+
+ Status = Md5Init (HashCtx);
+ break;
+
+ case HASH_SHA1:
+ CtxSize = Sha1GetContextSize ();
+ HashCtx = AllocatePool (CtxSize);
+ ASSERT (HashCtx != NULL);
+
+ Status = Sha1Init (HashCtx);
+ break;
+
+ case HASH_SHA256:
+ CtxSize = Sha256GetContextSize ();
+ HashCtx = AllocatePool (CtxSize);
+ ASSERT (HashCtx != NULL);
+
+ Status = Sha256Init (HashCtx);
+ break;
+
+ case HASH_SHA384:
+ CtxSize = Sha384GetContextSize ();
+ HashCtx = AllocatePool (CtxSize);
+ ASSERT (HashCtx != NULL);
+
+ Status = Sha384Init (HashCtx);
+ break;
+
+ case HASH_SHA512:
+ CtxSize = Sha512GetContextSize ();
+ HashCtx = AllocatePool (CtxSize);
+ ASSERT (HashCtx != NULL);
+
+ Status = Sha512Init (HashCtx);
+ break;
+
+ case HASH_SM3_256:
+ CtxSize = Sm3GetContextSize ();
+ HashCtx = AllocatePool (CtxSize);
+ ASSERT (HashCtx != NULL);
+
+ Status = Sm3Init (HashCtx);
+ break;
+
+ default:
+ Status = FALSE;
+ ASSERT (Status);
+ break;
+ }
+
+ *HashHandle = (HASH_HANDLE)HashCtx;
+
+ return Status;
+}
+
+/**
+ Update hash data.
+
+ @param HashHandle Hash handle.
+ @param DataToHash Data to be hashed.
+ @param DataToHashLen Data size.
+
+ @retval TRUE Hash updated.
+ @retval FALSE Hash updated unsuccessful.
+**/
+BOOLEAN
+EFIAPI
+HashApiUpdate (
+ IN HASH_HANDLE HashHandle,
+ IN VOID *DataToHash,
+ IN UINTN DataToHashLen
+ )
+{
+ BOOLEAN Status;
+ VOID *HashCtx;
+
+ HashCtx = (VOID *)HashHandle;
+
+ switch (PcdGet8 (PcdSystemHashPolicy)) {
+ case HASH_MD4:
+ Status = Md4Update (HashCtx, DataToHash, DataToHashLen);
+ break;
+
+ case HASH_MD5:
+ Status = Md5Update (HashCtx, DataToHash, DataToHashLen);
+ break;
+
+ case HASH_SHA1:
+ Status = Sha1Update (HashCtx, DataToHash, DataToHashLen);
+ break;
+
+ case HASH_SHA256:
+ Status = Sha256Update (HashCtx, DataToHash, DataToHashLen);
+ break;
+
+ case HASH_SHA384:
+ Status = Sha384Update (HashCtx, DataToHash, DataToHashLen);
+ break;
+
+ case HASH_SHA512:
+ Status = Sha512Update (HashCtx, DataToHash, DataToHashLen);
+ break;
+
+ case HASH_SM3_256:
+ Status = Sm3Update (HashCtx, DataToHash, DataToHashLen);
+ break;
+
+ default:
+ Status = FALSE;
+ ASSERT (Status);
+ break;
+ }
+
+ return Status;
+}
+
+/**
+ Hash complete.
+
+ @param HashHandle Hash handle.
+ @param Digest Hash Digest.
+
+ @retval TRUE Hash complete and Digest is returned.
+ @retval FALSE Hash complete unsuccessful.
+**/
+BOOLEAN
+EFIAPI
+HashApiFinal (
+ IN HASH_HANDLE HashHandle,
+ OUT UINT8 *Digest
+ )
+{
+ BOOLEAN Status;
+ VOID *HashCtx;
+
+ HashCtx = (VOID *)HashHandle;
+
+ switch (PcdGet8 (PcdSystemHashPolicy)) {
+ case HASH_MD4:
+ Status = Md4Final (HashCtx, Digest);
+ break;
+
+ case HASH_MD5:
+ Status = Md5Final (HashCtx, Digest);
+ break;
+
+ case HASH_SHA1:
+ Status = Sha1Final (HashCtx, Digest);
+ break;
+
+ case HASH_SHA256:
+ Status = Sha256Final (HashCtx, Digest);
+ break;
+
+ case HASH_SHA384:
+ Status = Sha384Final (HashCtx, Digest);
+ break;
+
+ case HASH_SHA512:
+ Status = Sha512Final (HashCtx, Digest);
+ break;
+
+ case HASH_SM3_256:
+ Status = Sm3Final (HashCtx, Digest);
+ break;
+
+ default:
+ Status = FALSE;
+ ASSERT (Status);
+ break;
+ }
+
+ FreePool (HashCtx);
+
+ return Status;
+}
diff --git a/CryptoPkg/CryptoPkg.dec b/CryptoPkg/CryptoPkg.dec
index a548ec7ddc71..9288c652f8e4 100644
--- a/CryptoPkg/CryptoPkg.dec
+++ b/CryptoPkg/CryptoPkg.dec
@@ -33,10 +33,31 @@ [LibraryClasses]
##
TlsLib|Include/Library/TlsLib.h
+ ## @libraryclass Provides Unified API for different hash implementations.
+ #
+ BaseHashLib|Include/Library/BaseHashLib.h
+
[Guids]
## Security package token space guid.
# Include/Guid/CryptoPkgTokenSpace.h
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.PcdSystemHashPolicy|0x04|UINT8|0x00000001
+
[UserExtensions.TianoCore."ExtraFiles"]
CryptoPkgExtra.uni
diff --git a/CryptoPkg/CryptoPkg.dsc b/CryptoPkg/CryptoPkg.dsc
index ec43c1f0a47e..1d2956d20483 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,9 +62,11 @@ [LibraryClasses.ARM]
[LibraryClasses.common.PEIM]
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
+ BaseHashLib|CryptoPkg/Library/BaseHashLib/BaseHashLibPei.inf
[LibraryClasses.common.DXE_DRIVER]
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
+ BaseHashLib|CryptoPkg/Library/BaseHashLib/BaseHashLibDxe.inf
[LibraryClasses.common.DXE_RUNTIME_DRIVER]
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
@@ -120,6 +122,8 @@ [Components]
CryptoPkg/Library/TlsLibNull/TlsLibNull.inf
CryptoPkg/Library/OpensslLib/OpensslLib.inf
CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
+ CryptoPkg/Library/BaseHashLib/BaseHashLibDxe.inf
+ CryptoPkg/Library/BaseHashLib/BaseHashLibPei.inf
[Components.IA32, Components.X64]
CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
diff --git a/CryptoPkg/CryptoPkg.uni b/CryptoPkg/CryptoPkg.uni
index beb0036ef583..ebbebed4924d 100644
--- a/CryptoPkg/CryptoPkg.uni
+++ b/CryptoPkg/CryptoPkg.uni
@@ -17,3 +17,20 @@
+#string STR_gEfiCryptoPkgTokenSpaceGuid_PcdSystemHashPolicy_PROMPT #language en-US "HASH algorithm to verify unsigned PE/COFF image"
+
+#string STR_gEfiCryptoPkgTokenSpaceGuid_PcdSystemHashPolicy_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/BaseHashLib.h b/CryptoPkg/Include/Library/BaseHashLib.h
new file mode 100644
index 000000000000..c9fa05a18876
--- /dev/null
+++ b/CryptoPkg/Include/Library/BaseHashLib.h
@@ -0,0 +1,81 @@
+/** @file
+ Unified Hash API Defines
+
+ This API when called will calculate the Hash using the
+ hashing algorithm specified by PcdSystemHashPolicy.
+
+ Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef __BASEHASHLIB_H_
+#define __BASEHASHLIB_H_
+
+#include <Uefi.h>
+
+typedef UINTN HASH_HANDLE;
+
+//
+// Hash Algorithms
+//
+#define HASH_INVALID 0x00000000
+#define HASH_MD4 0x00000001
+#define HASH_MD5 0x00000002
+#define HASH_SHA1 0x00000003
+#define HASH_SHA256 0x00000004
+#define HASH_SHA384 0x00000005
+#define HASH_SHA512 0x00000006
+#define HASH_SM3_256 0x00000007
+#define HASH_MAX 0x00000008
+
+
+/**
+ Init hash sequence.
+
+ @param HashHandle Hash handle.
+
+ @retval TRUE Hash start and HashHandle returned.
+ @retval FALSE Hash Init unsuccessful.
+**/
+BOOLEAN
+EFIAPI
+HashApiInit (
+ OUT HASH_HANDLE *HashHandle
+);
+
+/**
+ Update hash data.
+
+ @param HashHandle Hash handle.
+ @param DataToHash Data to be hashed.
+ @param DataToHashLen Data size.
+
+ @retval TRUE Hash updated.
+ @retval FALSE Hash updated unsuccessful.
+**/
+BOOLEAN
+EFIAPI
+HashApiUpdate (
+ IN HASH_HANDLE HashHandle,
+ IN VOID *DataToHash,
+ IN UINTN DataToHashLen
+);
+
+/**
+ Hash complete.
+
+ @param HashHandle Hash handle.
+ @param Digest Hash Digest.
+
+ @retval TRUE Hash complete and Digest is returned.
+ @retval FALSE Hash complete unsuccessful.
+**/
+BOOLEAN
+EFIAPI
+HashApiFinal (
+ IN HASH_HANDLE HashHandle,
+ OUT UINT8 *Digest
+);
+
+#endif
diff --git a/CryptoPkg/Library/BaseHashLib/BaseHashLibDxe.inf b/CryptoPkg/Library/BaseHashLib/BaseHashLibDxe.inf
new file mode 100644
index 000000000000..f4a6c939f509
--- /dev/null
+++ b/CryptoPkg/Library/BaseHashLib/BaseHashLibDxe.inf
@@ -0,0 +1,44 @@
+## @file
+# Provides Unified API for Hash Calculation
+#
+# This library is BaseHashLib. It will redirect hash request to each
+# individual hash API, such as SHA1, SHA256, SHA384, SM3 based on
+# hashing algorithm specified by PcdSystemHashPolicy.
+#
+# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = BaseHashLibDxe
+ MODULE_UNI_FILE = BaseHashLibDxe.uni
+ FILE_GUID = 158DC712-F15A-44dc-93BB-1675045BE066
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = BaseHashLib|DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = IA32 X64
+#
+
+[Sources]
+ BaseHashLib.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ CryptoPkg/CryptoPkg.dec
+
+[LibraryClasses]
+ BaseLib
+ BaseMemoryLib
+ DebugLib
+ MemoryAllocationLib
+ BaseCryptLib
+ PcdLib
+
+[Pcd]
+ gEfiCryptoPkgTokenSpaceGuid.PcdSystemHashPolicy ## CONSUMES
diff --git a/CryptoPkg/Library/BaseHashLib/BaseHashLibDxe.uni b/CryptoPkg/Library/BaseHashLib/BaseHashLibDxe.uni
new file mode 100644
index 000000000000..b1839120ec08
--- /dev/null
+++ b/CryptoPkg/Library/BaseHashLib/BaseHashLibDxe.uni
@@ -0,0 +1,17 @@
+// /** @file
+// Provides Unified API for Hash Calculation
+//
+// This library is BaseHashLib. It will redirect hash request to each
+// individual hash API, such as SHA1, SHA256, SHA384, SM3 based on
+// hashing algorithm specified by PcdSystemHashPolicy.
+//
+// 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 PcdSystemHashPolicy."
diff --git a/CryptoPkg/Library/BaseHashLib/BaseHashLibPei.inf b/CryptoPkg/Library/BaseHashLib/BaseHashLibPei.inf
new file mode 100644
index 000000000000..ca1e4d361954
--- /dev/null
+++ b/CryptoPkg/Library/BaseHashLib/BaseHashLibPei.inf
@@ -0,0 +1,45 @@
+## @file
+# Provides Unified API for Hash Calculation
+#
+# This library is BaseHashLib. It will redirect hash request to each
+# individual hash API, such as SHA1, SHA256, SHA384, SM3 based on
+# hashing algorithm specified by PcdSystemHashPolicy.
+#
+# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = BaseHashLibPei
+ MODULE_UNI_FILE = BaseHashLibPei.uni
+ FILE_GUID = DDCBCFBA-8EEB-488a-96D6-097831A6E50B
+ MODULE_TYPE = PEIM
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = BaseHashLib|PEIM
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = IA32 X64
+#
+
+[Sources]
+ BaseHashLib.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ CryptoPkg/CryptoPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+ BaseLib
+ BaseMemoryLib
+ DebugLib
+ MemoryAllocationLib
+ BaseCryptLib
+ PcdLib
+
+[Pcd]
+ gEfiCryptoPkgTokenSpaceGuid.PcdSystemHashPolicy ## CONSUMES
diff --git a/CryptoPkg/Library/BaseHashLib/BaseHashLibPei.uni b/CryptoPkg/Library/BaseHashLib/BaseHashLibPei.uni
new file mode 100644
index 000000000000..b1839120ec08
--- /dev/null
+++ b/CryptoPkg/Library/BaseHashLib/BaseHashLibPei.uni
@@ -0,0 +1,17 @@
+// /** @file
+// Provides Unified API for Hash Calculation
+//
+// This library is BaseHashLib. It will redirect hash request to each
+// individual hash API, such as SHA1, SHA256, SHA384, SM3 based on
+// hashing algorithm specified by PcdSystemHashPolicy.
+//
+// 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 PcdSystemHashPolicy."
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v5 0/2] CryptoPkg/BaseHashLib: Implement Unified Hash Calculation API
2020-01-23 16:46 [PATCH v5 0/2] CryptoPkg/BaseHashLib: Implement Unified Hash Calculation API Sukerkar, Amol N
2020-01-23 16:46 ` [PATCH v5 1/2] CryptoPkg: Add CryptoPkg Token Space GUID Sukerkar, Amol N
2020-01-23 16:46 ` [PATCH v5 2/2] CryptoPkg/BaseHashLib: Implement Unified Hash Calculation API Sukerkar, Amol N
@ 2020-01-26 19:48 ` Michael D Kinney
2020-01-28 15:12 ` Sukerkar, Amol N
2 siblings, 1 reply; 5+ messages in thread
From: Michael D Kinney @ 2020-01-26 19:48 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,
Thank your for providing patches for this feature.
You have done a very good job on following the
design patterns from similar feature types in the
EDK II and following the EDK II coding style. I
do have some recommended changes below and have
attempted to provide reasons for these recommendations.
Please let me know if you have any questions or need
any clarifications
I am thinking of the use case where a developer is
updating an existing module that uses the BaseCryptLib
to use the BaseHashLib. The BaseHashLib does not
provide that same set of services per hash algorithm
which means there may be module that would like to
switch to BaseHashLib, but can't.
Can we update BaseHashLib to provide the same set of
services:
HashApiGetContextSize()
HashApiInit()
HashApiDuplicate()
HashApiUpdate()
HashApiFinal()
HashApiHashAll()
The BaseHashLib uses the term "Handle" where the
BaseCryptLib uses the term "Context". This may confuse
developers that are porting from BaseCryptLib usage to
BaseHashLib usage. I recommend we continue the use
of the term "Context" in BasehHashLib.
The BaseCryptLib supports PEI, DXE, SMM, and Runtime.
is there a reason that BaseHashLib only supports PEI
and DXE INFs? Upon further review, the implementation of
this library only uses the services of other library
classes and is not phase specific at all. I think this
could be a single INF/UNI file with a MODULE_TYPE of
BASE and include <Base.h> at the top of BaseHashLib.c.
This lib instance would be compatible with all module
types.
I see the PCD name " PcdSystemHashPolicy". This PCD can
be set in the context of a single module if different
modules in a platform FW build use different hash algorithms.
As a result, this PCD can be used for both system wide
policy settings using a Dynamic/DynamicEx PCD type or
a module scoped PCD setting using a FixedAtBuild or
PatchableInModule PCD type. This PCD is scoped for use
by the BaseHashLib. Perhaps a better name would be
"PcdHashApiLibPolicy".
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.
The include file "CryptoPkgTokenSpace.h" can
be removed. I know the pattern in the existing
packages contains this include file, but there
was a change in the autogen that supports defining
a GUID value in the DEC file without requiring
an associated .h file. This new pattern only
applies to GUIDs that have no associated data
structures, Protocols, or PPIs. A token space
GUID is a good example of a GUID value with no
associated structures.
BaseHashLib.h includes the file <Uefi.h>. This
can be removed. Modules and library instances
that use a library class will include a top
level include file that matches the MODULE_TYPE
before including any library class .h files or
protocols/ppi/guid .h files.
<Base.h>
<PiPei.h>
<PiDxe.h>
<PiSmm.h>
<PiMm.h>
<Uefi.h>
Thanks,
Mike
> -----Original Message-----
> From: Sukerkar, Amol N <amol.n.sukerkar@intel.com>
> Sent: Thursday, January 23, 2020 8:47 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 v5 0/2] CryptoPkg/BaseHashLib: Implement
> Unified Hash Calculation API
>
> Currently, the UEFI drivers using the SHA/SM3 hashing
> algorithms use hard-coded
> API to calculate the hash, for instance, sha_256(...),
> etc. Since SHA384 and/or
> SM3_256 are being increasingly adopted for robustness,
> it becomes cumbersome to
> modify each driver that calls into hash calculating API.
>
> To better achieve this, we are proposing a Unified API,
> which can be used by UEFI
> drivers, that provides the drivers with flexibility to
> use the desired hashing
> algorithm based on the required robnustness.
>
> Alternatively, the design document is also attached to
> Bugzilla,
> https://bugzilla.tianocore.org/show_bug.cgi?id=2151.
>
> Sukerkar, Amol N (2):
> CryptoPkg: Add CryptoPkg Token Space GUID
> CryptoPkg/BaseHashLib: Implement Unified Hash
> Calculation API
>
> CryptoPkg/Library/BaseHashLib/BaseHashLib.c | 228
> ++++++++++++++++++++
> CryptoPkg/CryptoPkg.dec | 28
> ++-
> CryptoPkg/CryptoPkg.dsc | 6
> +-
> CryptoPkg/CryptoPkg.uni | 17
> ++
> CryptoPkg/Include/Guid/CryptoPkgTokenSpace.h | 19
> ++
> CryptoPkg/Include/Library/BaseHashLib.h | 81
> +++++++
> CryptoPkg/Library/BaseHashLib/BaseHashLibDxe.inf | 44
> ++++
> CryptoPkg/Library/BaseHashLib/BaseHashLibDxe.uni | 17
> ++
> CryptoPkg/Library/BaseHashLib/BaseHashLibPei.inf | 45
> ++++
> CryptoPkg/Library/BaseHashLib/BaseHashLibPei.uni | 17
> ++
> 10 files changed, 500 insertions(+), 2 deletions(-)
> create mode 100644
> CryptoPkg/Library/BaseHashLib/BaseHashLib.c
> create mode 100644
> CryptoPkg/Include/Guid/CryptoPkgTokenSpace.h
> create mode 100644
> CryptoPkg/Include/Library/BaseHashLib.h
> create mode 100644
> CryptoPkg/Library/BaseHashLib/BaseHashLibDxe.inf
> create mode 100644
> CryptoPkg/Library/BaseHashLib/BaseHashLibDxe.uni
> create mode 100644
> CryptoPkg/Library/BaseHashLib/BaseHashLibPei.inf
> create mode 100644
> CryptoPkg/Library/BaseHashLib/BaseHashLibPei.uni
>
> --
> 2.16.2.windows.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v5 0/2] CryptoPkg/BaseHashLib: Implement Unified Hash Calculation API
2020-01-26 19:48 ` [PATCH v5 0/2] " Michael D Kinney
@ 2020-01-28 15:12 ` Sukerkar, Amol N
0 siblings, 0 replies; 5+ messages in thread
From: Sukerkar, Amol N @ 2020-01-28 15:12 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
Thanks, Mike! I agree with the feedback below. I will upload the changes soon.
~ Amol
-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Sunday, January 26, 2020 12:48 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 v5 0/2] CryptoPkg/BaseHashLib: Implement Unified Hash Calculation API
Amol,
Thank your for providing patches for this feature.
You have done a very good job on following the design patterns from similar feature types in the EDK II and following the EDK II coding style. I do have some recommended changes below and have attempted to provide reasons for these recommendations.
Please let me know if you have any questions or need any clarifications
I am thinking of the use case where a developer is updating an existing module that uses the BaseCryptLib to use the BaseHashLib. The BaseHashLib does not provide that same set of services per hash algorithm which means there may be module that would like to switch to BaseHashLib, but can't.
Can we update BaseHashLib to provide the same set of
services:
HashApiGetContextSize()
HashApiInit()
HashApiDuplicate()
HashApiUpdate()
HashApiFinal()
HashApiHashAll()
The BaseHashLib uses the term "Handle" where the BaseCryptLib uses the term "Context". This may confuse developers that are porting from BaseCryptLib usage to BaseHashLib usage. I recommend we continue the use of the term "Context" in BasehHashLib.
The BaseCryptLib supports PEI, DXE, SMM, and Runtime.
is there a reason that BaseHashLib only supports PEI and DXE INFs? Upon further review, the implementation of this library only uses the services of other library classes and is not phase specific at all. I think this could be a single INF/UNI file with a MODULE_TYPE of BASE and include <Base.h> at the top of BaseHashLib.c.
This lib instance would be compatible with all module types.
I see the PCD name " PcdSystemHashPolicy". This PCD can be set in the context of a single module if different modules in a platform FW build use different hash algorithms.
As a result, this PCD can be used for both system wide policy settings using a Dynamic/DynamicEx PCD type or a module scoped PCD setting using a FixedAtBuild or PatchableInModule PCD type. This PCD is scoped for use by the BaseHashLib. Perhaps a better name would be "PcdHashApiLibPolicy".
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.
The include file "CryptoPkgTokenSpace.h" can be removed. I know the pattern in the existing packages contains this include file, but there was a change in the autogen that supports defining a GUID value in the DEC file without requiring an associated .h file. This new pattern only applies to GUIDs that have no associated data structures, Protocols, or PPIs. A token space GUID is a good example of a GUID value with no associated structures.
BaseHashLib.h includes the file <Uefi.h>. This can be removed. Modules and library instances that use a library class will include a top level include file that matches the MODULE_TYPE before including any library class .h files or protocols/ppi/guid .h files.
<Base.h>
<PiPei.h>
<PiDxe.h>
<PiSmm.h>
<PiMm.h>
<Uefi.h>
Thanks,
Mike
> -----Original Message-----
> From: Sukerkar, Amol N <amol.n.sukerkar@intel.com>
> Sent: Thursday, January 23, 2020 8:47 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 v5 0/2] CryptoPkg/BaseHashLib: Implement Unified Hash
> Calculation API
>
> Currently, the UEFI drivers using the SHA/SM3 hashing algorithms use
> hard-coded API to calculate the hash, for instance, sha_256(...), etc.
> Since SHA384 and/or
> SM3_256 are being increasingly adopted for robustness, it becomes
> cumbersome to modify each driver that calls into hash calculating API.
>
> To better achieve this, we are proposing a Unified API, which can be
> used by UEFI drivers, that provides the drivers with flexibility to
> use the desired hashing algorithm based on the required robnustness.
>
> Alternatively, the design document is also attached to Bugzilla,
> https://bugzilla.tianocore.org/show_bug.cgi?id=2151.
>
> Sukerkar, Amol N (2):
> CryptoPkg: Add CryptoPkg Token Space GUID
> CryptoPkg/BaseHashLib: Implement Unified Hash Calculation API
>
> CryptoPkg/Library/BaseHashLib/BaseHashLib.c | 228
> ++++++++++++++++++++
> CryptoPkg/CryptoPkg.dec | 28
> ++-
> CryptoPkg/CryptoPkg.dsc | 6
> +-
> CryptoPkg/CryptoPkg.uni | 17
> ++
> CryptoPkg/Include/Guid/CryptoPkgTokenSpace.h | 19
> ++
> CryptoPkg/Include/Library/BaseHashLib.h | 81
> +++++++
> CryptoPkg/Library/BaseHashLib/BaseHashLibDxe.inf | 44
> ++++
> CryptoPkg/Library/BaseHashLib/BaseHashLibDxe.uni | 17
> ++
> CryptoPkg/Library/BaseHashLib/BaseHashLibPei.inf | 45
> ++++
> CryptoPkg/Library/BaseHashLib/BaseHashLibPei.uni | 17
> ++
> 10 files changed, 500 insertions(+), 2 deletions(-) create mode
> 100644 CryptoPkg/Library/BaseHashLib/BaseHashLib.c
> create mode 100644
> CryptoPkg/Include/Guid/CryptoPkgTokenSpace.h
> create mode 100644
> CryptoPkg/Include/Library/BaseHashLib.h
> create mode 100644
> CryptoPkg/Library/BaseHashLib/BaseHashLibDxe.inf
> create mode 100644
> CryptoPkg/Library/BaseHashLib/BaseHashLibDxe.uni
> create mode 100644
> CryptoPkg/Library/BaseHashLib/BaseHashLibPei.inf
> create mode 100644
> CryptoPkg/Library/BaseHashLib/BaseHashLibPei.uni
>
> --
> 2.16.2.windows.1
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-01-28 15:12 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-01-23 16:46 [PATCH v5 0/2] CryptoPkg/BaseHashLib: Implement Unified Hash Calculation API Sukerkar, Amol N
2020-01-23 16:46 ` [PATCH v5 1/2] CryptoPkg: Add CryptoPkg Token Space GUID Sukerkar, Amol N
2020-01-23 16:46 ` [PATCH v5 2/2] CryptoPkg/BaseHashLib: Implement Unified Hash Calculation API Sukerkar, Amol N
2020-01-26 19:48 ` [PATCH v5 0/2] " Michael D Kinney
2020-01-28 15:12 ` 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