Heartbleed, a few naive questions

[Ronald F. Guilmette]

In message <53463A2E.90905 at delphij.net>, 
Xin Li <delphij at delphij.net> wrote:

>On 4/9/14, 10:28 PM, Ronald F. Guilmette wrote:
>> 1)  Why does OpenSSL even contain a function called
>> "OPENSSL_malloc"? Does anyone other than me think that it might
>> perhaps have been a better choice to provide only a function called
>> "OPENSSL_calloc"?
>
>For this bug, doing calloc() makes no difference.

I would very much like to know how you reached that conclusion.  I may
be wrong in this instance... I have been, on more than one occasion in
my lifetime... but I do disagree with your conclusion in this instance.

The bug is described in some great detail here:

   http://blog.existentialize.com/diagnosis-of-the-openssl-heartbleed-bug.html

What I believe to the the text of the buggy code in question appears to be
available here:

   http://code.woboq.org/crypto/openssl/ssl/d1_both.c.html

For the information being leaked to an attacker by this bug, where exactly
do _you_ believe it is coming from, if not out of a buffer which has been
allocated (out of recycled and non-cleared memory) by the call to
OPENSSL_malloc?

>Is doing calloc() a
>good practice in general?  Sometimes it is, sometimes it merely slows
>things down.

I agree entirely.  If data leakage is occurring within the program that
I use to crop my vacation pictures, the world will not be terribly the
worse for it.  but that was not the question I was asking.  Rather, I
was asking, albeit indirectly, whether a program or library, such as
OpenSSL, which is primarily a security-forcused tool, and upon which
a significant fraction of online humanity depends for its security,
is deserving of a "belt and suspenders" sytle of defensive programming.

>> 2)  Not that this would actually have eliminated the bug, but...
>> 
>> Was there some compelling reason why space for the "buffer" at
>> issue was allocated via a call to "OPENSSL_malloc", rather than,
>> say, simply being declared as a function-local "auto"
>> char[1+2+65536+padding] ?
>
>Automatic arrays are introduced with C99.

I am sorry that I failed to be clear.  My question has/has nothing to do
what what you are calling "automatic arrays" and what I would prefer to
call "dynamically sized arrays".  If you look at the actual code you will
see that "padding" is not really a variable quantity.  Rather it appears
to have a fixed value (16).  So the real question I had intended to pose
was actually: "Why wasn't space for the buffer obtained by a simple local
variable declaration of the following general form?"

	enum { padding = 16 };
	auto char buffer[1+2+65536+padding];

or more simply:

	auto char buffer[1+2+65536+16];

>By the way, how do you benefit from allocating from stack than from
>heap for relatively large chunk of memory?

Doing so is unambiguously more efficient, and provides an absolute assurance
that the memory will be fully and properly reclaimed at the instant the
containing function exits, however just with respect to the security issues
alone, I must admit that this might possibly not make any difference at all,
specifically in relation to memory leakage of those secrets which belong
*to the server itself*.  However depending on how (or whether) memory
which is allocated to hold execution stacks is pre-zeroed (before being
given to a process or a thread) and depending on whether each SSL session
gets its own separate thread (and thus also its own private execution stack)
allocating buffers out of the stack, rather than out of the (common) process-
wide heap might at least prevent leakage of secrets belonging to one client
to a different client.

(And by the way, it would not be a terribly difficult thing to add to any
C compiler a new option that, when used during compilation of select
"mission critical" secirity software, would have the effect of generating
code for each function that completely zeros at least the statically-sized
part of the new stack frame upon entry to the function, or conversely
would generate code which would zero the current frame just before
function exit.  Such an option might be useful when compiling critical
security code, such as the OpenSSL library, and if it had been available
and used in conjunction with allocation of the problematic buffer on the
stack, as I suggested, I believe that those steps, taken together would
have eliminated even the possibility of leakage of server-specific secrets.
Such an approach might seem extreme to some, but would it have been worse
that the current mess the net finds itself in?)

