[tor-commits] [tor/master] Never allow OpenSSL engines to replace the RAND_SSLeay method

[nickm at torproject.org]

commit 7b87003957530427eadce36ed03b4645b481a335
Author: Nick Mathewson <nickm at torproject.org>
Date:   Wed Dec 18 11:49:44 2013 -0500

    Never allow OpenSSL engines to replace the RAND_SSLeay method
    
    This fixes bug 10402, where the rdrand engine would use the rdrand
    instruction, not as an additional entropy source, but as a replacement
    for the entire userspace PRNG.  That's obviously stupid: even if you
    don't think that RDRAND is a likely security risk, the right response
    to an alleged new alleged entropy source is never to throw away all
    previously used entropy sources.
    
    Thanks to coderman and rl1987 for diagnosing and tracking this down.
---
 changes/bug10402    |   11 +++++++++++
 src/common/crypto.c |   13 ++++++++++---
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/changes/bug10402 b/changes/bug10402
new file mode 100644
index 0000000..eac00bd
--- /dev/null
+++ b/changes/bug10402
@@ -0,0 +1,11 @@
+  o Major bugfixes:
+    - Do not allow OpenSSL engines to replace the PRNG, even when
+      HardwareAccel is set. The only default builtin PRNG engine uses
+      the Intel RDRAND instruction to replace the entire PRNG, and
+      ignores all attempts to seed it with more entropy. That's
+      cryptographically stupid: the right response to a new alleged
+      entropy source is never to discard all previously used entropy
+      sources. Fixes bug 10402; works around behavior introduced in
+      OpenSSL 1.0.0. Diagnosis and investigation thanks to "coderman"
+      and "rl1987".
+
diff --git a/src/common/crypto.c b/src/common/crypto.c
index 0ababea..940a756 100644
--- a/src/common/crypto.c
+++ b/src/common/crypto.c
@@ -169,8 +169,8 @@ log_engine(const char *fn, ENGINE *e)
     const char *name, *id;
     name = ENGINE_get_name(e);
     id = ENGINE_get_id(e);
-    log_notice(LD_CRYPTO, "Using OpenSSL engine %s [%s] for %s",
-        name?name:"?", id?id:"?", fn);
+    log_notice(LD_CRYPTO, "Default OpenSSL engine for %s is %s [%s]",
+               fn, name?name:"?", id?id:"?");
   } else {
     log_info(LD_CRYPTO, "Using default implementation for %s", fn);
   }
@@ -288,7 +288,7 @@ crypto_global_init(int useAccel, const char *accelName, const char *accelDir)
       }
       log_engine("RSA", ENGINE_get_default_RSA());
       log_engine("DH", ENGINE_get_default_DH());
-      log_engine("RAND", ENGINE_get_default_RAND());
+      log_engine("RAND (which we will not use)", ENGINE_get_default_RAND());
       log_engine("SHA1", ENGINE_get_digest_engine(NID_sha1));
       log_engine("3DES", ENGINE_get_cipher_engine(NID_des_ede3_ecb));
       log_engine("AES", ENGINE_get_cipher_engine(NID_aes_128_ecb));
@@ -297,6 +297,13 @@ crypto_global_init(int useAccel, const char *accelName, const char *accelDir)
       log_info(LD_CRYPTO, "NOT using OpenSSL engine support.");
     }
 
+    if (RAND_get_rand_method() != RAND_SSLeay()) {
+      log_notice(LD_CRYPTO, "It appears that one of our engines has provided "
+                 "a replacement the OpenSSL RNG. Resetting it to the default "
+                 "implementation.");
+      RAND_set_rand_method(RAND_SSLeay());
+    }
+
     evaluate_evp_for_aes(-1);
     evaluate_ctr_for_aes();