Merge pull request #221 from webmaster128/bitint

Fix BigInt random_integer() distribution issue

4 files changed, 222 insertions, 19 deletions

diff --git a/src/lib/math/bigint/big_rand.cpp b/src/lib/math/bigint/big_rand.cpp
index ab66c6cdd..cfc1facee 100644
--- a/src/lib/math/bigint/big_rand.cpp
+++ b/src/lib/math/bigint/big_rand.cpp
@@ -7,6 +7,7 @@
 
 #include <botan/bigint.h>
 #include <botan/parsing.h>
+#include <botan/internal/rounding.h>
 
 namespace Botan {
 
@@ -14,20 +15,27 @@ namespace Botan {
 * Randomize this number
 */
 void BigInt::randomize(RandomNumberGenerator& rng,
-                       size_t bitsize)
+                       size_t bitsize, bool set_high_bit)
    {
    set_sign(Positive);
 
    if(bitsize == 0)
+      {
       clear();
+      }
    else
       {
-      secure_vector<byte> array = rng.random_vec((bitsize + 7) / 8);
+      secure_vector<byte> array = rng.random_vec(round_up(bitsize, 8) / 8);
 
+      // Always cut unwanted bits
       if(bitsize % 8)
          array[0] &= 0xFF >> (8 - (bitsize % 8));
-      array[0] |= 0x80 >> ((bitsize % 8) ? (8 - bitsize % 8) : 0);
-      binary_decode(array.data(), array.size());
+
+      // Set the highest bit if wanted
+      if (set_high_bit)
+         array[0] |= 0x80 >> ((bitsize % 8) ? (8 - bitsize % 8) : 0);
+
+      binary_decode(array);
       }
    }
 
@@ -37,12 +45,19 @@ void BigInt::randomize(RandomNumberGenerator& rng,
 BigInt BigInt::random_integer(RandomNumberGenerator& rng,
                               const BigInt& min, const BigInt& max)
    {
-   BigInt range = max - min;
+   BigInt delta_upper_bound = max - min - 1;
 
-   if(range <= 0)
+   if(delta_upper_bound < 0)
       throw Invalid_Argument("random_integer: invalid min/max values");
 
-   return (min + (BigInt(rng, range.bits() + 2) % range));
+   // Choose x in [0, delta_upper_bound]
+   BigInt x;
+   do {
+      auto bitsize = delta_upper_bound.bits();
+      x.randomize(rng, bitsize, false);
+   } while(x > delta_upper_bound);
+
+   return min + x;
    }
 
 }
diff --git a/src/lib/math/bigint/bigint.cpp b/src/lib/math/bigint/bigint.cpp
index 045117cbd..0a068c53e 100644
--- a/src/lib/math/bigint/bigint.cpp
+++ b/src/lib/math/bigint/bigint.cpp
@@ -87,9 +87,9 @@ BigInt::BigInt(const byte input[], size_t length, Base base)
 /*
 * Construct a BigInt from an encoded BigInt
 */
-BigInt::BigInt(RandomNumberGenerator& rng, size_t bits)
+BigInt::BigInt(RandomNumberGenerator& rng, size_t bits, bool set_high_bit)
    {
-   randomize(rng, bits);
+   randomize(rng, bits, set_high_bit);
    }
 
 /*
@@ -173,6 +173,11 @@ void BigInt::clear_bit(size_t n)
       m_reg[which] &= ~mask;
    }
 
+size_t BigInt::bytes() const
+   {
+   return round_up(bits(), 8) / 8;
+   }
+
 /*
 * Count how many bits are being used
 */
@@ -253,6 +258,12 @@ BigInt BigInt::abs() const
    return x;
    }
 
+void BigInt::grow_to(size_t n)
+   {
+   if(n > size())
+      m_reg.resize(round_up(n, 8));
+   }
+
 /*
 * Encode this number into bytes
 */
diff --git a/src/lib/math/bigint/bigint.h b/src/lib/math/bigint/bigint.h
index 4a37425f5..776884ad9 100644
--- a/src/lib/math/bigint/bigint.h
+++ b/src/lib/math/bigint/bigint.h
@@ -42,7 +42,7 @@ class BOTAN_DLL BigInt
      /**
      * Create empty BigInt
      */
-     BigInt() { m_signedness = Positive; }
+     BigInt() {}
 
      /**
      * Create BigInt from 64 bit integer
@@ -74,11 +74,15 @@ class BOTAN_DLL BigInt
      BigInt(const byte buf[], size_t length, Base base = Binary);
 
      /**
-     * Create a random BigInt of the specified size
+     * \brief Create a random BigInt of the specified size
+     *
      * @param rng random number generator
      * @param bits size in bits
+     * @param set_high_bit if true, the highest bit is always set
+     *
+     * @see randomize
      */
