summaryrefslogtreecommitdiffstats
path: root/crypto/rsa/rsa_crpt.c
diff options
context:
space:
mode:
authorMatt Caswell <matt@openssl.org>2015-11-24 11:09:00 +0000
committerMatt Caswell <matt@openssl.org>2015-11-26 10:20:36 +0000
commitfd7d252060c427b2e567295845a61d824539443b (patch)
tree42197df9569504eb2512394e59bde093c18e94a0 /crypto/rsa/rsa_crpt.c
parent6938c954b072c1ddddeb0ec9f6a151df0d2cd925 (diff)
Tighten up BN_with_flags usage and avoid a reachable assert
The function rsa_ossl_mod_exp uses the function BN_with_flags to create a temporary copy (local_r1) of a BIGNUM (r1) with modified flags. This temporary copy shares some state with the original r1. If the state of r1 gets updated then local_r1's state will be stale. This was occurring in the function so that when local_r1 was freed a call to bn_check_top was made which failed an assert due to the stale state. To resolve this we must free local_r1 immediately after we have finished using it and not wait until the end of the function. This problem prompted a review of all BN_with_flag usage within the codebase. All other usage appears to be correct, although often not obviously so. This commit refactors things to make it much clearer for these other uses. Reviewed-by: Emilia Käsper <emilia@openssl.org>
Diffstat (limited to 'crypto/rsa/rsa_crpt.c')
-rw-r--r--crypto/rsa/rsa_crpt.c32
1 files changed, 18 insertions, 14 deletions
diff --git a/crypto/rsa/rsa_crpt.c b/crypto/rsa/rsa_crpt.c
index 4df1662320..d08258e5e7 100644
--- a/crypto/rsa/rsa_crpt.c
+++ b/crypto/rsa/rsa_crpt.c
@@ -159,8 +159,7 @@ static BIGNUM *rsa_get_public_exp(const BIGNUM *d, const BIGNUM *p,
BN_BLINDING *RSA_setup_blinding(RSA *rsa, BN_CTX *in_ctx)
{
- BIGNUM *local_n = NULL;
- BIGNUM *e, *n;
+ BIGNUM *e;
BN_CTX *ctx;
BN_BLINDING *ret = NULL;
@@ -196,19 +195,25 @@ BN_BLINDING *RSA_setup_blinding(RSA *rsa, BN_CTX *in_ctx)
0.0);
}
- if (!(rsa->flags & RSA_FLAG_NO_CONSTTIME)) {
- /* Set BN_FLG_CONSTTIME flag */
- local_n = n = BN_new();
- if (local_n == NULL) {
- RSAerr(RSA_F_RSA_SETUP_BLINDING, ERR_R_MALLOC_FAILURE);
- goto err;
+ {
+ BIGNUM *local_n = NULL, *n;
+ if (!(rsa->flags & RSA_FLAG_NO_CONSTTIME)) {
+ /* Set BN_FLG_CONSTTIME flag */
+ local_n = n = BN_new();
+ if (local_n == NULL) {
+ RSAerr(RSA_F_RSA_SETUP_BLINDING, ERR_R_MALLOC_FAILURE);
+ goto err;
+ }
+ BN_with_flags(n, rsa->n, BN_FLG_CONSTTIME);
+ } else {
+ n = rsa->n;
}
- BN_with_flags(n, rsa->n, BN_FLG_CONSTTIME);
- } else
- n = rsa->n;
- ret = BN_BLINDING_create_param(NULL, e, n, ctx,
- rsa->meth->bn_mod_exp, rsa->_method_mod_n);
+ ret = BN_BLINDING_create_param(NULL, e, n, ctx, rsa->meth->bn_mod_exp,
+ rsa->_method_mod_n);
+ /* We MUST free local_n before any further use of rsa->n */
+ BN_free(local_n);
+ }
if (ret == NULL) {
RSAerr(RSA_F_RSA_SETUP_BLINDING, ERR_R_BN_LIB);
goto err;
@@ -220,7 +225,6 @@ BN_BLINDING *RSA_setup_blinding(RSA *rsa, BN_CTX *in_ctx)
BN_CTX_free(ctx);
if (e != rsa->e)
BN_free(e);
- BN_free(local_n);
return ret;
}