Various RAND improvements

Try to put DRBG and rand_bytes buffers in secure heap
Read the TSC fewer times (but it's still not enabled).
Short-circuit return in win RAND_poll_ex; other minor tweaks and
format-fixes.
Use the _bytes version of rdrand/rdseed
Fix ia32cap checks.

Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/4100)

diff --git a/crypto/rand/rand_lcl.h b/crypto/rand/rand_lcl.h
index c96625456df9ff28f42b371998a88861aa571dfd..e60f619d61335f5c5dcf7d0cd6e305fa16f35687 100644 (file)
--- a/crypto/rand/rand_lcl.h
+++ b/crypto/rand/rand_lcl.h
@@ -25,6 +25,9 @@
  */
 # define RANDOMNESS_NEEDED              16
 
+/* How many times to read the TSC as a randomness source. */
+# define TSC_READ_COUNT                 4
+
 /* Maximum amount of randomness to hold in RAND_BYTES_BUFFER. */
 # define MAX_RANDOMNESS_HELD            (4 * RANDOMNESS_NEEDED)
 
@@ -57,9 +60,10 @@ typedef enum drbg_status_e {
  */
 typedef struct rand_bytes_buffer_st {
     CRYPTO_RWLOCK *lock;
+    unsigned char *buff;
     size_t size;
     size_t curr;
-    unsigned char *buff;
+    int secure;
 } RAND_BYTES_BUFFER;
 
 /*
@@ -90,7 +94,8 @@ struct rand_drbg_st {
     int nid; /* the underlying algorithm */
     int fork_count;
     unsigned short flags; /* various external flags */
-    unsigned short filled;
+    char filled;
+    char secure;
     /*
      * This is a fixed-size buffer, but we malloc to make it a little
      * harder to find; a classic security/performance trade-off.

diff --git a/crypto/rand/rand_lib.c b/crypto/rand/rand_lib.c
index 0810518cc6a027a2c5764bf5fa27ca189df6544f..489b5380c9876eeb233032e058a596badf2096c7 100644 (file)
--- a/crypto/rand/rand_lib.c
+++ b/crypto/rand/rand_lib.c
@@ -30,8 +30,8 @@ int rand_fork_count;
 #ifdef OPENSSL_RAND_SEED_RDTSC
 /*
  * IMPORTANT NOTE:  It is not currently possible to use this code
- * because we are not sure about the amount of randomness.  Some
- * SP900 tests have been run, but there is internal skepticism.
+ * because we are not sure about the amount of randomness it provides.
+ * Some SP900 tests have been run, but there is internal skepticism.
  * So for now this code is not used.
  */
 # error "RDTSC enabled?  Should not be possible!"
@@ -47,45 +47,39 @@ void rand_read_tsc(RAND_poll_fn cb, void *arg)
     unsigned char c;
     int i;
 
-    for (i = 0; i < 10; i++) {
-        c = (unsigned char)(OPENSSL_rdtsc() & 0xFF);
-        cb(arg, &c, 1, 0.5);
+    if ((OPENSSL_ia32cap_P[0] & (1 << 4)) != 0) {
+        for (i = 0; i < TSC_READ_COUNT; i++) {
+            c = (unsigned char)(OPENSSL_rdtsc() & 0xFF);
+            cb(arg, &c, 1, 0.5);
+        }
     }
 }
 #endif
 
 #ifdef OPENSSL_RAND_SEED_RDCPU
-size_t OPENSSL_ia32_rdseed(void);
-size_t OPENSSL_ia32_rdrand(void);
+size_t OPENSSL_ia32_rdseed_bytes(char *buf, size_t len);
+size_t OPENSSL_ia32_rdrand_bytes(char *buf, size_t len);
 
 extern unsigned int OPENSSL_ia32cap_P[];
 
 int rand_read_cpu(RAND_poll_fn cb, void *arg)
 {
-    size_t i, s;
+    char buff[RANDOMNESS_NEEDED];
 
     /* If RDSEED is available, use that. */
-    if ((OPENSSL_ia32cap_P[1] & (1 << 18)) != 0) {
-        for (i = 0; i < RANDOMNESS_NEEDED; i += sizeof(s)) {
-            s = OPENSSL_ia32_rdseed();
-            if (s == 0)
-                break;
-            cb(arg, &s, (int)sizeof(s), sizeof(s));
-        }
-        if (i >= RANDOMNESS_NEEDED)
+    if ((OPENSSL_ia32cap_P[2] & (1 << 18)) != 0) {
+        if (OPENSSL_ia32_rdseed_bytes(buff, sizeof(buff)) == sizeof(buff)) {
+            cb(arg, buff, (int)sizeof(buff), sizeof(buff));
             return 1;
+        }
     }
 
     /* Second choice is RDRAND. */
     if ((OPENSSL_ia32cap_P[1] & (1 << (62 - 32))) != 0) {
-        for (i = 0; i < RANDOMNESS_NEEDED; i += sizeof(s)) {
-            s = OPENSSL_ia32_rdrand();
-            if (s == 0)
-                break;
-            cb(arg, &s, (int)sizeof(s), sizeof(s));
-        }
-        if (i >= RANDOMNESS_NEEDED)
+        if (OPENSSL_ia32_rdrand_bytes(buff, sizeof(buff)) == sizeof(buff)) {
+            cb(arg, buff, (int)sizeof(buff), sizeof(buff));
             return 1;
+        }
     }
 
     return 0;
@@ -186,7 +180,10 @@ static int setup_drbg(RAND_DRBG *drbg)
     drbg->lock = CRYPTO_THREAD_lock_new();
     ret &= drbg->lock != NULL;
     drbg->size = RANDOMNESS_NEEDED;
-    drbg->randomness = OPENSSL_malloc(drbg->size);
+    drbg->secure = CRYPTO_secure_malloc_initialized();
+    drbg->randomness = drbg->secure
+        ? OPENSSL_secure_malloc(drbg->size)
+        : OPENSSL_malloc(drbg->size);
     ret &= drbg->randomness != NULL;
     /* If you change these parameters, see RANDOMNESS_NEEDED */
     ret &= RAND_DRBG_set(drbg,
@@ -199,7 +196,10 @@ static int setup_drbg(RAND_DRBG *drbg)
 static void free_drbg(RAND_DRBG *drbg)
 {
     CRYPTO_THREAD_lock_free(drbg->lock);
-    OPENSSL_clear_free(drbg->randomness, drbg->size);
+    if (drbg->secure)
+        OPENSSL_secure_clear_free(drbg->randomness, drbg->size);
+    else
+        OPENSSL_clear_free(drbg->randomness, drbg->size);
     RAND_DRBG_uninstantiate(drbg);
 }
 
@@ -223,9 +223,10 @@ DEFINE_RUN_ONCE_STATIC(do_rand_init)
     ret &= rand_bytes.lock != NULL;
     rand_bytes.curr = 0;
     rand_bytes.size = MAX_RANDOMNESS_HELD;
-    /* TODO: Should this be secure malloc? */
-    rand_bytes.buff = malloc(rand_bytes.size);
-
+    rand_bytes.secure = CRYPTO_secure_malloc_initialized();
+    rand_bytes.buff = rand_bytes.secure
+        ? OPENSSL_secure_malloc(rand_bytes.size)
+        : OPENSSL_malloc(rand_bytes.size);
     ret &= rand_bytes.buff != NULL;
     ret &= setup_drbg(&rand_drbg);
     ret &= setup_drbg(&priv_drbg);
@@ -244,7 +245,10 @@ void rand_cleanup_int(void)
 #endif
     CRYPTO_THREAD_lock_free(rand_meth_lock);
     CRYPTO_THREAD_lock_free(rand_bytes.lock);
-    OPENSSL_clear_free(rand_bytes.buff, rand_bytes.size);
+    if (rand_bytes.secure)
+        OPENSSL_secure_clear_free(rand_bytes.buff, rand_bytes.size);
+    else
+        OPENSSL_clear_free(rand_bytes.buff, rand_bytes.size);
     free_drbg(&rand_drbg);
     free_drbg(&priv_drbg);
 }

diff --git a/crypto/rand/rand_win.c b/crypto/rand/rand_win.c
index 5685ee84b7a69396254d202dfba66127440120c4..457e2ade39561695edaa844abedf0e6d79e0b968 100644 (file)
--- a/crypto/rand/rand_win.c
+++ b/crypto/rand/rand_win.c
@@ -43,34 +43,35 @@ int RAND_poll_ex(RAND_poll_fn cb, void *arg)
 {
 # ifndef USE_BCRYPTGENRANDOM
     HCRYPTPROV hProvider;
+    int ok = 0;
 # endif
-    DWORD w;
     BYTE buf[RANDOMNESS_NEEDED];
-    int ok = 0;
 
 # ifdef OPENSSL_RAND_SEED_RDTSC
     rand_read_tsc(cb, arg);
 # endif
 # ifdef OPENSSL_RAND_SEED_RDCPU
     if (rand_read_cpu(cb, arg))
-        ok++;
+        return 1;
 # endif
 
 # ifdef USE_BCRYPTGENRANDOM
     if (BCryptGenRandom(NULL, buf, (ULONG)sizeof(buf),
-                        BCRYPT_USE_SYSTEM_PREFERRED_RNG) != STATUS_SUCCESS)
-        return 0;
-    cb(arg, buf, sizeof(buf), sizeof(buf));
-    return 1;
+                        BCRYPT_USE_SYSTEM_PREFERRED_RNG) == STATUS_SUCCESS) {
+        cb(arg, buf, sizeof(buf), sizeof(buf));
+        return 1;
+    }
 # else
     /* poll the CryptoAPI PRNG */
     if (CryptAcquireContextW(&hProvider, NULL, NULL, PROV_RSA_FULL,
                              CRYPT_VERIFYCONTEXT | CRYPT_SILENT) != 0) {
         if (CryptGenRandom(hProvider, (DWORD)sizeof(buf), buf) != 0) {
             cb(arg, buf, sizeof(buf), sizeof(buf));
-            ok++;
+            ok = 1;
         }
         CryptReleaseContext(hProvider, 0);
+        if (ok)
+            return 1;
     }
 
     /* poll the Pentium PRG with CryptoAPI */
@@ -78,16 +79,18 @@ int RAND_poll_ex(RAND_poll_fn cb, void *arg)
                              CRYPT_VERIFYCONTEXT | CRYPT_SILENT) != 0) {
         if (CryptGenRandom(hProvider, (DWORD)sizeof(buf), buf) != 0) {
             cb(arg, buf, sizeof(buf), sizeof(buf));
-            ok++;
+            ok = 1;
         }
         CryptReleaseContext(hProvider, 0);
+        if (ok)
+            return 1;
     }
 # endif
 
-    return ok ? 1 : 0;
+    return 0;
 }
 
-#if OPENSSL_API_COMPAT < 0x10100000L
+# if OPENSSL_API_COMPAT < 0x10100000L
 int RAND_event(UINT iMsg, WPARAM wParam, LPARAM lParam)
 {
     RAND_poll();
@@ -98,6 +101,6 @@ void RAND_screen(void)
 {
     RAND_poll();
 }
-#endif
+# endif
 
 #endif