-     BigInt(RandomNumberGenerator& rng, size_t bits);
+     BigInt(RandomNumberGenerator& rng, size_t bits, bool set_high_bit = true);
 
      /**
      * Create BigInt of specified size, all zeros
@@ -400,7 +404,7 @@ class BOTAN_DLL BigInt
      * Give byte length of the integer
      * @result byte length of the represented integer value
      */
-     size_t bytes() const { return (bits() + 7) / 8; }
+     size_t bytes() const;
 
      /**
      * Get the bit length of the integer
@@ -427,18 +431,20 @@ class BOTAN_DLL BigInt
      * Increase internal register buffer to at least n words
      * @param n new size of register
      */
-     void grow_to(size_t n)
-        {
-        if(n > size())
-           m_reg.resize(n + (8 - n % 8));
-        }
+     void grow_to(size_t n);
 
      /**
      * Fill BigInt with a random number with size of bitsize
+     *
+     * If \p set_high_bit is true, the highest bit will be set, which causes
+     * the entropy to be \a bits-1. Otherwise the highest bit is randomly choosen
+     * by the rng, causing the entropy to be \a bits.
+     *
      * @param rng the random number generator to use
      * @param bitsize number of bits the created random value should have
+     * @param set_high_bit if true, the highest bit is always set
      */
-     void randomize(RandomNumberGenerator& rng, size_t bitsize = 0);
+     void randomize(RandomNumberGenerator& rng, size_t bitsize, bool set_high_bit = true);
 
      /**
      * Store BigInt-value in a given byte array
diff --git a/src/tests/catchy/test_bigint.cpp b/src/tests/catchy/test_bigint.cpp
new file mode 100644
index 000000000..9bd99e919
--- /dev/null
+++ b/src/tests/catchy/test_bigint.cpp
@@ -0,0 +1,171 @@
+// (C) 2015 Simon Warta (Kullo GmbH)
+// Botan is released under the Simplified BSD License (see license.txt)
+
+#include "catch.hpp"
+
+#include <botan/build.h>
+
+#if defined(BOTAN_HAS_BIGINT)
+
+#include <botan/bigint.h>
+
+using namespace Botan;
+
+TEST_CASE("Bigint basics", "[bigint]")
+   {
+   SECTION("in 0-bit border")
+      {
+      BigInt a(0u);
+      CHECK(( a.bits() == 0 ));
+      CHECK(( a.bytes() == 0 ));
+      CHECK(( a.to_u32bit() == 0 ));
+      }
+   SECTION("above 0-bit border")
+      {
+      BigInt a(1u);
+      CHECK(( a.bits() == 1 ));
+      CHECK(( a.bytes() == 1 ));
+      CHECK(( a.to_u32bit() == 1 ));
+      }
+   SECTION("in 8-bit border")
+      {
+      BigInt a(255u);
+      CHECK(( a.bits() == 8 ));
+      CHECK(( a.bytes() == 1 ));
+      CHECK(( a.to_u32bit() == 255 ));
+      }
+   SECTION("above 8-bit border")
+      {
+      BigInt a(256u);
+      CHECK(( a.bits() == 9 ));
+      CHECK(( a.bytes() == 2 ));
+      CHECK(( a.to_u32bit() == 256 ));
+      }
+   SECTION("in 16-bit border")
+      {
+      BigInt a(65535u);
+      CHECK(( a.bits() == 16 ));
+      CHECK(( a.bytes() == 2 ));
+      CHECK(( a.to_u32bit() == 65535 ));
+      }
+   SECTION("above 16-bit border")
+      {
+      BigInt a(65536u);
+      CHECK(( a.bits() == 17 ));
+      CHECK(( a.bytes() == 3 ));
+      CHECK(( a.to_u32bit() == 65536 ));
+      }
+   SECTION("in 32-bit border")
+      {
+      BigInt a(4294967295u);
+      CHECK(( a.bits() == 32 ));
+      CHECK(( a.bytes() == 4 ));
+      CHECK(( a.to_u32bit() == 4294967295u ));
+      }
+   SECTION("above 32-bit border")
+      {
+      BigInt a(4294967296u);
+      CHECK(( a.bits() == 33 ));
+      CHECK(( a.bytes() == 5 ));
+      CHECK_THROWS( a.to_u32bit() );
+      }
+   }
+
+TEST_CASE("Bigint random_integer", "[bigint]")
+   {
+   RandomNumberGenerator *rng = RandomNumberGenerator::make_rng();
+
+   SECTION("min is 0")
+      {
+      // 0–9
+      const size_t MIN = 0;
+      const size_t MAX = 10; // excluded
+      const int ITERATIONS = 10000;
+
+      std::vector<int> counts(MAX, 0);
+      std::vector<double> ratios(MAX, 1.0);
+
+      for (size_t i = 0; i < ITERATIONS; i++)
+         {
+         BigInt b = BigInt::random_integer(*rng, MIN, MAX);
+         size_t x = b.to_u32bit();
+         counts[x]++;
+         }
+
+      std::stringstream debug;
+      for (size_t d = MIN; d < MAX; ++d)
+         {
+         auto ratio = static_cast<double>(counts[d]) / ITERATIONS;
+         ratios[d] = ratio;
+
+         if (!debug.str().empty())
+            {
+            debug << ", ";
+            }
+         debug << d << ": " << std::setprecision(3) << ratio;
+         }
+
+      INFO( debug.str() )
+
+      // Have ~ 10 % on each digit from 0-9
+      CHECK(( 0.09 < ratios[0] )); CHECK(( ratios[0] < 0.11 ));
+      CHECK(( 0.09 < ratios[1] )); CHECK(( ratios[1] < 0.11 ));
+      CHECK(( 0.09 < ratios[2] )); CHECK(( ratios[2] < 0.11 ));
+      CHECK(( 0.09 < ratios[3] )); CHECK(( ratios[3] < 0.11 ));
+      CHECK(( 0.09 < ratios[4] )); CHECK(( ratios[4] < 0.11 ));
+      CHECK(( 0.09 < ratios[5] )); CHECK(( ratios[5] < 0.11 ));
+      CHECK(( 0.09 < ratios[6] )); CHECK(( ratios[6] < 0.11 ));
+      CHECK(( 0.09 < ratios[7] )); CHECK(( ratios[7] < 0.11 ));
+      CHECK(( 0.09 < ratios[8] )); CHECK(( ratios[8] < 0.11 ));
+      CHECK(( 0.09 < ratios[9] )); CHECK(( ratios[9] < 0.11 ));
+      //CHECK( false );
+      }
+
+   SECTION("min is 10")
+      {
+      // 10–19
+      const size_t MIN = 10;
+      const size_t MAX = 20; // excluded
+      const size_t ITERATIONS = 10000;
+
+      std::vector<int> counts(MAX, 0);
+      std::vector<double> ratios(MAX, 1.0);
+
+      for (size_t i = 0; i < ITERATIONS; i++)
+         {
+         BigInt b = BigInt::random_integer(*rng, MIN, MAX);
+         size_t x = b.to_u32bit();
+         counts[x]++;
+         }
+
+      std::stringstream debug;
+      for (size_t d = MIN; d < MAX; ++d)
+         {
+         auto ratio = static_cast<double>(counts[d]) / ITERATIONS;
+         ratios[d] = ratio;
+
+         if (!debug.str().empty())
+            {
+            debug << ", ";
+            }
+         debug << d << ": " << std::setprecision(3) << ratio;
+         }
+
+      INFO( debug.str() )
+
+      // Have ~ 10 % on each digit from 10-19
+      CHECK(( 0.09 < ratios[10] )); CHECK(( ratios[10] < 0.11 ));
+      CHECK(( 0.09 < ratios[11] )); CHECK(( ratios[11] < 0.11 ));
+      CHECK(( 0.09 < ratios[12] )); CHECK(( ratios[12] < 0.11 ));
+      CHECK(( 0.09 < ratios[13] )); CHECK(( ratios[13] < 0.11 ));
+      CHECK(( 0.09 < ratios[14] )); CHECK(( ratios[14] < 0.11 ));
+      CHECK(( 0.09 < ratios[15] )); CHECK(( ratios[15] < 0.11 ));
+      CHECK(( 0.09 < ratios[16] )); CHECK(( ratios[16] < 0.11 ));
+      CHECK(( 0.09 < ratios[17] )); CHECK(( ratios[17] < 0.11 ));
+      CHECK(( 0.09 < ratios[18] )); CHECK(( ratios[18] < 0.11 ));
+      CHECK(( 0.09 < ratios[19] )); CHECK(( ratios[19] < 0.11 ));
+      //CHECK( false );
+      }
+   }
+
+#endif