[openssl-commits] [openssl] master update

matthias.st.pierre at ncp-e.com matthias.st.pierre at ncp-e.com
Tue Oct 16 20:24:17 UTC 2018


The branch master has been updated
       via  3064b55134434a0b2850f07eff57120f35bb269a (commit)
      from  628ee796389b555ddb5fc28355e16e9417ab1724 (commit)


- Log -----------------------------------------------------------------
commit 3064b55134434a0b2850f07eff57120f35bb269a
Author: Dr. Matthias St. Pierre <Matthias.St.Pierre at ncp-e.com>
Date:   Wed Oct 10 01:53:29 2018 +0200

    DRBG: fix reseeding via RAND_add()/RAND_seed() with large input
    
    In pull request #4328 the seeding of the DRBG via RAND_add()/RAND_seed()
    was implemented by buffering the data in a random pool where it is
    picked up later by the rand_drbg_get_entropy() callback. This buffer
    was limited to the size of 4096 bytes.
    
    When a larger input was added via RAND_add() or RAND_seed() to the DRBG,
    the reseeding failed, but the error returned by the DRBG was ignored
    by the two calling functions, which both don't return an error code.
    As a consequence, the data provided by the application was effectively
    ignored.
    
    This commit fixes the problem by a more efficient implementation which
    does not copy the data in memory and by raising the buffer the size limit
    to INT32_MAX (2 gigabytes). This is less than the NIST limit of 2^35 bits
    but it was chosen intentionally to avoid platform dependent problems
    like integer sizes and/or signed/unsigned conversion.
    
    Additionally, the DRBG is now less permissive on errors: In addition to
    pushing a message to the openssl error stack, it enters the error state,
    which forces a reinstantiation on next call.
    
    Thanks go to Dr. Falko Strenzke for reporting this issue to the
    openssl-security mailing list. After internal discussion the issue
    has been categorized as not being security relevant, because the DRBG
    reseeds automatically and is fully functional even without additional
    randomness provided by the application.
    
    Fixes #7381
    
    Reviewed-by: Paul Dale <paul.dale at oracle.com>
    (Merged from https://github.com/openssl/openssl/pull/7382)

-----------------------------------------------------------------------

Summary of changes:
 CHANGES                            | 12 +++++-
 crypto/err/openssl.txt             |  1 +
 crypto/include/internal/rand_int.h |  2 +
 crypto/rand/drbg_ctr.c             |  4 +-
 crypto/rand/drbg_hash.c            |  4 +-
 crypto/rand/drbg_hmac.c            |  4 +-
 crypto/rand/drbg_lib.c             | 17 +++++---
 crypto/rand/rand_err.c             |  1 +
 crypto/rand/rand_lcl.h             | 46 +++++++++++++++++-----
 crypto/rand/rand_lib.c             | 79 ++++++++++++++++++++++++++++----------
 crypto/rand/rand_unix.c            |  2 +-
 include/openssl/randerr.h          |  1 +
 test/drbgtest.c                    |  1 -
 13 files changed, 128 insertions(+), 46 deletions(-)

diff --git a/CHANGES b/CHANGES
index a1fa57c..aa00369 100644
--- a/CHANGES
+++ b/CHANGES
@@ -25,6 +25,17 @@
      list of built in objects, i.e. OIDs with names.
      [Richard Levitte]
 
+ Changes between 1.1.1 and 1.1.1a [xx XXX xxxx]
+
+  *) Fixed the issue that RAND_add()/RAND_seed() silently discards random input
+     if its length exceeds 4096 bytes. The limit has been raised to a buffer size
+     of two gigabytes and the error handling improved.
+
+     This issue was reported to OpenSSL by Dr. Falko Strenzke. It has been
+     categorized as a normal bug, not a security issue, because the DRBG reseeds
+     automatically and is fully functional even without additional randomness
+     provided by the application.
+
  Changes between 1.1.0i and 1.1.1 [11 Sep 2018]
 
   *) Add a new ClientHello callback. Provides a callback interface that gives
