[openssl] master update

Matt Caswell matt at openssl.org
Tue Sep 10 09:06:49 UTC 2019


The branch master has been updated
       via  fa3eb248e29ca8031e6a14e8a2c6f3cd58b5450e (commit)
       via  e301c147a763f67dcc5ba63eb7e2ae40d83a68aa (commit)
      from  1d3cd983f56e0a580ee4216692ee3c9c7bf14de9 (commit)


- Log -----------------------------------------------------------------
commit fa3eb248e29ca8031e6a14e8a2c6f3cd58b5450e
Author: Bernd Edlinger <bernd.edlinger at hotmail.de>
Date:   Fri Sep 6 21:54:13 2019 +0200

    Fix a potential crash in rand_unix.c
    
    Due to the dynamic allocation that was added to rand_pool_add_begin
    this function could now return a null pointer where it was previously
    guaranteed to succeed. But the return value of this function does
    not need to be checked by design.
    
    Move rand_pool_grow from rand_pool_add_begin to rand_pool_bytes_needed.
    Make an allocation error persistent to avoid falling back to less secure
    or blocking entropy sources.
    
    Fixes: a6a66e4511ee ("Make rand_pool buffers more dynamic in their sizing.")
    
    Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre at ncp-e.com>
    Reviewed-by: Paul Dale <paul.dale at oracle.com>
    (Merged from https://github.com/openssl/openssl/pull/9687)

commit e301c147a763f67dcc5ba63eb7e2ae40d83a68aa
Author: Bernd Edlinger <bernd.edlinger at hotmail.de>
Date:   Sat Aug 24 11:38:32 2019 +0200

    Fix a strict warnings error in rand_pool_acquire_entropy
    
    There was a warning about unused variables in this config:
    ./config --strict-warnings --with-rand-seed=rdcpu
    
    Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre at ncp-e.com>
    Reviewed-by: Paul Dale <paul.dale at oracle.com>
    (Merged from https://github.com/openssl/openssl/pull/9687)

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

Summary of changes:
 crypto/rand/rand_lib.c  | 115 +++++++++++++++++++++++++++++++++++-------------
 crypto/rand/rand_unix.c |  39 +++++++++-------
 2 files changed, 106 insertions(+), 48 deletions(-)

diff --git a/crypto/rand/rand_lib.c b/crypto/rand/rand_lib.c
index c865ece978..1ab2a8246c 100644
--- a/crypto/rand/rand_lib.c
+++ b/crypto/rand/rand_lib.c
@@ -545,6 +545,42 @@ size_t rand_pool_entropy_needed(RAND_POOL *pool)
     return 0;
 }
 
+/* Increase the allocation size -- not usable for an attached pool */
+static int rand_pool_grow(RAND_POOL *pool, size_t len)
+{
+    if (len > pool->alloc_len - pool->len) {
+        unsigned char *p;
+        const size_t limit = pool->max_len / 2;
+        size_t newlen = pool->alloc_len;
+
+        if (pool->attached || len > pool->max_len - pool->len) {
+            RANDerr(RAND_F_RAND_POOL_GROW, ERR_R_INTERNAL_ERROR);
+            return 0;
+        }
+
+        do
+            newlen = newlen < limit ? newlen * 2 : pool->max_len;
+        while (len > newlen - pool->len);
+
+        if (pool->secure)
+            p = OPENSSL_secure_zalloc(newlen);
+        else
+            p = OPENSSL_zalloc(newlen);
+        if (p == NULL) {
+            RANDerr(RAND_F_RAND_POOL_GROW, ERR_R_MALLOC_FAILURE);
+            return 0;
+        }
+        memcpy(p, pool->buffer, pool->len);
+        if (pool->secure)
+            OPENSSL_secure_clear_free(pool->buffer, pool->alloc_len);
+        else
+            OPENSSL_clear_free(pool->buffer, pool->alloc_len);
+        pool->buffer = p;
+        pool->alloc_len = newlen;
+    }
+    return 1;
+}
+
 /*
  * Returns the number of bytes needed to fill the pool, assuming
  * the input has 1 / |entropy_factor| entropy bits per data bit.
@@ -574,6 +610,24 @@ size_t rand_pool_bytes_needed(RAND_POOL *pool, unsigned int entropy_factor)
         /* to meet the min_len requirement */
         bytes_needed = pool->min_len - pool->len;
 
+    /*
+     * Make sure the buffer is large enough for the requested amount
+     * of data. This guarantees that existing code patterns where
+     * rand_pool_add_begin, rand_pool_add_end or rand_pool_add
+     * are used to collect entropy data without any error handling
+     * whatsoever, continue to be valid.
+     * Furthermore if the allocation here fails once, make sure that
+     * we don't fall back to a less secure or even blocking random source,
+     * as that could happen by the existing code patterns.
+     * This is not a concern for additional data, therefore that
+     * is not needed if rand_pool_grow fails in other places.
+     */
+    if (!rand_pool_grow(pool, bytes_needed)) {
+        /* persistent error for this pool */
+        pool->max_len = pool->len = 0;
+        return 0;
+    }
+
     return bytes_needed;
 }
 
