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
[EMAIL PROTECTED] / [EMAIL PROTECTED]

---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.




--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]

Reply via email to