Don't auto-instantiate a DRBG when trying to use it and it's not

The one creating the DRBG should instantiate it, it's there that we
know which parameters we should use to instantiate it.

This splits the rand init in two parts to avoid a deadlock
because when the global drbg is created it wands to call
rand_add on the global rand method.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
GH: #4268

diff --git a/crypto/include/internal/rand_int.h b/crypto/include/internal/rand_int.h
index 90b00946a8302150bc272530c66f371aba7bd951..d0999f28adb4b6a2c2042384ee6470d8ec0aaef2 100644 (file)
--- a/crypto/include/internal/rand_int.h
+++ b/crypto/include/internal/rand_int.h
@@ -18,4 +18,5 @@
 #include <openssl/rand.h>
 
 void rand_cleanup_int(void);
+void rand_cleanup_drbg_int(void);
 void rand_fork(void);

diff --git a/crypto/init.c b/crypto/init.c
index c8f0a3f1ee4b870920b80bc144fa12eb7796f5fc..ccfd003bb630c1dfc11169394ce1a8753e5eb10f 100644 (file)
--- a/crypto/init.c
+++ b/crypto/init.c
@@ -488,6 +488,7 @@ void OPENSSL_cleanup(void)
      * obj_cleanup_int() must be called last
      */
     rand_cleanup_int();
+    rand_cleanup_drbg_int();
     conf_modules_free_int();
 #ifndef OPENSSL_NO_ENGINE
     engine_cleanup_int();

diff --git a/crypto/rand/drbg_lib.c b/crypto/rand/drbg_lib.c
index d1f419dddfb7246eaa27aa639677ee63036edff6..3690976051b5991c65a6296479861f45153cda53 100644 (file)
--- a/crypto/rand/drbg_lib.c
+++ b/crypto/rand/drbg_lib.c
@@ -12,6 +12,8 @@
 #include <openssl/err.h>
 #include <openssl/rand.h>
 #include "rand_lcl.h"
+#include "internal/thread_once.h"
+#include "internal/rand_int.h"
 
 /*
  * Support framework for NIST SP 800-90A DRBG, AES-CTR mode.
@@ -25,6 +27,8 @@
  * a much bigger deal than just re-setting an allocated resource.)
  */
 
+static CRYPTO_ONCE rand_init_drbg = CRYPTO_ONCE_STATIC_INIT;
+
 /*
  * Set/initialize |drbg| to be of type |nid|, with optional |flags|.
  * Return -2 if the type is not supported, 1 on success and -1 on
@@ -76,18 +80,9 @@ RAND_DRBG *RAND_DRBG_new(int type, unsigned int flags, RAND_DRBG *parent)
         goto err;
 
     if (parent != NULL) {
-        if (parent->state == DRBG_UNINITIALISED
-                && RAND_DRBG_instantiate(parent, NULL, 0) == 0)
-            goto err;
         if (!RAND_DRBG_set_callbacks(drbg, drbg_entropy_from_parent,
                                      drbg_release_entropy,
-                                     NULL, NULL)
-                /*
-                 * Add in our address.  Note we are adding the pointer
-                 * itself, not its contents!
-                 */
-                || !RAND_DRBG_instantiate(drbg,
-                                          (unsigned char*)&drbg, sizeof(drbg)))
+                                     NULL, NULL))
             goto err;
     }
 
@@ -98,17 +93,12 @@ err:
     return NULL;
 }
 
