Weak entropy for password generation

[jvoisin]

Currently, some passwords are generated with mt_rand, which is bad idea. This method is also used to generate sms tokens, making them predictable.

As explained in the php documentation:

This function does not generate cryptographically secure values, and should not be used for cryptographic purposes. If you need a cryptographically secure value, consider using random_int(), random_bytes(), or openssl_random_pseudo_bytes() instead.

[coudot]

Well, mt_rand is used to generate the salt in SSHA and SMD5 passwords, not to generate passwords, so I don't see the real issue here.

And SMS token is not a crypto value, it is just a simple text token.

[jvoisin]

My bad about the hashes, I misread.

But the SMS token is a cryptographic value, that shouldn't be predictable by an attacker

[coudot]

Could you give us an example on how to guess the SMS token?

[jvoisin]

This paper provides a nice overview of attacks against php's PRNG.

[plewin]

I suggest replacing all mt_rand uses by a copy of Zend Math's implementation from this line to this one.

Edit : php mcrypt will be required. Currently it is optional

[jvoisin]

mcrypt needs to die in a fire, it's deprecated and relying on a dead-since-2007 (almost 10 years ago) project. Please do not make it mandatory.

You might want to use openssl_random_pseudo_bytes instead.

Or, at stated in php's documentation:

Note: Although this function was added to PHP in PHP 7.0, a » userland implementation is available for PHP 5.2 to 5.6, inclusive.

The userland implementation being random_compat

[plewin]

Sorry, I mean php-mcrypt is required for php < 7.0 because random_bytes is not available.

It seems openssl_random_pseudo_bytes does not produce cryptography secure bytes for all php versions and should be avoided issue other

Thank you for pointing out random_compat. It looks like it is the best polyfill for random_bytes().

Extract from its code for discussion/convenience :

       /**
         * PHP 5.2.0 - 5.6.x way to implement random_bytes()
         *
         * We use conditional statements here to define the function in accordance
         * to the operating environment. It's a micro-optimization.
         *
         * In order of preference:
         *   1. Use libsodium if available.
         *   2. fread() /dev/urandom if available (never on Windows)
         *   3. mcrypt_create_iv($bytes, MCRYPT_DEV_URANDOM)
         *   4. COM('CAPICOM.Utilities.1')->GetRandom()
         *   5. openssl_random_pseudo_bytes() (absolute last resort)
         *
         * See RATIONALE.md for our reasoning behind this particular order
         */

RATIONALE.md of random_compat

@coudot
Is it acceptable to include random_compat files in SSP ?
https://github.com/paragonie/random_compat/tree/v2.0.4/lib
If yes, is it acceptable to add a check on index.php to verify if random_bytes is usable and if not, requesting the installation of php-mcrypt or php libsodium

[jvoisin]

The openssl_random_pseudo_bytes issues was fixed in php 5.4

[coudot]

I would prefer use a standard PHP extension.

We can indeed have checks on PHP version and use the best cryptographic function depending on that.

[jvoisin]

Then please fail early and hard, instead of hiding gracefully degrading to unsafe primitives.

[plewin]

@coudot
What about :

1. A new config option
   $enforce_strong_cryptography = true; // Default true, false allows weak cryptography at your own risks.

2. 2 new functions in lib/functions.inc.php
   is_strong_cryptography_available()
   returns true if PHP7 || ((PHP >=5.4.44 || PHP >=5.5.28 || PHP >=5.6.12) && extension_loaded(openssl)) || extension_loaded(mcrypt)
   random_bytes_compat()
   // use php7 random_bytes if available
   // use openssl if (PHP >=5.4.44 || PHP >=5.5.28 || PHP >=5.6.12) because otherwise it is weak
   // use mcrypt
   // if $enforce_strong_cryptography === false
   // use openssl on other versions if installed
   // use mt_rand

3. New dependency check on index.php
   Report and error if is_strong_cryptography_available() is false and $enforce_strong_cryptography is true
   Error : Your platform does not support strong cryptography. Please upgrade to PHP 7 or install openssl extension.

4. Replace all mt_rand usages by random_bytes_compat.

Not covered by this proposition : sms tokens

Pro/Cons vs random_compat

• ++ Implementation smaller and customized for SSP
• -- Not supported/maintened by random_compat project
• -- Not all alternatives supported (libsodium / fread /dev/urandom/ / Windows CAPICOM.Utilities.1)

[coudot]

@plewin, yes it can be a solution

[coudot]

Done by @plewin in PR #92