From time to time, the old Debian OpenSSL bug resurfaces in a conversation. Usually resulting in somebody (not everybody, but at least one person) drawing completely wrong conclusions. Many of the writeups I’ve read focused on the real bug, which is tricky, because the real code is... real. It’s scattered throughout several files and many functions. I think recreating a conceptually similar bug, but with all the code in one place, will make it easier to understand.

Another major problem is that the popular perception of the bug as constructed via online forum telephone game is inaccurate. In fact, while researching this post, I discovered my own understanding of the bug was imprecise. I’ll deal with this second.

story version one

At least one version of the story (that I have helped perpetuate at times, oops!) goes like this. valgrind detected the use of uninitialized memory. A Debian developer freaked out and sprinkled a whole bunch of memset calls throughout the code, accidentally wiping out the collected entropy. The erroneous conclusion then drawn from this is some variant of “initializing memory is bad”. This is totally wrong and refuting this point was my original motivation for writing.

Let’s look at some pseudo-C code that implements something like what OpenSSL was doing. We want to generate a random key for crypto by reading from the random device and hashing the result.

unsigned char *makeRandomKey()
{
    unsigned char buffer[40];

    read("/dev/random", buffer, 32);

    return SHA256(buffer, 40); /* valgrind complains about this line */
}

Notice that we’re hashing the full 40 bytes of the buffer, even though only 32 bytes were filled in with data. valgrind correctly warns about this. But it’s harmless. A few extra bytes on the stack, which may or may not be zero, will get fed into the hash function. We’re never going to try to recreate the exact contents of buffer so it doesn’t matter. It’s shoddy programming, but not life threatening.

In any case, Debian doesn’t like warnings and fixed it like so:

unsigned char *makeRandomKey()
{
    unsigned char buffer[40];

    read("/dev/random", buffer, 32);
    memset(buffer, 0, 40); /* there i fixed it */
    return SHA256(buffer, 40);
}

OK, here we do have a life threatening problem. We’ve thrown away all the entropy we just collected! And now people offer up bad advice like, “If you initialize memory that’s supposed to be used uninitialized, bad things will happen.” No. Memory should never be used uninitialized. The issue is that it needs to be initialized correctly.

unsigned char *makeRandomKey()
{
    unsigned char buffer[40];

    memset(buffer, 0, 40); /* correct fix is correct */
    read("/dev/random", buffer, 32);
    return SHA256(buffer, 40);
}

No warnings, no bugs, no problems. Everything is groovy. Except for the small detail that this isn’t what actually happened...

the real story

I wrote up that version of the story because I wanted to refute the conclusion that adding memset calls is bad. It’s based on the popular perception that Debian caused the bug by adding memset calls. I did some reference checking, however, and discovered reality is a little different. Consider this similar version of the above code:

unsigned char *makeRandomKey()
{
    unsigned char buffer[40];
    struct shactx ctx;

    SHA256_Init(&ctx);
    read("/dev/random", buffer, 32);

    SHA256_AddData(&ctx, buffer, 40);
    return SHA256_Hash(&ctx);
}

Similar code, same bug. Reading some bytes of the buffer that aren’t filled in. The fix is quite different, however.

unsigned char *makeRandomKey()
{
    unsigned char buffer[40];
    struct shactx ctx;

    SHA256_Init(&ctx);
    read("/dev/random", buffer, 32);

    return SHA256_Hash(&ctx);
}

Just delete the problematic call entirely! The net result is the same; we aren’t mixing in any entropy from the random device. The difference is there’s no memset call. You can’t conclude anything about “too much” initialization from this story, because there isn’t any initialization. Correctly fixing the bug is the same as before.

references

Russ Cox wrote Lessons from the Debian/OpenSSL Fiasco. Ben Laurie wrote Debian and OpenSSL: The Aftermath and Vendors Are Bad For Security, which contains links to several original sources. All these pages are good to read, but they get lost in some of the minutiae of how and when the uninitialized memory will be used. Hopefully reducing everything to five line examples clarifies what happened, why it’s bad, and how a correct fix could have been made.