Repository: randombit/botan
Pull request #2175 (2019-10-23 17:11:08)

Add check for permitted range of reseed_interval
Adds a check and corresponding test in HMAC_DRBG whether reseed_interval is in the permitted range (Table 2 in NIST SP 800-90A, Rev. 1).

Stumbled upon this while reading https://eprint.iacr.org/2018/349.pdf.


Patch:
diff --git a/src/lib/rng/hmac_drbg/hmac_drbg.cpp b/src/lib/rng/hmac_drbg/hmac_drbg.cpp
index a01b761d9..318498d7f 100644
--- a/src/lib/rng/hmac_drbg/hmac_drbg.cpp
+++ b/src/lib/rng/hmac_drbg/hmac_drbg.cpp
@@ -10,6 +10,26 @@
 
 namespace Botan {
 
+namespace {
+
+void check_limits(size_t reseed_interval,
+                  size_t max_number_of_bytes_per_request)
+   {
+   // SP800-90A permits up to 2^48, but it is not usable on 32 bit
+   // platforms, so we only allow up to 2^24, which is still reasonably high
+   if(reseed_interval == 0 || reseed_interval > static_cast<size_t>(1) << 24)
+      {
+      throw Invalid_Argument("Invalid value for reseed_interval");
+      }
+
+   if(max_number_of_bytes_per_request == 0 || max_number_of_bytes_per_request > 64 * 1024)
+      {
+      throw Invalid_Argument("Invalid value for max_number_of_bytes_per_request");
+      }
+   }
+
+}
+
 HMAC_DRBG::HMAC_DRBG(std::unique_ptr<MessageAuthenticationCode> prf,
                      RandomNumberGenerator& underlying_rng,
                      size_t reseed_interval,
@@ -20,10 +40,7 @@ HMAC_DRBG::HMAC_DRBG(std::unique_ptr<MessageAuthenticationCode> prf,
    {
    BOTAN_ASSERT_NONNULL(m_mac);
 
-   if(m_max_number_of_bytes_per_request == 0 || m_max_number_of_bytes_per_request > 64 * 1024)
-      {
-      throw Invalid_Argument("Invalid value for max_number_of_bytes_per_request");
-      }
+   check_limits(reseed_interval, max_number_of_bytes_per_request);
 
    clear();
    }
@@ -32,17 +49,14 @@ HMAC_DRBG::HMAC_DRBG(std::unique_ptr<MessageAuthenticationCode> prf,
                      RandomNumberGenerator& underlying_rng,
                      Entropy_Sources& entropy_sources,
                      size_t reseed_interval,
-                     size_t max_number_of_bytes_per_request ) :
+                     size_t max_number_of_bytes_per_request) :
    Stateful_RNG(underlying_rng, entropy_sources, reseed_interval),
    m_mac(std::move(prf)),
    m_max_number_of_bytes_per_request(max_number_of_bytes_per_request)
    {
    BOTAN_ASSERT_NONNULL(m_mac);
 
-   if(m_max_number_of_bytes_per_request == 0 || m_max_number_of_bytes_per_request > 64 * 1024)
-      {
-      throw Invalid_Argument("Invalid value for max_number_of_bytes_per_request");
-      }
+   check_limits(reseed_interval, max_number_of_bytes_per_request);
 
    clear();
    }
@@ -57,10 +71,7 @@ HMAC_DRBG::HMAC_DRBG(std::unique_ptr<MessageAuthenticationCode> prf,
    {
    BOTAN_ASSERT_NONNULL(m_mac);
 
-   if(m_max_number_of_bytes_per_request == 0 || m_max_number_of_bytes_per_request > 64 * 1024)
-      {
-      throw Invalid_Argument("Invalid value for max_number_of_bytes_per_request");
-      }
+   check_limits(reseed_interval, max_number_of_bytes_per_request);
 
    clear();
    }
diff --git a/src/lib/rng/hmac_drbg/hmac_drbg.h b/src/lib/rng/hmac_drbg/hmac_drbg.h
index 6ead498fc..1d4c81ab3 100644
--- a/src/lib/rng/hmac_drbg/hmac_drbg.h
+++ b/src/lib/rng/hmac_drbg/hmac_drbg.h
@@ -43,7 +43,7 @@ class BOTAN_PUBLIC_API(2,0) HMAC_DRBG final : public Stateful_RNG
       * @param underlying_rng is a reference to some RNG which will be used
       * to perform the periodic reseeding
       * @param reseed_interval specifies a limit of how many times
-      * the RNG will be called before automatic reseeding is performed
+      * the RNG will be called before automatic reseeding is performed (max. 2^24)
       * @param max_number_of_bytes_per_request requests that are in size higher
       * than max_number_of_bytes_per_request are treated as if multiple single
       * requests of max_number_of_bytes_per_request size had been made.
@@ -70,7 +70,7 @@ class BOTAN_PUBLIC_API(2,0) HMAC_DRBG final : public Stateful_RNG
       * @param prf MAC to use as a PRF
       * @param entropy_sources will be polled to perform reseeding periodically
       * @param reseed_interval specifies a limit of how many times
-      * the RNG will be called before automatic reseeding is performed.
+      * the RNG will be called before automatic reseeding is performed (max. 2^24)
       * @param max_number_of_bytes_per_request requests that are in size higher
       * than max_number_of_bytes_per_request are treated as if multiple single
       * requests of max_number_of_bytes_per_request size had been made.
@@ -100,7 +100,7 @@ class BOTAN_PUBLIC_API(2,0) HMAC_DRBG final : public Stateful_RNG
       * to perform the periodic reseeding
       * @param entropy_sources will be polled to perform reseeding periodically
       * @param reseed_interval specifies a limit of how many times
-      * the RNG will be called before automatic reseeding is performed.
+      * the RNG will be called before automatic reseeding is performed (max. 2^24)
       * @param max_number_of_bytes_per_request requests that are in size higher
       * than max_number_of_bytes_per_request are treated as if multiple single
       * requests of max_number_of_bytes_per_request size had been made.


Test patch:
diff --git a/src/tests/test_rng.cpp b/src/tests/test_rng.cpp
index 3751ac5ca..1dbb0444e 100644
--- a/src/tests/test_rng.cpp
+++ b/src/tests/test_rng.cpp
@@ -55,6 +55,7 @@ class Stateful_RNG_Tests : public Test
          std::vector<Test::Result> results;
          results.push_back(test_reseed_kat());
          results.push_back(test_reseed());
+         results.push_back(test_reseed_interval_limits());
          results.push_back(test_max_number_of_bytes_per_request());
          results.push_back(test_broken_entropy_input());
          results.push_back(test_check_nonce());
@@ -107,6 +108,8 @@ class Stateful_RNG_Tests : public Test
       virtual Test::Result test_security_level() = 0;
 
       virtual Test::Result test_max_number_of_bytes_per_request() = 0;
+
+      virtual Test::Result test_reseed_interval_limits() = 0;
    private:
       Test::Result test_reseed()
          {
@@ -489,6 +492,29 @@ class HMAC_DRBG_Unit_Tests final : public Stateful_RNG_Tests
          return result;
          }
 
+      Test::Result test_reseed_interval_limits() override
+         {
+         Test::Result result("HMAC_DRBG reseed_interval limits");
+
+         const std::string mac_string = "HMAC(SHA-256)";
+
+         Request_Counting_RNG counting_rng;
+
+         result.test_throws("HMAC_DRBG does not accept 0 for reseed_interval",
+                            [&mac_string, &counting_rng]()
+            {
+            Botan::HMAC_DRBG failing_rng(Botan::MessageAuthenticationCode::create(mac_string), counting_rng, 0);
+            });
+
+         result.test_throws("HMAC_DRBG does not accept values higher than 2^24 for reseed_interval",
+                            [&mac_string, &counting_rng]()
+            {
+            Botan::HMAC_DRBG failing_rng(Botan::MessageAuthenticationCode::create(mac_string), counting_rng, (static_cast<size_t>(1) << 24) + 1);
+            });
+
+         return result;
+         }
+
       Test::Result test_security_level() override
          {
          Test::Result result("HMAC_DRBG Security Level");
@@ -590,6 +616,13 @@ class ChaCha_RNG_Unit_Tests final : public Stateful_RNG_Tests
          return result;
          }
 
+      Test::Result test_reseed_interval_limits() override
+         {
+         Test::Result result("ChaCha_RNG reseed_interval limits");
+         // ChaCha_RNG doesn't apply any limits to reseed_interval
+         return result;
+         }
+
       Test::Result test_reseed_kat() override
          {
          Test::Result result("ChaCha_RNG Reseed KAT");