Commit 1b0fe00e authored by Dr. Matthias St. Pierre's avatar Dr. Matthias St. Pierre Committed by Matt Caswell
Browse files

drbg: ensure fork-safety without using a pthread_atfork handler



When the new OpenSSL CSPRNG was introduced in version 1.1.1,
it was announced in the release notes that it would be fork-safe,
which the old CSPRNG hadn't been.

The fork-safety was implemented using a fork count, which was
incremented by a pthread_atfork handler. Initially, this handler
was enabled by default. Unfortunately, the default behaviour
had to be changed for other reasons in commit b5319bdb, so
the new OpenSSL CSPRNG failed to keep its promise.

This commit restores the fork-safety using a different approach.
It replaces the fork count by a fork id, which coincides with
the process id on UNIX-like operating systems and is zero on other
operating systems. It is used to detect when an automatic reseed
after a fork is necessary.

To prevent a future regression, it also adds a test to verify that
the child reseeds after fork.

CVE-2019-1549

Reviewed-by: default avatarPaul Dale <paul.dale@oracle.com>
Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9802)
parent 73a683b7
Loading
Loading
Loading
Loading
+0 −1
Original line number Original line Diff line number Diff line
@@ -26,7 +26,6 @@ typedef struct rand_pool_st RAND_POOL;
void rand_cleanup_int(void);
void rand_cleanup_int(void);
void rand_drbg_cleanup_int(void);
void rand_drbg_cleanup_int(void);
void drbg_delete_thread_state(void);
void drbg_delete_thread_state(void);
void rand_fork(void);


/* Hardware-based seeding functions. */
/* Hardware-based seeding functions. */
size_t rand_acquire_entropy_from_tsc(RAND_POOL *pool);
size_t rand_acquire_entropy_from_tsc(RAND_POOL *pool);
+0 −1
Original line number Original line Diff line number Diff line
@@ -847,6 +847,5 @@ void OPENSSL_fork_parent(void)


void OPENSSL_fork_child(void)
void OPENSSL_fork_child(void)
{
{
    rand_fork();
}
}
#endif
#endif
+6 −3
Original line number Original line Diff line number Diff line
@@ -197,7 +197,7 @@ static RAND_DRBG *rand_drbg_new(int secure,
    }
    }


    drbg->secure = secure && CRYPTO_secure_allocated(drbg);
    drbg->secure = secure && CRYPTO_secure_allocated(drbg);
    drbg->fork_count = rand_fork_count;
    drbg->fork_id = openssl_get_fork_id();
    drbg->parent = parent;
    drbg->parent = parent;


    if (parent == NULL) {
    if (parent == NULL) {
@@ -578,6 +578,7 @@ int RAND_DRBG_generate(RAND_DRBG *drbg, unsigned char *out, size_t outlen,
                       int prediction_resistance,
                       int prediction_resistance,
                       const unsigned char *adin, size_t adinlen)
                       const unsigned char *adin, size_t adinlen)
{
{
    int fork_id;
    int reseed_required = 0;
    int reseed_required = 0;


    if (drbg->state != DRBG_READY) {
    if (drbg->state != DRBG_READY) {
@@ -603,8 +604,10 @@ int RAND_DRBG_generate(RAND_DRBG *drbg, unsigned char *out, size_t outlen,
        return 0;
        return 0;
    }
    }


    if (drbg->fork_count != rand_fork_count) {
    fork_id = openssl_get_fork_id();
        drbg->fork_count = rand_fork_count;

    if (drbg->fork_id != fork_id) {
        drbg->fork_id = fork_id;
        reseed_required = 1;
        reseed_required = 1;
    }
    }


+5 −18
Original line number Original line Diff line number Diff line
@@ -186,12 +186,12 @@ struct rand_drbg_st {
    int secure; /* 1: allocated on the secure heap, 0: otherwise */
    int secure; /* 1: allocated on the secure heap, 0: otherwise */
    int type; /* the nid of the underlying algorithm */
    int type; /* the nid of the underlying algorithm */
    /*
    /*
     * Stores the value of the rand_fork_count global as of when we last
     * Stores the return value of openssl_get_fork_id() as of when we last
     * reseeded.  The DRBG reseeds automatically whenever drbg->fork_count !=
     * reseeded.  The DRBG reseeds automatically whenever drbg->fork_id !=
     * rand_fork_count.  Used to provide fork-safety and reseed this DRBG in
     * openssl_get_fork_id().  Used to provide fork-safety and reseed this
     * the child process.
     * DRBG in the child process.
     */
     */
    int fork_count;
    int fork_id;
    unsigned short flags; /* various external flags */
    unsigned short flags; /* various external flags */


    /*
    /*
@@ -283,19 +283,6 @@ struct rand_drbg_st {
/* The global RAND method, and the global buffer and DRBG instance. */
/* The global RAND method, and the global buffer and DRBG instance. */
extern RAND_METHOD rand_meth;
extern RAND_METHOD rand_meth;


/*
 * A "generation count" of forks.  Incremented in the child process after a
 * fork.  Since rand_fork_count is increment-only, and only ever written to in
 * the child process of the fork, which is guaranteed to be single-threaded, no
 * locking is needed for normal (read) accesses; the rest of pthread fork
 * processing is assumed to introduce the necessary memory barriers.  Sibling
 * children of a given parent will produce duplicate values, but this is not
 * problematic because the reseeding process pulls input from the system CSPRNG
 * and/or other global sources, so the siblings will end up generating
 * different output streams.
 */
extern int rand_fork_count;

/* DRBG helpers */
/* DRBG helpers */
int rand_drbg_restart(RAND_DRBG *drbg,
int rand_drbg_restart(RAND_DRBG *drbg,
                      const unsigned char *buffer, size_t len, size_t entropy);
                      const unsigned char *buffer, size_t len, size_t entropy);
+0 −7
Original line number Original line Diff line number Diff line
@@ -26,8 +26,6 @@ static CRYPTO_RWLOCK *rand_meth_lock;
static const RAND_METHOD *default_RAND_meth;
static const RAND_METHOD *default_RAND_meth;
static CRYPTO_ONCE rand_init = CRYPTO_ONCE_STATIC_INIT;
static CRYPTO_ONCE rand_init = CRYPTO_ONCE_STATIC_INIT;


int rand_fork_count;

static CRYPTO_RWLOCK *rand_nonce_lock;
static CRYPTO_RWLOCK *rand_nonce_lock;
static int rand_nonce_count;
static int rand_nonce_count;


@@ -303,11 +301,6 @@ void rand_drbg_cleanup_additional_data(RAND_POOL *pool, unsigned char *out)
    rand_pool_reattach(pool, out);
    rand_pool_reattach(pool, out);
}
}


void rand_fork(void)
{
    rand_fork_count++;
}

DEFINE_RUN_ONCE_STATIC(do_rand_init)
DEFINE_RUN_ONCE_STATIC(do_rand_init)
{
{
#ifndef OPENSSL_NO_ENGINE
#ifndef OPENSSL_NO_ENGINE
Loading