kris/boringssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Don't leak widths in bn_mod_mul_montgomery_fallback.

Browse Source 
The fallback functions still themselves leak, but I've left TODOs there.

This only affects BN_mod_mul_montgomery on platforms where we don't use
the bn_mul_mont assembly, but BN_mul additionally affects the final
multiplication in RSA CRT.

Bug: 232
Change-Id: Ia1ae16162c38e10c056b76d6b2afbed67f1a5e16
Reviewed-on: https://boringssl-review.googlesource.com/25260
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This commit is contained in:
David Benjamin 2018-01-24 12:10:56 -05:00 committed by CQ bot account: commit-bot@chromium.org
parent 08d774a45f
commit c7b6e0a664
3 changed files with 66 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
crypto/fipsmodule/bn/internal.h
View File

@ -352,6 +352,25 @@ int bn_one_to_montgomery(BIGNUM *r, const BN_MONT_CTX *mont, BN_CTX *ctx);
int bn_less_than_montgomery_R(const BIGNUM *bn, const BN_MONT_CTX *mont);
// Fixed-width arithmetic.
//
// The following functions implement non-modular arithmetic in constant-time
// and pessimally set |r->width| to the largest possible word size.
//
// Note this means that, e.g., repeatedly multiplying by one will cause widths
// to increase without bound. The corresponding public API functions minimize
// their outputs to avoid regressing calculator consumers.
// bn_mul_fixed behaves like |BN_mul|, but it rejects negative inputs and
// pessimally sets |r->width| to |a->width| + |b->width|, to avoid leaking
// information about |a| and |b|.
int bn_mul_fixed(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, BN_CTX *ctx);
// bn_sqrt_fixed behaves like |BN_sqrt|, but it pessimally sets |r->width| to
// 2*|a->width|, to avoid leaking information about |a| and |b|.
int bn_sqr_fixed(BIGNUM *r, const BIGNUM *a, BN_CTX *ctx);
// Low-level operations for small numbers.
//
// The following functions implement algorithms suitable for use with scalars

4
crypto/fipsmodule/bn/montgomery.c
View File

@ -376,11 +376,11 @@ static int bn_mod_mul_montgomery_fallback(BIGNUM *r, const BIGNUM *a,
}
if (a == b) {
if (!BN_sqr(tmp, a, ctx)) {
if (!bn_sqr_fixed(tmp, a, ctx)) {
goto err;
}
} else {
if (!BN_mul(tmp, a, b, ctx)) {
if (!bn_mul_fixed(tmp, a, b, ctx)) {
goto err;
}
}

49
crypto/fipsmodule/bn/mul.c
View File

@ -316,6 +316,9 @@ static void bn_mul_recursive(BN_ULONG *r, const BN_ULONG *a, const BN_ULONG *b,
return;
}
// TODO(davidben): This function is not constant-time, but should be. See
// https://crbug.com/boringssl/234.
// r=(a[0]-a[1])*(b[1]-b[0])
c1 = bn_cmp_part_words(a, &(a[n]), tna, n - tna);
c2 = bn_cmp_part_words(&(b[n]), b, tnb, tnb - n);
@ -436,6 +439,9 @@ static void bn_mul_part_recursive(BN_ULONG *r, const BN_ULONG *a,
return;
}
// TODO(davidben): This function is not constant-time, but should be. See
// https://crbug.com/boringssl/234.
// r=(a[0]-a[1])*(b[1]-b[0])
c1 = bn_cmp_part_words(a, &(a[n]), tna, n - tna);
c2 = bn_cmp_part_words(&(b[n]), b, tnb, tnb - n);
@ -559,7 +565,11 @@ static void bn_mul_part_recursive(BN_ULONG *r, const BN_ULONG *a,
}
}
int BN_mul(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, BN_CTX *ctx) {
// bn_mul_impl implements |BN_mul| and |bn_mul_fixed|. Note this function breaks
// |BIGNUM| invariants and may return a negative zero. This is handled by the
// callers.
static int bn_mul_impl(BIGNUM *r, const BIGNUM *a, const BIGNUM *b,
BN_CTX *ctx) {
int ret = 0;
int top, al, bl;
BIGNUM *rr;
@ -646,7 +656,6 @@ int BN_mul(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, BN_CTX *ctx) {
bn_mul_normal(rr->d, a->d, al, b->d, bl);
end:
bn_set_minimal_width(rr);
if (r != rr && !BN_copy(r, rr)) {
goto err;
}
@ -657,6 +666,26 @@ err:
return ret;
}
int BN_mul(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, BN_CTX *ctx) {
if (!bn_mul_impl(r, a, b, ctx)) {
return 0;
}
// This additionally fixes any negative zeros created by |bn_mul_impl|.
bn_set_minimal_width(r);
return 1;
}
int bn_mul_fixed(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, BN_CTX *ctx) {
// Prevent negative zeros.
if (a->neg || b->neg) {
OPENSSL_PUT_ERROR(BN, BN_R_NEGATIVE_NUMBER);
return 0;
}
return bn_mul_impl(r, a, b, ctx);
}
int bn_mul_small(BN_ULONG *r, size_t num_r, const BN_ULONG *a, size_t num_a,
const BN_ULONG *b, size_t num_b) {
if (num_r != num_a + num_b) {
@ -737,6 +766,10 @@ static void bn_sqr_recursive(BN_ULONG *r, const BN_ULONG *a, int n2,
bn_sqr_normal(r, a, n2, t);
return;
}
// TODO(davidben): This function is not constant-time, but should be. See
// https://crbug.com/boringssl/234.
// r=(a[0]-a[1])*(a[1]-a[0])
c1 = bn_cmp_words(a, &(a[n]), n);
zero = 0;
@ -813,7 +846,7 @@ int BN_mul_word(BIGNUM *bn, BN_ULONG w) {
return 1;
}
int BN_sqr(BIGNUM *r, const BIGNUM *a, BN_CTX *ctx) {
int bn_sqr_fixed(BIGNUM *r, const BIGNUM *a, BN_CTX *ctx) {
int max, al;
int ret = 0;
BIGNUM *tmp, *rr;
@ -867,7 +900,6 @@ int BN_sqr(BIGNUM *r, const BIGNUM *a, BN_CTX *ctx) {
rr->neg = 0;
rr->width = max;
bn_set_minimal_width(rr);
if (rr != r && !BN_copy(r, rr)) {
goto err;
@ -879,6 +911,15 @@ err:
return ret;
}
int BN_sqr(BIGNUM *r, const BIGNUM *a, BN_CTX *ctx) {
if (!bn_sqr_fixed(r, a, ctx)) {
return 0;
}
bn_set_minimal_width(r);
return 1;
}
int bn_sqr_small(BN_ULONG *r, size_t num_r, const BN_ULONG *a, size_t num_a) {
if (num_r != 2 * num_a || num_a > BN_SMALL_MAX_WORDS) {
OPENSSL_PUT_ERROR(BN, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);