* [PATCH v3 0/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API @ 2020-01-15 22:57 Sukerkar, Amol N 2020-01-15 22:57 ` [PATCH v3 1/1] " Sukerkar, Amol N 2020-01-15 23:52 ` [PATCH v3 0/1] " Michael D Kinney 0 siblings, 2 replies; 8+ messages in thread From: Sukerkar, Amol N @ 2020-01-15 22:57 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 (1): SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c | 151 ++++++++++++++++++++ SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c | 100 +++++++++++++ SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c | 103 +++++++++++++ SecurityPkg/Include/Library/BaseHashLib.h | 85 +++++++++++ SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h | 141 ++++++++++++++++++ SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf | 46 ++++++ SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni | 17 +++ SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf | 51 +++++++ SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni | 16 +++ SecurityPkg/SecurityPkg.dec | 23 ++- SecurityPkg/SecurityPkg.dsc | 10 +- SecurityPkg/SecurityPkg.uni | 15 +- 12 files changed, 755 insertions(+), 3 deletions(-) create mode 100644 SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c create mode 100644 SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c create mode 100644 SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c create mode 100644 SecurityPkg/Include/Library/BaseHashLib.h create mode 100644 SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h create mode 100644 SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf create mode 100644 SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni create mode 100644 SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf create mode 100644 SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni -- 2.16.2.windows.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 1/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API 2020-01-15 22:57 [PATCH v3 0/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API Sukerkar, Amol N @ 2020-01-15 22:57 ` Sukerkar, Amol N 2020-01-15 23:52 ` [PATCH v3 0/1] " Michael D Kinney 1 sibling, 0 replies; 8+ messages in thread From: Sukerkar, Amol N @ 2020-01-15 22:57 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 CryptoPkg. 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> --- Notes: v2: - Fixed the commit message format V3: - Changed design to use global array instead of switch..case - Removed unused constructors - Removed trailing white spaces SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c | 151 ++++++++++++++++++++ SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c | 100 +++++++++++++ SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c | 103 +++++++++++++ SecurityPkg/Include/Library/BaseHashLib.h | 85 +++++++++++ SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h | 141 ++++++++++++++++++ SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf | 46 ++++++ SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni | 17 +++ SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf | 51 +++++++ SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni | 16 +++ SecurityPkg/SecurityPkg.dec | 23 ++- SecurityPkg/SecurityPkg.dsc | 10 +- SecurityPkg/SecurityPkg.uni | 15 +- 12 files changed, 755 insertions(+), 3 deletions(-) diff --git a/SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c b/SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c new file mode 100644 index 000000000000..999fea3fed9e --- /dev/null +++ b/SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c @@ -0,0 +1,151 @@ +/** @file + Implement image verification services for secure boot service + + Caution: This file requires additional review when modified. + This library will have external input - PE/COFF image. + This external input must be validated carefully to avoid security issue like + buffer overflow, integer overflow. + + DxeImageVerificationLibImageRead() function will make sure the PE/COFF image content + read is within the image buffer. + + DxeImageVerificationHandler(), HashPeImageByType(), HashPeImage() function will accept + untrusted PE/COFF image and validate its data structure within this image buffer before use. + +Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.<BR> +(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR> +This program and the accompanying materials +are licensed and made available under the terms and conditions of the BSD License +which accompanies this distribution. The full text of the license may be found at +http://opensource.org/licenses/bsd-license.php + +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. + +**/ + +#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> + +#include "BaseHashLibCommon.h" + +STATIC CONST HASH_API_INTERFACE mHashApi [] = { + {NULL, NULL, NULL, NULL}, + {Md4GetContextSize, Md4Init, Md4Update, Md4Final, MD4_DIGEST_SIZE}, + {Md5GetContextSize, Md5Init, Md5Update, Md5Final, MD5_DIGEST_SIZE}, + {Sha1GetContextSize, Sha1Init, Sha1Update, Sha1Final, SHA1_DIGEST_SIZE}, + {Sha256GetContextSize, Sha256Init, Sha256Update, Sha256Final, SHA256_DIGEST_SIZE}, + {Sha384GetContextSize, Sha384Init, Sha384Update, Sha384Final, SHA384_DIGEST_SIZE}, + {Sha512GetContextSize, Sha512Init, Sha512Update, Sha512Final, SHA512_DIGEST_SIZE}, + {Sm3GetContextSize, Sm3Init, Sm3Update, Sm3Final, SM3_256_DIGEST_SIZE} +}; + +/** + Init hash sequence with Hash Algorithm specified by HashPolicy. + + @param HashPolicy Hash Algorithm Policy. + @param HashHandle Hash handle. + + @retval TRUE Hash start and HashHandle returned. + @retval FALSE Hash Init unsuccessful. +**/ +BOOLEAN +EFIAPI +HashInitInternal ( + IN UINT8 HashPolicy, + OUT HASH_HANDLE *HashHandle + ) +{ + BOOLEAN Status; + VOID *HashCtx; + UINTN CtxSize; + + if (HashPolicy == HASH_INVALID || HashPolicy >= HASH_MAX) { + ASSERT (FALSE); + } + + CtxSize = mHashApi[HashPolicy].HashGetContextSize (); + HashCtx = AllocatePool (CtxSize); + ASSERT (HashCtx != NULL); + + Status = mHashApi[HashPolicy].HashInit (HashCtx); + + *HashHandle = (HASH_HANDLE)HashCtx; + + return Status; +} + +/** + Update hash data with Hash Algorithm specified by HashPolicy. + + @param HashPolicy Hash Algorithm Policy. + @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 +HashUpdateInternal ( + IN UINT8 HashPolicy, + IN HASH_HANDLE HashHandle, + IN VOID *DataToHash, + IN UINTN DataToHashLen + ) +{ + BOOLEAN Status; + VOID *HashCtx; + + if (HashPolicy == HASH_INVALID || HashPolicy >= HASH_MAX) { + ASSERT (FALSE); + } + + HashCtx = (VOID *)HashHandle; + + Status = mHashApi[HashPolicy].HashUpdate (HashCtx, DataToHash, DataToHashLen); + + return Status; +} + +/** + Hash complete with Hash Algorithm specified by HashPolicy. + + @param HashPolicy Hash Algorithm Policy. + @param HashHandle Hash handle. + @param Digest Hash Digest. + + @retval TRUE Hash complete and Digest is returned. + @retval FALSE Hash complete unsuccessful. +**/ +BOOLEAN +EFIAPI +HashFinalInternal ( + IN UINT8 HashPolicy, + IN HASH_HANDLE HashHandle, + OUT UINT8 **Digest + ) +{ + BOOLEAN Status; + VOID *HashCtx; + UINT8 DigestData[SHA512_DIGEST_SIZE]; + + if (HashPolicy == HASH_INVALID || HashPolicy >= HASH_MAX) { + ASSERT (FALSE); + } + + HashCtx = (VOID *)HashHandle; + + Status = mHashApi[HashPolicy].HashFinal (HashCtx, DigestData); + CopyMem (*Digest, DigestData, mHashApi[HashPolicy].DigestSize); + + FreePool (HashCtx); + + return Status; +} diff --git a/SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c b/SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c new file mode 100644 index 000000000000..226c2d6a4aae --- /dev/null +++ b/SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c @@ -0,0 +1,100 @@ +/** @file + This library is Unified Hash API. It will redirect hash request to + the hash handler specified by PcdSystemHashPolicy such as SHA1, SHA256, + SHA384 and SM3... + +Copyright (c) 2013 - 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/DebugLib.h> +#include <Library/PcdLib.h> +#include <Library/BaseHashLib.h> + +#include "BaseHashLibCommon.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; + UINT8 HashPolicy; + HASH_HANDLE Handle; + + HashPolicy = PcdGet8 (PcdSystemHashPolicy); + + Status = HashInitInternal (HashPolicy, &Handle); + + *HashHandle = Handle; + + 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; + UINT8 HashPolicy; + + HashPolicy = PcdGet8 (PcdSystemHashPolicy); + + Status = HashUpdateInternal (HashPolicy, HashHandle, DataToHash, DataToHashLen); + + 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; + UINT8 HashPolicy; + + HashPolicy = PcdGet8 (PcdSystemHashPolicy); + + Status = HashFinalInternal (HashPolicy, &HashHandle, &Digest); + + return Status; +} diff --git a/SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c b/SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c new file mode 100644 index 000000000000..43aa0f22277a --- /dev/null +++ b/SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c @@ -0,0 +1,103 @@ +/** @file + This library is Unified Hash API. It will redirect hash request to + the hash handler specified by PcdSystemHashPolicy such as SHA1, SHA256, + SHA384 and SM3... + +Copyright (c) 2013 - 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/DebugLib.h> +#include <Library/PcdLib.h> +#include <Library/HashLib.h> +#include <Library/HobLib.h> +#include <Guid/ZeroGuid.h> + +#include <Library/BaseHashLib.h> +#include "BaseHashLibCommon.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; + UINT8 HashPolicy; + HASH_HANDLE Handle; + + HashPolicy = PcdGet8 (PcdSystemHashPolicy); + + Status = HashInitInternal (HashPolicy, &Handle); + + *HashHandle = Handle; + + 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; + UINT8 HashPolicy; + + HashPolicy = PcdGet8 (PcdSystemHashPolicy); + + Status = HashUpdateInternal (HashPolicy, HashHandle, DataToHash, DataToHashLen); + + 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; + UINT8 HashPolicy; + + HashPolicy = PcdGet8 (PcdSystemHashPolicy); + + Status = HashFinalInternal (HashPolicy, HashHandle, &Digest); + + return Status; +} diff --git a/SecurityPkg/Include/Library/BaseHashLib.h b/SecurityPkg/Include/Library/BaseHashLib.h new file mode 100644 index 000000000000..4b939bbc2c79 --- /dev/null +++ b/SecurityPkg/Include/Library/BaseHashLib.h @@ -0,0 +1,85 @@ +/** @file + The internal header file includes the common header files, defines + internal structure and functions used by ImageVerificationLib. + +Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.<BR> +This program and the accompanying materials +are licensed and made available under the terms and conditions of the BSD License +which accompanies this distribution. The full text of the license may be found at +http://opensource.org/licenses/bsd-license.php + +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. + +**/ + +#ifndef __BASEHASHLIB_H_ +#define __BASEHASHLIB_H_ + +#include <Uefi.h> +#include <Protocol/Hash.h> +#include <Library/HashLib.h> + +// +// 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/SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h b/SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h new file mode 100644 index 000000000000..d8e2caa0bf8d --- /dev/null +++ b/SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h @@ -0,0 +1,141 @@ +/** @file + The internal header file includes the common header files, defines + internal structure and functions used by ImageVerificationLib. + +Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.<BR> +This program and the accompanying materials +are licensed and made available under the terms and conditions of the BSD License +which accompanies this distribution. The full text of the license may be found at +http://opensource.org/licenses/bsd-license.php + +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. + +**/ + +#ifndef __BASEHASHLIB_COMMON_H_ +#define __BASEHASHLIB_COMMON_H_ + +/** + Init hash sequence with Hash Algorithm specified by HashPolicy. + + @param HashHandle Hash handle. + + @retval EFI_SUCCESS Hash start and HashHandle returned. + @retval EFI_UNSUPPORTED System has no HASH library registered. +**/ +BOOLEAN +EFIAPI +HashInitInternal ( + IN UINT8 HashPolicy, + OUT HASH_HANDLE *HashHandle + ); + +/** + Hash complete with Hash Algorithm specified by HashPolicy. + + @param HashPolicy Hash Algorithm Policy. + @param HashHandle Hash handle. + @param Digest Hash Digest. + + @retval TRUE Hash complete and Digest is returned. + @retval FALSE Hash complete unsuccessful. +**/ +BOOLEAN +EFIAPI +HashUpdateInternal ( + IN UINT8 HashPolicy, + IN HASH_HANDLE HashHandle, + IN VOID *DataToHash, + IN UINTN DataToHashLen + ); + +/** + Update hash data with Hash Algorithm specified by HashPolicy. + + @param HashPolicy Hash Algorithm Policy. + @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 +HashFinalInternal ( + IN UINT8 HashPolicy, + IN HASH_HANDLE HashHandle, + OUT UINT8 **Digest + ); + +/** + 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. + +**/ +typedef +UINTN +(EFIAPI *HASH_API_GET_CONTEXT_SIZE) ( + VOID + ); + +/** + Start hash. + + @param HashCtx Hash Context. + + @retval EFI_SUCCESS Hash start and HashHandle returned. + @retval EFI_UNSUPPORTED Unsupported Hash Policy specified. +**/ +typedef +BOOLEAN +(EFIAPI *HASH_API_INIT) ( + OUT VOID *HashCtx + ); + + +/** + Update hash data. + + @param HashCtx Hash Context. + @param Data Data to be hashed. + @param DataSize Data size. + + @retval TRUE Hash updated. + @retval FALSE Hash updated unsuccessful or hash unsupported. +**/ +typedef +BOOLEAN +(EFIAPI *HASH_API_UPDATE) ( + IN OUT VOID *HashCtx, + IN CONST VOID *Data, + IN UINTN DataSize + ); + +/** + Hash complete. + + @param HashCtx Hash Context. + @param Digest Digest. + + @retval TRUE Hash complete and Digest is returned. + @retval FALSE Hash complete unsuccessful. +**/ +typedef +BOOLEAN +(EFIAPI *HASH_API_FINAL) ( + IN OUT VOID *HashCtx, + OUT UINT8 *Digest + ); + +typedef struct { + HASH_API_GET_CONTEXT_SIZE HashGetContextSize; + HASH_API_INIT HashInit; + HASH_API_UPDATE HashUpdate; + HASH_API_FINAL HashFinal; + UINTN DigestSize; +} HASH_API_INTERFACE; + +#endif diff --git a/SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf b/SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf new file mode 100644 index 000000000000..94a497d91e78 --- /dev/null +++ b/SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf @@ -0,0 +1,46 @@ +## @file +# Provides hash service by registered hash handler +# +# This library is Base Hash Lib. It will redirect hash request to each individual +# hash handler registered, such as SHA1, SHA256, SHA384, SM3. +# +# Copyright (c) 2018 - 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] + BaseHashLibCommon.h + BaseHashLibCommon.c + BaseHashLibDxe.c + +[Packages] + MdePkg/MdePkg.dec + CryptoPkg/CryptoPkg.dec + SecurityPkg/SecurityPkg.dec + +[LibraryClasses] + BaseLib + BaseMemoryLib + DebugLib + MemoryAllocationLib + BaseCryptLib + PcdLib + +[Pcd] + gEfiSecurityPkgTokenSpaceGuid.PcdSystemHashPolicy ## CONSUMES diff --git a/SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni b/SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni new file mode 100644 index 000000000000..53e025918828 --- /dev/null +++ b/SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni @@ -0,0 +1,17 @@ +// /** @file +// Provides hash service by registered hash handler +// +// This library is Unified Hash API. It will redirect hash request to each individual +// hash handler registered, such as SHA1, SHA256. Platform can use PcdTpm2HashMask to +// mask some hash engines. +// +// Copyright (c) 2018 - 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/SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf b/SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf new file mode 100644 index 000000000000..1eea1d80b29d --- /dev/null +++ b/SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf @@ -0,0 +1,51 @@ +## @file +# Provides hash service by registered hash handler +# +# This library is BaseCrypto router. It will redirect hash request to each individual +# hash handler registered, such as SHA1, SHA256, SM3. +# +# Copyright (c) 2018 - 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] + BaseHashLibCommon.h + BaseHashLibCommon.c + BaseHashLibPei.c + +[Packages] + MdePkg/MdePkg.dec + SecurityPkg/SecurityPkg.dec + CryptoPkg/CryptoPkg.dec + MdeModulePkg/MdeModulePkg.dec + +[LibraryClasses] + BaseLib + BaseMemoryLib + DebugLib + MemoryAllocationLib + BaseCryptLib + PcdLib + +[Guids] + ## SOMETIMES_CONSUMES ## GUID + gZeroGuid + +[Pcd] + gEfiSecurityPkgTokenSpaceGuid.PcdSystemHashPolicy ## CONSUMES diff --git a/SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni b/SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni new file mode 100644 index 000000000000..a1abcc1cdfa0 --- /dev/null +++ b/SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni @@ -0,0 +1,16 @@ +// /** @file +// Provides hash service by registered hash handler +// +// This library is Unified Hash API. It will redirect hash request to each individual +// hash handler registered, such as SHA1, SHA256. +// +// Copyright (c) 2018 - 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/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec index 5335cc53973a..b40199da8211 100644 --- a/SecurityPkg/SecurityPkg.dec +++ b/SecurityPkg/SecurityPkg.dec @@ -5,7 +5,7 @@ # It also provides the definitions(including PPIs/PROTOCOLs/GUIDs and library classes) # and libraries instances, which are used for those features. # -# Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR> +# Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.<BR> # (C) Copyright 2015 Hewlett Packard Enterprise Development LP <BR> # Copyright (c) 2017, Microsoft Corporation. All rights reserved. <BR> # SPDX-License-Identifier: BSD-2-Clause-Patent @@ -27,6 +27,10 @@ [LibraryClasses] # HashLib|Include/Library/HashLib.h + ## @libraryclass Provides hash interfaces from different implementations. + # + BaseHashLib|Include/Library/HashLib.h + ## @libraryclass Provides a platform specific interface to detect physically present user. # PlatformSecureLib|Include/Library/PlatformSecureLib.h @@ -500,5 +504,22 @@ [PcdsDynamic, PcdsDynamicEx] # @Prompt Tpm2AcpiTableLasa LASA field in TPM2 ACPI table. gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableLasa|0|UINT64|0x00010023 +[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 + gEfiSecurityPkgTokenSpaceGuid.PcdSystemHashPolicy|0x04|UINT8|0x00010024 + [UserExtensions.TianoCore."ExtraFiles"] SecurityPkgExtra.uni diff --git a/SecurityPkg/SecurityPkg.dsc b/SecurityPkg/SecurityPkg.dsc index a2eeadda7a7e..86a5847e2509 100644 --- a/SecurityPkg/SecurityPkg.dsc +++ b/SecurityPkg/SecurityPkg.dsc @@ -1,7 +1,7 @@ ## @file # Security Module Package for All Architectures. # -# Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR> +# Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.<BR> # (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR> # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -95,6 +95,7 @@ [LibraryClasses.common.PEIM] Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf Tcg2PhysicalPresenceLib|SecurityPkg/Library/PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.inf RngLib|MdePkg/Library/BaseRngLib/BaseRngLib.inf + BaseHashLib|SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf [LibraryClasses.common.DXE_DRIVER] HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf @@ -110,6 +111,7 @@ [LibraryClasses.common.DXE_DRIVER] Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibTcg/Tpm12DeviceLibTcg.inf Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf + BaseHashLib|SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf [LibraryClasses.common.UEFI_DRIVER, LibraryClasses.common.DXE_RUNTIME_DRIVER, LibraryClasses.common.DXE_SAL_DRIVER,] HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf @@ -211,6 +213,12 @@ [Components] SecurityPkg/Library/HashLibTpm2/HashLibTpm2.inf + # + # Unified Hash API + # + SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf + SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf + # # TCG Storage. # diff --git a/SecurityPkg/SecurityPkg.uni b/SecurityPkg/SecurityPkg.uni index 68587304d779..41a60f7ec22d 100644 --- a/SecurityPkg/SecurityPkg.uni +++ b/SecurityPkg/SecurityPkg.uni @@ -5,7 +5,7 @@ // It also provides the definitions(including PPIs/PROTOCOLs/GUIDs and library classes) // and libraries instances, which are used for those features. // -// 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 // @@ -295,3 +295,16 @@ #string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTpm2AcpiTableLasa_HELP #language en-US "This PCD defines LASA of TPM2 ACPI table\n\n" "0 means this field is unsupported\n" + +#string STR_gEfiSecurityPkgTokenSpaceGuid_PcdSystemHashPolicy_PROMPT #language en-US "HASH algorithm to verify unsigned PE/COFF image" + +#string STR_gEfiSecurityPkgTokenSpaceGuid_PcdSystemHashPolicy_HELP #language en-US "This PCD indicates the HASH algorithm used by Unified Hash API.<BR><BR>\n" + "Based on the value set, the required algorithm is chosen to calculate\n" + "the hash desired.<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>" -- 2.16.2.windows.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 0/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API 2020-01-15 22:57 [PATCH v3 0/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API Sukerkar, Amol N 2020-01-15 22:57 ` [PATCH v3 1/1] " Sukerkar, Amol N @ 2020-01-15 23:52 ` Michael D Kinney 2020-01-16 0:47 ` Sukerkar, Amol N 1 sibling, 1 reply; 8+ messages in thread From: Michael D Kinney @ 2020-01-15 23:52 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, I still think the handle based registration is too complex for this feature. I recommend a simpler lib design and add it to CryptoPkg instead of SecurityPkg. Providing a different method to access the hashing functions in BaseCryptLib is not a Security feature, it is a Crypto feature. Thanks, Mike > -----Original Message----- > From: Sukerkar, Amol N <amol.n.sukerkar@intel.com> > Sent: Wednesday, January 15, 2020 2:57 PM > To: devel@edk2.groups.io > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; > Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J > <jian.j.wang@intel.com>; Agrawal, Sachin > <sachin.agrawal@intel.com>; Musti, Srinivas > <srinivas.musti@intel.com>; Lakkimsetti, Subash > <subash.lakkimsetti@intel.com> > Subject: [PATCH v3 0/1] SecurityPkg/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 (1): > SecurityPkg/BaseHashLib: Implement Unified Hash > Calculation API > > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c | > 151 ++++++++++++++++++++ > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c | > 100 +++++++++++++ > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c | > 103 +++++++++++++ > SecurityPkg/Include/Library/BaseHashLib.h | > 85 +++++++++++ > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h | > 141 ++++++++++++++++++ > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf | > 46 ++++++ > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni | > 17 +++ > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf | > 51 +++++++ > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni | > 16 +++ > SecurityPkg/SecurityPkg.dec | > 23 ++- > SecurityPkg/SecurityPkg.dsc | > 10 +- > SecurityPkg/SecurityPkg.uni | > 15 +- > 12 files changed, 755 insertions(+), 3 deletions(-) > create mode 100644 > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c > create mode 100644 > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c > create mode 100644 > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c > create mode 100644 > SecurityPkg/Include/Library/BaseHashLib.h > create mode 100644 > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h > create mode 100644 > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf > create mode 100644 > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni > create mode 100644 > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf > create mode 100644 > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni > > -- > 2.16.2.windows.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 0/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API 2020-01-15 23:52 ` [PATCH v3 0/1] " Michael D Kinney @ 2020-01-16 0:47 ` Sukerkar, Amol N 2020-01-16 0:55 ` Michael D Kinney 0 siblings, 1 reply; 8+ messages in thread From: Sukerkar, Amol N @ 2020-01-16 0:47 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, This design does not implement any registration. The hashing algorithm is selected from the array index specified by PcdSystemHashPolicy value, just like switch..case, based on recommendation by Jian. Are you referring to the document attached to Bugzilla ticket mentioned below? I plan to update it as soon as we agree on the final design. Apologies if it was misleading today. The reason this lib was added to SecurityPkg and not CryptoPkg was done because the decision to choose hashing algorithm is based on PCD, PcdSystemHashPolicy. CryptoPkg only provides API for accessing specific hashing algorithm and there is no mechanism to choose, as there is no precedent to using a PCD in CryptoPkg and it does not look like that needs to change. On the other hand, we actually do have API support in SecurityPkg (HashInstanceLib). Our design provides similar API support, although, it is much simpler and does not involve registration as in HashInstanceLib. Do you still think this lib should be implemented in CryptoPkg? If yes, how do you propose the user choose the desired hashing mechanism? Thanks, Amol -----Original Message----- From: Kinney, Michael D <michael.d.kinney@intel.com> Sent: Wednesday, January 15, 2020 4:52 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 v3 0/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API Amol, I still think the handle based registration is too complex for this feature. I recommend a simpler lib design and add it to CryptoPkg instead of SecurityPkg. Providing a different method to access the hashing functions in BaseCryptLib is not a Security feature, it is a Crypto feature. Thanks, Mike > -----Original Message----- > From: Sukerkar, Amol N <amol.n.sukerkar@intel.com> > Sent: Wednesday, January 15, 2020 2:57 PM > To: devel@edk2.groups.io > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Yao, Jiewen > <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Agrawal, > Sachin <sachin.agrawal@intel.com>; Musti, Srinivas > <srinivas.musti@intel.com>; Lakkimsetti, Subash > <subash.lakkimsetti@intel.com> > Subject: [PATCH v3 0/1] SecurityPkg/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 (1): > SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API > > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c | > 151 ++++++++++++++++++++ > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c | > 100 +++++++++++++ > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c | > 103 +++++++++++++ > SecurityPkg/Include/Library/BaseHashLib.h | > 85 +++++++++++ > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h | > 141 ++++++++++++++++++ > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf | > 46 ++++++ > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni | > 17 +++ > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf | > 51 +++++++ > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni | > 16 +++ > SecurityPkg/SecurityPkg.dec | > 23 ++- > SecurityPkg/SecurityPkg.dsc | > 10 +- > SecurityPkg/SecurityPkg.uni | > 15 +- > 12 files changed, 755 insertions(+), 3 deletions(-) create mode > 100644 SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c > create mode 100644 > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c > create mode 100644 > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c > create mode 100644 > SecurityPkg/Include/Library/BaseHashLib.h > create mode 100644 > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h > create mode 100644 > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf > create mode 100644 > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni > create mode 100644 > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf > create mode 100644 > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni > > -- > 2.16.2.windows.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 0/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API 2020-01-16 0:47 ` Sukerkar, Amol N @ 2020-01-16 0:55 ` Michael D Kinney 2020-01-16 2:14 ` Wang, Jian J 0 siblings, 1 reply; 8+ messages in thread From: Michael D Kinney @ 2020-01-16 0:55 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, Add a PCD to CryptoPkg. There are other CryptoPkg extensions I am working on that will also define a PCD. When you build your code and disassemble, are all the hash algorithms included even through a module only needs one? The design I have in mind allows unused hash services to always be optimized away. Mike > -----Original Message----- > From: Sukerkar, Amol N <amol.n.sukerkar@intel.com> > Sent: Wednesday, January 15, 2020 4:48 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 v3 0/1] SecurityPkg/BaseHashLib: > Implement Unified Hash Calculation API > > Hi Mike, > > This design does not implement any registration. The > hashing algorithm is selected from the array index > specified by PcdSystemHashPolicy value, just like > switch..case, based on recommendation by Jian. Are you > referring to the document attached to Bugzilla ticket > mentioned below? I plan to update it as soon as we > agree on the final design. Apologies if it was > misleading today. > > The reason this lib was added to SecurityPkg and not > CryptoPkg was done because the decision to choose > hashing algorithm is based on PCD, PcdSystemHashPolicy. > CryptoPkg only provides API for accessing specific > hashing algorithm and there is no mechanism to choose, > as there is no precedent to using a PCD in CryptoPkg > and it does not look like that needs to change. On the > other hand, we actually do have API support in > SecurityPkg (HashInstanceLib). Our design provides > similar API support, although, it is much simpler and > does not involve registration as in HashInstanceLib. Do > you still think this lib should be implemented in > CryptoPkg? If yes, how do you propose the user choose > the desired hashing mechanism? > > Thanks, > Amol > > -----Original Message----- > From: Kinney, Michael D <michael.d.kinney@intel.com> > Sent: Wednesday, January 15, 2020 4:52 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 v3 0/1] SecurityPkg/BaseHashLib: > Implement Unified Hash Calculation API > > Amol, > > I still think the handle based registration is too > complex for this feature. > > I recommend a simpler lib design and add it to > CryptoPkg instead of SecurityPkg. Providing a > different method to access the hashing functions in > BaseCryptLib is not a Security feature, it is a Crypto > feature. > > Thanks, > > Mike > > > > -----Original Message----- > > From: Sukerkar, Amol N <amol.n.sukerkar@intel.com> > > Sent: Wednesday, January 15, 2020 2:57 PM > > To: devel@edk2.groups.io > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; > Yao, Jiewen > > <jiewen.yao@intel.com>; Wang, Jian J > <jian.j.wang@intel.com>; Agrawal, > > Sachin <sachin.agrawal@intel.com>; Musti, Srinivas > > <srinivas.musti@intel.com>; Lakkimsetti, Subash > > <subash.lakkimsetti@intel.com> > > Subject: [PATCH v3 0/1] SecurityPkg/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 (1): > > SecurityPkg/BaseHashLib: Implement Unified Hash > Calculation API > > > > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c > | > > 151 ++++++++++++++++++++ > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c > | > > 100 +++++++++++++ > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c > | > > 103 +++++++++++++ > > SecurityPkg/Include/Library/BaseHashLib.h > | > > 85 +++++++++++ > > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h > | > > 141 ++++++++++++++++++ > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf > | > > 46 ++++++ > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni > | > > 17 +++ > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf > | > > 51 +++++++ > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni > | > > 16 +++ > > SecurityPkg/SecurityPkg.dec > | > > 23 ++- > > SecurityPkg/SecurityPkg.dsc > | > > 10 +- > > SecurityPkg/SecurityPkg.uni > | > > 15 +- > > 12 files changed, 755 insertions(+), 3 deletions(-) > create mode > > 100644 > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c > > create mode 100644 > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c > > create mode 100644 > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c > > create mode 100644 > > SecurityPkg/Include/Library/BaseHashLib.h > > create mode 100644 > > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h > > create mode 100644 > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf > > create mode 100644 > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni > > create mode 100644 > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf > > create mode 100644 > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni > > > > -- > > 2.16.2.windows.1 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 0/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API 2020-01-16 0:55 ` Michael D Kinney @ 2020-01-16 2:14 ` Wang, Jian J 2020-01-16 4:25 ` Sukerkar, Amol N 0 siblings, 1 reply; 8+ messages in thread From: Wang, Jian J @ 2020-01-16 2:14 UTC (permalink / raw) To: Kinney, Michael D, Sukerkar, Amol N, devel@edk2.groups.io Cc: Yao, Jiewen, Agrawal, Sachin, Musti, Srinivas, Lakkimsetti, Subash Mike, If I remember correctly, the optimization will be left to the PPI/Protocol version of BaseCryptLib, which will be merged into edk2 code base from Mu project. Regards, Jian > -----Original Message----- > From: Kinney, Michael D <michael.d.kinney@intel.com> > Sent: Thursday, January 16, 2020 8:56 AM > 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 v3 0/1] SecurityPkg/BaseHashLib: Implement Unified Hash > Calculation API > > Amol, > > Add a PCD to CryptoPkg. > > There are other CryptoPkg extensions I am working on > that will also define a PCD. > > When you build your code and disassemble, are all the > hash algorithms included even through a module only > needs one? The design I have in mind allows unused > hash services to always be optimized away. > > Mike > > > -----Original Message----- > > From: Sukerkar, Amol N <amol.n.sukerkar@intel.com> > > Sent: Wednesday, January 15, 2020 4:48 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 v3 0/1] SecurityPkg/BaseHashLib: > > Implement Unified Hash Calculation API > > > > Hi Mike, > > > > This design does not implement any registration. The > > hashing algorithm is selected from the array index > > specified by PcdSystemHashPolicy value, just like > > switch..case, based on recommendation by Jian. Are you > > referring to the document attached to Bugzilla ticket > > mentioned below? I plan to update it as soon as we > > agree on the final design. Apologies if it was > > misleading today. > > > > The reason this lib was added to SecurityPkg and not > > CryptoPkg was done because the decision to choose > > hashing algorithm is based on PCD, PcdSystemHashPolicy. > > CryptoPkg only provides API for accessing specific > > hashing algorithm and there is no mechanism to choose, > > as there is no precedent to using a PCD in CryptoPkg > > and it does not look like that needs to change. On the > > other hand, we actually do have API support in > > SecurityPkg (HashInstanceLib). Our design provides > > similar API support, although, it is much simpler and > > does not involve registration as in HashInstanceLib. Do > > you still think this lib should be implemented in > > CryptoPkg? If yes, how do you propose the user choose > > the desired hashing mechanism? > > > > Thanks, > > Amol > > > > -----Original Message----- > > From: Kinney, Michael D <michael.d.kinney@intel.com> > > Sent: Wednesday, January 15, 2020 4:52 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 v3 0/1] SecurityPkg/BaseHashLib: > > Implement Unified Hash Calculation API > > > > Amol, > > > > I still think the handle based registration is too > > complex for this feature. > > > > I recommend a simpler lib design and add it to > > CryptoPkg instead of SecurityPkg. Providing a > > different method to access the hashing functions in > > BaseCryptLib is not a Security feature, it is a Crypto > > feature. > > > > Thanks, > > > > Mike > > > > > > > -----Original Message----- > > > From: Sukerkar, Amol N <amol.n.sukerkar@intel.com> > > > Sent: Wednesday, January 15, 2020 2:57 PM > > > To: devel@edk2.groups.io > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; > > Yao, Jiewen > > > <jiewen.yao@intel.com>; Wang, Jian J > > <jian.j.wang@intel.com>; Agrawal, > > > Sachin <sachin.agrawal@intel.com>; Musti, Srinivas > > > <srinivas.musti@intel.com>; Lakkimsetti, Subash > > > <subash.lakkimsetti@intel.com> > > > Subject: [PATCH v3 0/1] SecurityPkg/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 (1): > > > SecurityPkg/BaseHashLib: Implement Unified Hash > > Calculation API > > > > > > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c > > | > > > 151 ++++++++++++++++++++ > > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c > > | > > > 100 +++++++++++++ > > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c > > | > > > 103 +++++++++++++ > > > SecurityPkg/Include/Library/BaseHashLib.h > > | > > > 85 +++++++++++ > > > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h > > | > > > 141 ++++++++++++++++++ > > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf > > | > > > 46 ++++++ > > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni > > | > > > 17 +++ > > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf > > | > > > 51 +++++++ > > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni > > | > > > 16 +++ > > > SecurityPkg/SecurityPkg.dec > > | > > > 23 ++- > > > SecurityPkg/SecurityPkg.dsc > > | > > > 10 +- > > > SecurityPkg/SecurityPkg.uni > > | > > > 15 +- > > > 12 files changed, 755 insertions(+), 3 deletions(-) > > create mode > > > 100644 > > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c > > > create mode 100644 > > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c > > > create mode 100644 > > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c > > > create mode 100644 > > > SecurityPkg/Include/Library/BaseHashLib.h > > > create mode 100644 > > > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h > > > create mode 100644 > > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf > > > create mode 100644 > > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni > > > create mode 100644 > > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf > > > create mode 100644 > > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni > > > > > > -- > > > 2.16.2.windows.1 > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 0/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API 2020-01-16 2:14 ` Wang, Jian J @ 2020-01-16 4:25 ` Sukerkar, Amol N 2020-01-17 3:49 ` Sukerkar, Amol N 0 siblings, 1 reply; 8+ messages in thread From: Sukerkar, Amol N @ 2020-01-16 4:25 UTC (permalink / raw) To: Wang, Jian J, Kinney, Michael D, devel@edk2.groups.io Cc: Yao, Jiewen, Agrawal, Sachin, Musti, Srinivas, Lakkimsetti, Subash, Sukerkar, Amol N Also, I don't have a definitive way of checking the optimization. I have only been checking size of the FVs. Can you suggest how to? Use of UefiTool does not help or at least, I don't know how to properly use it. Thanks, Amol -----Original Message----- From: Wang, Jian J <jian.j.wang@intel.com> Sent: Wednesday, January 15, 2020 7:15 PM To: Kinney, Michael D <michael.d.kinney@intel.com>; Sukerkar, Amol N <amol.n.sukerkar@intel.com>; devel@edk2.groups.io Cc: Yao, Jiewen <jiewen.yao@intel.com>; Agrawal, Sachin <sachin.agrawal@intel.com>; Musti, Srinivas <srinivas.musti@intel.com>; Lakkimsetti, Subash <subash.lakkimsetti@intel.com> Subject: RE: [PATCH v3 0/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API Mike, If I remember correctly, the optimization will be left to the PPI/Protocol version of BaseCryptLib, which will be merged into edk2 code base from Mu project. Regards, Jian > -----Original Message----- > From: Kinney, Michael D <michael.d.kinney@intel.com> > Sent: Thursday, January 16, 2020 8:56 AM > 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 v3 0/1] SecurityPkg/BaseHashLib: Implement Unified > Hash Calculation API > > Amol, > > Add a PCD to CryptoPkg. > > There are other CryptoPkg extensions I am working on that will also > define a PCD. > > When you build your code and disassemble, are all the hash algorithms > included even through a module only needs one? The design I have in > mind allows unused hash services to always be optimized away. > > Mike > > > -----Original Message----- > > From: Sukerkar, Amol N <amol.n.sukerkar@intel.com> > > Sent: Wednesday, January 15, 2020 4:48 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 v3 0/1] SecurityPkg/BaseHashLib: > > Implement Unified Hash Calculation API > > > > Hi Mike, > > > > This design does not implement any registration. The hashing > > algorithm is selected from the array index specified by > > PcdSystemHashPolicy value, just like switch..case, based on > > recommendation by Jian. Are you referring to the document attached > > to Bugzilla ticket mentioned below? I plan to update it as soon as > > we agree on the final design. Apologies if it was misleading today. > > > > The reason this lib was added to SecurityPkg and not CryptoPkg was > > done because the decision to choose hashing algorithm is based on > > PCD, PcdSystemHashPolicy. > > CryptoPkg only provides API for accessing specific hashing algorithm > > and there is no mechanism to choose, as there is no precedent to > > using a PCD in CryptoPkg and it does not look like that needs to > > change. On the other hand, we actually do have API support in > > SecurityPkg (HashInstanceLib). Our design provides similar API > > support, although, it is much simpler and does not involve > > registration as in HashInstanceLib. Do you still think this lib > > should be implemented in CryptoPkg? If yes, how do you propose the > > user choose the desired hashing mechanism? > > > > Thanks, > > Amol > > > > -----Original Message----- > > From: Kinney, Michael D <michael.d.kinney@intel.com> > > Sent: Wednesday, January 15, 2020 4:52 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 v3 0/1] SecurityPkg/BaseHashLib: > > Implement Unified Hash Calculation API > > > > Amol, > > > > I still think the handle based registration is too complex for this > > feature. > > > > I recommend a simpler lib design and add it to CryptoPkg instead of > > SecurityPkg. Providing a different method to access the hashing > > functions in BaseCryptLib is not a Security feature, it is a Crypto > > feature. > > > > Thanks, > > > > Mike > > > > > > > -----Original Message----- > > > From: Sukerkar, Amol N <amol.n.sukerkar@intel.com> > > > Sent: Wednesday, January 15, 2020 2:57 PM > > > To: devel@edk2.groups.io > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; > > Yao, Jiewen > > > <jiewen.yao@intel.com>; Wang, Jian J > > <jian.j.wang@intel.com>; Agrawal, > > > Sachin <sachin.agrawal@intel.com>; Musti, Srinivas > > > <srinivas.musti@intel.com>; Lakkimsetti, Subash > > > <subash.lakkimsetti@intel.com> > > > Subject: [PATCH v3 0/1] SecurityPkg/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 (1): > > > SecurityPkg/BaseHashLib: Implement Unified Hash > > Calculation API > > > > > > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c > > | > > > 151 ++++++++++++++++++++ > > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c > > | > > > 100 +++++++++++++ > > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c > > | > > > 103 +++++++++++++ > > > SecurityPkg/Include/Library/BaseHashLib.h > > | > > > 85 +++++++++++ > > > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h > > | > > > 141 ++++++++++++++++++ > > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf > > | > > > 46 ++++++ > > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni > > | > > > 17 +++ > > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf > > | > > > 51 +++++++ > > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni > > | > > > 16 +++ > > > SecurityPkg/SecurityPkg.dec > > | > > > 23 ++- > > > SecurityPkg/SecurityPkg.dsc > > | > > > 10 +- > > > SecurityPkg/SecurityPkg.uni > > | > > > 15 +- > > > 12 files changed, 755 insertions(+), 3 deletions(-) > > create mode > > > 100644 > > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c > > > create mode 100644 > > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c > > > create mode 100644 > > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c > > > create mode 100644 > > > SecurityPkg/Include/Library/BaseHashLib.h > > > create mode 100644 > > > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h > > > create mode 100644 > > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf > > > create mode 100644 > > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni > > > create mode 100644 > > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf > > > create mode 100644 > > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni > > > > > > -- > > > 2.16.2.windows.1 > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 0/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API 2020-01-16 4:25 ` Sukerkar, Amol N @ 2020-01-17 3:49 ` Sukerkar, Amol N 0 siblings, 0 replies; 8+ messages in thread From: Sukerkar, Amol N @ 2020-01-17 3:49 UTC (permalink / raw) To: Wang, Jian J, Kinney, Michael D, devel@edk2.groups.io Cc: Yao, Jiewen, Agrawal, Sachin, Musti, Srinivas, Lakkimsetti, Subash, Sukerkar, Amol N Hi Mike, I had an offline conversation with Jian and, now, I agree on the point that bitmap will not optimize the API. I will work on changing back to switch..case. Thanks, Amol -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@intel.com> Sent: Wednesday, January 15, 2020 9:26 PM To: Wang, Jian J <jian.j.wang@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io Cc: Yao, Jiewen <jiewen.yao@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 v3 0/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API Also, I don't have a definitive way of checking the optimization. I have only been checking size of the FVs. Can you suggest how to? Use of UefiTool does not help or at least, I don't know how to properly use it. Thanks, Amol -----Original Message----- From: Wang, Jian J <jian.j.wang@intel.com> Sent: Wednesday, January 15, 2020 7:15 PM To: Kinney, Michael D <michael.d.kinney@intel.com>; Sukerkar, Amol N <amol.n.sukerkar@intel.com>; devel@edk2.groups.io Cc: Yao, Jiewen <jiewen.yao@intel.com>; Agrawal, Sachin <sachin.agrawal@intel.com>; Musti, Srinivas <srinivas.musti@intel.com>; Lakkimsetti, Subash <subash.lakkimsetti@intel.com> Subject: RE: [PATCH v3 0/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API Mike, If I remember correctly, the optimization will be left to the PPI/Protocol version of BaseCryptLib, which will be merged into edk2 code base from Mu project. Regards, Jian > -----Original Message----- > From: Kinney, Michael D <michael.d.kinney@intel.com> > Sent: Thursday, January 16, 2020 8:56 AM > 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 v3 0/1] SecurityPkg/BaseHashLib: Implement Unified > Hash Calculation API > > Amol, > > Add a PCD to CryptoPkg. > > There are other CryptoPkg extensions I am working on that will also > define a PCD. > > When you build your code and disassemble, are all the hash algorithms > included even through a module only needs one? The design I have in > mind allows unused hash services to always be optimized away. > > Mike > > > -----Original Message----- > > From: Sukerkar, Amol N <amol.n.sukerkar@intel.com> > > Sent: Wednesday, January 15, 2020 4:48 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 v3 0/1] SecurityPkg/BaseHashLib: > > Implement Unified Hash Calculation API > > > > Hi Mike, > > > > This design does not implement any registration. The hashing > > algorithm is selected from the array index specified by > > PcdSystemHashPolicy value, just like switch..case, based on > > recommendation by Jian. Are you referring to the document attached > > to Bugzilla ticket mentioned below? I plan to update it as soon as > > we agree on the final design. Apologies if it was misleading today. > > > > The reason this lib was added to SecurityPkg and not CryptoPkg was > > done because the decision to choose hashing algorithm is based on > > PCD, PcdSystemHashPolicy. > > CryptoPkg only provides API for accessing specific hashing algorithm > > and there is no mechanism to choose, as there is no precedent to > > using a PCD in CryptoPkg and it does not look like that needs to > > change. On the other hand, we actually do have API support in > > SecurityPkg (HashInstanceLib). Our design provides similar API > > support, although, it is much simpler and does not involve > > registration as in HashInstanceLib. Do you still think this lib > > should be implemented in CryptoPkg? If yes, how do you propose the > > user choose the desired hashing mechanism? > > > > Thanks, > > Amol > > > > -----Original Message----- > > From: Kinney, Michael D <michael.d.kinney@intel.com> > > Sent: Wednesday, January 15, 2020 4:52 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 v3 0/1] SecurityPkg/BaseHashLib: > > Implement Unified Hash Calculation API > > > > Amol, > > > > I still think the handle based registration is too complex for this > > feature. > > > > I recommend a simpler lib design and add it to CryptoPkg instead of > > SecurityPkg. Providing a different method to access the hashing > > functions in BaseCryptLib is not a Security feature, it is a Crypto > > feature. > > > > Thanks, > > > > Mike > > > > > > > -----Original Message----- > > > From: Sukerkar, Amol N <amol.n.sukerkar@intel.com> > > > Sent: Wednesday, January 15, 2020 2:57 PM > > > To: devel@edk2.groups.io > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; > > Yao, Jiewen > > > <jiewen.yao@intel.com>; Wang, Jian J > > <jian.j.wang@intel.com>; Agrawal, > > > Sachin <sachin.agrawal@intel.com>; Musti, Srinivas > > > <srinivas.musti@intel.com>; Lakkimsetti, Subash > > > <subash.lakkimsetti@intel.com> > > > Subject: [PATCH v3 0/1] SecurityPkg/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 (1): > > > SecurityPkg/BaseHashLib: Implement Unified Hash > > Calculation API > > > > > > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c > > | > > > 151 ++++++++++++++++++++ > > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c > > | > > > 100 +++++++++++++ > > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c > > | > > > 103 +++++++++++++ > > > SecurityPkg/Include/Library/BaseHashLib.h > > | > > > 85 +++++++++++ > > > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h > > | > > > 141 ++++++++++++++++++ > > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf > > | > > > 46 ++++++ > > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni > > | > > > 17 +++ > > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf > > | > > > 51 +++++++ > > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni > > | > > > 16 +++ > > > SecurityPkg/SecurityPkg.dec > > | > > > 23 ++- > > > SecurityPkg/SecurityPkg.dsc > > | > > > 10 +- > > > SecurityPkg/SecurityPkg.uni > > | > > > 15 +- > > > 12 files changed, 755 insertions(+), 3 deletions(-) > > create mode > > > 100644 > > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c > > > create mode 100644 > > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c > > > create mode 100644 > > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c > > > create mode 100644 > > > SecurityPkg/Include/Library/BaseHashLib.h > > > create mode 100644 > > > SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h > > > create mode 100644 > > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf > > > create mode 100644 > > > SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni > > > create mode 100644 > > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf > > > create mode 100644 > > > SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni > > > > > > -- > > > 2.16.2.windows.1 > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-01-17 3:49 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-01-15 22:57 [PATCH v3 0/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API Sukerkar, Amol N 2020-01-15 22:57 ` [PATCH v3 1/1] " Sukerkar, Amol N 2020-01-15 23:52 ` [PATCH v3 0/1] " Michael D Kinney 2020-01-16 0:47 ` Sukerkar, Amol N 2020-01-16 0:55 ` Michael D Kinney 2020-01-16 2:14 ` Wang, Jian J 2020-01-16 4:25 ` Sukerkar, Amol N 2020-01-17 3:49 ` 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