[Pkg-openssl-devel] Bug#363516: Bug#363516: The actual change

[Tim Hudson]

[
resent not as tjh at cryptsoft.com - too many bounces from overly aggressive relays 
which trust black lists from which you seem never to be able to get removed from.

sorry to those who get two copies of this --tjh
]


Richard Kettlewell wrote:
> A couple of people have suggested I mention the change that was actually 
> made.  These are the relevant URLs:
> 
> http://svn.debian.org/viewsvn/pkg-openssl?rev=141&view=rev
> http://svn.debian.org/viewsvn/pkg-openssl/openssl/trunk/rand/md_rand.c?rev=141&r1=140&r2=141

What is missing is the context of the change. There seems to be some confusion
around this.

Although it is the same line of code that is being changed the context is
entirely different (and it is easy to not be aware of that context when seeing a
diff and that certainly contributed to this change not being noticed when
discussed on the openssl lists before it was made in the debian repository).

Basically there are two identical lines of code in completely different
contexts. One was safe to remove, the other most certainly was not.

You can follow the context at:

http://svn.debian.org/viewsvn/pkg-openssl/openssl/trunk/rand/md_rand.c?rev=141&view=markup

The context of the first change:

Inside ssleay_rand_add (which is called by RAND_add_bytes when using this PRNG).
This is used when application code is adding entropy to the PRNG and it is
entirely up to the application where that entropy is coming from - it may be
from any source including uninitialised buffers if that is what gets passed in.

The code change commented out the *input* bytes coming from the application to
mix into the PRNG which reduces the seeding of the PRNG to code paths which
don't go via RAND_add_bytes of which there are few - which is reflected in the
tool for checking for a weak key with its 250k entries in its internal table.

The context of the second change:

Inside ssleay_rand_bytes (which is called by RAND_bytes when using this PRNG).
This is used when the application code (or any logic in the rest of OpenSSL)
needs some random data - typically in the context of key generation or random
padding. What the original code in OpenSSL does is actually mix in the callers
buffer into which the output is to be copied into the PRNG pool. OpenSSL made a
change to make this code conditional on PURIFY not being defined as it is of
course a source of a large number of reports of 'errors' in OpenSSL when using
purify and when using valgrind where the call chain varies substantially back
into the application code and so isn't obvious to the casual developer when
looking at it as to what is going on.

The value of adding that output buffer into the entropy pool can be debated but
every bit helps and a conservative approach of mixing in whatever is available
is prudent. The annoyance of a pile of purify and valgrind errors being reported
against OpenSSL from other packages and applications which use it without a
clean way of disabling the tools by noting that the usage is safe is
unfortunate. There are ways to reorganise the code to make it straight forward
to include the known safe call chain so the other 'real' errors are not hidden
in the stream of output from purify and valgrind about this issue.

If anyone wants some assistance at writing FAQ entries or responses for this
then drop me a line - I used to handle vulnerability responses for all the RSA
security related SDKs so I'm well aware of the process and the importance of
clear notices to impacted users.

Tim Hudson
tjh at cryptsoft.com / tim.hudson at attglobal.net

---8<---

Attributing blame for this issue is a pretty pointless exercise IMHO. The code
has been in existance for two years. It is installed on systems I myself use and
I didn't see the context of the diff in my first reading of the patch when this
issue was announced which makes me think it was easy to miss. It required closer
looking at the code (and finding a URL to the actual patch and the whole file in
context). It is an extremely serious security issue and systems should be
patched as quickly as possible and keys regenerated.