public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2 0/5] Implement SM3 measured boot
@ 2019-05-28 20:40 Imran Desai
  2019-05-28 20:40 ` [PATCH v2 1/5] MdePkg/Protocol/Hash: introduce GUID for SM3 Imran Desai
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Imran Desai @ 2019-05-28 20:40 UTC (permalink / raw)
  To: devel

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.


Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@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>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Stefan Berger <stefanb@linux.ibm.com>


Imran Desai (5):
  MdePkg/Protocol/Hash: introduce GUID for SM3 digest algorithm
  SecurityPkg: introduce the SM3 digest algorithm
  SecurityPkg/HashLibBaseCryptoRouter: recognize the SM3 digest
    algorithm
  SecurityPkg: set SM3 bit in TPM 2.0 hash mask by default
  OvmfPkg: link SM3 support into Tcg2Pei and Tcg2Dxe

 SecurityPkg/SecurityPkg.dec                   |   5 +-
 OvmfPkg/OvmfPkgIa32.dsc                       |   2 +
 OvmfPkg/OvmfPkgIa32X64.dsc                    |   2 +
 OvmfPkg/OvmfPkgX64.dsc                        |   2 +
 SecurityPkg/SecurityPkg.dsc                   |   3 +
 .../HashInstanceLibSm3/HashInstanceLibSm3.inf |  46 ++++++
 MdePkg/Include/Protocol/Hash.h                |   5 +
 SecurityPkg/Include/Library/HashLib.h         |   1 +
 .../HashInstanceLibSm3/HashInstanceLibSm3.c   | 155 ++++++++++++++++++
 .../HashLibBaseCryptoRouterCommon.c           |   1 +
 .../HashInstanceLibSm3/HashInstanceLibSm3.uni |  21 +++
 11 files changed, 241 insertions(+), 2 deletions(-)
 create mode 100644 SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
 create mode 100644 SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.c
 create mode 100644 SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.uni

-- 
2.17.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/5] MdePkg/Protocol/Hash: introduce GUID for SM3
  2019-05-28 20:40 [PATCH v2 0/5] Implement SM3 measured boot Imran Desai
@ 2019-05-28 20:40 ` Imran Desai
  2019-05-28 20:40 ` [PATCH v2 2/5] SecurityPkg: introduce the SM3 digest algorithm Imran Desai
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Imran Desai @ 2019-05-28 20:40 UTC (permalink / raw)
  To: devel

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 adds GUID for SM3 digest algorithm.


Signed-off-by: Imran Desai <imran.desai@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>

---
 MdePkg/Include/Protocol/Hash.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/MdePkg/Include/Protocol/Hash.h b/MdePkg/Include/Protocol/Hash.h
index 931d7916ef1e..8abf1a4fa305 100644
--- a/MdePkg/Include/Protocol/Hash.h
+++ b/MdePkg/Include/Protocol/Hash.h
@@ -48,6 +48,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
     0xcaa4381e, 0x750c, 0x4770, {0xb8, 0x70, 0x7a, 0x23, 0xb4, 0xe4, 0x21, 0x30 } \
   }
 
+#define EFI_HASH_ALGORITHM_SM3_256_GUID \
+  { \
+    0x251C7818, 0x0DBF, 0xE619, { 0x7F, 0xC2, 0xD6, 0xAC, 0x43, 0x42, 0x7D, 0xA3 } \
+  }
+
 #define EFI_HASH_ALGORTIHM_MD5_GUID \
   { \
     0xaf7c79c, 0x65b5, 0x4319, {0xb0, 0xae, 0x44, 0xec, 0x48, 0x4e, 0x4a, 0xd7 } \
-- 
2.17.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 2/5] SecurityPkg: introduce the SM3 digest algorithm
  2019-05-28 20:40 [PATCH v2 0/5] Implement SM3 measured boot Imran Desai
  2019-05-28 20:40 ` [PATCH v2 1/5] MdePkg/Protocol/Hash: introduce GUID for SM3 Imran Desai
