random_compat icon indicating copy to clipboard operation
random_compat copied to clipboard

Simplify random number implementation

Open timoh6 opened this issue 12 years ago • 12 comments

First of all, this is a "must have" addition to the PHP core. Great it is being worked on!

Looking at the current implementation, I think there are a few important design decisions we should re-think.

Now, the Random constructor looks like: __construct($secure = false). I think we should remove this "unnecessary" parameter. This is just a hassle for the end user (for no practical reasons).

I propose we should have just one good source for random bytes. That is /dev/urandom (and alike). No /dev/random, because it is basically not any better than /dev/urandom, but it is most probably unusable on an interactive web app scenario (due to its blocking nature).

I have got previously disagreements with this from various PHP folks, but I'd like to show some comments from expers on cryptography: https://news.ycombinator.com/item?id=6216101 http://security.stackexchange.com/a/3939

And finally, If you look at NaCl source code, you see it is relying /dev/urandom for its random numbers: http://nacl.cr.yp.to/index.html

If we use just one straight forward method to get random numbers, random_compat library code can be simplified alot. Which is critical for secure design and implementation.

We should trust the experts on this, and not roll our own.

This way, we can get rid of the constructor parameter, different random byte fetch implementations and the mergeBuffers() method.

Having just one "genNormal()" method is enough to do the job.

And finally, regarding the current implementation of genNormal(), it is critical to exit immediately if random bytes could not be generated (currently it falls back to a mt_rand(), which is no good).

Also, it is all fine if we return the bytes from the first source that was available (mcrypt_create_iv() or openssl_random_pseudo_bytes() or /dev/urandom). No need to fetch bytes from all of them.

Timo

timoh6 avatar Aug 23 '13 11:08 timoh6

It turned out that openssl_random_pseudo_bytes() might not be a reliable source of randomness (at least) due to the "PID wrapping bug". For more info, see http://emboss.github.io/blog/2013/08/21/openssl-prng-is-not-really-fork-safe/

However, I'm not sure does this practically affect the usage of openssl_random_pseudo_bytes() in PHP, but for the safety, I personally stopped relying on openssl_random_pseudo_bytes(), at least for now.

The underlying OpenSSL code is also a mess to read and understand due to its complexity, so this is as well a solid reason not to use it.

So my final proposal is to fetch the random bytes using mcrypt_create_iv (with URANDOM) or read straight from /dev/urandom (or possibly /dev/arandom).

Thoughts?

timoh6 avatar Aug 29 '13 11:08 timoh6

Wow am just a new bie in PHP but am working on this real world application for a company in my area and I cannot rely on some random numbers function I have been using, This work looks great but also more code, but my simple question is can I use this in my application to generate unique numbers for my invoice column. Thank you

mycota avatar Feb 07 '19 14:02 mycota

This library is really just for random numbers. For invoicing you could just generate a UUID or something right? Or increment a single counter? Using a random generator for unique numbers is probably not what you want given the little you've described.

You could use the random bytes function to generate some invoice ID like in test.php. If the size of the random string was large enough that might prevent duplicates. Why not just use the standard library though if this is just for invoice numbers? http://php.net/manual/en/function.uniqid.php

MattSurabian avatar Feb 07 '19 14:02 MattSurabian

Please am not getting point of yours " it maybe should give you pause. " Yes I have, but do you mean i can you that instead of the the main one?

On Thu, Feb 7, 2019 at 8:05 PM Matthew Surabian [email protected] wrote:

If you're just looking for invoice random numbers you could use this library, but it has been several years since it has been worked on so if you need something "secure" it maybe should give you pause.

Have you seen the example in test.php https://github.com/ircmaxell/random_compat/blob/master/test.php#L10-L12?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ircmaxell/random_compat/issues/2#issuecomment-461444542, or mute the thread https://github.com/notifications/unsubscribe-auth/ATAymCbc44Bu-t6Ec-Yg8xbWqciGUQUeks5vLDnCgaJpZM4A7gD4 .

mycota avatar Feb 07 '19 14:02 mycota

Sorry I removed my previous comment and updated it. Given what you're describing you don't really need or want random numbers, you just want unique numbers to identify the invoice. If you don't care about the format why not just use a unix timestamp?

MattSurabian avatar Feb 07 '19 14:02 MattSurabian

Okay Thank you big Man but are talking about this $d = date("Ymds"); because it is only the seconds that get change which I think might repeat, else I this it good and it even tell the date of the invoice.

On Thu, Feb 7, 2019 at 8:14 PM Matthew Surabian [email protected] wrote:

Sorry I removed my previous comment and updated it. Given what you're describing you don't really need or want random numbers, you just want unique numbers to identify the invoice. If you don't care about the format why not just use a unix timestamp?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ircmaxell/random_compat/issues/2#issuecomment-461448602, or mute the thread https://github.com/notifications/unsubscribe-auth/ATAymOnTjPPtG9A5Ig-Y-CtUf1FpQOb0ks5vLDvLgaJpZM4A7gD4 .

mycota avatar Feb 07 '19 14:02 mycota

No, not data("Ymds"). You want epoch time. So either time() or microtime().

http://sandbox.onlinephpfunctions.com/code/2195cb359a7d29a8b2d95dd6bdac2c6cf1f38c6e

Time will update every second, microtime will do microseconds. Depending on the throughput of your system you might need the extra precision of microtime.

MattSurabian avatar Feb 07 '19 15:02 MattSurabian

Microtime example: http://sandbox.onlinephpfunctions.com/code/189032361e67a69038fc900d063b3408841ef5b9

MattSurabian avatar Feb 07 '19 15:02 MattSurabian

Okay thank you soo much for the help I really appreciate it

On Thu, Feb 7, 2019 at 8:47 PM Matthew Surabian [email protected] wrote:

Microtime example: http://sandbox.onlinephpfunctions.com/code/189032361e67a69038fc900d063b3408841ef5b9

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ircmaxell/random_compat/issues/2#issuecomment-461464153, or mute the thread https://github.com/notifications/unsubscribe-auth/ATAymODGl0dEYCumtzCqspG1n6oKt4Jsks5vLEOSgaJpZM4A7gD4 .

mycota avatar Feb 07 '19 15:02 mycota

No problem, I forgot I was watching this repo! Well everything worked out I guess. Good luck

MattSurabian avatar Feb 07 '19 15:02 MattSurabian

Not really and am thinking of using the last inserted auto increment to use But I really wanted to use your random function because it gives me 10 digits but if it may repeat itself that is wanted to know and necessarily the order of the numbers or tracking.

On Thu, Feb 7, 2019 at 8:58 PM Matthew Surabian [email protected] wrote:

No problem, I forgot I was watching this repo! Well everything worked out I guess. Good luck

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ircmaxell/random_compat/issues/2#issuecomment-461469570, or mute the thread https://github.com/notifications/unsubscribe-auth/ATAymJJJoGQd4_o9cDJCujUhoiT_hvBkks5vLEYkgaJpZM4A7gD4 .

mycota avatar Feb 07 '19 15:02 mycota

Time and Microtime are counting the seconds since a specific date in 1970 (January 1) so it will never repeat and will always grow.

MattSurabian avatar Feb 07 '19 16:02 MattSurabian