Willem Pinckaers on Akamai's flawed OpenSSL allocator patch | Hacker News Reader

Willem Pinckaers on Akamai's flawed OpenSSL allocator patch

lekkertech.net

248 pointstptacek73 comments

If this code was running on the Akamai networks for years, why did they have a copy laying around that didn't call mprotect?

Given the above problems I wonder how Akamai manages to run this in production.

How could you misread a two paragraph email so badly. Here, let me shorten it up for you:

  This patch is a _variant_ of what we've been using to help 
  protect customer keys for a decade.

  This should really be considered more of a proof of 
  concept than something that you want to put directly into 
  production. Let me restate that: do not just take this 
  patch and put it into production without careful review.

So to answer your question: this version was never running in production.. and it the initial patch didn't call mprotect because they stripped it out from their version when creating the POC.

This would be a more compelling rebuttal to Willem's post if Akamai hadn't confirmed his central thesis:

https://blogs.akamai.com/2014/04/heartbleed-update-v3.html

> As a result, we have begin the process of rotating all customer SSL keys/certificates.

AGHHHHHHH! Even if their code was likely to have worked perfectly, this is a huge mistake. And I mean huge. They should've operated under the assumption that their defense didn't work and immediately rotated all keys. Period.

They used their patch for some length of time, release it to the community and a few hours later a bug is spotted and fixed. Peer review is a good thing.

I would be more sympathetic if they'd said "do not take this patch and put it into production, it's almost certainly incomplete and buggy". "without careful review" suggests that they actually expected some possibility of it being complete and correct, at which point having somebody verify that currently it is neither is still really rather useful.

It's more an 'exemplar of concept' or something, really - which is still valuable, but I do wish they'd made that more clear.

A review by a security engineer would have prevented a false sense of security

Everyone involved in this, from the people who wrote the heartbeat code, the people who committed the code, the people at Akami who wrote this patch, this poster ... all would describe themselves as security engineers. Is everyone but Willem Pinckars incompetent?

I think the one lesson to learn is to treat crypto just like you do cloud providers. Make sure you have an exit strategy from day one; the ability to push a button to roll keys, switch algorithms, switch cert providers, etc.

There's been a lot of overly-confident statements about Heartbleed not being a big deal from apparently competent people these last few days. I'm not sure what it is about it that causes security professionals to spout off without testing their assumptions, but...

When in doubt, always asssume a compromise of anything is a compromise of everything. Golden rule.

I don't think I've worked anywhere where there was a simple way to switch cloud providers with the push of a button. Even migrating to different locations within a cloud provider isn't trivial. You'd have to invest a lot of engineering time from the beginning to get this kind of capability, and you may never realize any value for all the pain.

Yep; not easy at all ... if it were easy everyone would do it :) I think with tools like puppet and orchestration tools like heat, etc it is getting closer to being easier.

I did got to a Netflix talk once where they said that while moving from AWS would huge, they had a plan and I think from memory they said it would take about a week.

in my team we had the scripts ready for a second provider (but not the hot backups, I couldn't get that far).

Which of the assertions in the post do you disagree with?

None, it's good work.

But the whole episode, and this post in particular, highlights the issue that enough review is never enough. When is the the code secure? When it is written by someone with a good track record? When it gets reviewed and committed by knowledgeable parties? When it's been running for several years without incident? When it passes coverity? When Willem has time to review it?

When I just think about the embedded ssh keys I've got in my few toy systems; if I discovered ssh was broken (and it's happened; look at the Debian bug) I'd be sinking a lot of time figuring out just how to change the keys everywhere. I should have designed for this from the start; lesson learnt. I bet there's lots of admins out there who wish they had better ways to update certs on a moments notice over the past week...

I think it's reasonable to disagree with the part he quoted. "A review by a security engineer ..." Well, it was reviewed by security engineers, and the false sense of security remained. The rest seems fine though.

My experience in large companies is more like this: A (security) engineer reviews, objects and management says: "Go ahead anyway you don't have the whole picture".

Apparently, he is not competent enough to understand what "this is not our actual code but merely a POC" means.

It's not exactly a proof of concept if it doesn't protect the keys, is it?

It strongly hints that even if this is 'only a POC' that their actual implementation is still vulnerable since their POC failed to protect against the very attack is was written to protect against.

Yes. Our actual implementation was vulnerable. We disabled TLS heartbeat before 5 April 2014, so are not still vulnerable.

