Embrace variable time behavior in vartime_divide

Historically this function made some attempts to be constant-time-ish (despite
not being constant time at all), largely because it was the only implemented
division algorithm. Now we have dedicated constant time division algorithms
which are used for secret data; we can readily assume the inputs to
vartime_divide are public.

Author: randombit

src/lib/math/bigint/divide.cpp

@@ -27,7 +27,7 @@ void sign_fixup(const BigInt& x, const BigInt& y, BigInt& q, BigInt& r) {
    }
 }
 
-inline bool division_check(word q, word y2, word y1, word x3, word x2, word x1) {
+inline bool division_check_vartime(word q, word y2, word y1, word x3, word x2, word x1) {
    /*
    Compute (y3,y2,y1) = (y2,y1) * q
    and return true if (y3,y2,y1) > (x3,x2,x1)
@@ -37,10 +37,13 @@ inline bool division_check(word q, word y2, word y1, word x3, word x2, word x1)
    y1 = word_madd2(q, y1, &y3);
    y2 = word_madd2(q, y2, &y3);
 
-   const word x[3] = {x1, x2, x3};
-   const word y[3] = {y1, y2, y3};
-
-   return bigint_ct_is_lt(x, 3, y, 3).as_bool();
+   if(x3 != y3) {
+      return (y3 > x3);
+   }
+   if(x2 != y2) {
+      return (y2 > x2);
+   }
+   return (y1 > x1);
 }
 
 }  // namespace
@@ -245,8 +248,10 @@ void vartime_divide(const BigInt& x, const BigInt& y_arg, BigInt& q_out, BigInt&
    // Calculate shifts needed to normalize y with high bit set
    const size_t shifts = y.top_bits_free();
 
-   y <<= shifts;
-   r <<= shifts;
+   if(shifts > 0) {
+      y <<= shifts;
+      r <<= shifts;
+   }
 
    // we know y has not changed size, since we only shifted up to set high bit
    const size_t t = y_words - 1;
@@ -272,27 +277,30 @@ void vartime_divide(const BigInt& x, const BigInt& y_arg, BigInt& q_out, BigInt&
       const word x_j1 = r.word_at(j - 1);
       const word x_j2 = r.word_at(j - 2);
 
-      word qjt = bigint_divop_vartime(x_j0, x_j1, y_t0);
-
-      qjt = CT::Mask<word>::is_equal(x_j0, y_t0).select(WordInfo<word>::max, qjt);
+      word qjt = (x_j0 == y_t0) ? WordInfo<word>::max : bigint_divop_vartime(x_j0, x_j1, y_t0);
 
       // Per HAC 14.23, this operation is required at most twice
-      qjt -= division_check(qjt, y_t0, y_t1, x_j0, x_j1, x_j2);
-      qjt -= division_check(qjt, y_t0, y_t1, x_j0, x_j1, x_j2);
-      BOTAN_DEBUG_ASSERT(division_check(qjt, y_t0, y_t1, x_j0, x_j1, x_j2) == false);
+      while(division_check_vartime(qjt, y_t0, y_t1, x_j0, x_j1, x_j2)) {
+         qjt--;
+      }
 
       shifted_y >>= WordInfo<word>::bits;
       // Now shifted_y == y << (WordInfo<word>::bits * (j-t-1))
 
       // TODO this sequence could be better
       r -= qjt * shifted_y;
-      qjt -= r.is_negative();
-      r += static_cast<word>(r.is_negative()) * shifted_y;
+      if(r.is_negative()) {
+         qjt--;
+         r += shifted_y;
+         BOTAN_DEBUG_ASSERT(r.is_positive());
+      }
 
       q_words[j - t - 1] = qjt;
    }
 
-   r >>= shifts;
+   if(shifts > 0) {
+      r >>= shifts;
+   }
 
    sign_fixup(x, y_arg, q, r);