I asked Pierre Joye, from the PHP team, to review our random number generation function (drupal_random_bytes()).
He suggested to use openssl_random_pseudo_bytes() if available (that's available since PHP 5.3), as it seems to be the only reliable source of random numbers of Windows for now.
Comment | File | Size | Author |
---|---|---|---|
#17 | drupal-838800-4.patch | 1.7 KB | jromine |
#10 | drupal-838800-3.patch | 2.07 KB | kotnik |
#8 | drupal-838800-2.patch | 1.74 KB | kotnik |
#1 | 838800.patch | 959 bytes | Damien Tournoud |
Comments
Comment #1
Damien Tournoud CreditAttribution: Damien Tournoud commentedSomething like that should do it.
Comment #2
MustangGB CreditAttribution: MustangGB commentedCan't see a reason not to include openssl_random_pseudo_bytes() for a better system agnostic solution
Comment #3
Dries CreditAttribution: Dries commentedThis looks RTBC to me.
It could be good to extend the code comment so it mentions that it is the only reliable source of random numbers on Microsoft Windows.
Comment #4
xjmNote that the patch needs a reroll for
core/
. If we're doingfunction_exists()
, we can also backport this.Comment #5
droplet CreditAttribution: droplet commentedanyone do a real test, I read this from PHP.net
and seems fixed on latest PHP version..
http://stackoverflow.com/questions/1940168/openssl-random-pseudo-bytes-i...
Comment #6
catchSince we only require 5.3.2, we might want to do a version check here. Moving back to needs review.
Comment #7
xjmTagging as novice for
elseif (function_exists('openssl_random_pseudo_bytes'))
. It should be at least5.3.25.3.4.Comment #8
kotnik CreditAttribution: kotnik commentedHere's a patch that does what's been requested in #7.
But, shouldn't it check for PHP 5.3.4 since that version fixed openssl_random_pseudo_bytes locking on Windows?
Comment #9
xjm#8: You're correct, 5.3.4, not 5.3.2. Thanks!
Comment #10
kotnik CreditAttribution: kotnik commentedOk, here's the re-roll.
Comment #11
marvil07 CreditAttribution: marvil07 commentedPatch is doing what was agreed on comment 7 and 9.
Comment #12
droplet CreditAttribution: droplet commentedManually tested ??
Maybe bump up PHP version again? PHP 5.3.8 and below has hash collision vulnerability. I think all hosting will bump up their PHP version.
Comment #13
Damien Tournoud CreditAttribution: Damien Tournoud commentedThe hash collision is unrelated to this patch.
Comment #14
droplet CreditAttribution: droplet commented@#13,
yes, unrelated :)
Comment #15
marvil07 CreditAttribution: marvil07 commentedYes, from a PHP 5.3.9
Since I do not have a non *nix platform to test the exact case, I have just commented the /dev/urandom if and only left the elseif as if; and it works fine.
Comment #16
Dries CreditAttribution: Dries commentedCommitted to 8.x. We may be able to remove the version check before we release 8.x. We'll see.
Comment #17
jromine CreditAttribution: jromine commentedRe-rolled patch for Drupal 7.x.
Comment #18
philosurfer CreditAttribution: philosurfer commented#17 Works!
Drupal v7.12
PHP v5.3.5
Ubuntu v11.10
Comment #19
webchickYay for more randomness. :)
Committed and pushed to 7.x. Thanks!