Links

Ben Laurie blathering


Vendors Are Bad For Security

I’ve ranted about this at length before, I’m sure – even in print, in O’Reily’s Open Sources 2. But now Debian have proved me right (again) beyond my wildest expectations. Two years ago, they “fixed” a “problem” in OpenSSL reported by valgrind[1] by removing any possibility of adding any entropy to OpenSSL’s pool of randomness[2].

The result of this is that for the last two years (from Debian’s “Etch” release until now), anyone doing pretty much any crypto on Debian (and hence Ubuntu) has been using easily guessable keys. This includes SSH keys, SSL keys and OpenVPN keys.

What can we learn from this? Firstly, vendors should not be fixing problems (or, really, anything) in open source packages by patching them locally – they should contribute their patches upstream to the package maintainers. Had Debian done this in this case, we (the OpenSSL Team) would have fallen about laughing, and once we had got our breath back, told them what a terrible idea this was. But no, it seems that every vendor wants to “add value” by getting in between the user of the software and its author.

Secondly, if you are going to fix bugs, then you should install this maxim of mine firmly in your head: never fix a bug you don’t understand. I’m not sure I’ve ever put that in writing before, but anyone who’s worked with me will have heard me say it multiple times.

Incidentally, while I am talking about vendors who are bad for security, it saddens me to have to report that FreeBSD, my favourite open source operating system, are also guilty. Not only do they have local patches in their ports system that should clearly be sent upstream, but they also install packages without running the self-tests. This has bitten me twice by installing broken crypto, most recently in the py-openssl package.

[1] Valgrind is a wonderful tool, I recommend it highly.

[2] Valgrind tracks the use of uninitialised memory. Usually it is bad to have any kind of dependency on uninitialised memory, but OpenSSL happens to include a rare case when its OK, or even a good idea: its randomness pool. Adding uninitialised memory to it can do no harm and might do some good, which is why we do it. It does cause irritating errors from some kinds of debugging tools, though, including valgrind and Purify. For that reason, we do have a flag (PURIFY) that removes the offending code. However, the Debian maintainers, instead of tracking down the source of the uninitialised memory instead chose to remove any possibility of adding memory to the pool at all. Clearly they had not understood the bug before fixing it.

P.S. I’d link to the offending patch in Debian’s source repository. If I could find a source repository. But I can’t.

(Update)

Thanks to Cat Okita, I have now found the repo. Here’s the offending patch. But I have to admit to being astonished again by the fix, which was committed five days before the advisory! Do these guys have no clue whatsoever?

