Re: [PATCH] lib/crypto: Enable more algorithms in cert verification

Hi Ilias,

On Tue, Jan 18, 2022 at 01:12:37PM +0200, Ilias Apalodimas wrote:

Right now the code explicitly limits us to sha1,256 hashes with RSA2048 encryption. But the limitation is artificial since U-Boot supports a wider range of algorithms.

The internal image_get_[checksum|crypto]_algo() functions expect an argument in the format of <checksum>,<crypto>. So let's remove the size checking and create the needed string on the fly in order to support more hash/signing combinations.

Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org

lib/crypto/public_key.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/lib/crypto/public_key.c b/lib/crypto/public_key.c index df6033cdb499..b783c63f5a51 100644 --- a/lib/crypto/public_key.c +++ b/lib/crypto/public_key.c @@ -97,6 +97,7 @@ int public_key_verify_signature(const struct public_key *pkey, const struct public_key_signature *sig) { struct image_sign_info info;

• char algo[256]; int ret;

  pr_devel("==>%s()\n", __func__);

@@ -108,29 +109,27 @@ int public_key_verify_signature(const struct public_key *pkey, return -EINVAL;

memset(&info, '\0', sizeof(info));

• memset(algo, 0, sizeof(algo)); 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 this patch is applied, the TODO comment above will make no sense :)

• 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);
  

• if (strcmp(sig->pkey_algo, "rsa")) {

• pr_err("Encryption is not RSA: %s\n", sig->pkey_algo);
  

  return -ENOPKG; }
• ret = snprintf(algo, sizeof(algo), "%s,%s%d", sig->hash_algo,

•        sig->pkey_algo, sig->s_size * 8);
  

I'm not sure that this naming rule, in particular the latter part, will always hold in the future while all the existing algo's observe it. (Maybe we need some note somewhere?)

-Takahiro Akashi

• if (ret >= sizeof(algo))

• return -EINVAL;
  

• info.checksum = image_get_checksum_algo((const char *)algo);
• info.name = (const char *)algo; info.crypto = image_get_crypto_algo(info.name);

• if (IS_ERR(info.checksum) || IS_ERR(info.crypto))

• if (!info.checksum || !info.crypto) return -ENOPKG;

  info.key = pkey->key;

-- 2.30.2