Browse Source

Make crypto_seed_rng() and crypto_rand() less scary.

These functions must really never fail; so have crypto_rand() assert
that it's working okay, and have crypto_seed_rng() demand that
callers check its return value.  Also have crypto_seed_rng() check
RAND_status() before returning.
Nick Mathewson 8 years ago
parent
commit
dedea28c2e
7 changed files with 26 additions and 9 deletions
  1. 2 0
      src/common/compat.h
  2. 6 4
      src/common/crypto.c
  3. 2 1
      src/common/crypto.h
  4. 4 1
      src/or/main.c
  5. 4 1
      src/test/bench.c
  6. 4 1
      src/test/test_workqueue.c
  7. 4 1
      src/test/testing_common.c

+ 2 - 0
src/common/compat.h

@@ -118,6 +118,7 @@
 #define ATTR_CONST __attribute__((const))
 #define ATTR_MALLOC __attribute__((malloc))
 #define ATTR_NORETURN __attribute__((noreturn))
+#define ATTR_WUR __attribute__((warn_unused_result))
 /* Alas, nonnull is not at present a good idea for us.  We'd like to get
  * warnings when we pass NULL where we shouldn't (which nonnull does, albeit
  * spottily), but we don't want to tell the compiler to make optimizations
@@ -153,6 +154,7 @@
 #define ATTR_NORETURN
 #define ATTR_NONNULL(x)
 #define ATTR_UNUSED
+#define ATTR_WUR
 #define PREDICT_LIKELY(exp) (exp)
 #define PREDICT_UNLIKELY(exp) (exp)
 #endif

+ 6 - 4
src/common/crypto.c

@@ -2358,7 +2358,7 @@ crypto_seed_rng(void)
 
   memwipe(buf, 0, sizeof(buf));
 
-  if (rand_poll_ok || load_entropy_ok)
+  if ((rand_poll_ok || load_entropy_ok) && RAND_status() == 1)
     return 0;
   else
     return -1;
@@ -2380,12 +2380,14 @@ int
 crypto_rand_unmocked(char *to, size_t n)
 {
   int r;
+  if (n == 0)
+    return 0;
+
   tor_assert(n < INT_MAX);
   tor_assert(to);
   r = RAND_bytes((unsigned char*)to, (int)n);
-  if (r == 0)
-    crypto_log_errors(LOG_WARN, "generating random data");
-  return (r == 1) ? 0 : -1;
+  tor_assert(r >= 0);
+  return 0;
 }
 
 /** Return a pseudorandom integer, chosen uniformly from the values

+ 2 - 1
src/common/crypto.h

@@ -16,6 +16,7 @@
 #include <stdio.h>
 #include "torint.h"
 #include "testsupport.h"
+#include "compat.h"
 
 /*
   Macro to create an arbitrary OpenSSL version number as used by
@@ -258,7 +259,7 @@ int crypto_expand_key_material_rfc5869_sha256(
                                     uint8_t *key_out, size_t key_out_len);
 
 /* random numbers */
-int crypto_seed_rng(void);
+int crypto_seed_rng(void) ATTR_WUR;
 MOCK_DECL(int,crypto_rand,(char *to, size_t n));
 int crypto_rand_unmocked(char *to, size_t n);
 int crypto_strongest_rand(uint8_t *out, size_t out_len);

+ 4 - 1
src/or/main.c

@@ -1389,7 +1389,10 @@ run_scheduled_events(time_t now)
   if (time_to.add_entropy < now) {
     if (time_to.add_entropy) {
       /* We already seeded once, so don't die on failure. */
-      crypto_seed_rng();
+      if (crypto_seed_rng() < 0) {
+        log_warn(LD_GENERAL, "Tried to re-seed RNG, but failed. We already "
+                 "seeded once, though, so we won't exit here.");
+      }
     }
 /** How often do we add more entropy to OpenSSL's RNG pool? */
 #define ENTROPY_INTERVAL (60*60)

+ 4 - 1
src/test/bench.c

@@ -643,7 +643,10 @@ main(int argc, const char **argv)
 
   reset_perftime();
 
-  crypto_seed_rng();
+  if (crypto_seed_rng() < 0) {
+    printf("Couldn't seed RNG; exiting.\n");
+    return 1;
+  }
   crypto_init_siphash_key();
   options = options_new();
   init_logging(1);

+ 4 - 1
src/test/test_workqueue.c

@@ -391,7 +391,10 @@ main(int argc, char **argv)
   init_logging(1);
   network_init();
   crypto_global_init(1, NULL, NULL);
-  crypto_seed_rng();
+  if (crypto_seed_rng() < 0) {
+    printf("Couldn't seed RNG; exiting.\n");
+    return 1;
+  }
 
   rq = replyqueue_new(as_flags);
   tor_assert(rq);

+ 4 - 1
src/test/testing_common.c

@@ -272,7 +272,10 @@ main(int c, const char **v)
     return 1;
   }
   crypto_set_tls_dh_prime();
-  crypto_seed_rng();
+  if (crypto_seed_rng() < 0) {
+    printf("Couldn't seed RNG; exiting.\n");
+    return 1;
+  }
   rep_hist_init();
   network_init();
   setup_directory();