Stop using |ERR_peek_last_error| in RSA blinding.

History has shown there are bugs in not setting the error code
appropriately, which makes any decision making based on
|ERR_peek_last_error|, etc. suspect. Also, this call was interfering
with the link-time optimizer's ability to discard the implementations of
many functions in crypto/err during dead code elimination.

Change-Id: Iba9e553bf0a72a1370ceb17ff275f5a20fca31ec
Reviewed-on: https://boringssl-review.googlesource.com/5748
Reviewed-by: Adam Langley <agl@google.com>
This commit is contained in:
Adam Langley 2015-09-08 16:31:33 -07:00
parent 8f1c268692
commit 06fa67c8d3
3 changed files with 33 additions and 10 deletions

View File

@ -223,20 +223,23 @@ err:
} }
/* solves ax == 1 (mod n) */ /* solves ax == 1 (mod n) */
static BIGNUM *BN_mod_inverse_no_branch(BIGNUM *out, const BIGNUM *a, static BIGNUM *BN_mod_inverse_no_branch(BIGNUM *out, int *out_no_inverse,
const BIGNUM *n, BN_CTX *ctx); const BIGNUM *a, const BIGNUM *n,
BN_CTX *ctx);
BIGNUM *BN_mod_inverse(BIGNUM *out, const BIGNUM *a, const BIGNUM *n, BIGNUM *BN_mod_inverse_ex(BIGNUM *out, int *out_no_inverse, const BIGNUM *a,
BN_CTX *ctx) { const BIGNUM *n, BN_CTX *ctx) {
BIGNUM *A, *B, *X, *Y, *M, *D, *T, *R = NULL; BIGNUM *A, *B, *X, *Y, *M, *D, *T, *R = NULL;
BIGNUM *ret = NULL; BIGNUM *ret = NULL;
int sign; int sign;
if ((a->flags & BN_FLG_CONSTTIME) != 0 || if ((a->flags & BN_FLG_CONSTTIME) != 0 ||
(n->flags & BN_FLG_CONSTTIME) != 0) { (n->flags & BN_FLG_CONSTTIME) != 0) {
return BN_mod_inverse_no_branch(out, a, n, ctx); return BN_mod_inverse_no_branch(out, out_no_inverse, a, n, ctx);
} }
*out_no_inverse = 0;
BN_CTX_start(ctx); BN_CTX_start(ctx);
A = BN_CTX_get(ctx); A = BN_CTX_get(ctx);
B = BN_CTX_get(ctx); B = BN_CTX_get(ctx);
@ -522,6 +525,7 @@ BIGNUM *BN_mod_inverse(BIGNUM *out, const BIGNUM *a, const BIGNUM *n,
} }
} }
} else { } else {
*out_no_inverse = 1;
OPENSSL_PUT_ERROR(BN, BN_R_NO_INVERSE); OPENSSL_PUT_ERROR(BN, BN_R_NO_INVERSE);
goto err; goto err;
} }
@ -535,16 +539,25 @@ err:
return ret; return ret;
} }
BIGNUM *BN_mod_inverse(BIGNUM *out, const BIGNUM *a, const BIGNUM *n,
BN_CTX *ctx) {
int no_inverse;
return BN_mod_inverse_ex(out, &no_inverse, a, n, ctx);
}
/* BN_mod_inverse_no_branch is a special version of BN_mod_inverse. /* BN_mod_inverse_no_branch is a special version of BN_mod_inverse.
* It does not contain branches that may leak sensitive information. */ * It does not contain branches that may leak sensitive information. */
static BIGNUM *BN_mod_inverse_no_branch(BIGNUM *out, const BIGNUM *a, static BIGNUM *BN_mod_inverse_no_branch(BIGNUM *out, int *out_no_inverse,
const BIGNUM *n, BN_CTX *ctx) { const BIGNUM *a, const BIGNUM *n,
BN_CTX *ctx) {
BIGNUM *A, *B, *X, *Y, *M, *D, *T, *R = NULL; BIGNUM *A, *B, *X, *Y, *M, *D, *T, *R = NULL;
BIGNUM local_A, local_B; BIGNUM local_A, local_B;
BIGNUM *pA, *pB; BIGNUM *pA, *pB;
BIGNUM *ret = NULL; BIGNUM *ret = NULL;
int sign; int sign;
*out_no_inverse = 0;
BN_CTX_start(ctx); BN_CTX_start(ctx);
A = BN_CTX_get(ctx); A = BN_CTX_get(ctx);
B = BN_CTX_get(ctx); B = BN_CTX_get(ctx);
@ -682,6 +695,7 @@ static BIGNUM *BN_mod_inverse_no_branch(BIGNUM *out, const BIGNUM *a,
} }
} }
} else { } else {
*out_no_inverse = 1;
OPENSSL_PUT_ERROR(BN, BN_R_NO_INVERSE); OPENSSL_PUT_ERROR(BN, BN_R_NO_INVERSE);
goto err; goto err;
} }

View File

@ -325,10 +325,11 @@ BN_BLINDING *BN_BLINDING_create_param(
if (!BN_rand_range(ret->A, ret->mod)) { if (!BN_rand_range(ret->A, ret->mod)) {
goto err; goto err;
} }
if (BN_mod_inverse(ret->Ai, ret->A, ret->mod, ctx) == NULL) {
int no_inverse;
if (BN_mod_inverse_ex(ret->Ai, &no_inverse, ret->A, ret->mod, ctx) == NULL) {
/* this should almost never happen for good RSA keys */ /* this should almost never happen for good RSA keys */
uint32_t error = ERR_peek_last_error(); if (no_inverse) {
if (ERR_GET_REASON(error) == BN_R_NO_INVERSE) {
if (retry_counter-- == 0) { if (retry_counter-- == 0) {
OPENSSL_PUT_ERROR(RSA, RSA_R_TOO_MANY_ITERATIONS); OPENSSL_PUT_ERROR(RSA, RSA_R_TOO_MANY_ITERATIONS);
goto err; goto err;

View File

@ -706,6 +706,14 @@ OPENSSL_EXPORT int BN_gcd(BIGNUM *r, const BIGNUM *a, const BIGNUM *b,
OPENSSL_EXPORT BIGNUM *BN_mod_inverse(BIGNUM *out, const BIGNUM *a, OPENSSL_EXPORT BIGNUM *BN_mod_inverse(BIGNUM *out, const BIGNUM *a,
const BIGNUM *n, BN_CTX *ctx); const BIGNUM *n, BN_CTX *ctx);
/* BN_mod_inverse_ex acts like |BN_mod_inverse| except that, when it returns
* zero, it will set |*out_no_inverse| to one if the failure was caused because
* |a| has no inverse mod |n|. Otherwise it will set |*out_no_inverse| to
* zero. */
OPENSSL_EXPORT BIGNUM *BN_mod_inverse_ex(BIGNUM *out, int *out_no_inverse,
const BIGNUM *a, const BIGNUM *n,
BN_CTX *ctx);
/* BN_kronecker returns the Kronecker symbol of |a| and |b| (which is -1, 0 or /* BN_kronecker returns the Kronecker symbol of |a| and |b| (which is -1, 0 or
* 1), or -2 on error. */ * 1), or -2 on error. */
OPENSSL_EXPORT int BN_kronecker(const BIGNUM *a, const BIGNUM *b, BN_CTX *ctx); OPENSSL_EXPORT int BN_kronecker(const BIGNUM *a, const BIGNUM *b, BN_CTX *ctx);