OpenSSL leaks RSA private key information into entropy subsystem

http://marc.info/?l=openbsd-cvs&m=139773689013690&w=2

by throwaway2048 • 12 years ago

60 20 12 years ago

tptacek5• 12 years ago

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.

mikeash• 12 years ago

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.

clarry• 12 years ago

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

kzrdude• 12 years ago

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

pbsd• 12 years ago

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.

abus• 12 years ago

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.

tptacek5• 12 years ago

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.

X-Istence5• 12 years ago

Here is the patch:

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

tptacek5• 12 years ago

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

throwaway2048• 12 years ago

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.

dlgeek• 12 years ago

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.

X-Istence5• 12 years ago

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

RAND_status() always returns 1.

parfe2• 12 years ago

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"

X-Istence5• 12 years ago

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

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

parfe2• 12 years ago

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

OpenSSL leaks RSA private key information into entropy subsystem

Related Stories