View Issue Details

WikiRelated ChangesetsJump to Notes
IDProjectCategoryView StatusDate SubmittedLast Update
0010730mantisbtsecuritypublic2009-07-15 04:432014-12-08 00:33
Reporterdhx Assigned Todhx  
PriorityhighSeveritymajorReproducibilityN/A
Status closedResolutionfixed 
Product Version1.2.0rc2 
Target Version1.3.0-beta.1Fixed in Version1.3.0-beta.1 
Summary0010730: Improve random number generation with openssl_random_pseudo_bytes
Description

mt_rand produces very predictable outcomes and is not very useful for situations where we need secure random numbers. See:
http://www.suspekt.org/2008/08/17/mt_srand-and-not-so-random-numbers/
http://en.wikipedia.org/wiki/Mersenne_twister

PHP 5.3.0 introduced a new function openssl_random_pseudo_bytes() which uses OpenSSL to return pseudo random data. This is a superior choice (when available) as the OpenSSL PRNG can use system gathered entropy, entropy daemons and hardware RNG devices to generate a random number.

TagsNo tags attached.

Relationships

has duplicate 0009191 closeddhx obsolete and remove $g_password_confirm_hash_magic_string 
has duplicate 0009190 closeddhx Improve robustness of auth_generate_confirm_hash() 
related to 0017849 confirmed Salt missing error not very helpful for users 
child of 0015721 closedgrangeway Functionality to consider porting to master-2.0.x 

Activities

jreese

jreese

2009-07-15 09:48

reporter   ~0022505

I'm not sure that your suggestion makes sense. The suggested function is only available in PHP 5.3+, but in PHP 5.2.1, the mt_rand() algorithms gained a new, superior, and unpredictable method for seeding and generating data. This new implementation basically addresses every problem mentioned in the article you posted. The OpenSSL methods technically generate superior data, but they have a much higher cost on the host system, and require the host machine to have a pool of entropy available.
grangeway

grangeway

2009-07-15 18:11

reporter   ~0022511

and open ssl compiled in.

So maybe this issue is just suggestion we up our php version requirement to 5.2.1?

Maybe the best way to handle this is just add an admin check that warns the user if on <5.2.1 that the rand function mgiht not be so random.

Paul
dhx

dhx

2009-07-15 20:16

reporter   ~0022512

The problem with using mt_rand is that once you observe a certain number of outputs from the Mersenne Twister algorithm, you can build up your own PRNG state and thus predict all future outputs. One of the worst possible things we could do is pipe 624 bytes straight from mt_rand() to the user. But users don't even need that much data to rebuild the PRNG state - they can quickly fill in the blanks themselves by guessing.

I've had a look at the PHP source code and the only entropy used in seeding the PRNG is the process ID and current time. These are very predictable numbers.

OpenSSL gathers PRNG entropy from /dev/urandom by default, so it is non-blocking. Users could also potentially configure OpenSSL to gather entropy from a hardware RNG or some other more secure source of randomness.

I was thinking of just having a check to see if PHP >= 5.3 and if so, use OpenSSL (if available) for gathering random data. PHP < 5.3 would have to stick with using the insecure mt_rand() method.

I should point out that the use of a secure PRNG is only really needed where we need security (confirmation hashes, etc). So it'd still be OK to use mt_rand for cosmetic display reasons or something else where we only need the illusion of randomness.
dhx

dhx

2010-02-08 10:16

reporter   ~0024347

Completed. We now no longer use just mt_rand() or gasp rand() for generating hashes and keys in the MantisDB codebase. An exception is in schema.php where the cookie string for the original administrator account is generated. The creation of a default administrator account needs to be fixed anyway, so I'll leave it for another issue.

PS. I haven't tested openssl_random_pseudo_bytes() yet because I don't have PHP 5.3 setup. Also we need a fallback mechanism on Windows that uses something like CryptoAPI via COM.
grangeway

grangeway

2013-04-05 17:57

reporter   ~0036518

Marking as 'acknowledged' not resolved/closed to track that change gets ported to master-2.0.x branch

Related Changesets

MantisBT: master 045a8970

2010-02-08 05:46

dhx


Details Diff		 Issue 0010730: Implement new crypto_api

This implements the foundation of a new Cryptography API which is
responsible for providing cryptography functionality to MantisBT.

For now, the only feature available in this new API is the generation of
secure and strong randomness using openssl_random_pseudo_bytes in PHP
5.3 (if available), /dev/urandom if available on the system or an
enhanced mt_rand generator built on top of PHP's existing Mersenne
Twister pseudo random number.

