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.
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 ...
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.
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.
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"
23 comments
[ 3.4 ms ] story [ 58.0 ms ] threadThe 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.
Edit: apparently my brain went offline at a critical sentence. Consider this reinforcement of that point.
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.
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...
http://freshbsd.org/commit/openbsd/e5136d69ece4682e6167c8f4a...
There will be no CSPRNG to initalize.
API/ABI compat will be maintained by NOOPing the functionality.
RAND_status() always returns 1.
"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"
http://freshbsd.org/search?project=openbsd&q=file.name%3Alib...