Vitavonni

Wed, 14 May 2008

The Debian OpenSSL disaster

I've read some more about this and especially had a look at some of the source code, so I've completely revised this blog post.

There is no doubt that the Debian Maintainer who added this patch screwed up, seriously. But he's not the only one to blame:

  • OpenSSL isn't valgrind-clean. It should be made, since valgrind is a very important debugging tool; the bad patch was introduced to make OpenSSL better in this respect due to the request by the users.
  • The OpenSSL source code could be better documented in these places.
  • There are two instances of this line in OpenSSL which can both generate the 'using uninitialized data' warning. One is safe to remove (it's supposed to fill the buffer with random data, and just uses the existing contents as additional source of randomness), the other is not (it's used to feed randomness coming from e.g. /dev/random into the pool).
  • The Debian maintainer received the reply "if it helps with debugging, I'm in favour of removing them" by one of the current OpenSSL devs on the openssl-dev mailing list. Probably just referring to the 'safe one' of the two locations where this occurs, though?
  • Nobody noticed the severity of this change for more than 2 years. We're all to blame.

I'm really sick of hearing comments like "Still, whoever took out the entire initialization should not be trusted with security intensive code." (guest comment on LWN). Yes, he screwed up there. But you bet he's going to be a lot more careful with any change in the future: he has learned his lesson. Better than having someone else screw up in a similar way again. And actually he didn't do this change half as easy-hearted as many people suggest, if you look at the discussions on the bug report and mailing lists. He was trying to fix the valgrind bug, and he talked to several people on how to do it properly.

The best way the bug could have been avoided was if the OpenSSL upstream developers had cared more about the valgrind issue themselves. (E.g. by teaching valgrind to ignore the issue, documenting in the source code why this is intentional and not an issue, ...)

P.S. in the previous version of this blog post I had asked why OpenSSL apparently relies on uninitialized data for security. It doesn't; the same lines exist in two places, and it's the other change that caused the problems. One place will 'usually' generate the warning, and it's not important (that's the place where you could just remove the line). The other place will generate the same warning when someone passes uninitialized data into the RNG. As long as the RNG is sufficiently seeded with other random data that isn't much of a problem. So if you take a buffer of 1024 bytes and fill it with 1000 bytes of good random data (e.g. from a hardware randomness source) and feed the whole buffer to the RNG, it will be seeded quite well, but the warning will be generated. The 24 uninitizialized bytes won't take the entropy away.

(This blog does intentionally not have a comment function. Sorry.)

[category: /en/linux | Permalink]
Menu
[planet.debian]
[planet.xmlhack]
[planet SELinux]
[munichblogs]
[email]
[RSS 2 feed]
[English RSS 2]
Categories
< May 2008 >
SuMoTuWeThFrSa
     1 2 3
4 5 6 7 8 910
11121314151617
18192021222324
25262728293031
Archives
2010-Mar
2010-Feb
2010-Jan
2009-Dec
2009-Nov
2009-Oct
2009-Sep
2009-Aug
2009-Jul
2009-Jun
2009-May
2009-Apr
2009-Mar
2009-Feb
2009-Jan
2008-Dec
2008-Nov
2008-Oct
2008-Sep
2008-Aug
2008-Jul
2008-May
2008-Apr
2008-Mar
2008-Feb
2008-Jan
2007-Dec
2007-Nov
2007-Oct
2007-Sep
2007-Aug
2007-Jul
2007-Jun
2007-May
2007-Apr
2007-Mar
2007-Feb
2007-Jan
2006-Dec
2006-Nov
2006-Oct
2006-Sep
2006-Aug
2006-Jul
2006-Jun
2006-May
2006-Apr
2006-Mar
2006-Feb
2006-Jan
2005-Dec
2005-Nov
2005-Oct
2005-Sep
2005-Aug
2005-Jul
2005-Jun
2005-May
2005-Apr
2005-Mar
2005-Feb
2005-Jan
2004-Dec
2004-Nov
2004-Oct
2004-Sep
2004-Aug
2004-Jul
Other links:
Swing and the City - Lindy Hop in Munich