178 Comments

  1. The discussion about this “problem” can be found here: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=363516

    Comment by Adrian-Ken Rüegsegger — 13 May 2008 @ 14:47

  2. http://svn.debian.org/wsvn/pkg-openssl / svn://svn.debian.org/pkg-openssl/ are the repositories you’re after.

    Comment by Dominic Hargreaves — 13 May 2008 @ 14:49

  3. This is a lame rant, but your own opinion nonetheless. I am used to submitting patches upstream and not getting them to approve the patch in a timely fashion. Sometimes we need these patches applied to stuff that’s broken and need fixing on a set schedule, so, we patch the broken upstream sources until we get a response from upstream…

    The bad thing about those workarounds is that usually these kinds of fixes become permanent, and at times, as shown in this case, they actually do more harm than good… But, as humans, we learn from our mistakes; and I’m sure the Debian team learned from this one.

    Case close. Now let’s move on to the next problem…

    Comment by Luis Mondesi — 13 May 2008 @ 15:18

  4. Ben, would you be able to comment on whether you think the recently announced oCERT could help here. In particular, is there a role for central security-aware bodies such as these to monitor vendor patches to security-critical software?

    While the vendor is primarily responsible here, is there a role for authors of security-critical software to monitor vendor patches (particularly for large distributions on which many other distributions depend) to ensure they don’t break the very security the software is supposed to be providing?

    Comment by Toby — 13 May 2008 @ 15:41

  5. Wow. This is a real eye-opener.

    I’m still struggling to understand how somebody with privileges to change code in a security package went and removed parts of the code without first understanding what it does or checking with somebody that it would not compromise security.

    Comment by Charles Darke — 13 May 2008 @ 15:53

  6. Is removing the other MD_Update OK? That’s still in the security update as far as I can see.

    Comment by Brian — 13 May 2008 @ 15:55

  7. Why don’t you like the fix? Debian (Florian Weimar) says: “[...] the other hunk is benign. It mixes data from the target buffer of
    RAND_bytes into the pool, and this is completely optional (because it’s
    not guaranteed that this data is random anyway).”

    Is this right? Or is it really a security problem to leave out the second MD_Update()?

    Thanks,
    Nicolas

    Comment by Nicolas — 13 May 2008 @ 15:59

  8. I agree with Ben and Luis, which is a shame considering their opinions conflict slightly.
    Where do the openssl developers get their copies from? Are none of them using Debian/Ubuntu?

    To my mind, a reality of being an upstream developer is that you have downstream customers and it is in your best interests to engage with them, just as much as it is in their best interests to engage with you. Patches in .deb packages are easily extracted and checked, but obviously nobody with the appropriate knowledge (either from openssl or debian) did this for 2 years.

    Comment by Chris jones — 13 May 2008 @ 16:05

  9. [...] Ben Laurie of OpenSSL is making a point that Vendors Are Bad For Security, which I would not follow in that general form. What I have to grant him: Mechanisms of peer review [...]

    Pingback by robert’s rant room » Blog Archive » Use(d) Debian? Check your keys! — 13 May 2008 @ 16:21

  10. It may well be shame-on-Debian for not sending patches upstream, but it’s NOT shame-on-Debian for patching something they didn’t understand. Instead, it’s shame-on-OpenSSL for leaving an undocumented landmine in security-critical code.

    Everybody knows that crypto is full of subtlety, and it’s *really* easy for one not immersed in it to not fully understand the intricacies. But crypto is still written by humans, and crypto can still have run-of-the-mill bugs too.

    In more than 25 years of C development experience, I’ve never run across code whose proper operation depended on a variable being uninitialized, and so it’s not ridiculous that even a skilled, thoughtful developer would have assumed this was one of those run-of-the-mill bugs. “Who’d ever want to do *that*?”

    Well in this case the OpenSSL team did want to do that, and it’s perfectly reasonable (and a kind of clever idea), but when one uses a construct that’s vanishingly rare, one really ought to document it:

    unsigned char tmpbuf[ENTROPY_NEEDED]; /* do not initialize – this is starting entropy! */

    Adding that single comment to rand_unix.c would have likely avoided this whole issue by leaving a big sign: “Don’t step on the landmine”.

    Comment by Steve Friedl — 13 May 2008 @ 16:21

  11. [...] Ben Laurie tears into Debian for [...]

    Pingback by taint.org: Justin Mason’s Weblog » Serious Debian/Ubuntu openssl/openssh bug found — 13 May 2008 @ 16:25

  12. … I see Ben has posted an update that includes the actual patch (which appears to be different from the tmpbuf patch discussed in related links), and this does indeed look a bit more boneheaded on the part of the Debian fixers, but the general notion of documenting strange stuff that others might be confused about (use of uninitialized data) seems to remain unchanged.

    Comment by Steve Friedl — 13 May 2008 @ 16:31

  13. Just a point of clarification.

    According to the advisory, the bug was introduced into testing in April of 2006 and Sarge was not affected. Debian Etch was released on April 8, 2007 according to Wikipedia. Until that release, a little over a year ago, this would have only impacted users of testing and unstable. Since then, it would have been picked up as a base for other distributions such as Ubuntu, Knoppix, and other run-live CD’s and BBC’s (bootable business cards).

    I have to agree with Ben on this one very important point, that they did not submit their local patch to the upstream for evaluation and incorporation is unconscionable. Even if they felt they need to do a local patch, to not forward it to the upsteam team is just not right. This would have been caught much sooner if they had taken that simple step. I’ve authored and hosted patches for a variety of packages over the years and always notified the upstream (and they don’t always take my patches, but that’s no skin off my nose).

    Comment by Michael H. Warfield — 13 May 2008 @ 16:38

  14. > never fix a bug you don’t understand

    This is a good maxim, but it has a problem: people often don’t understand when they don’t understand. The folks who applied the original patch *thought* that they understood what they were doing: They thought they were removing the seed-from-uninitialized-ram feature, and didn’t realize that they were neutering seeding entirely.

    Maybe a better maxim might be “never patch code you don’t understand”? A closer read-through of the function in question would have shown its purpose (“Add entropy from a seed to the entropy pool”) that the line they removed was the only line in ssleay_rand_seed() that actually did anything _with_ the seed.

    Other lessons (mostly old) I’m taking away are:
    – Never take anybody’s word about what a patch does.
    – Reading patches out-of-context is no way to review patches. Looking only at the original diff, I personally wouldn’t have realized that it was the crucial MD_update in ssleay_rand_seed() that was being removed without looking at the surrounding code.
    – Pay extra to classes of potential bugs (like RNG bugs) that aren’t likely to be noticed in practice.
    – Work out how to re-key all your systems well in advance.
    – In audits, pay extra attention to distribution patches. They may not have gotten quite so many eyeballs (or quite so many of the *right* eyeballs) as the upstream code.
    – If tools like valgrind are known to give warnings when checking certain parts of your code, it’s not a bad idea to add inline comments to explain what the code is doing. [Not that a lack of such comments is any excuse.]

    I’ll be interested to see what kind of process changes Debian adopts in response to this. I’d be surprised if the extent of their long-term response were limited to “Bugs happen; let’s fix the bug, move on, and be more careful in the future.”

    Comment by Nick Mathewson — 13 May 2008 @ 17:07

  15. Just speaking for myself, for the few FreeBSD ports that I maintained, I generally had a very hard time getting patches accepted upstream. My feeling was that it was better to at least keep the patch in the ports system than to not have it at all.

    Comment by Joseph Scott — 13 May 2008 @ 17:11

  16. As Luis pointed out, vendors may be forced to include their own patches precisely because upstream won’t honor them (or in the case of FreeBSD, I have had upstream vendors claim that a particular port version is no longer under development and should be deprecated all the way to “we won’t fix this because it’s a problem with the way FreeBSD implements foo syscall differently from Linux”).

    Comment by cowbert — 13 May 2008 @ 17:37

  17. [...] the solution they chose to implement apparently removed all entropy from the OpenSSL random number generator.  As the OpenSSL team comments “Had Debian [contributed the patches to the package [...]

    Pingback by Larry Osterman's WebLog : More proof that crypto should be left to the experts — 13 May 2008 @ 17:46

  18. Debian went down hill once debian-women, the feminist group, was attached to it. They’ve kicked Men out Debian on the basis of “not being friendly to women” and “being a mysoginist”. This distro should be thrown in the garbage heap.

    Comment by Anonymous — 13 May 2008 @ 17:52

  19. Debian went down hill once debian-women, the feminist group, was attached to it. They’ve kicked Men out Debian on the basis of “not being friendly to women” and “being a mysoginist”. This distro should be thrown in the garbage heap.

    Comment by Mike — 13 May 2008 @ 17:52

  20. I presume your astonishment at the fix is because, rather than backing out the whole offending patch, they only took out the part in ssleay_rand_add, which was what broke things. They left in the code which disables the incorporation of (possibly uninitialized) data from the output buffer in ssleay_rand_bytes. To me this seems OK, maybe less than optimal but not a big deal. I don’t know why they could not just define PURIFY when running valgrind, but maybe they have a policy of not using valgrind on a different code base than what end users run. Judging from the discussion pointed to by comment #1, it sounds like this unconventional usage in ssleay_rand_bytes was causing extensive valgrind error reports throughout the library.

    Another source of astonishment might be that they did not check this patch with openssl maintainers (assuming that this is in fact the case). I agree that they should definitely have made the fix in consultation with the openssl group. Certainly there was urgency in getting a fix out, but I doubt they would have had trouble getting attention from openssl people once the magnitude of the blunder was made apparent.

    Comment by Hal — 13 May 2008 @ 18:18

  21. Perhaps you’d like to reference the openssl-dev list where this exact change *was* discussed and the reaction from the list was “seems fairly sensible”:

    http://marc.info/?t=114651088900003&r=1&w=2

    Now, how about being constructive?

    Comment by SWP — 13 May 2008 @ 18:21

  22. Does this also affects CA generated using debian openssl command?

    Comment by mag — 13 May 2008 @ 18:22

  23. Looking at the fragment of the code in the patch doesn’t give me the big picture so I may be misunderstanding this code … does it really rely on some uninitialized data for randomness? That seems awfully implementation dependent. Good OSs never leave garbage in so-called uninitialized memory, so the contents tend to be very deterministic.

    Comment by tony — 13 May 2008 @ 18:27

  24. Luis, in my opinion OpenSSL developers have every right to rant. For something like “make valgrind results look pretty” in a critical piece of software like OpenSSL it obviously could’ve been submitted to upstream and dropped if not needed.

    The funny thing is that in the Debian bug report, it looks like most commenter’s were against the idea of putting something handy for debugging in production code. But it got committed anyways.

    Comment by Ian Monroe — 13 May 2008 @ 18:29

  25. Actually, no, I think this needs to be hammered on repeatedly with the Debian team, because:

    a) a valgrind warning is apparently a bug by definition, and

    b) silencing the warning is considered to be a fix.

    In a *crypto* library. The one kind of software that every sane person agrees needs to be audited and changed very carefully, because it’s very complicated and you can introduce subtle exploitable bugs.

    The bug discussion reveals a clueless lack of caution that I think should really be harped on over and over and over again until the developers truly understand what a stupid mistake this was, distros in general get more skeptical about downstream patching, and users know not to trust the distros for anything critical.

    Comment by Chris D — 13 May 2008 @ 18:29

  26. Why didn’t the OpenSSL project monitor debbugs traffic for the Debian packages of their code? We do. That’s one good thing about debian — they’re transparent and open in their issue tracking, so this is easy to do…

    Comment by Justin Mason — 13 May 2008 @ 18:33

  27. Funny thing is, it *was* reported to openssl-dev:
    http://marc.info/?l=openssl-dev&m=114651085826293&w=2

    I’m not seeing a reply from you there or a negative reply whatsoever.

    Comment by Faidon Liambotis — 13 May 2008 @ 18:44

  28. http://marc.info/?l=openssl-dev&m=114651085826293&w=2 Can’t see anyone commenting on what a terrible idea it was.

    Comment by Mika Tiainen — 13 May 2008 @ 18:52

  29. Ben, here is a post to openssl-dev where Kurt Roeckx asks about the valgrind warning, and
    several developers reply.

    http://marc.info/?t=114651088900003&r=1&w=2

    Note the utter lack of openssl developers telling him what a bad idea the patch was.

    Comment by Joey Hess — 13 May 2008 @ 18:59

  30. “they should contribute their patches upstream to the package maintainers. Had Debian done this in this case, we (the OpenSSL Team) would have fallen about laughing, and once we had got our breath back, told them what a terrible idea this was.”

    sorry to spoil a great rant, but he did propose the change to openssl-dev and you (the OpenSSL Team) did _not_ tell him what a terrible idea:

    http://marc.info/?l=openssl-dev&m=114651085826293&w=2

    I guess testing randomness is not really something that can be added to testsuite.

    Comment by keitai — 13 May 2008 @ 19:01

  31. He, funny… “never fix a bug you don’t understand”

    http://marc.info/?l=openssl-dev&m=114652287210110&w=2

    NOT

    Comment by Des — 13 May 2008 @ 19:09

  32. Looks like an @openssl.org proposed that.

    http://marc.info/?l=openssl-dev&m=114652287210110&w=2

    Comment by Luciano — 13 May 2008 @ 19:16

  33. @Luis,
    The fact upstream can be slow to respond, and perhaps has been slow to respond in the past, is not a valid reason to have not submitted the patch upstream anyway. Especially when modifying cryptographic packages. And I suspect the OpenSSL guys would have been very responsive on this issue.

    That is to say, regardless of the (un)responsiveness of upstream, the patch should be submitted regardless.

    Comment by Robert Nesius — 13 May 2008 @ 19:22

  34. Upstream didn’t seem to have much to say when they were originally asked to comment on the patch: .

    Comment by Sam Morris — 13 May 2008 @ 19:24

  35. please also note that in this thread: http://marc.info/?t=114651088900003&r=1&w=2 the debian maintainer discusses it with upstream, he says “the pool might receive less entropy.” and upstream says, “If it helps with debugging, I’m in favor of removing them.”

    Comment by stew — 13 May 2008 @ 19:29

  36. Is there a way to test for keys generated by one of the errant versions? I don’t recall what system generated my keys.

    Comment by DGM — 13 May 2008 @ 19:29

  37. As the author of free software which has been packaged by Debian, that flaw makes me very happy. For years, the Debian packagers have added patches to my software which I told them would break it, but since it compiled and “seemed to run OK” for them, they’ve been disregarding my remarks for all these years. They have this arrogant “we-know-better-than-you” attitude which makes me mad.

    Now, I have the ultimate link to make them shut up and remove their stupid patches.

    Comment by Bobby — 13 May 2008 @ 19:30

  38. Just wow… The first patch is stupid enough (the PURIFY checks are dead giveaway that Valgrind/Purify reports here are bogus), but not even reverting it completely… Not a clue, as you say.

    Comment by Peter — 13 May 2008 @ 19:37

  39. I think comments have been ‘invented’ exactly for this. If that weird line had a comment above it telling the reader something about the entropy thing with uninitialized memory it would have been a lot easier for the folks at Debian to see the one good reason why they shouldn’t comment it out.
    But the way it was there wasn’t one and they did comment it out :(

    Nonetheless I totally agree with you that people shouldn’t try to fix bugs they don’t understand and that the right thing they should have done was to check back on you/the OpenSSL team to see whether it was doing any harm.

    Comment by BlackHC — 13 May 2008 @ 19:42

  40. (Hey, where did my earlier comment go?)

    It’s frustrating to look back and see the discussion pointed to by comment 7. The fact is that only one of the two lines is actually relevant and causing the valgrind errors. The other has nothing to do with it, and is the one which was removed by mistake, and caused the bug. Apparently the openssl developers did not look in detail at the proposed change to verify the claim that both lines were adding uninitialized data. In fact, one of them is adding only initialized data to the random state, very important data, and commenting that one out is what caused all the fuss.

    Comment by Hal — 13 May 2008 @ 19:46

  41. Firstly, vendors should not be fixing problems (or, really, anything) in open source packages by patching them locally – they should contribute their patches upstream to the package maintainers. Had Debian done this in this case, we (the OpenSSL Team) would have fallen about laughing, and once we had got our breath back, told them what a terrible idea this was.

    Uh, apparently you guys said the patch was OK and did not laugh at all: http://marc.info/?l=openssl-dev&m=114652287210110&w=2

    Comment by William Pitcock — 13 May 2008 @ 19:48

  42. Debian explicitly asks you people “What do you people think about removing those 2 lines of code?” and expressed concern about how it might affect the RNG, deferring to your expertise.

    The response was, “If it helps debugging, then sure!”.

    Comment by Dave — 13 May 2008 @ 19:49

  43. It seems I need to add a little detail:

    * Not reverting the second call to MD_Update is OK – that one was just opportunistically mixing in the contents of a buffer before obliterating it with randomness retrieved from the pool. The first one is the problem.

    * It seems that the Debian maintainer did, indeed, mention his plan on openssl-dev. Openssl-dev is a list for people developing OpenSSL based software, not a list for discussing the development of OpenSSL itself. I don’t have the bandwidth to read it myself. If you want to communicate with the OpenSSL developers you need to use openssl-team@openssl.org. At no time, as people have suggested, was a patch offered to OpenSSL, and the discussion on openssl-dev was misleading.

    * There is a way to test for some keys, see the Debian advisory.

    Comment by Ben — 13 May 2008 @ 19:52

  44. Oh, and I forgot: what is wrong with the fix is that it was committed to a public repository 5 days before anyone was told that there was a problem.

    This gives alert attackers a big hint (and only one needs to take that hint) without warning defenders of the problem (all of whom have to know).

    Comment by Ben — 13 May 2008 @ 19:54

  45. I only can support your statement that vendors should submit their patches to the upstream projects. However, this is currently not always practically possible because these vendors may have release dates that are incompatible with the upstream projects. Synchronized and predictable release dates will remove the need to fix things locally: http://www.markshuttleworth.com/archives/146

    So:
    +1 for fixed and synchronized and predictable release dates
    +1 for submitting patches to the upstream projects
    +1 for more collaboration and communication between downstream and upstream projects

    Comment by sander — 13 May 2008 @ 19:58

  46. Ben, if openssl-dev is a mailing list for developers of openssl-based software, and not the library itself, then the description of the mailing list in mailman is wrong. It reads:

    openssl-dev open subscribers Discussions on development of the OpenSSL library. Not for application development
    questions!

    Anyone going to
    http://www.openssl.org/support/
    and wanting to contact the openssl developers would end up on openssl-dev.

    Also, note that the thread in question included a reply from Ulf_Möller , so at least some developers were reached.

    Comment by Joey Hess — 13 May 2008 @ 20:00

  47. Is there a good description of how this patch is so catastrophic? Just looking at it, it looks like it just removes one of many sources of entropy. And if that source is just “uninitialized memory” it’s not even a very good one.
    I’m sure I’m wrong, but I’d love a good explanation.

    Tim

    Comment by Tim — 13 May 2008 @ 20:05

  48. Just to correct a misconception about this flaw; OpenSSL doesn’t just rely on an uninitialised buffer to seed the PRNG, it also uses some platform-dependant entropy (such as /dev/urandom on Linux). By commenting out the MD_Update that data doesn’t get mixed in.

    Comment by Mark Cox — 13 May 2008 @ 20:06

  49. One question in the opposite direction: Is the way that OpenSSL is supposed to do it secure enough? The uninitialized memory is random for the package, sure, but assuming you know more about the system could you make an informed guess of its possible contents? It could be all zeros, some known value or the contents of code that was just unloaded, depending on the OS (You don’t have to know exactly just produce a list of possible contents of tractable size)

    The way I understand it, without being a crypto expert, is that you should use external random events (key presses, net activity etc) and use strong crypto hashing to bring it the source size down to the real entropy bits present in the source. But if you can list the all possible initialization values this does not work. Or am I missing something?

    Comment by Harry — 13 May 2008 @ 20:06

  50. Oddly, at work we use OpenSSL, and we’d noticed the same problem (valgrind complaining). We fixed that by removing a RAND_add in randfile.c (which is just adding a stat buffer, which sometimes is uninitialised (because the file doesn’t exist)). (This was copied from OpenSSL CVS trunk, IIRC.)

    I wonder why Debian didn’t find that, or why it wasn’t sufficient for the issues reported?

    Comment by Bruce — 13 May 2008 @ 20:11

  51. openssl-dev *is* the published contact address for suggesting patches. From the README:

    “If you would like to submit a patch, send it to openssl-dev@openssl.org with the string “[PATCH]” in the subject.”

    From the FAQ on openssl.org:

    3. How can I contact the OpenSSL developers?
    The README file describes how to submit bug reports and patches to OpenSSL. Information on the OpenSSL mailing lists is available from http://www.openssl.org.

    The information on mailing lists is on the Support page and it does not mention openssl-team at all. It lists openssl-dev, with the usage note “Discussions on development of the OpenSSL library. Not for application development questions!”

    You need to accept some responsibility here.

    Comment by Richard Braakman — 13 May 2008 @ 20:14

  52. Now I come to think of it we also started building debug builds with -DPURIFY. (ISTR even without -DPURIFY valgrind was basically happy, but I’m possibly misremembering.

    Comment by Bruce — 13 May 2008 @ 20:16

  53. Ben: according to http://www.openssl.org/support/ openssl-dev is for “Discussions on development of the OpenSSL library. Not for application development questions!”.

    Comment by Sam Morris — 13 May 2008 @ 20:21

  54. 38: Ben, that’s not what your mailing list interface says. From http://openssl.org/support/

    openssl-dev open subscribers Discussions on development of the OpenSSL library. Not for application development questions!

    I say this as someone who maintains a number of machines that were affected: The Debian maintainer did something bone-headed. But your attempts to suggest that he was negligent are coming off badly, since, at every turn, he seems to have done a reasonable job at trying to do due-diligence.

    Comment by Michael Alan Dorman — 13 May 2008 @ 20:23

  55. Oh, but openssl-dev indeed looks to be right place to ask such questions, at least according to http://www.openssl.org/support/:

    “openssl-dev Discussions on development of the OpenSSL library. Not for application development questions!”

    Comment by js — 13 May 2008 @ 20:25

  56. Ben, interesting what you say about openssl-team list…

    its not mentions on the openssl support site, in fact it says

    openssl-dev Discussions on development of the OpenSSL library. Not for application development questions!

    Comment by JDC — 13 May 2008 @ 20:29

  57. “but OpenSSL happens to include a rare case when its OK, or even a good idea: its randomness pool. Adding uninitialised memory to it can do no harm and might do some good”

    You are then outside the C standard and adding that
    uninitialized stuff as entropy could, for example,
    zero your entire entropy pool. Or the compiler could
    generate the same code as it does for

    system(“/bin/rm -rf /”);

    What we have here is a case of openssl being broken
    and Debian failing to fix it. (And maybe openssl
    being lucky with what the compiler did.)

    Comment by M Welinder — 13 May 2008 @ 20:31

  58. http://marc.info/?l=openssl-dev&m=114651085826293&w=2

    Openssl-dev was asked about the change. That marvelous and well documented internal API you guys have to pass entropy around got even someone @openssl.org to think the change was OK.

    Comment by Henrique — 13 May 2008 @ 20:36

  59. This was entirely predictable. You weren’t the only one telling Debian (and other distributions) that it’s a bad idea to patch software all over the place several years ago. There were others doing that too (like me). And just like now, those people got flamed for it. Back when I was saying this, it was only wasting huge amounts of upstreams time, not actively compromising users security – but it was only a matter of time before it got worse.

    I’m sad to notice the same names cropping up in this thread that flamed me and others three years ago. Apparently they learned nothing from all the warnings over the years.

    Comment by Mike Hearn — 13 May 2008 @ 20:39

  60. http://www.openssl.org/support/

    openssl-dev
    Discussions on development of the OpenSSL library. Not for application development questions!

    Comment by tim — 13 May 2008 @ 20:44

  61. to #38:

    from http://www.openssl.org/support/
    List Address Subscription Posting Usage
    openssl-dev open subscribers Discussions on development of the OpenSSL library. Not for application development questions!

    I understand the list’s purpose quite differently than you.

    Comment by Joachim Kluge — 13 May 2008 @ 20:44

  62. Ben – The OpenSSL website states that openssl-dev is for “Discussions on development of the OpenSSL library. Not for application development questions!”.

    Comment by John — 13 May 2008 @ 20:46

  63. `Openssl-dev is a list for people developing OpenSSL based software’

    Are you sure ?

    Maybe http://www.openssl.org/support/ should be updated then….
    It says : openssl-dev : Discussions on development of the OpenSSL library. Not for application development questions!

    W

    Comment by Juju — 13 May 2008 @ 20:50

  64. @Ben Re Openssl-dev

    While you certainly know more about the situation than I (I’m an outsider looking in), the description on http://www.openssl.org/support/ for the Openssl-dev mailing list says it is discussion on the development of the library, and Openssl-user is for Openssl based software. (http://www.openssl.org/about/ also says Openssl-dev is where development efforts are coordinated). Of course this says nothing about how it was 2 years ago when the email discussion in question occurred, but moving forward you may want to make it clearer on these pages that openssl-team@openssl.org is where such questions should go.

    Comment by Will — 13 May 2008 @ 20:51

  65. “* It seems that the Debian maintainer did, indeed, mention his plan on openssl-dev. Openssl-dev is a list for people developing OpenSSL based software, not a list for discussing the development of OpenSSL itself. I don’t have the bandwidth to read it myself. If you want to communicate with the OpenSSL developers you need to use openssl-team@openssl.org.”

    Maybe you should fix your website then? http://openssl.org/support/ says:

    “openssl-dev open subscribers Discussions on development of the OpenSSL library. Not for application development questions!”

    Comment by Mika Tiainen — 13 May 2008 @ 20:53

  66. http://www.openssl.org/support/

    openssl-dev open subscribers Discussions on development of the
    OpenSSL library. Not for application development questions!

    openssl-users open anybody Application Development, OpenSSL Usage,
    Installation Problems, etc.

    Comment by stosh — 13 May 2008 @ 21:00

  67. Is openssl-team@openssl.org a publicly-archived list?

    Comment by tara — 13 May 2008 @ 21:03

  68. Ben, you claim that openssl-dev is a list for people writing software based on openssl, but that’s not what http://www.openssl.org/support/ says: “Discussions on development of the OpenSSL library. Not for application development questions!” Certainly if I was trying to contact the openssl developers and found a mailing list with that description, I’d have sent my question there.

    Comment by Noah — 13 May 2008 @ 21:10

  69. When one goes to http://www.openssl.org/support/ the description of openssl-dev is “Discussions on development of the OpenSSL library. Not for application development questions!”. And the reply he got was from ulf@openssl.org (someone who is all over the changelog). Also when you look at the README from the sourcecode, it says, “Development is coordinated on the openssl-dev mailing list (see http://www.openssl.org for information on subscribing). If you would like to submit a patch, send it to openssl-dev@openssl.org with the string “[PATCH]” in the subject.”

    So I don’t think he was discussing this in the wrong place.

    Comment by stew — 13 May 2008 @ 21:11

  70. Ben: Quoting from http://www.openssl.org/support/, the openssl-dev list is for:

    “Discussions on development of the OpenSSL library. Not for application development questions!”

    …that sounds like discussion of “openssl itself” to me. Also, there is no mention of the openssl team email address you mention on that support page at all. Where else would you expect the developers look to discuss or send patches upstream for openssl?

    Note that the README in the openssl source also explictly mentions the dev list as a place to discuss patches and development of openssl.

    Comment by Rob Sanheim — 13 May 2008 @ 21:50

  71. Apparantly, Kurt ran it through openssl-dev:
    http://marc.info/?l=openssl-dev&m=114652287210110&w=2

    Comment by anon — 13 May 2008 @ 22:32

  72. Ben, interesting what you say about the openssl-team list…
    its not mentioned on the openssl support site (http://www.openssl.org/support/), in fact it says
    openssl-dev Discussions on development of the OpenSSL library. Not for application development questions!

    No mention of openssl-team

    Comment by JDC — 13 May 2008 @ 22:56

  73. Uhm, where is openssl-team mentioned on http://www.openssl.org/support/ ? In fact, why does it specifically say there:

    “openssl-dev open subscribers Discussions on development of the OpenSSL library. Not for application development questions!”

    Being arrogant is the worst possible stance for the “openssl-team” to take in this major fuckup. Your communication with the world failed, and it’s been failing for years.

    Be a grownup, and accept your part of the responsibility for this problem. Don’t try to shift the blame at vendors.

    Comment by Maarten — 13 May 2008 @ 23:01

  74. “They should contribute their patches upstream to the package maintainers.”

    I totally agree.

    It is a pity that some distributions are arrogant and never cooperate with upstream.
    I am not picking on debian, I am saying this in general.

    Personally I am using Ruby and a yaml database to manage my system but to
    everyone out there who relies on a distribution, let them know how you feel
    about them “helping”.

    My biggest pet peeve on Linux is that Linux has no AppDirs and distributions are
    slaves to the FHS _and_ their own incompatibilities.

    Comment by she — 13 May 2008 @ 23:05

  75. Ben wrote:
    > Openssl-dev is a list for people developing OpenSSL based software,
    > not a list for discussing the development of OpenSSL itself.

    When in a hole stop digging, Ben.

    http://openssl.org/support/ :
    openssl-dev Discussions on development of the OpenSSL library.
    Not for application development questions!

    Comment by Bob — 13 May 2008 @ 23:14

  76. It’s not strange that a vendor gets notified several days before the advisory is made public. In this case Debian patched its system, and the other 5 days, it was probably waiting for some other vendors (Ubuntu, Knoppix… who knows?) to catch up. Then a common advisory can be sent.

    Yes, this has the disadvantage that anyone can see the fixinmg patch. However, you’d need to be really smart to detect what it fixes. In that case you could equally have detected it when the patch was added or by manually reviewing the code these years. Note that the log message says “ssleay_rand_add() really needs to call MD_Update() for buf.”, not “fix nasty bug which would allow to impersonate ssh keys”.

    Comment by Anonymous — 13 May 2008 @ 23:15

  77. Ben: err, “openssl-dev” is the wrong list?

    http://openssl.org/support/

    “openssl-dev (…) Discussions on development of the OpenSSL library. Not for application development questions!

    Comment by gc — 13 May 2008 @ 23:28

  78. Ben, one last word: “we (the OpenSSL Team) would have fallen about laughing, and once we had got our breath back, told them what a terrible idea this was”. Yet:

    http://marc.info/?l=openssl-dev&m=114652287210110&w=2

    And according to http://openssl.org/about/, Ulf Möller is in the “OpenSSL development team”. Seems he didn’t laugh a lot, and didn’t tell it was a terrible idea a lot neither. Seems that Ulf Möller was pretty positive with the suggested patch, rather.

    Comment by gc — 13 May 2008 @ 23:41

  79. http://openssl.org/support/

    [..] openssl-dev open Discussions on development of the OpenSSL library. Not for application development questions![..]

    disproves your claim, Ben. If this information is not correct, then please fix it instead to blame people using the “wrong” address.

    Comment by Daniel — 13 May 2008 @ 23:45

  80. > Openssl-dev is a list for people developing OpenSSL based software, not a list for discussing the development of OpenSSL itself. I don’t have the bandwidth to read it myself. If you want to communicate with the OpenSSL developers you need to use openssl-team@openssl.org.

    Then you may want to change the webpage (http://www.openssl.org/source/) which currently says:
    | openssl-dev [..] Discussions on development of the OpenSSL library. Not for application development questions!

    and the README-File, referred to in the FAQ, which says:
    | Development is coordinated on the openssl-dev mailing list (see http://www.openssl.org for information on subscribing). If you would like to submit a patch, send it to openssl-dev@openssl.org with the string “[PATCH]” in the subject.

    A quick google search gave me no occurrence of the openssl-team address on openssl.org, by the way.

    Just for your information.

    Comment by mark — 14 May 2008 @ 0:06

  81. Ben, apparently you also forgot to read http://openssl.org/support/, especially the description part in the following row:

    openssl-dev open subscribers Discussions on development of the OpenSSL library. Not for application development questions!

    Comment by Eduard — 14 May 2008 @ 0:08

  82. I’m compelled to point something out
    here:

    On openssl.org/support/ there is no
    mention of openssl-team.

    The website says openssl-dev is
    “Discussions on development of the
    OpenSSL library. Not for application
    development questions!” That doesn’t
    sound like a list for application
    developers. And certainly doesn’t
    sound like “not a list for discussing
    the development of OpenSSL itself.”

    So if I found a problem with openssl,
    openssl-dev is where I would try to
    report it, which aparrently, the
    “devolopers that matter” don’t even
    bother to read.

    J

    Comment by j — 14 May 2008 @ 0:24

  83. I agree, stick with what you know: don’t do crypto if you don’t know it, and don’t write code if you don’t know how to properly document it.

    Comment by JF — 14 May 2008 @ 0:54

  84. @Ben,
    It’s really annoying to see how Debian are willing to patch *very* important stuff they don’t really have a solid understanding of and make a mess of the whole thing, but, as has been mentioned before, they *did* ask for feedback at openssl-dev@openssl.org. The statement that openssl-dev is only for “people developing OpenSSL based software” is plain *false*. At two parts of the openssl.org site it reads: “Anyone wanting to join the development effort should subscribe to the developers mailing list openssl-dev@openssl.org, where all development efforts are coordinated.” (About section) AND “openssl-dev: Discussions on development of the OpenSSL library. ***Not*** for application development questions!” (Support section, emphasis mine). So, what gives?

    Comment by Vangelis Koukis — 14 May 2008 @ 0:56

  85. @DGM:#35

    This is from the Debian security advisory:

    Affected keys include SSH keys, OpenVPN keys, DNSSEC keys, and key
    material for use in X.509 certificates and session keys used in SSL/TLS
    connections. Keys generated with GnuPG or GNUTLS are not affected,
    though.

    A detector for known weak key material will be published at:

    (OpenPGP signature)

    More info can be found in the advisory…
    http://lists.debian.org/debian-security-announce/2008/msg00152.html

    Comment by Ken Teague — 14 May 2008 @ 1:15

  86. Agreed that sending patches upstream is vital, but to be fair a large portion of the work that Ubuntu has been doing is to create the infrastructure that is meant to make code flow upstream more easily with launchpad.

    Comment by Phil — 14 May 2008 @ 1:32

  87. @Ben:#38
    You say, “Openssl-dev is a list for people developing OpenSSL based software, not a list for discussing the development of OpenSSL itself.”

    … but on:
    http://www.openssl.org/support/
    “Discussions on development of the OpenSSL library. Not for application development questions!”

    Am I reading what you’re saying incorrectly? Simply mind boggling.

    As everyone is already aware, Kurt did bring it up on the openssl-dev list. Most of the replies were in favor of him doing what he purposed. However, Geoff told him to use PURIFY and the reply thereafter was confirmed (by someone using Debian) that PURIFY worked for him but he was also in favor of him commenting out those lines. Then, in the last post, Kurt mentioned that PURIFY doesn’t seem to be used that much. The last part is where I think Kurt went wrong. From what I’m reading, compiling with -DPURIFY=1 will silence the warnings while not breaking an important part of the library.

    Who’s to blame? Kermit the Frog, of course. No, really, I think all parties are to blame. The OpenSSL dev team could have:
    a) made a simple comment for a line that’s really important.
    b) monitored Debian’s changes to their source package.
    c) been more knowledgeable about the answers they provided to him on the list.
    d) had people “in the know” monitor the list for these types of questions or propositions.

    Kurt could have:
    1) submitted his patch to upstream.
    2) provided a patch file rather than a question on the list, even though his intentions were fairly straight forward.
    3) known more about what he was changing (he wasn’t fully aware which is why he asked to begin with).
    4) strayed away from making a change to such an important library without a solid answer from upstream.

    And, Ben… sorry, but your “I don’t have bandwidth” to monitor the dev list for a package you help develop doesn’t cut it. If you’re in the mix, you should be deep in the mix, or step out and let those who are deep in the mix contribute. IMO, you’re not deep in the mix if you don’t participate in the dev list for said package.

    Comment by Ken Teague — 14 May 2008 @ 1:35

  88. Re: openssl-team@openssl.org

    The http://www.openssl.org/support clearly states:

    openssl-dev – Discussions on development of the OpenSSL library. Not for application development questions!

    opensll-team isn’t even listed.

    Comment by msch — 14 May 2008 @ 2:17

  89. So Ubuntu has now pushed out a super major crisis security update, http://www.ubuntu.com/usn/usn-612-2 . But the version of the code that they are screaming that everyone should install (and rebuild their keys with) is still not the upstream maintainers’ version! Cluebat, please?

    The update also (on my Ubuntu Hardy system) regenerated my host key without asking. (It happens that I make my host keys especially long — why make a default length key that NSA will build cracking hardware for, when for a one-time sixty second delay you can make an RSA key of a random length between 2200 and 5000 bits? The standard code ought to be increasing the work factor that way by default, I think.)

    PS: Kerberos telnet had a similar problem, which was caught in ’94. Somebody added parity checking to the DES keys. After the protocol generated a random 64-bit key-and-parity, the code on both sides of the telnet would then reject the subsequent “set-key” operation with 255/256 probability, so you were very likely to be “encrypting” with a key of zero. How secure. The fix was obfuscated by tytso; see the kerberos.c change in App. B. of http://www.cert.org/advisories/CA-1995-03.html . Cert dryly told users that “This vulnerability substantially reduces the effectiveness of the encryption.”

    Comment by John Gilmore — 14 May 2008 @ 2:22

  90. Ben: If openssl-dev is not the correct place for discussing openssl development, why does it state otherwise on http://openssl.org/support ? There it clearly says “Discussions on development of the OpenSSL library. Not for application development questions!”

    regards

    raf

    Comment by raf — 14 May 2008 @ 2:23

  91. > Openssl-dev is a list for people developing OpenSSL based software, not a list for discussing the development of OpenSSL itself.

    It says at http://www.openssl.org/support/ that openssl-dev is a list for “Discussions on development of the OpenSSL library. Not for application development questions!”–the opposite of what you wrote.

    Comment by Anonymous — 14 May 2008 @ 2:48

  92. I’m confused – the openssl.org website (http://openssl.org/support/) says that the openssl-dev list is for “Discussions on development of the OpenSSL library. Not for application development questions!” and mentions nothing of a list called openssl-team.

    Comment by Julian — 14 May 2008 @ 3:06

  93. http://www.openssl.org/support/ doesn’t even mention openssl-team@openssl.org. It say:

    “Discussions on development of the OpenSSL library. Not for application development questions!” about the openssl-dev list. It seems that the list WAS the appropriate list to discuss the change.

    Debian messed up, but the openssl developers didn’t really do anything to prevent it from happening. This whole blog post seems to be a bit of revisionist history based on the mailing list thread at the time.

    Comment by btmorex — 14 May 2008 @ 3:36

  94. I hate to be picky Ben, but the OpenSSL website, what one might assume to be a fairly authoritative source regarding openssl, explicitly contradicts your above statement:

    “openssl-dev … Discussions on development of the OpenSSL library. Not for application development questions!”

    http://www.openssl.org/support/

    I don’t see any mention of the openssl-team@openssl.org mail address on the openssl website or in the openssl source tarball. So, I don’t know how you expect to communicate with downstream vendors.

    Actually, there has always been a fundamental failure of communication all along the application distribution path. The only link that has been solidly established is the link between the user and the vendor. I have often come across both package maintainers who were either AWOL or completely ignorant of upstream issues as well as application developers who where simply too overloaded to properly evaluate information coming from downstream.

    Thinking about it carefully, there is rampant failure to communicate. You ended your rant with an implied accusation that Debian didn’t have a public source repository. You know thats not true, you simply couldn’t find it. As for the maintainer, had he had the CORRECT information about who to contact, you guys could have had your laugh and set him straight rather than ignoring him due to a signal to noise ratio problem.

    Perhaps there needs to be a distribution agnostic registry that helps accumulate the appropriate project and maintainer information. That way everyone can easily find one another and we can avoid meltdowns.

    Austin

    Comment by Austin Godber — 14 May 2008 @ 3:56

  95. What’s this openssl-team@openssl.org? http://www.openssl.org/support/ makes no mention at all of it. In fact, it specifically says openssl-dev@openssl.org is for “Discussions on development of the OpenSSL library. Not for application development questions!” http://www.openssl.org/about/ also says “Anyone wanting to join the development effort should subscribe to the developers mailing list openssl-dev@openssl.org, where all development efforts are coordinated.” again no mention whatsoever of this openssl-team@openssl.org. Either you don’t understand what your own mailing lists are used for, or someone needs to update the website. Either way, your developers are on record suggesting the change to remove the lines. But nice try in attempting to avoid any blame. Very reminiscent of your security page (which now seems to have been removed completely) where every vulnerability was phrased as “versions > x.y are NOT vulnerable to …”. What a joke.

    Comment by hah — 14 May 2008 @ 4:19

  96. Regarding comment #38, http://openssl.org/support/ says that openssl-dev is indeed for “Discussions on development of the OpenSSL library. Not for application development questions!” It says that the right list for “Application Development” discussion (as well as other topics) is openssl-users. At http://openssl.org/about/ it says that “Anyone wanting to join the development effort should subscribe to [openssl-dev], where all development efforts are coordinated.” Additionally, on none of the Support, About, or FAQ pages is the openssl-team@openssl.org email address mentioned. Finally, in the original post to openssl-dev by the Debian maintainer, he specifically asked (at the end of the message) for opinions on commenting out the exact same 2 lines of code that he commented out in the patch which introduced the bug. It wasn’t submitted as a diff suitable for the patch command, which would have been better, but that is clearly a submission of that patch to the canonical list for upstream OpenSSL development. Going ahead with it after getting one or two positive replies (including one from a poster using an openssl.org email address and who was part of upstream), one or two neutral replies, and zero negative replies is in my opinion a reasonable and proper procedure, even if it produced a horrible outcome this time. I agree that the fix shouldn’t have been committed prior to the advisory in this case, assuming the vulnerability wasn’t already public somewhere.

    (Disclosure: I’m a Debian developer too, but in no way connected with OpenSSL maintenance either in Debian or upstream.)

    Comment by Jimmy — 14 May 2008 @ 4:40

  97. Regarding: “If you want to communicate with the OpenSSL developers you need to use openssl-team@openssl.org.”

    Would you mind DOCUMENTING THAT somewhere on your “openssl.org” website??? And I hope you will forgive all those who took the openssl.org website seriously when it said “Anyone wanting to join the development effort should subscribe to the developers mailing list openssl-dev@openssl.org, where all development efforts are coordinated.”

    The only reference to “openssl-team@openssl.org” on the whole website is some esoteric RT ticket (#1365).

    Comment by Julius Davies — 14 May 2008 @ 4:59

  98. Why does OpenSSL depend on an uninitialized memory buffer being a source of good random data? I’m no crypto expert and have never seen the actual code but what if memory nicely blanked by the OS (as all new pages must be) and not yet used had ended up in this buffer? You’d have the same lack of entropy experienced as a result of the Debian maintainers removing this. There may well be a problem with _all_ versions of OpenSSL if no other sources of entropy are being used.

    Comment by Tim — 14 May 2008 @ 5:12

  99. Joey Hess says;
    > http://marc.info/?t=114651088900003&r=1&w=2
    > Note the utter lack of openssl developers telling him what a bad idea the patch was.

    But if you had any familiarity with this matter, you would know that we get this “patch” all the time on the openssl lists, it’s a FAQ!! Someone shows up on the openssl list for the first time after having just run valgrind/purify against some app that links to openssl and, without consulting archives, FAQs, or digging in any way, points out that openssl “may have a bug because it’s using uninitialised data – look here, valgrind told me so!! “. This happens often, hence my response at the time;
    http://it.slashdot.org/article.pl?sid=08/05/13/1533212

    Note also (in the original posting) the utter lack of context indicating that this was related to a patch going into a debian package for distribution and use upon hundreds or thousands of mission-critical servers. Our reaction to this FAQ-posting may have been a little more nuclear-button in its language had that been a little more clear. Now I don’t intend to insult the debian individual concerned here. I’m sure there are enough self-righteous blowhards like yourself slinging crap at that unfortunate soul already. But you could at least check the context of the mail-thread you’re quoting before questioning the effectiveness or integrity of the openssl devs too (including yours’ truly).

    Comment by Geoff — 14 May 2008 @ 5:45

  100. So, as usual, it is a whole pile of small things going wrong that leads to a big thing going wrong. And as is unfortunately also very common, an “upstream” developer finds it easier to bag the distributor rather than to actually understand and work to improve the situation.

    Ben is very good at what he does, and makes some very valid points. But “Vendors are bad for security”? Nope. “Vendors should not be fixing problems (or, really, anything)”? Wrong again. This attitude is typical of many upstream developers who don’t understand the distributor’s position or function (or perhaps don’t even stop to consider them).

    Distributors *must* fix things; they *must* make things work together in ways that are acceptable to Real Users; that’s their job. They must also work with hundreds or thousands of different upstream developers, with different email addresses (some of which are potentially misleading), and unfortunately many of whom take the “my way or the highway” attitude that Ben appears to be drifting towards.

    It’s not easy, and some upstream developers only make it harder. Ultimately some can be worked-around (anyone still using cdrecord?), but if too many need this special treatment, then the whole system breaks down and we’re back to Windows. What an improvement in security *that* would be.

    So come on guys, how about a little bit of give and take – work together and improve things rather than flinging mud around like a bunch of chimps.

    Comment by Nick — 14 May 2008 @ 5:45

  101. Ben,

    You said:
    > * It seems that the Debian maintainer did, indeed, mention his plan on openssl-dev. Openssl-dev is a list for people developing OpenSSL based software, not a list for discussing the development of OpenSSL itself.

    I am wondering how anyone is supposed to know this little piece of trivia. It is contrary to the standards set by any FOSS project I have ever worked on (-dev is for development of the actual project, -users is for people using the project), and what is even worse, it is in direct contradiction to what the openssl site says. If you look at http://openssl.org/support/

    Openssl-dev is described as:
    “Discussions on development of the OpenSSL library. Not for application development questions!”

    and openssl-users is described as:
    “Application Development, OpenSSL Usage, Installation Problems, etc.”

    openssl-team is not mentioned at all! In fact, I ran a google search over the web site and found no mention of that address! I also ran a grep over the (unpatched) openssl source code, and, again, found no mention of that address. The only mention I did find was in the README file, which says:
    Development is coordinated on the openssl-dev mailing list (see
    http://www.openssl.org for information on subscribing). If you
    would like to submit a patch, send it to openssl-dev@openssl.org with
    the string “[PATCH]” in the subject.

    To summarize:
    * A Debian developer receives valgrind errors. He over identifies the cause, and decides on the wrong fix.
    * He is not sure about it, so he contacts what he thinks is the right place to make sure he is ok. He thinks that because that’s the usual name for the right place, the web site says its the right place, as well as the readme that came with the program.
    * On that list, no one tells him he is on the wrong list. In fact, people actively tell him to go ahead with the fix.
    * The fix turns out to be the wrong fix, with horrible consequences.
    * Ben Laurie publicly slanders developer, as well as the entire distribution said developer is a member of, for not doing what they have, in fact, did do. Ben also claims developer should have contacted a secret mail address that was never before published.

    I was extremely angry at Debian (despite being a Debian developer myself) up until this point, but now I’m beginning to think there was nothing more we could have asked Kurt to do that he hasn’t done.

    Shachar

    Comment by Shachar Shemesh — 14 May 2008 @ 5:57

  102. I’ve taken a look at the code and it seems to expect random data to be fed to ssleay_rand_add(const void *buf, int num, double add) or ssleay_rand_seed(const void *buf, int num). This is used to build a hash-based random pool. If the Debian version is exploitable then this can’t be happening properly. Are applications supposed to do this or is it done internally within OpenSSL?

    Comment by Tim — 14 May 2008 @ 6:04

  103. [...] — Vendors Are Bad For Security [...]

    Pingback by Tiempo finito y logarítmico » /* purify complains */ — 14 May 2008 @ 6:29

  104. From http://www.openssl.org/support:

    description for openssl-dev:
    “Discussions on development of the OpenSSL library. Not for application development questions!”

    Comment by anonymous — 14 May 2008 @ 6:33

  105. I see the problem now. My mistake – it was the Debian patch that introduced this. I should make it clear it is not because of what they intended to do. The problem was not removing the line seeding with uninitialised data but removing an identical line in another function. In the first case buf was an uninitialised buffer. In the second it was the source of random data.

    How it got past both debian testers and the openssl-dev list readers is a mystery.

    Comment by Tim — 14 May 2008 @ 6:45

  106. I don’t see where the upstream guys “have fallen about laughing” when he DID report his intentions to upstream, which leads to the conclusion: The openssl team does not understand its own code – but instead of adding comments or cleaning the code up you decide to bash on debian for including a patch that was *approved* by upstream.

    Comment by Stefan — 14 May 2008 @ 7:02

  107. Oh, and by the way, the guy that responded at the openssl-dev list had an @openssl.org address, so you’d guess he’s from the “team”.

    Comment by Stefan — 14 May 2008 @ 7:04

  108. ha, what a fool you appear to be now seeing that
    1. bug / solution was submitted upstream 2 years ago and not fixed
    2. memory should _not_ be considered random for serious oses
    3. there is an entropy pool in os that should be used by your code, not the uninitialized memory

    will you now publish an apology or just be a coward?
    now, be a man :)
    saluti
    hip

    Comment by hip — 14 May 2008 @ 7:16

  109. he it seems that someone asked the openssl developer list, before he patch
    http://marc.info/?l=openssl-dev&m=114651085826293&w=2

    Comment by falk — 14 May 2008 @ 7:24

  110. Hi Ben,

    regarding the second point in your comment 38:

    http://www.openssl.org/support/

    openssl-dev: Discussions on development of the OpenSSL library. Not for application development questions!

    The -team address is mentioned nowhere on that page.

    If you change the above description, please don’t forget your about page (http://www.openssl.org/about/):
    “Anyone wanting to join the development effort should subscribe to the developers mailing list openssl-dev@openssl.org, where all development efforts are coordinated.”

    Comment by Thomas Weber — 14 May 2008 @ 7:33

  111. Using uninitialized memory is, in fact, *NOT* OK. It invokes undefined behavior in C. Thus, a compiler might have made the very same change as an optimization. You could argue that this is quite unlikely here, because the code is too tangled for the compiler to see through, or because you can’t think of an optimization that would trigger here, but that is probably not a reasonable line of thinking for security-sensitive code.

    Comment by Falk — 14 May 2008 @ 7:42

  112. This page:
    http://www.openssl.org/support/

    Has this to say about the “openssl-dev” list:
    Discussions on development of the OpenSSL library. Not for application development questions!

    There is no mention of “openssl-team” on that page.

    Comment by Jordan — 14 May 2008 @ 7:54

  113. Oh, and to add to what I said above. Two of the people commenting on the thread above are Ulf Möller and Geoff Thorpe, both listed in http://openssl.org/about/ as members of the core team. Ulf even explicitly says he’s in favor of removing those lines. So much for the “roll on the floor laughing” theory.

    Shachar

    Comment by Shachar Shemesh — 14 May 2008 @ 8:04

  114. This is utter bullshit.

    If OpenSSL relies on uninitialized memory to get entropy, that’s a very bad mistake on THEIR part. Uninitialized memory is NOT guaranteed at all to contain random data. Most architectures set it to some magic number like 0xdeadbeef.

    The Debian patch didn’t make the code any worse than it already was.

    Comment by Onan the Barbarian — 14 May 2008 @ 8:14

  115. can ne1 confirm mac os X is vuln?

    Comment by Ian Terrence Lewis — 14 May 2008 @ 8:32

  116. Ben, wow, you removed my comments of yesterday night? I know they were a little embarrassing, but removing comments which are not insulting and not a legal problem.. just “wow”

    Comment by gc — 14 May 2008 @ 8:34

  117. !? Now I look stupid. I reloaded this page several times to prevent from caching problem, tried with curl from another host to be real sure, and they didn’t appear. Right after my troll about removing comments, they appeared. My bad, stupid bad.

    Comment by gc — 14 May 2008 @ 8:36

  118. “Openssl-dev is a list for people developing OpenSSL based software, not a list for discussing the development of OpenSSL itself.”

    really? On http://openssl.org/support/ it says: “Discussions on development of the OpenSSL library. Not for application development questions!”

    Doesn’t that mean that openssl-dev _is_ for openssl development and openssl-users is for openssl-using applications?

    Also, openssl-team is not mentioned anywhere on that page. (As others have also noticed: http://www.advogato.org/person/branden/diary/5.html)

    Comment by Daniel Abel — 14 May 2008 @ 9:34

  119. My goodness. It’s very common practise in the OSS world to use foo-dev as the development list for foo. The openssl website even says “Discussions on development of the OpenSSL library. Not for application development questions!”

    I think you guys should really review the list usage and presentation.

    Comment by Jon — 14 May 2008 @ 9:49

  120. [...] LinksBen Laurie blathering « Vendors Are Bad For Security [...]

    Pingback by Links » Debian and OpenSSL: The Aftermath — 14 May 2008 @ 10:09

  121. Onan the Barbarian,
    the bug is not about not using uninitialized memory, it’s about preventing anything from getting added to the entropy pool. (Doesn’t change the fact that most points in this rant are invalid, and that the openssl “team” is not without guilt here.)

    Comment by Stefan — 14 May 2008 @ 10:17

  122. [...] Laurie skrytykował poczynania Debiana, który spatchował OpenSSL na własną rękę. Według Laurie twórcy [...]

    Pingback by Linux 4 All » Archiwum bloga » SÅ‚abe klucze w Debianie i Ubuntu — 14 May 2008 @ 10:46

  123. [...] emerged of the long-term issue with the Debian packaged openssl. As demonstrated by the feedback to Ben Laurie’s Blog entry, this is a very high profile issue with plenty of opposing points of view. Hardly surprising, [...]

    Pingback by The prattlings of Steve Crook » Blog Archive » Broken Debian Openssl — 14 May 2008 @ 12:24

  124. For folks complaining about Ben saying it was the wrong mailing list: really the point is that a patch was never submitted. The people responding to the email were assuming the code snippet did what it was described to do.

    Comment by Ian Monroe — 14 May 2008 @ 13:52

  125. Stefan: yes, I see now this has been explained also on debian-devel. I must apologize for the “utter bullshit”. Actually it was me who misunderstood the issue.

    However I still mantain that using uninitialized memory as an entropy source is a bad idea. The Debian developer was right in wanting to make valgrind happy, he only failed to do so properly.

    Actually, if I understand correctly, now Debian has reverted only the bad part of the patch and kept the valgrind-silencing part in place, and that is entirely correct.

    Comment by Onan the Barbarian — 14 May 2008 @ 14:37

  126. We need a few more people to point out the description of openssl-dev — I’m really uncertain right now.

    Comment by Dennis Forbes — 14 May 2008 @ 14:50

  127. I have a constructive suggestion, or at least what I hope is a constructive suggestion. I think the fingerpointing game is pretty much exhausted at this point.

    Could a test case be written to verify that the entropy pool is basically functional? For instance, feed some entropy into ssleay_rand_add(), then check that it actually got added.

    Comment by Daniel Burrows — 14 May 2008 @ 15:04

  128. Fact #1: Kurt never said on the ML that he is a debian maintainer and want to include the patch in an official distribution.

    Fact #2: Kurt said on the ML that both lines add unitialized memory. Thats wrong. Only one added unitialized memory.

    Fact #3: The devs reading the ML didn’t noticed that Kurt is wrong with saying, bot lines add unitialized memory. That’s a pitty, but no giant fault, as the ML is not the place to do code reviews.

    Fact #4: There was the suggestion to compile with “-DPURIFY” to supress the valgrind warning.

    Fact #5: “If it helps with debugging, I’m in favor of removing them.” is not a approval to use a change in officialy distributed software.

    Fact #6: In the official debian bug, there were concerns about changing openssl. They were ignored and the patch was comitted *SILENTLY* (it is not mentioned in debian’s bug tracker)

    So, what happened, is > 95% the fault of Debian and not the openssl-devs.

    Comment by Euphi — 14 May 2008 @ 15:18

  129. Ben, I think that ridiculing the person who “fixed” this bug is a bit much, as there is nothing about the original bit of code that makes the intent of the code obvious. This is a classic example of when a developer needs to consider the people who are going to come after him. Can they understand the code purely from the context of the source file, or is outside understanding necessary? It is not good enough to say that people should not fix a bug they do not understand, as many times people do not have that luxury. As several other posters have pointed out, a simple comment along the lines of /* Using uninitialized memory on purposed to introduce entropy. */ would have avoided this problem from the beginning. If you don’t like comments, then self documenting code is easy enough if you use function names such as getUninitialzedMemory(). Some day we all move on to other projects, get hit by a bus, lose interest, or simply do not have the time to be fixing every bug in code we wrote. When we set code free with the GPL, it becomes alive and has its own destiny. It is our responsibility as developers to write code that other people can maintain.

    Comment by Tad Marko — 14 May 2008 @ 16:01

  130. The OpenSSL website says, at http://www.openssl.org/support/, “openssl-dev … Discussions on development of the OpenSSL library. Not for application development questions!” so if you think that openssl-dev is not the place to ask about changes to the OpenSSL library then I suggest you get the website changed.
    Having asked in the advertised place for advice and received several assents and no dissents, I consider it entirely reasonable for Debian to put the patch in their build. Their failure to submit it upstream at all is a mistake. Their putting the patch into their own build is also reasonable since many software maintainers ignore patches or are very slow to include them in a new release.
    So the extent of their unreasonable mistake is to fail to submit the patch upstream.
    Let the truly innocent in this throw the first stone.

    Comment by Nicolai Plum — 14 May 2008 @ 16:08

  131. My dear, poor Ben, you sure have some apes reading your blog. It’s amazing how many people haven’t a fscking clue what they’re talking about, but are perfectly happy to hold forth with commentaries about what happened, whose fault it is, etc. Lamentable. For those saying that openssl is broken because it relies on uninitialised data, you are similarly in a fantasy land of your own construction. openssl does not rely on uninitialised data, the uninitialised data in question could be utterly predictable and openssl is no worse off, any entropy there is simply a bonus. And for the ignoramus who thinks that using predictable/uninitialised data will harm the PRNG, do yourself a favour – get a book on PRNGs and get a life within which to read it.

    As for Stefan our onanistic barbarian, perhaps he would care to substantiate why the “openssl team is not without guilt here”? Ie. explaining himself rather than tosspotting accusations around blindly. If distributions stopped patching low-level application source code like this, there wouldn’t be a problem (or at least, we could be held responsible for any such problems that occur). It’s not our job to pick up innocuous FAQ questions posted to the development lists (where such questions show up with monotonous regularity) and trace those back to determine whether it’s (a) another damned intern who just stumbled across valgrind for the first time and immediately wants to save the world, or (b) a debian package maintainer about to inadvertently cripple the PRNG on thousands of mission-critical servers. With the number of people nitpicking about mail list definitions and trying to find where the communication failed, I can only assume that there’s a few fanboys trying to evade the conclusion that this was a debian cockup. FWIW, I use ubuntu and have no particular issue with debian or ubuntu here. The more general issue is that distros spend way too much time constructing layers of patching and “value add” between applications and users, and too little time ironing out issues and managing risk. This was a cavalier patch-and-move-on on the part of the package maintainer and he got badly burnt by it, *really* badly burnt. He has my sympathies. Next time, I’d suggest that he (or any other package maintainer) send an email to an openssl mail list that gives us a vague hope of saving him from such a disaster. Ie. “here’s a patch we’re applying to openssl in because we think it fixes a bug, please fix the up-stream accordingly or advise what the alternatives are”. That isn’t what happened, and anyone pretending that the openssl team is somehow responsible/negligent to that effect is just recklessly throwing stones.

    Comment by Geoff — 14 May 2008 @ 16:57

  132. [...] http://www.links.org/?p=327 [...]

    Pingback by Å ta se desi kada se otkrije propust u RNG-u? - Blind Injection — 14 May 2008 @ 17:00

  133. Nicolai Plum: The problem with the Mailing list is, that there was never said that the two lines are to be removed from official code.

    What happened is that some individual asked if it is okay to remove to lines of code that add unitialised data to get rid of valgrind warnings.

    I think the devs reading the post assumed that someone has a problem with debugging and needs to get rid of warnings temporarily. So they did not verify that the second mentioned line really accesses unitialized data.
    So the “If it helps with debugging, I’m in favor of removing them.” happened.

    Tad Marko: I dont think that a comment on the harmless line would have stopped an over-motivated debian maintainer to comment out the critical one.
    You cannot comment every security-critical line in security-critcal code with “// Important. Do not remove” because most of the lines in security-critical code are security-critical. *g*

    Comment by Euphi — 14 May 2008 @ 17:12

  134. Just a meta-comment, obviously this site has been somewhat swamped (slashdotted and more) and as a result, comments have not been appearing immediately. I thought an earlier comment of mine had been removed, and someone else above had the same experience, only to have it re-appear later. That is why we see so many redundant comments about the purpose of the mailing list, etc. It’s not that those people were “piling on”, rather at the time they posted, the earlier comments making the same point were not visible.

    I hope it has become clear now that the problem was that there was one call which added randomness from a *potentially* uninitialized buffer, which is not necessary but does not hurt (it is not credible to suppose that real-world C compilers would turn that into rm -r /); and another which added real randomness from a seed buffer; but that both calls accidentally were removed when only one of them was safe to remove. OpenSSL did not depend on the randomness from the potentially uninitialized buffer, that was just taking advantage of some data that might happen to have some randomness. It was the other routine, which added real randomness from a real seed buffer, which OpenSSL depends on, and which got mistakenly commented out. In fact the fix restores that second randomness addition but leaves the first one, from the potentially uninitialized buffer, disabled.

    As far as who is at fault, I’d say there is enough blame to go around. One thing you find when you look at these kinds of failures is that there were usually many opportunities to catch the problem, and all of them were missed. In retrospect it’s frustrating, but in reality it is inevitable. Of course there have been any number of bad patches proposed over the years which have been caught before they got into the source. The filter usually works. But no system is perfect, hence when we see a failure, by definition it is one where the mechanisms didn’t work for whatever reason. In retrospect, failure always looks like a curious combination of circumstances where it is hard to see why people didn’t notice what was happening. That’s not because people are careless, it’s because any failure by definition occurred when control mechanisms didn’t work.

    Comment by Hal — 14 May 2008 @ 17:35

  135. Wow, the Debian defenders are out in force…

    The mail message sent to openssl-dev doesn’t contain a patch. It contains a fairly vague description. If you *include the patch*, it can be checked directly! This wasn’t, so the patch wasn’t.

    Epic fail on the part of the Debian guys, and I’m glad I switched to OpenSUSE.

    Comment by Ed Ropple — 14 May 2008 @ 18:02

  136. Lets call users of Debian ‘customers’. What should they do when packages which their ‘customers’ use all the time wind up broken and the project maintainers are either unresponsive or have vanished? Drop it from the repo entirely?

    There are numerous projects which spring to mind, like l2tpd which are pretty much completely dead upstream. Debian, much less anyone else trying to get on with it, can’t afford to just throw these away because the grand master of the project can’t approve their patches. That is a ridiculous notion.

    Comment by Colin Alston — 14 May 2008 @ 18:14

  137. [...] paar bissige Kommentare von einem OpenSSL Entwickler [...]

    Pingback by Debian SSL Super-GAU at blog.gauner.org — 14 May 2008 @ 19:11

  138. Completely agreed — downstream should not try and “fix” critical packages like OpenSSL. Instead bug fixes and patches _must_ be sent upstream so that it benefits all users.

    Comment by Christophe Devine — 14 May 2008 @ 20:50

  139. This is not exactly a diplomatic approach to problem reporting or solving, and doesn’t do much to help anyone on either side. To be fair, vendors are the ONLY reason why linux has become as popular as it is – without them, it would not happen plain and simple. Everyone makes mistakes, I’m sure the openssl folks have made some themselves. Just stick to reporting the issues, there’s no need to throw stones. We all live in glass houses.

    Comment by Sean — 14 May 2008 @ 21:41

  140. [...] Debian/OpenSSL bug by people closer to it than I am.   An expert view comes from Ben Laurie, who lays in to the Debian packagers for fixing an apparent bug locally, and not sharing it with upstream.  Ben’s points are [...]

    Pingback by Lack of Entropy « niq’s soapbox — 14 May 2008 @ 21:45

  141. [...] Sachen gelesen von Ben Laurie: Erstens hat er, ein super Artickel geschrieben mit dem Title “Vendors are bad for Security” wo er seine Meinung über sogesagt “Liefernaten”, die für Debian geearbeitet [...]

    Pingback by Razvan Munteanu`s Blog » Blog Archive » Kryptographische Schlüssel in Debian — 14 May 2008 @ 22:42

  142. Anonymous wrote:

    > It’s not strange that a vendor gets notified several days before the advisory is made public.
    > In this case Debian patched its system, and the other 5 days, it was probably waiting for some > other vendors (Ubuntu, Knoppix… who knows?) to catch up. Then a common advisory can be sent.

    And yet unstable had the fix well before it got to stable even though stable is the recommended distribution for those wanting a secure system.

    It is also strange that even though this vulnerability was not in the public domain sid was updated before etch. Paragraph 5.8.5.1 at

    http://www.debian.org/doc/developers-reference/ch-pkgs.en.html

    gives this advice:

    > Please note that if secrecy is needed you may not upload a fix to unstable (or anywhere else,
    > such as a public CVS repository). It is not sufficient to obfuscate the details of the change, > as the code itself is public, and can (and will) be examined by the general public.

    Comment by Brian — 15 May 2008 @ 0:45

  143. @ben
    Im speechless … sending in the patch before the advisory …

    http://stats.denyhosts.net/stats.html

    just a coincidence? I did wonder a little whats going on at the weekend with our systems and who got crazy about knocking on the ssh ports but i guess …
    PS: if you check the changelog dates of the debian packages you can also see that they was ready since 08.05 – but in the end i agree that this is SO big they needed some time to prepare …

    Comment by Daniel — 15 May 2008 @ 8:27

  144. I’m sorry, maybe I’m a bit stupid, but I still don’t understand. :) There was a reply to someone’s question saying that the randomness in OpenSSL is not dependent on a single uninitialized buffer, but yet somehow without it, the available keys number 32K. How exactly does that compute? :P

    Comment by No Name — 15 May 2008 @ 11:15

  145. @#144: There are two changes in the debian “fix”. One removing the seeding with unintialized data (and that one is ok), and one removing the “regular” seeding of the pool. That one is not okay.

    And I can’t believe how anyone can say the OpenSSL team is at fault here. They should’ve had the correct email address, sure, but no patch was supplied for review, and the description of the fix he wanted was vague. No one can be blamed for thinking the Debian maintainer wanted to comment out the #ifndef PURIFY bit, not both.

    Comment by Henrik — 15 May 2008 @ 12:20

  146. Well, Debian “developers” are no real developers but _packagers_. A good packager makes no good developer (and vice versa).

    I agree distributors should not do security changes based in copy/paste or without understanding the bugs being fixed.

    They should discuss their security changes with upstream.

    Comment by Sakuraba — 15 May 2008 @ 13:40

  147. [...] favorite is the long thread of hatement about this. Having done similarly (but not as spectacularly) stupid things, I’m going with [...]

    Pingback by nerdgod.com » Blog Archive » Behind the times — 15 May 2008 @ 15:01

  148. @Falk (111) “Using uninitialized memory is, in fact, *NOT* OK. It invokes undefined behavior in C.”

    No, it doesn’t as long as you access it through a character type. For example, the following function is clearly nonsense (and may very well be optimized to nothing) but it won’t cause undefined behaviour.

    void nonsense(void) {
    char a[100], b[100];
    memcpy(a, b, sizeof a);
    }

    Read 6.2.6.1 of the C99 standard.

    Comment by Chris — 15 May 2008 @ 17:29

  149. To all along those lines:

    “And according to http://openssl.org/about/, Ulf Möller is in the “OpenSSL development team”. Seems he didn’t laugh a lot, and didn’t tell it was a terrible idea a lot neither. Seems that Ulf Möller was pretty positive with the suggested patch, rather.”

    1. as already stated – distro patches are stupid in most cases
    2. it is a fact that it was a stupid patch so just admit the possibility that the people that “told you so” WERE right and learn something from it instead of nitpicking like spoiled kids.
    3. i would figure that when ulf moeller said “if it helps with debugging” then he was referring to “remove this for and during debugging”. i think anyone experienced with bugfixing / testing would interpret it the same way. I dont think it’s fair to assume he was “pretty positive with the suggested patch” beyond debugging. I am pretty positive he, knowing the function of the code, did simply NOT assume someone would do something as stupid.
    4. if you think that he should have done so, you should REALLY learn how to take responsibility, people dont have to be on the lookout for every stupid action people take.
    5. the lacking-comment thing is definitely an issue and should be fixed… but still, “if i dont get it i dont fix it”. as a sysadmin i know this very, very well. I have to touch other peoples scripts almost daily and i’ll do changes while debugging, but when i found the error i’ll put the original back into place and not just put my fix to someone elses code into production. and if my limited coding experience made me figure that out, i can think of little reason why some devs shouldn’t have to come up with that too.
    6. otoh, i dont think this has to be much of an issue (yes i have to fix a lot of systems now); errors happen, but what really matters is how they’re dealt with. and so far i just see a lot of people trying to blame it back on openssl instead of just saying “oh damn, bad mistake, hope they cooperate more in the future”

    Comment by darkfader — 15 May 2008 @ 22:38

  150. Comment to #99 Comment by Geoff, which is completely off the ground:

    But if you had any familiarity with this matter, you
    would know that we get this “patch” all the time on
    the openssl lists, it’s a FAQ!! Someone shows up on
    the openssl list for the first time after having
    just run valgrind/purify against some app that links
    to openssl and, without consulting archives, FAQs, or
    digging in any way, points out that openssl “may have
    a bug because it’s using uninitialised data – look
    here, valgrind told me so!! “. This happens often,
    hence my response at the time;

    Well — in 2006 there were no time machine to go in the future to see your FAQ from 2008… but now we can go back in time to see that THERE WERE NO SUCK FAQ even until Aug 2007 (time machine broken then), so please be considerate in your comments. Indeed maybe issue was hit in mailing lists few times, but o well — if it didn’t make it into FAQ, I guess it wasn’t that frequent ;-)

    http://web.archive.org/web/*/http://www.openssl.org/support/faq.html

    Comment by Yaroslav Halchenko — 16 May 2008 @ 6:18

  151. [...] je točno tu kriv teško je reći (slično kao i kod Debian vs. OpenSSL fijaska neki dan), ali kombinacija svega ovoga je neupotrebljivost [...]

    Pingback by Free Software Stuff » Blog Archive » Workaround za Firefox3b5 probleme u Linuxu — 16 May 2008 @ 7:18

  152. I’m not against downstream dev’s writing their own patches. It’s my personal preference that they refrain from doing so as MUCH as possible, but there exist cases where it is justifiable that the dev patch something personally.

    This situation, however, was not one of the acceptable situations. Consider how the events, unfolded:

    1) Debian dev finds code he doesn’t understand. His code-checker complains, he thinks the code might not be necessary. It is not a bug, but simply code that, if indeed unnecessary, makes the program less “clean”.
    2) Debian dev devises a patch. But he does not know whether the patch is safe or not.
    3) Debian dev posts proposed patch to the openssl-dev mailing list, stating that he does not know whether or not the patch is safe to use.
    4) The OpenSSL devs fail to review the Debian dev’s proposed patch.
    5) The Debian dev,goes ahead and implements the patch anyway, despite the fact that he does not know whether or not it is a secure patch and despite the fact that the patch doesn’t , technically, actually even fix any problems.
    6) It turned out that the patch was not secure, and thus Debian and Debian-derived distributions (which compose a healthy percentage of total Linux users) were left vulnerable.

    The Debian dev had nothing (real) to gain and everything to lose. Bad move on his part. When making a mistake has consequences *that* dire, you don’t mess around making things look pretty. Especially not in crypto libraries. Period.

    Comment by B-Con — 16 May 2008 @ 8:47

  153. [...] תגובת המפתחים [...]

    Pingback by סטארטר · את מי מעניין הבאג בOpenSSL בדביאן? — 16 May 2008 @ 9:15

  154. “Not only do they have local patches in their ports system that should clearly be sent upstream, but they also install packages without running the self-tests.”

    1) I like to point out that FreeBSD patches have been send upstream, and some are still unresolved.
    e.G. Mi. 01. Okt. 2003, http://rt.openssl.org/Ticket/Display.html?id=706

    2) Also the regression-test are availible by typing “make test” in the portsdir.

    3) The package cluster build packages and runs the self-tests:
    Please check e.G.: http://pointyhat.freebsd.org/errorlogs/amd64-7-latest-logs/openssl-0.9.8g.log

    Comment by Dinoex — 16 May 2008 @ 11:30

  155. Initially while reading the bug report, I thought that it was completely Debian developers fault, but I am not convinced of that anymore. Certainly, the Debian developer should have submitted the patch to upstream, but, at least, he discussed the proposed change with the OpenSSL Team (or, more precisely, with some members of that team who is reading the openssl-dev list) and they did not fall laughing about it as Ben Laurie suggested.

    What is _really_ upsetting about the current situation is the following Ben’s statement: “Openssl-dev is a list for people developing OpenSSL based software, not a list for discussing the development of OpenSSL itself. I don’t have the bandwidth to read it myself. If you want to communicate with the OpenSSL developers you need to use openssl-team@openssl.org.”

    If openssl-dev is a wrong place then why it is announced on the OpenSSL site as the ML for “Discussions on development of the OpenSSL library. Not for application development questions!” and also in README in the “HOW TO CONTRIBUTE TO OpenSSL” section, we can read: “Development is coordinated on the openssl-dev mailing list (see http://www.openssl.org for information on subscribing). If you would like to submit a patch, send it to openssl-dev@openssl.org with the string “[PATCH]” in the subject.” No official documentation mentions openssl-team@openssl.org.

    That makes me wonder if the OpenSSL team really wants to receive patches or only to bash Debian developers. Had the Debian developer submitted the patch, wouldn’t the patch simply ignore, because many OpenSSL developers do not bother to read the ML that they announced as the place for submitting patches?

    Comment by Dmitry — 16 May 2008 @ 14:48

  156. I’s not a good security practice to add data from uninitialized buffer to a random pool, for at least two reasons:

    1. It could contain information of a GRATER security level than the pseudo-random bytes intended to generate, leaking that information. e.g: a private key.

    2. It could contain previous pseudo-random bytes that could potentially CANCEL the mewly generated random bytes, if mixing function is a simple XOR. Even if mixing function is it’s not a XOR, it could also degradate the cryptographic primitives security, because they were not designed to be mixed with a previous hash-function state.

    I think 50% of the responsibility lies on the original openssl developers.

    Comment by Sergio Demian Lerner — 16 May 2008 @ 16:47

  157. Sergio (#156), if you’re so smart why don’t you read the code in question? The data is not added to a random pool, it is added to a cryptographic hash function. You can add as much deterministic, predictable data as you like, it won’t make the result more predictable. The uninitialized data is only a small part of (hopefully random) data fed to the hash function to generate a seed, anyways.

    Comment by Chris — 16 May 2008 @ 17:39

  158. B-Con (#152), point 3 and 4 are incorrect. The maintainer of the Debian package never provided any patch to the OpenSSL developers. He didn’t submit a bug report either. All he did was asking about some debug issue involving valgrind. This was simply completely unprofessional.

    Comment by Chris — 16 May 2008 @ 17:46

  159. Sergio (#156) – Where I can I download this GRATER security level? Will it slice and dice those attempting to crack my systems? I am also interested in the kitten-powered RNG you mention in reason #2.

    Comment by brad — 16 May 2008 @ 19:34

  160. (in all seriousness though, I think several improvements could be made by both “sides,” and all the flaming is unfortunate – the hyperbolic criticisms of both have some merit, and it’d be nice to see both teams apologize and improve! i.e. patch vetting/transparency on the debian side, and code commenting/dev team contact info on the openssl side)

    Comment by brad — 16 May 2008 @ 19:39

  161. I disagree with Chris. I’m not smarter than anyone here. This is what cryptographers (which I’m not) recommed: just do what the crypto primitives where designed to do. Don’t try do clever tricks.

    You cannot feed ANYTHING to a crypto primitive: it doesn’t matter if the data is deterministic or not. There may be special cases where feeding a crypto function with data related to the internal state of a primitive may actually DECREASE it’s security.
    To make my point clear, (theorically) the output entropy could DECREASE if you manage to cancel the existent entropy bits. Internal states are meant to be internal. Uninitialized data could contain bits of internal states from previous function calls.

    I’m not saying that in this particular case that’s what happens. But as far as I know, it’s not recommended to feed a crypto primitive with uninitialized data.

    Bye!

    Comment by Sergio Demian Lerner — 17 May 2008 @ 0:29

  162. I don’t agree with Brad.
    Let’s say you use a very important and secure 15360-bit private key for message signing which happened to be left on uninitialized memory. (realistic assumption)
    Afterwards you use that memory to feed an old and vulnerable PRNG like MD2 (a fact). Some time later a clever mathematician discover a severe weakness in the PRNG or some guy removes a critical line of code and suddenly some manages to recover the seed of a pseudo-random stream. Then you will have the private key also compromised.

    To minimize the damage in case of a bug you should not expose unnecessarily sensitive information. Uninitialized memory could contain sensitive information.

    It’s about damage control.

    Comment by Sergio Demian Lerner — 17 May 2008 @ 1:09

  163. [...] Vendors Are Bad For Security http://www.links.org/?p=327 [...]

    Pingback by Linux - Graves problemas en el algoritmo que genera los números aleatorios en debian. ( OpenSSL) | Zaragoceando.com — 17 May 2008 @ 20:17

  164. @Chris: Can you quote the exact parts of the C99 standard that show that this behavior is not undefined? I can’t find anything about that in section 6.2.6.1 of the C99 standard. However, the standard definitely says that “If an object that has automatic storage duration is not initialized explicitly, its value is indeterminate.” (6.7.8,#10 in N869, the latest public draft of the C99 standard, I don’t have the final version here right now, but would be surprised if it said anything else). Use of an object with indeterminate value results in undefined behavior, 3.18.

    Even with trap representations, from the section that you quoted, undefined behavior results:
    “Thus, an automatic variable can be initialized to a trap |
    representation without causing undefined behavior, but
    the value of the variable cannot be used until a proper
    value is stored in it.”

    Comment by hb — 19 May 2008 @ 9:34

  165. [...] Ben Laurie explains more about the problems: I’ve ranted about this at length before, I’m sure. But now Debian have proved me right (again) beyond my wildest expectations. Two years ago, they “fixed” a “problem” in OpenSSL. The result of this is that for the last two years anyone doing pretty much any crypto on Debian (and hence Ubuntu) has been using easily guessable keys. This includes SSH keys, SSL keys and OpenVPN keys. [...]

    Pingback by simpleigh.com - blog » Open Source Software - a Chink in the Armour — 19 May 2008 @ 17:09

  166. lol normally i love a good rant, but when its justified. laurie remove your head from your buttox – the proposed change was posted to openssl-dev for comment. [ref: @SWP above]

    if anyone is going to use archaic code and rely on admitted atypical use, quoting “usually it is bad to have any kind of dependency on uninitialised [sic] memory”, then the author must document it. plain and simple. ref: [@Steve Friedl above]

    as referenced by the openssl-dev link, even the supposed authority didn’t have a clue. if they don’t know how the code works, how is anyone else supposed to?

    this is software dev 101: document your shit.

    Comment by just me — 19 May 2008 @ 19:05

  167. [...] not the first commenter to say this. I just feel really strongly about writing maintainable [...]

    Pingback by This Blog Needs No Name | May | 2008 — 19 May 2008 @ 22:06

  168. hb (#164), I replied here: http://www.links.org/?p=328 (In a nutshell, it seems I was wrong and this particular use of uninitialized memory causes indeed undefined behaviour.)

    To those who say “you must document all non-obvious code”, could you maybe show us some non-trivial real-life code that you consider properly documented? I actually think that documentation can be bad. Namely, I think it should be limited to documenting interfaces (of functions and modules) – most of the time, as a rule of thumb. Comments can quickly get out of sync and often they are worded badly, so that they are misleading or harder to understand than the code. It can also happen that people rather read the comments (which are wrong) than the code and therefore make incorrect assumptions. If some code is hard to understand, I think it’s better to force an reader to read the whole context, than adding some comments which are unmaintained and never checked for correctness neither by humans or a compiler.

    Also specialized code very often requires a lot of expert knowledge on a topic which you simply cannot explain in a couple of comments. Even if this doesn’t apply to this particular example of using uninitialized memory, I find it horribly naive to assume that you only have to add comments and everything will be fine and easy to understand.

    Comment by Chris — 20 May 2008 @ 13:08

  169. [...] happened; for those who are interested and are of a technical bent, some good articles are here and here (and here, [...]

    Pingback by No Comment « James Viscosi’s Scribblings — 22 May 2008 @ 4:13

  170. I want to know that there aren’t other morons like this with the ability to commit to crypto packages in my distro.

    This is really, really sad. This guy should never touch software again

    Comment by circuit_breaker — 22 May 2008 @ 6:31

  171. [...] the OpenSSL programmers themselves, but of the Debian team, known for their security expertise. OpenSSL developer Ben Laurie raged, "Never fix a bug you don’t understand! Had Debian [sent the bug to us] in this case, we (the [...]

    Pingback by Debian team opens linux to hackers - HardwareLogic — 23 May 2008 @ 20:53

  172. Install Windows, problem solved!

    Comment by Jerry — 24 May 2008 @ 7:19

  173. [...] OpenSSL developer Ben Laurie raged, “Never fix a bug you don’t understand!  Had Debian [sent the bug to us] in this case, we (the OpenSSL Team) would have fallen about laughing, and once we had got our breath back, told them what a terrible idea this was. But no, it seems that every vendor wants to ‘add value’ by getting in between the user of the software and its author.” [...]

    Pingback by Huge Hole in Open Source Software Found, Leaves Millions Vulnerable — 25 May 2008 @ 4:41

  174. “Guilt” might be on both sides, but I definitely only see unfounded arrogance – and lies, sorry – from the side represented by this blog’s author.

    LOOK at what was said on the mailing list:

    “But I have no idea what effect this
    really has on the RNG. The only effect I see is that the pool
    might receive less entropy. But on the other hand, I’m not even
    sure how much entropy some unitialised data has.
    What do you people think about removing those 2 lines of code?”

    So, he pretty clearly ASKED if there could be any horrible side effects the change might provoke. That’s precisely what he was asking.

    In subsequent posting, he made it CRYSTAL CLEAR that he intended to remove BOTH lines, because when Marco Roeland said:

    “So yes I think not using the uninitialized memory (it’s only a single
    line, the other occurrence is already commented out) helps valgrind.”

    He re-stated (just as he had on the very first posting, very clearly even there) that both lines came into the equation:

    “Afaik, both are actually being used, the one without the #ifndef
    PURIFY just doesn’t seem to be used that much.”

    So… an unfortunate misunderstanding – maybe. But you, on this blog, are coming across as the one to blame for that arrogance that helps cause such misunderstandings!
    As has already been said countless times now, 1) that mailing list was definitely the right place to publish the stuff, 2) it’s the vendors’ job to look at problem and fix them, 3) they did notify upstream and ask for their opinion on the ML, 4) team members did reply, 5) the question was stated pretty clearly, 6) it was not a FAQ at the time as archive.org testifies.

    And I’ll add that it’s, clearly, utterly preposterous to say that the respondents to the ML inquiry assumed that the lines would only be removed for debugging and then restored. Ridiculous. If someone says “But I have no idea what effect this really has on the RNG”, it obviously means they CARE about any adverse effects on the RNG! Why would you care, if you only wanted to debug?

    Right: he didn’t mark his posting as a patch, and he didn’t state he was a Debian mantainer.
    So, thinking he wasn’t a Debian maintainer, but rather “it’s only this poor fool, at worst he’ll break the security of his own machine” (which has been a line of defense from some OpenSSL people on this blog, I seem to see) is, uhm, a good defense?! I’ll let it speak for itself.

    Comment by LjL — 25 May 2008 @ 15:26

  175. [...] are a lot of sites around the web that inform us about the “OpenSSL debacle” in the Debian based Linux systems. A piece of code that was committed “accidentally” [...]

    Pingback by tOMPSON’s blog » Blog Archive » OpenSSL Debacle — 29 May 2008 @ 8:25

  176. [...] #14: Commenting – sometimes it’s crucial Recently, some controversy (see, for example, here) erupted around a mistake made in the OpenSSL library used by the Debian project. The mistake was [...]

    Pingback by Jotting #14: Commenting - sometimes it’s crucial « Jottings on Software — 1 Jun 2008 @ 23:31

  177. [...] flagged by static analysis software. (For other takes on the problem: my colleague Ben Laurie has taken the Debian maintainers to task and added some clarifications about the response, XKCD has neatly summed up the issue with a comic [...]

    Pingback by Debian/OpenSSL vulnerability: subtle and fatal (1/2) « Random Oracle — 7 Jun 2008 @ 20:39

  178. [...] the 7th of May, 2008, Debian fixed the now famous OpenSSL Weak PRNG bug. So, I’m pretty stunned to read, over 9 months later, Verisign’s newsletter saying [...]

    Pingback by Links » Verisign Demonstrate Their Lack of Integrity — 14 Jan 2009 @ 14:29

RSS feed for comments on this post.

Sorry, the comment form is closed at this time.

Powered by WordPress