Flutterby™! : Theo de Raad on OpenSSL deliberately

Next unread comment / Catchup all unread comments User Account Info | Logout | XML/Pilot/etc versions | Long version (with comments) | Weblog archives | Site Map | | Browse Topics

Theo de Raad on OpenSSL deliberately

2014-04-11 00:10:07.555226+00 by Dan Lyke 6 comments

Theo de Raadt on OpenSSL deliberately coding around OpenBSD's exploit countermeasures http://article.gmane.org/gmane.os.openbsd.misc/211963

[ related topics: Software Engineering ]

comments in ascending chronological order (reverse):

#Comment Re: made: 2014-04-11 11:12:54.219337+00 by: DaveP

I'd seen that before, and my first comment on seeing the code was "Well, if malloc returns memory that's been zeroed, this is a non-issue." I'm with Theo on this one. The OpenSSL team is not to be trusted.

#Comment Re: made: 2014-04-11 13:31:14.523951+00 by: Dan Lyke


#Comment Re: made: 2014-04-11 14:50:33.654893+00 by: DaveP

The problem was reported four years ago.

That goes from "untrustworthy" to "criminally negligent" in my book.

I think Theo's comments were actually pretty restrained.

#Comment Re: made: 2014-04-11 22:29:56.446243+00 by: spc476

DaveP, the standard for malloc() does not guarantee the returned memory is set to any particular value. If you want that, you need to call calloc(), or after the malloc() call, set it to a value of your choice (realloc() does not clear out memory either). A system is free to set the value on malloc() (or realloc()) but a programmer cannot rely upon that behavior in a portable program.

#Comment Re: made: 2014-04-11 23:53:10.328359+00 by: Dan Lyke

DaveP wasn't suggesting that it did, he was suggesting that it should. And since the first thing you do with any C or C++ app is wrap malloc() and free() (and, in the case of C++, new and delete) for ease of debugging, it's also trivial to make that new allocator zero things (or, more likely, set it to something non-zero but recognizable, so errors show up quickly).

And in the case of security code, it's also completely within the realm of a good idea to make the de-allocator do similarly destructive things to memory.

However, going immediately to a pool allocator (as OpenSSL did) is a really bad idea unless you've got your own boundary detection code, and even then being able to turn it off for automated testing, or muck with other parameters for automated testing, is a necessity.

#Comment Re: made: 2014-04-12 16:00:21.323998+00 by: DaveP

As Dan said, I didn't suggest that malloc() sets memory.

But malloc(9) (the kernel version) can clear memory, and does so in some implementations. And probably will in more after the dust settles from this fiasco.

Almost all malloc() implementations have some way to set flags so malloc will either zero all memory it hands out, or fill the memory with useful values for debugging (such as all odd values so that an attempt to treat it a a pointer will cause an exception). But the OpenSSL team walled themselves off from ways to let their tools help them, and even worse, ignored a bug report pointing to the fact that their memory implementation was unsafe.

The malloc() implementation I use most gives you even more debugging options. I also have a thin wrapper which causes malloc to fail once in a while. Both are useful in tracking down bugs.

There were many possible tools for the OpenSSL team to use that would have helped catch this problem, and they chose to make it impossible to use those tools.