This would have been a cutting bit of wit indeed, had Akamai not stepped on your moment by confirming Willem's central thesis. Ouch.

We're still evaluating some of his arguments. I still believe some of them are true in the general case, but do not apply to our specific embedding of this code. I say that aware that I was mistaken 12 hours ago, and so very well could be mistaken now.

But I am reasonably convinced that the CRT values are loaded into the normal heap, where they're available to a normal Heartbleed attack. Pinckaers doesn't have to be right about all his points to be right---just once---and I'm pretty sure he's right at least that once.

If the tone of my message was a bit harsh, it was mostly to reflect his, and thankfully I do not have a life boring enough that this is "my moment".

Still, I stand by my point of view that at the time of writing, akamai's POC was presented neither as an absolute final fix nor as their own production version of it, and judging it as such was misplaced.

That akamai realized the flaws he noted in their patch also applies to the real world code doesn't change that.

I never said the flaws he pointed were not real flaws nor unimportant ones, I merely disliked the ridiculous and unjustified tone he used to to destroy a proposal and used the same against him.

Here's a patch that prevents any exploit ever occurring in OpenSSL:

    void *custom_malloc() {  };

It's only a POC though so you'll have to adapt it. You can go ahead and begin praising me and flaming my critics.

The funny thing is the fact that everybody is ignoring that the patch wasn't a patch. It was a POC (read demonstration) with the notes:

'...This patch is a variant of what we've been using to help protect customer keys for a decade.

This should really be considered more of a proof of concept than something that you want to put directly into production. It slides into the ASN1 code rather than adding a new API (OPENSSL_secure_allocate et al), the overall code isn't portable, and so on. If there is community interest, we would be happy to help work on addressing those issues. Let me restate that: do not just take this patch and put it into production without careful review.'

Oh but if you do that how are you going to make a pretentious critic to shoot it down ? The akamai guy straight up said "that's not actually our code, and that should not and cannot be used as-is, this is a POC", rendering this entire "answer" irrelevant especially given its mocking tone.

Isn't a POC supposed to prove a concept? i.e. shouldn't a POC defend against the attack it is stated to defend against?

Willem, Thanks for the well-reasoned set of claims for us to evaluate. We are still looking at them, but a critical one is accurate: our own implementation of the secure memory area did not include the CRT values.

More here: https://blogs.akamai.com/2014/04/heartbleed-update-v3.html

It comes down to intent. There are two distinct ways to evaluate Akamai's patch:

1. Did Akamai release the PoC patch to start a discussion about how to protect private keys and share their work as a starting point for changing the code? If so, their efforts here should be considered in that vein and any criticism should be used simply to guide the development of a usable and functional patch.

2. On the other hand, if they are supplying this code as assurance that customer keys were properly protected against exposure by the Heartbleed bug and that certificate replacement is not needed, the patch and the subsequent criticism should give Akamai customers pause.

edit: Akamai acknowledges the bug, and has started rotating all customer SSL keys/certificates, per https://blogs.akamai.com/2014/04/heartbleed-update-v3.html

Even if their intent was just to start a discussion, they are not being helpful by "submitting" such bogus code. It would be easier for an expert to rewrite this patch from scratch than try to figure out all of their flaws and unfinished sections.