We used to just rely on a single mt_rand() for generating nonces or
providing other cryptographic functionality. This posed a number of
problems including the leakage of the internal state of the Mersenne
Twister PRNG, enabling users to predict all future outputs of the PRNG.
Additionally, the total number of combinations available from mt_rand()
is very small when in many cases we need more than a few million
combinations of keys.

The new approach calls mt_rand() multiple times and then using a secret
unique salt known only to each MantisBT installation, hashes the output
using the Whirlpool algorithm. This produces 512bits of output that can
be used for creating a random string/nonce. If more than 512bits of
output are required, we simply perform this operation multiple times
until we have generated enough output.

While the new Mersenne Twister method for generating random strings is
still anything but strong or secure, it does raise the bar
significantly. It is hoped that this method is only used as a last
resort when no other options for generating strong randomness are
available.

A new configuration option $g_crypto_master_salt was also added to form
the basis of salting and hashing functions in the future. Currently we
use different keys for RSS, signup/lost password verification and so
forth when it'd be much easier to just derive keys as needed from the
master salt.

If $g_crypto_master_salt is not defined by the user, MantisBT will
refuse to work. This salt must be at least 16 characters long in the
hope that users who don't understand the importance of setting a strong
master salt are informed of their mistake. This refusal to work unless
the user sets a strong $g_crypto_master_salt value in config_inc.php is
necessary because it forms the basis for a lot of the security features
implemented in MantisBT. We don't want users to forgetting to set
$g_crypto_master_salt and using a default value known to the entire
world.		 Affected Issues
0010730
mod - core/constant_inc.php Diff File
mod - admin/install.php Diff File
mod - admin/upgrade_unattended.php Diff File
mod - lang/strings_english.txt Diff File
mod - core.php Diff File
mod - admin/test_langs.php Diff File
add - core/crypto_api.php Diff File
mod - docbook/adminguide/en/configuration.sgml Diff File
mod - config_defaults_inc.php Diff File
mod - admin/check.php Diff File

MantisBT: master eb562360

2010-02-08 09:54

dhx


Details Diff		 Issue 0010730: Use crypto_api for generating nonces and improve hashing

A new Crypto API function crypto_generate_uri_safe_nonce has been added
which generates base64 encoded URI safe alphabet nonces according to
RFC4648. This nonce creation function can thus be used throughout
MantisBT where we need a random nonce. The primary use at the moment is
with form_api tokens.

Hashing throughout the codebase has been improved to use the newly
implemented $g_crypto_master_salt configuration option. This deprecates
a number of older salt configuration options as we now derive salts
from the master salt as needed. The Whirlpool hashing function is used
to generate stronger hashes (instead of the original md5 hashing that is
now deprecated).

RSS keys, cookie strings, lost password confirmation hashes, CAPTCHA
keys, form CSRF tokens and so forth have all been upgraded to make use
of the new Crypto API infrastructure and better hashing/salting methods.		 Affected Issues
0010730
mod - core/rss_api.php Diff File
mod - make_captcha_img.php Diff File
mod - docbook/adminguide/en/configuration.sgml Diff File
mod - config_defaults_inc.php Diff File
mod - core/form_api.php Diff File
mod - core/authentication_api.php Diff File
mod - core/obsolete.php Diff File
mod - core/crypto_api.php Diff File
mod - core/config_api.php Diff File
mod - signup.php Diff File
mod - signup_page.php Diff File

MantisBT: master ab7dad32

2010-02-26 19:42

dhx


Details Diff		 Issue 0010730: Implement CAPICOM PRNG source for Windows servers

First attempt at generating strong randomness on Windows servers by
using the CAPICOM.Utilities library call GetRandom(). Needs testing!		 Affected Issues
0010730
mod - core/crypto_api.php Diff File

MantisBT: master 457d8c9e

2010-03-01 06:19

dhx


Details Diff		 Revert "Issue 0010730: Implement CAPICOM PRNG source for Windows servers"

This reverts commit ab7dad32cd2e53124f1cc78cb62964861ee7c87f.

Relying upon the CAPICOM.Utilities.1 library for providing a PRNG source
for Windows environments seems to be problematic. Problems include lack
of CAPICOM libraries in certain Windows environments as well as
potentially garbled and unpredictable output from the CAPICOM library.		 Affected Issues
0010730
mod - core/crypto_api.php Diff File