>> 3)  Not that this would actually have eliminated the bug, but...
>> 
>> After making some effort to do so, I have been unable to find any 
>> information online which would serve to document the initial state 
>> of the blocks of memory allocated by calls to pthread_create(). 
>> Would any of you happen to know if said blocks are or, conversely, 
>> are not cleared to zeros prior to execution of the created
>> thread(s)?
>
>I don't think I understood this question...

I'll try again.  A call to pthread_create() causes space to be allocated
for an execution stack for the newly created thread.

I was asking if that space is cleared to zeros (by pthread_create) before
it is handed over to the nely created thread.

My hope and belief is that it is, but if it is, that fact does not seem to
have made its way into the relevant man page.

>> 4)  If, as has been suggested in some quarters, the _actual_ size
>> of the heartbeat client-supplied payload may be correctly
>> determined without any reference whatsoever to the _content_ of the
>> request packet, then why is/was it the case that the SSL protocol
>> specified that such packets should contain a payload length
>> specification... a value that, as we all now know, may be spoofed
>> with disasterous consequences?
>
>Do you have any reference here?  I'm not an expert in OpenSSL code but
>looking at the specification, it seems like, without the payload
>length there is no way to figure out what length the padding is.

Please review the detailed description of the bug provided here:

    http://blog.existentialize.com/diagnosis-of-the-openssl-heartbleed-bug.html

    "Note that the actual length in the SSLv3 record is not checked."
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This is the crux of the matter.  The actual received record is this big:

        ->.<-

Unfortunately however there is a field within the record which says how big
it is _alleged_ to be... by the client (aka the attacker).  The value
within that field can be a lie.  It can claim that the received record
is actually this big:

	->...........................................................<-

The problem is that the buggy OpenSSL code believed the lie, rather than
believing the actual size of the record it received, which it also had
ready access to.

My question was about the specification of the SSL protocol.  I do wonder
why the authors/developers of that protocol engineered it in such a way
that a client was even able to lie to the server about how much data he
had actually sent.  Why was that field even present in the protocol??
It was clearly not needed, since... as is now clear... servers have other
(and accurate) ways of deducing the _actual_ lenth of the _actual_ payload.

In effect it seems to me that the very design of the protocol may be to
blame here.  Is the SSL protocol, as currently defined, walking around
with a giant "KICK ME" sign taped to its back?

The protocol provided the ability... if not to say the outright INVITATION...
to lie to the server.  Then we all wake up one day and learn that, lo and
behold, some nefarious people might actually be doing that.  Now, everybody
is running around expressing shock and surprise.  I'm not sure that I
understand that.  Having read a couple of explanations of this bug now,
the whole thing seems (a) inevitable and (b) fundamentally the fault of
the protocol specification itself.  And yet various online commentators
seem all too ready and willing to lay the blame on the C language itself,
almost as if *it* created the flawed protocol, or developed the bad code.
>From where I am sitting, nothing could be further from the truth.

>> P.S.  Looking just now at the man page for execve(2) I could not
>> help but notice that it neglects to say a single word about the
>> state of the memory comprising the initial page of the (new)
>> execution stack...
>...
>New pages are always initialized to zeros before access is given to
>process,

Right.  My point is that it wouldn't kill anyone to explicitly say that,
specifically in the man pages for exec*() and also for brk()/sbrk().

(The latter is relevant to the heap, while the former is relevant to the
stack.  In both cases, new space is zeroed before being given to the
process.  These facts are undoubtedly widely known but are nontheless
currently UNDOCUMENTED within the relevant FreeBSD man pages.  This
should be corrected.  Worse yet, looking now at my dog-eared copy of
IEEE 1003.1-1990 I see that even the official POSIX.1 standard entirely
dropped the ball by also and likewise failing to even mention that the
stack space given to the new process after a call to exec*() may be
relied upon to contain something other than flotsam and jetsum left
over from previously demised processes.  The fact that the POSIX
comittee was guilty of this shameful omission is no excuse for the
FreeBSD team to repeat their mistake.)

>Hope this helps.  Should you find anything that we need to correct in
>the manual page to help others please speak up.

I just did.


Regards,
rfg