@@ -13121,4 +13132,3 @@ des-cbc           3624.96k     5258.21k     5530.91k     5624.30k     5628.26k
   *) A minor bug in ssl/s3_clnt.c where there would always be 4 0
      bytes sent in the client random.
      [Edward Bishop <ebishop at spyglass.com>]
-
diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
index 84e6393..489ccc0 100644
--- a/crypto/err/openssl.txt
+++ b/crypto/err/openssl.txt
@@ -1016,6 +1016,7 @@ RAND_F_RAND_POOL_ACQUIRE_ENTROPY:122:rand_pool_acquire_entropy
 RAND_F_RAND_POOL_ADD:103:rand_pool_add
 RAND_F_RAND_POOL_ADD_BEGIN:113:rand_pool_add_begin
 RAND_F_RAND_POOL_ADD_END:114:rand_pool_add_end
+RAND_F_RAND_POOL_ATTACH:124:rand_pool_attach
 RAND_F_RAND_POOL_BYTES_NEEDED:115:rand_pool_bytes_needed
 RAND_F_RAND_POOL_NEW:116:rand_pool_new
 RAND_F_RAND_WRITE_FILE:112:RAND_write_file
diff --git a/crypto/include/internal/rand_int.h b/crypto/include/internal/rand_int.h
index d91ee4c..3c966ab 100644
--- a/crypto/include/internal/rand_int.h
+++ b/crypto/include/internal/rand_int.h
@@ -53,6 +53,8 @@ void rand_drbg_cleanup_additional_data(unsigned char *out, size_t outlen);
  * RAND_POOL functions
  */
 RAND_POOL *rand_pool_new(int entropy_requested, size_t min_len, size_t max_len);
+RAND_POOL *rand_pool_attach(const unsigned char *buffer, size_t len,
+                            size_t entropy);
 void rand_pool_free(RAND_POOL *pool);
 
 const unsigned char *rand_pool_buffer(RAND_POOL *pool);
diff --git a/crypto/rand/drbg_ctr.c b/crypto/rand/drbg_ctr.c
index 0548237..894c77d 100644
--- a/crypto/rand/drbg_ctr.c
+++ b/crypto/rand/drbg_ctr.c
@@ -416,9 +416,9 @@ int drbg_ctr_init(RAND_DRBG *drbg)
             return 0;
 
         drbg->min_entropylen = ctr->keylen;
-        drbg->max_entropylen = DRBG_MINMAX_FACTOR * drbg->min_entropylen;
+        drbg->max_entropylen = DRBG_MAX_LENGTH;
         drbg->min_noncelen = drbg->min_entropylen / 2;