@@ -583,36 +637,6 @@ size_t rand_pool_bytes_remaining(RAND_POOL *pool)
     return pool->max_len - pool->len;
 }
 
-static int rand_pool_grow(RAND_POOL *pool, size_t len)
-{
-    if (len > pool->alloc_len - pool->len) {
-        unsigned char *p;
-        const size_t limit = pool->max_len / 2;
-        size_t newlen = pool->alloc_len;
-
-        do
-            newlen = newlen < limit ? newlen * 2 : pool->max_len;
-        while (len > newlen - pool->len);
-
-        if (pool->secure)
-            p = OPENSSL_secure_zalloc(newlen);
-        else
-            p = OPENSSL_zalloc(newlen);
-        if (p == NULL) {
-            RANDerr(RAND_F_RAND_POOL_GROW, ERR_R_MALLOC_FAILURE);
-            return 0;
-        }
-        memcpy(p, pool->buffer, pool->len);
-        if (pool->secure)
-            OPENSSL_secure_clear_free(pool->buffer, pool->alloc_len);
-        else
-            OPENSSL_clear_free(pool->buffer, pool->alloc_len);
-        pool->buffer = p;
-        pool->alloc_len = newlen;
-    }
-    return 1;
-}
-
 /*
  * Add random bytes to the random pool.
  *
@@ -636,6 +660,25 @@ int rand_pool_add(RAND_POOL *pool,
     }
 
     if (len > 0) {
+        /*
+         * This is to protect us from accidentally passing the buffer
+         * returned from rand_pool_add_begin.
+         * The check for alloc_len makes sure we do not compare the
+         * address of the end of the allocated memory to something
+         * different, since that comparison would have an
+         * indeterminate result.
+         */
+        if (pool->alloc_len > pool->len && pool->buffer + pool->len == buffer) {
+            RANDerr(RAND_F_RAND_POOL_ADD, ERR_R_INTERNAL_ERROR);
+            return 0;
+        }
+        /*
+         * We have that only for cases when a pool is used to collect
+         * additional data.
+         * For entropy data, as long as the allocation request stays within
+         * the limits given by rand_pool_bytes_needed this rand_pool_grow
+         * below is guaranteed to succeed, thus no allocation happens.
+         */
         if (!rand_pool_grow(pool, len))
             return 0;
         memcpy(pool->buffer + pool->len, buffer, len);
@@ -673,8 +716,18 @@ unsigned char *rand_pool_add_begin(RAND_POOL *pool, size_t len)
         return NULL;
     }
 
+    /*
+     * As long as the allocation request stays within the limits given
+     * by rand_pool_bytes_needed this rand_pool_grow below is guaranteed
+     * to succeed, thus no allocation happens.
+     * We have that only for cases when a pool is used to collect
+     * additional data. Then the buffer might need to grow here,
+     * and of course the caller is responsible to check the return
+     * value of this function.
+     */
     if (!rand_pool_grow(pool, len))
         return NULL;
