Heartbleed Coder: Bug In OpenSSL Was an Honest Mistake 447
nk497 (1345219) writes "The Heartbleed bug in OpenSSL wasn't placed there deliberately, according to the coder responsible for the mistake — despite suspicions from many that security services may have been behind it. OpenSSL logs show that German developer Robin Seggelmann introduced the bug into OpenSSL when working on the open-source project two and a half years ago, according to an Australian newspaper. The change was logged on New Year's Eve 2011. 'I was working on improving OpenSSL and submitted numerous bug fixes and added new features,' Seggelmann told the Sydney Morning Herald. 'In one of the new features, unfortunately, I missed validating a variable containing a length.' His work was reviewed, but the reviewer also missed the error, and it was included in the released version of OpenSSL."
Re:Whatever you may think ... (Score:2, Informative)
The RFC and the Git commit both have his name firmly attached to the "feature". He hasn't admitted anything that wasn't proven already.
Re:It's not just the implementation (Score:5, Informative)
and all running on top of TCP
Not necessarily. OpenVPN is an SSL VPN which defaults to UDP/1194.
Re:Names! (Score:5, Informative)
Re:He's sorry now ... (Score:3, Informative)
Lawyers love EULA's and licenses. The OpenSSL license disclaims all liability
* THIS SOFTWARE IS PROVIDED BY THE OpenSSL PROJECT ``AS IS'' AND ANY
* EXPRESSED OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
* PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE OpenSSL PROJECT OR
* ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
* NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
* STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
* OF THE POSSIBILITY OF SUCH DAMAGE.
https://www.openssl.org/source... [openssl.org]
If you never agreed to that license, you're violating their copyright.
Re:Names! (Score:5, Informative)
Have a look for yourself. [github.com] The reviewer "steve" is Stephen Henson.
Sloppy code (Score:5, Informative)
Re:Whatever you may think ... (Score:5, Informative)
RSA has denied having knowledge of the backdoor, says NSA tricked them, and has never denied the $10M payout. Some of Snowden's leaks mention it.
Reuters has a summary [reuters.com]
proof-of-concept backdoor with a link to the github repo.
None of that is a smoking gun, but there is enough smoke to tell me there is a fire.
Re:Whatever you may think ... (Score:3, Informative)
I'll go as far as it is reasonable to suspect there could be a fire. All of the background reading that I have done only shows that it may be possible that a backdoor exists, not that there actually is one. As for the RSA affair, it is entirely possible that they were simply trying to promote what was at the time a very hot and promising encryption technology.
Of course keep in mind that it took people 20 years to figure out that NSA strengthened DES against cryptanalysis methods that were still secret when it changed the S-boxes before the DES standard was approved. NSA made DES stronger, not weaker.
Re:Whatever you may think ... (Score:5, Informative)
From the proof-of-concept page [0xbadc0de.be] I mentioned above.
Here is the Github repo for the PoC code. [github.com]
This PRNG is not the NSA making a crypto system stronger ala DES, it's a backdoor.
Re:It's not just the implementation (Score:5, Informative)
As others have indicated, the primary stated use of heartbeat is for DTLS, which is not over TCP.
The payload length is not actually superfluous. The packet has an arbitrary amount of both payload and padding, of which only the payload is echoed to the sender. Roughly: { uint16 payload_len; char payload[]; char padding[]; } The intent of payload_len is to tell you which of the bytes following it are payload rather than padding. Of course, you need to check that it's less than the remaining data in the packet. (Per the spec, at least 16 less -- at least 16 bytes of random padding are required.)
Re:Whatever you may think ... (Score:2, Informative)
A more fitting analogy is that you wrote an open source project, took the time to explain that the code is free and may not be perfect, and then someone ignored the warning that was available at the top of every source file, and ran into the current situation.
It's not complicated enough to need analogies.
Re:Not malicious but not honest? (Score:5, Informative)
considering that the real bug is OpenSSL's malloc, where it was reusing 'freed' memory I think that's the bug that is critical.
Well. no.
The bad bit of code is, per http://www.tedunangst.com/flak/post/heartbleed-vs-mallocconf [tedunangst.com]:
struct {
unsigned short len;
char payload[];
} *packet;
packet = malloc(amt);
read(s, packet, amt);
buffer = malloc(packet->len);
memcpy(buffer, packet->payload, packet->len);
write(s, buffer, packet->len);
The bad bit is that "amt" is not checked against "packet->len", so the copy into "buffer" reads off the end of the allocated data structure "packet". The data read may be freed memory, or it may be allocated memory.
The only way malloc could completely protect against the bug would be by putting an unmapped guard page after every malloced block - making every malloced block at least one page long and slowing malloc down by the time needed for all those munmaps. (Probably making malloc slow enough to incite OpenSSL devs to implement their own malloc layer...)
I think the real bug is in the RFC.
Look at the fix:
if (1 + 2 + 16 > s->s3->rrec.length)
return 0;
hbtype = *p++;
n2s(p, payload);
if (1 + 2 + payload + 16 > s->s3->rrec.length)
return 0;
pl = p;
Why does the heartbeat request even contain the length of the heartbeat block? We know the length of the SSL record!
(Not even bothering with the whole problem that the heartbeat thing is ridiculous - there are already ways of keeping connections alive at the TCP level - why does every layer of the protocol need it's own keepalive).
Comment removed (Score:5, Informative)