Refactor BN_rand_range to reduce code duplication.

Besides reducing code duplication, also move the relative location of
the check of |count|. Previously, the code was generating a random
value and then terminating the loop without using it if |count| went
to zero. Now the wasted call to |BN_rand| is not made.

Also add a note about the applicability of the special case logic for
|range| of the form |0b100...| to RSA blinding.

Change-Id: Iaa33b9529f1665ac59aefcc8b371fa32445e7578
Reviewed-on: https://boringssl-review.googlesource.com/8960
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

1 files changed, 16 insertions, 19 deletions

diff --git a/crypto/bn/random.c b/crypto/bn/random.c
index 3116e547..83334306 100644
--- a/crypto/bn/random.c
+++ b/crypto/bn/random.c
@@ -195,10 +195,19 @@ int BN_rand_range(BIGNUM *r, const BIGNUM *range) {
   /* BN_is_bit_set(range, n - 1) always holds */
   if (n == 1) {
     BN_zero(r);
-  } else if (!BN_is_bit_set(range, n - 2) && !BN_is_bit_set(range, n - 3)) {
-    /* range = 100..._2,
-     * so  3*range (= 11..._2)  is exactly one bit longer than  range */
-    do {
+    return 1;
+  }
+
+  do {
+    if (!--count) {
+      OPENSSL_PUT_ERROR(BN, BN_R_TOO_MANY_ITERATIONS);
+      return 0;
+    }
+
+    if (!BN_is_bit_set(range, n - 2) && !BN_is_bit_set(range, n - 3)) {
+      /* range = 100..._2, so 3*range (= 11..._2) is exactly one bit longer
+       * than range. This is a common scenario when generating a random value
+       * modulo an RSA public modulus, e.g. for RSA base blinding. */
       if (!BN_rand(r, n + 1, -1 /* don't set most significant bits */,
                    0 /* don't set least significant bits */)) {
         return 0;
@@ -217,25 +226,13 @@ int BN_rand_range(BIGNUM *r, const BIGNUM *range) {
           }
         }
       }
-
-      if (!--count) {
-        OPENSSL_PUT_ERROR(BN, BN_R_TOO_MANY_ITERATIONS);
-        return 0;
-      }
-    } while (BN_cmp(r, range) >= 0);
-  } else {
-    do {
+    } else {
       /* range = 11..._2  or  range = 101..._2 */
       if (!BN_rand(r, n, -1, 0)) {
         return 0;
       }
-
-      if (!--count) {
-        OPENSSL_PUT_ERROR(BN, BN_R_TOO_MANY_ITERATIONS);
-        return 0;
-      }
-    } while (BN_cmp(r, range) >= 0);
-  }
+    }
+  } while (BN_cmp(r, range) >= 0);
 
   return 1;
 }