+
     return pool->buffer + pool->len;
 }
 
@@ -689,7 +742,7 @@ unsigned char *rand_pool_add_begin(RAND_POOL *pool, size_t len)
  */
 int rand_pool_add_end(RAND_POOL *pool, size_t len, size_t entropy)
 {
-    if (len > pool->max_len - pool->len) {
+    if (len > pool->alloc_len - pool->len) {
         RANDerr(RAND_F_RAND_POOL_ADD_END, RAND_R_RANDOM_POOL_OVERFLOW);
         return 0;
     }
diff --git a/crypto/rand/rand_unix.c b/crypto/rand/rand_unix.c
index fb8a7b7c6d..813964665f 100644
--- a/crypto/rand/rand_unix.c
+++ b/crypto/rand/rand_unix.c
@@ -557,12 +557,12 @@ size_t rand_pool_acquire_entropy(RAND_POOL *pool)
 #  if defined(OPENSSL_RAND_SEED_NONE)
     return rand_pool_entropy_available(pool);
 #  else
-    size_t bytes_needed;
-    size_t entropy_available = 0;
-    unsigned char *buffer;
+    size_t entropy_available;
 
 #   if defined(OPENSSL_RAND_SEED_GETRANDOM)
     {
+        size_t bytes_needed;
+        unsigned char *buffer;
         ssize_t bytes;
         /* Maximum allowed number of consecutive unsuccessful attempts */
         int attempts = 3;
@@ -593,6 +593,8 @@ size_t rand_pool_acquire_entropy(RAND_POOL *pool)
 
 #   if defined(OPENSSL_RAND_SEED_DEVRANDOM)
     if (wait_random_seeded()) {
+        size_t bytes_needed;
+        unsigned char *buffer;
         size_t i;
 
         bytes_needed = rand_pool_bytes_needed(pool, 1 /*entropy_factor*/);
@@ -642,26 +644,29 @@ size_t rand_pool_acquire_entropy(RAND_POOL *pool)
 #   endif
 
 #   if defined(OPENSSL_RAND_SEED_EGD)
-    bytes_needed = rand_pool_bytes_needed(pool, 1 /*entropy_factor*/);
-    if (bytes_needed > 0) {
+    {
         static const char *paths[] = { DEVRANDOM_EGD, NULL };
+        size_t bytes_needed;
+        unsigned char *buffer;
         int i;
 
-        for (i = 0; paths[i] != NULL; i++) {
+        bytes_needed = rand_pool_bytes_needed(pool, 1 /*entropy_factor*/);
+        for (i = 0; bytes_needed > 0 && paths[i] != NULL; i++) {
+            size_t bytes = 0;
+            int num;
+
             buffer = rand_pool_add_begin(pool, bytes_needed);
-            if (buffer != NULL) {
-                size_t bytes = 0;
-                int num = RAND_query_egd_bytes(paths[i],
-                                               buffer, (int)bytes_needed);
-                if (num == (int)bytes_needed)
-                    bytes = bytes_needed;
+            num = RAND_query_egd_bytes(paths[i],
+                                       buffer, (int)bytes_needed);
+            if (num == (int)bytes_needed)
+                bytes = bytes_needed;
 
-                rand_pool_add_end(pool, bytes, 8 * bytes);
-                entropy_available = rand_pool_entropy_available(pool);
-            }
-            if (entropy_available > 0)
-                return entropy_available;
+            rand_pool_add_end(pool, bytes, 8 * bytes);
+            bytes_needed = rand_pool_bytes_needed(pool, 1);
         }
+        entropy_available = rand_pool_entropy_available(pool);
+        if (entropy_available > 0)
+            return entropy_available;
     }
 #   endif
 


More information about the openssl-commits mailing list