Re: Re: Improving mt_rand() seed

The proposed interface has way too many methods. Nor should CSPRNG extend the other PRNG interface. And also not the other way round, because then you can pass a non-CSPRNG to an only CSPRNG-accepting function. interface RandomInterface { public function getInt(int $min = NULL, int $max = NULL); // Int random public function getBytes(int $length); // Raw bytes public function getString(int $length, int $bits = 6); // String [0-9a-zA-Z,-]+ public function seed($seed = NULL); // No use with CS RNG, return TRUE always. public function getState(); // Return string representation PRNG state. No use with CS RNG, return NULL. public function setState(string $state); // Set PRNG state. No use with CS RNG, return TRUE always. public function getCount(); // No use with CS RNG, return 0 always. public function getReseedCycle(); // No use with CS RNG, return 0 always. public function setReseedCycle(int $count); // No use with CS RNG, return TRUE always. } You can already see from the comments that there are too many methods, e.g. "No use with CS RNG, return 0 always." I think the interfaces should look like that: interface PRNG { public function getInt(int $min = null, int $max = null): int; public function getBytes(int $length): string; } interface SeedablePRNG extends PRNG { public function seed(?? $seed): void; } interface CSPRNG { public function getInt(int $min = null, int $max = null): int; public function getBytes(int $length): string; } `getString()` should probably be a function accepting a PRNG / CSPRNG and a string containing possible chars. Maybe also an array, so you can use non-single-byte encodings like UTF-8. While getState and setState might be useful to save a state and continue somewhere in the future with the same state, it's not something for an interface. If you do something like that, you need the same RNG again, so those can be exposed only in specific implementations, they shouldn't be in the interface. `getCount` and `getReseedCycle`: What's the use case? I think those should be handled internally, nothing that should be user configurable. There are also a lot of functions, I don't think we should introduce them. We can just add static methods with a default instance to each RNG. Regards, Niklas

On 2017-02-02 05:47, Yasuo Ohgaki wrote:

I think this RFC is badly prepared. You're overhauling the whole mt_rand system in one go, but you're not doing it properly. There is no justification for breaking compability, not in 7.x and not even in 8.0 in my opinion. There's now three completely unrelated "issues": 1) You want to improve automatic seeding and GENERATE_SEED. You could just generate a 32-bit value from php_random_bytes and silently use the current as fallback; this solution was practically accepted already. You just waste time with your arguments about CSPRNG being sooooo important: everybody has already heard you, and most people seem to disagree. 2) You want to support long seeds. However, 2^32 is a lot of random states. It's enough for almost any legitimate MT use case. As was earlier discussed, adding this support to the global mt_srand is not practical. Anyone who really needs a longer seed should most probably also use a PRNG object to avoid cases where some internal function (say, shuffle) modifies the MT state by accident. 3) You want to use long seeds by default. This would be possible, as discussed earlier, by seeding the whole MT state buffer from a CSPRNG. However, you should consider also the possible performance impact of generating 2,5 kB from CSPRNG on each request/reseed. And again, 2^32 is probably enough already. FWIW, my Raspberry Pi kernel log has several lines about /dev/urandom not being properly seeded before the system is fully started, so using a CSPRNG is not guaranteed to work so well.

On 1 Feb 2017, at 22:47, Yasuo Ohgaki wrote:

I don't see in it any results of recent PRNG related discussions. I only see your ideas and a disregard for other opinions. # 1 The very first sentence "Current predictable PRNG, i.e. mt_rand() and rand(), produces very weak random values even produces non random values." is plain nonsense. mt_rand implements the Mersenne Twister 19937 with 32-bit seed. It's a standard algorithm that's been used widely. It does exactly what it's supposed to do. The idea that it produces "very weak random values" expresses your obsession with its use to produce unpredictable values, which it cannot possibly do. Nobody else is trying to make it do this. # 2 Purpose of object API You've misunderstood the idea of the "object-based PRNG interface". It's purpose is not to instrument mt_rand internals but to introduce alternative new generators. We concluded last year that there's very little we can do about mt_rand without breaking BC and/or making more mess, so it should be left to rot while we offer users something better. The new OOP API might offer things like MT, Xorshift, PCG, legacy LCGs, or whatever you fancy (maybe even MT-PHP :), distributions and utilities. # 3 This API Where to start? Have you any experience with the random APIs of any decent statistical packages? Take a look at Boost Random, R, ... Who wants a low-level interface to the MT algorithm's internals? # 4 "Fix" for abuse of mt_rand Your concern that people abuse mt_rand to get unpredictable values is valid. But I don't think your attempts to mitigate this are useful. Hypothetically, say we take a radical approach and make mt_rand draw from php_random_bytes by default on every call. How much good would it do? Unmaintained legacy apps are unaffected because nobody upgrades them from whatever PHP 4 or 5 they are on. Only apps that have stopped receiving security updates but that are nevertheless actively maintained to migrate them new major PHP versions can benefit. It's a niche we can better target with new APIs and education. W.r.t. unpredictable randoms, that's already done in 7.0. Hence I don't believe abuse of mt_rand is something we can fix by modifying its behavior. It's too late. But if you really insist on doing *something* to mt_rand then make its automatic seed use php_random_bytes and, if that fails, fall back to GENERATE_SEED. While I doubt it will make the world any safer, it is as effective as my hypothetical radical "fix" and should be harmless so long as you don't introduce new bugs. Tom

Hi all, On Thu, Feb 2, 2017 at 12:47 PM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:

I realized BC in PHP 7.1 is not so obvious. I added what's broken by PHP 7.1. I also made to raise exception for insane usage. Reseed cycle can be set up to 2^31. https://wiki.php.net/rfc/improve_predictable_prng_random RFC is written to implement to be minimal/enough to fix issues consistently. Adding new Random object or methods are easy if these are needed. Please note that RFC objectives are - Fix BC made by PHP 7.1. (For PHP 7.1, we may simply add new state for rand()/srand(), but this is out of scope) - Use MT rand optimal way - Keep it simple to be minimal - Keep consistency across APIs There are worry about CSPRNG overhead, reading 2500 bytes from CSPRNG on PHP 7.1-dev debug build took less than 0.00001 sec with my PC. [yohgaki@dev PHP-7.1]$ ./php-bin -d error_reporting=-1 -r '$s = microtime(true);for ($i=0; $i<1000; $i++) $n=random_bytes(2500); var_dump(microtime(true)-$s);' float(0.010347843170166) Since it will reseed 1 in 100, average overhead is less than 0.0000001 sec with debug build. 100 seems good choice. IMO. I don't mind to use larger reseed cycle default if it isn't too large. Regards,