In one of the email responses, someone pointed out a problem with their code and they responded, "Oops we posted the wrong version" ( http://article.gmane.org/gmane.comp.encryption.openssl.user/... ). They aren't exactly being respectful of other people's time with stuff like that.

In secure coding the phrase "the devil is in the details" applies very strongly. So if you don't have the details figured out, you don't have much of anything.

> Even if their intent was just to start a discussion, they are not being helpful by "submitting" such bogus code.

All code is bogus code until reviewed. That is absolutely central to understand. Linus's Law only works _if_ people are looking at the code. Implicitly thumbs-upping it doesn't solve problems.

Akamai submitted the code, people reviewed it and found flaws. They're taking action to fix their own code, and the community is coming up with various fixes of their own. That's how Open Source Software Development should work.

While I agree with you that it'd probably be better to rewrite the code with a similar approach, it's also important to note that nobody in the OpenSSL community even considered this approach publicly until Akamai published their code.

Any claims they're being disrespectful of people's time is specious - they said from the beginning that this code needed review and shouldn't be merged. This is just one of those issues that comes out in the wash of code review.

TL;DR: Akamai should be lauded on their intentions but like noted by everyone, the code wasn't good enough. Now, with proper review and rewrites, they will be able to protect their customers into the future, and maybe OpenSSL will become a slightly better product for it too.

It was released as both: we thought that our old protection against swap protected us against Heartbleed. What a stroke of luck. We did check for key values visible in the heap, and on our implementation, in our lab, didn't find them on any version of our software used since we took OpenSSL 1.0.1.

We weren't looking for the CRT values.

As one part of our response, we decided to publish the code we thought was keeping us safe. If we were right, sure, there's good PR from that. If we were wrong, it's a chance to find out and get right. Less wrong, anyway.

Decisions were made about certificate revocation based on assumptions about this code and Akamai customers ended up being exposed.

Perhaps third-party security validation of such a critical piece of code should be a prerequisite before asserting that no further countermeasures such as key rotation were necessary. Such an action would demonstrate significant diligence as compared to a public release days after you've told customers there was nothing to worry about.

Additionally, that public release didn't directly encourage security review, the deprecating comments on the post were primarily around portability and design.

It's a smokescreen. If you were running a version of OpenSSL that supported the HEARTBEAT extension (patched or not) it's easy enough to determine whether or not it was vulnerable, simply by running the exploit code. Any theoretical discussion of how your patch made it invulnerable is pure speculation. They can release packet captures that prove they were invulnerable without releasing any code at all. At this stage, it's all PR.

I'm guessing the patch was broken because they didn't have an actual patch laying around, they had to do a diff against the upstream and try to pick out the relevant parts. That doesn't excuse some of the other errors though, like integer overflows and not checking return codes. That also doesn't excuse the fact that they didn't actually try the patch before sending it out.

There could be other pieces that we're missing, and this patch alone doesn't prove that you can obtain private keys from Akamai's servers with the Heartbleed bug. It just proves that there could be key parts outside of their protected storage area and they suck at creating patches of their modifications. That being said, I'd love to see someone apply Akamai's OpenSSL patch and still pull the key with Heartbleed.

Yes. It's not really fair of the OP to say "this is broken in totally obvious ways and would clearly not actually run" and yet critique it as if it was production code. It's fairly obviously (at this point) a hand-constructed pseudo-diff.

(I worked with Willem at Matasano for a couple years, and he knows what he's talking about.)

The tone of his post is rather unfortunate. Still, he raises good points, even if they are put in an unnecessarily aggressive manner.

I disagree. The post is written bluntly, but it argues with factual assertions, not with emotional appeals. I don't think criticism of its tone is warranted or really all that appropriate.

This:

> Perhaps Akamai is not actually running this version in production, but another 'super secure' allocator. In either case they should not be sending out non-functional, bug ridden patches to the OpenSSL community, while claiming they protected Akamai against the Heartbleed attack. Andy Ellis, CSO of Akamai, said on Twitter that the 'secure' allocator was written 13 years ago. I'm happy to provide the results of my 15 minute security review, since it is so overdue. (To be fair, I found the issue in minutes, but confirmation took longer.)

Comes across quite passive aggressive indeed. The tone is otherwise fine, except that paragraph which is just one giant jab at Akamai.

I guess it is true what they say: No good deed goes unpunished. I'm sure Akamai has learned their lesson and will keep all future patches private to avoid public criticism and ridicule. But on the positive side at least we all know how smart Willem is, which I'm sure was the real point anyway.

This post isn't simply "ridiculing" the proposed allocator design.

Did you work on this allocator? If not, can I suggest that you be a little careful? It's one thing to stick up for Akamai's developers; it's another to be thin-skinned on their behalf. It's possible that Akamai's devs, being adults, professionals, and engaged with software security, actually want to hear Willem Pinckaers' take on their allocator.

Speaking as a security researcher at Akamai, I can say that tptacek is 100% correct here. We're absolutely better off for having received Willem's report, and I'm pretty sure we're all mature enough to tolerate the jabs that accompanied it.

I didn't have any part in writing this allocator, but I was asked to do a code review prior to publication. I told Rich Salz that it would take me at least two days to do a thorough one, and we both made the decision that it was better that we just get this code out there for public review and discussion than that we wait until we thought it was perfect.

So, with the caveat that we're still evaluating most of Willem's technical claims (and I'm probably going to be in the office all night doing so), the only sentence in Willem's report that I really take exception to is this one:

  In either case they should not be sending out non-functional,
  bug ridden patches to the OpenSSL community, while claiming
  they protected Akamai against the Heartbleed attack.

This statement is self-refuting. If we hadn't published this patch, we wouldn't be having this discussion, and some of the bugs that Willem and others are finding would have gone unnoticed. I almost certainly would have caught the issue with the CRT intermediates if Willem hadn't done so first, but I doubt I'd have caught everything that has or will be identified through public scrutiny.

There's probably a middle ground here. If you'd taken the time to make sure the patch actually compiled, ran and attempted the things it was advertised to do before releasing it, I suspect problems with it would've been obvious sooner.

That's quite possible. It's also quite possible that we still wouldn't have released it yet, as we'd be doing that work to make it generally usable this week. Therefore, the critical failure - that we weren't protecting the CRT values - might not have been known yet.

Sadly, the multiverse doesn't yet let me monitor its A/B tests.

tptacek, we all want to hear Willem Pinckaers' take, it is really good stuff.

I also want to hear ideas from Akamai, even if they aren't perfect. Perhaps they can lead to good things.

Unfortunately Pinckaers' commentary is a little bit too hostile and calls for Akamai to cease sharing ideas[1].

I'm sure Akamai's developers are "adult" enough, as you say, to handle it. However there is a trope in software development community that if you share something, you should be fine with being open to no holds barred attacks. Wouldn't the more "adult" behavior be to criticize in a more professional tone that is open to refinement of ideas and could spark further collaboration? I'd like to see this type of communication more in the software world, I think it would encourage more participation.

[1]"they should not be sending out non-functional, bug ridden patches to the OpenSSL community"

I think there's a difference between sharing ideas and sharing code.

An idea or concept on its own can't really do much, at least until it's put into practice somehow. The potential for harm is quite minimal, if it even exists.

Code, on the other hand, can often be directly used with relative ease by people who may not fully understand the possible implications of using such code. The potential for harm exists, and could be significant.

In the context of security, it's important to avoid potentially-harmful code wherever possible. If somebody has concerns about some code, regardless of who wrote it, it is best to express those concerns in a very blunt and direct manner.

Security is just not something to fool around with. The hard questions and painful facts should be out in the open, especially when code is involved and capable of being used. It's just not the time or place for pussyfooting around.

Yup. Hard to read! Better that than not to read it.

As Rich Salz said in the post to openssl-dev, this is a prototype that nobody should take and use straight. We did think we were pretty lucky that our old patch to keep keys from being swapped to disk could help us against Heartbleed. A major voice in the internal decision about whether to release it or keep it secret as a "competitive advantage" was the possibility that we were wrong---in the hopes that someone would discover this and tell us if so. We were wrong. Pinckaers discovered this, and told us so.

He can mock my coffee as weak and my nose as big for all I care, in return for that necessary warning that we were mistaken.

You probably meant something subtly different than what "competitive advantage" sounds like, w/r/t hardening the public OpenSSL code. :)

> Comes across quite passive aggressive indeed.

Oh so what. Tone arguments are the least interesting arguments (he says, passive aggressively). In seriousness though, this is a blunt assessment from an expert in the field. The tone is entirely appropriate.

> No good deed goes unpunished.

Akamai strongly implied that their patch protected customers. If you make that claim then you need to be prepared to have the claim attacked. This is an issue of good will toward their customers - not to the Open Source community.

This isn't a case of "punishing a good deed", it's a case of "critically examining a provider's claim to their customers".

> Akamai strongly implied that their patch protected customers.

Actually they said exactly the opposite of that:

> This should really be considered more of a proof of concept than something that you want to put directly into production. Let me restate that: do not just take this patch and put it into production without careful review.

I think polemic was referencing this statement[1] when he talks about "their patch" (and not the released patch):-

> This patch is a variant of what we've been using to help protect customer keys for a decade.

1. From the original Akamai missive: http://article.gmane.org/gmane.comp.encryption.openssl.user/...

> Comes across quite passive aggressive indeed

Surely it's not passive aggressive when the author not only critiqued the code (an arguably "active" act of initiative), but offers to review the unpublished patch as well.

I hope Akamai has learned our lesson: upstream earlier, among others.

Watch and see.

Empathy is a core engineering value. If not exercised, people will unsurprisingly often ignore what you have to say.

It's possible to be correct and helpful without saying unkind things.

I don't find it aggressive, it seems more like Dutch directness to me. As an inhabitant of a neighbouring country, I must say it's an acquired taste ;)

How would a less-aggressive tone be better at delivering the necessary info to Akamai and its customers?

Deleted comment

The fact the code is badly written and wouldn't run isn't even the biggest problem with it though - a bigger issue is that the author didn't appear to realise there were copies of the key they'd missed that weren't protected. They very likely made the same mistake with their production systems and their analysis of those production systems.

Ha. This is probably something that we mountain view folks would enjoy working on :-).

