diff mbox series

Move SM2 digest calculation to signature verification

Message ID 20240513230718.447895-1-luhuaxin1@huawei.com
State New
Headers show
Series Move SM2 digest calculation to signature verification | expand

Commit Message

Huaxin Lu May 13, 2024, 11:07 p.m. UTC
In the commit of e5221fa6a355 ("KEYS: asymmetric: Move sm2 code into
x509_public_key"), the SM2 digest hashing is moved to the process of
certificate loading. It cause the SM2 certificate chain validation
failure. For example, when importing a SM2 IMA certificate (x509_ima.der)
verified by the trusted kering. The import fails due to the wrong Z value
calculating. Because he Z value should be calculated from the public key
of the signing certificate, not from the public key of the certificate
itself (reference: datatracker.ietf.org/doc/html/draft-shen-sm2-ecdsa-02).

This commit partially revert the previous commit. Restore SM2 digest value
calculating into the signature verification process, and use the right
information to calculate Z value and SM2 digest.

Fixes: e5221fa6a355 ("KEYS: asymmetric: Move sm2 code into x509_public_key")
Signed-off-by: Huaxin Lu <luhuaxin1@huawei.com>
---
 crypto/asymmetric_keys/public_key.c      | 57 ++++++++++++++++++++++++
 crypto/asymmetric_keys/x509_public_key.c | 20 +++------
 include/crypto/public_key.h              |  2 +
 3 files changed, 64 insertions(+), 15 deletions(-)

Comments

Herbert Xu May 17, 2024, 10:46 a.m. UTC | #1
On Tue, May 14, 2024 at 07:07:18AM +0800, Huaxin Lu wrote:
> In the commit of e5221fa6a355 ("KEYS: asymmetric: Move sm2 code into
> x509_public_key"), the SM2 digest hashing is moved to the process of
> certificate loading. It cause the SM2 certificate chain validation
> failure. For example, when importing a SM2 IMA certificate (x509_ima.der)
> verified by the trusted kering. The import fails due to the wrong Z value
> calculating. Because he Z value should be calculated from the public key
> of the signing certificate, not from the public key of the certificate
> itself (reference: datatracker.ietf.org/doc/html/draft-shen-sm2-ecdsa-02).
> 
> This commit partially revert the previous commit. Restore SM2 digest value
> calculating into the signature verification process, and use the right
> information to calculate Z value and SM2 digest.
> 
> Fixes: e5221fa6a355 ("KEYS: asymmetric: Move sm2 code into x509_public_key")
> Signed-off-by: Huaxin Lu <luhuaxin1@huawei.com>
> ---
>  crypto/asymmetric_keys/public_key.c      | 57 ++++++++++++++++++++++++
>  crypto/asymmetric_keys/x509_public_key.c | 20 +++------
>  include/crypto/public_key.h              |  2 +
>  3 files changed, 64 insertions(+), 15 deletions(-)

Sorry about this.

> diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
> index 6a4f00be2..54738af7d 100644
> --- a/crypto/asymmetric_keys/x509_public_key.c
> +++ b/crypto/asymmetric_keys/x509_public_key.c
> @@ -32,6 +32,9 @@ int x509_get_sig_params(struct x509_certificate *cert)
>  
>  	pr_devel("==>%s()\n", __func__);
>  
> +	sig->data = cert->tbs;
> +	sig->data_size = cert->tbs_size;
> +
>  	sig->s = kmemdup(cert->raw_sig, cert->raw_sig_size, GFP_KERNEL);
>  	if (!sig->s)
>  		return -ENOMEM;
> @@ -64,21 +67,8 @@ int x509_get_sig_params(struct x509_certificate *cert)
>  
>  	desc->tfm = tfm;
>  
> -	if (strcmp(cert->pub->pkey_algo, "sm2") == 0) {
> -		ret = strcmp(sig->hash_algo, "sm3") != 0 ? -EINVAL :
> -		      crypto_shash_init(desc) ?:
> -		      sm2_compute_z_digest(desc, cert->pub->key,
> -					   cert->pub->keylen, sig->digest) ?:
> -		      crypto_shash_init(desc) ?:
> -		      crypto_shash_update(desc, sig->digest,
> -					  sig->digest_size) ?:
> -		      crypto_shash_finup(desc, cert->tbs, cert->tbs_size,
> -					 sig->digest);
> -	} else {
> -		ret = crypto_shash_digest(desc, cert->tbs, cert->tbs_size,
> -					  sig->digest);
> -	}
> -
> +	ret = crypto_shash_digest(desc, cert->tbs, cert->tbs_size,
> +				  sig->digest);

This (and the original code) breaks the blacklisting calculations
since those were dependent on the calculated hash.

There's also the issue of PKCS7 digests which probably should also
be modified for SM2.

I think we should probably just remove SM2 unless someone can
rearchitect this properly to support these digests.

Cheers,
diff mbox series

Patch

diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index e314fd57e..647a03e00 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -9,8 +9,11 @@ 
 
 #define pr_fmt(fmt) "PKEY: "fmt
 #include <crypto/akcipher.h>
+#include <crypto/hash.h>
 #include <crypto/public_key.h>
 #include <crypto/sig.h>
+#include <crypto/sm2.h>
+#include <crypto/sm3.h>
 #include <keys/asymmetric-subtype.h>
 #include <linux/asn1.h>
 #include <linux/err.h>
@@ -376,6 +379,54 @@  static int software_key_eds_op(struct kernel_pkey_params *params,
 	return ret;
 }
 
+#if IS_REACHABLE(CONFIG_CRYPTO_SM2)
+static int cert_sig_digest_update(const struct public_key_signature *sig,
+				  void *pkey, size_t pkey_len)
+{
+	struct crypto_shash *tfm;
+	struct shash_desc *desc;
+	size_t desc_size;
+	unsigned char dgst[SM3_DIGEST_SIZE];
+	int ret;
+
+	BUG_ON(!sig->data);
+
+	/* SM2 signatures always use the SM3 hash algorithm */
+	if (!sig->hash_algo || strcmp(sig->hash_algo, "sm3") != 0)
+		return -EINVAL;
+
+	tfm = crypto_alloc_shash(sig->hash_algo, 0, 0);
+	if (IS_ERR(tfm))
+		return PTR_ERR(tfm);
+
+	desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
+	desc = kzalloc(desc_size, GFP_KERNEL);
+	if (!desc) {
+		crypto_free_shash(tfm);
+		return -ENOMEM;
+	}
+
+	desc->tfm = tfm;
+
+	ret = crypto_shash_init(desc) ?:
+	      sm2_compute_z_digest(desc, pkey, pkey_len, dgst) ?:
+	      crypto_shash_init(desc) ?:
+	      crypto_shash_update(desc, dgst, SM3_DIGEST_SIZE) ?:
+	      crypto_shash_finup(desc, sig->data, sig->data_size, sig->digest);
+
+	kfree(desc);
+	crypto_free_shash(tfm);
+	return ret;
+}
+#else
+static inline int cert_sig_digest_update(
+	const struct public_key_signature *sig,
+	void *pkey, size_t pkey_len)
+{
+	return -ENOTSUPP;
+}
+#endif /* ! IS_REACHABLE(CONFIG_CRYPTO_SM2) */
+
 /*
  * Verify a signature using a public key.
  */
@@ -439,6 +490,12 @@  int public_key_verify_signature(const struct public_key *pkey,
 	if (ret)
 		goto error_free_key;
 
+	if (strcmp(pkey->pkey_algo, "sm2") == 0 && sig->data_size) {
+		ret = cert_sig_digest_update(sig, key, pkey->keylen);
+		if (ret)
+			goto error_free_key;
+	}
+
 	ret = crypto_sig_verify(tfm, sig->s, sig->s_size,
 				sig->digest, sig->digest_size);
 
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 6a4f00be2..54738af7d 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -32,6 +32,9 @@  int x509_get_sig_params(struct x509_certificate *cert)
 
 	pr_devel("==>%s()\n", __func__);
 
+	sig->data = cert->tbs;
+	sig->data_size = cert->tbs_size;
+
 	sig->s = kmemdup(cert->raw_sig, cert->raw_sig_size, GFP_KERNEL);
 	if (!sig->s)
 		return -ENOMEM;
@@ -64,21 +67,8 @@  int x509_get_sig_params(struct x509_certificate *cert)
 
 	desc->tfm = tfm;
 
-	if (strcmp(cert->pub->pkey_algo, "sm2") == 0) {
-		ret = strcmp(sig->hash_algo, "sm3") != 0 ? -EINVAL :
-		      crypto_shash_init(desc) ?:
-		      sm2_compute_z_digest(desc, cert->pub->key,
-					   cert->pub->keylen, sig->digest) ?:
-		      crypto_shash_init(desc) ?:
-		      crypto_shash_update(desc, sig->digest,
-					  sig->digest_size) ?:
-		      crypto_shash_finup(desc, cert->tbs, cert->tbs_size,
-					 sig->digest);
-	} else {
-		ret = crypto_shash_digest(desc, cert->tbs, cert->tbs_size,
-					  sig->digest);
-	}
-
+	ret = crypto_shash_digest(desc, cert->tbs, cert->tbs_size,
+				  sig->digest);
 	if (ret < 0)
 		goto error_2;
 
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index b7f308977..fce68803b 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -49,6 +49,8 @@  struct public_key_signature {
 	const char *pkey_algo;
 	const char *hash_algo;
 	const char *encoding;
+	const void *data;
+	unsigned int data_size;
 };
 
 extern void public_key_signature_free(struct public_key_signature *sig);