From mboxrd@z Thu Jan  1 00:00:00 1970
From: Heinrich Schuchardt <xypron.glpk@gmx.de>
Date: Sun, 19 Jul 2020 10:20:58 +0200
Subject: [PATCH v4 1/7] lib: crypto: add public_key_verify_signature()
In-Reply-To: <20200717071630.7363-2-takahiro.akashi@linaro.org>
References: <20200717071630.7363-1-takahiro.akashi@linaro.org>
 <20200717071630.7363-2-takahiro.akashi@linaro.org>
Message-ID: <b1b00d1e-45dd-1414-49a4-785e3615d307@gmx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 7/17/20 9:16 AM, AKASHI Takahiro wrote:
> This function will be called from x509_check_for_self_signed() and
> pkcs7_verify_one(), which will be imported from linux in a later patch.
>
> While it does exist in linux code and has a similar functionality of
> rsa_verify(), it calls further linux-specific interfaces inside.
> That could lead to more files being imported from linux.
>
> So simply re-implement it here instead of re-using the code.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  include/crypto/public_key.h |  2 +-
>  lib/crypto/public_key.c     | 70 ++++++++++++++++++++++++++++++++++++-
>  2 files changed, 70 insertions(+), 2 deletions(-)
>
> diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> index 436a1ee1ee64..3ba90fcc3483 100644
> --- a/include/crypto/public_key.h
> +++ b/include/crypto/public_key.h
> @@ -82,9 +82,9 @@ extern int decrypt_blob(struct kernel_pkey_params *, const void *, void *);
>  extern int create_signature(struct kernel_pkey_params *, const void *, void *);
>  extern int verify_signature(const struct key *,
>  			    const struct public_key_signature *);
> +#endif /* __UBOOT__ */
>
>  int public_key_verify_signature(const struct public_key *pkey,
>  				const struct public_key_signature *sig);
> -#endif /* !__UBOOT__ */
>
>  #endif /* _LINUX_PUBLIC_KEY_H */
> diff --git a/lib/crypto/public_key.c b/lib/crypto/public_key.c
> index e12ebbb3d0c5..71a0e0356beb 100644
> --- a/lib/crypto/public_key.c
> +++ b/lib/crypto/public_key.c
> @@ -25,7 +25,10 @@
>  #include <keys/asymmetric-subtype.h>
>  #endif
>  #include <crypto/public_key.h>
> -#ifndef __UBOOT__
> +#ifdef __UBOOT__
> +#include <image.h>
> +#include <u-boot/rsa.h>
> +#else
>  #include <crypto/akcipher.h>
>  #endif

Shouldn't we describe in the header of the file that the code is taken
(at least partially) from Linux' crypto/asymmetric_keys/public_key.c?

Please, also state the Linux version number.

>
> @@ -80,6 +83,71 @@ void public_key_signature_free(struct public_key_signature *sig)
>  }
>  EXPORT_SYMBOL_GPL(public_key_signature_free);
>
> +/**
> + * public_key_verify_signature - Verify a signature using a public key.
> + *
> + * @pkey:	Public key
> + * @sig:	Signature
> + *
> + * Verify a signature, @sig, using a RSA public key, @pkey.
> + *
> + * Return:	0 - verified, non-zero error code - otherwise
> + */
> +int public_key_verify_signature(const struct public_key *pkey,
> +				const struct public_key_signature *sig)
> +{
> +	struct image_sign_info info;
> +	struct image_region region;
> +	int ret;
> +
> +	pr_devel("==>%s()\n", __func__);
> +
> +	if (!pkey || !sig)
> +		return -EINVAL;
> +
> +	if (pkey->key_is_private)
> +		return -EINVAL;
> +
> +	memset(&info, '\0', sizeof(info));
> +	info.padding = image_get_padding_algo("pkcs-1.5");
> +	/*
> +	 * Note: image_get_[checksum|crypto]_algo takes a string
> +	 * argument like "<checksum>,<crypto>"
> +	 * TODO: support other hash algorithms
> +	 */
> +	if (strcmp(sig->pkey_algo, "rsa") || (sig->s_size * 8) != 2048) {
> +		pr_warn("Encryption is not RSA2048: %s%d\n",
> +			sig->pkey_algo, sig->s_size * 8);
> +		return -ENOPKG;
> +	}
> +	if (!strcmp(sig->hash_algo, "sha1")) {
> +		info.checksum = image_get_checksum_algo("sha1,rsa2048");
> +		info.name = "sha1,rsa2048";
> +	} else if (!strcmp(sig->hash_algo, "sha256")) {
> +		info.checksum = image_get_checksum_algo("sha256,rsa2048");
> +		info.name = "sha256,rsa2048";
> +	} else {
> +		pr_warn("unknown msg digest algo: %s\n", sig->hash_algo);
> +		return -ENOPKG;
> +	}
> +	info.crypto = image_get_crypto_algo(info.name);
> +	if (unlikely(IS_ERR(info.checksum) || IS_ERR(info.crypto)))
> +		return -ENOPKG;

scripts/checkpatch.pl complains:

WARNING: nested (un)?likely() calls, IS_ERR already uses unlikely()
internally
#104: FILE: lib/crypto/public_key.c:134:
+       if (unlikely(IS_ERR(info.checksum) || IS_ERR(info.crypto)))

I cannot find this unlikely in the Linux v5.8-rc4
crypto/asymmetric_keys/public_key.c file. From where did you copy this code?

Best regards

Heinrich

> +
> +	info.key = pkey->key;
> +	info.keylen = pkey->keylen;
> +
> +	region.data = sig->digest;
> +	region.size = sig->digest_size;
> +
> +	if (rsa_verify_with_pkey(&info, sig->digest, sig->s, sig->s_size))
> +		ret = -EKEYREJECTED;
> +	else
> +		ret = 0;
> +
> +	pr_devel("<==%s() = %d\n", __func__, ret);
> +	return ret;
> +}
>  #else
>  /*
>   * Destroy a public key algorithm key.
>