I've updated my blog on how to recover the private key from the CRT parameters stored in the private key: http://vnhacker.blogspot.com/2014/04/idea-to-solve-cloudflar.... Trivial math, but still interesting to see how it actually works.

Pls elaborate

Tptacek has a software security company, and he is one of the unofficially designated security experts here (with cperciva). He is the #1 in the karma ranking and has even more karma that the site founder (pg)! If he says that “[Willem Pinckaers] knows what he's talking about”, for me it’s very strong reference for Willem Pinckaers.

Also, we generally don’t like oneliners here, try writing a longer comment. Definitively avoid Pls!!!! (And just in case, also try to avoid unoriginal jokes and memes.)

You are quite new here and your last 2 comments were heavily downvoted. I hope that someone upvote them until they are dark gray again. (The second comment, were you tried to explain your intentions, is not a bad comment.) [You’ve just deleted your second comment.]

Alternative redaction:

“I searched for Willem Pinckaers in Google and there is a lot of information about him. Can you recommend a few pages with his main projects to understand his background and credentials. Also, I’d love to know more about the projects in which you worked together in Matasano.”

[Note: perhaps the internal projects are under a NDA/top-secret/paranoid-client, and they can’t be explained until 2050.]

Ok, Thank you for the clarification. The irony is I probably should have been a bit more elaborate in the context of my question. I'm fairly new on Hacker News and I probably should read more comments pertaining to other posts to get a better perspective of the etiquette on here. But again, thanks for the clarification and I will commit to bringing proper thought to my comments

