Cryptographic weakness on Debian systems

And the rationale for the change can be found in the thread at
<http://marc.info/?t=114651088900003&r=1&w=2>.

It's interesting, looks like there's plenty of embarassment to go round.

This function does not just take arbitrary "uninitialized memory" and feed it into the entropy
pool. That would be stupid. It takes a parameter, which is supposed to be an array of entropy
bytes to put into the pool.

I haven't analysed the code enough to see whether the Valgrind reports are triggered by this
function going off the end of the provided buffer, or by the buffer providers (elsewhere in
OpenSSL code) not fully initialising their buffers, or even by a subtle false alarm (Valgrind
is good but it's not perfect, not yet).

In any case, with the patch removing this vital line of code, more or less none of the
intended entropy ends up being used. Entropy from /dev/random, from hardware RNG sources, and
so on, is discarded silently by Debian's patch AFAICT. Hence the list of millions of possible
"bogus" keys which can be generated from the little remaining entropy.

What's fascinating is that when Debian reported this terrible goof today some OpenSSL people
said if they'd come forward with this patch two years ago the OpenSSL developers would have
laughed at the obviously wrong fix... But it seems that Debian people did go to the OpenSSL
developers two years ago, and they got told more or less that if it shuts up Valgrind then
it's fine.

Now in parallel to proposing this goofy patch, someone proposed to Debian a fix which simply
writes zeroes to an uninitialised buffer elsewhere in OpenSSL. This patch seems to have been
rejected by Debian for unclear reasons, but an equivalent change has been included in newer
versions of OpenSSL and from there integrated back into Debian. It is probably one of several
such fixes which should have taken place instead of this silly one line patch that disables
the RNG more or less altogether.

The level of entropy would be essentially zero. Memory starts out as zeroed at process
creation time.  It may end up being recycled internally to the same process, but that would be
very predictable.

I suppose you could have some situation like:

 x = malloc(10);
 memcpy(x, randsrc, 10);
 free(x);
 y = malloc(10); /* we just happen to get the same memory */

could cause the situation as described, but that would be a horrible bug -- there is no
guarantee that y wouldn't be all zeros anyway.

I don't think that's what happened here though.

From other comments I think it may be the case that the zeroed buffer contained real entropy,
but possibly not filling the whole thing, thus reading it causes reads of uninitialized bytes.
It could also be that the entropy in the buffer was generated using an uninitialized variable
somewhere (any result of a calculation with uninitialized values must be treated as if it
results in uninitialized values).

I look forward to someone doing a more in-depth inspection of this code -- it should be made
clear exactly how the entropy is gathered and passed around, and there should be no
uninitialized values stored or read from the buffer.  Adding initializations at random points
in the code is not the way to write secure software.

I don't think it was completely uninitialized.
If it is, then it was a really bad idea, as there should be no expectation that it have any
useful entropy any more than you should expect it to be filled with zeros.

Plus, a C program which does that is buggy -- you can't read memory you haven't initialized.
Sure it mostly worked, but it is still a bug.

(Not that any of that excuses the "fix" applied.)