Bug #24410

Parameter for function "mcrypt_create_iv" not correct

Added by Wim Roukema about 9 years ago. Updated over 1 year ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
-
Target version:
-
Start date:
2010-12-24
Due date:
% Done:

100%

TYPO3 Version:
4.4
PHP Version:
5.3
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

In a Windows environment the parameter "MCRYPT_DEV_URANDOM" is unknown. This parameter is defined in class.t3lib_div.php on line 1679.

The problem is solved by changing the parameter to: MCRYPT_RAND.

Complete statement:
$output = mcrypt_create_iv($count, MCRYPT_DEV_URANDOM);
changed to
$output = mcrypt_create_iv($count, MCRYPT_RAND);

(issue imported from #M16837)

16837.diff View (689 Bytes) Administrator Admin, 2010-12-29 19:35

16837_v2.diff View (580 Bytes) Administrator Admin, 2010-12-29 19:56

16837_v3.diff View (720 Bytes) Administrator Admin, 2010-12-30 11:21

16837_v3a.diff View (734 Bytes) Administrator Admin, 2010-12-30 11:24

16837_v4.diff View (1.02 KB) Administrator Admin, 2010-12-30 11:50

test_mcrypt_randoms.php View (773 Bytes) Tomasz Krawczyk, 2011-08-24 11:35


Related issues

Related to TYPO3 Core - Bug #22369: Mitigate PHP's RNG vulnerability Closed 2010-04-01
Related to TYPO3 Core - Bug #24440: Improve the used random generators on *nix platforms Closed 2010-12-29
Related to TYPO3 Core - Bug #23355: Speed up / restructure of random byte generator to address e.g. WIN OS specifics Closed 2010-08-05
Duplicated by TYPO3 Core - Bug #24439: TYPO3 fails to install on Windows due to faulty disposal of MCRYPT_DEV_URANDOM Closed 2010-12-29

Associated revisions

Revision f7e9b0bc (diff)
Added by Helmut Hummel over 8 years ago

[BUGFIX] Restructure the random byte generator

Restructure the code to use the most performant methods first
if available. Take specialities of Windows OS and special
PHP versions into account.

Read/ generate more bytes than needed in one call, because it
does not cost (much) more to generate more random bytes, but it's
much cheaper for the next calls, because the bytes are already there.

Resolves: #23355, #23860, #24410, #24440, #23496
Releases: 4.6, 4.5, 4.4, 4.3

Change-Id: I6bad300842f3da40c620b3d79b8116345a2749a0

Revision 98c2451d (diff)
Added by Helmut Hummel about 8 years ago

[BUGFIX] Restructure the random byte generator

Restructure the code to use the most performant methods first
if available. Take specialities of Windows OS and special
PHP versions into account.

Read/ generate more bytes than needed in one call, because it
does not cost (much) more to generate more random bytes, but it's
much cheaper for the next calls, because the bytes are already there.

Resolves: #23355, #23860, #24410, #24440, #23496
Releases: 4.6, 4.5, 4.4, 4.3

Change-Id: I42eea55dcbcd8d8f5b1a6e9493993e9ccd967dfa
Reviewed-on: http://review.typo3.org/4555
Reviewed-by: Oliver Hader
Tested-by: Oliver Hader
Reviewed-by: Tomasz Krawczyk
Tested-by: Tomasz Krawczyk
Reviewed-by: Steffen Gebert
Tested-by: Steffen Gebert

History

#1 Updated by Steffen Gebert about 9 years ago

Hi Wim,

thanks for your report!

Are you able to create a patch against the subversion checkout from https://svn.typo3.org/TYPO3v4/Core/trunk/?

I would prefer the following solution:

$source = defined('MCRYPT_DEV_URANDOM') ? MCRYPT_DEV_URANDOM : MCRYPT_RAND;
$output = mcrypt_create_iv($count, $source);

Could you please test this?

Thanks for your contribution
Steffen

#2 Updated by Steffen Gebert about 9 years ago

After looking at the code, I wonder why this bug exists at all:

if ($output === '' && version_compare(PHP_VERSION, '5.3.0', '>=')) {
    if (function_exists('mcrypt_create_iv')) {
        $source = defined('MCRYPT_DEV_URANDOM') ? MCRYPT_DEV_URANDOM : MCRYPT_RAND;
        $output = mcrypt_create_iv($count, $source);
    }
}

PHP manual tells that MCRYPT_DEV_URANDOM is also available on Windows platforms starting with PHP 5.3.0 (see http://de3.php.net/mcrypt_create_iv).

However, could you please try the attached patch?

#3 Updated by Steffen Gebert about 9 years ago

I would go for v2, which checks

if (function_exists('mcrypt_create_iv') && defined('MCRYPT_DEV_URANDOM'))

as we're fine with pseudo-randomness and MCRYPT_RAND can get very slow.

#4 Updated by Jan Reilink about 9 years ago

Yes, MCRYPT_DEV_URANDOM is defined on Windows, but failes with a fatal error.

Shortened test-code:
$output = ''; $count = (int) '16'; $source = MCRYPT_DEV_URANDOM;
if (function_exists('mcrypt_create_iv')) {
$output = mcrypt_create_iv($count, $source);
echo 'output is: ' . $output;
}

Fatal error message:
Fatal error: mcrypt_create_iv(): Could not gather sufficient random data in [...] on line [...]

URL:
http://testingcode.org/php/typo3.php

#5 Updated by Steffen Gebert about 9 years ago

Umm.. urandom should never get out of "random" data as it's only pseudo random.

I just found the following PHP bug report (which.. the funny thing.. also writes about TYPO3's generateRandomBytes()): http://bugs.php.net/bug.php?id=52523

Derick Rethands wrote:

This is a bug actually. /dev/random is supposed to wait as long as there is enough
entropy. /dev/urandom cares less (and is a worse source of entropy). The behaviour on
Windows needs to behave the same as on a Unix.

The bug is set to fixed after a commit on 2010-08-09 - could you try PHP 5.3.4?

There are also further comments regarding our used code, I will add them to the related #24440.

So.. Jan - could you try a more recent PHP version (5.3.4 seems to be the only possible one currently ;-))? Which one are you using currently?

Further steps? Dunno.. remove mcrypt_create_iv() as it doesn't work well? Using MCRYPT_RAND is IMHO no option (as this one can get very slow..).
Prefer openssl?

#6 Updated by Wim Roukema about 9 years ago

I tested the solution of Steffen Gebert, but the result is the same as Jan Reilink has mentioned.
The constant 'MCRYPT_DEV_URANDOM' is known, but the /dev/urandom directory is not present.

I implemented patch2 of related issue 16871.That works correct.
I don't notice a very slow performance of MCRYPT_RAND, even after clearing the typo3-cache.

Further on "phpf1.com" I found this note:
Note: When using MCRYPT_RAND, remember to call srand() before mcrypt_create_iv() to initialize the random number generator; it is not seeded automatically like rand() is.

Implement this?

#7 Updated by Steffen Gebert about 9 years ago

Silverstripe disabled mcrypt_create_iv() on windows: http://open.silverstripe.org/changeset/114510.

Jan, are you using IIS or Apache?

#8 Updated by Steffen Gebert about 9 years ago

@Wim
sorry, writing my previous comment took quite some time and appears below yours ;)

Of course, /dev/urandom is not known on Windows - that's why mycrypt is used ;)

You reported <em> In a Windows environment the parameter "MCRYPT_DEV_URANDOM" is unknown.</em>. What does "unknown" mean? Seems like it <em>is</em> defined, as Jan reported. Do you also get the "Could not gather sufficient random data" (I assume).

Problem with random generators is that you never know, when there comes a lack of random data. Relying on true random generator functions (instead of pseudo-randomness) can cause long waiting times.. dependent on no more random data being available (e.g. caused by other applications gathering data).

Please see the offical PHP manual, which states that srand() isn't needed anymore with PHP 5.3
http://de.php.net/mcrypt_create_iv

#9 Updated by Jan Reilink about 9 years ago

Hi Steffen,

I tested on Windows Server 2008 with IIS 7.5 and PHP version 5.3.3. I found the same bug reports on php.net.

Of course I can't set-up PHP 5.3.4 on a production web server of ours (earlier mentioned testingcode.org-URL), so I set it up on a test web server. Still no go.

My test PHP-souce (<br>'s removed):

// needed to disable Wincache caching
ini_set('wincache.fcenabled','0');
ini_set('wincache.ucenabled','0');

$output = ''; $count = (int) '16'; $source = MCRYPT_DEV_URANDOM;
echo('PHP version '. PHP_VERSION . ' on server operating system ' 
. $_SERVER['OS'] . ' with web server software ' . $_SERVER['SERVER_SOFTWARE']);
if (extension_loaded('mcrypt')) {
echo('extension mcrypt is loaded'); // mcrypt is statically linked
}
if (defined('MCRYPT_DEV_URANDOM')) {
echo('MCRYPT_DEV_URANDOM is defined');
}
if (function_exists('mcrypt_create_iv')) {
echo 'function mcrypt_create_iv exists';
$output = mcrypt_create_iv($count, $source);
echo 'output is: ' . $output;
}
?>

Under what circumstances gets MCRYPT_RAND slow? I would go with either MCRYPT_RAND if it isn't slow, or openssl.

PS: for performance, drop the CAPICOM ActiveX object support. It's not available by default on Windows.

#10 Updated by Steffen Gebert about 9 years ago

MCRYPT_RAND must be slow, if the server's Random Number Generator has a lack of entropy. I have no proof for MCRYPT_RAND in particular, but every RNG's performance is limited, while pseudo-RNGs are much faster.

Please try attached patch (v3), which suppresses the PHP warning and falls back to other RNGs, if mcrypt_create_iv() returned false.
Solution is taken from Marcus' refactoring PoC #23355

EDIT: v3a slightly improved

#11 Updated by Jan Reilink about 9 years ago

The patch doesn't work. The problem lies in the if .. elseif. The latter never gets executed, since the function mcrypt_create_iv exists. Further, despite suppressing the PHP error (!warning), the PHP execution still stops because it is a fatal error.

Quickest fix would be to go for MCRYPT_RAND, with openssl as second option or fallback.

#12 Updated by Steffen Gebert about 9 years ago

Ah, okay, wasn't aware that it emits a Fatal Error.

v4.. use MCRYPT_RAND for 5.3.0 <= PHP_version <= 5.3.3 and MCRYPT_DEV_URANDOM for PHP_version >= 5.3.4

If mcrypt_create_iv() returns FALSE (in case of this really happens sometimes.. at least the lousy PHP docu says this), openssl is used.

#13 Updated by Jan Reilink about 9 years ago

There is no "MCRYPT_RANDOM":

- $source = (version_compare(PHP_VERSION, '5.3.4', '>=') ? MCRYPT_DEV_URANDOM : MCRYPT_RANDOM);
+ $source = (version_compare(PHP_VERSION, '5.3.4', '>=') ? MCRYPT_DEV_URANDOM : MCRYPT_RAND);

http://nl.php.net/manual/en/mcrypt.constants.php

It runs on PHP 5.3.3 (because of mcrypt_rand) but still fails with a fatal error on 5.3.4. For the time being, please drop mcrypt_dev_urandom and stick with mcrypt_rand or openssl.

The error is still "Fatal error: mcrypt_create_iv(): Could not gather sufficient random data in [...]"

#14 Updated by Steffen Gebert about 9 years ago

Sorry for the typo. So let's go for MCRYPT_RAND. I don't want to spend more time on this PHP shit, seems like the bug is not fixed with 5.3.4.

Could you please send the RFC to the typo3-teams-core mailing list?
See http://typo3.org/teams/core/core-mailinglist-rules/

#15 Updated by Ian Burman about 9 years ago

Same problem after upgrading a Typo3 4.2 site to Typo3 4.4.6 on IIS 6 with PHP 5.3.5
Substituting MCRYPT_RAND for MCRYPT_DEV_URANDOM in class.t3lib_div.php on line 1613 allows the show to go on.

#16 Updated by Tomasz Krawczyk over 8 years ago

  • Target version deleted (0)

Guys I have no such problems on the Apaches but I have this problem on the IIS 6.0. I use PHP 5.3.6.

I resolved the problem by changeing
$output = mcrypt_create_iv($count, MCRYPT_DEV_URANDOM);
to
$output = mcrypt_create_iv($count, MCRYPT_URAND);

So we should simply add condition

if $_SERVER['OS'] = 'Windows_NT') or $_SERVER['SERVER_SOFTWARE'] contains 'IIS' then use
$output = mcrypt_create_iv($count, MCRYPT_URAND);
else use
$output = mcrypt_create_iv($count, MCRYPT_DEV_URANDOM);
.

#17 Updated by Helmut Hummel over 8 years ago

Thomasz: There is no "MCRYPT_URAND" only: MCRYPT_RAND, MCRYPT_DEV_RANDOM, MCRYPT_DEV_URANDOM

So you're using PHP 5.3.6 and MCRYPT_DEV_URANDOM does not work for you but MCRYPT_RAND does?

#18 Updated by Tomasz Krawczyk over 8 years ago

Helmut: Yes, you are right. Sorry for the typo.

I would like to emphasize that it happens only on the IIS.

#19 Updated by Helmut Hummel over 8 years ago

Thomasz: On e.g. apache on Windows with the same PHP version there's no problem?

Please check out:
https://review.typo3.org/4555
if it also fixes your problem.

#20 Updated by Tomasz Krawczyk over 8 years ago

Yes. On the Apache there is no problems but on the IIS there is a problem.

I made simple script to test it (attached). On my two Apaches (WAMP with PHP 5.3.1 and PHP 5.3.6) everything works fine but on the IIS 6.0 with PHP 5.3.6 I have such result:

11:01:11
MCRYPT_RAND is defined
MCRYPT_DEV_URANDOM is defined
MCRYPT_DEV_RANDOM is defined
MCRYPT_RAND - ר*�cdH�
Fatal error: mcrypt_create_iv(): Could not gather sufficient random data in D:\(...)\test_mcrypt_randoms.php on line 24 

If I switch line 24 with 25 result is the same.

I'll upgrade PHP to 5.3.7 and 5.3.8 and test it again, I'll also test your patch in the evening.

#21 Updated by Tomasz Krawczyk over 8 years ago

I had some free time and development server available so, I tested PHP versions on IIS6 (NTS) with my test script. This are results:

5.3.2 - OK
5.3.3 - ERROR
5.3.4 - ERROR
5.3.5 - ERROR
5.3.6 - ERROR
5.3.7 - OK
5.3.8 - OK

It looks MCRYPT_RAND must be used on IIS with PHP from 5.3.3 to 5.3.6.

#22 Updated by Tomasz Krawczyk over 8 years ago

Helmut: Method generateRandomBytes() from https://review.typo3.org/4555 works on IIS6 (PHP 5.3.6) and Apache (PHP 5.3.6).

#23 Updated by Anonymous over 8 years ago

  • Status changed from New to Resolved
  • % Done changed from 0 to 100

#24 Updated by David Bruchmann over 8 years ago

Had to change the proposed solution to

if (function_exists('mcrypt_create_iv')) {
    $source = defined('MCRYPT_DEV_URANDOM') ? (MCRYPT_DEV_URANDOM ? MCRYPT_DEV_URANDOM : MCRYPT_RAND) :  MCRYPT_RAND;
    $output = $source ? @mcrypt_create_iv($count, $source) : '';
}

ENV:
php-5.3.3-Win32-VC6-x86
WinXP SP3
TYPO3 4.5.6

FE loaded after Steffen's Solution but BE still reported the Error-Message.
So a test if MCRYPT_DEV_URANDOM is defined is not enough, it has to be checked if it's filled with some value.
Furthermore mcrypt_create_iv() has only to be called if $source is filled - it occured that even with my extended 2nd line $source was empty.
Other Compilations of PHP may behave different - don't know, didn't succeed to enable VC9-compilations on this OS ...

still had to suppress errormessages in BE by prepending the function with '@'.

#25 Updated by Andreas Wolf over 8 years ago

  • Status changed from Resolved to Needs Feedback

I guess from the most recent commit that this issue is not solved, right? If there are still problems, could someone please open a new issue and link it to this one. This will be closed afterwards.

#26 Updated by Anonymous about 8 years ago

  • Status changed from Needs Feedback to Resolved

#27 Updated by Benni Mack over 1 year ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF