linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] tpm: use SM3 instead of SM3_256
@ 2021-10-09 13:08 Tianjia Zhang
  2021-10-09 13:08 ` [PATCH 1/2] crypto: " Tianjia Zhang
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Tianjia Zhang @ 2021-10-09 13:08 UTC (permalink / raw)
  To: James Bottomley, Jarkko Sakkinen, Mimi Zohar, Jonathan Corbet,
	Herbert Xu, David S. Miller, Peter Huewe, Jason Gunthorpe,
	David Howells, James Morris, Serge E. Hallyn, Jerry Snitselaar,
	linux-integrity, keyrings, linux-doc, linux-kernel, linux-crypto,
	linux-security-module
  Cc: Tianjia Zhang

According to https://tools.ietf.org/id/draft-oscca-cfrg-sm3-01.html,
SM3 always produces a 256-bit hash value and there are no plans for
other length development, so there is no ambiguity in the name of sm3.

Tianjia Zhang (2):
  crypto: use SM3 instead of SM3_256
  tpm: use SM3 instead of SM3_256

 Documentation/security/keys/trusted-encrypted.rst | 2 +-
 crypto/hash_info.c                                | 4 ++--
 drivers/char/tpm/tpm-sysfs.c                      | 4 ++--
 drivers/char/tpm/tpm2-cmd.c                       | 2 +-
 include/crypto/hash_info.h                        | 2 +-
 include/linux/tpm.h                               | 2 +-
 include/uapi/linux/hash_info.h                    | 2 +-
 security/keys/trusted-keys/trusted_tpm2.c         | 2 +-
 8 files changed, 10 insertions(+), 10 deletions(-)

-- 
2.19.1.3.ge56e4f7


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

* [PATCH 1/2] crypto: use SM3 instead of SM3_256
  2021-10-09 13:08 [PATCH 0/2] tpm: use SM3 instead of SM3_256 Tianjia Zhang
@ 2021-10-09 13:08 ` Tianjia Zhang
  2021-10-18 13:05   ` James Bottomley
  2021-10-09 13:08 ` [PATCH 2/2] tpm: " Tianjia Zhang
  2021-10-09 13:29 ` [PATCH 0/2] " James Bottomley
  2 siblings, 1 reply; 16+ messages in thread
From: Tianjia Zhang @ 2021-10-09 13:08 UTC (permalink / raw)
  To: James Bottomley, Jarkko Sakkinen, Mimi Zohar, Jonathan Corbet,
	Herbert Xu, David S. Miller, Peter Huewe, Jason Gunthorpe,
	David Howells, James Morris, Serge E. Hallyn, Jerry Snitselaar,
	linux-integrity, keyrings, linux-doc, linux-kernel, linux-crypto,
	linux-security-module
  Cc: Tianjia Zhang

According to https://tools.ietf.org/id/draft-oscca-cfrg-sm3-01.html,
SM3 always produces a 256-bit hash value and there are no plans for
other length development, so there is no ambiguity in the name of sm3.

Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
---
 Documentation/security/keys/trusted-encrypted.rst | 2 +-
 crypto/hash_info.c                                | 4 ++--
 drivers/char/tpm/tpm2-cmd.c                       | 2 +-
 include/crypto/hash_info.h                        | 2 +-
 include/uapi/linux/hash_info.h                    | 2 +-
 security/keys/trusted-keys/trusted_tpm2.c         | 2 +-
 6 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
index 80d5a5af62a1..3292461517f6 100644
--- a/Documentation/security/keys/trusted-encrypted.rst
+++ b/Documentation/security/keys/trusted-encrypted.rst
@@ -162,7 +162,7 @@ Usage::
                      default 1 (resealing allowed)
        hash=         hash algorithm name as a string. For TPM 1.x the only
                      allowed value is sha1. For TPM 2.x the allowed values
-                     are sha1, sha256, sha384, sha512 and sm3-256.
+                     are sha1, sha256, sha384, sha512 and sm3.
        policydigest= digest for the authorization policy. must be calculated
                      with the same hash algorithm as specified by the 'hash='
                      option.
diff --git a/crypto/hash_info.c b/crypto/hash_info.c
index a49ff96bde77..fe0119407219 100644
--- a/crypto/hash_info.c
+++ b/crypto/hash_info.c
@@ -26,7 +26,7 @@ const char *const hash_algo_name[HASH_ALGO__LAST] = {
 	[HASH_ALGO_TGR_128]	= "tgr128",
 	[HASH_ALGO_TGR_160]	= "tgr160",
 	[HASH_ALGO_TGR_192]	= "tgr192",
-	[HASH_ALGO_SM3_256]	= "sm3",
+	[HASH_ALGO_SM3]		= "sm3",
 	[HASH_ALGO_STREEBOG_256] = "streebog256",
 	[HASH_ALGO_STREEBOG_512] = "streebog512",
 };
@@ -50,7 +50,7 @@ const int hash_digest_size[HASH_ALGO__LAST] = {
 	[HASH_ALGO_TGR_128]	= TGR128_DIGEST_SIZE,
 	[HASH_ALGO_TGR_160]	= TGR160_DIGEST_SIZE,
 	[HASH_ALGO_TGR_192]	= TGR192_DIGEST_SIZE,
-	[HASH_ALGO_SM3_256]	= SM3256_DIGEST_SIZE,
+	[HASH_ALGO_SM3]		= SM3_DIGEST_SIZE,
 	[HASH_ALGO_STREEBOG_256] = STREEBOG256_DIGEST_SIZE,
 	[HASH_ALGO_STREEBOG_512] = STREEBOG512_DIGEST_SIZE,
 };
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index a25815a6f625..20f55de9d87b 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -19,7 +19,7 @@ static struct tpm2_hash tpm2_hash_map[] = {
 	{HASH_ALGO_SHA256, TPM_ALG_SHA256},
 	{HASH_ALGO_SHA384, TPM_ALG_SHA384},
 	{HASH_ALGO_SHA512, TPM_ALG_SHA512},
-	{HASH_ALGO_SM3_256, TPM_ALG_SM3_256},
+	{HASH_ALGO_SM3, TPM_ALG_SM3_256},
 };
 
 int tpm2_get_timeouts(struct tpm_chip *chip)
diff --git a/include/crypto/hash_info.h b/include/crypto/hash_info.h
index dd4f06785049..c1e6b2884732 100644
--- a/include/crypto/hash_info.h
+++ b/include/crypto/hash_info.h
@@ -32,7 +32,7 @@
 #define TGR192_DIGEST_SIZE 24
 
 /* not defined in include/crypto/ */
-#define SM3256_DIGEST_SIZE 32
+#define SM3_DIGEST_SIZE 32
 
 extern const char *const hash_algo_name[HASH_ALGO__LAST];
 extern const int hash_digest_size[HASH_ALGO__LAST];
diff --git a/include/uapi/linux/hash_info.h b/include/uapi/linux/hash_info.h
index 74a8609fcb4d..1355525dd4aa 100644
--- a/include/uapi/linux/hash_info.h
+++ b/include/uapi/linux/hash_info.h
@@ -32,7 +32,7 @@ enum hash_algo {
 	HASH_ALGO_TGR_128,
 	HASH_ALGO_TGR_160,
 	HASH_ALGO_TGR_192,
-	HASH_ALGO_SM3_256,
+	HASH_ALGO_SM3,
 	HASH_ALGO_STREEBOG_256,
 	HASH_ALGO_STREEBOG_512,
 	HASH_ALGO__LAST
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 0165da386289..52a696035176 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -23,7 +23,7 @@ static struct tpm2_hash tpm2_hash_map[] = {
 	{HASH_ALGO_SHA256, TPM_ALG_SHA256},
 	{HASH_ALGO_SHA384, TPM_ALG_SHA384},
 	{HASH_ALGO_SHA512, TPM_ALG_SHA512},
-	{HASH_ALGO_SM3_256, TPM_ALG_SM3_256},
+	{HASH_ALGO_SM3, TPM_ALG_SM3_256},
 };
 
 static u32 tpm2key_oid[] = { 2, 23, 133, 10, 1, 5 };
-- 
2.19.1.3.ge56e4f7


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

* [PATCH 2/2] tpm: use SM3 instead of SM3_256
  2021-10-09 13:08 [PATCH 0/2] tpm: use SM3 instead of SM3_256 Tianjia Zhang
  2021-10-09 13:08 ` [PATCH 1/2] crypto: " Tianjia Zhang
@ 2021-10-09 13:08 ` Tianjia Zhang
  2021-10-12 15:21   ` Jarkko Sakkinen
  2021-10-09 13:29 ` [PATCH 0/2] " James Bottomley
  2 siblings, 1 reply; 16+ messages in thread
From: Tianjia Zhang @ 2021-10-09 13:08 UTC (permalink / raw)
  To: James Bottomley, Jarkko Sakkinen, Mimi Zohar, Jonathan Corbet,
	Herbert Xu, David S. Miller, Peter Huewe, Jason Gunthorpe,
	David Howells, James Morris, Serge E. Hallyn, Jerry Snitselaar,
	linux-integrity, keyrings, linux-doc, linux-kernel, linux-crypto,
	linux-security-module
  Cc: Tianjia Zhang

According to https://tools.ietf.org/id/draft-oscca-cfrg-sm3-01.html,
SM3 always produces a 256-bit hash value and there are no plans for
other length development, so there is no ambiguity in the name of sm3.

Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
---
 drivers/char/tpm/tpm-sysfs.c              | 4 ++--
 drivers/char/tpm/tpm2-cmd.c               | 2 +-
 include/linux/tpm.h                       | 2 +-
 security/keys/trusted-keys/trusted_tpm2.c | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 63f03cfb8e6a..fe6c785dc84a 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -471,7 +471,7 @@ PCR_ATTR_BUILD(TPM_ALG_SHA1, sha1);
 PCR_ATTR_BUILD(TPM_ALG_SHA256, sha256);
 PCR_ATTR_BUILD(TPM_ALG_SHA384, sha384);
 PCR_ATTR_BUILD(TPM_ALG_SHA512, sha512);
-PCR_ATTR_BUILD(TPM_ALG_SM3_256, sm3);
+PCR_ATTR_BUILD(TPM_ALG_SM3, sm3);
 
 
 void tpm_sysfs_add_device(struct tpm_chip *chip)
@@ -500,7 +500,7 @@ void tpm_sysfs_add_device(struct tpm_chip *chip)
 		case TPM_ALG_SHA512:
 			chip->groups[chip->groups_cnt++] = &pcr_group_sha512;
 			break;
-		case TPM_ALG_SM3_256:
+		case TPM_ALG_SM3:
 			chip->groups[chip->groups_cnt++] = &pcr_group_sm3;
 			break;
 		default:
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 20f55de9d87b..d5a9410d2273 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -19,7 +19,7 @@ static struct tpm2_hash tpm2_hash_map[] = {
 	{HASH_ALGO_SHA256, TPM_ALG_SHA256},
 	{HASH_ALGO_SHA384, TPM_ALG_SHA384},
 	{HASH_ALGO_SHA512, TPM_ALG_SHA512},
-	{HASH_ALGO_SM3, TPM_ALG_SM3_256},
+	{HASH_ALGO_SM3, TPM_ALG_SM3},
 };
 
 int tpm2_get_timeouts(struct tpm_chip *chip)
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index aa11fe323c56..56a79fee1250 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -40,7 +40,7 @@ enum tpm_algorithms {
 	TPM_ALG_SHA384		= 0x000C,
 	TPM_ALG_SHA512		= 0x000D,
 	TPM_ALG_NULL		= 0x0010,
-	TPM_ALG_SM3_256		= 0x0012,
+	TPM_ALG_SM3		= 0x0012,
 };
 
 /*
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 52a696035176..b15a9961213d 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -23,7 +23,7 @@ static struct tpm2_hash tpm2_hash_map[] = {
 	{HASH_ALGO_SHA256, TPM_ALG_SHA256},
 	{HASH_ALGO_SHA384, TPM_ALG_SHA384},
 	{HASH_ALGO_SHA512, TPM_ALG_SHA512},
-	{HASH_ALGO_SM3, TPM_ALG_SM3_256},
+	{HASH_ALGO_SM3, TPM_ALG_SM3},
 };
 
 static u32 tpm2key_oid[] = { 2, 23, 133, 10, 1, 5 };
-- 
2.19.1.3.ge56e4f7


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

* Re: [PATCH 0/2] tpm: use SM3 instead of SM3_256
  2021-10-09 13:08 [PATCH 0/2] tpm: use SM3 instead of SM3_256 Tianjia Zhang
  2021-10-09 13:08 ` [PATCH 1/2] crypto: " Tianjia Zhang
  2021-10-09 13:08 ` [PATCH 2/2] tpm: " Tianjia Zhang
@ 2021-10-09 13:29 ` James Bottomley
  2021-10-11  7:02   ` Tianjia Zhang
  2 siblings, 1 reply; 16+ messages in thread
From: James Bottomley @ 2021-10-09 13:29 UTC (permalink / raw)
  To: Tianjia Zhang, Jarkko Sakkinen, Mimi Zohar, Jonathan Corbet,
	Herbert Xu, David S. Miller, Peter Huewe, Jason Gunthorpe,
	David Howells, James Morris, Serge E. Hallyn, Jerry Snitselaar,
	linux-integrity, keyrings, linux-doc, linux-kernel, linux-crypto,
	linux-security-module

On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
> According to https://tools.ietf.org/id/draft-oscca-cfrg-sm3-01.html,
> SM3 always produces a 256-bit hash value and there are no plans for
> other length development, so there is no ambiguity in the name of
> sm3.

For the TPM we're following the TPM Library specification

https://trustedcomputinggroup.org/resource/tpm-library-specification/

Which is very clear: the algorithm name is TPM_ALG_SM3_256

We're using sm3 as our exposed name because that's what linux crypto
uses, so there should be no problem in what the end user sees, but
changing to non standard TPM definitions is only going to cause
confusion at the kernel level.

James



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

* Re: [PATCH 0/2] tpm: use SM3 instead of SM3_256
  2021-10-09 13:29 ` [PATCH 0/2] " James Bottomley
@ 2021-10-11  7:02   ` Tianjia Zhang
  0 siblings, 0 replies; 16+ messages in thread
From: Tianjia Zhang @ 2021-10-11  7:02 UTC (permalink / raw)
  To: jejb, Jarkko Sakkinen, Mimi Zohar, Jonathan Corbet, Herbert Xu,
	David S. Miller, Peter Huewe, Jason Gunthorpe, David Howells,
	James Morris, Serge E. Hallyn, Jerry Snitselaar, linux-integrity,
	keyrings, linux-doc, linux-kernel, linux-crypto,
	linux-security-module

Hi James,

On 10/9/21 9:29 PM, James Bottomley wrote:
> On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
>> According to https://tools.ietf.org/id/draft-oscca-cfrg-sm3-01.html,
>> SM3 always produces a 256-bit hash value and there are no plans for
>> other length development, so there is no ambiguity in the name of
>> sm3.
> 
> For the TPM we're following the TPM Library specification
> 
> https://trustedcomputinggroup.org/resource/tpm-library-specification/
> 
> Which is very clear: the algorithm name is TPM_ALG_SM3_256
> 
> We're using sm3 as our exposed name because that's what linux crypto
> uses, so there should be no problem in what the end user sees, but
> changing to non standard TPM definitions is only going to cause
> confusion at the kernel level.
> 
> James
> 

Thanks for your attention. This is really tricky. I will contact 
trustedcomputinggroup first and give some suggestions, It would be best 
if a more standard algorithm name can be used from the source of the 
specification.

I think the macro definition of the crypto directory can remove this 
suffix first, that is, apply patch 1. What's your opinion?

Best regards,
Tianjia

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

* Re: [PATCH 2/2] tpm: use SM3 instead of SM3_256
  2021-10-09 13:08 ` [PATCH 2/2] tpm: " Tianjia Zhang
