Edit report at https://bugs.php.net/bug.php?id=71915&edit=1
ID: 71915
Updated by: bukka@php.net
Reported by: mathieuk at gmail dot com
Summary: openssl_random_pseudo_bytes is not fork-safe
-Status: Assigned
+Status: Closed
Type: Bug
Package: OpenSSL related
Operating System: any
PHP Version: 5.6.19
Assigned To: bukka
Block user comment: N
Private report: N
New Comment:
Automatic comment on behalf of bukka
Revision: http://git.php.net/?p=php-src.git;a=commit;h=0e2447cd11f4b72257e5d2609f923177e9736c3c
Log: Fix bug #71915 (openssl_random_pseudo_bytes is not fork-safe)
Previous Comments:
------------------------------------------------------------------------
[2016-04-12 19:43:54] bukka@php.net
I think that your approach is sufficient but I also think the same about my approach. I don't
really like adding global just for something that won't be needed when linked with OpenSSL
1.1+. As I said, just adding the time will be more compatible with what OpenSSL 1.1 does.
------------------------------------------------------------------------
[2016-04-12 09:03:09] mathieuk at gmail dot com
Could you elaborate on why you don't want to use RAND_poll() and/or why you think my approach
isn't sufficient? Seems to me using RAND_poll() would give the best result in terms of
providing random values. The value gets reset for each RINIT, which means it should work as expected
for SAPI's like FPM.
------------------------------------------------------------------------
[2016-04-12 09:03:05] mathieuk at gmail dot com
Could you elaborate on why you don't want to use RAND_poll() and/or why you think my approach
isn't sufficient? Seems to me using RAND_poll() would give the best result in terms of
providing random values. The value gets reset for each RINIT, which means it should work as expected
for SAPI's like FPM.
------------------------------------------------------------------------
[2016-04-10 17:27:03] bukka@php.net
I have been thinking about it and just came up with a bit different PR ( https://github.com/php/php-src/pull/1857 ). It
just adds time before each RAND_bytes which is sort of what OpenSSL 1.1 does. It means it adds a
time to the entropy for OpenSSL 0.9.x and 1.0.x. Both of them adds already PID so there
shouldn't be a need to add it again. I think that this is a more in line of what OpenSSL does.
It also doesn't require any extra lib dependency or adding globals.
------------------------------------------------------------------------
[2016-04-04 09:16:20] mathieuk at gmail dot com
Sorry bout that RAND_bytes() source link, I thought I linked to the 1.0.1 branch version of
md_rand.c but that link seems to go to master indeed.
For me it boils down to this:
* Using non-easily guessable seeds (/dev/urandom realistically being the best we have) seems better
than ones that are (pid/time), even if this information doesn't seem easily (ab)usable at this
time:
** PIDs are easily guessable
** Time is guessable, a machine's time may be reset, a website may even purposefully tell you
the time it created a blob of bytes etc.
* OpenSSL itself suggests using /dev/urandom over RAND_bytes() and only offers pid/time as
mitigations if that's not an option.
* RAND_poll() seems like an easy (cross-OS) way to mix in bytes from /dev/urandom or similar.
So, sure, time would help but given that we have an easily accessible way to create better
randomness than pid/time, it seems like the right way to go to help the people who use
openssl_random_pseudo_bytes() today.
That's about all I can give you on the matter :)
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=71915
--
Edit this bug report at https://bugs.php.net/bug.php?id=71915&edit=1