Update RAND_bytes to return return 0 on failure (aws#950)

Update RAND_bytes to return return 0 on failure, update all libcrypto
callers to handle this new failure.

crypto/curve25519/curve25519.c

@@ -1872,7 +1872,12 @@ static void sc_muladd(uint8_t *s, const uint8_t *a, const uint8_t *b,
 
 void ED25519_keypair(uint8_t out_public_key[32], uint8_t out_private_key[64]) {
   uint8_t seed[ED25519_SEED_LEN];
-  RAND_bytes(seed, ED25519_SEED_LEN);
+  if (!RAND_bytes(seed, ED25519_SEED_LEN)) {
+    // This is a public void function and can't be updated
+    OPENSSL_cleanse(out_public_key, 32);
+    OPENSSL_cleanse(out_private_key, 64);
+    return;
+  }
   ED25519_keypair_from_seed(out_public_key, out_private_key, seed);
   OPENSSL_cleanse(seed, ED25519_SEED_LEN);
 }
@@ -2091,7 +2096,12 @@ static void x25519_scalar_mult(uint8_t out[32], const uint8_t scalar[32],
 }
 
 void X25519_keypair(uint8_t out_public_value[32], uint8_t out_private_key[32]) {
-  RAND_bytes(out_private_key, 32);
+  if (!RAND_bytes(out_private_key, 32)) {
+    // This is a public void function and can't be updated
+    OPENSSL_cleanse(out_public_value, 32);
+    OPENSSL_cleanse(out_private_key, 32);
+    return;
+  }
 
   // All X25519 implementations should decode scalars correctly (see
   // https://tools.ietf.org/html/rfc7748#section-5). However, if an

crypto/curve25519/spake25519.c

@@ -353,7 +353,9 @@ int SPAKE2_generate_msg(SPAKE2_CTX *ctx, uint8_t *out, size_t *out_len,
   }
 
   uint8_t private_tmp[64];
-  RAND_bytes(private_tmp, sizeof(private_tmp));
+  if (!RAND_bytes(private_tmp, sizeof(private_tmp))) {
+    return 0;
+  }
   x25519_sc_reduce(private_tmp);
   // Multiply by the cofactor (eight) so that we'll clear it when operating on
   // the peer's point later in the protocol.

crypto/fips_callback_test.cc

@@ -8,6 +8,7 @@
 
 #include <gtest/gtest.h>
 #include <openssl/nid.h>
+#include <openssl/rand.h>
 #include <algorithm>
 #include <list>
 #include <string>
@@ -95,4 +96,25 @@ TEST(FIPSCallback, PowerOnTests) {
   }
 }
 
+TEST(FIPSCallback, DRBGRuntime) {
+  // At this point the library has loaded, if a self test was broken
+  // AWS_LC_FIPS_Callback would have already been called. If this test
+  // wasn't broken the call count should be zero
+  char*broken_runtime_test = getenv("BORINGSSL_FIPS_BREAK_TEST");
+  ASSERT_EQ(0, failure_count);
+  ASSERT_EQ(1, FIPS_mode());
+  uint8_t buf[10];
+  if (broken_runtime_test != nullptr && strcmp(broken_runtime_test, "CRNG" ) == 0) {
+    ASSERT_FALSE(RAND_bytes(buf, sizeof(buf)));
+    // TODO: make AWS_LC_FIPS_error update a new global state so FIPS_mode returns 0
+    ASSERT_EQ(1, FIPS_mode());
+    ASSERT_EQ(1, failure_count);
+  } else {
+    // BORINGSSL_FIPS_BREAK_TEST has not been set and everything should work
+    ASSERT_TRUE(RAND_bytes(buf, sizeof(buf)));
+    ASSERT_EQ(1, FIPS_mode());
+    ASSERT_EQ(0, failure_count);
+  }
+}
+
 #endif

crypto/fipsmodule/bn/random.c

@@ -158,8 +158,11 @@ int BN_rand(BIGNUM *rnd, int bits, int top, int bottom) {
   // |RAND_bytes| calls within the fipsmodule should be wrapped with state lock
   // functions to avoid updating the service indicator with the DRBG functions.
   FIPS_service_indicator_lock_state();
-  RAND_bytes((uint8_t *)rnd->d, words * sizeof(BN_ULONG));
+  int rand_success = RAND_bytes((uint8_t *)rnd->d, words * sizeof(BN_ULONG));
   FIPS_service_indicator_unlock_state();
+  if (!rand_success) {
+    return 0;
+  }
 
   rnd->d[words - 1] &= mask;
   if (top != BN_RAND_TOP_ANY) {
@@ -333,7 +336,9 @@ int bn_rand_secret_range(BIGNUM *r, int *out_is_uniform, BN_ULONG min_inclusive,
   }
 
   // Select a uniform random number with num_bits(max_exclusive) bits.
-  RAND_bytes((uint8_t *)r->d, words * sizeof(BN_ULONG));
+  if (!RAND_bytes((uint8_t *)r->d, words * sizeof(BN_ULONG))) {
+    goto end;
+  }
   r->d[words - 1] &= mask;
 
   // Check, in constant-time, if the value is in range.

crypto/fipsmodule/cipher/e_aes.c

@@ -1295,8 +1295,11 @@ static int aead_aes_gcm_seal_scatter_randnonce(
   // |RAND_bytes| calls within the fipsmodule should be wrapped with state lock
   // functions to avoid updating the service indicator with the DRBG functions.
   FIPS_service_indicator_lock_state();
-  RAND_bytes(nonce, sizeof(nonce));
+  int rand_result = RAND_bytes(nonce, sizeof(nonce));
   FIPS_service_indicator_unlock_state();
+  if (!rand_result)  {
+    return 0;
+  }
   const struct aead_aes_gcm_ctx *gcm_ctx =
       (const struct aead_aes_gcm_ctx *)&ctx->state;
   if (!aead_aes_gcm_seal_scatter_impl(gcm_ctx, out, out_tag, out_tag_len,

crypto/fipsmodule/rand/internal.h

@@ -33,7 +33,7 @@ extern "C" {
 
 // RAND_bytes_with_additional_data samples from the RNG after mixing 32 bytes
 // from |user_additional_data| in.
-void RAND_bytes_with_additional_data(uint8_t *out, size_t out_len,
+int RAND_bytes_with_additional_data(uint8_t *out, size_t out_len,
                                      const uint8_t user_additional_data[32]);
 
 // CRYPTO_sysrand fills |len| bytes at |buf| with entropy from the operating

crypto/fipsmodule/rand/rand.c

@@ -301,7 +301,7 @@ static void CRYPTO_get_fips_seed(uint8_t *out_entropy, size_t out_entropy_len,
 
 // rand_get_seed fills |seed| with entropy and sets |*out_want_additional_input|
 // to one if that entropy came directly from the CPU and zero otherwise.
-static void rand_get_seed(struct rand_thread_state *state,
+static int rand_get_seed(struct rand_thread_state *state,
                           uint8_t seed[CTR_DRBG_ENTROPY_LEN],
                           int *out_want_additional_input) {
   if (!state->last_block_valid) {
@@ -317,46 +317,45 @@ static void rand_get_seed(struct rand_thread_state *state,
   // generator test” which causes the program to randomly abort. Hopefully the
   // rate of failure is small enough not to be a problem in practice.
   if (CRYPTO_memcmp(state->last_block, entropy, CRNGT_BLOCK_SIZE) == 0) {
-    fprintf(stderr, "CRNGT failed.\n");
-    // TODO return the result from AWS_LC_FIPS_error and update rand_get_seed to return an int
-    AWS_LC_FIPS_error("CRNGT failed.", ERR_R_FIPS_TEST_FAILURE);
+    return AWS_LC_FIPS_error("CRNGT failed.", ERR_R_FIPS_TEST_FAILURE);
   }
 
   OPENSSL_STATIC_ASSERT(sizeof(entropy) % CRNGT_BLOCK_SIZE == 0, _)
   for (size_t i = CRNGT_BLOCK_SIZE; i < sizeof(entropy);
        i += CRNGT_BLOCK_SIZE) {
     if (CRYPTO_memcmp(entropy + i - CRNGT_BLOCK_SIZE, entropy + i,
                       CRNGT_BLOCK_SIZE) == 0) {
-      // TODO return the result from AWS_LC_FIPS_error and update rand_get_seed to return an int
-      AWS_LC_FIPS_error("CRNGT failed.", ERR_R_FIPS_TEST_FAILURE);
+      return AWS_LC_FIPS_error("CRNGT failed.", ERR_R_FIPS_TEST_FAILURE);
     }
   }
   OPENSSL_memcpy(state->last_block,
                  entropy + sizeof(entropy) - CRNGT_BLOCK_SIZE,
                  CRNGT_BLOCK_SIZE);
 
   OPENSSL_memcpy(seed, entropy, CTR_DRBG_ENTROPY_LEN);
+  return 1;
 }
 
 #else // BORINGSSL_FIPS
 
 // rand_get_seed fills |seed| with entropy and sets |*out_want_additional_input|
 // to one if that entropy came directly from the CPU and zero otherwise.
-static void rand_get_seed(struct rand_thread_state *state,
+static int rand_get_seed(struct rand_thread_state *state,
                           uint8_t seed[CTR_DRBG_ENTROPY_LEN],
                           int *out_want_additional_input) {
   // If not in FIPS mode, we don't overread from the system entropy source and
   // we don't depend only on the hardware RDRAND.
   CRYPTO_sysrand_for_seed(seed, CTR_DRBG_ENTROPY_LEN);
   *out_want_additional_input = 0;
+  return 1;
 }
 
 #endif // BORINGSSL_FIPS
 
-void RAND_bytes_with_additional_data(uint8_t *out, size_t out_len,
+int RAND_bytes_with_additional_data(uint8_t *out, size_t out_len,
                                      const uint8_t user_additional_data[32]) {
   if (out_len == 0) {
-    return;
+    return 1;
   }
 
   const uint64_t fork_generation = CRYPTO_get_fork_generation();
@@ -422,7 +421,9 @@ void RAND_bytes_with_additional_data(uint8_t *out, size_t out_len,
     state->last_block_valid = 0;
     uint8_t seed[CTR_DRBG_ENTROPY_LEN];
     int want_additional_input;
-    rand_get_seed(state, seed, &want_additional_input);
+    if (!rand_get_seed(state, seed, &want_additional_input)) {
+      return 0;
+    }
 
     uint8_t personalization[CTR_DRBG_ENTROPY_LEN] = {0};
     size_t personalization_len = 0;
@@ -462,7 +463,9 @@ void RAND_bytes_with_additional_data(uint8_t *out, size_t out_len,
       state->fork_generation != fork_generation) {
     uint8_t seed[CTR_DRBG_ENTROPY_LEN];
     int want_additional_input;
-    rand_get_seed(state, seed, &want_additional_input);
+    if (!rand_get_seed(state, seed, &want_additional_input)){
+      return 0;
+    }
 
     uint8_t add_data_for_reseed[CTR_DRBG_ENTROPY_LEN];
     size_t add_data_for_reseed_len = 0;
@@ -526,12 +529,12 @@ void RAND_bytes_with_additional_data(uint8_t *out, size_t out_len,
 #if defined(BORINGSSL_FIPS)
   CRYPTO_STATIC_MUTEX_unlock_read(state_clear_all_lock_bss_get());
 #endif
+  return 1;
 }
 
 int RAND_bytes(uint8_t *out, size_t out_len) {
   static const uint8_t kZeroAdditionalData[32] = {0};
-  RAND_bytes_with_additional_data(out, out_len, kZeroAdditionalData);
-  return 1;
+  return RAND_bytes_with_additional_data(out, out_len, kZeroAdditionalData);
 }
 
 int RAND_pseudo_bytes(uint8_t *buf, size_t len) {

crypto/fipsmodule/rsa/padding.c

@@ -145,19 +145,27 @@ int RSA_padding_check_PKCS1_type_1(uint8_t *out, size_t *out_len,
   return 1;
 }
 
-static void rand_nonzero(uint8_t *out, size_t len) {
+static int rand_nonzero(uint8_t *out, size_t len) {
+  int result = 0;
   // |RAND_bytes| calls within the fipsmodule should be wrapped with state lock
   // functions to avoid updating the service indicator with the DRBG functions.
   FIPS_service_indicator_lock_state();
-  RAND_bytes(out, len);
+  if (!RAND_bytes(out, len)) {
+    goto out;
+  }
 
   for (size_t i = 0; i < len; i++) {
     while (out[i] == 0) {
-      RAND_bytes(out + i, 1);
+      if (!RAND_bytes(out + i, 1)){
+        goto out;
+      }
     }
   }
-
+  result = 1;
+
+out:
   FIPS_service_indicator_unlock_state();
+  return result;
 }
 
 int RSA_padding_add_PKCS1_type_2(uint8_t *to, size_t to_len,

crypto/hpke/hpke.c

@@ -487,7 +487,9 @@ int EVP_HPKE_CTX_setup_sender(EVP_HPKE_CTX *ctx, uint8_t *out_enc,
                               size_t peer_public_key_len, const uint8_t *info,
                               size_t info_len) {
   uint8_t seed[MAX_SEED_LEN];
-  RAND_bytes(seed, kem->seed_len);
+  if (!RAND_bytes(seed, kem->seed_len)) {
+    return 0;
+  }
   return EVP_HPKE_CTX_setup_sender_with_seed_for_testing(
       ctx, out_enc, out_enc_len, max_enc, kem, kdf, aead, peer_public_key,
       peer_public_key_len, info, info_len, seed, kem->seed_len);

crypto/hrss/hrss.c

@@ -1999,7 +1999,8 @@ int HRSS_generate_key(
   if (!vars) {
     // If the caller ignores the return value the output will still be safe.
     // The private key output is randomised in case it's later passed to
-    // |HRSS_encap|.
+    // |HRSS_encap|. As this is the error case we can't error out harder
+    // hence the result of RAND_bytes is not checked.
     memset(out_pub, 0, sizeof(struct HRSS_public_key));
     RAND_bytes((uint8_t*) out_priv, sizeof(struct HRSS_private_key));
     return 0;
@@ -2062,7 +2063,8 @@ int HRSS_encap(uint8_t out_ciphertext[POLY_BYTES], uint8_t out_shared_key[32],
   if (!vars) {
     // If the caller ignores the return value the output will still be safe.
     // The private key output is randomised in case it's used to encrypt and
-    // transmit something.
+    // transmit something. As this is the error case we can't error out harder
+    // hence the result of RAND_bytes is not checked.
     memset(out_ciphertext, 0, POLY_BYTES);
     RAND_bytes(out_shared_key, 32);
     return 0;
@@ -2124,7 +2126,8 @@ int HRSS_decap(uint8_t out_shared_key[HRSS_KEY_BYTES],
   if (!vars) {
     // If the caller ignores the return value the output will still be safe.
     // The private key output is randomised in case it's used to encrypt and
-    // transmit something.
+    // transmit something. As this is the error case we can't error out harder
+    // hence the result of RAND_bytes is not checked.
     RAND_bytes(out_shared_key, HRSS_KEY_BYTES);
     return 0;
   }

crypto/pool/pool.c

@@ -55,7 +55,11 @@ CRYPTO_BUFFER_POOL* CRYPTO_BUFFER_POOL_new(void) {
   }
 
   CRYPTO_MUTEX_init(&pool->lock);
-  RAND_bytes((uint8_t *)&pool->hash_key, sizeof(pool->hash_key));
+  if (!RAND_bytes((uint8_t *)&pool->hash_key, sizeof(pool->hash_key))) {
+    OPENSSL_free(pool);
+    return NULL;
+
+  }
 
   return pool;
 }

crypto/trust_token/pmbtoken.c

@@ -346,7 +346,9 @@ static STACK_OF(TRUST_TOKEN_PRETOKEN) *pmbtoken_blind(
       goto err;
     }
 
-    RAND_bytes(pretoken->salt, sizeof(pretoken->salt));
+    if (!RAND_bytes(pretoken->salt, sizeof(pretoken->salt))) {
+      goto err;
+    }
     if (include_message) {
       assert(SHA512_DIGEST_LENGTH == TRUST_TOKEN_NONCE_SIZE);
       SHA512_Init(&hash_ctx);
@@ -854,7 +856,9 @@ static int pmbtoken_sign(const PMBTOKEN_METHOD *method,
     ec_scalar_select(group, &yb, mask, &key->y1, &key->y0);
 
     uint8_t s[TRUST_TOKEN_NONCE_SIZE];
-    RAND_bytes(s, TRUST_TOKEN_NONCE_SIZE);
+    if(!RAND_bytes(s, TRUST_TOKEN_NONCE_SIZE)) {
+      goto err;
+    }
     // The |jacobians| and |affines| contain Sp, Wp, and Wsp.
     EC_RAW_POINT jacobians[3];
     EC_AFFINE affines[3];

crypto/trust_token/voprf.c

@@ -227,7 +227,9 @@ static STACK_OF(TRUST_TOKEN_PRETOKEN) *voprf_blind(const VOPRF_METHOD *method,
       goto err;
     }
 
-    RAND_bytes(pretoken->salt, sizeof(pretoken->salt));
+    if(!RAND_bytes(pretoken->salt, sizeof(pretoken->salt))) {
+      goto err;
+    }
     if (include_message) {
       assert(SHA512_DIGEST_LENGTH == TRUST_TOKEN_NONCE_SIZE);
       SHA512_Init(&hash_ctx);

tests/ci/run_fips_callback_tests.sh

@@ -28,3 +28,14 @@ for kat in $KATS; do
   $broken_test --gtest_filter=FIPSCallback.PowerOnTests
   unset FIPS_CALLBACK_TEST_POWER_ON_TEST_FAILURE
 done
+
+runtime_tests=("CRNG")
+for runtime_test in "${runtime_tests[@]}"; do
+  # Tell our test what test is expected to fail
+  export FIPS_CALLBACK_TEST_RUNTIME_TEST_FAILURE="$runtime_test"
+  # Tell bcm which test to break
+  export BORINGSSL_FIPS_BREAK_TEST="$runtime_test"
+  # These tests will have side affects in the future (modifying the global FIPS state) and must be run in separate process
+  $original_test --gtest_filter=FIPSCallback.PowerOnTests
+  $original_test --gtest_filter=FIPSCallback.DRBGRuntime
+done