@ 2021-10-12 15:21   ` Jarkko Sakkinen
  2021-10-14  9:46     ` Tianjia Zhang
  0 siblings, 1 reply; 16+ messages in thread
From: Jarkko Sakkinen @ 2021-10-12 15:21 UTC (permalink / raw)
  To: Tianjia Zhang, James Bottomley, Mimi Zohar, Jonathan Corbet,
	Herbert Xu, David S. Miller, Peter Huewe, Jason Gunthorpe,
	David Howells, James Morris, Serge E. Hallyn, Jerry Snitselaar,
	linux-integrity, keyrings, linux-doc, linux-kernel, linux-crypto,
	linux-security-module

On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
> According to https://tools.ietf.org/id/draft-oscca-cfrg-sm3-01.html,
> SM3 always produces a 256-bit hash value and there are no plans for
> other length development, so there is no ambiguity in the name of sm3.
> 
> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>

This is not enough to make any changes because the commit message
does not describe what goes wrong if we keep it as it was.

/Jarkko


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

* Re: [PATCH 2/2] tpm: use SM3 instead of SM3_256
  2021-10-12 15:21   ` Jarkko Sakkinen
@ 2021-10-14  9:46     ` Tianjia Zhang
  2021-10-15 15:19       ` Jarkko Sakkinen
  0 siblings, 1 reply; 16+ messages in thread
From: Tianjia Zhang @ 2021-10-14  9:46 UTC (permalink / raw)
  To: Jarkko Sakkinen, James Bottomley, Mimi Zohar, Jonathan Corbet,
	Herbert Xu, David S. Miller, Peter Huewe, Jason Gunthorpe,
	David Howells, James Morris, Serge E. Hallyn, Jerry Snitselaar,
	linux-integrity, keyrings, linux-doc, linux-kernel, linux-crypto,
	linux-security-module

Hi Jarkko,

On 10/12/21 11:21 PM, Jarkko Sakkinen wrote:
> On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
>> According to https://tools.ietf.org/id/draft-oscca-cfrg-sm3-01.html,
>> SM3 always produces a 256-bit hash value and there are no plans for
>> other length development, so there is no ambiguity in the name of sm3.
>>
>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> 
> This is not enough to make any changes because the commit message
> does not describe what goes wrong if we keep it as it was.
> 
> /Jarkko
> 

This did not cause an error, just to use a more standard algorithm name. 
If it is possible to use the SM3 name instead of SM3_256 if it can be 
specified from the source, it is of course better. I have contacted the 
trustedcomputinggroup and have not yet received a reply.

Best regards,
Tianjia

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

* Re: [PATCH 2/2] tpm: use SM3 instead of SM3_256
  2021-10-14  9:46     ` Tianjia Zhang
@ 2021-10-15 15:19       ` Jarkko Sakkinen
  2021-10-18  2:37         ` Tianjia Zhang
  0 siblings, 1 reply; 16+ messages in thread
From: Jarkko Sakkinen @ 2021-10-15 15:19 UTC (permalink / raw)
  To: Tianjia Zhang, James Bottomley, Mimi Zohar, Jonathan Corbet,
	Herbert Xu, David S. Miller, Peter Huewe, Jason Gunthorpe,
	David Howells, James Morris, Serge E. Hallyn, Jerry Snitselaar,
	linux-integrity, keyrings, linux-doc, linux-kernel, linux-crypto,
	linux-security-module

On Thu, 2021-10-14 at 17:46 +0800, Tianjia Zhang wrote:
> Hi Jarkko,
> 
> On 10/12/21 11:21 PM, Jarkko Sakkinen wrote:
> > On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
> > > According to https://tools.ietf.org/id/draft-oscca-cfrg-sm3-01.html,
> > > SM3 always produces a 256-bit hash value and there are no plans for
> > > other length development, so there is no ambiguity in the name of sm3.
> > > 
> > > Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> > 
> > This is not enough to make any changes because the commit message
> > does not describe what goes wrong if we keep it as it was.
> > 
> > /Jarkko
> > 
> 
> This did not cause an error, just to use a more standard algorithm name. 
> If it is possible to use the SM3 name instead of SM3_256 if it can be 
> specified from the source, it is of course better. I have contacted the 
> trustedcomputinggroup and have not yet received a reply.
> 
> Best regards,
> Tianjia

