Reject negative numbers in BN_{mod_mul,to,from}_montgomery.
These functions already require their inputs to be reduced mod N (or, in
some cases, bounded by R or N*R), so negative numbers are nonsense. The
code still attempted to account for them by working on the absolute
value and fiddling with the sign bit. (The output would be in range (-N,
N) instead of [0, N).)
This complicates relaxing bn_correct_top because bn_correct_top is also
used to prevent storing a negative zero. Instead, just reject negative
inputs.
Upgrade-Note: These functions are public API, so some callers may
notice. Code search suggests there is only one caller outside
BoringSSL, and it looks fine.
Bug: 232
Change-Id: Ieba3acbb36b0ff6b72b8ed2b14882ec9b88e4665
Reviewed-on: https://boringssl-review.googlesource.com/25249
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/fipsmodule/bn/montgomery.c b/crypto/fipsmodule/bn/montgomery.c
index 1521a4d..49979fd 100644
--- a/crypto/fipsmodule/bn/montgomery.c
+++ b/crypto/fipsmodule/bn/montgomery.c
@@ -300,6 +300,11 @@
static int BN_from_montgomery_word(BIGNUM *ret, BIGNUM *r,
const BN_MONT_CTX *mont) {
+ if (r->neg) {
+ OPENSSL_PUT_ERROR(BN, BN_R_NEGATIVE_NUMBER);
+ return 0;
+ }
+
const BIGNUM *n = &mont->N;
if (n->top == 0) {
ret->top = 0;
@@ -321,7 +326,7 @@
if (!bn_from_montgomery_in_place(ret->d, ret->top, r->d, r->top, mont)) {
return 0;
}
- ret->neg = r->neg;
+ ret->neg = 0;
bn_correct_top(r);
bn_correct_top(ret);
@@ -407,6 +412,11 @@
int BN_mod_mul_montgomery(BIGNUM *r, const BIGNUM *a, const BIGNUM *b,
const BN_MONT_CTX *mont, BN_CTX *ctx) {
+ if (a->neg || b->neg) {
+ OPENSSL_PUT_ERROR(BN, BN_R_NEGATIVE_NUMBER);
+ return 0;
+ }
+
#if defined(OPENSSL_BN_ASM_MONT)
// |bn_mul_mont| requires at least 128 bits of limbs, at least for x86.
int num = mont->N.top;
@@ -422,7 +432,7 @@
OPENSSL_PUT_ERROR(BN, ERR_R_INTERNAL_ERROR);
return 0;
}
- r->neg = a->neg ^ b->neg;
+ r->neg = 0;
r->top = num;
bn_correct_top(r);