Comments: Case study in risk management: Debian's patch to OpenSSL

You're missing an important part of the tech issue.

OpenSSL does this, which is good, but may be doing it poorly. What it (apparently) does is to mix in uninitialised buffers with the OS-supplied randoms, and little else (those who can read the code might confirm this).

The Debian patch in question (http://svn.debian.org/viewsvn/pkg-openssl/openssl/trunk/rand/md_rand.c?rev=141&view=diff&r1=141&r2=140&p1=openssl/trunk/rand/md_rand.c&p2=/openssl/trunk/rand/md_rand.c) modifies two places in the code. One of them does as you suggest (mixing in uninitialized stack data); it was already annotated with #ifndef PURIFY, and is not a correctness issue. The bug report (http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=363516) from rjk@ only talks about fixing that one, but kurt@ generalized to another place (line 247 in ssleay_rand_add) which is the same line of code, but in a completely different context and extremely critical -- commenting it out turned ssleay_rand_add() into a noop!

As far as what needs to be done, or what lessons can be learned -- obviously this should be a wakeup call to distributors that they need to push patches upstream. Hopefully upstream authors will take this opportunity to work more closely with downstream as well. And it's important that people testing security (for example, testing RNGs as used in the field) test the software that people actually use rather than the ivory-tower upstream code.

Posted by Andy at May 14, 2008 04:54 PM

I presume you meant to write "What is good practice for the OS is _not_ good practice for the application"?

Posted by Frank Hecker at May 14, 2008 04:54 PM

Sorry, ignore my previous comment. I misread your meaning; you simply meant that it's good practice for applications to use the same techniques as are used in the OS.

Posted by Frank Hecker at May 14, 2008 09:51 PM

Now that I had to change a bunch of certificates, I used openssl's feature of taking randomness from external files, which I filled with random data from different sources (OS, gpg, microphone, etc.). Each time I needed to generate a new key, I collected randomness first like this.

What is good practice for the OS is good practice for the application is good practice for the user.

Posted by Daniel Nagy at May 15, 2008 05:30 AM

Hi Ian,

Your FC diary does not appear to be accepting comments at the moment. Other interesting aspects of this affair: You hinted, in passing, that the OpenSSL code is unreadable. Shouldn't that assertion be at the top of your list?

To "those who can read the code," one might add "those who can compile the code." The problematic patch on May 2, 2006 also added a nested comment, which wasn't fixed until September 17, 2006. In a way, this may be a good thing; nobody can have been using this revision for the duration.

Finally, in the advisory it is mentioned that all DSA keys ought to be replaced, since the DSA handshake uses "random" data generated by the offending code, and the result would be susceptible to a known plaintext attack. This is true even if the DSA key predates May 2006. But Debian's update will only replace a DSA key that itself is bad.

Posted by Felix at May 15, 2008 07:05 PM

> OpenSSL does this, which is good, but may be doing it poorly. What it
> (apparently) does is to mix in *uninitialised buffers* with the
> OS-supplied randoms, and little else (those who can read the code might
> confirm this).

This is a misunderstanding. OpenSSL was simply not zeroing out buffers before using them to carry random data from the caller and stuff it into the entropy pool. Make sense? It has a temp buffer to hold data which is on its way to the entropy pool, and it doesn't bother to zero out that buffer (or the unused end of it).

A Debian developer falsely thought that the entire buffer held nothing but uninitialized RAM in all cases.

Here is a nice summary of the code failure:

http://lwn.net/SubscriberLink/282230/8c93c55edd44ccdc/

And here is a nice summary of the communication failure:

http://lwn.net/SubscriberLink/282038/ed89274bd6da5d90/

Posted by Regards, Zooko at May 15, 2008 07:09 PM

A deliberately fingerpointing article about the issue for the dull rainy hours:
http://www.gergely.risko.hu/debian-dsa1571.en.html

Posted by Gergely Riskó at May 19, 2008 06:02 PM

After looking at the actual original code in http://svn.debian.org/viewsvn/pkg-openssl/openssl/trunk/rand/md_rand.c?rev=141&view=diff&r1=141&r2=140&p1=openssl/trunk/rand/md_rand.c&p2=/openssl/trunk/rand/md_rand.c it became clear why developers can be misled. There is no comment in the code documenting the use of uninitialized memory to increase randomness! In that respect, I agree with comment 10 in http://www.links.org/?p=327 that openssl developers did not do their part to prevent this whole drama from happenning.

Posted by anon_concern at June 1, 2008 01:38 PM
Post a comment









Remember personal info?






Hit Preview to see your comment.
MT::App::Comments=HASH(0x564087d07918) Subroutine MT::Blog::SUPER::site_url redefined at /home/iang/www/fc/cgi-bin/mt/lib/MT/Object.pm line 125.