Larry Osterman's WebLog

Confessions of an Old Fogey
Blog - Title

More proof that crypto should be left to the experts

More proof that crypto should be left to the experts

  • Comments 41

Apparently two years ago, someone ran a static analysis tool named "Valgrind" against the source code to OpenSSL in the Debian Linux distribution.  The Valgrind tool reported an issue with the OpenSSL package distributed by Debian, so the Debian team decided that they needed to fix this "security bug".

 

Unfortunately, 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 maintainers], 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."

 

And it IS a terrible idea.  It means that for the past two years, all crypto done on Debian Linux distributions (and Debian derivatives like Ubuntu) has been done with a weak random number generator.  While this might seem to be geeky and esoteric, it's not.  It means that every cryptographic key that has been generated on a Debian or Ubuntu distribution needs to be recycled (after you pick up the fix).  If you don't, any data that was encrypted with the weak RNG can be easily decrypted.

 

Bruce Schneier has long said that cryptography is too important to be left to amateurs (I'm not sure of the exact quote, so I'm using a paraphrase).  That applies to all aspects of cryptography (including random number generators) - even tiny changes to algorithms can have profound effects on the security of the algorithm.   He's right - it's just too easy to get this stuff wrong.

 

The good news is that there IS a fix for the problem, users of Debian or Ubuntu should read the advisory and take whatever actions are necessary to protect their data.

  • As has already been commented several times in these discussions, the Debian maintainer did ask the package maintainers: http://marc.info/?t=114651088900003&r=1&w=2

  • Nice bit of Schadenfreude there.

    "More proof that crypto should be left to the experts"

    With experts you mean Microsoft, right? Well, it happens to you guys too. I personally found a flaw in a Microsoft product which resulted in the same private key being generated every time, possibly due to faulty seeding. And then you secretly patched the flaw in a non-related Hotfix.

    Give me the openness of Debian/OpenSSL/etc. anytime.

  • Um, no.

    It wasn't a security bug per se.

    There were two calls that were commented out. One call was folding uninitialized memory into random state (which wouldn't hurt anything as it turns out) and one was being used to actually add to the random state.

    Commenting out the latter was the problem.

    Valgrind complained about the former. The former was "a rare case where you actually don't mind uninitialized memory."

    However since both calls were very very similar, the maintainer erroneously commented both out.

    Better summary:

    http://it.slashdot.org/comments.pl?sid=551636&cid=23392602

    So don't just blame Debian. It's more along the lines of "don't do things you normally wouldn't do without telling people why."

  • Jack, did I EVER say that Microsoft was a crypto expert?  

    We do have crypto experts at Microsoft, and they review all crypto-specific work.  The SDL also has an entire section on cryptography which defines what can and can not be used in SDL covered products.

    But the vast majority of the developers at MSFT aren't crypto experts, that's why we rely on the crypto team to review our decisions that are crypto-specific.

    I don't know which team you reported this to, but honestly I'm embarassed they shipped a security bug fix as a hotfix, that's simply unacceptable in my opinion (it goes to the core of "trustworthy computing").

    AC: Apparently (as Brian mentioned) they only talked about the first of the two changes with the OpenSSL team.  The change they committed went beyond what they dscussed.  

    Brian: IMHO, any bug that affects the strength of a crypto cypher is a security bug.  

    And I'm not "blaming Debian".  I'd be more than happy to blame stupid teams at Microsoft who did something similar.  And I made certain to include a pointer to the Debian security advisory, which very nicely spells out the risks associated with the vuln and the mitigations.  

    Stuff happens, that's why it's important to have checks on developers making random modifications to code, especially on developers making modifications to code they don't own.

  • "Apparently (as Brian mentioned) they only talked about the first of the two changes with the OpenSSL team.  The change they committed went beyond what they discussed."

    I thought the problem was that the first change was not a big deal, but the second one is the one that had been adding the entropy to the buffer.  The problem was they were both the same line of code, so the change to the first mistakenly was applied also to the second: the first iteration of that line read from uninitialized data, and the second iteration of the line was after the entropy code, and when the second one was commented out...oops.

  • Slight correction: valgrind is a *dynamic* analysis tool.

  • Yeah, have to say that Debian, or at the very least, this package maintainer, gets a big demerit on security for this one.  It was definitely a fairly bad thing to see the ssh hostkeys for *all* of my Debian boxes showing up in their known-bad-keys database...

    Had a bit of fun today regenerating all my host keys, and changing all passwords and all other sensitive information that had been sent over SSH, ever, on my Debian boxes.  Not a cool thing to wake up to, certainly.

    Part of their security process seems to be a bit questionable here, too:

    - Debian shouldn't be making functional changes to a package in the first place unless they're either forking it or the upstream package is dead and the upstream provider isn't maintaining it at all, anymore, IMO.

    - Somebody who is, as you say, clearly not a crypto expert or an expert in the code base itself seems to be making changes to security-sensitive bits like that.

    Furthermore, this also kind of reeks of "we're getting a strange toolchain/debugger tool warning, so let's do whatever we can to silence the warning without really understanding what's causing it", which is practically always the *worst* possible thing to do in that scenario.

    I'm also a bit wondering how much they bother to test their security fixes, too.  Just last week, Debian put out a fix for a cacti SQL injection bug (DSA-1569-1) that will completely hose a default install of cacti.  They re-released it the next day, but that's really... highly questionable, in my opinion; even the most basic of regression tests would have picked up on the fact that their security fix completely broke the entire program that it was "fixing".

    On their defense, however, checking for a broken RNG is probably not something that's likely to be typically regression-tested, and is certainly a bit harder to test in a deterministic fashion than most other problems.  Nonetheless, I've been a bit less than highly impressed with Debian lately in that respect.

  • Actually, it looks like they did ask the OpenSSL folks, or am I wrong?

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

    Also, look how poor the compromised RNG is:

    http://mag.entropy.be/blog/2008/05/13/how-badly-debianubunutu-openssl-is-fscked-up/

  • I'm also curious to hear more about this bug Jack. Since it's fixed, can you give us more specifics on the exact scenario, affected component, what you specifically saw?

    As for the openness of Debian (and Ubuntu and the rest), it clearly did not matter at all - two years of people looking at the code and countless millions (billions?) of compromised keys. And for every 1 admin that regenerates new keys (and new certificates that cover how many SSL webpages, TLS connections, etc), 2 or 3 won't even know to do it. The volume of this problem is mind boggling, as a simple hotfix doesn't do *anything* to close your vulnerability - this is very different than most security vulnerabilities. It's no different than saying you must change every single password in your organization.

    This is a good punctuation on the idiotic idea that "given enough eyeballs, all bugs are shallow".

  • Ha, ha. The limits of the 'infinite monkeys' approach of open source start to become apparent.

  • Mark, the problem is that they asked the OpenSSL folks, but the change they actually committed went beyond the change they described to the OpenSSL folks.  If they'd submitted the patch to the OpenSSL folks, they presumably would have explained the error.

  • "What I currently see as best option is to actually comment out those 2 lines of code" is from the original mailing list message, so Kurt Roeckx mentions both changes. The reply from the OpenSSL dev even quotes that bit.

    Ben Laurie's excuse that he sent it to the wrong list is laughable since his suggested list is mentioned only in a random bug report on OpenSSL's website, and the website specifically states that openssl-dev is the correct place for such issues. It seems to have been that way since at least 2005.

    Still doesn't excuse the fact that a proper patch should have been sent upstream following OpenSSL's guidelines so that the nature of the change would have been explicit and not buried at the bottom of a long message. It's also bad that the Debian-specific patch was submitted the day after the original query, which shows he didn't even try to submit it upstream.

  • Pewnie już słyszeliście o problemach z PRNG w OpenSSL, które są specyficzne dla Debiana (i dystrybucji na nim opartych). Ciekawe jest jednak jak do tego doszło... More proof that crypto should be left to the experts, Vendors Are Bad For Security

  • Addendum to a previous comment: The OpenSSL dev's response wasn't made in the context of "this change will go out to many more people than one" as Kurt's messages didn't call out the fact that he was doing this for a package to be distributed by Debian and not just for his own personal testing. It still seems like he should have probed more instead of just saying "uninitialized data doesn't add much entropy, you should be fine deleting those lines".

  • Valgrind is a /dynamic/ analysis tool. It detected that OpenSSL was using uninitialized memory as part of its entropy pool (nothing wrong with that, if it's done carefully), and that this was tainting all data produced by OpenSSL as being "uninitialized" (valgrind's way of warning about nondeterministic behaviour). Now, in this case, that's exactly what was wanted, but it made it harder to debug real problems with programs which use OpenSSL.

    No-one thought this use of uninitialized memory was a security bug. But someone messed up the patch, and removed not only the use of uninitialized data, but also another line which happened to look the same, but did something fundamentally different. That's the first mistake, which could happen to anyone.

    Then, when communicating with the openssl maintainers, the Debian folks didn't actually provide the patch they were going to use, for critique. That's the second mistake; the bug should have been caught when reviewed by the original developers -- but there are plenty of reasons why this would slip through, expert developers or not.

    Then, millions of Debian users upgraded this security-sensitive package, picking up a patch to aid a handful of people debug their own code, which had the side-effect of breaking the pRNG.  This is where the fairy-tale of open-source security software is supposed to kick in. Of the millions of people upgrading, hundreds of people are expected to review the changes and spot the mistake. But not a single person did.

    I hope the open source community learns something from this. In principle their code could get more review, and so could be more secure than corresponding closed-source code, but it requires people to actually do said review.

Page 1 of 3 (41 items) 123