Restrict RAND_bytes request length and reorganise reseeding

crypto/fipsmodule/rand/new_rand.c

@@ -96,6 +96,37 @@ static int rand_ensure_ctr_drbg_uniqueness(struct rand_thread_local_state *state
   return 0;
 }
 
+// rand_ensure_ctr_drbg_reseed_interval computes whether |state| must be
+// randomized due to not being able to stay below its reseed interval. |len|
+// is the request size by a consumer. |len| must be strictly less than 2^32.
+//
+// Return 1 if |state| must be randomized. 0 otherwise.
+static int rand_ensure_ctr_drbg_reseed_interval(struct rand_thread_local_state *state,
+  size_t len) {
+
+  // This will be the majority case.
+  if (len <= CTR_DRBG_MAX_GENERATE_LENGTH) {
+    return (state->generate_calls_since_seed + 1) > kCtrDrbgReseedInterval ? 1 : 0;
+  }
+
+  // CTR_DRBG_MAX_GENERATE_LENGTH * kCtrDrbgReseedInterval in |RAND_bytes_core|
+  // is far less than 2^32. However, still be explicit in case we ever reach
+  // here from another code path.
+  OPENSSL_STATIC_ASSERT(sizeof(size_t) >= 4, size_t_not_big_enough)
+  if (len > 0xFFFFFFFF) {
+    abort();
+  }
+
+  uint32_t len32 = (uint32_t) len;
+
+  // Given a 32-bit positive integer I and a power of two 2^N. We can compute:
+  // ceiling(I/2^N) = ((I - 1) >> N) + 1).
+  // In our case, I is |len32| and N is |CTR_DRBG_MAX_GENERATE_LENGTH_POW2|.
+  uint32_t generate_calls_needed = ((len32-1) >> CTR_DRBG_MAX_GENERATE_LENGTH_POW2) + 1;
+
+  return (state->generate_calls_since_seed + generate_calls_needed) > kCtrDrbgReseedInterval ? 1 : 0;
+}
+
 // rand_maybe_get_ctr_drbg_pred_resistance maybe fills |pred_resistance| with
 // |RAND_PRED_RESISTANCE_LEN| bytes. The bytes are sourced from the prediction
 // resistance source from the entropy source in |state|, if such a source has
@@ -156,29 +187,20 @@ static void rand_get_ctr_drbg_seed_entropy(
 }
 
 // rand_ctr_drbg_reseed reseeds the CTR-DRBG state in |state|.
