*Version:
*This bug was found in openssl-fips 2.0.2; I looked in 2.0.5, and the
problem appears to be present there still.
*
Issue:*
The fips module has a bug that can result in segfaults when
fips_get_entropy() fails during initialization of openssl-linked-with-fips.
*Fix:
*Because the fix is extremely trivial, I'll identify it first, and give
the rationale afterwards (which may in fact be better explained by a
patch than by my detailed analysis that follows). The patch to cure this
problem is simply:
--- fips/rand/fips_drbg_lib.c.orig 2013-10-22 11:11:28.000000000 -0700
+++ fips/rand/fips_drbg_lib.c 2013-10-22 11:20:29.000000000 -0700
@@ -160,7 +160,8 @@ static size_t fips_get_entropy(DRBG_CTX
return dctx->get_entropy(dctx, pout, entropy, min_len,
max_len);
rv = dctx->get_entropy(dctx, &tout, entropy + bl,
min_len + bl, max_len + bl);
- *pout = tout + bl;
+ if (tout != NULL)
+ *pout = tout + bl;
if (rv < (min_len + bl) || (rv % bl))
return 0;
/* Compare consecutive blocks for continuous PRNG test */
/White space is NOT correct in the above (I'm afraid tabs have been
replaced with spaces); making the described change manually will almost
certainly work better than copy/pasting the above snip into patch's stdin./
*Reproducibility:*
I was not able to find a convenient way to reproduce; HOWEVER, it is
easy to see that there is a problem in the code that I will point out to
you. In our case, we could only get the bug to exercise on one machine
(not another), and it would disappear when we ran it through a tool like
valgrind. Morover, it seemed to appear during initialization in one
program (curl), but didn't appear when an attempt was made to exactly
duplicate the openssl initialization in a small test program (that did
nothing more than initialize openssl. In other words, reproduction was
difficult.
The bug was later determined to only occur when the call to
dctx->get_entropy failed, called within fips_get_entropy, so if you can
reliably cause that to fail, you should be able to reliably reproduce
the issue. Perhaps by reading from /dev/random until it blocks, just
before attempting openssl (with fips) initialization?
*Bug Profile:*
All of this takes place within FIPS_drbg_instantiate, and the functions
called there, all located in the source file fips/rand/fips_drbg_lib.c.
All line numbers are as of openssl-fips 2.0.5, and refer to lines within
fips_drbg_lib.c.
FIPS_drbg_instantiate (beginning at 194) calls fips_get_entropy (at line
237):
entlen = fips_get_entropy(dctx, &entropy, dctx->strength,
dctx->min_entropy, dctx->max_entropy);
fips_get_entropy has this signature (line 152):
static size_t fips_get_entropy(DRBG_CTX *dctx, unsigned char **pout,
int entropy, size_t min_len, size_t max_len)
(Notice that the second argument in the call is &entropy, whereas the
second parameter in the function prototype is pout.)
and contains the following lines (beginning at line 161):
...
rv = dctx->get_entropy(dctx, &tout, entropy + bl,
min_len + bl, max_len + bl);
*pout = tout + bl;
if (rv < (min_len + bl) || (rv % bl))
return 0;
....
Note that the final if-statement is a "return early if we didn't get
enough entropy" check, but even before we check such a thing, or whether
tout == NULL, we add the value of bl to it. So, if bl == 20 and
dctx->get_entropy returned 0 and set tout to NULL, the variable entropy
in FIPS_drbg_instantiate() will now have the pointer value 20.
In such a case (as seen in the if-statement above), fips_get_entropy()
will have returned 0. FIPS_drbg_instantiate() detects this situation and
jumps to some cleanup code, which includes the following (line 277):
if (entropy && dctx->cleanup_entropy)
fips_cleanup_entropy(dctx, entropy, entlen);
Now, there's a good check for entropy == NULL before attempting to clean
it up; the trouble is, it is IMPOSSIBLE for entropy to ever be NULL at
this stage. In our failure case, it has the pointer value 20 (because bl
was 20). What happens in fips_cleanup_entropy() is that it subtracts the
value of bl (resulting in the NULL pointer value once again, but too
late to fail the check), and then passes that to dctx->cleanup_entropy,
which, upon dereferencing the NULL pointer, pukes.
Note that the function drbg_reseed() in the same module has the same
logic as was just described for FIPS_drbg_instantiate, probably has the
same bug, and the patch given should fix it as well. Only those two
functions appear to call fips_get_entropy() or fips_cleanup_entropy().
-mjc
______________________________________________________________________
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager majord...@openssl.org
