OpenSSL leaks RSA private key information into entropy subsystem | Hacker News Reader

OpenSSL leaks RSA private key information into entropy subsystem

marc.info

60 pointsthrowaway204820 comments

I'm sure this is a good patch, but before you freak out, here's the line he's probably referring to:

        if ((RAND_status() == 0) && rsa->d != NULL && rsa->d->d != NULL)
                {
                /* if PRNG is not properly seeded, resort to secret
                 * exponent as unpredictable seed */
                RAND_add(rsa->d->d, rsa->d->dmax * sizeof rsa->d->d[0], 0.0);
                }

This line is equivalent to "if you are already totally and royally fucked, do something cosmetic to pretend otherwise".

The line occurs during the setup for RSA blinding, which is the only point in the core RSA code (outside keygen) that requires randomness --- blinding mixes a random integer into the otherwise deterministic RSA operation, to foil timing analysis. The code should probably abort when that condition happens. I think, at first glance, that the condition never really happens on normal systems.

That's slightly better, but shouldn't a crypto library return an error and refuse to continue when it encounters a "totally fucked" situation?

Edit: apparently my brain went offline at a critical sentence. Consider this reinforcement of that point.

I'm pretty sure that's what he meant by abort.

Right, so what do crypto coding standards say about leaving weaknesses around as hard-to-hit special cases?

It's not a weakness. If the condition triggers, nothing bad really happens.

The justification given in the patch is that the private key might be given to a pluggable random number generator, which may presumably be controlled by an attacker or otherwise leak the key somehow. In that case, this bit of code is the least of your problems, since every other entropy is also being equally leaked.

That is the code that was removed. It was removed entirely. No error handling was added to replace it. I would hope it will error or pause elsewhere if there is not enough random data though.

That's bad. The condition should remain, but the RAND_ call should be replaced with an abort(). I doubt crappy randomness is much of an issue for RSA blinding, but it's a dealbreaker for other crypto operations.

RAND_status() can never return 0 in the new OpenBSD code. All of the OpenSSL PRNG has been removed, and arc4random_buf() is used instead.

This means that this particular piece of code could NEVER be hit because there is never a time that RAND_status() is going to return 0! i.e. This is unreachable code.

On top of that RAND_add() is a no-op that won't do anything, since there is no way to add "seed" to the PRNG ...

See this commit: http://freshbsd.org/commit/openbsd/58777eed1cff7c5b34cbc0262...

Yep, thanks. Makes sense.

Browsing other messages at the submitted link, did OpenBSD just start tearing apart the guts of OpenSSL since Heartbleed? Looks like OpenBSD lost all respect for the OpenSSL devs.

"But apparently the OpenSSL guys could find no objects of lesser value to pass to the pluggable random subsystem, and had to resort to private keys and digests. Classy."

"OPENSSL_DECLARE_EXIT serves no purpose."

"OPENSSL_gmtime() is not a gmtime() wrapper. It is a gmtime_r(). Always trying to confuse people..."

"Change library to use intrinsic memory allocation functions instead of OPENSSL_foo wrappers. This changes: OPENSSL_malloc->malloc OPENSSL_free->free OPENSSL_relloc->realloc OPENSSL_freeFunc->free"

Yep. It is even easier to see the carnage here:

http://freshbsd.org/search?project=openbsd&q=file.name%3Alib...

Thanks for the link. Looks like the OpenBSD devs enjoy their work.

Also: http://opensslrampage.org/

Here is the patch:

http://freshbsd.org/commit/openbsd/e5136d69ece4682e6167c8f4a...

Wait, so the patch just IGNORES the case where the CSPRNG isn't initialized?

openbsd is switching openssl to be entirely reliant on its system CSPRNG (/dev/random and/or arc4random (which amusingly enough now uses chacha20 not rc4), which as you frequently recommend, cannot block and functions relying on it cannot fail), all internal RNG functionality is beening ripped out.

There will be no CSPRNG to initalize.

API/ABI compat will be maintained by NOOPing the functionality.

That makes a lot of sense. Thanks for the clarity.

If the call to BN_BLINDING_create_param() after that code fails, it will throw an error immediately after. I haven't checked the code for that function, but it probably returns error when it tries to use the CSPRNG.

The CSPRNG has been replaced with arc4random_buf() in the OpenBSD code, and thus can not fail.

Cause there is no CSPRNG anymore: http://freshbsd.org/commit/openbsd/58777eed1cff7c5b34cbc0262...

RAND_status() always returns 1.

Reply on news.ycombinator.com