When available, prefer php's built in openssl_random_pseudo_bytes() to /dev/urandom. Test for functionality and fallback to /dev/urandom if necessary to get data. Simplifies the code a bit, and is somewhat more functional as running inside a chroot is now possible.
- Group Reviewers
- rPHU13ad06352d84: Default to openssl_random_pseudo_bytes instead of /dev/urandom.
Add unit test for readRandomBytes() Would be nice to have someone test this on windows. Working on ubuntu at the moment.
Unit Tests Skipped
I googled for openssl_random_pseudo_bytes and clicked at least one link without seeing any warnings that it was completely broken.
I also ran it manually a few times on my machine and looked at the output, which sure looked random to me.
These diligent actions of careful research have given me a high degree of confidence that this does not compromise anything.
I'll fix the two tiny whitespace/comment things in the pull.
We could remove this, since we no longer use it.
If the caller requests readRandomBytes(0), and the fread fails, the strlen() of false is 0, so this check will pass, and then we'll return false instead of '' (empty string).
I think this is fine since readRandomBytes(0) is pathological, I'm just proving that I'm paying attention.
The old implementation had the same problem anyway.
This string -- and thus the error message -- includes a huge amount of whitespace and a newline. Better as:
'a b c '. 'd'
Can you give me a diff against origin/master? It looks like there's some phutil_is_windows() removal stuff behind this on your branch. I can manually sort that out but it's probably easier to just have a full patch here.
arc diff origin/master