Ensure |BN_div| never gives negative zero in the no_branch code.

Have |bn_correct_top| fix |bn->neg| if the input is zero so that we
don't have negative zeros lying around.

Thanks to Brian Smith for noticing.

Change-Id: I91bcadebc8e353bb29c81c4367e85853886c8e4e
Reviewed-on: https://boringssl-review.googlesource.com/9074
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This commit is contained in:
David Benjamin 2016-08-02 13:09:19 -04:00 committed by CQ bot account: commit-bot@chromium.org
parent 875bf04237
commit 899b9b19a4
3 changed files with 41 additions and 26 deletions

View File

@ -342,6 +342,10 @@ void bn_correct_top(BIGNUM *bn) {
} }
bn->top = tmp_top; bn->top = tmp_top;
} }
if (bn->top == 0) {
bn->neg = 0;
}
} }
int BN_get_flags(const BIGNUM *bn, int flags) { int BN_get_flags(const BIGNUM *bn, int flags) {

View File

@ -1123,8 +1123,7 @@ static bool TestNegativeZero(BN_CTX *ctx) {
ScopedBIGNUM a(BN_new()); ScopedBIGNUM a(BN_new());
ScopedBIGNUM b(BN_new()); ScopedBIGNUM b(BN_new());
ScopedBIGNUM c(BN_new()); ScopedBIGNUM c(BN_new());
ScopedBIGNUM d(BN_new()); if (!a || !b || !c) {
if (!a || !b || !c || !d) {
return false; return false;
} }
@ -1142,30 +1141,42 @@ static bool TestNegativeZero(BN_CTX *ctx) {
return false; return false;
} }
// Test that BN_div never gives negative zero in the quotient. for (int consttime = 0; consttime < 2; consttime++) {
if (!BN_set_word(a.get(), 1) || ScopedBIGNUM numerator(BN_new()), denominator(BN_new());
!BN_set_word(b.get(), 2)) { if (!numerator || !denominator) {
return false; return false;
} }
BN_set_negative(a.get(), 1);
if (!BN_div(d.get(), c.get(), a.get(), b.get(), ctx)) {
return false;
}
if (!BN_is_zero(d.get()) || BN_is_negative(d.get())) {
fprintf(stderr, "Division test failed.\n");
return false;
}
// Test that BN_div never gives negative zero in the remainder. if (consttime) {
if (!BN_set_word(b.get(), 1)) { BN_set_flags(numerator.get(), BN_FLG_CONSTTIME);
return false; BN_set_flags(denominator.get(), BN_FLG_CONSTTIME);
} }
if (!BN_div(d.get(), c.get(), a.get(), b.get(), ctx)) {
return false; // Test that BN_div never gives negative zero in the quotient.
} if (!BN_set_word(numerator.get(), 1) ||
if (!BN_is_zero(c.get()) || BN_is_negative(c.get())) { !BN_set_word(denominator.get(), 2)) {
fprintf(stderr, "Division test failed.\n"); return false;
return false; }
BN_set_negative(numerator.get(), 1);
if (!BN_div(a.get(), b.get(), numerator.get(), denominator.get(), ctx)) {
return false;
}
if (!BN_is_zero(a.get()) || BN_is_negative(a.get())) {
fprintf(stderr, "Incorrect quotient (consttime = %d).\n", consttime);
return false;
}
// Test that BN_div never gives negative zero in the remainder.
if (!BN_set_word(denominator.get(), 1)) {
return false;
}
if (!BN_div(a.get(), b.get(), numerator.get(), denominator.get(), ctx)) {
return false;
}
if (!BN_is_zero(b.get()) || BN_is_negative(b.get())) {
fprintf(stderr, "Incorrect remainder (consttime = %d).\n", consttime);
return false;
}
} }
// Test that BN_set_negative will not produce a negative zero. // Test that BN_set_negative will not produce a negative zero.

View File

@ -323,7 +323,7 @@ OPENSSL_EXPORT int BN_marshal_asn1(CBB *cbb, const BIGNUM *bn);
* what you want before turning to these. */ * what you want before turning to these. */
/* bn_correct_top decrements |bn->top| until |bn->d[top-1]| is non-zero or /* bn_correct_top decrements |bn->top| until |bn->d[top-1]| is non-zero or
* until |top| is zero. */ * until |top| is zero. If |bn| is zero, |bn->neg| is set to zero. */
OPENSSL_EXPORT void bn_correct_top(BIGNUM *bn); OPENSSL_EXPORT void bn_correct_top(BIGNUM *bn);
/* bn_wexpand ensures that |bn| has at least |words| works of space without /* bn_wexpand ensures that |bn| has at least |words| works of space without