[U-Boot] [PATCH] rsa: Fix two errors in the implementation

1. Failure to set the return code correctly 2. Failure to detect the loop end condition when the value is equal to the modulus.
Reported-by: Jeroen Hofstee jeroen@myspectrum.nl Signed-off-by: Simon Glass sjg@chromium.org ---
lib/rsa/rsa-sign.c | 1 + lib/rsa/rsa-verify.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/rsa/rsa-sign.c b/lib/rsa/rsa-sign.c index 83f5e87..6905131 100644 --- a/lib/rsa/rsa-sign.c +++ b/lib/rsa/rsa-sign.c @@ -76,6 +76,7 @@ static int rsa_get_pub_key(const char *keydir, const char *name, RSA **rsap) rsa = EVP_PKEY_get1_RSA(key); if (!rsa) { rsa_err("Couldn't convert to a RSA style key"); + ret = -EINVAL; goto err_rsa; } fclose(f); diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c index bcb9063..02e3eeb 100644 --- a/lib/rsa/rsa-verify.c +++ b/lib/rsa/rsa-verify.c @@ -54,9 +54,9 @@ static void subtract_modulus(const struct rsa_public_key *key, uint32_t num[]) static int greater_equal_modulus(const struct rsa_public_key *key, uint32_t num[]) { - uint32_t i; + int i;
- for (i = key->len - 1; i >= 0; i--) { + for (i = (int)key->len - 1; i >= 0; i--) { if (num[i] < key->modulus[i]) return 0; if (num[i] > key->modulus[i])

Hello Simon,
On 30-07-14 18:00, Simon Glass wrote:
- Failure to set the return code correctly
- Failure to detect the loop end condition when the value is equal to
the modulus.
Reported-by: Jeroen Hofstee jeroen@myspectrum.nl Signed-off-by: Simon Glass sjg@chromium.org
lib/rsa/rsa-sign.c | 1 + lib/rsa/rsa-verify.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/rsa/rsa-sign.c b/lib/rsa/rsa-sign.c index 83f5e87..6905131 100644 --- a/lib/rsa/rsa-sign.c +++ b/lib/rsa/rsa-sign.c @@ -76,6 +76,7 @@ static int rsa_get_pub_key(const char *keydir, const char *name, RSA **rsap) rsa = EVP_PKEY_get1_RSA(key); if (!rsa) { rsa_err("Couldn't convert to a RSA style key");
goto err_rsa; } fclose(f);ret = -EINVAL;
diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c index bcb9063..02e3eeb 100644 --- a/lib/rsa/rsa-verify.c +++ b/lib/rsa/rsa-verify.c @@ -54,9 +54,9 @@ static void subtract_modulus(const struct rsa_public_key *key, uint32_t num[]) static int greater_equal_modulus(const struct rsa_public_key *key, uint32_t num[]) {
- uint32_t i;
- int i;
- for (i = key->len - 1; i >= 0; i--) {
- for (i = (int)key->len - 1; i >= 0; i--) { if (num[i] < key->modulus[i]) return 0; if (num[i] > key->modulus[i])
I did indeed not post a patch, since I do not know how this code is used and how critical it is. And I still haven't bothered to look it up.
So just a general comment, which might not make any sense at all for the actual usage. If num can somehow be controlled by an evil source, passing a large enough value or 0 now causes this function to return equal. I have no idea if this causes any practical issue.
Warnings / error wise, this seems fine, thanks!
Regards, Jeroen

Hello Simon,
{
- uint32_t i;
- int i;
- for (i = key->len - 1; i >= 0; i--) {
- for (i = (int)key->len - 1; i >= 0; i--) { if (num[i] < key->modulus[i]) return 0; if (num[i] > key->modulus[i])
I did indeed not post a patch, since I do not know how this code is used and how critical it is. And I still haven't bothered to look it up.
So just a general comment, which might not make any sense at all for the actual usage. If num can somehow be controlled by an
I meant key->len here of course ^
Regards, Jeroen

Hi Jeroen,
On 30 July 2014 15:17, Jeroen Hofstee dasuboot@myspectrum.nl wrote:
Hello Simon,
{
- uint32_t i;
- int i;
- for (i = key->len - 1; i >= 0; i--) {
- for (i = (int)key->len - 1; i >= 0; i--) { if (num[i] < key->modulus[i]) return 0; if (num[i] > key->modulus[i])
I did indeed not post a patch, since I do not know how this code is used and how critical it is. And I still haven't bothered to look it up.
So just a general comment, which might not make any sense at all for the actual usage. If num can somehow be controlled by an
I meant key->len here of course ^
OK I see. Well the key length is range-checked in pow_mod(). If a key length of 0 were used, it would not be a valid signature - this function might do strange things. But the key length has to match the public key, so something like that would juts cause a verification failure higher up the stack.
Regards, Simon

On Wed, Jul 30, 2014 at 10:00:17AM -0600, Simon Glass wrote:
- Failure to set the return code correctly
- Failure to detect the loop end condition when the value is equal to
the modulus.
Reported-by: Jeroen Hofstee jeroen@myspectrum.nl Signed-off-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
participants (4)
-
Jeroen Hofstee
-
Jeroen Hofstee
-
Simon Glass
-
Tom Rini