Why don't you then create a patch set that fully removes SM3_256, if it
is incorrect?

This looks a bit half-baked patch set.

/Jarkko

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

* Re: [PATCH 2/2] tpm: use SM3 instead of SM3_256
  2021-10-15 15:19       ` Jarkko Sakkinen
@ 2021-10-18  2:37         ` Tianjia Zhang
  2021-10-18 12:48           ` Jarkko Sakkinen
  0 siblings, 1 reply; 16+ messages in thread
From: Tianjia Zhang @ 2021-10-18  2:37 UTC (permalink / raw)
  To: Jarkko Sakkinen, James Bottomley, Mimi Zohar, Jonathan Corbet,
	Herbert Xu, David S. Miller, Peter Huewe, Jason Gunthorpe,
	David Howells, James Morris, Serge E. Hallyn, Jerry Snitselaar,
	linux-integrity, keyrings, linux-doc, linux-kernel, linux-crypto,
	linux-security-module

Hi Jarkko,

On 10/15/21 11:19 PM, Jarkko Sakkinen wrote:
> On Thu, 2021-10-14 at 17:46 +0800, Tianjia Zhang wrote:
>> Hi Jarkko,
>>
>> On 10/12/21 11:21 PM, Jarkko Sakkinen wrote:
>>> On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
>>>> According to https://tools.ietf.org/id/draft-oscca-cfrg-sm3-01.html,
>>>> SM3 always produces a 256-bit hash value and there are no plans for
>>>> other length development, so there is no ambiguity in the name of sm3.
>>>>
>>>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
>>>
>>> This is not enough to make any changes because the commit message
>>> does not describe what goes wrong if we keep it as it was.
>>>
>>> /Jarkko
>>>
>>
>> This did not cause an error, just to use a more standard algorithm name.
>> If it is possible to use the SM3 name instead of SM3_256 if it can be
>> specified from the source, it is of course better. I have contacted the
>> trustedcomputinggroup and have not yet received a reply.
>>
>> Best regards,
>> Tianjia
> 
> Why don't you then create a patch set that fully removes SM3_256, if it
> is incorrect?
> 
> This looks a bit half-baked patch set.
> 
> /Jarkko
> 

This series of patch is a complete replacement. Patch 1 is a replacement 
of the crypto subsystem, and patch 2 is a replacement of the tpm driver.

Best regards,
Tianjia

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

* Re: [PATCH 2/2] tpm: use SM3 instead of SM3_256
  2021-10-18  2:37         ` Tianjia Zhang
@ 2021-10-18 12:48           ` Jarkko Sakkinen
  0 siblings, 0 replies; 16+ messages in thread
From: Jarkko Sakkinen @ 2021-10-18 12:48 UTC (permalink / raw)
  To: Tianjia Zhang, James Bottomley, Mimi Zohar, Jonathan Corbet,
	Herbert Xu, David S. Miller, Peter Huewe, Jason Gunthorpe,
	David Howells, James Morris, Serge E. Hallyn, Jerry Snitselaar,
	linux-integrity, keyrings, linux-doc, linux-kernel, linux-crypto,
	linux-security-module

On Mon, 2021-10-18 at 10:37 +0800, Tianjia Zhang wrote:
> Hi Jarkko,
> 
> On 10/15/21 11:19 PM, Jarkko Sakkinen wrote:
> > On Thu, 2021-10-14 at 17:46 +0800, Tianjia Zhang wrote:
> > > Hi Jarkko,
> > > 
> > > On 10/12/21 11:21 PM, Jarkko Sakkinen wrote:
> > > > On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
> > > > > According to https://tools.ietf.org/id/draft-oscca-cfrg-sm3-01.html,
> > > > > SM3 always produces a 256-bit hash value and there are no plans for
> > > > > other length development, so there is no ambiguity in the name of sm3.
> > > > > 
> > > > > Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> > > > 
> > > > This is not enough to make any changes because the commit message
> > > > does not describe what goes wrong if we keep it as it was.
> > > > 
> > > > /Jarkko
> > > > 
> > > 
> > > This did not cause an error, just to use a more standard algorithm name.
> > > If it is possible to use the SM3 name instead of SM3_256 if it can be
> > > specified from the source, it is of course better. I have contacted the
> > > trustedcomputinggroup and have not yet received a reply.
> > > 
> > > Best regards,
> > > Tianjia
> > 
> > Why don't you then create a patch set that fully removes SM3_256, if it
> > is incorrect?
> > 
> > This looks a bit half-baked patch set.
> > 
> > /Jarkko
> > 
> 
> This series of patch is a complete replacement. Patch 1 is a replacement 
> of the crypto subsystem, and patch 2 is a replacement of the tpm driver.
> 
> Best regards,
> Tianjia

In which patch that symbol is removed?

/Jarkko

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

* Re: [PATCH 1/2] crypto: use SM3 instead of SM3_256
  2021-10-09 13:08 ` [PATCH 1/2] crypto: " Tianjia Zhang
