Reorganise reseeding (#1941)

To avoid synchronisation issues for codepoints that reads/mutates the ctr-drbg state in #1919, we must reorganise the reseed logic. This decouples the code that reads/mutates the ctr-drbg state and the code that gathers entropy. The latter is not an issue. The former will later be wrapped as critical code needing synchronisation as part of the global zeroisation.
aws · Nov 1, 2024 · 72f0a9f · 72f0a9f
1 parent ccb97ef
commit 72f0a9f
Show file tree

Hide file tree

Showing 4 changed files with 97 additions and 53 deletions.
diff --git a/crypto/fipsmodule/rand/entropy/entropy_sources.c b/crypto/fipsmodule/rand/entropy/entropy_sources.c
@@ -37,14 +37,14 @@ static int entropy_default_randomize(void) {
 
 // The default entropy source configuration using
 // - OS randomness source for seeding.
-// - Doesn't have a personalization string source.
+// - Doesn't have a extra entropy source.
 // - If run-time is on an Intel CPU and it supports rdrand, use it as a source
 //   for prediction resistance. Otherwise, no source.
 DEFINE_LOCAL_DATA(struct entropy_source, default_entropy_source) {
   out->initialize = entropy_default_initialize;
   out->cleanup = entropy_default_cleanup;
   out->get_seed = entropy_default_get_seed;
-  out->get_personalization_string = NULL;
+  out->get_extra_entropy = NULL;
   if (have_fast_rdrand() == 1) {
     out->get_prediction_resistance = entropy_default_get_prediction_resistance;
   } else {

diff --git a/crypto/fipsmodule/rand/entropy/internal.h b/crypto/fipsmodule/rand/entropy/internal.h
@@ -17,7 +17,7 @@ struct entropy_source {
   int (*initialize)(void);
   void (*cleanup)(void);
   int (*get_seed)(uint8_t seed[CTR_DRBG_ENTROPY_LEN]);
-  int (*get_personalization_string)(uint8_t personalization_string[CTR_DRBG_ENTROPY_LEN]);
+  int (*get_extra_entropy)(uint8_t extra_entropy[CTR_DRBG_ENTROPY_LEN]);
   int (*get_prediction_resistance)(uint8_t pred_resistance[RAND_PRED_RESISTANCE_LEN]);
   int (*randomize)(void);
 };

diff --git a/crypto/fipsmodule/rand/new_rand.c b/crypto/fipsmodule/rand/new_rand.c
@@ -56,7 +56,7 @@ static void rand_thread_local_state_free(void *state_in) {
 // reasons are documented with inline comments in the function.
 //
 // Returns 1 if |state| is in a valid state and 0 otherwise.
-static int rand_ensure_valid_state(struct rand_thread_local_state *state) {
+static int rand_ensure_valid_state(const struct rand_thread_local_state *state) {
 
   // We do not allow the UBE generation number to change while executing AWS-LC
   // randomness generation code e.g. while |RAND_bytes| executes. One way to hit
@@ -71,29 +71,29 @@ static int rand_ensure_valid_state(struct rand_thread_local_state *state) {
   return 1;
 }
 
-// rand_ensure_ctr_drbg_uniqueness computes whether |state| must be randomized
+// rand_check_ctr_drbg_uniqueness computes whether |state| must be randomized
 // to ensure uniqueness.
 //
-// Note: If |rand_ensure_ctr_drbg_uniqueness| returns 1 it does not necessarily
+// Note: If |rand_check_ctr_drbg_uniqueness| returns 0 it does not necessarily
 // imply that an UBE occurred. It can also mean that no UBE detection is
 // supported or that UBE detection failed. In these cases, |state| must also be
 // randomized to ensure uniqueness. Any special future cases can be handled in
 // this function. 
 //
-// Return 1 if |state| must be randomized. 0 otherwise.
-static int rand_ensure_ctr_drbg_uniqueness(struct rand_thread_local_state *state) {
+// Return 0 if |state| must be randomized. 1 otherwise.
+static int rand_check_ctr_drbg_uniqueness(struct rand_thread_local_state *state) {
 
   uint64_t current_generation_number = 0;
   if (CRYPTO_get_ube_generation_number(&current_generation_number) != 1) {
-    return 1;
+    return 0;
   }
 
   if (current_generation_number != state->generation_number) {
     state->generation_number = current_generation_number;
-    return 1;
+    return 0;
   }
 
-  return 0;
+  return 1;
 }
 
 // rand_maybe_get_ctr_drbg_pred_resistance maybe fills |pred_resistance| with
@@ -124,61 +124,52 @@ static void rand_maybe_get_ctr_drbg_pred_resistance(
 // rand_get_ctr_drbg_seed_entropy source entropy for seeding and reseeding the
 // CTR-DRBG state. Firstly, |seed| is filled with |CTR_DRBG_ENTROPY_LEN| bytes
 // from the seed source configured in |entropy_source|. Secondly, if available,
-// |CTR_DRBG_ENTROPY_LEN| bytes is filled into |personalization_string| sourced
-// from the personalization string source configured in |entropy_source|.
+// |CTR_DRBG_ENTROPY_LEN| bytes is filled into |extra_entropy| sourced
+// from the extra entropy source configured in |entropy_source|.
 //
-// |*personalization_string_len| is set to 0 if no personalization string source
+// |*extra_entropy_len| is set to 0 if no extra entropy source
 // is available and |CTR_DRBG_ENTROPY_LEN| otherwise.
 static void rand_get_ctr_drbg_seed_entropy(
   const struct entropy_source *entropy_source,
   uint8_t seed[CTR_DRBG_ENTROPY_LEN],
-  uint8_t personalization_string[CTR_DRBG_ENTROPY_LEN],
-  size_t *personalization_string_len) {
+  uint8_t extra_entropy[CTR_DRBG_ENTROPY_LEN],
+  size_t *extra_entropy_len) {
 
   GUARD_PTR_ABORT(entropy_source);
-  GUARD_PTR_ABORT(personalization_string_len);
+  GUARD_PTR_ABORT(extra_entropy_len);
 
-  *personalization_string_len = 0;
+  *extra_entropy_len = 0;
 
   // If the seed source is missing it is impossible to source any entropy.
   if (entropy_source->get_seed(seed) != 1) {
     abort();
   }
 
-  // Not all entropy source configurations will have a personalization string
-  // source. Hence, it's optional. But use it if configured.
-  if (entropy_source->get_personalization_string != NULL) {
-    if(entropy_source->get_personalization_string(personalization_string) != 1) {
+  // Not all entropy source configurations will have an extra entropy source.
+  // Hence, it's optional. But use it if configured.
+  if (entropy_source->get_extra_entropy != NULL) {
+    if(entropy_source->get_extra_entropy(extra_entropy) != 1) {
       abort();
     }
-    *personalization_string_len = CTR_DRBG_ENTROPY_LEN;
+    *extra_entropy_len = CTR_DRBG_ENTROPY_LEN;
   }
 }
 
 // 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,
+  const uint8_t seed[CTR_DRBG_ENTROPY_LEN],
+  const uint8_t additional_data[CTR_DRBG_ENTROPY_LEN],
+  size_t additional_data_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) {
+  if (CTR_DRBG_reseed(&(state->drbg), seed, additional_data,
+        additional_data_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 +222,13 @@ 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;
+
+  // Ensure that the CTR-DRBG state is unique.
+  if (rand_check_ctr_drbg_uniqueness(state) != 1) {
+    must_reseed_before_generate = 1;
   }
 
   // If a prediction resistance source is available, use it.
@@ -255,7 +250,9 @@ 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
+  // TODO: lock here
+
+  // 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,17 +264,24 @@ 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 (must_reseed_before_generate == 1 ||
+       (state->generate_calls_since_seed + 1) > kCtrDrbgReseedInterval) {
+
+      must_reseed_before_generate = 0;
+
+      // TODO: unlock here
+      uint8_t seed[CTR_DRBG_ENTROPY_LEN];
+      uint8_t additional_data[CTR_DRBG_ENTROPY_LEN];
+      size_t additional_data_len = 0;
+      rand_get_ctr_drbg_seed_entropy(state->entropy_source, seed,
+        additional_data, &additional_data_len);
+
+      // TODO: lock here
+      rand_ctr_drbg_reseed(state, seed, additional_data,
+        additional_data_len);
+
+      OPENSSL_cleanse(seed, CTR_DRBG_ENTROPY_LEN);
+      OPENSSL_cleanse(additional_data, CTR_DRBG_ENTROPY_LEN);
     }
 
     if (!CTR_DRBG_generate(&(state->drbg), out, todo, pred_resistance,
@@ -296,6 +300,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,

diff --git a/crypto/fipsmodule/rand/new_rand_test.cc b/crypto/fipsmodule/rand/new_rand_test.cc
@@ -39,6 +39,44 @@ TEST(NewRand, Basic) {
   }
 }
 
+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);
+  ASSERT_EQ(get_thread_generate_calls_since_seed(), 1ULL);
+}
+
 static void MockedUbeDetection(std::function<void(uint64_t)> set_detection_method_gn) {
 
   const size_t request_size_one_generate = 10;