
Willem Pinckaers on Akamai's flawed OpenSSL allocator patch - tptacek
http://lekkertech.net/akamai.txt
======
rgbrenner
_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.

~~~
tptacek
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](https://blogs.akamai.com/2014/04/heartbleed-update-v3.html)

~~~
daeken
> 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.

------
delsarto
_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.

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

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

~~~
brians
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.

------
ironghost
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._ '

~~~
nolok
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.

------
csoandy
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](https://blogs.akamai.com/2014/04/heartbleed-update-v3.html)

------
cscott
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](https://blogs.akamai.com/2014/04/heartbleed-update-v3.html)

~~~
paulhodge
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/...](http://article.gmane.org/gmane.comp.encryption.openssl.user/51246)
). 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.

~~~
awalton
> 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.

------
fancy_betta
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.

~~~
kalkin
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.

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

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

~~~
tptacek
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.

~~~
UnoriginalGuy
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.

~~~
tptacek
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.

~~~
dfranke
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.

~~~
makomk
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.

~~~
csoandy
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.

------
yukichan
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.

~~~
abus
It would be better if someone independent set it up.

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

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

~~~
pilif
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.

~~~
brians
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.

~~~
yukichan
> 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.

~~~
vidarh
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.

------
__alexs
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.

~~~
keithgabryelski
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.

------
comice
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!

------
yeukhon
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?

------
nraynaud
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.