@ 2019-05-28 20:40 ` Imran Desai
  2019-06-07 22:17   ` [edk2-devel] " Wang, Jian J
  2019-05-28 20:40 ` [PATCH v2 3/5] SecurityPkg/HashLibBaseCryptoRouter: recognize " Imran Desai
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Imran Desai @ 2019-05-28 20:40 UTC (permalink / raw)
  To: devel


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.
+#
+##
+
+[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
 
 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.
+
+**/
+
+#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
+  @param Sm3Digest SM3 digest
+**/
+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.
+**/
+EFI_STATUS
+EFIAPI
+Sm3HashInit (
+  OUT HASH_HANDLE    *HashHandle
+  )
+{
+  VOID     *Sm3Ctx;
+  UINTN    CtxSize;
+
+  CtxSize = Sm3GetContextSize ();
+  Sm3Ctx = AllocatePool (CtxSize);
+  ASSERT (Sm3Ctx != 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.
+//
+// **/
+
+
+#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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 3/5] SecurityPkg/HashLibBaseCryptoRouter: recognize the SM3 digest algorithm
  2019-05-28 20:40 [PATCH v2 0/5] Implement SM3 measured boot Imran Desai
  2019-05-28 20:40 ` [PATCH v2 1/5] MdePkg/Protocol/Hash: introduce GUID for SM3 Imran Desai
  2019-05-28 20:40 ` [PATCH v2 2/5] SecurityPkg: introduce the SM3 digest algorithm Imran Desai
@ 2019-05-28 20:40 ` Imran Desai
  2019-06-07 22:18   ` [edk2-devel] " Wang, Jian J
  2019-05-28 20:40 ` [PATCH v2 4/5] SecurityPkg: set SM3 bit in TPM 2.0 hash mask by default Imran Desai
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Imran Desai @ 2019-05-28 20:40 UTC (permalink / raw)
  To: devel


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 adds SM3 as an available digest algorithm to crypto router.


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/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterCommon.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterCommon.c b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterCommon.c
index 7f3bdab53066..aec874a9e072 100644
--- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterCommon.c
+++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterCommon.c
@@ -25,6 +25,7 @@ TPM2_HASH_MASK mTpm2HashMask[] = {
   {HASH_ALGORITHM_SHA256_GUID,       HASH_ALG_SHA256},
   {HASH_ALGORITHM_SHA384_GUID,       HASH_ALG_SHA384},
   {HASH_ALGORITHM_SHA512_GUID,       HASH_ALG_SHA512},
+  {HASH_ALGORITHM_SM3_256_GUID,      HASH_ALG_SM3_256},
 };
 
 /**
-- 
2.17.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 4/5] SecurityPkg: set SM3 bit in TPM 2.0 hash mask by default
  2019-05-28 20:40 [PATCH v2 0/5] Implement SM3 measured boot Imran Desai
                   ` (2 preceding siblings ...)
  2019-05-28 20:40 ` [PATCH v2 3/5] SecurityPkg/HashLibBaseCryptoRouter: recognize " Imran Desai
@ 2019-05-28 20:40 ` Imran Desai
  2019-06-07 22:19   ` [edk2-devel] " Wang, Jian J
  2019-05-28 20:40 ` [PATCH v2 5/5] OvmfPkg: link SM3 support into Tcg2Pei and Tcg2Dxe Imran Desai
  2019-05-30  4:55 ` [edk2-devel] [PATCH v2 0/5] Implement SM3 measured boot Wang, Jian J
  5 siblings, 1 reply; 15+ messages in thread
From: Imran Desai @ 2019-05-28 20:40 UTC (permalink / raw)
  To: devel


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 sets SM3 bit in TPM2.0 hash mask by default.

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.dec | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
index 3314f1854be4..fa3a4fcf5869 100644
--- a/SecurityPkg/SecurityPkg.dec
+++ b/SecurityPkg/SecurityPkg.dec
@@ -438,9 +438,10 @@ [PcdsDynamic, PcdsDynamicEx]
   #    BIT1  -  SHA256.<BR>
   #    BIT2  -  SHA384.<BR>
   #    BIT3  -  SHA512.<BR>
+  #    BIT4  -  SM3_256.<BR>
   # @Prompt Hash mask for TPM 2.0
-  # @ValidRange 0x80000001 | 0x00000000 - 0x0000000F
-  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2HashMask|0x0000000F|UINT32|0x00010010
+  # @ValidRange 0x80000001 | 0x00000000 - 0x0000001F
+  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2HashMask|0x0000001F|UINT32|0x00010010
 
   ## This PCD indicated final BIOS supported Hash mask.
   #    Bios may choose to register a subset of PcdTpm2HashMask.
-- 
2.17.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 5/5] OvmfPkg: link SM3 support into Tcg2Pei and Tcg2Dxe
  2019-05-28 20:40 [PATCH v2 0/5] Implement SM3 measured boot Imran Desai
                   ` (3 preceding siblings ...)
  2019-05-28 20:40 ` [PATCH v2 4/5] SecurityPkg: set SM3 bit in TPM 2.0 hash mask by default Imran Desai
@ 2019-05-28 20:40 ` Imran Desai
  2019-07-03 20:37   ` [edk2-devel] " Laszlo Ersek
  2019-05-30  4:55 ` [edk2-devel] [PATCH v2 0/5] Implement SM3 measured boot Wang, Jian J
  5 siblings, 1 reply; 15+ messages in thread
From: Imran Desai @ 2019-05-28 20:40 UTC (permalink / raw)
  To: devel


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 links SM3 support into Tcg2Pei and Tcg2Dxe.


Signed-off-by: Imran Desai <imran.desai@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Stefan Berger <stefanb@linux.ibm.com>
---
 OvmfPkg/OvmfPkgIa32.dsc    | 2 ++
 OvmfPkg/OvmfPkgIa32X64.dsc | 2 ++
 OvmfPkg/OvmfPkgX64.dsc     | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 578fc6c98ec8..fb5944aa6945 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -628,6 +628,7 @@ [Components]
       NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
       NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
       NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
   }
 !if $(TPM2_CONFIG_ENABLE) == TRUE
   SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
@@ -914,5 +915,6 @@ [Components]
       NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
       NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
       NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
   }
 !endif
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index eade8f62d3de..64c231f735c2 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -636,6 +636,7 @@ [Components.IA32]
       NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
       NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
       NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
   }
 !if $(TPM2_CONFIG_ENABLE) == TRUE
   SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
@@ -924,5 +925,6 @@ [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
   }
 !endif
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 733a4c9d8a43..7e46d401a36f 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -635,6 +635,7 @@ [Components]
       NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
       NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
       NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
   }
 !if $(TPM2_CONFIG_ENABLE) == TRUE
   SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
@@ -922,5 +923,6 @@ [Components]
       NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
       NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
       NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
   }
 !endif
-- 
2.17.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v2 0/5] Implement SM3 measured boot
  2019-05-28 20:40 [PATCH v2 0/5] Implement SM3 measured boot Imran Desai
                   ` (4 preceding siblings ...)
  2019-05-28 20:40 ` [PATCH v2 5/5] OvmfPkg: link SM3 support into Tcg2Pei and Tcg2Dxe Imran Desai
@ 2019-05-30  4:55 ` Wang, Jian J
  5 siblings, 0 replies; 15+ messages in thread
From: Wang, Jian J @ 2019-05-30  4:55 UTC (permalink / raw)
  To: devel@edk2.groups.io, Desai, Imran; +Cc: Lu, XiaoyuX

Hi Imran,

You're using non-existing api in this patch series. The openssl upgrading
patch (BZ1089) won't provide them as well. Please wait for another patch.
Xiaoyu is working on it.

Regards,
Jian


> -----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 0/5] Implement SM3 measured boot
> 
> 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.
> 
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@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>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Stefan Berger <stefanb@linux.ibm.com>
> 
> 
> Imran Desai (5):
>   MdePkg/Protocol/Hash: introduce GUID for SM3 digest algorithm
>   SecurityPkg: introduce the SM3 digest algorithm
>   SecurityPkg/HashLibBaseCryptoRouter: recognize the SM3 digest
>     algorithm
>   SecurityPkg: set SM3 bit in TPM 2.0 hash mask by default
>   OvmfPkg: link SM3 support into Tcg2Pei and Tcg2Dxe
> 
>  SecurityPkg/SecurityPkg.dec                   |   5 +-
>  OvmfPkg/OvmfPkgIa32.dsc                       |   2 +
>  OvmfPkg/OvmfPkgIa32X64.dsc                    |   2 +
>  OvmfPkg/OvmfPkgX64.dsc                        |   2 +
>  SecurityPkg/SecurityPkg.dsc                   |   3 +
>  .../HashInstanceLibSm3/HashInstanceLibSm3.inf |  46 ++++++
>  MdePkg/Include/Protocol/Hash.h                |   5 +
>  SecurityPkg/Include/Library/HashLib.h         |   1 +
>  .../HashInstanceLibSm3/HashInstanceLibSm3.c   | 155 ++++++++++++++++++
>  .../HashLibBaseCryptoRouterCommon.c           |   1 +
>  .../HashInstanceLibSm3/HashInstanceLibSm3.uni |  21 +++
>  11 files changed, 241 insertions(+), 2 deletions(-)
>  create mode 100644
> SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
>  create mode 100644
> SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.c
>  create mode 100644
> SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.uni
> 
> --
> 2.17.0
> 
> 
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v2 2/5] SecurityPkg: introduce the SM3 digest algorithm
  2019-05-28 20:40 ` [PATCH v2 2/5] SecurityPkg: introduce the SM3 digest algorithm Imran Desai
@ 2019-06-07 22:17   ` Wang, Jian J
  0 siblings, 0 replies; 15+ messages in thread
From: Wang, Jian J @ 2019-06-07 22:17 UTC (permalink / raw)
  To: devel@edk2.groups.io, Desai, Imran

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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v2 3/5] SecurityPkg/HashLibBaseCryptoRouter: recognize the SM3 digest algorithm
  2019-05-28 20:40 ` [PATCH v2 3/5] SecurityPkg/HashLibBaseCryptoRouter: recognize " Imran Desai
@ 2019-06-07 22:18   ` Wang, Jian J
  0 siblings, 0 replies; 15+ messages in thread
From: Wang, Jian J @ 2019-06-07 22:18 UTC (permalink / raw)
  To: devel@edk2.groups.io, Desai, Imran


Reviewed-by: Jian J Wang <jian.j.wang@intel.com>


> -----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 3/5] SecurityPkg/HashLibBaseCryptoRouter:
> recognize 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 adds SM3 as an available digest algorithm to crypto router.
> 
> 
> 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/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterComm
> on.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterCom
> mon.c
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterCom
> mon.c
> index 7f3bdab53066..aec874a9e072 100644
> ---
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterCom
> mon.c
> +++
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterCom
> mon.c
> @@ -25,6 +25,7 @@ TPM2_HASH_MASK mTpm2HashMask[] = {
>    {HASH_ALGORITHM_SHA256_GUID,       HASH_ALG_SHA256},
>    {HASH_ALGORITHM_SHA384_GUID,       HASH_ALG_SHA384},
>    {HASH_ALGORITHM_SHA512_GUID,       HASH_ALG_SHA512},
> +  {HASH_ALGORITHM_SM3_256_GUID,      HASH_ALG_SM3_256},
>  };
> 
>  /**
> --
> 2.17.0
> 
> 
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v2 4/5] SecurityPkg: set SM3 bit in TPM 2.0 hash mask by default
  2019-05-28 20:40 ` [PATCH v2 4/5] SecurityPkg: set SM3 bit in TPM 2.0 hash mask by default Imran Desai
@ 2019-06-07 22:19   ` Wang, Jian J
  0 siblings, 0 replies; 15+ messages in thread
From: Wang, Jian J @ 2019-06-07 22:19 UTC (permalink / raw)
  To: devel@edk2.groups.io, Desai, Imran


Reviewed-by: Jian J Wang <jian.j.wang@intel.com>


> -----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 4/5] SecurityPkg: set SM3 bit in TPM 2.0 hash
> mask by default
> 
> 
> 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 sets SM3 bit in TPM2.0 hash mask by default.
> 
> 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.dec | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
> index 3314f1854be4..fa3a4fcf5869 100644
> --- a/SecurityPkg/SecurityPkg.dec
> +++ b/SecurityPkg/SecurityPkg.dec
> @@ -438,9 +438,10 @@ [PcdsDynamic, PcdsDynamicEx]
>    #    BIT1  -  SHA256.<BR>
>    #    BIT2  -  SHA384.<BR>
>    #    BIT3  -  SHA512.<BR>
> +  #    BIT4  -  SM3_256.<BR>
>    # @Prompt Hash mask for TPM 2.0
> -  # @ValidRange 0x80000001 | 0x00000000 - 0x0000000F
> -
> gEfiSecurityPkgTokenSpaceGuid.PcdTpm2HashMask|0x0000000F|UINT32|0x00
> 010010
> +  # @ValidRange 0x80000001 | 0x00000000 - 0x0000001F
> +
> gEfiSecurityPkgTokenSpaceGuid.PcdTpm2HashMask|0x0000001F|UINT32|0x00
> 010010
> 
>    ## This PCD indicated final BIOS supported Hash mask.
>    #    Bios may choose to register a subset of PcdTpm2HashMask.
> --
> 2.17.0
> 
> 
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v2 5/5] OvmfPkg: link SM3 support into Tcg2Pei and Tcg2Dxe
  2019-05-28 20:40 ` [PATCH v2 5/5] OvmfPkg: link SM3 support into Tcg2Pei and Tcg2Dxe Imran Desai
@ 2019-07-03 20:37   ` Laszlo Ersek
  2019-07-03 21:18     ` Leif Lindholm
  2019-07-05  3:02     ` Wang, Jian J
  0 siblings, 2 replies; 15+ messages in thread
From: Laszlo Ersek @ 2019-07-03 20:37 UTC (permalink / raw)
  To: imran.desai, Jian J Wang
  Cc: devel, Jordan Justen, Ard Biesheuvel, Marc-André Lureau,
	Stefan Berger, Stephano Cetola, Michael Kinney, Andrew Fish,
	Leif Lindholm

On 05/28/19 22:40, Imran Desai wrote:
> 
> 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 links SM3 support into Tcg2Pei and Tcg2Dxe.
> 
> 
> Signed-off-by: Imran Desai <imran.desai@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Stefan Berger <stefanb@linux.ibm.com>
> ---
>  OvmfPkg/OvmfPkgIa32.dsc    | 2 ++
>  OvmfPkg/OvmfPkgIa32X64.dsc | 2 ++
>  OvmfPkg/OvmfPkgX64.dsc     | 2 ++
>  3 files changed, 6 insertions(+)

Wow, what just happened here?

I'm noticing now that this patch has been pushed to the master branch as
commit a7c7d21ffa9a.

However, *NONE* of the OvmfPkg co-maintainers or reviewers have reviewed
this patch!

The commit message includes "Cc:" lines, but that's a lie. Probably not
an intentional lie, but a lie nonetheless. These patches have *never*
been delivered to my inbox, and if I look at the address list on the
message instance that was reflected by the mailing list, that address
list confirms the same. I'm pretty sure Imran's git configuration has a
bug related to CC's. (I've extended the address list now, manually.)

Jian: please revert this patch immediately, stating, as reason, that the
patch review process was not honored. I'm sorry but I cannot let this
slide -- if you look at commit a7c7d21ffa9a now, it suggests that the
OvmfPkg maintainers / reviewers were CC'd (they weren't), but they
ignored the patch (they didn't -- they couldn't see it), and that
another maintainer pushed the patch just the same (which is factual, but
*wrong*).

After the revert, Imran can resubmit the patch, with *functional* CC's,
and then we can discuss it.

In general, it is fine for one maintainer to push a series that touches
multiple top-level packages. However, that maintainer *MUST* make sure
that each patch has sufficient "M" reviews, and he/she is responsible
for picking up the feedback tags for *all* patches from the list.

Come on now, guys -- have you really known me to be a person that
*silently ignores* an OvmfPkg patch for more than a month? No automated
out-of-office reply, no "please give me some time to review" reply, just
silence?

And even if I missed a patch like that, don't you think a maintainer
deserves a ping first?

... Why are we, as a community, *still* failing at this process?

Laszlo

> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 578fc6c98ec8..fb5944aa6945 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -628,6 +628,7 @@ [Components]
>        NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
>        NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
>        NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
> +      NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
>    }
>  !if $(TPM2_CONFIG_ENABLE) == TRUE
>    SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> @@ -914,5 +915,6 @@ [Components]
>        NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
>        NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
>        NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
> +      NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
>    }
>  !endif
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index eade8f62d3de..64c231f735c2 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -636,6 +636,7 @@ [Components.IA32]
>        NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
>        NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
>        NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
> +      NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
>    }
>  !if $(TPM2_CONFIG_ENABLE) == TRUE
>    SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> @@ -924,5 +925,6 @@ [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
>    }
>  !endif
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 733a4c9d8a43..7e46d401a36f 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -635,6 +635,7 @@ [Components]
>        NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
>        NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
>        NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
> +      NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
>    }
>  !if $(TPM2_CONFIG_ENABLE) == TRUE
>    SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> @@ -922,5 +923,6 @@ [Components]
>        NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
>        NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
>        NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
> +      NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
>    }
>  !endif
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v2 5/5] OvmfPkg: link SM3 support into Tcg2Pei and Tcg2Dxe
  2019-07-03 20:37   ` [edk2-devel] " Laszlo Ersek
@ 2019-07-03 21:18     ` Leif Lindholm
  2019-07-03 22:48       ` Imran Desai
  2019-07-05  3:02     ` Wang, Jian J
  1 sibling, 1 reply; 15+ messages in thread
From: Leif Lindholm @ 2019-07-03 21:18 UTC (permalink / raw)
  To: devel, lersek
  Cc: imran.desai, Jian J Wang, Jordan Justen, Ard Biesheuvel,
	Marc-André Lureau, Stefan Berger, Stephano Cetola,
	Michael Kinney, Andrew Fish

On Wed, Jul 03, 2019 at 10:37:40PM +0200, Laszlo Ersek wrote:
> On 05/28/19 22:40, Imran Desai wrote:
> > 
> > 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 links SM3 support into Tcg2Pei and Tcg2Dxe.
> > 
> > 
> > Signed-off-by: Imran Desai <imran.desai@intel.com>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Cc: Stefan Berger <stefanb@linux.ibm.com>
> > ---
> >  OvmfPkg/OvmfPkgIa32.dsc    | 2 ++
> >  OvmfPkg/OvmfPkgIa32X64.dsc | 2 ++
> >  OvmfPkg/OvmfPkgX64.dsc     | 2 ++
> >  3 files changed, 6 insertions(+)
> 
> Wow, what just happened here?
> 
> I'm noticing now that this patch has been pushed to the master branch as
> commit a7c7d21ffa9a.
> 
> However, *NONE* of the OvmfPkg co-maintainers or reviewers have reviewed
> this patch!

Nor has Jian, or Jiewen, on-list - there is only the tags in the commit.
Other patches in the set have R-b: from Jiewen in the commit, but never
on the list.

Not that it would matter. Patches do not go in without package
maintainer review. Unless there is a *really* good reason, at which
point I expect at least one steward to Ack/R-b:.

> The commit message includes "Cc:" lines, but that's a lie. Probably not
> an intentional lie, but a lie nonetheless. These patches have *never*
> been delivered to my inbox, and if I look at the address list on the
> message instance that was reflected by the mailing list, that address
> list confirms the same. I'm pretty sure Imran's git configuration has a
> bug related to CC's. (I've extended the address list now, manually.)
> 
> Jian: please revert this patch immediately, stating, as reason, that the
> patch review process was not honored. I'm sorry but I cannot let this
> slide -- if you look at commit a7c7d21ffa9a now, it suggests that the
> OvmfPkg maintainers / reviewers were CC'd (they weren't), but they
> ignored the patch (they didn't -- they couldn't see it), and that
> another maintainer pushed the patch just the same (which is factual, but
> *wrong*).

Based on the included Reviewed-by tags from Jiewen that never appeared
on the list, this entire series need to be reverted.

1/5 then needs Reviewed-by: from an MdePkg maintainer, after which
2-4/5 can be pushed again if the commit messages are updated to
contain only the Reviewed-by: and Cc: that have actually happened
on-list.

/
    Leif

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v2 5/5] OvmfPkg: link SM3 support into Tcg2Pei and Tcg2Dxe
  2019-07-03 21:18     ` Leif Lindholm
@ 2019-07-03 22:48       ` Imran Desai
  2019-07-04  8:30         ` Laszlo Ersek
  0 siblings, 1 reply; 15+ messages in thread
From: Imran Desai @ 2019-07-03 22:48 UTC (permalink / raw)
  To: Leif Lindholm, devel@edk2.groups.io, lersek@redhat.com
  Cc: Wang, Jian J, Justen, Jordan L, Ard Biesheuvel,
	Marc-André Lureau, Stefan Berger, Cetola, Stephano,
	Kinney, Michael D, Andrew Fish

Hello Lersek/ Leif,

Like Lersek hinted, this in fact was an inadvertent issue with the git configuration. 
I suspect the suppresscc  in my configuration may have been the source of all confusion. My apologies.

I will send in a new version v5 of the full series of the patches.

I appreciate all the help and comments. Thanks.

PS:

Being used to GitHub pull request methods where in all reviews and conversation happen on the same page i was a little thrown by this process to be honest. In any case as a first time contributor this was a hard way to find out. Perhaps it could be helpful to update the https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process. Particularly the line "git send-email *.patch" could use further commentary regarding CC list.

Thanks and Regards,

Imran Desai
________________________________________
From: Leif Lindholm [leif.lindholm@linaro.org]
Sent: Wednesday, July 03, 2019 2:18 PM
To: devel@edk2.groups.io; lersek@redhat.com
Cc: Desai, Imran; Wang, Jian J; Justen, Jordan L; Ard Biesheuvel; Marc-André Lureau; Stefan Berger; Cetola, Stephano; Kinney, Michael D; Andrew Fish
Subject: Re: [edk2-devel] [PATCH v2 5/5] OvmfPkg: link SM3 support into Tcg2Pei and Tcg2Dxe

On Wed, Jul 03, 2019 at 10:37:40PM +0200, Laszlo Ersek wrote:
> On 05/28/19 22:40, Imran Desai wrote:
> >
> > 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 links SM3 support into Tcg2Pei and Tcg2Dxe.
> >
> >
> > Signed-off-by: Imran Desai <imran.desai@intel.com>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Cc: Stefan Berger <stefanb@linux.ibm.com>
> > ---
> >  OvmfPkg/OvmfPkgIa32.dsc    | 2 ++
> >  OvmfPkg/OvmfPkgIa32X64.dsc | 2 ++
> >  OvmfPkg/OvmfPkgX64.dsc     | 2 ++
> >  3 files changed, 6 insertions(+)
>
> Wow, what just happened here?
>
> I'm noticing now that this patch has been pushed to the master branch as
> commit a7c7d21ffa9a.
>
> However, *NONE* of the OvmfPkg co-maintainers or reviewers have reviewed
> this patch!

Nor has Jian, or Jiewen, on-list - there is only the tags in the commit.
Other patches in the set have R-b: from Jiewen in the commit, but never
on the list.

Not that it would matter. Patches do not go in without package
maintainer review. Unless there is a *really* good reason, at which
point I expect at least one steward to Ack/R-b:.

> The commit message includes "Cc:" lines, but that's a lie. Probably not
> an intentional lie, but a lie nonetheless. These patches have *never*
> been delivered to my inbox, and if I look at the address list on the
> message instance that was reflected by the mailing list, that address
> list confirms the same. I'm pretty sure Imran's git configuration has a
> bug related to CC's. (I've extended the address list now, manually.)
>
> Jian: please revert this patch immediately, stating, as reason, that the
> patch review process was not honored. I'm sorry but I cannot let this
> slide -- if you look at commit a7c7d21ffa9a now, it suggests that the
> OvmfPkg maintainers / reviewers were CC'd (they weren't), but they
> ignored the patch (they didn't -- they couldn't see it), and that
> another maintainer pushed the patch just the same (which is factual, but
> *wrong*).

Based on the included Reviewed-by tags from Jiewen that never appeared
on the list, this entire series need to be reverted.

1/5 then needs Reviewed-by: from an MdePkg maintainer, after which
2-4/5 can be pushed again if the commit messages are updated to
contain only the Reviewed-by: and Cc: that have actually happened
on-list.

/
    Leif

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v2 5/5] OvmfPkg: link SM3 support into Tcg2Pei and Tcg2Dxe
  2019-07-03 22:48       ` Imran Desai
@ 2019-07-04  8:30         ` Laszlo Ersek
  0 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2019-07-04  8:30 UTC (permalink / raw)
  To: Desai, Imran, Leif Lindholm, devel@edk2.groups.io
  Cc: Wang, Jian J, Justen, Jordan L, Ard Biesheuvel,
	Marc-André Lureau, Stefan Berger, Cetola, Stephano,
	Kinney, Michael D, Andrew Fish

On 07/04/19 00:48, Desai, Imran wrote:
> Hello Lersek/ Leif,
> 
> Like Lersek hinted, this in fact was an inadvertent issue with the git configuration. 
> I suspect the suppresscc  in my configuration may have been the source of all confusion. My apologies.

That explains why the CC's didn't occur in reality. It doesn't excuse
that the series was pushed.

> I will send in a new version v5 of the full series of the patches.

Let's wait for the revert first, please.

> I appreciate all the help and comments. Thanks.
> 
> PS:
> 
> Being used to GitHub pull request methods where in all reviews and conversation happen on the same page i was a little thrown by this process to be honest. In any case as a first time contributor this was a hard way to find out. Perhaps it could be helpful to update the https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process. Particularly the line "git send-email *.patch" could use further commentary regarding CC list.

https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers

In particular, the permanent git config given there does not include
"sendemail.suppresscc" at all, and contributor step#24 suggests,

git send-email         \
  --suppress-cc=author \
  --suppress-cc=self   \
  --suppress-cc=cc     \
  --suppress-cc=sob    \
  *.patch

Note that "bodycc" is specifically absent from this option list. That's
deliberate.

Also, now we have a script that automates most if not all of the
permanent git settings, for one's edk2 clone:
"BaseTools/Scripts/SetupGit.py".

Thanks
Laszlo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v2 5/5] OvmfPkg: link SM3 support into Tcg2Pei and Tcg2Dxe
  2019-07-03 20:37   ` [edk2-devel] " Laszlo Ersek
  2019-07-03 21:18     ` Leif Lindholm
@ 2019-07-05  3:02     ` Wang, Jian J
  1 sibling, 0 replies; 15+ messages in thread
From: Wang, Jian J @ 2019-07-05  3:02 UTC (permalink / raw)
  To: Laszlo Ersek, Desai, Imran
  Cc: devel@edk2.groups.io, Justen, Jordan L, Ard Biesheuvel,
	Marc-André Lureau, Stefan Berger, Cetola, Stephano,
	Kinney, Michael D, Andrew Fish, Leif Lindholm

Hi Laszlo,

Very sorry for the mistake. I'm totally responsible for it.

Regards,
Jian


> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Thursday, July 04, 2019 4:38 AM
> To: Desai, Imran <imran.desai@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>
> Cc: devel@edk2.groups.io; Justen, Jordan L <jordan.l.justen@intel.com>;
> Ard Biesheuvel <ard.biesheuvel@linaro.org>; Marc-André Lureau
> <marcandre.lureau@redhat.com>; Stefan Berger <stefanb@linux.ibm.com>;
> Cetola, Stephano <stephano.cetola@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Andrew Fish <afish@apple.com>; Leif
> Lindholm <leif.lindholm@linaro.org>
> Subject: Re: [edk2-devel] [PATCH v2 5/5] OvmfPkg: link SM3 support into
> Tcg2Pei and Tcg2Dxe
> 
> On 05/28/19 22:40, Imran Desai wrote:
> >
> > 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 links SM3 support into Tcg2Pei and Tcg2Dxe.
> >
> >
> > Signed-off-by: Imran Desai <imran.desai@intel.com>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Cc: Stefan Berger <stefanb@linux.ibm.com>
> > ---
> >  OvmfPkg/OvmfPkgIa32.dsc    | 2 ++
> >  OvmfPkg/OvmfPkgIa32X64.dsc | 2 ++
> >  OvmfPkg/OvmfPkgX64.dsc     | 2 ++
> >  3 files changed, 6 insertions(+)
> 
> Wow, what just happened here?
> 
> I'm noticing now that this patch has been pushed to the master branch as
> commit a7c7d21ffa9a.
> 
> However, *NONE* of the OvmfPkg co-maintainers or reviewers have
> reviewed
> this patch!
> 
> The commit message includes "Cc:" lines, but that's a lie. Probably not
> an intentional lie, but a lie nonetheless. These patches have *never*
> been delivered to my inbox, and if I look at the address list on the
> message instance that was reflected by the mailing list, that address
> list confirms the same. I'm pretty sure Imran's git configuration has a
> bug related to CC's. (I've extended the address list now, manually.)
> 
> Jian: please revert this patch immediately, stating, as reason, that the
> patch review process was not honored. I'm sorry but I cannot let this
> slide -- if you look at commit a7c7d21ffa9a now, it suggests that the
> OvmfPkg maintainers / reviewers were CC'd (they weren't), but they
> ignored the patch (they didn't -- they couldn't see it), and that
> another maintainer pushed the patch just the same (which is factual, but
> *wrong*).
> 
> After the revert, Imran can resubmit the patch, with *functional* CC's,
> and then we can discuss it.
> 
> In general, it is fine for one maintainer to push a series that touches
> multiple top-level packages. However, that maintainer *MUST* make sure
> that each patch has sufficient "M" reviews, and he/she is responsible
> for picking up the feedback tags for *all* patches from the list.
> 
> Come on now, guys -- have you really known me to be a person that
> *silently ignores* an OvmfPkg patch for more than a month? No
> automated
> out-of-office reply, no "please give me some time to review" reply, just
> silence?
> 
> And even if I missed a patch like that, don't you think a maintainer
> deserves a ping first?
> 
> ... Why are we, as a community, *still* failing at this process?
> 
> Laszlo
> 
> > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> > index 578fc6c98ec8..fb5944aa6945 100644
> > --- a/OvmfPkg/OvmfPkgIa32.dsc
> > +++ b/OvmfPkg/OvmfPkgIa32.dsc
> > @@ -628,6 +628,7 @@ [Components]
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.
> inf
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.
> inf
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.
> inf
> > +
> NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
> >    }
> >  !if $(TPM2_CONFIG_ENABLE) == TRUE
> >    SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> > @@ -914,5 +915,6 @@ [Components]
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.
> inf
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.
> inf
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.
> inf
> > +
> NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
> >    }
> >  !endif
> > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc
> b/OvmfPkg/OvmfPkgIa32X64.dsc
> > index eade8f62d3de..64c231f735c2 100644
> > --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> > @@ -636,6 +636,7 @@ [Components.IA32]
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.
> inf
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.
> inf
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.
> inf
> > +
> NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
> >    }
> >  !if $(TPM2_CONFIG_ENABLE) == TRUE
> >    SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> > @@ -924,5 +925,6 @@ [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
> >    }
> >  !endif
> > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> > index 733a4c9d8a43..7e46d401a36f 100644
> > --- a/OvmfPkg/OvmfPkgX64.dsc
> > +++ b/OvmfPkg/OvmfPkgX64.dsc
> > @@ -635,6 +635,7 @@ [Components]
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.
> inf
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.
> inf
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.
> inf
> > +
> NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
> >    }
> >  !if $(TPM2_CONFIG_ENABLE) == TRUE
> >    SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> > @@ -922,5 +923,6 @@ [Components]
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.
> inf
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.
> inf
> >
> NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.
> inf
> > +
> NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
> >    }
> >  !endif
> >


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2019-07-05  3:02 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-28 20:40 [PATCH v2 0/5] Implement SM3 measured boot Imran Desai
2019-05-28 20:40 ` [PATCH v2 1/5] MdePkg/Protocol/Hash: introduce GUID for SM3 Imran Desai
2019-05-28 20:40 ` [PATCH v2 2/5] SecurityPkg: introduce the SM3 digest algorithm Imran Desai
2019-06-07 22:17   ` [edk2-devel] " Wang, Jian J
2019-05-28 20:40 ` [PATCH v2 3/5] SecurityPkg/HashLibBaseCryptoRouter: recognize " Imran Desai
2019-06-07 22:18   ` [edk2-devel] " Wang, Jian J
2019-05-28 20:40 ` [PATCH v2 4/5] SecurityPkg: set SM3 bit in TPM 2.0 hash mask by default Imran Desai
2019-06-07 22:19   ` [edk2-devel] " Wang, Jian J
2019-05-28 20:40 ` [PATCH v2 5/5] OvmfPkg: link SM3 support into Tcg2Pei and Tcg2Dxe Imran Desai
2019-07-03 20:37   ` [edk2-devel] " Laszlo Ersek
2019-07-03 21:18     ` Leif Lindholm
2019-07-03 22:48       ` Imran Desai
2019-07-04  8:30         ` Laszlo Ersek
2019-07-05  3:02     ` Wang, Jian J
2019-05-30  4:55 ` [edk2-devel] [PATCH v2 0/5] Implement SM3 measured boot Wang, Jian J

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox