On 1/18/22 19:12, Ilias Apalodimas wrote:
Hi Heinrich,
On Tue, 18 Jan 2022 at 18:22, Heinrich Schuchardt <[email protected]> wrote:
On 1/18/22 15:03, Ilias Apalodimas wrote:
Hi Heinrich,
- 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?
We don't, but the current code allows it as well. Should we enforce this
from U-Boot though? The spec doesn't forbid it as far as I remember
Collisions for SHA1 have been first created successfully in 2017.
It is feasible to create two different EFI binaries with the same SHA1.
One will be reviewed and signed. After copying the signature to the
other one it will happily boot on U-Boot. Ouch. This is exactly what
signatures are meant to avoid.
We must not accept SHA1 for signatures.
Right, but is this the right place to do it? This is function to
verify signatures. Isn't it better to keep this as is and then
explicitly deny adding sha1 hashed keys into db?
You must assume that PK, KEK, db are preexisting or are seeded from a
file. So the check should be done when loading an image.
To be more precise:
An image should not be validated based on an SHA1 signature or SHA1 hash
but it must be possible to reject an image based on an SHA1 hash.
Best regards
Heinrich
Cheers
/Ilias
Best regards
Heinrich
Regards
/Ilias
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
