| Message ID | 010101584549df3d-f1973c42-cc9a-4ead-9101-6dc6da46ed0e-000000@us-west-2.amazonses.com |
|---|---|
| State | Accepted |
| Commit | 5300a4f9336291fb713fcfaf9ea6e51b71824800 |
| Delegated to: | Tom Rini |
| Headers | show |
Hi, On 8 November 2016 at 11:53, aduda <aduda@meraki.com> wrote: > From: Andrew Duda <aduda@meraki.com> > > checksum_algo's pad_len field isn't actually used to store the length of > the padding but the total length of the RSA key (msg_len + pad_len) Perhaps it should be padded_key_len or padded_len? > > Signed-off-by: Andrew Duda <aduda@meraki.com> > Signed-off-by: aduda <aduda@meraki.com> > --- > > include/image.h | 2 +- > lib/rsa/rsa-verify.c | 6 +++--- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/include/image.h b/include/image.h > index 2b1296c..bfe10a0 100644 > --- a/include/image.h > +++ b/include/image.h > @@ -1070,7 +1070,7 @@ struct image_region { > struct checksum_algo { > const char *name; > const int checksum_len; > - const int pad_len; > + const int key_len; > #if IMAGE_ENABLE_SIGN > const EVP_MD *(*calculate_sign)(void); > #endif > diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c > index 442b769..5418f59 100644 > --- a/lib/rsa/rsa-verify.c > +++ b/lib/rsa/rsa-verify.c > @@ -84,7 +84,7 @@ static int rsa_verify_key(struct key_prop *prop, const uint8_t *sig, > } > > padding = algo->rsa_padding; > - pad_len = algo->pad_len - algo->checksum_len; > + pad_len = algo->key_len - algo->checksum_len; > > /* Check pkcs1.5 padding bytes. */ > if (memcmp(buf, padding, pad_len)) { > @@ -160,7 +160,7 @@ int rsa_verify(struct image_sign_info *info, > { > const void *blob = info->fdt_blob; > /* Reserve memory for maximum checksum-length */ > - uint8_t hash[info->algo->checksum->pad_len]; > + uint8_t hash[info->algo->checksum->key_len]; > int ndepth, noffset; > int sig_node, node; > char name[100]; > @@ -171,7 +171,7 @@ int rsa_verify(struct image_sign_info *info, > * rsa-signature-length > */ > if (info->algo->checksum->checksum_len > > - info->algo->checksum->pad_len) { > + info->algo->checksum->key_len) { > debug("%s: invlaid checksum-algorithm %s for %s\n", > __func__, info->algo->checksum->name, info->algo->name); > return -EINVAL; > -- > 2.10.2 > Regards, Simon
Simon, padded_len could work. I decided to go with key_len to be more RSA-independent since I have been dealing with ECDSA primarily. More specifically, ECDSA has no notion of padding or padded_len, but it does have a notion of key_len. And in RSA, I believe the padded_len is the same as the key_len. So the name key_len name would be applicable to both RSA and ECDSA. Granted, only RSA is currently supported in u-boot so I wouldn't have much of a problem updating this to padded_len. (sorry for the duplicate Simon) Thanks, Andrew On Fri, Nov 11, 2016 at 8:17 AM, Simon Glass <sjg@chromium.org> wrote: > Hi, > > On 8 November 2016 at 11:53, aduda <aduda@meraki.com> wrote: >> From: Andrew Duda <aduda@meraki.com> >> >> checksum_algo's pad_len field isn't actually used to store the length of >> the padding but the total length of the RSA key (msg_len + pad_len) > > Perhaps it should be padded_key_len or padded_len? > >> >> Signed-off-by: Andrew Duda <aduda@meraki.com> >> Signed-off-by: aduda <aduda@meraki.com> >> --- >> >> include/image.h | 2 +- >> lib/rsa/rsa-verify.c | 6 +++--- >> 2 files changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/include/image.h b/include/image.h >> index 2b1296c..bfe10a0 100644 >> --- a/include/image.h >> +++ b/include/image.h >> @@ -1070,7 +1070,7 @@ struct image_region { >> struct checksum_algo { >> const char *name; >> const int checksum_len; >> - const int pad_len; >> + const int key_len; >> #if IMAGE_ENABLE_SIGN >> const EVP_MD *(*calculate_sign)(void); >> #endif >> diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c >> index 442b769..5418f59 100644 >> --- a/lib/rsa/rsa-verify.c >> +++ b/lib/rsa/rsa-verify.c >> @@ -84,7 +84,7 @@ static int rsa_verify_key(struct key_prop *prop, const uint8_t *sig, >> } >> >> padding = algo->rsa_padding; >> - pad_len = algo->pad_len - algo->checksum_len; >> + pad_len = algo->key_len - algo->checksum_len; >> >> /* Check pkcs1.5 padding bytes. */ >> if (memcmp(buf, padding, pad_len)) { >> @@ -160,7 +160,7 @@ int rsa_verify(struct image_sign_info *info, >> { >> const void *blob = info->fdt_blob; >> /* Reserve memory for maximum checksum-length */ >> - uint8_t hash[info->algo->checksum->pad_len]; >> + uint8_t hash[info->algo->checksum->key_len]; >> int ndepth, noffset; >> int sig_node, node; >> char name[100]; >> @@ -171,7 +171,7 @@ int rsa_verify(struct image_sign_info *info, >> * rsa-signature-length >> */ >> if (info->algo->checksum->checksum_len > >> - info->algo->checksum->pad_len) { >> + info->algo->checksum->key_len) { >> debug("%s: invlaid checksum-algorithm %s for %s\n", >> __func__, info->algo->checksum->name, info->algo->name); >> return -EINVAL; >> -- >> 2.10.2 >> > > Regards, > Simon
On 11 November 2016 at 14:16, Andrew Duda <andrew.duda@meraki.net> wrote: > > Simon, > > padded_len could work. I decided to go with key_len to be more > RSA-independent since I have been dealing with ECDSA primarily. More > specifically, ECDSA has no notion of padding or padded_len, but it > does have a notion of key_len. And in RSA, I believe the padded_len is > the same as the key_len. So the name key_len name would be applicable > to both RSA and ECDSA. Granted, only RSA is currently supported in > u-boot so I wouldn't have much of a problem updating this to > padded_len. > > (sorry for the duplicate Simon) OK I see. Reviewed-by: Simon Glass <sjg@chromium.org> > > Thanks, > Andrew > > On Fri, Nov 11, 2016 at 8:17 AM, Simon Glass <sjg@chromium.org> wrote: > > Hi, > > > > On 8 November 2016 at 11:53, aduda <aduda@meraki.com> wrote: > >> From: Andrew Duda <aduda@meraki.com> > >> > >> checksum_algo's pad_len field isn't actually used to store the length of > >> the padding but the total length of the RSA key (msg_len + pad_len) > > > > Perhaps it should be padded_key_len or padded_len? > > > >> > >> Signed-off-by: Andrew Duda <aduda@meraki.com> > >> Signed-off-by: aduda <aduda@meraki.com> > >> --- > >> > >> include/image.h | 2 +- > >> lib/rsa/rsa-verify.c | 6 +++--- > >> 2 files changed, 4 insertions(+), 4 deletions(-) > >> > >> diff --git a/include/image.h b/include/image.h > >> index 2b1296c..bfe10a0 100644 > >> --- a/include/image.h > >> +++ b/include/image.h > >> @@ -1070,7 +1070,7 @@ struct image_region { > >> struct checksum_algo { > >> const char *name; > >> const int checksum_len; > >> - const int pad_len; > >> + const int key_len; > >> #if IMAGE_ENABLE_SIGN > >> const EVP_MD *(*calculate_sign)(void); > >> #endif > >> diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c > >> index 442b769..5418f59 100644 > >> --- a/lib/rsa/rsa-verify.c > >> +++ b/lib/rsa/rsa-verify.c > >> @@ -84,7 +84,7 @@ static int rsa_verify_key(struct key_prop *prop, const uint8_t *sig, > >> } > >> > >> padding = algo->rsa_padding; > >> - pad_len = algo->pad_len - algo->checksum_len; > >> + pad_len = algo->key_len - algo->checksum_len; > >> > >> /* Check pkcs1.5 padding bytes. */ > >> if (memcmp(buf, padding, pad_len)) { > >> @@ -160,7 +160,7 @@ int rsa_verify(struct image_sign_info *info, > >> { > >> const void *blob = info->fdt_blob; > >> /* Reserve memory for maximum checksum-length */ > >> - uint8_t hash[info->algo->checksum->pad_len]; > >> + uint8_t hash[info->algo->checksum->key_len]; > >> int ndepth, noffset; > >> int sig_node, node; > >> char name[100]; > >> @@ -171,7 +171,7 @@ int rsa_verify(struct image_sign_info *info, > >> * rsa-signature-length > >> */ > >> if (info->algo->checksum->checksum_len > > >> - info->algo->checksum->pad_len) { > >> + info->algo->checksum->key_len) { > >> debug("%s: invlaid checksum-algorithm %s for %s\n", > >> __func__, info->algo->checksum->name, info->algo->name); > >> return -EINVAL; > >> -- > >> 2.10.2 > >> > > > > Regards, > > Simon
On Tue, Nov 08, 2016 at 06:53:39PM +0000, aduda wrote: > From: Andrew Duda <aduda@meraki.com> > > checksum_algo's pad_len field isn't actually used to store the length of > the padding but the total length of the RSA key (msg_len + pad_len) > > Signed-off-by: Andrew Duda <aduda@meraki.com> > Signed-off-by: aduda <aduda@meraki.com> > Reviewed-by: Simon Glass <sjg@chromium.org> Applied to u-boot/master, thanks!
diff --git a/include/image.h b/include/image.h index 2b1296c..bfe10a0 100644 --- a/include/image.h +++ b/include/image.h @@ -1070,7 +1070,7 @@ struct image_region { struct checksum_algo { const char *name; const int checksum_len; - const int pad_len; + const int key_len; #if IMAGE_ENABLE_SIGN const EVP_MD *(*calculate_sign)(void); #endif diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c index 442b769..5418f59 100644 --- a/lib/rsa/rsa-verify.c +++ b/lib/rsa/rsa-verify.c @@ -84,7 +84,7 @@ static int rsa_verify_key(struct key_prop *prop, const uint8_t *sig, } padding = algo->rsa_padding; - pad_len = algo->pad_len - algo->checksum_len; + pad_len = algo->key_len - algo->checksum_len; /* Check pkcs1.5 padding bytes. */ if (memcmp(buf, padding, pad_len)) { @@ -160,7 +160,7 @@ int rsa_verify(struct image_sign_info *info, { const void *blob = info->fdt_blob; /* Reserve memory for maximum checksum-length */ - uint8_t hash[info->algo->checksum->pad_len]; + uint8_t hash[info->algo->checksum->key_len]; int ndepth, noffset; int sig_node, node; char name[100]; @@ -171,7 +171,7 @@ int rsa_verify(struct image_sign_info *info, * rsa-signature-length */ if (info->algo->checksum->checksum_len > - info->algo->checksum->pad_len) { + info->algo->checksum->key_len) { debug("%s: invlaid checksum-algorithm %s for %s\n", __func__, info->algo->checksum->name, info->algo->name); return -EINVAL;