| Message ID | 20210111154137.621732-4-mr.nuke.me@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Patrick Delaunay |
| Headers | show |
| Series | Enable ECDSA FIT verification for stm32mp | expand |
On Mon, 11 Jan 2021 at 08:41, Alexandru Gagniuc <mr.nuke.me@gmail.com> wrote: > > Implement the crypto_algo .verify() function for ecdsa256. Because > it backends on UCLASS_ECDSA, this change is focused on parsing the > keys from devicetree and passing this information to the specific > UCLASS driver. > > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com> > --- > lib/ecdsa/ecdsa-verify.c | 117 ++++++++++++++++++++++++++++++++++++++- > 1 file changed, 116 insertions(+), 1 deletion(-) Reviewed-by: Simon Glass <sjg@chromium.org>
Hi, On 1/11/21 4:41 PM, Alexandru Gagniuc wrote: > Implement the crypto_algo .verify() function for ecdsa256. Because > it backends on UCLASS_ECDSA, this change is focused on parsing the > keys from devicetree and passing this information to the specific > UCLASS driver. > > Signed-off-by: Alexandru Gagniuc<mr.nuke.me@gmail.com> > --- > lib/ecdsa/ecdsa-verify.c | 117 ++++++++++++++++++++++++++++++++++++++- > 1 file changed, 116 insertions(+), 1 deletion(-) > > diff --git a/lib/ecdsa/ecdsa-verify.c b/lib/ecdsa/ecdsa-verify.c > index d2e6a40f4a..d84f6eb093 100644 > --- a/lib/ecdsa/ecdsa-verify.c > +++ b/lib/ecdsa/ecdsa-verify.c > @@ -1,13 +1,128 @@ > // SPDX-License-Identifier: GPL-2.0+ > /* > + * ECDSA signature verification for u-boot > + * > + * This implements the firmware-side wrapper for ECDSA verification. It bridges > + * the struct crypto_algo API to the ECDSA uclass implementations. > + * > * Copyright (c) 2020, Alexandru Gagniuc<mr.nuke.me@gmail.com> > */ > http://www.denx.de/wiki/U-Boot/CodingStyle #Include files #include <crypto/ecdsa-uclass.h> it is normally the first in alphabetic order of directory > +#include <dm/uclass.h> > #include <u-boot/ecdsa.h> > +#include <crypto/ecdsa-uclass.h> > + > +/* > + * Derive size of an ECDSA key from the curve name > + * > + * While it's possible to extract the key size by using string manipulation, > + * use a list of known curves for the time being. > + */ > +static int ecdsa_key_size(const char *curve_name) > +{ > + if (!strcmp(curve_name, "prime256v1")) > + return 256; > + else > + return 0; > +} > + To prepare the future can you parse a array of supported curves with associated ID used as parameter of ECDSA parameter = enum ECDSA_CURVES for example: char * name, int size, enum ECDSA_CURVES const [] = { {"prime256v1", 256, ECDSA_PRIME256V1 }, } > +static int fdt_get_key(struct ecdsa_public_key *key, const void *fdt, int node) > +{ > + int x_len, y_len; > + > + key->curve_name = fdt_getprop(fdt, node, "ecdsa,curve", NULL); > + key->size_bits = ecdsa_key_size(key->curve_name); > + if (key->size_bits == 0) { > + debug("Unknown ECDSA curve '%s'", key->curve_name); > + return -EINVAL; > + } > + > + key->x = fdt_getprop(fdt, node, "ecdsa,x-point", &x_len); > + key->y = fdt_getprop(fdt, node, "ecdsa,y-point", &y_len); > + > + if (!key->x || !key->y) > + return -EINVAL; > + > + if (x_len != (key->size_bits / 8) || y_len != (key->size_bits / 8)) { > + printf("%s: node=%d, curve@%p x@%p+%i y@%p+%i\n", __func__, > + node, key->curve_name, key->x, x_len, key->y, y_len); > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int ecdsa_verify_hash(struct udevice *dev, > + const struct image_sign_info *info, > + const void *hash, const void *sig, uint sig_len) > +{ > + const struct ecdsa_ops *ops = device_get_ops(dev); > + const struct checksum_algo *algo = info->checksum; > + struct ecdsa_public_key key; > + int sig_node, key_node, ret; > + > + if (!ops || !ops->verify) > + return -ENODEV; > + > + if (info->required_keynode > 0) { > + ret = fdt_get_key(&key, info->fdt_blob, info->required_keynode); > + if (ret < 0) > + return ret; > + > + return ops->verify(dev, &key, hash, algo->checksum_len, > + sig, sig_len); Need to indicate the used curve here as parameter of the verify opts ? > + } > + > + sig_node = fdt_subnode_offset(info->fdt_blob, 0, FIT_SIG_NODENAME); > + if (sig_node < 0) > + return -ENOENT; > + > + /* Try all possible keys under the "/signature" node */ > + fdt_for_each_subnode(key_node, info->fdt_blob, sig_node) { > + ret = fdt_get_key(&key, info->fdt_blob, key_node); > + if (ret < 0) > + continue; > + > + ret = ops->verify(dev, &key, hash, algo->checksum_len, > + sig, sig_len); > + > + /* On success, don't worry about remaining keys */ > + if (ret == 0) issue raised by chekpatch I think if (!ret) > + return 0; > + } > + > + return -EPERM; > +} > > int ecdsa_verify(struct image_sign_info *info, > const struct image_region region[], int region_count, > uint8_t *sig, uint sig_len) > { > - return -EOPNOTSUPP; > + const struct checksum_algo *algo = info->checksum; > + uint8_t hash[algo->checksum_len]; > + struct udevice *dev; > + int ret; > + > + ret = uclass_first_device(UCLASS_ECDSA, &dev); > + if (ret) { > + debug("ECDSA: Could not find ECDSA implementation: %d\n", ret); > + return ret; > + } > + > + ret = algo->calculate(algo->name, region, region_count, hash); > + if (ret < 0) > + return -EINVAL; > + > + return ecdsa_verify_hash(dev, info, hash, sig, sig_len); > } > + > +/* > + * uclass definition for ECDSA API > + * > + * We don't implement any wrappers around ecdsa_ops->verify() because it's > + * trivial to call ops->verify(). > + */ > +UCLASS_DRIVER(ecdsa) = { > + .id = UCLASS_ECDSA, > + .name = "ecdsa_verifier", > +}; Regards Patrick
On 2/9/21 9:56 AM, Patrick DELAUNAY wrote: > Hi, [snip] >> diff --git a/lib/ecdsa/ecdsa-verify.c b/lib/ecdsa/ecdsa-verify.c >> index d2e6a40f4a..d84f6eb093 100644 >> --- a/lib/ecdsa/ecdsa-verify.c >> +++ b/lib/ecdsa/ecdsa-verify.c >> @@ -1,13 +1,128 @@ >> // SPDX-License-Identifier: GPL-2.0+ >> /* >> + * ECDSA signature verification for u-boot >> + * >> + * This implements the firmware-side wrapper for ECDSA verification. >> It bridges >> + * the struct crypto_algo API to the ECDSA uclass implementations. >> + * >> * Copyright (c) 2020, Alexandru Gagniuc<mr.nuke.me@gmail.com> >> */ > > http://www.denx.de/wiki/U-Boot/CodingStyle #Include files > > #include <crypto/ecdsa-uclass.h> > > it is normally the first in alphabetic order of directory Thank your for catching that. I will have it fixed in the next iteration of this series. >> +#include <dm/uclass.h> >> #include <u-boot/ecdsa.h> >> +#include <crypto/ecdsa-uclass.h> >> + >> +/* >> + * Derive size of an ECDSA key from the curve name >> + * >> + * While it's possible to extract the key size by using string >> manipulation, >> + * use a list of known curves for the time being. >> + */ >> +static int ecdsa_key_size(const char *curve_name) >> +{ >> + if (!strcmp(curve_name, "prime256v1")) >> + return 256; >> + else >> + return 0; >> +} >> + > > > To prepare the future can you parse a array of supported curves with > associated ID > > used as parameter of ECDSA parameter = enum ECDSA_CURVES > > for example: char * name, int size, enum ECDSA_CURVES > > const [] = { > {"prime256v1", 256, ECDSA_PRIME256V1 }, > > } That is possible. If I were to have a longer list of curve names, I change things to extract the key length from the name itself. So instead of running strncmp() of N keyname strings, I would extract the digits from 'curve_name'. I chose not to do that here because I want this patch to be didactic. That is, I'm trying to achieve the goal clearly, with the lowest number of lines of code. [snip] >> +static int ecdsa_verify_hash(struct udevice *dev, >> + const struct image_sign_info *info, >> + const void *hash, const void *sig, uint sig_len) >> +{ >> + const struct ecdsa_ops *ops = device_get_ops(dev); >> + const struct checksum_algo *algo = info->checksum; >> + struct ecdsa_public_key key; >> + int sig_node, key_node, ret; >> + >> + if (!ops || !ops->verify) >> + return -ENODEV; >> + >> + if (info->required_keynode > 0) { >> + ret = fdt_get_key(&key, info->fdt_blob, info->required_keynode); >> + if (ret < 0) >> + return ret; >> + >> + return ops->verify(dev, &key, hash, algo->checksum_len, >> + sig, sig_len); > > > Need to indicate the used curve here as parameter of the verify opts ? The curve name is part of the key (struct ecdsa_public_key). [snip] >> + ret = ops->verify(dev, &key, hash, algo->checksum_len, >> + sig, sig_len); >> + >> + /* On success, don't worry about remaining keys */ >> + if (ret == 0) > > > issue raised by chekpatch I think > > if (!ret) Oh! I'll get this fixed. Thanks! Alex
diff --git a/lib/ecdsa/ecdsa-verify.c b/lib/ecdsa/ecdsa-verify.c index d2e6a40f4a..d84f6eb093 100644 --- a/lib/ecdsa/ecdsa-verify.c +++ b/lib/ecdsa/ecdsa-verify.c @@ -1,13 +1,128 @@ // SPDX-License-Identifier: GPL-2.0+ /* + * ECDSA signature verification for u-boot + * + * This implements the firmware-side wrapper for ECDSA verification. It bridges + * the struct crypto_algo API to the ECDSA uclass implementations. + * * Copyright (c) 2020, Alexandru Gagniuc <mr.nuke.me@gmail.com> */ +#include <dm/uclass.h> #include <u-boot/ecdsa.h> +#include <crypto/ecdsa-uclass.h> + +/* + * Derive size of an ECDSA key from the curve name + * + * While it's possible to extract the key size by using string manipulation, + * use a list of known curves for the time being. + */ +static int ecdsa_key_size(const char *curve_name) +{ + if (!strcmp(curve_name, "prime256v1")) + return 256; + else + return 0; +} + +static int fdt_get_key(struct ecdsa_public_key *key, const void *fdt, int node) +{ + int x_len, y_len; + + key->curve_name = fdt_getprop(fdt, node, "ecdsa,curve", NULL); + key->size_bits = ecdsa_key_size(key->curve_name); + if (key->size_bits == 0) { + debug("Unknown ECDSA curve '%s'", key->curve_name); + return -EINVAL; + } + + key->x = fdt_getprop(fdt, node, "ecdsa,x-point", &x_len); + key->y = fdt_getprop(fdt, node, "ecdsa,y-point", &y_len); + + if (!key->x || !key->y) + return -EINVAL; + + if (x_len != (key->size_bits / 8) || y_len != (key->size_bits / 8)) { + printf("%s: node=%d, curve@%p x@%p+%i y@%p+%i\n", __func__, + node, key->curve_name, key->x, x_len, key->y, y_len); + return -EINVAL; + } + + return 0; +} + +static int ecdsa_verify_hash(struct udevice *dev, + const struct image_sign_info *info, + const void *hash, const void *sig, uint sig_len) +{ + const struct ecdsa_ops *ops = device_get_ops(dev); + const struct checksum_algo *algo = info->checksum; + struct ecdsa_public_key key; + int sig_node, key_node, ret; + + if (!ops || !ops->verify) + return -ENODEV; + + if (info->required_keynode > 0) { + ret = fdt_get_key(&key, info->fdt_blob, info->required_keynode); + if (ret < 0) + return ret; + + return ops->verify(dev, &key, hash, algo->checksum_len, + sig, sig_len); + } + + sig_node = fdt_subnode_offset(info->fdt_blob, 0, FIT_SIG_NODENAME); + if (sig_node < 0) + return -ENOENT; + + /* Try all possible keys under the "/signature" node */ + fdt_for_each_subnode(key_node, info->fdt_blob, sig_node) { + ret = fdt_get_key(&key, info->fdt_blob, key_node); + if (ret < 0) + continue; + + ret = ops->verify(dev, &key, hash, algo->checksum_len, + sig, sig_len); + + /* On success, don't worry about remaining keys */ + if (ret == 0) + return 0; + } + + return -EPERM; +} int ecdsa_verify(struct image_sign_info *info, const struct image_region region[], int region_count, uint8_t *sig, uint sig_len) { - return -EOPNOTSUPP; + const struct checksum_algo *algo = info->checksum; + uint8_t hash[algo->checksum_len]; + struct udevice *dev; + int ret; + + ret = uclass_first_device(UCLASS_ECDSA, &dev); + if (ret) { + debug("ECDSA: Could not find ECDSA implementation: %d\n", ret); + return ret; + } + + ret = algo->calculate(algo->name, region, region_count, hash); + if (ret < 0) + return -EINVAL; + + return ecdsa_verify_hash(dev, info, hash, sig, sig_len); } + +/* + * uclass definition for ECDSA API + * + * We don't implement any wrappers around ecdsa_ops->verify() because it's + * trivial to call ops->verify(). + */ +UCLASS_DRIVER(ecdsa) = { + .id = UCLASS_ECDSA, + .name = "ecdsa_verifier", +};
Implement the crypto_algo .verify() function for ecdsa256. Because it backends on UCLASS_ECDSA, this change is focused on parsing the keys from devicetree and passing this information to the specific UCLASS driver. Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com> --- lib/ecdsa/ecdsa-verify.c | 117 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 116 insertions(+), 1 deletion(-)