LWN: Comments on "Heartbleed bug exposes OpenSSL installations"

"all bugs are shallow"

Wol — Fri, 18 Apr 2014 20:28:41 +0000

The other thing about it, as I understand the saying, is that ONCE THE BUG IS FOUND it is quickly dealt with. Bugs are "shallow" in that finding the cause is usually a very swift process.

What the saying does not say (or imply) imho, is that the hidden bugs are easily found.

Cheers,
Wol

Heartbleed bug exposes OpenSSL installations

a9db0 — Sun, 13 Apr 2014 00:39:40 +0000

Another really good, simple explanation:

http://xkcd.com/1354/

Codenomicon created the logo, and found the bug

hamjudo — Fri, 11 Apr 2014 17:58:02 +0000

heartbleed.com tells us This bug was independently discovered by a team of security engineers (Riku, Antti and Matti) at Codenomicon and Neel Mehta of Google Security, who first reported it to the OpenSSL team. We should not be surprised that the people who found the bug new about it before they notified anyone else.

I am only assuming that they created the logo after they found the bug. It is possible, buy highly unlikely, that they made a cool logo, then went looking for a bug to match.

OpenBSD hardening vs. OpenSSL

marcH — Fri, 11 Apr 2014 12:49:10 +0000

Google " valgrind openssl " for some more fun.

Heartbleed bug exposes OpenSSL installations

marcH — Fri, 11 Apr 2014 12:48:01 +0000

> It seems to me the real root cause of the bug here is having two length fields for the data in the heartbeat packet.

Indeed.

Even if having two length fields in the PACKET can be explained because of some unknown and twisted logic, it cannot make any kind of sense to have the SOFTWARE use sometimes one field and sometimes the other.

> Any time you needlessly duplicate information, you practically guarantee the appearance of a bug involving the two bits of data getting out of sync. It is a common "anti-pattern".

DRY principle.

Heartbleed bug exposes OpenSSL installations

mbanck — Fri, 11 Apr 2014 11:44:01 +0000

"Reminder: the logo designer knew before the Debian, Ubuntu, and RedHat teams." -- https://twitter.com/jamesgolick/status/453605646114783232

Heartbleed bug exposes OpenSSL installations

khim — Fri, 11 Apr 2014 09:03:50 +0000

It's not necessarily anti-pattern. It can be a pattern, too (that's why official forms often include duplication), but if you don't compare these duplicated fields and don't reject inconsistent input that yes, in that case it's an anti-pattern.

Heartbleed bug exposes OpenSSL installations

eru — Fri, 11 Apr 2014 05:01:37 +0000

It seems to me the real root cause of the bug here is having two length fields for the data in the heartbeat packet. Any time you needlessly duplicate information, you practically guarantee the appearance of a bug involving the two bits of data getting out of sync. It is a common "anti-pattern".

Heartbleed bug exposes OpenSSL installations

ianw — Thu, 10 Apr 2014 23:40:13 +0000

"To perform PMTU discovery, HeartbeatRequest messages containing padding can be used as probe packets, as described in [RFC4821]."

Heartbleed bug exposes OpenSSL installations

fandingo — Thu, 10 Apr 2014 22:25:18 +0000

I'm really struggling to see why RFC 6520 has such a large payload size (64KiB). Is there any good reason for it? I'm struggling to see why it should be more than ~8B. Obviously, this particular bug would still occur, but at least it would decrease the amount of information leaked (or at least rate).

"all bugs are shallow"

iam.TJ — Thu, 10 Apr 2014 20:07:16 +0000

In a perfect world project's would operate Continuous Reviews^™ - think 'Continuous Integration' but testing the ability of reviewers and commiters to identify and reject deliberately bad code - off-by-one, null-pointer return values, use-after-free, over-runs, input sanitisation, and many other common 'fail' templates.

OpenBSD hardening vs. OpenSSL

apoelstra — Thu, 10 Apr 2014 17:40:53 +0000

Those links are shocking, especially the second one. But it's hard to follow Ted's analysis — he claims there is a reuse-after-free which has well-defined behaviour since SSL's custom malloc keeps a freelist intact.

