Weak PRNG used

[paragonie-scott]

Insecure RNG:

stormpath-sdk-php/src/Util/UUID.php

Lines 167 to 181 in 15aee30

	/*
	* Generate an UUID version 4 (pseudo random)
	*/
	static private function generateRandom($ns, $node) {
	$uuid = self::$m_uuid_field;
	$uuid['time_hi'] = (4 << 12) | (mt_rand(0, 0x1000));
	$uuid['clock_seq_hi'] = (1 << 7) | mt_rand(0, 128);
	$uuid['time_low'] = mt_rand(0, 0xffff) + (mt_rand(0, 0xffff) << 16);
	$uuid['time_mid'] = mt_rand(0, 0xffff);
	$uuid['clock_seq_low'] = mt_rand(0, 255);
	for ($i = 0; $i < 6; $i++)
	$uuid['node'][$i] = mt_rand(0, 255);
	return ($uuid);
	}

Insecure RNG fallback:

stormpath-sdk-php/src/Authc/Api/ApiKeyEncryptionOptions.php

Lines 48 to 50 in 62698ea

	$salt = function_exists('openssl_random_pseudo_bytes') ?
	openssl_random_pseudo_bytes(16) :
	substr(md5(uniqid('', true)), -16);

Background: https://paragonie.com/blog/2015/07/how-safely-generate-random-strings-and-integers-in-php

Might I recommend https://github.com/paragonie/random_compat instead?

[paragonie-scott]

The reason for this, we have tried to use CSPRNG's in the past but do not work for every application. Our goal here is to give everyone the ability to use our product and using '/dev/urandom' does not work for everyone

For balance, random_compat is used by WordPress (as of 4.4) and currently works cross-platform even on ancient versions of PHP. It will almost certainly work for your users (as you require PHP 5.4+).

and can also slow the system down a lot

I think you're thinking of /dev/random not /dev/urandom? I've benchmarked mt_rand() vs CSPRNGs previously. (OpenSSL is the fastest, but it's a userspace CSPRNG that shares state across forks, which caused collisions in ramsey/uuid and other libraries.)

This is still using mt_rand but is ok with us.

[TomMD]

Scary to say the least. It's rather easy to make a generator that draws entropy from /dev/urandom (OS X, *nix) or CryptAPI (Windows). Would that suffice or are there more platforms you are referring to?

[bretterer]

@paragonie-scott,

Sorry, I hit the X instead of the edit on my Github comment and didn't mean to.

You are correct, I was thinking /dev/random in my comment. I understand that /dev/urandom is being used in WordPress. Use of /dev/urandom does follow current best practice for generating random numbers. At this time however, we do not have this in our backlog to change out in the PHP SDK but will make sure we put this on our priority list to investigate.

When we create packages for developers to use, we try to use built in php functions and limit the number of external packages we require during the installation of the package. This is the main reason why we initially used built in methods.

Here is the orig comment for reference:

The reason for this, we have tried to use CSPRNG's in the past but do not work for every application. Our goal here is to give everyone the ability to use our product and using '/dev/urandom' does not work for everyone and can also slow the system down a lot.
This is still using mt_rand but is ok with us.

[paragonie-scott]

When we create packages for developers to use, we try to use built in php functions and limit the number of external packages we require during the installation of the package. This is the main reason why we initially used built in methods.

Luckily, random_compat is a polyfill for the new internal functions in PHP 7.

• https://secure.php.net/random_bytes
• https://secure.php.net/random_int

In PHP 7, including the library is a NOP. In PHP 5, it uses the most secure backend available.

[bretterer]

@paragonie-scott so for a little bit of information on how we currently use the UUID::v4() method in our system. This is used inside of our Stormpath\Resource\Application resource during our creation of the ID Site URL generation. In our JWT, we define a jti which is a random number. This does not require a cryptographically secure values which is why we are still using mt_rand() function inside of PHP.

As I said before, We are happy to look at these suggestions of changing our current UUID::v4 method to use a different method of generating random numbers. I will leave this ticket open until we discuss this internally at more depth.

[paragonie-scott]

For what it's worth: https://paragonie.com/blog/2015/07/common-uses-for-csprngs-cryptographically-secure-pseudo-random-number-generators#uuid-v4

[AshleyPinner]

Use of /dev/urandom does follow current best practice for generating random numbers.

Say's who?

For reference, urandom is what you want: http://sockpuppet.org/blog/2014/02/25/safely-generate-random-numbers/

[atoponce]

Use of /dev/urandom does follow current best practice for generating random numbers.

Say's who?

As you linked to, if you have access to a kernelspace CSPRNG, whether through an API, or accessing the device directly, you should use it, instead of relying on a userspace CSPRNG, unless you know you need otherwise. Chances are high that the system CSPRNG has already been correctly seeded with sufficient entropy, and it's output is indistinguishable from true random.

[AshleyPinner]

Herp. For some reason pre-coffee I misread that as "Use of /dev/urandom doesn't follow..."

Ignore me!

[bretterer]

Thank you again for reporting this. We have removed the weak PRNG from our API Key Options file as it was dead code. Since the PHP SDK requires PHP 5.4 and above, there is never a case where openssl_random_pseudo_bytes would not be available.

[paragonie-scott]

Please be aware that OpenSSL's userspace PRNG isn't fork-safe.

[bretterer]

@paragonie-scott, Thank you for mentioning this. After discussing this with our internal team, I would like to re-visit this. I will reach out to you via email to discuss this further and the possibility of adding this into our SDK.