On 1/18/22 13:50, Ilias Apalodimas wrote:
Akashi-san,
On Tue, Jan 18, 2022 at 09:38:22PM +0900, AKASHI Takahiro wrote:
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.apalodi...@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 :)
We are still only handle SHA, but there's a printable error now, so i'll
get rid of the comment.
- 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);
How do we ensure that the unsafe SHA1 algorithm is not used?
Best regards
Heinrich
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?)
The if a few lines below will shield us and return -EINVAL. How about
adding an error message there?
Cheers
/Ilias
-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
