How Heartbleed Leaked Private Keys | Hacker News Reader

How Heartbleed Leaked Private Keys

blog.cloudflare.com

99 pointsjgrahamc26 comments

Every time I see a blog post from the Cloudflare team I am impressed.

It isn't just their attention to detail that is impressive. They go out of their way not just to point out issue, but suggest fixes.

Too many Heartbleed blog posts from across the blogosphere, and sadly, HN comments, set out to criticize without suggesting something better.

Cloudflare has shown again that they are actively engaging the community for the communities benefit as well as their own.

This seems to be a very effective marketing strategy (I assume it is anyway).

I would have been more impressed had their original assessment of heartbleed (on nginx) not been "We've reviewed the code and we don't think it's vulnerable."

Looking impressive with hindsight is relatively easy.

The original blog post on Heartbleed said: "Here’s the good news: after extensive testing on our software stack, we have been unable to successfully use Heartbleed on a vulnerable server to retrieve any private key data. Note that is not the same as saying it is impossible to use Heartbleed to get private keys. We do not yet feel comfortable saying that."

In that same blog post we also said that we were revoking and reissuing all our SSL keys, and started the challenge web site to see if private keys could be retrieved.

http://blog.cloudflare.com/answering-the-critical-question-c...

So, yeah, we didn't from the outset figure out how to get the private keys, but we decided that the risk was too high so we want ahead with revocation.

I don't think it's unfair to paraphrase this statement:-

"And, we have reason to believe based on the data structures used by OpenSSL and the modified version of NGINX that we use, that it may in fact be impossible."

"We've reviewed the code and we don't think it's vulnerable."

Obviously you disagree.

Context is important when you see that many weasel words. "We do not yet feel comfortable saying that." Means there still undecided as does 'may' in 'may be impossible'.

Yeap, when I read the original statement it really seemed to be telling the customers there was nothing to worry about, and I think that's how it was meant to be read.

When the original statement was posted I actually went back to the header to check the author, thinking "surely jgc wouldn't post something this marketroid?"

Only in hindsight do they point at the get-out clauses.

Argh. That's spin. The Cloudflare post you are talking about was written so convincingly that Bruce Schneier summed it up as "Cloudflare believes it is next to impossible that this vulnerability will leak keys".

You have to consider the whole post, which led off with why Cloudflare believed keys wouldn't leak, then went on with a 1000+ word description of how malloc() worked, complete with diagrams, and then concluded with a belief the keys wouldn't leak.

If, instead of running "The Cloudflare Challenge", Cloudflare had simply had you generate the memory diagrams that you built last week, it would have been a different post and less of a waste of everyone's time.

Hindsight. Nobody had the technology for the allocation diagrams then, they were encouraged to implement them only once people presented them the fetched keys.

The results of the challenge were a good learning possibility for a lot of people and we, the observers, have now also something we can point to as an example.

I dispute that it requires hindsight to believe, with some certainty, that OpenSSL was going to leak keys all over the heap; especially because (a) people were immediately pointing out that that was likely to be the case, along with potential scenarios where it could happen, and (b) people were posting to Twitter that they had working proofs of concept.

Doing any challenge to demonstrate key recovery might have been a sound plan, but the specific challenge that Cloudflare promoted was one that pessimized the discovery function of the challenge and optimized the the reinforcement of their false prediction; to wit, they tied people's hands behind their back, forcing them to attack a remote instance of a process with an unsure memory layout.

Yes, I agree that the challenge was more challenging than needed. Still now we have even better example of how wrong is to believe you solved the security problem unless there is some active attempt to disprove the claimed solution. Cloudflare were lucky to have that crowd-sourced this time. But it's still a great example of how wrong the claims "we solved the security problems" can be.

No! Do not agree! There's a different simple maxim that would have worked here just fine: assume the worst. If you're going to heavily promote something other than "assume the worst", the onus should be on you to make sure you're right!

I am still surprised there is code like

      if(b->d) OPENSSL_free(b->d);

in OpenSSL :(

some folks much smarter than I thought it best to proverbially take it out back and shoot it. if you're not familiar with it already, the left overs are called LibreSSL.

And if you want to watch the carnage, the highlights are at http://opensslrampage.org/ .

Forgive me for asking but what is wrong in particular with this single line?

To me it reads fine and the idea is that is frees that data address if its occupied. I'm sure I'm not the only one who is curious why this is bad.

My concerns are well addressed in the patch:

           if (b->d != NULL)
           {

There's no need to explicitly include NULL in there, it buys you nothing.

Insisting on braces is a style concern, though it does seem to protect people from themselves to some extent.

If you're careful to use NULL only in pointer expressions, it can be used to indicate that a pointer is being tested, as opposed to any other sort of integer or boolean flag. You can justify it on grounds similar to Hungarian notation, giving the reader some contextual information that might otherwise require a look at a declaration somewhere else.

Code should be written for the benefit of humans, not compilers.

I agree, but the humans also need to be able to understand the code. I've worked in places where the ternary operator was banned because 'some people might not understand it'. This is silly.

Yeah, that's pretty goofy. Being intimidated by the ternary operator is how you know you should have gone into marketing instead.

I don't think you should be testing for truth implicitly like that, try

if ((bool(b->d != NULL) == true) { }

much better to be explicit, don't you think?

Assuming this is satire, LOL :)

The sensitive data that was saved in that address is still there. Memory has been freed so the os can use is again but the actual data is still there is memory untill get get overwritten by something else...

The program will work with no problems, but sensitive data that has been used then freed is available for retrieval when bugs like heartbleed are found.

As the article suggests the right way is to clean the data from memory ( by overwriting it with something else) before freeing it.

I've been looking at this recently, part of the problem with that approach is that compilers will often optimise out an overwrite if they can't see anything happening afterwards.

For instance if you set a stack-resident buffer that contained a key to all zeros using memset, then simply exit the scope, most optimisations will detect it as unnecessary (wtf? this never gets read back, who cares?) and ditch the line.

Search for memset_s (part of the C11 standard) for a clear function that can survive optimisers.

Gotcha, I wasn't thinking about that.

For one, free() is a no-op on a null.

Reply on news.ycombinator.com