Add missing RAND_DRBG locking
authorBenjamin Kaduk <bkaduk@akamai.com>
Wed, 11 Oct 2017 17:25:26 +0000 (19:25 +0200)
committerBen Kaduk <kaduk@mit.edu>
Wed, 18 Oct 2017 13:39:20 +0000 (08:39 -0500)
commit2139145b72d084a3f974a94accd7d9812de286e4
tree4c81571b7f35acd01c44e159fb75b756d856b818
parente0b625f9db00509af9004b7907d44b78f332754a
Add missing RAND_DRBG locking

The drbg's lock must be held across calls to RAND_DRBG_generate()
to prevent simultaneous modification of internal state.

This was observed in practice with simultaneous SSL_new() calls attempting
to seed the (separate) per-SSL RAND_DRBG instances from the global
rand_drbg instance; this eventually led to simultaneous calls to
ctr_BCC_update() attempting to increment drbg->bltmp_pos for their
respective partial final block, violating the invariant that bltmp_pos < 16.
The AES operations performed in ctr_BCC_blocks() makes the race window
quite easy to trigger.  A value of bltmp_pos greater than 16 induces
catastrophic failure in ctr_BCC_final(), with subtraction overflowing
and leading to an attempt to memset() to zero a very large range,
which eventually reaches an unmapped page and segfaults.

Provide the needed locking in get_entropy_from_parent(), as well as
fixing a similar issue in RAND_priv_bytes().  There is also an
unlocked call to RAND_DRBG_generate() in ssl_randbytes(), but the
requisite serialization is already guaranteed by the requirements on
the application's usage of SSL objects, and no further locking is
needed for correct behavior.  In that case, leave a comment noting
the apparent discrepancy and the reason for its safety (at present).

Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4328)
crypto/rand/drbg_lib.c
crypto/rand/rand_lib.c
ssl/ssl_lib.c