Stop using openssl_random_pseudo_bytes()

[tom--]

I am uncomfortable with \base\Security::generateRandomKey()'s use of openssl_random_pseudo_bytes(). It's complicated, sorry, but worth discussing.

Why I don't like OpenSSL's RNG

TL;DR: I don't properly understand how OpenSSL's RNG works nor how it is configured in PHP but I do know that are ways that it can be configured so that it has properties that a CSPRNG must not have.

For example, the only way to be sure that OpenSSL's RNG is properly seeded is to do it yourself from a known-good source, i.e. the kernel or CryptGenRandom. But generateRandomKey() uses openssl_random_pseudo_bytes() as its last-chance source after trying and failing to read from those. So I think it likely in practice that generateRandomKey() uses OpenSSL only when it seeded itself using its fall-back, undocumented, user-space methods that I don't trust.

For another example, OpenSSL can be configured to use, when available, Intel's RDRAND, now known to be unsafe.

What to do?

1. Simply removing it will break everything on Windows and some non-Windows deployments.
2. We removed mcrypt_create_iv() when we replaced Mcrypt with OpenSSL. But mcrypt_create_iv() does not use libmcrypt and it is the only good way to access CryptGenRandom in PHP < 7. I think it's ok to use it where available so we could restore it.
3. We could leave openssl_random_pseudo_bytes() in but disabled by default, with a configuration member variable, Security::$allowOpenSslRng = false with new documentation to educate the user about the concerns.

Perhaps doing both 2. and 3. is reasonable.

Opinions?

[samdark]

Hmm... mcrypt_create_iv requires mcrypt dependency so there will be both dependencies on mcrypt and openssl.

[cebe]

we could check if function exists and only then use it.

[tom--]

I don't want to bring the Mcrypt PHP extension back as a Yii requirement.

But if (function_exists('mcrypt_create_iv')) {} is, I think, a better way to get random numbers on Windows that OpenSSL.

[SamMousa]

How about not implementing a solution yourselves, but using an existing one:

https://github.com/paragonie/random_compat

That way you can use random_bytes in Yii code and keep it clean and simple.

Obviously, you should review their code, but in general I think it's better to use outside code than reinvent the wheel right?

[tom--]

I'm glad you mentioned random_compat because I haven't looked at for a while. I took a quick look now and it's way better than it used to be. I like it. php.net recommends it and I think it's safe to make Yii depend on it.

I will need to read it carefully but it would settle two issues that I entered at the weekend. And as @SamMousa says, it will be clean and simple.

[samdark]

Am I right that we'd be able to use PHP7 API with it?

[SamMousa]

Yes

On Mon, Oct 12, 2015, 22:40 Alexander Makarov [email protected]
wrote:

Am I right that we'd be able to use PHP7 API with it?

—
Reply to this email directly or view it on GitHub
#9879 (comment).

[samdark]

I love the idea then. Waiting on @tom-- to confirm that the library is good.

@paragonie-scott what's your plan on supporting the library?

[samdark]

Weird comments:

• https://github.com/paragonie/random_compat/blob/master/lib/random.php#L36
• https://github.com/paragonie/random_compat/blob/master/lib/random.php#L84

[SamMousa]

Not weird just in progress:paragonie/random_compat#5

[paragonie-scott]

what's your plan on supporting the library?

random_compat? The plan is to immediately and quickly fix any bugs or deficits I find and have other crypto folks check my work before releasing anything new.

I'm not sure if I answered your question.

Not weird just in progress:

Yeah, look at the 1.0.5 tag, not master.

[paragonie-scott]

(For the record, the next release will probably be tagged 1.1.0 if we do decide to drop OpenSSL in known-to-be-bad-versions.)

[samdark]

@paragonie-scott, @tom-- what do you think about https://github.com/ircmaxell/random_compat?

[paragonie-scott]

ircmaxell/RandomLib#37 (comment)

[tom--]

I'm going to let the discussions ongoing elsewhere proceed some more before deciding what to do with Security.