Bug #71915 [Asn]: openssl_random_pseudo_bytes is not fork-safe

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: mathieuk@gmail.com +Summary: openssl_random_pseudo_bytes is not fork-safe Status: Assigned 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: 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. Previous Comments: ------------------------------------------------------------------------ [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 :) ------------------------------------------------------------------------ [2016-04-03 18:27:27] bukka@php.net > RAND_poll(): > https://github.com/openssl/openssl/blob/OpenSSL_1_0_1-stable/crypto/rand/rand_unix.c#L411-L417 Of course I meant just RAND_bytes as RAND_poll is irrelevant here (it's not called after fork). > RAND_bytes(): > https://github.com/openssl/openssl/blob/bfd53c32cd840ed381ba557c4de8f21e3615655c/crypto/rand/md_rand.c#L538-L552 Yeah I saw this but it's just in master (meaning it won't be available before 1.1) so it's not used with PHP for sure. I just don't get why you say that mixing time and PID is not safe. Of course PID alone is an issue, but actually mixing it with time seems fine and it is also mentioned so in the last paragraph on OpenSSL wiki that you linked: https://wiki.openssl.org/index.php/Random_fork-safety . So I will ask once again. What issue do you see with adding pid + time to entropy pool? ------------------------------------------------------------------------ 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