@ 2021-10-18 13:05   ` James Bottomley
  2021-10-18 13:27     ` Jarkko Sakkinen
  2021-10-19  9:35     ` Tianjia Zhang
  0 siblings, 2 replies; 16+ messages in thread
From: James Bottomley @ 2021-10-18 13:05 UTC (permalink / raw)
  To: Tianjia Zhang, Jarkko Sakkinen, Mimi Zohar, Jonathan Corbet,
	Herbert Xu, David S. Miller, Peter Huewe, Jason Gunthorpe,
	David Howells, James Morris, Serge E. Hallyn, Jerry Snitselaar,
	linux-integrity, keyrings, linux-doc, linux-kernel, linux-crypto,
	linux-security-module

On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
[...]
> diff --git a/include/uapi/linux/hash_info.h
> b/include/uapi/linux/hash_info.h
> index 74a8609fcb4d..1355525dd4aa 100644
> --- a/include/uapi/linux/hash_info.h
> +++ b/include/uapi/linux/hash_info.h
> @@ -32,7 +32,7 @@ enum hash_algo {
>  	HASH_ALGO_TGR_128,
>  	HASH_ALGO_TGR_160,
>  	HASH_ALGO_TGR_192,
> -	HASH_ALGO_SM3_256,
> +	HASH_ALGO_SM3,
>  	HASH_ALGO_STREEBOG_256,
>  	HASH_ALGO_STREEBOG_512,
>  	HASH_ALGO__LAST

This is another one you can't do: all headers in UAPI are exports to
userspace and the definitions constitute an ABI.  If you simply do a
rename, every userspace program that uses the current definition will
immediately break on compile.  You could add HASH_ALGO_SM3, but you
can't remove HASH_ALGO_SM3_256

James



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

* Re: [PATCH 1/2] crypto: use SM3 instead of SM3_256
  2021-10-18 13:05   ` James Bottomley
@ 2021-10-18 13:27     ` Jarkko Sakkinen
  2021-10-18 13:32       ` James Bottomley
  2021-10-19  9:35     ` Tianjia Zhang
  1 sibling, 1 reply; 16+ messages in thread
From: Jarkko Sakkinen @ 2021-10-18 13:27 UTC (permalink / raw)
  To: jejb, Tianjia Zhang, Mimi Zohar, Jonathan Corbet, Herbert Xu,
	David S. Miller, Peter Huewe, Jason Gunthorpe, David Howells,
	James Morris, Serge E. Hallyn, Jerry Snitselaar, linux-integrity,
	keyrings, linux-doc, linux-kernel, linux-crypto,
	linux-security-module

On Mon, 2021-10-18 at 09:05 -0400, James Bottomley wrote:
> On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
> [...]
> > diff --git a/include/uapi/linux/hash_info.h
> > b/include/uapi/linux/hash_info.h
> > index 74a8609fcb4d..1355525dd4aa 100644
> > --- a/include/uapi/linux/hash_info.h
> > +++ b/include/uapi/linux/hash_info.h
> > @@ -32,7 +32,7 @@ enum hash_algo {
> >         HASH_ALGO_TGR_128,
> >         HASH_ALGO_TGR_160,
> >         HASH_ALGO_TGR_192,
> > -       HASH_ALGO_SM3_256,
> > +       HASH_ALGO_SM3,
> >         HASH_ALGO_STREEBOG_256,
> >         HASH_ALGO_STREEBOG_512,
> >         HASH_ALGO__LAST
> 
> This is another one you can't do: all headers in UAPI are exports to
> userspace and the definitions constitute an ABI.  If you simply do a
> rename, every userspace program that uses the current definition will
> immediately break on compile.  You could add HASH_ALGO_SM3, but you
> can't remove HASH_ALGO_SM3_256
> 
> James

So: shouldn't then also the old symbol continue to work also
semantically?

/Jarkko

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

* Re: [PATCH 1/2] crypto: use SM3 instead of SM3_256
  2021-10-18 13:27     ` Jarkko Sakkinen
@ 2021-10-18 13:32       ` James Bottomley
  2021-10-18 13:41         ` Jarkko Sakkinen
  0 siblings, 1 reply; 16+ messages in thread
From: James Bottomley @ 2021-10-18 13:32 UTC (permalink / raw)
  To: Jarkko Sakkinen, Tianjia Zhang, Mimi Zohar, Jonathan Corbet,
	Herbert Xu, David S. Miller, Peter Huewe, Jason Gunthorpe,
	David Howells, James Morris, Serge E. Hallyn, Jerry Snitselaar,
	linux-integrity, keyrings, linux-doc, linux-kernel, linux-crypto,
	linux-security-module

