From 046b279ae2be163d8362b75a567f6c075de4bd9b Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Thu, 4 Mar 2021 10:04:42 +1000 Subject: [PATCH] MP: fixes for negative sp_int.c: - sp_addmod_ct(), sp_submod_ct(), sp_gcd() and sp_lcm() only support positive numbers: updated comments. - sp_mod(0, neg): fix to not add 0 and neg. - sp_div(): set sign on rem when a is greater than d but same bit length and fix sign setting on result when absolute values equal or close. - Modular exponentation functions: compare absolute values when determining whether base needs to be reduced. - Fix calculation of hex string when negative: add -ve nibble before checking for need of extra 0. - Fix size allocation in sp_mod when WOLFSSL_SP_INT_NEGATIVE defined tfm.c: - fp_mod(0, neg): fix to not add 0 and neg. - fp_isone(): fixed to check for negative - fp_add_d(): fix small stack version to support negative numbers integer.c: - mp_isone(): fixed to check for negative --- wolfcrypt/src/sp_int.c | 45 +++++++++++++++++++++++-------------- wolfcrypt/src/tfm.c | 44 ++++++++++++++++-------------------- wolfssl/wolfcrypt/integer.h | 3 ++- wolfssl/wolfcrypt/tfm.h | 15 ++++++++----- 4 files changed, 58 insertions(+), 49 deletions(-) diff --git a/wolfcrypt/src/sp_int.c b/wolfcrypt/src/sp_int.c index cbd24ee19..0822861a8 100644 --- a/wolfcrypt/src/sp_int.c +++ b/wolfcrypt/src/sp_int.c @@ -4142,7 +4142,7 @@ static int sp_cmp_mag_ct(sp_int* a, sp_int* b, int len) #if defined(WOLFSSL_SP_MATH_ALL) && defined(HAVE_ECC) /* Add two value and reduce: r = (a + b) % m * - * r = a + b (mod m) - constant time (|a| < m and |b| < m and positive) + * r = a + b (mod m) - constant time (a < m and b < m, a, b and m are positive) * * Assumes a, b, m and r are not NULL. * @@ -4193,7 +4193,7 @@ int sp_addmod_ct(sp_int* a, sp_int* b, sp_int* m, sp_int* r) /* Sub b from a and reduce: r = (a - b) % m * Result is always positive. * - * r = a - b (mod m) - constant time (a < n and b < m and positive) + * r = a - b (mod m) - constant time (a < m and b < m, a, b and m are positive) * * Assumes a, b, m and r are not NULL. * @@ -4482,7 +4482,7 @@ int sp_div(sp_int* a, sp_int* d, sp_int* r, sp_int* rem) if (r != NULL) { sp_set(r, 1); #ifdef WOLFSSL_SP_INT_NEGATIVE - r->sign = aSign; + r->sign = (aSign == dSign) ? MP_ZPOS : MP_NEG; #endif /* WOLFSSL_SP_INT_NEGATIVE */ } done = 1; @@ -4491,11 +4491,14 @@ int sp_div(sp_int* a, sp_int* d, sp_int* r, sp_int* rem) /* a is greater than d but same bit length */ if (rem != NULL) { _sp_sub_off(a, d, rem, 0); + #ifdef WOLFSSL_SP_INT_NEGATIVE + rem->sign = aSign; + #endif } if (r != NULL) { sp_set(r, 1); #ifdef WOLFSSL_SP_INT_NEGATIVE - r->sign = aSign; + r->sign = (aSign == dSign) ? MP_ZPOS : MP_NEG; #endif /* WOLFSSL_SP_INT_NEGATIVE */ } done = 1; @@ -4681,7 +4684,7 @@ int sp_mod(sp_int* a, sp_int* m, sp_int* r) { int err = MP_OKAY; #ifdef WOLFSSL_SP_INT_NEGATIVE - DECL_SP_INT(t, (m == NULL) ? 1 : m->used); + DECL_SP_INT(t, (a == NULL) ? 1 : a->used + 1); #endif /* WOLFSSL_SP_INT_NEGATIVE */ if ((a == NULL) || (m == NULL) || (r == NULL)) { @@ -4695,11 +4698,11 @@ int sp_mod(sp_int* a, sp_int* m, sp_int* r) #else ALLOC_SP_INT(t, m->used, err, NULL); if (err == MP_OKAY) { - sp_init_size(t, m->used); + sp_init_size(t, a->used + 1); err = sp_div(a, m, NULL, t); } if (err == MP_OKAY) { - if (t->sign != m->sign) { + if ((!sp_iszero(t)) && (t->sign != m->sign)) { err = sp_add(t, m, r); } else { @@ -8013,7 +8016,7 @@ static int _sp_exptmod_ex(sp_int* b, sp_int* e, int bits, sp_int* m, sp_int* r) #endif /* Ensure base is less than exponent. */ - if (_sp_cmp(b, m) != MP_LT) { + if (_sp_cmp_abs(b, m) != MP_LT) { err = sp_mod(b, m, t[0]); if ((err == MP_OKAY) && sp_iszero(t[0])) { sp_set(r, 0); @@ -8113,7 +8116,7 @@ static int _sp_exptmod_mont_ex(sp_int* b, sp_int* e, int bits, sp_int* m, sp_init_size(t[3], m->used * 2 + 1); /* Ensure base is less than exponent. */ - if (_sp_cmp(b, m) != MP_LT) { + if (_sp_cmp_abs(b, m) != MP_LT) { err = sp_mod(b, m, t[0]); if ((err == MP_OKAY) && sp_iszero(t[0])) { sp_set(r, 0); @@ -8248,7 +8251,7 @@ static int _sp_exptmod_mont_ex(sp_int* b, sp_int* e, int bits, sp_int* m, sp_init_size(tr, m->used * 2 + 1); /* Ensure base is less than exponent. */ - if (_sp_cmp(b, m) != MP_LT) { + if (_sp_cmp_abs(b, m) != MP_LT) { err = sp_mod(b, m, t[1]); if ((err == MP_OKAY) && sp_iszero(t[1])) { sp_set(r, 0); @@ -8767,7 +8770,7 @@ static int _sp_exptmod_nct(sp_int* b, sp_int* e, sp_int* m, sp_int* r) sp_init_size(bm, m->used * 2 + 1); /* Ensure base is less than exponent. */ - if (_sp_cmp(b, m) != MP_LT) { + if (_sp_cmp_abs(b, m) != MP_LT) { err = sp_mod(b, m, bm); if ((err == MP_OKAY) && sp_iszero(bm)) { sp_set(r, 0); @@ -8980,7 +8983,7 @@ static int _sp_exptmod_nct(sp_int* b, sp_int* e, sp_int* m, sp_int* r) sp_init_size(t[1], m->used * 2 + 1); /* Ensure base is less than exponent. */ - if (_sp_cmp(b, m) != MP_LT) { + if (_sp_cmp_abs(b, m) != MP_LT) { err = sp_mod(b, m, t[0]); if ((err == MP_OKAY) && sp_iszero(t[0])) { sp_set(r, 0); @@ -12491,6 +12494,7 @@ int sp_tohex(sp_int* a, char* str) i = a->used - 1; #ifndef WC_DISABLE_RADIX_ZERO_PAD + /* Find highest non-zero byte in most-significant word. */ for (j = SP_WORD_SIZE - 8; j >= 0; j -= 8) { if (((a->dp[i] >> j) & 0xff) != 0) { break; @@ -12500,8 +12504,10 @@ int sp_tohex(sp_int* a, char* str) --i; } } + /* Start with high nibble of byte. */ j += 4; #else + /* Find highest non-zero nibble in most-significant word. */ for (j = SP_WORD_SIZE - 4; j >= 0; j -= 4) { if (((a->dp[i] >> j) & 0xf) != 0) { break; @@ -12512,6 +12518,7 @@ int sp_tohex(sp_int* a, char* str) } } #endif /* WC_DISABLE_RADIX_ZERO_PAD */ + /* Most-significant word. */ for (; j >= 0; j -= 4) { *(str++) = sp_hex_char[(a->dp[i] >> j) & 0xf]; } @@ -12660,16 +12667,16 @@ int sp_radix_size(sp_int* a, int radix, int* size) } else { int nibbles = (sp_count_bits(a) + 3) / 4; - #ifdef WOLFSSL_SP_INT_NEGATIVE - if (a->sign == MP_NEG) { - nibbles++; - } - #endif /* WOLFSSL_SP_INT_NEGATIVE */ #ifndef WC_DISABLE_RADIX_ZERO_PAD if (nibbles & 1) { nibbles++; } #endif /* WC_DISABLE_RADIX_ZERO_PAD */ + #ifdef WOLFSSL_SP_INT_NEGATIVE + if (a->sign == MP_NEG) { + nibbles++; + } + #endif /* WOLFSSL_SP_INT_NEGATIVE */ /* One more for \0 */ *size = nibbles + 1; } @@ -13233,6 +13240,8 @@ int sp_prime_is_prime_ex(sp_int* a, int t, int* result, WC_RNG* rng) #if !defined(NO_RSA) && defined(WOLFSSL_KEY_GEN) /* Calculates the Greatest Common Denominator (GCD) of a and b into r. + * + * a and b are positive integers. * * @param [in] a SP integer of first operand. * @param [in] b SP integer of second operand. @@ -13342,6 +13351,8 @@ int sp_gcd(sp_int* a, sp_int* b, sp_int* r) #if defined(WOLFSSL_SP_MATH_ALL) && !defined(NO_RSA) && defined(WOLFSSL_KEY_GEN) /* Calculates the Lowest Common Multiple (LCM) of a and b and stores in r. + * + * a and b are positive integers. * * @param [in] a SP integer of first operand. * @param [in] b SP integer of second operand. diff --git a/wolfcrypt/src/tfm.c b/wolfcrypt/src/tfm.c index 505fc556f..4352567cc 100644 --- a/wolfcrypt/src/tfm.c +++ b/wolfcrypt/src/tfm.c @@ -990,7 +990,7 @@ int fp_mod(fp_int *a, fp_int *b, fp_int *c) fp_init(t); err = fp_div(a, b, NULL, t); if (err == FP_OKAY) { - if (t->sign != b->sign) { + if (!fp_iszero(t) && (t->sign != b->sign)) { err = fp_add(t, b, c); } else { fp_copy(t, c); @@ -5328,32 +5328,26 @@ int fp_gcd(fp_int *a, fp_int *b, fp_int *c) int fp_add_d(fp_int *a, fp_digit b, fp_int *c) { #ifndef WOLFSSL_SMALL_STACK - fp_int tmp; - int err; - fp_init(&tmp); - fp_set(&tmp, b); - err = fp_add(a, &tmp, c); - return err; + fp_int tmp[1]; #else - int i; - fp_word t = b; - int err = FP_OKAY; - - fp_copy(a, c); - for (i = 0; t != 0 && i < FP_SIZE && i < c->used; i++) { - t += c->dp[i]; - c->dp[i] = (fp_digit)t; - t >>= DIGIT_BIT; - } - if (i == c->used && i < FP_SIZE && t != 0) { - c->dp[i] = t; - c->used++; - } - if (i == FP_SIZE && t != 0) { - err = FP_VAL; - } - return err; + fp_int* tmp; #endif + int err; + +#ifdef WOLFSSL_SMALL_STACK + tmp = (fp_int*)XMALLOC(sizeof(fp_int), NULL, DYNAMIC_TYPE_BIGINT); + if (tmp == NULL) + return FP_MEM; +#endif + + fp_init(tmp); + fp_set(tmp, b); + err = fp_add(a, tmp, c); + +#ifdef WOLFSSL_SMALL_STACK + XFREE(tmp, NULL, DYNAMIC_TYPE_BIGINT); +#endif + return err; } /* external compatibility */ diff --git a/wolfssl/wolfcrypt/integer.h b/wolfssl/wolfcrypt/integer.h index 8f3665b60..59a366bb1 100644 --- a/wolfssl/wolfcrypt/integer.h +++ b/wolfssl/wolfcrypt/integer.h @@ -227,7 +227,8 @@ typedef int ltm_prime_callback(unsigned char *dst, int len, void *dat); /* ---> Basic Manipulations <--- */ #define mp_iszero(a) (((a)->used == 0) ? MP_YES : MP_NO) #define mp_isone(a) \ - (((((a)->used == 1)) && ((a)->dp[0] == 1u)) ? MP_YES : MP_NO) + (((((a)->used == 1)) && ((a)->dp[0] == 1u) && ((a)->sign == MP_ZPOS)) \ + ? MP_YES : MP_NO) #define mp_iseven(a) \ (((a)->used > 0 && (((a)->dp[0] & 1u) == 0u)) ? MP_YES : MP_NO) #define mp_isodd(a) \ diff --git a/wolfssl/wolfcrypt/tfm.h b/wolfssl/wolfcrypt/tfm.h index 5546b57bf..3f4c43276 100644 --- a/wolfssl/wolfcrypt/tfm.h +++ b/wolfssl/wolfcrypt/tfm.h @@ -422,13 +422,16 @@ MP_API void fp_free(fp_int* a); /* zero/one/even/odd/neg/word ? */ #define fp_iszero(a) (((a)->used == 0) ? FP_YES : FP_NO) #define fp_isone(a) \ - ((((a)->used == 1) && ((a)->dp[0] == 1)) ? FP_YES : FP_NO) -#define fp_iseven(a) (((a)->used > 0 && (((a)->dp[0] & 1) == 0)) ? FP_YES : FP_NO) -#define fp_isodd(a) (((a)->used > 0 && (((a)->dp[0] & 1) == 1)) ? FP_YES : FP_NO) -#define fp_isneg(a) (((a)->sign != 0) ? FP_YES : FP_NO) -#define fp_isword(a, w) \ - ((((a)->used == 1) && ((a)->dp[0] == w)) || ((w == 0) && ((a)->used == 0)) \ + ((((a)->used == 1) && ((a)->dp[0] == 1) && ((a)->sign == FP_ZPOS)) \ ? FP_YES : FP_NO) +#define fp_iseven(a) \ + (((a)->used > 0 && (((a)->dp[0] & 1) == 0)) ? FP_YES : FP_NO) +#define fp_isodd(a) \ + (((a)->used > 0 && (((a)->dp[0] & 1) == 1)) ? FP_YES : FP_NO) +#define fp_isneg(a) (((a)->sign != FP_ZPOS) ? FP_YES : FP_NO) +#define fp_isword(a, w) \ + (((((a)->used == 1) && ((a)->dp[0] == w)) || \ + ((w == 0) && ((a)->used == 0))) ? FP_YES : FP_NO) /* set to a small digit */ void fp_set(fp_int *a, fp_digit b);