commit 06fa67c8d3b2e68df45b34b315dd26193f45645a
author Adam Langley <agl@google.com> Tue Sep 08 16:31:33 2015 -0700
committer Adam Langley <agl@google.com> Tue Sep 08 23:32:48 2015 +0000
tree 07ef64d0a29459db89b6a7a0b63e53e4144b5c27
parent 8f1c268692d39995245898676ec4fc72e385806f

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>

crypto/bn/gcd.c

diff --git a/crypto/bn/gcd.c b/crypto/bn/gcd.c
index c33a3cd..e106149 100644
--- a/crypto/bn/gcd.c
+++ b/crypto/bn/gcd.c
@@ -223,20 +223,23 @@
 }
 
 /* solves ax == 1 (mod n) */
-static BIGNUM *BN_mod_inverse_no_branch(BIGNUM *out, const BIGNUM *a,
-                                        const BIGNUM *n, BN_CTX *ctx);
+static BIGNUM *BN_mod_inverse_no_branch(BIGNUM *out, int *out_no_inverse,
+                                        const BIGNUM *a, const BIGNUM *n,
+                                        BN_CTX *ctx);
 
-BIGNUM *BN_mod_inverse(BIGNUM *out, const BIGNUM *a, const BIGNUM *n,
-                       BN_CTX *ctx) {
+BIGNUM *BN_mod_inverse_ex(BIGNUM *out, int *out_no_inverse, const BIGNUM *a,
+                          const BIGNUM *n, BN_CTX *ctx) {
   BIGNUM *A, *B, *X, *Y, *M, *D, *T, *R = NULL;
   BIGNUM *ret = NULL;
   int sign;
 
   if ((a->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);
   A = BN_CTX_get(ctx);
   B = BN_CTX_get(ctx);
@@ -522,6 +525,7 @@
       }
     }
   } else {
+    *out_no_inverse = 1;
     OPENSSL_PUT_ERROR(BN, BN_R_NO_INVERSE);
     goto err;
   }
@@ -535,16 +539,25 @@
   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.
  * It does not contain branches that may leak sensitive information. */
-static BIGNUM *BN_mod_inverse_no_branch(BIGNUM *out, const BIGNUM *a,
-                                        const BIGNUM *n, BN_CTX *ctx) {
+static BIGNUM *BN_mod_inverse_no_branch(BIGNUM *out, int *out_no_inverse,
+                                        const BIGNUM *a, const BIGNUM *n,
+                                        BN_CTX *ctx) {
   BIGNUM *A, *B, *X, *Y, *M, *D, *T, *R = NULL;
   BIGNUM local_A, local_B;
   BIGNUM *pA, *pB;
   BIGNUM *ret = NULL;
   int sign;
 
+  *out_no_inverse = 0;
+
   BN_CTX_start(ctx);
   A = BN_CTX_get(ctx);
   B = BN_CTX_get(ctx);
@@ -682,6 +695,7 @@
       }
     }
   } else {
+    *out_no_inverse = 1;
     OPENSSL_PUT_ERROR(BN, BN_R_NO_INVERSE);
     goto err;
   }

crypto/rsa/blinding.c

diff --git a/crypto/rsa/blinding.c b/crypto/rsa/blinding.c
index 75c3422..c93cee1 100644
--- a/crypto/rsa/blinding.c
+++ b/crypto/rsa/blinding.c
@@ -325,10 +325,11 @@
     if (!BN_rand_range(ret->A, ret->mod)) {
       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 */
-      uint32_t error = ERR_peek_last_error();
-      if (ERR_GET_REASON(error) == BN_R_NO_INVERSE) {
+      if (no_inverse) {
         if (retry_counter-- == 0) {
           OPENSSL_PUT_ERROR(RSA, RSA_R_TOO_MANY_ITERATIONS);
           goto err;

include/openssl/bn.h

diff --git a/include/openssl/bn.h b/include/openssl/bn.h
index 7edb778..0a1f5e5 100644
--- a/include/openssl/bn.h
+++ b/include/openssl/bn.h
@@ -706,6 +706,14 @@
 OPENSSL_EXPORT BIGNUM *BN_mod_inverse(BIGNUM *out, const BIGNUM *a,
                                       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
  * 1), or -2 on error. */
 OPENSSL_EXPORT int BN_kronecker(const BIGNUM *a, const BIGNUM *b, BN_CTX *ctx);