On Mon, 2021-10-18 at 16:27 +0300, Jarkko Sakkinen wrote:
> On Mon, 2021-10-18 at 09:05 -0400, James Bottomley wrote:
> > On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
> > [...]
> > > diff --git a/include/uapi/linux/hash_info.h
> > > b/include/uapi/linux/hash_info.h
> > > index 74a8609fcb4d..1355525dd4aa 100644
> > > --- a/include/uapi/linux/hash_info.h
> > > +++ b/include/uapi/linux/hash_info.h
> > > @@ -32,7 +32,7 @@ enum hash_algo {
> > >         HASH_ALGO_TGR_128,
> > >         HASH_ALGO_TGR_160,
> > >         HASH_ALGO_TGR_192,
> > > -       HASH_ALGO_SM3_256,
> > > +       HASH_ALGO_SM3,
> > >         HASH_ALGO_STREEBOG_256,
> > >         HASH_ALGO_STREEBOG_512,
> > >         HASH_ALGO__LAST
> > 
> > This is another one you can't do: all headers in UAPI are exports
> > to userspace and the definitions constitute an ABI.  If you simply
> > do a rename, every userspace program that uses the current
> > definition will immediately break on compile.  You could add
> > HASH_ALGO_SM3, but you can't remove HASH_ALGO_SM3_256
> > 
> > James
> 
> So: shouldn't then also the old symbol continue to work also
> semantically?

Yes, that's the point: you can add a new definition ... in this case an
alias for the old one, but you can't remove a definition that's been
previously exported.

James



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

* Re: [PATCH 1/2] crypto: use SM3 instead of SM3_256
  2021-10-18 13:32       ` James Bottomley
@ 2021-10-18 13:41         ` Jarkko Sakkinen
  2021-10-19  9:39           ` Tianjia Zhang
  0 siblings, 1 reply; 16+ messages in thread
From: Jarkko Sakkinen @ 2021-10-18 13:41 UTC (permalink / raw)
  To: jejb, Tianjia Zhang, Mimi Zohar, Jonathan Corbet, Herbert Xu,
	David S. Miller, Peter Huewe, Jason Gunthorpe, David Howells,
	James Morris, Serge E. Hallyn, Jerry Snitselaar, linux-integrity,
	keyrings, linux-doc, linux-kernel, linux-crypto,
	linux-security-module

On Mon, 2021-10-18 at 09:32 -0400, James Bottomley wrote:
> On Mon, 2021-10-18 at 16:27 +0300, Jarkko Sakkinen wrote:
> > On Mon, 2021-10-18 at 09:05 -0400, James Bottomley wrote:
> > > On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
> > > [...]
> > > > diff --git a/include/uapi/linux/hash_info.h
> > > > b/include/uapi/linux/hash_info.h
> > > > index 74a8609fcb4d..1355525dd4aa 100644
> > > > --- a/include/uapi/linux/hash_info.h
> > > > +++ b/include/uapi/linux/hash_info.h
> > > > @@ -32,7 +32,7 @@ enum hash_algo {
> > > >         HASH_ALGO_TGR_128,
> > > >         HASH_ALGO_TGR_160,
> > > >         HASH_ALGO_TGR_192,
> > > > -       HASH_ALGO_SM3_256,
> > > > +       HASH_ALGO_SM3,
> > > >         HASH_ALGO_STREEBOG_256,
> > > >         HASH_ALGO_STREEBOG_512,
> > > >         HASH_ALGO__LAST
> > > 
> > > This is another one you can't do: all headers in UAPI are exports
> > > to userspace and the definitions constitute an ABI.  If you simply
> > > do a rename, every userspace program that uses the current
> > > definition will immediately break on compile.  You could add
> > > HASH_ALGO_SM3, but you can't remove HASH_ALGO_SM3_256
> > > 
> > > James
> > 
> > So: shouldn't then also the old symbol continue to work also
> > semantically?
> 
> Yes, that's the point: you can add a new definition ... in this case an
> alias for the old one, but you can't remove a definition that's been
> previously exported.

Thanks, this of course obvious :-) I forgot temporarily that crypto
has uapi interface. Tianjia, this patch set break production systems,
so no chance we would ever merge it in this form.

Why not just do this:

...
HASH_ALGO_SM3_256,
HASH_ALOG_SM3 = HASH_ALOG_SM_256,
...

There is not good reason to mod the implementation because both symbols
are kept.

/Jarkko

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

* Re: [PATCH 1/2] crypto: use SM3 instead of SM3_256
  2021-10-18 13:05   ` James Bottomley
  2021-10-18 13:27     ` Jarkko Sakkinen
@ 2021-10-19  9:35     ` Tianjia Zhang
  1 sibling, 0 replies; 16+ messages in thread
From: Tianjia Zhang @ 2021-10-19  9:35 UTC (permalink / raw)
  To: jejb, Jarkko Sakkinen, Mimi Zohar, Jonathan Corbet, Herbert Xu,
	David S. Miller, Peter Huewe, Jason Gunthorpe, David Howells,
	James Morris, Serge E. Hallyn, Jerry Snitselaar, linux-integrity,
	keyrings, linux-doc, linux-kernel, linux-crypto,
	linux-security-module

Hi James,

On 10/18/21 9:05 PM, James Bottomley wrote:
> On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
> [...]
>> diff --git a/include/uapi/linux/hash_info.h
>> b/include/uapi/linux/hash_info.h
>> index 74a8609fcb4d..1355525dd4aa 100644
>> --- a/include/uapi/linux/hash_info.h
>> +++ b/include/uapi/linux/hash_info.h
>> @@ -32,7 +32,7 @@ enum hash_algo {
>>   	HASH_ALGO_TGR_128,
>>   	HASH_ALGO_TGR_160,
>>   	HASH_ALGO_TGR_192,
>> -	HASH_ALGO_SM3_256,
>> +	HASH_ALGO_SM3,
>>   	HASH_ALGO_STREEBOG_256,
>>   	HASH_ALGO_STREEBOG_512,
>>   	HASH_ALGO__LAST
> 
> This is another one you can't do: all headers in UAPI are exports to
> userspace and the definitions constitute an ABI.  If you simply do a
> rename, every userspace program that uses the current definition will
> immediately break on compile.  You could add HASH_ALGO_SM3, but you
> can't remove HASH_ALGO_SM3_256
> 
> James
> 

Correct, Thanks for pointing it out, redefining a macro is indeed a more 
appropriate method.

Best regards,
Tianjia

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

* Re: [PATCH 1/2] crypto: use SM3 instead of SM3_256
  2021-10-18 13:41         ` Jarkko Sakkinen
@ 2021-10-19  9:39           ` Tianjia Zhang
  0 siblings, 0 replies; 16+ messages in thread
From: Tianjia Zhang @ 2021-10-19  9:39 UTC (permalink / raw)
  To: Jarkko Sakkinen, jejb, Mimi Zohar, Jonathan Corbet, Herbert Xu,
	David S. Miller, Peter Huewe, Jason Gunthorpe, David Howells,
	James Morris, Serge E. Hallyn, Jerry Snitselaar, linux-integrity,
	keyrings, linux-doc, linux-kernel, linux-crypto,
	linux-security-module

Hi Jarkko,

On 10/18/21 9:41 PM, Jarkko Sakkinen wrote:
> On Mon, 2021-10-18 at 09:32 -0400, James Bottomley wrote:
>> On Mon, 2021-10-18 at 16:27 +0300, Jarkko Sakkinen wrote:
>>> On Mon, 2021-10-18 at 09:05 -0400, James Bottomley wrote:
>>>> On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
>>>> [...]
>>>>> diff --git a/include/uapi/linux/hash_info.h
>>>>> b/include/uapi/linux/hash_info.h
>>>>> index 74a8609fcb4d..1355525dd4aa 100644
>>>>> --- a/include/uapi/linux/hash_info.h
>>>>> +++ b/include/uapi/linux/hash_info.h
>>>>> @@ -32,7 +32,7 @@ enum hash_algo {
>>>>>          HASH_ALGO_TGR_128,
>>>>>          HASH_ALGO_TGR_160,
>>>>>          HASH_ALGO_TGR_192,
>>>>> -       HASH_ALGO_SM3_256,
>>>>> +       HASH_ALGO_SM3,
>>>>>          HASH_ALGO_STREEBOG_256,
>>>>>          HASH_ALGO_STREEBOG_512,
>>>>>          HASH_ALGO__LAST
>>>>
>>>> This is another one you can't do: all headers in UAPI are exports
>>>> to userspace and the definitions constitute an ABI.  If you simply
>>>> do a rename, every userspace program that uses the current
>>>> definition will immediately break on compile.  You could add
>>>> HASH_ALGO_SM3, but you can't remove HASH_ALGO_SM3_256
>>>>
>>>> James
>>>
>>> So: shouldn't then also the old symbol continue to work also
>>> semantically?
>>
>> Yes, that's the point: you can add a new definition ... in this case an
>> alias for the old one, but you can't remove a definition that's been
>> previously exported.
> 
> Thanks, this of course obvious :-) I forgot temporarily that crypto
> has uapi interface. Tianjia, this patch set break production systems,
> so no chance we would ever merge it in this form.
> 
> Why not just do this:
> 
> ...
> HASH_ALGO_SM3_256,
> HASH_ALOG_SM3 = HASH_ALOG_SM_256,
> ...
> 
> There is not good reason to mod the implementation because both symbols
> are kept.
> 
> /Jarkko
> 

Very good suggestion, I will do this in the next version patch. Maybe 
this is more appropriate:

   HASH_ALGO_SM3,
   HASH_ALGO_SM3_256 = HASH_ALGO_SM3,

Best regards,
Tianjia

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

end of thread, other threads:[~2021-10-19  9:40 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-09 13:08 [PATCH 0/2] tpm: use SM3 instead of SM3_256 Tianjia Zhang
2021-10-09 13:08 ` [PATCH 1/2] crypto: " Tianjia Zhang
2021-10-18 13:05   ` James Bottomley
2021-10-18 13:27     ` Jarkko Sakkinen
2021-10-18 13:32       ` James Bottomley
2021-10-18 13:41         ` Jarkko Sakkinen
2021-10-19  9:39           ` Tianjia Zhang
2021-10-19  9:35     ` Tianjia Zhang
2021-10-09 13:08 ` [PATCH 2/2] tpm: " Tianjia Zhang
2021-10-12 15:21   ` Jarkko Sakkinen
2021-10-14  9:46     ` Tianjia Zhang
2021-10-15 15:19       ` Jarkko Sakkinen
2021-10-18  2:37         ` Tianjia Zhang
2021-10-18 12:48           ` Jarkko Sakkinen
2021-10-09 13:29 ` [PATCH 0/2] " James Bottomley
2021-10-11  7:02   ` Tianjia Zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).