Re: [edk2-devel] [PATCH v2 2/5] SecurityPkg: introduce the SM3 digest algorithm

["Wang, Jian J" <jian.j.wang@intel.com>]

Imran,

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Imran
> Desai
> Sent: Wednesday, May 29, 2019 4:41 AM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH v2 2/5] SecurityPkg: introduce the SM3 digest
> algorithm
> 
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1781
> 
> EDK2 Support for SM3 digest algorithm is needed to enable TPM with SM3 PCR
> banks. This digest algorithm is part of the China Crypto algorithm suite.
> This integration has dependency on the openssl_1_1_1b integration into
> edk2.
> This patch add SM3 algorithm in the hashinstance library.
> 
> 
> Signed-off-by: Imran Desai <imran.desai@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian Wang <jian.j.wang@intel.com>
> ---
>  SecurityPkg/SecurityPkg.dsc                                   |   3 +
>  SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf |  46 ++++++
>  SecurityPkg/Include/Library/HashLib.h                         |   1 +
>  SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.c   | 155
> ++++++++++++++++++++
>  SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.uni |  21 +++
>  5 files changed, 226 insertions(+)
> 
> diff --git a/SecurityPkg/SecurityPkg.dsc b/SecurityPkg/SecurityPkg.dsc
> index a2ee0528f0d2..044319ab5e36 100644
> --- a/SecurityPkg/SecurityPkg.dsc
> +++ b/SecurityPkg/SecurityPkg.dsc
> @@ -222,6 +222,7 @@ [Components.IA32, Components.X64]
>    SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
>    SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
>    SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
> +  SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
> 
>    SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf {
>      <LibraryClasses>
> @@ -236,6 +237,7 @@ [Components.IA32, Components.X64]
> 
> NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
> 
> NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
> 
> NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
> +      NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
>    }
> 
>    SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf {
> @@ -246,6 +248,7 @@ [Components.IA32, Components.X64]
> 
> NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
> 
> NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
> 
> NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
> +      NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
>        PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
>    }
>    SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf {
> diff --git a/SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
> b/SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
> new file mode 100644
> index 000000000000..b2c68b784211
> --- /dev/null
> +++ b/SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
> @@ -0,0 +1,46 @@
> +## @file
> +#  Provides BaseCrypto SM3 hash service
> +#
> +#  This library can be registered to BaseCrypto router, to serve as hash engine.
> +#
> +# Copyright (c) 2013 - 2018, 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.
> +#
> +##

The license header is obsolete. Please use the 2-clause one.

> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005
> +  BASE_NAME                      = HashInstanceLibSm3
> +  MODULE_UNI_FILE                = HashInstanceLibSm3.uni
> +  FILE_GUID                      = C5865D5D-9ACE-39FB-DC7C-0511891D40F9
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = NULL
> +  CONSTRUCTOR                    = HashInstanceLibSm3Constructor
> +
> +#
> +# The following information is for reference only and not required by the build
> tools.
> +#
> +#  VALID_ARCHITECTURES           = IA32 X64
> +#
> +
> +[Sources]
> +  HashInstanceLibSm3.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  SecurityPkg/SecurityPkg.dec
> +  CryptoPkg/CryptoPkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  BaseMemoryLib
> +  DebugLib
> +  Tpm2CommandLib
> +  MemoryAllocationLib
> +  BaseCryptLib
> diff --git a/SecurityPkg/Include/Library/HashLib.h
> b/SecurityPkg/Include/Library/HashLib.h
> index 63f08398788b..24b4c425d7b8 100644
> --- a/SecurityPkg/Include/Library/HashLib.h
> +++ b/SecurityPkg/Include/Library/HashLib.h
> @@ -137,6 +137,7 @@ EFI_STATUS
>  #define HASH_ALGORITHM_SHA256_GUID
> EFI_HASH_ALGORITHM_SHA256_GUID
>  #define HASH_ALGORITHM_SHA384_GUID
> EFI_HASH_ALGORITHM_SHA384_GUID
>  #define HASH_ALGORITHM_SHA512_GUID
> EFI_HASH_ALGORITHM_SHA512_GUID
> +#define HASH_ALGORITHM_SM3_256_GUID
> EFI_HASH_ALGORITHM_SM3_256_GUID
> 

The macro value is not aligned with above line.

>  typedef struct {
>    EFI_GUID                           HashGuid;
> diff --git a/SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.c
> b/SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.c
> new file mode 100644
> index 000000000000..504475ca193a
> --- /dev/null
> +++ b/SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.c
> @@ -0,0 +1,155 @@
> +/** @file
> +  This library is BaseCrypto SM3 hash instance.
> +  It can be registered to BaseCrypto router, to serve as hash engine.
> +
> +Copyright (c) 2013 - 2018, 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.
> +
> +**/
> +

The license header is obsolete. Please use the 2-clause one.

> +#include <PiPei.h>
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/Tpm2CommandLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/BaseCryptLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/HashLib.h>
> +
> +/**
> +  The function set SM3 to digest list.
> +
> +  @param DigestList   digest list

Please capitalize the parameter description.

> +  @param Sm3Digest SM3 digest

The parameter description is not aligned to above line.

> +**/
> +VOID
> +Tpm2SetSm3ToDigestList (
> +  IN TPML_DIGEST_VALUES *DigestList,
> +  IN UINT8              *Sm3Digest
> +  )
> +{
> +  DigestList->count = 1;
> +  DigestList->digests[0].hashAlg = TPM_ALG_SM3_256;
> +  CopyMem (
> +    DigestList->digests[0].digest.sm3_256,
> +    Sm3Digest,
> +    SM3_256_DIGEST_SIZE
> +    );
> +}
> +
> +/**
> +  Start hash sequence.
> +
> +  @param HashHandle Hash handle.
> +
> +  @retval EFI_SUCCESS          Hash sequence start and HandleHandle returned.
> +  @retval EFI_OUT_OF_RESOURCES No enough resource to start hash.

No code to return this value. Consider replace ASSERT with if check. See below.

> +**/
> +EFI_STATUS
> +EFIAPI
> +Sm3HashInit (
> +  OUT HASH_HANDLE    *HashHandle
> +  )
> +{
> +  VOID     *Sm3Ctx;
> +  UINTN    CtxSize;
> +
> +  CtxSize = Sm3GetContextSize ();
> +  Sm3Ctx = AllocatePool (CtxSize);
> +  ASSERT (Sm3Ctx != NULL);

Consider to replace ASSERT with if(...) and return EFI_OUT_OF_RESOURCES if NULL.

> +
> +  Sm3Init (Sm3Ctx);
> +
> +  *HashHandle = (HASH_HANDLE)Sm3Ctx;
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  Update hash sequence data.
> +
> +  @param HashHandle    Hash handle.
> +  @param DataToHash    Data to be hashed.
> +  @param DataToHashLen Data size.
> +
> +  @retval EFI_SUCCESS     Hash sequence updated.
> +**/
> +EFI_STATUS
> +EFIAPI
> +Sm3HashUpdate (
> +  IN HASH_HANDLE    HashHandle,
> +  IN VOID           *DataToHash,
> +  IN UINTN          DataToHashLen
> +  )
> +{
> +  VOID     *Sm3Ctx;
> +
> +  Sm3Ctx = (VOID *)HashHandle;
> +  Sm3Update (Sm3Ctx, DataToHash, DataToHashLen);
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  Complete hash sequence complete.
> +
> +  @param HashHandle    Hash handle.
> +  @param DigestList    Digest list.
> +
> +  @retval EFI_SUCCESS     Hash sequence complete and DigestList is returned.
> +**/
> +EFI_STATUS
> +EFIAPI
> +Sm3HashFinal (
> +  IN HASH_HANDLE         HashHandle,
> +  OUT TPML_DIGEST_VALUES *DigestList
> +  )
> +{
> +  UINT8         Digest[SM3_256_DIGEST_SIZE];
> +  VOID          *Sm3Ctx;
> +
> +  Sm3Ctx = (VOID *)HashHandle;
> +  Sm3Final (Sm3Ctx, Digest);
> +
> +  FreePool (Sm3Ctx);
> +
> +  Tpm2SetSm3ToDigestList (DigestList, Digest);
> +
> +  return EFI_SUCCESS;
> +}
> +
> +HASH_INTERFACE  mSm3InternalHashInstance = {
> +  HASH_ALGORITHM_SM3_256_GUID,
> +  Sm3HashInit,
> +  Sm3HashUpdate,
> +  Sm3HashFinal,
> +};
> +
> +/**
> +  The function register SM3 instance.
> +
> +  @retval EFI_SUCCESS   SM3 instance is registered, or system dose not support
> register SM3 instance
> +**/
> +EFI_STATUS
> +EFIAPI
> +HashInstanceLibSm3Constructor (
> +  VOID
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  Status = RegisterHashInterfaceLib (&mSm3InternalHashInstance);
> +  if ((Status == EFI_SUCCESS) || (Status == EFI_UNSUPPORTED)) {
> +    //
> +    // Unsupported means platform policy does not need this instance enabled.
> +    //
> +    return EFI_SUCCESS;
> +  }
> +  return Status;
> +}
> diff --git a/SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.uni
> b/SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.uni
> new file mode 100644
> index 000000000000..8d985feeaca1
> --- /dev/null
> +++ b/SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.uni
> @@ -0,0 +1,21 @@
> +// /** @file
> +// Provides BaseCrypto SM3 hash service
> +//
> +// This library can be registered to BaseCrypto router, to serve as hash engine.
> +//
> +// Copyright (c) 2013 - 2014, 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.
> +//
> +// **/
> +

The license header is obsolete. Please use the 2-clause one.

Regards,
Jian

> +
> +#string STR_MODULE_ABSTRACT             #language en-US "Provides BaseCrypto
> SM3 hash service"
> +
> +#string STR_MODULE_DESCRIPTION          #language en-US "This library can be
> registered to BaseCrypto router, to serve as hash engine."
> +
> --
> 2.17.0
> 
> 
>