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

Don't leak the exponent bit width in BN_mod_exp_mont_consttime.

Browse Source 
(See also https://github.com/openssl/openssl/pull/5154.)

The exponent here is one of d, dmp1, or dmq1 for RSA. This value and its
bit length are both secret. The only public upper bound is the bit width
of the corresponding modulus (RSA n, p, and q, respectively).

Although BN_num_bits is constant-time (sort of; see bn_correct_top notes
in preceding patch), this does not fix the root problem, which is that
the windows are based on the minimal bit width, not the upper bound. We
could use BN_num_bits(m), but BN_mod_exp_mont_consttime is public API
and may be called with larger exponents. Instead, use all top*BN_BITS2
bits in the BIGNUM. This is still sensitive to the long-standing
bn_correct_top leak, but we need to fix that regardless.

This may cause us to do a handful of extra multiplications for RSA keys
which are just above a whole number of words, but that is not a standard
RSA key size.

Change-Id: I5e2f12b70c303b27c597a7e513b7bf7288f7b0e3
Reviewed-on: https://boringssl-review.googlesource.com/25185
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-22 10:54:28 -05:00 committed by CQ bot account: commit-bot@chromium.org
parent cb1ad205d0
commit 32b5940267
1 changed files with 5 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
crypto/fipsmodule/bn/exponentiation.c
View File

@ -1006,7 +1006,7 @@ static int copy_from_prebuf(BIGNUM *b, int top, unsigned char *buf, int idx,
int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
const BIGNUM *m, BN_CTX *ctx,
const BN_MONT_CTX *mont) {
int i, bits, ret = 0, window, wvalue;
int i, ret = 0, window, wvalue;
int top;
BN_MONT_CTX *new_mont = NULL;
@ -1024,7 +1024,10 @@ int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
top = m->top;
bits = BN_num_bits(p);
// Use all bits stored in |p|, rather than |BN_num_bits|, so we do not leak
// whether the top bits are zero.
int max_bits = p->top * BN_BITS2;
int bits = max_bits;
if (bits == 0) {
// x**0 mod 1 is still zero.
if (BN_is_one(m)) {
@ -1217,7 +1220,6 @@ int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
}
} else {
const uint8_t *p_bytes = (const uint8_t *)p->d;
int max_bits = p->top * BN_BITS2;
assert(bits < max_bits);
// |p = 0| has been handled as a special case, so |max_bits| is at least
// one word.