| Message ID | 20240513230718.447895-1-luhuaxin1@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | Move SM2 digest calculation to signature verification | expand |
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 --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);
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(-)