I'd like to see Akamai create a CloudFlare style challenge using their patch and a compromised version of OpenSSL and see if anyone can get the private keys from it.

It would be better if someone independent set it up.

We're talking about it. The patch is dependent on details of our unreleasable (GPL+OpenSSL+all sorts of other things) server.

uh if it's GPL, don't you have to release it?

Nope. As long as they don't distribute it but only run it in a server, they don't have to. This is what the AGPL is changing.

Not here: you can't do much with AGPL+OpenSSL. AGPL requires release under AGPL. OpenSSL requires advertising clause. There is no legal way to distribute software including parts you have only under APGL and parts you have only under OpenSSL license.

> There is no legal way to distribute software including parts you have only under APGL and parts you have only under OpenSSL license.

If you modified AGPL code you have to release it. If you've mixed up GPL or AGPL code such that you can't release it you've violated the license for the hard work of others. You sound confused, to phrase it charitably.

My interpretation of what he wrote is that it does not change things for them in the sense that due to conflicting licenses, they can't use AGPL'd software, and so the AGPL changes nothing.

OpenSSL has pretty good support for HSMs in the form of their ENGINE API. It seems like it would be possible to use this layer to move all key handling and crypto operations out of the process that was dealing with TLS. Process isolation seems like a much better way of getting this kind of security than weird allocator tricks.

gosh, i see this "weird allocator trick" to be exceptionally insightful as to the possible attacks that and likely vulnerabilities that may be introduced into code.

One of the biggest issues for a development team isn't how to do something, but how to do something so that someone (who has taken over the project) won't screw it up in 10 years when you and all your cohorts have left and are island hoping and coconut drink doing.

Kind of amused at how Akamai smugly keep their openssl security improvements private for years, only for them to be shown to be broken days after open sourcing.

With enough eyeballs, all bugs are indeed shallow!

Even if the PoC patch was perfect and is approved by many of the top security folks out there, would Akamai ever share their perfect custom memory allocator with the upstream community.

edit:

okay, whats' wrong with my statement? If they do have a perfect allocator, wouldn't it be nice to know exactly what they did?

I like how he pounds on one company who made a mistake on one network, while having no word for the developer who made a mistake for almost the entire internet.

Reply on news.ycombinator.com