diff options
author | Matt Caswell <matt@openssl.org> | 2018-01-15 12:23:07 +0100 |
---|---|---|
committer | Matt Caswell <matt@openssl.org> | 2018-01-16 16:19:01 +0100 |
commit | 7d461736f7bd3af3c2f266f8541034ecf6f41ed9 (patch) | |
tree | cf355e014da7ec90f113e99d5f1dbcaf36f066f3 /crypto/bn | |
parent | Fix memory leak in do_rand_drbg_init() (diff) | |
download | openssl-7d461736f7bd3af3c2f266f8541034ecf6f41ed9.tar.xz openssl-7d461736f7bd3af3c2f266f8541034ecf6f41ed9.zip |
Revert BN_copy() flag copy semantics change
Commit 9f9442918a changed the semantics of BN_copy() to additionally
copy the BN_FLG_CONSTTIME flag if it is set. This turns out to be
ill advised as it has unintended consequences. For example calling
BN_mod_inverse_no_branch() can sometimes return a result with the flag
set and sometimes not as a result. This can lead to later failures if we
go down code branches that do not support constant time, but check for
the presence of the flag.
The original commit was made due to an issue in BN_MOD_CTX_set(). The
original PR fixed the problem in that function, but it was changed in
review to fix it in BN_copy() instead. The solution seems to be to revert
the BN_copy() change and go back to the originally proposed way.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/5080)
Diffstat (limited to 'crypto/bn')
-rw-r--r-- | crypto/bn/bn_lib.c | 3 | ||||
-rw-r--r-- | crypto/bn/bn_mont.c | 2 |
2 files changed, 2 insertions, 3 deletions
diff --git a/crypto/bn/bn_lib.c b/crypto/bn/bn_lib.c index dd79f94502..fee4063b92 100644 --- a/crypto/bn/bn_lib.c +++ b/crypto/bn/bn_lib.c @@ -321,9 +321,6 @@ BIGNUM *BN_copy(BIGNUM *a, const BIGNUM *b) if (b->top > 0) memcpy(a->d, b->d, sizeof(b->d[0]) * b->top); - if (BN_get_flags(b, BN_FLG_CONSTTIME) != 0) - BN_set_flags(a, BN_FLG_CONSTTIME); - a->top = b->top; a->neg = b->neg; bn_check_top(a); diff --git a/crypto/bn/bn_mont.c b/crypto/bn/bn_mont.c index b073a41089..adda238b6a 100644 --- a/crypto/bn/bn_mont.c +++ b/crypto/bn/bn_mont.c @@ -258,6 +258,8 @@ int BN_MONT_CTX_set(BN_MONT_CTX *mont, const BIGNUM *mod, BN_CTX *ctx) R = &(mont->RR); /* grab RR as a temp */ if (!BN_copy(&(mont->N), mod)) goto err; /* Set N */ + if (BN_get_flags(mod, BN_FLG_CONSTTIME) != 0) + BN_set_flags(&(mont->N), BN_FLG_CONSTTIME); mont->N.neg = 0; #ifdef MONT_WORD |