-        drbg->max_noncelen = DRBG_MINMAX_FACTOR * drbg->min_noncelen;
+        drbg->max_noncelen = DRBG_MAX_LENGTH;
         drbg->max_perslen = DRBG_MAX_LENGTH;
         drbg->max_adinlen = DRBG_MAX_LENGTH;
     } else {
diff --git a/crypto/rand/drbg_hash.c b/crypto/rand/drbg_hash.c
index 9caf5b2..cae567b 100644
--- a/crypto/rand/drbg_hash.c
+++ b/crypto/rand/drbg_hash.c
@@ -332,10 +332,10 @@ int drbg_hash_init(RAND_DRBG *drbg)
         drbg->seedlen = HASH_PRNG_SMALL_SEEDLEN;
 
     drbg->min_entropylen = drbg->strength / 8;
-    drbg->max_entropylen = DRBG_MINMAX_FACTOR * drbg->min_entropylen;
+    drbg->max_entropylen = DRBG_MAX_LENGTH;
 
     drbg->min_noncelen = drbg->min_entropylen / 2;
-    drbg->max_noncelen = DRBG_MINMAX_FACTOR * drbg->min_noncelen;
+    drbg->max_noncelen = DRBG_MAX_LENGTH;
 
     drbg->max_perslen = DRBG_MAX_LENGTH;
     drbg->max_adinlen = DRBG_MAX_LENGTH;
diff --git a/crypto/rand/drbg_hmac.c b/crypto/rand/drbg_hmac.c
index 25c5b03..424c88c 100644
--- a/crypto/rand/drbg_hmac.c
+++ b/crypto/rand/drbg_hmac.c
@@ -223,10 +223,10 @@ int drbg_hmac_init(RAND_DRBG *drbg)
     drbg->seedlen = hmac->blocklen;
 
     drbg->min_entropylen = drbg->strength / 8;
-    drbg->max_entropylen = DRBG_MINMAX_FACTOR * drbg->min_entropylen;
+    drbg->max_entropylen = DRBG_MAX_LENGTH;
 
     drbg->min_noncelen = drbg->min_entropylen / 2;
-    drbg->max_noncelen = DRBG_MINMAX_FACTOR * drbg->min_noncelen;
+    drbg->max_noncelen = DRBG_MAX_LENGTH;
 
     drbg->max_perslen = DRBG_MAX_LENGTH;
     drbg->max_adinlen = DRBG_MAX_LENGTH;
diff --git a/crypto/rand/drbg_lib.c b/crypto/rand/drbg_lib.c
index c59f3f0..36b20c9 100644
--- a/crypto/rand/drbg_lib.c
+++ b/crypto/rand/drbg_lib.c
@@ -556,8 +556,10 @@ int rand_drbg_restart(RAND_DRBG *drbg,
 
     if (drbg->pool != NULL) {
         RANDerr(RAND_F_RAND_DRBG_RESTART, ERR_R_INTERNAL_ERROR);
+        drbg->state = DRBG_ERROR;
         rand_pool_free(drbg->pool);
         drbg->pool = NULL;
+        return 0;
     }
 
     if (buffer != NULL) {
@@ -565,24 +567,25 @@ int rand_drbg_restart(RAND_DRBG *drbg,
             if (drbg->max_entropylen < len) {
                 RANDerr(RAND_F_RAND_DRBG_RESTART,
                     RAND_R_ENTROPY_INPUT_TOO_LONG);
+                drbg->state = DRBG_ERROR;
                 return 0;
             }
 
             if (entropy > 8 * len) {
                 RANDerr(RAND_F_RAND_DRBG_RESTART, RAND_R_ENTROPY_OUT_OF_RANGE);
+                drbg->state = DRBG_ERROR;
                 return 0;
             }
 
             /* will be picked up by the rand_drbg_get_entropy() callback */
-            drbg->pool = rand_pool_new(entropy, len, len);
+            drbg->pool = rand_pool_attach(buffer, len, entropy);
             if (drbg->pool == NULL)
                 return 0;
-
-            rand_pool_add(drbg->pool, buffer, len, entropy);
         } else {
             if (drbg->max_adinlen < len) {
                 RANDerr(RAND_F_RAND_DRBG_RESTART,
                         RAND_R_ADDITIONAL_INPUT_TOO_LONG);
+                drbg->state = DRBG_ERROR;
                 return 0;
             }
             adin = buffer;
@@ -1040,14 +1043,16 @@ static int drbg_add(const void *buf, int num, double randomness)
     if (num < 0 || randomness < 0.0)
         return 0;
 
-    if (randomness > (double)drbg->max_entropylen) {
+    if (randomness > (double)RAND_DRBG_STRENGTH) {
         /*
          * The purpose of this check is to bound |randomness| by a
          * relatively small value in order to prevent an integer
          * overflow when multiplying by 8 in the rand_drbg_restart()
-         * call below.
+         * call below. Note that randomness is measured in bytes,
+         * not bits, so this value corresponds to eight times the
+         * security strength.
          */
-        return 0;
+        randomness = (double)RAND_DRBG_STRENGTH;
     }
 
     rand_drbg_lock(drbg);
diff --git a/crypto/rand/rand_err.c b/crypto/rand/rand_err.c
index 31480a6..6a87045 100644
--- a/crypto/rand/rand_err.c
+++ b/crypto/rand/rand_err.c
@@ -44,6 +44,7 @@ static const ERR_STRING_DATA RAND_str_functs[] = {
     {ERR_PACK(ERR_LIB_RAND, RAND_F_RAND_POOL_ADD_BEGIN, 0),
      "rand_pool_add_begin"},
     {ERR_PACK(ERR_LIB_RAND, RAND_F_RAND_POOL_ADD_END, 0), "rand_pool_add_end"},
+    {ERR_PACK(ERR_LIB_RAND, RAND_F_RAND_POOL_ATTACH, 0), "rand_pool_attach"},
     {ERR_PACK(ERR_LIB_RAND, RAND_F_RAND_POOL_BYTES_NEEDED, 0),
      "rand_pool_bytes_needed"},
     {ERR_PACK(ERR_LIB_RAND, RAND_F_RAND_POOL_NEW, 0), "rand_pool_new"},
diff --git a/crypto/rand/rand_lcl.h b/crypto/rand/rand_lcl.h
index 7b1c74d..13f8cc8 100644
--- a/crypto/rand/rand_lcl.h
+++ b/crypto/rand/rand_lcl.h
@@ -32,18 +32,42 @@
 
 
 
-/* Max size of additional input and personalization string. */
-# define DRBG_MAX_LENGTH                4096
+/*
+ * Maximum input size for the DRBG (entropy, nonce, personalization string)
+ *
+ * NIST SP800 90Ar1 allows a maximum of (1 << 35) bits i.e., (1 << 32) bytes.
+ *
+ * We lower it to 'only' INT32_MAX bytes, which is equivalent to 2 gigabytes.
+ */
+# define DRBG_MAX_LENGTH                         INT32_MAX
+
+
 
 /*
- * The quotient between max_{entropy,nonce}len and min_{entropy,nonce}len
+ * Maximum allocation size for RANDOM_POOL buffers
  *
- * The current factor is large enough that the RAND_POOL can store a
- * random input which has a lousy entropy rate of 0.0625 bits per byte.
- * This input will be sent through the derivation function which 'compresses'
- * the low quality input into a high quality output.
+ * The max_len value for the buffer provided to the rand_drbg_get_entropy()
+ * callback is currently 2^31 bytes (2 gigabytes), if a derivation function
+ * is used. Since this is much too large to be allocated, the rand_pool_new()
+ * function chooses more modest values as default pool length, bounded
+ * by RAND_POOL_MIN_LENGTH and RAND_POOL_MAX_LENGTH
+ *
+ * The choice of the RAND_POOL_FACTOR is large enough such that the
+ * RAND_POOL can store a random input which has a lousy entropy rate of
+ * 8/256 (= 0.03125) bits per byte. This input will be sent through the
+ * derivation function which 'compresses' the low quality input into a
+ * high quality output.
+ *
+ * The factor 1.5 below is the pessimistic estimate for the extra amount
+ * of entropy required when no get_nonce() callback is defined.
+ */
+# define RAND_POOL_FACTOR        256
+# define RAND_POOL_MAX_LENGTH    (RAND_POOL_FACTOR * \
+                                  3 * (RAND_DRBG_STRENGTH / 16))
+/*
+ *                             = (RAND_POOL_FACTOR * \
+ *                                1.5 * (RAND_DRBG_STRENGTH / 8))
  */
-# define DRBG_MINMAX_FACTOR              128
 
 
 /* DRBG status values */
@@ -142,10 +166,12 @@ struct rand_pool_st {
     unsigned char *buffer;  /* points to the beginning of the random pool */
     size_t len; /* current number of random bytes contained in the pool */
 
+    int attached;  /* true pool was attached to existing buffer */
+
     size_t min_len; /* minimum number of random bytes requested */
     size_t max_len; /* maximum number of random bytes (allocated buffer size) */
     size_t entropy; /* current entropy count in bits */
-    size_t requested_entropy; /* requested entropy count in bits */
+    size_t entropy_requested; /* requested entropy count in bits */
 };
 
 /*
@@ -167,7 +193,7 @@ struct rand_drbg_st {
     unsigned short flags; /* various external flags */
 
     /*
-     * The random pool is used by RAND_add()/drbg_add() to attach random
+     * The random_data is used by RAND_add()/drbg_add() to attach random
      * data to the global drbg, such that the rand_drbg_get_entropy() callback
      * can pull it during instantiation and reseeding. This is necessary to
      * reconcile the different philosophies of the RAND and the RAND_DRBG
diff --git a/crypto/rand/rand_lib.c b/crypto/rand/rand_lib.c
index e9bc952..f40c513 100644
--- a/crypto/rand/rand_lib.c
+++ b/crypto/rand/rand_lib.c
@@ -146,17 +146,11 @@ size_t rand_drbg_get_entropy(RAND_DRBG *drbg,
         return 0;
     }
 
-    pool = rand_pool_new(entropy, min_len, max_len);
-    if (pool == NULL)
-        return 0;
-
-    if (drbg->pool) {
-        rand_pool_add(pool,
-                      rand_pool_buffer(drbg->pool),
-                      rand_pool_length(drbg->pool),
-                      rand_pool_entropy(drbg->pool));
-        rand_pool_free(drbg->pool);
-        drbg->pool = NULL;
+    if (drbg->pool != NULL) {
+        pool = drbg->pool;
+        pool->entropy_requested = entropy;
+    } else {
+        pool = rand_pool_new(entropy, min_len, max_len);
     }
 
     if (drbg->parent) {
@@ -217,7 +211,10 @@ size_t rand_drbg_get_entropy(RAND_DRBG *drbg,
 void rand_drbg_cleanup_entropy(RAND_DRBG *drbg,
                                unsigned char *out, size_t outlen)
 {
-    OPENSSL_secure_clear_free(out, outlen);
+    if (drbg->pool == NULL)
+        OPENSSL_secure_clear_free(out, outlen);
+    else
+        drbg->pool = NULL;
 }
 
 
@@ -405,7 +402,7 @@ int RAND_poll(void)
         /* fill random pool and seed the current legacy RNG */
         pool = rand_pool_new(RAND_DRBG_STRENGTH,
                              RAND_DRBG_STRENGTH / 8,
-                             DRBG_MINMAX_FACTOR * (RAND_DRBG_STRENGTH / 8));
+                             RAND_POOL_MAX_LENGTH);
         if (pool == NULL)
             return 0;
 
@@ -430,17 +427,18 @@ err:
  * Allocate memory and initialize a new random pool
  */
 
-RAND_POOL *rand_pool_new(int entropy, size_t min_len, size_t max_len)
+RAND_POOL *rand_pool_new(int entropy_requested, size_t min_len, size_t max_len)
 {
     RAND_POOL *pool = OPENSSL_zalloc(sizeof(*pool));
 
     if (pool == NULL) {
         RANDerr(RAND_F_RAND_POOL_NEW, ERR_R_MALLOC_FAILURE);
-        goto err;
+        return NULL;
     }
 
     pool->min_len = min_len;
-    pool->max_len = max_len;
+    pool->max_len = (max_len > RAND_POOL_MAX_LENGTH) ?
+        RAND_POOL_MAX_LENGTH : max_len;
 
     pool->buffer = OPENSSL_secure_zalloc(pool->max_len);
     if (pool->buffer == NULL) {
@@ -448,7 +446,7 @@ RAND_POOL *rand_pool_new(int entropy, size_t min_len, size_t max_len)
         goto err;
     }
 
-    pool->requested_entropy = entropy;
+    pool->entropy_requested = entropy_requested;
 
     return pool;
 
@@ -458,6 +456,38 @@ err:
 }
 
 /*
+ * Attach new random pool to the given buffer
+ *
+ * This function is intended to be used only for feeding random data
+ * provided by RAND_add() and RAND_seed() into the <master> DRBG.
+ */
+RAND_POOL *rand_pool_attach(const unsigned char *buffer, size_t len,
+                            size_t entropy)
+{
+    RAND_POOL *pool = OPENSSL_zalloc(sizeof(*pool));
+
+    if (pool == NULL) {
+        RANDerr(RAND_F_RAND_POOL_ATTACH, ERR_R_MALLOC_FAILURE);
+        return NULL;
+    }
+
+    /*
+     * The const needs to be cast away, but attached buffers will not be
+     * modified (in contrary to allocated buffers which are zeroed and
+     * freed in the end).
+     */
+    pool->buffer = (unsigned char *) buffer;
+    pool->len = len;
+
+    pool->attached = 1;
+
+    pool->min_len = pool->max_len = pool->len;
+    pool->entropy = entropy;
+
+    return pool;
+}
+
+/*
  * Free |pool|, securely erasing its buffer.
  */
 void rand_pool_free(RAND_POOL *pool)
@@ -465,7 +495,14 @@ void rand_pool_free(RAND_POOL *pool)
     if (pool == NULL)
         return;
 
-    OPENSSL_secure_clear_free(pool->buffer, pool->max_len);
+    /*
+     * Although it would be advisable from a cryptographical viewpoint,
+     * we are not allowed to clear attached buffers, since they are passed
+     * to rand_pool_attach() as `const unsigned char*`.
+     * (see corresponding comment in rand_pool_attach()).
+     */
+    if (!pool->attached)
+        OPENSSL_secure_clear_free(pool->buffer, pool->max_len);
     OPENSSL_free(pool);
 }
 
@@ -524,7 +561,7 @@ unsigned char *rand_pool_detach(RAND_POOL *pool)
  */
 size_t rand_pool_entropy_available(RAND_POOL *pool)
 {
-    if (pool->entropy < pool->requested_entropy)
+    if (pool->entropy < pool->entropy_requested)
         return 0;
 
     if (pool->len < pool->min_len)
@@ -540,8 +577,8 @@ size_t rand_pool_entropy_available(RAND_POOL *pool)
 
 size_t rand_pool_entropy_needed(RAND_POOL *pool)
 {
-    if (pool->entropy < pool->requested_entropy)
-        return pool->requested_entropy - pool->entropy;
+    if (pool->entropy < pool->entropy_requested)
+        return pool->entropy_requested - pool->entropy;
 
     return 0;
 }
diff --git a/crypto/rand/rand_unix.c b/crypto/rand/rand_unix.c
index e53b876..cb3a6b2 100644
--- a/crypto/rand/rand_unix.c
+++ b/crypto/rand/rand_unix.c
@@ -264,7 +264,7 @@ static ssize_t syscall_random(void *buf, size_t buflen)
      * Note: 'buflen' equals the size of the buffer which is used by the
      * get_entropy() callback of the RAND_DRBG. It is roughly bounded by
      *
-     *   2 * DRBG_MINMAX_FACTOR * (RAND_DRBG_STRENGTH / 8) = 2^13
+     *   2 * RAND_POOL_FACTOR * (RAND_DRBG_STRENGTH / 8) = 2^14
      *
      * which is way below the OSSL_SSIZE_MAX limit. Therefore sign conversion
      * between size_t and ssize_t is safe even without a range check.
diff --git a/include/openssl/randerr.h b/include/openssl/randerr.h
index 128f4de..599a2a1 100644
--- a/include/openssl/randerr.h
+++ b/include/openssl/randerr.h
@@ -40,6 +40,7 @@ int ERR_load_RAND_strings(void);
 # define RAND_F_RAND_POOL_ADD                             103
 # define RAND_F_RAND_POOL_ADD_BEGIN                       113
 # define RAND_F_RAND_POOL_ADD_END                         114
+# define RAND_F_RAND_POOL_ATTACH                          124
 # define RAND_F_RAND_POOL_BYTES_NEEDED                    115
 # define RAND_F_RAND_POOL_NEW                             116
 # define RAND_F_RAND_WRITE_FILE                           112
diff --git a/test/drbgtest.c b/test/drbgtest.c
index f2054a8..bb51e01 100644
--- a/test/drbgtest.c
+++ b/test/drbgtest.c
@@ -911,7 +911,6 @@ static size_t get_pool_entropy(RAND_DRBG *drbg,
  */
 static void cleanup_pool_entropy(RAND_DRBG *drbg, unsigned char *out, size_t outlen)
 {
-    OPENSSL_secure_clear_free(drbg->pool->buffer, drbg->pool->max_len);
     OPENSSL_free(drbg->pool);
     drbg->pool = NULL;
 }


More information about the openssl-commits mailing list