-static void rand_ctr_drbg_reseed(struct rand_thread_local_state *state) {
+static void rand_ctr_drbg_reseed(struct rand_thread_local_state *state,
+  uint8_t seed[CTR_DRBG_ENTROPY_LEN],
+  uint8_t personalization_string[CTR_DRBG_ENTROPY_LEN],
+  size_t personalization_string_len) {
 
   GUARD_PTR_ABORT(state);
 
-  uint8_t seed[CTR_DRBG_ENTROPY_LEN];
-  uint8_t personalization_string[CTR_DRBG_ENTROPY_LEN];
-  size_t personalization_string_len = 0;
-  rand_get_ctr_drbg_seed_entropy(state->entropy_source, seed,
-    personalization_string, &personalization_string_len);
-
-  assert(personalization_string_len == 0 ||
-         personalization_string_len == CTR_DRBG_ENTROPY_LEN);
-
   if (CTR_DRBG_reseed(&(state->drbg), seed, personalization_string,
         personalization_string_len) != 1) {
     abort();
   }
 
   state->reseed_calls_since_initialization++;
   state->generate_calls_since_seed = 0;
-
-  OPENSSL_cleanse(seed, CTR_DRBG_ENTROPY_LEN);
-  OPENSSL_cleanse(personalization_string, CTR_DRBG_ENTROPY_LEN);
 }
 
 // rand_state_initialize initializes the thread-local state |state|. In
@@ -231,9 +253,28 @@ static void RAND_bytes_core(
   GUARD_PTR_ABORT(state);
   GUARD_PTR_ABORT(out);
 
-  // Ensure the CTR-DRBG state is unique.
-  if (rand_ensure_ctr_drbg_uniqueness(state) == 1) {
-    rand_ctr_drbg_reseed(state);
+  // must_reseed_before_generate is 1 if we must reseed before invoking the
+  // CTR-DRBG generate function CTR_DRBG_generate().
+  int must_reseed_before_generate = 0;
+
+  // We can support a request size (out_len) up to
+  // CTR_DRBG_MAX_GENERATE_LENGTH * kCtrDrbgReseedInterval bytes. This is to
+  // ensure that we do not need to reseed under the while-loop where we are
+  // invoking CTR_DRBG_generate. The motivation is to ensure that we can lock
+  // access to the thread-local state for both reseed and generate without
+  // potentially making system calls.
+  // The value CTR_DRBG_MAX_GENERATE_LENGTH * kCtrDrbgReseedInterval is the
+  // maximum number of bytes that can be generated by invoking the CTR-DRBG
+  // generate function kCtrDrbgReseedInterval times. That is, the maximum number
+  // of bytes that can be produced for a full reseed interval.
+  if (out_len > CTR_DRBG_MAX_GENERATE_LENGTH * kCtrDrbgReseedInterval) {
+    abort();
+  }
+
+  // Ensure the CTR-DRBG state is safe to use.
+  if (rand_ensure_ctr_drbg_uniqueness(state) == 1 ||
+      rand_ensure_ctr_drbg_reseed_interval(state, out_len) == 1) {
+      must_reseed_before_generate = 1;
   }
 
   // If a prediction resistance source is available, use it.
@@ -255,7 +296,22 @@ static void RAND_bytes_core(
   assert(first_pred_resistance_len == 0 ||
          first_pred_resistance_len == RAND_PRED_RESISTANCE_LEN);
 
-  // Iterate CTR-DRBG generate until we |out_len| bytes of randomness have been
+  if (must_reseed_before_generate == 1) {
+    uint8_t seed[CTR_DRBG_ENTROPY_LEN];
+    uint8_t personalization_string[CTR_DRBG_ENTROPY_LEN];
+    size_t personalization_string_len = 0;
+    rand_get_ctr_drbg_seed_entropy(state->entropy_source, seed,
+      personalization_string, &personalization_string_len);
+
+    // TODO: lock here
+    rand_ctr_drbg_reseed(state, seed, personalization_string,
+      personalization_string_len);
+
+    OPENSSL_cleanse(seed, CTR_DRBG_ENTROPY_LEN);
+    OPENSSL_cleanse(personalization_string, CTR_DRBG_ENTROPY_LEN);
+  }
+
+  // Iterate CTR-DRBG generate until |out_len| bytes of randomness have been
   // generated. CTR_DRBG_generate can maximally generate
   // |CTR_DRBG_MAX_GENERATE_LENGTH| bytes per usage of its state see
   // SP800-90A Rev 1 Table 3. If user requests more, we most generate output in
@@ -267,19 +323,6 @@ static void RAND_bytes_core(
       todo = CTR_DRBG_MAX_GENERATE_LENGTH;
     }
 
-    // Each reseed interval can generate up to
-    // |CTR_DRBG_MAX_GENERATE_LENGTH*kCtrDrbgReseedInterval| bytes.
-    // Determining the time(s) to reseed prior to entering the CTR-DRBG generate
-    // loop is a doable strategy. But tracking reseed times adds unnecessary
-    // complexity. Instead our strategy is optimizing for simplicity.
-    // |out_len < CTR_DRBG_MAX_GENERATE_LENGTH| will be the majority case
-    // (by far) and requires a single check in either strategy.
-    // Note if we already reseeded through |rand_ctr_drbg_reseed()|, we won't
-    // reseed again here.
-    if( state->generate_calls_since_seed + 1 >= kCtrDrbgReseedInterval) {
-      rand_ctr_drbg_reseed(state);
-    }
-
     if (!CTR_DRBG_generate(&(state->drbg), out, todo, pred_resistance,
           first_pred_resistance_len)) {
       abort();
@@ -296,6 +339,8 @@ static void RAND_bytes_core(
   if (rand_ensure_valid_state(state) != 1) {
     abort();
   }
+
+  // TODO: unlock here
 }
 
 static void RAND_bytes_private(uint8_t *out, size_t out_len,

crypto/fipsmodule/rand/new_rand_test.cc

@@ -37,6 +37,59 @@ TEST(NewRand, Basic) {
     ASSERT_TRUE(RAND_bytes_with_additional_data(randomness, i, user_personalization_string));
     ASSERT_TRUE(RAND_bytes_with_user_prediction_resistance(randomness, i, user_personalization_string));    
   }
+
+#if !defined(OPENSSL_ANDROID)
+  size_t request_len_too_large = CTR_DRBG_MAX_GENERATE_LENGTH * kCtrDrbgReseedInterval + 1;
+  ASSERT_DEATH_IF_SUPPORTED(RAND_bytes(randomness, request_len_too_large), "");
+  ASSERT_DEATH_IF_SUPPORTED(RAND_priv_bytes(randomness, request_len_too_large), "");
+  ASSERT_DEATH_IF_SUPPORTED(RAND_pseudo_bytes(randomness, request_len_too_large), "");
+  ASSERT_DEATH_IF_SUPPORTED(RAND_bytes_with_additional_data(randomness, request_len_too_large, user_personalization_string), "");
+  ASSERT_DEATH_IF_SUPPORTED(RAND_bytes_with_user_prediction_resistance(randomness, request_len_too_large, user_personalization_string), "");
+#endif
+}
+
+TEST(NewRand, ReseedInterval) {
+  uint8_t randomness[CTR_DRBG_MAX_GENERATE_LENGTH * 5 + 1] = {0};
+  uint64_t reseed_calls_since_initialization = get_thread_reseed_calls_since_initialization();
+  uint64_t generate_calls_since_seed = get_thread_generate_calls_since_seed();
+
+  // First check that we can predict when a reseed happens based on the current
+  // number of invoked generate calls. After the loop, we expect to be one
+  // invoke generate call from a reseed.
+  for(size_t i = 0; i < (kCtrDrbgReseedInterval - generate_calls_since_seed); i++) {
+    ASSERT_TRUE(RAND_bytes(randomness, 1));
+    ASSERT_EQ(get_thread_reseed_calls_since_initialization(), reseed_calls_since_initialization);
+  }
+  ASSERT_TRUE(RAND_bytes(randomness, 1));
+  ASSERT_EQ(get_thread_reseed_calls_since_initialization(), reseed_calls_since_initialization + 1);
+  ASSERT_EQ(get_thread_generate_calls_since_seed(), 1ULL);
+
+  ASSERT_TRUE(RAND_bytes(randomness, 1));
+  ASSERT_EQ(get_thread_reseed_calls_since_initialization(), reseed_calls_since_initialization + 1);
+  ASSERT_EQ(get_thread_generate_calls_since_seed(), 2ULL);
+
+  // Should be able to perform kCtrDrbgReseedInterval-2 generate calls before a
+  // reseed is emitted. Requesting
+  // CTR_DRBG_MAX_GENERATE_LENGTH * (kCtrDrbgReseedInterval-2) + 1 would require
+  // quite a large buffer. Instead iterate until we need
+  // 5 iterations and request 5 * CTR_DRBG_MAX_GENERATE_LENGTH+1, which is a
+  // much smaller buffer.
+  for(size_t i = 0; i < (kCtrDrbgReseedInterval - 7); i++) {
+    ASSERT_TRUE(RAND_bytes(randomness, 1));
+    ASSERT_EQ(get_thread_reseed_calls_since_initialization(), reseed_calls_since_initialization + 1);
+    ASSERT_EQ(get_thread_generate_calls_since_seed(), 2 + (i + 1));
+  }
+  ASSERT_EQ(get_thread_generate_calls_since_seed(), kCtrDrbgReseedInterval - 5);
+  size_t request_len_new_reseed = CTR_DRBG_MAX_GENERATE_LENGTH * 5 + 1;
+  ASSERT_TRUE(RAND_bytes(randomness, request_len_new_reseed));
+  ASSERT_EQ(get_thread_reseed_calls_since_initialization(), reseed_calls_since_initialization + 2);
+  // Note that the number of invoked generate calls will be 6, even though the
+  // state after the for-loop would allow another 5 invocation without a reseed.
+  // The reason is that when we observe that generation request_len_new_reseed
+  // will reach past the reseed interval upper bound, the reseed is performed
+  // before the first invoked generate call. Additionally,
+  // request_len_new_reseed can't be satisfied with only 5 invocations.
+  ASSERT_EQ(get_thread_generate_calls_since_seed(), 6ULL);
 }
 
 static void MockedUbeDetection(std::function<void(uint64_t)> set_detection_method_gn) {

include/openssl/ctrdrbg.h

@@ -37,7 +37,9 @@ extern "C" {
 
 // CTR_DRBG_MAX_GENERATE_LENGTH is the maximum number of bytes that can be
 // generated in a single call to |CTR_DRBG_generate|.
+// Must be a power of two.
 #define CTR_DRBG_MAX_GENERATE_LENGTH 65536
+#define CTR_DRBG_MAX_GENERATE_LENGTH_POW2 16
 
 // CTR_DRBG_new returns an initialized |CTR_DRBG_STATE|, or NULL if either
 // allocation failed or if |personalization_len| is invalid.