Weak CSPRNG for the salt in some situations.

[AshleyPinner]

https://github.com/DomBlack/php-scrypt/blob/master/scrypt.php#L69-L111

This code seems to do the following to try generate a salt:

• Try use mcrypt_create_iv
• Try use openssl_random_pseudo_bytes
• Try use /dev/urandom
• use mt_rand

This is a flawed approach (mainly because of windows/lack of sanity checking/usage of mt_rand). In general, you should consider the following path:

• Use libsodium if available.
• fread() /dev/urandom if available (never on Windows)
• mcrypt_create_iv($bytes, MCRYPT_DEV_URANDOM)
• COM('CAPICOM.Utilities.1')->GetRandom()
• openssl_random_pseudo_bytes() (absolute last resort)
• if none of the above, FAIL. Your execution can't continue past this point if the platform is unable to give you sufficient randomness for the salt.

You also must check when you read /dev/urandom that you've been provided with a char device, rather than something pretending to be /dev/urandom.

To be honest, though, it's best to just look at including/requiring something like https://github.com/paragonie/random_compat/releases and just using random_bytes() as this'll then ensure you get the right amount of randomess on any platform that this code could possibly run under.

[Quix0r]

What is COM('CAPICOM.Utilities.1')->GetRandom() ? I seem not to find so much about it.

[AshleyPinner]

TL;DR: it's the windows equiv of /dev/urandom.

In the long version, it's a COM call to the CAPICOM Utilities (Windows Cryptographic service) to generate a CSPRNG value.

[DomBlack]

Adding a requirement on another library simply for an example file seems overkill.

Also remember that the salt is not a cryptographic secret, while it has to be random and unique, it doesn't need to be an CSPRNG, as having it guessable by an attacker has no affect on the strength of the hash.

[joepie91]

CSPRNGs aren't just about predictability. A salt being random isn't good enough - if a third party can influence the generation process, this will make you vulnerable as well, since an attacker could intentionally generate repeated salts (thus defeating the point of a salt).

For this reason, you probably still want a CSPRNG.

[paragonie-scott]

Also remember that the salt is not a cryptographic secret, while it has to be random and unique, it doesn't need to be an CSPRNG, as having it guessable by an attacker has no affect on the strength of the hash.

If you use mt_rand(), you will (with 50% probability) get at least one collision after 65,536 salts. This is because mt_rand() (and rand() before it) only support 32-bit seeds. If you have a website with several million users, the chance of multiple salt collisions becomes non-negligible.

A CSPRNG doesn't have this issue. Your first 50%-chance collision is after 2^64 users for a 128-bit salt (which is a couple billion times larger than the current world population) and 2^128 users for a 256-bit salt (which is squarely in the realm of "legally qualifies as an 'act of god'" rarity).

[paragonie-scott]

This is, by the way, a sequel to #20

[DomBlack]

Seeing as we're talking about an example file and not actually a library file, what's the best solution which doesn't add a requirement on another library?

I'm thinking of simply added a a preferred call to random_bytes if it's present on the system, otherwise falling back through the existing code path. ( https://github.com/DomBlack/php-scrypt/blob/master/scrypt.php#L71-L108 )

That way PHP 7 users (or PHP 5 users with random_compat installed) will get a CSPRNG.

Is this an acceptable comprise for everybody? (again I'm thinking as this is purely an example file)

[paragonie-scott]

I think #49 is an appropriate compromise. If users get an exception, they should install random_compat. If they still get an exception from random_compat, they should fix their environment. :)

[AshleyPinner]

Seeing as we're talking about an example file and not actually a library file

The solutions proposed are acceptable, but I will just note one thing. Example code will, without exception, end up copied verbatim into projects. Thus example code must set the best example that it can.

A somewhat related example is the reason why you see so many bcrypt examples using a static salt of R.gJb2U2N.FmZ4hPp1y2CN - it came from an example in a book and people copied it verbatim without further reading.

However, the issue has now been resolved, so thank you all for your time :)