In the latest SSL git HEAD, the offending free occurs around s3_pkt.c:1333, as opposed to s3_both.c:1059 as claimed in the article. The quoted code is followed immediately by return(n), so it is hard to tell where the "freed" memory is referenced again. This occurs in the function ssl3_read_bytes, which according to grep is only called through macro obfuscation, so to a casual observer the trail runs cold here.

According to the article, the offending read occurs at "the next call to ssl3_read_n()", a function which is only called by ssl3_read_bytes() around line 1259 — before the offending free! So it seems to me that if Ted's analysis is correct, there must be a repeated call to ssl3_read_bytes() somewhere, and because of the macro obfuscation I can't tell where. :(

Many eyes make all bugs shallow, indeed.

OpenBSD hardening vs. OpenSSL

yokem_55 — Thu, 10 Apr 2014 15:26:41 +0000

I'm surprised Theo's missive on this wasn't included in the security Quotes of the week. Ted Unangst also has a couple of good posts on OpenSSL's custom memory allocator that was masking heartbeat bug -
http://www.tedunangst.com/flak/post/heartbleed-vs-mallocconf
http://www.tedunangst.com/flak/post/analysis-of-openssl-f...

"all bugs are shallow"

dag- — Thu, 10 Apr 2014 13:25:13 +0000

A valid point and there is definitely some responsibility with the various Enterprise distributions and vendors of Linux devices that ship this sensitive code. I am sure the awareness and responsibilities will be discussed within those organisations after the fallout to partake in their due diligence.

But coming back to the original statement from the subject "given enough eyeballs, all bugs are shallow". That statement still holds true. Even though during incidents like this one the validity is being questioned (often by opponents with self-interests).

The statement doesn't say "in open source all bugs are shallow", instead it indicates that there is a certain quantity of "eyeballs" needed. And the fact is that with Open Source, that quantity can be quite large indeed, but it does not guarantee that a sufficient number is reached ;-) Unlike proprietary software where users have no way of doing a security audit which is an exclusive of the vendor, at least in Open Source it is possible. (But apparently not done sufficiently well)

So one of the great strengths (read: freedoms) of Open Source was not used to its own benefit and that is unfortunate. How can we (industry, users) improve the process to avoid this ?

OpenBSD hardening vs. OpenSSL

ptman — Thu, 10 Apr 2014 06:51:52 +0000

http://thread.gmane.org/gmane.os.openbsd.misc/211952/focu...

"all bugs are shallow"

iam.TJ — Thu, 10 Apr 2014 05:54:39 +0000

I'd like to side-step the arguments over C-language allowing buffer overrun here to focus on how this code was reviewed, since the "anyone can read the source code" argument is often raised and here it has spectacularly failed.

In this case Robin Seggelmann co-authored RFC6520 [1] (with Michael Tuexen and Michael Williams) "Transport Layer Security (TLS) and Datagram Transport Layer Security (DTLS) Heartbeat Extension".

He also submitted the OpenSSL patches to add TLS Heartbeat to the openssl-dev mailing list [2] at 2011-12-15 20:04:58 and received only one review response (from Dr Stephen Henson).

After several code tidies the patches were committed [3] by Dr Stephen Henson at 2011-12-31 22:59:57.

Dr Stephen Henson, a core developer of OpenSSL since 1999 [4], was the "eyes" - the reviewer and committer. As such he, more than anyone, should be aware of the gotchyas of writing bullet-proof security code, and especially aware of input sanitisation and where it would be required.

I used the original mailing-list patch submission as the basis for a memory-safe review and found the bug was glaringly obvious - the variable "payload" is read from the network using n2s() - network-to-signed macro - and is then immediately used as a component of 'size' in OPENSSL_malloc() and again in the following memcpy() from the received packet to the outgoing packet.

There is no obfuscation via macros or indirection.

If this class of bug can get through the 'review process' so easily it is a worrying statement about the code-quality of a critical security library, especially given how many sensitive and valuable services rely upon it.

I suggest it serves as a reminder to us all to give a little more time collectively to reviewing proposed patches in our favourite projects and posting responses *before* the patches are committed and subsequently forgotten until they bite.

[1] https://tools.ietf.org/html/rfc6520
[2] http://marc.info/?t=132397969800002&r=1&w=2
[3] http://git.openssl.org/gitweb/?p=openssl.git;a=commit;h=4...
[4] http://www.drh-consultancy.demon.co.uk/openssl-contrib.html