Skip to content

Commit

Permalink
Ensure we test all parameters for BN_FLG_CONSTTIME
Browse files Browse the repository at this point in the history 
RSA_setup_blinding() calls BN_BLINDING_create_param() which later calls
BN_mod_exp() as follows:

BN_mod_exp(ret->A, ret->A, ret->e, ret->mod, ctx)

ret->mod will have BN_FLG_CONSTTIME set, but ret->e does not. In
BN_mod_exp() we only test the third param for the existence of this flag.
We should test all the inputs.

Thanks to Samuel Weiser (samuel.weiser@iaik.tugraz.at) for reporting this
issue.

This typically only happens once at key load, so this is unlikely to be
exploitable in any real scenario.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from #4477)
  • Loading branch information
@mattcaswell
mattcaswell committed Oct 11, 2017
1 parent be9b311 commit e913d11
Showing 1 changed file with 16 additions and 6 deletions.
22 changes: 16 additions & 6 deletions crypto/bn/bn_exp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ int BN_exp(BIGNUM *r, const BIGNUM *a, const BIGNUM *p, BN_CTX *ctx)
int i, bits, ret = 0;
BIGNUM *v, *rr;

if (BN_get_flags(p, BN_FLG_CONSTTIME) != 0) {
if (BN_get_flags(p, BN_FLG_CONSTTIME) != 0
|| BN_get_flags(a, BN_FLG_CONSTTIME) != 0) {
/* BN_FLG_CONSTTIME only supported by BN_mod_exp_mont() */
BNerr(BN_F_BN_EXP, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
return 0;
Expand Down Expand Up @@ -133,7 +134,9 @@ int BN_mod_exp(BIGNUM *r, const BIGNUM *a, const BIGNUM *p, const BIGNUM *m,
if (BN_is_odd(m)) {
# ifdef MONT_EXP_WORD
if (a->top == 1 && !a->neg
&& (BN_get_flags(p, BN_FLG_CONSTTIME) == 0)) {
&& (BN_get_flags(p, BN_FLG_CONSTTIME) == 0)
&& (BN_get_flags(a, BN_FLG_CONSTTIME) == 0)
&& (BN_get_flags(m, BN_FLG_CONSTTIME) == 0)) {
BN_ULONG A = a->d[0];
ret = BN_mod_exp_mont_word(r, A, p, m, ctx, NULL);
} else
Expand Down Expand Up @@ -165,7 +168,9 @@ int BN_mod_exp_recp(BIGNUM *r, const BIGNUM *a, const BIGNUM *p,
BIGNUM *val[TABLE_SIZE];
BN_RECP_CTX recp;

if (BN_get_flags(p, BN_FLG_CONSTTIME) != 0) {
if (BN_get_flags(p, BN_FLG_CONSTTIME) != 0
|| BN_get_flags(a, BN_FLG_CONSTTIME) != 0
|| BN_get_flags(m, BN_FLG_CONSTTIME) != 0) {
/* BN_FLG_CONSTTIME only supported by BN_mod_exp_mont() */
BNerr(BN_F_BN_MOD_EXP_RECP, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
return 0;
Expand Down Expand Up @@ -299,7 +304,9 @@ int BN_mod_exp_mont(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
BIGNUM *val[TABLE_SIZE];
BN_MONT_CTX *mont = NULL;

if (BN_get_flags(p, BN_FLG_CONSTTIME) != 0) {
if (BN_get_flags(p, BN_FLG_CONSTTIME) != 0
|| BN_get_flags(a, BN_FLG_CONSTTIME) != 0
|| BN_get_flags(m, BN_FLG_CONSTTIME) != 0) {
return BN_mod_exp_mont_consttime(rr, a, p, m, ctx, in_mont);
}

Expand Down Expand Up @@ -1111,7 +1118,8 @@ int BN_mod_exp_mont_word(BIGNUM *rr, BN_ULONG a, const BIGNUM *p,
#define BN_TO_MONTGOMERY_WORD(r, w, mont) \
(BN_set_word(r, (w)) && BN_to_montgomery(r, r, (mont), ctx))

if (BN_get_flags(p, BN_FLG_CONSTTIME) != 0) {
if (BN_get_flags(p, BN_FLG_CONSTTIME) != 0
|| BN_get_flags(m, BN_FLG_CONSTTIME) != 0) {
/* BN_FLG_CONSTTIME only supported by BN_mod_exp_mont() */
BNerr(BN_F_BN_MOD_EXP_MONT_WORD, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
return 0;
Expand Down Expand Up @@ -1241,7 +1249,9 @@ int BN_mod_exp_simple(BIGNUM *r, const BIGNUM *a, const BIGNUM *p,
/* Table of variables obtained from 'ctx' */
BIGNUM *val[TABLE_SIZE];

if (BN_get_flags(p, BN_FLG_CONSTTIME) != 0) {
if (BN_get_flags(p, BN_FLG_CONSTTIME) != 0
|| BN_get_flags(a, BN_FLG_CONSTTIME) != 0
|| BN_get_flags(m, BN_FLG_CONSTTIME) != 0) {
/* BN_FLG_CONSTTIME only supported by BN_mod_exp_mont() */
BNerr(BN_F_BN_MOD_EXP_SIMPLE, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
return 0;
Expand Down

0 comments on commit e913d11

Please sign in to comment.