-RAND_DRBG *RAND_DRBG_get0_global(void)
-{
-    return &rand_drbg;
-}
-
 /*
  * Uninstantiate |drbg| and free all memory.
  */
 void RAND_DRBG_free(RAND_DRBG *drbg)
 {
-    /* The global DRBG is free'd by rand_cleanup_int() */
+    /* The global DRBG is free'd by rand_cleanup_drbg_int() */
     if (drbg == NULL || drbg == &rand_drbg)
         return;
 
@@ -340,28 +330,80 @@ void *RAND_DRBG_get_ex_data(const RAND_DRBG *drbg, int idx)
  * global DRBG.  They lock.
  */
 
+/*
+ * Creates a global DRBG with default settings.
+ * Returns 1 on success, 0 on failure
+ */
+static int setup_drbg(RAND_DRBG *drbg)
+{
+    int ret = 1;
+
+    drbg->lock = CRYPTO_THREAD_lock_new();
+    ret &= drbg->lock != NULL;
+    drbg->size = RANDOMNESS_NEEDED;
+    drbg->secure = CRYPTO_secure_malloc_initialized();
+    /* If you change these parameters, see RANDOMNESS_NEEDED */
+    ret &= RAND_DRBG_set(drbg,
+                         NID_aes_128_ctr, RAND_DRBG_FLAG_CTR_USE_DF) == 1;
+    ret &= RAND_DRBG_set_callbacks(drbg, drbg_entropy_from_system,
+                                   drbg_release_entropy, NULL, NULL) == 1;
+    ret &= RAND_DRBG_instantiate(drbg, NULL, 0) == 1;
+    return ret;
+}
+
+/*
+ * Initialize the global DRBGs on first use.
+ * Returns 1 on success, 0 on failure.
+ */
+DEFINE_RUN_ONCE_STATIC(do_rand_init_drbg)
+{
+    int ret = 1;
+
+    ret &= setup_drbg(&rand_drbg);
+    ret &= setup_drbg(&priv_drbg);
+
+    return ret;
+}
+
+/* Clean up a DRBG and free it */
+static void free_drbg(RAND_DRBG *drbg)
+{
+    CRYPTO_THREAD_lock_free(drbg->lock);
+    RAND_DRBG_uninstantiate(drbg);
+}
+
+/* Clean up the global DRBGs before exit */
+void rand_cleanup_drbg_int(void)
+{
+    free_drbg(&rand_drbg);
+    free_drbg(&priv_drbg);
+}
+
 static int drbg_bytes(unsigned char *out, int count)
 {
     int ret = 0;
     size_t chunk;
+    RAND_DRBG *drbg = RAND_DRBG_get0_global();
 
-    CRYPTO_THREAD_write_lock(rand_drbg.lock);
-    if (rand_drbg.state == DRBG_UNINITIALISED
-            && RAND_DRBG_instantiate(&rand_drbg, NULL, 0) == 0)
+    if (drbg == NULL)
+        return 0;
+
+    CRYPTO_THREAD_write_lock(drbg->lock);
+    if (drbg->state == DRBG_UNINITIALISED)
         goto err;
 
     for ( ; count > 0; count -= chunk, out += chunk) {
         chunk = count;
-        if (chunk > rand_drbg.max_request)
-            chunk = rand_drbg.max_request;
-        ret = RAND_DRBG_generate(&rand_drbg, out, chunk, 0, NULL, 0);
+        if (chunk > drbg->max_request)
+            chunk = drbg->max_request;
+        ret = RAND_DRBG_generate(drbg, out, chunk, 0, NULL, 0);
         if (!ret)
             goto err;
     }
     ret = 1;
 
 err:
-    CRYPTO_THREAD_unlock(rand_drbg.lock);
+    CRYPTO_THREAD_unlock(drbg->lock);
     return ret;
 }
 
@@ -396,15 +438,41 @@ static int drbg_seed(const void *buf, int num)
 static int drbg_status(void)
 {
     int ret;
+    RAND_DRBG *drbg = RAND_DRBG_get0_global();
+
+    if (drbg == NULL)
+        return 0;
 
-    CRYPTO_THREAD_write_lock(rand_drbg.lock);
-    if (rand_drbg.state == DRBG_UNINITIALISED)
-        RAND_DRBG_instantiate(&rand_drbg, NULL, 0);
-    ret = rand_drbg.state == DRBG_READY ? 1 : 0;
-    CRYPTO_THREAD_unlock(rand_drbg.lock);
+    CRYPTO_THREAD_write_lock(drbg->lock);
+    ret = drbg->state == DRBG_READY ? 1 : 0;
+    CRYPTO_THREAD_unlock(drbg->lock);
     return ret;
 }
 
+/*
+ * Get the global public DRBG.
+ * Returns pointer to the DRBG on success, NULL on failure.
+ */
+RAND_DRBG *RAND_DRBG_get0_global(void)
+{
+    if (!RUN_ONCE(&rand_init_drbg, do_rand_init_drbg))
+        return NULL;
+
+    return &rand_drbg;
+}
+
+/*
+ * Get the global private DRBG.
+ * Returns pointer to the DRBG on success, NULL on failure.
+ */
+RAND_DRBG *RAND_DRBG_get0_priv_global(void)
+{
+    if (!RUN_ONCE(&rand_init_drbg, do_rand_init_drbg))
+        return NULL;
+
+    return &priv_drbg;
+}
+
 RAND_DRBG rand_drbg; /* The default global DRBG. */
 RAND_DRBG priv_drbg; /* The global private-key DRBG. */
 

diff --git a/crypto/rand/rand_lib.c b/crypto/rand/rand_lib.c
index 5ed08f1e2396733fcb6ca7eff83d8ec535e6bc69..a27281c6e1a4e560c91d816ad5be82c21e39a504 100644 (file)
--- a/crypto/rand/rand_lib.c
+++ b/crypto/rand/rand_lib.c
@@ -144,7 +144,7 @@ size_t drbg_entropy_from_parent(RAND_DRBG *drbg,
 {
     int st;
     unsigned char *randomness;
-    
+
     if (min_len > (size_t)drbg->size) {
         /* Should not happen.  See comment near RANDOMNESS_NEEDED. */
         min_len = drbg->size;
@@ -172,32 +172,6 @@ void drbg_release_entropy(RAND_DRBG *drbg, unsigned char *out, size_t outlen)
         OPENSSL_clear_free(out, outlen);
 }
 
-
-/*
- * Set up a global DRBG.
- */
-static int setup_drbg(RAND_DRBG *drbg)
-{
-    int ret = 1;
-
-    drbg->lock = CRYPTO_THREAD_lock_new();
-    ret &= drbg->lock != NULL;
-    drbg->size = RANDOMNESS_NEEDED;
-    drbg->secure = CRYPTO_secure_malloc_initialized();
-    /* If you change these parameters, see RANDOMNESS_NEEDED */
-    ret &= RAND_DRBG_set(drbg,
-                         NID_aes_128_ctr, RAND_DRBG_FLAG_CTR_USE_DF) == 1;
-    ret &= RAND_DRBG_set_callbacks(drbg, drbg_entropy_from_system,
-                                   drbg_release_entropy, NULL, NULL) == 1;
-    return ret;
-}
-
-static void free_drbg(RAND_DRBG *drbg)
-{
-    CRYPTO_THREAD_lock_free(drbg->lock);
-    RAND_DRBG_uninstantiate(drbg);
-}
-
 void rand_fork()
 {
     rand_fork_count++;
@@ -223,8 +197,6 @@ DEFINE_RUN_ONCE_STATIC(do_rand_init)
         ? 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);
     return ret;
 }
 
@@ -244,8 +216,6 @@ void rand_cleanup_int(void)
         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);
 }
 
 /*
@@ -357,15 +327,16 @@ void RAND_add(const void *buf, int num, double randomness)
 int RAND_priv_bytes(unsigned char *buf, int num)
 {
     const RAND_METHOD *meth = RAND_get_rand_method();
+    RAND_DRBG *drbg;
 
     if (meth != RAND_OpenSSL())
         return RAND_bytes(buf, num);
 
-    if (priv_drbg.state == DRBG_UNINITIALISED
-            && RAND_DRBG_instantiate(&priv_drbg, NULL, 0) == 0)
+    drbg = RAND_DRBG_get0_priv_global();
+    if (drbg == NULL)
         return 0;
-    return RAND_DRBG_generate(&priv_drbg, buf, num, 0, NULL, 0);
 
+    return RAND_DRBG_generate(drbg, buf, num, 0, NULL, 0);
 }
 
 int RAND_bytes(unsigned char *buf, int num)

diff --git a/include/internal/rand.h b/include/internal/rand.h
index 444c8064759e3cc7f6ae4f254b9dac187438ecf7..07f141d6cc52b2c96e2a5bbca403c1270725a219 100644 (file)
--- a/include/internal/rand.h
+++ b/include/internal/rand.h
@@ -33,6 +33,7 @@ int RAND_DRBG_generate(RAND_DRBG *drbg, unsigned char *out, size_t outlen,
                        const unsigned char *adin, size_t adinlen);
 int RAND_DRBG_set_reseed_interval(RAND_DRBG *drbg, int interval);
 RAND_DRBG *RAND_DRBG_get0_global(void);
+RAND_DRBG *RAND_DRBG_get0_priv_global(void);
 
 /*
  * EXDATA

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index ed2113caa5f76559689964277bc7ebaefb2680b4..501a12ce502056986630a487f435aa53bfa73d02 100644 (file)
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -630,7 +630,8 @@ SSL *SSL_new(SSL_CTX *ctx)
     if (RAND_get_rand_method() == RAND_OpenSSL()) {
         s->drbg = RAND_DRBG_new(NID_aes_128_ctr, RAND_DRBG_FLAG_CTR_USE_DF,
                                 RAND_DRBG_get0_global());
-        if (s->drbg == NULL) {
+        if (s->drbg == NULL
+            || RAND_DRBG_instantiate(s->drbg, NULL, 0) == 0) {
             CRYPTO_THREAD_lock_free(s->lock);
             goto err;
         }

diff --git a/util/libcrypto.num b/util/libcrypto.num
index c7f1905d44c4ed37faefd824ae06d2c3d75fff28..df89345c6c1f027d93bb0f9be7ea7515bc18fa0a 100644 (file)
--- a/util/libcrypto.num
+++ b/util/libcrypto.num
@@ -4382,3 +4382,4 @@ ASN1_TIME_compare                       4325      1_1_1   EXIST::FUNCTION:
 EVP_PKEY_CTX_ctrl_uint64                4326   1_1_1   EXIST::FUNCTION:
 EVP_DigestFinalXOF                      4327   1_1_1   EXIST::FUNCTION:
 ERR_clear_last_mark                     4328   1_1_1   EXIST::FUNCTION:
+RAND_DRBG_get0_priv_global              4329   1_1_1   EXIST::FUNCTION: