Conversation
|
If this is intended to allow people to plug in a CSPRNG, why not just add a builtin function random_bytes that does that? |
|
Wouldn't that have the exact same problem i described above?
How would "just add[ing] a builtin function [like] random_bytes" make List::Util' end up using my random number generator? What would your version of look like? |
No, it's not. Allowing the global RNG to be overridden can open up all sorts of problems, especially if people are using it for a CSPRNG and forget to override it locally, so someone else can override it (intentionally or not) and break things. Also, Also, GitHub isnt working so I cant look at
However, SHA256 is designed as a message digest algorithm, not as a stream cipher, and there will in subtle flaws in using that as a stream There are techniques for converting hash algorithms into block ciphers, e.g. Luby-Rackoff, but then you may as well as a CSPRNG algorithm for generating a stream of bytes, e.g. ISAAC or ChaCha (which is used by recent Linux /dev/urandom). But again, generating random floats and generating random bytes are two entirely different beasts. |
53ff922 to
d3f8bc6
Compare
I look at it as a solution, but of course yes, someone could if they wished use it irresponsibly to do dumb things. But stopping people from using something that solves real problems isn't the perl way. We might as well remove 'unlink' because someone could do something dumb with it. I consider the base design of rand() as broken. And perl has never defined anything that replaces that brokenness. If someone writes code which uses rand(), then that code becomes hugely breakable because everything is using shared state. So if I modify my code to use rand() when previously it did not, I might break your tests, because your use of srand(123) call which you expect to make your code to one thing is now being mixed with calls from me to getting entropy you didn't expect me to use. If we both want to use rand() then we have a problem. At least with ${^RNG} I have options. I can globally fix all the code that uses rand() on my system to use a different function if i want. I can make specific pieces of code use designated and well seeded independent rngs for when they operate, I can do a bunch of things I cant do right now. For me that is the point, i can do things and solve problems I couldn't solve without this patch.
Sorry, what are you talking about?
Hashing with a cryptographic grade hash the seed concatenated to a counter which is incremented each time will provide a usable stream of secure random bits. If you think otherwise then prove it and report it to NIST. FWIW, I have actually used this algorithm (actually with SHA-1) at large scale and we never found any bias. We also ran Test01 and Dieharder on the code for days on end without any sign of RNG weakness. Sure there are way way way better ways to do a secure RNG, a gold standard being AES but this is basically demo code, using SHA256 for any real RNG work is ridiculous, you'd die of old age before you got anything done. Maybe the code needs a note to that effect. |
|
How many bits worth of decimal do we have after the |
If the seed isn't already good, then it's not secure. Too many Perl modules have used similar techniques, and they are easily broken.
If you re going to cite the NIST, then please use something based on NIST recommendations, which are more complicated thansimply incrementing a counter:
There is no need for that kind of construction on most systems nowadays. (A possible reason hashes were chosen instead of stream ciphers was because hashes were not subject to US export controls in the 1990s.)
It isn't just about bias. It's about predictability. Rainbow tables for cracking this kind of construction are practical, and the hardware for attacking these is much more powerful and accessible. CPANSec has been issuing CVEs for the type of construction that you are using. Putting this in core will give new life to this zombie algorithm that CPANSec has been trying to kill off. |
|
AFAIK it's already possible to override rand and srand the usual way: I'm not sure an additional hook is needed. |
| if (!sv_isobject(provider) | ||
| && (!SvROK(provider) || SvTYPE(SvRV(provider)) != SVt_PVCV)) | ||
| croak("${^RNG} must be an object, a CODE reference, or undef"); | ||
| return (NV)S_rng_uv(aTHX_ provider) / ((NV)UV_MAX + 1.0); |
There was a problem hiding this comment.
I'd be inclined to have the RNG provider return an NV directly.
UV can be 32-bits.
You may want to look at #24294 (comment) to see why the simple division may not be the best choice.
One big problem I see is that rand() becomes a whole lot slower with an object in ${^RNG}, even with just the method call overhead.
| { | ||
| local ${^RNG} = My::RNG->new($seed); | ||
| my $value = rand(100); | ||
| srand(42); | ||
| } |
There was a problem hiding this comment.
You can mention this, but I'd really hope you'd promote direct calls over this, ie. that new code accepts an RNG object instead of rand() hook hackery.
There was a problem hiding this comment.
Especially since RNG is in dist/, so meant to work with older perls.
| * This is the two-dimensional PCG-XSH-RR construction. The base | ||
| * generator has 64 bits of state and produces 32-bit values. Two extra | ||
| * 32-bit values form the extension array, giving the complete generator | ||
| * 128 bits of state without requiring 128-bit arithmetic. | ||
| */ | ||
| typedef struct { | ||
| UV state; | ||
| U32 extension[2]; | ||
| UV initial; | ||
| } pcg_data; | ||
|
|
||
| #define PCG_MULTIPLIER ((UV)0x5851f42d4c957f2dULL) | ||
| #define PCG_INCREMENT ((UV)0x14057b7ef767814fULL) |
There was a problem hiding this comment.
Perl doesn't require 64-bit UVs at this point. This should probably use U64.
| RETVAL = ((UV)pcg_next_data(&state) << 32) | ||
| | pcg_next_data(&state); |
There was a problem hiding this comment.
Do you care whether the first call (in time order) to pcg_next_data() is the high bits or the low bits of the result?
| value = (items < 2 || !SvOK(seed) | ||
| || (SvPOK(seed) && !SvIOK(seed) && !SvNOK(seed))) | ||
| ? 0 : SvUV(seed); |
There was a problem hiding this comment.
Checking flags before fetching magic for the seed SV.
There was a problem hiding this comment.
The requirement for integer representation seems a bit un-perlish too.
It also disallows overloads.
| pcg_store(SV *state, const pcg_data *value) | ||
| { | ||
| STRLEN len; | ||
| char *bytes = SvPV_force(state, len); |
There was a problem hiding this comment.
You use SvPVbyte() above, should this use SvPVbyte_force()?
There was a problem hiding this comment.
It also checks for magic, which probably shouldn't happen here.
| U8 numeric_seed[sizeof(UV)]; | ||
| const U8 *seed_bytes; | ||
| STRLEN seed_len; | ||
| U64 extension; |
There was a problem hiding this comment.
extension declaration should be combined with the assignment below.
There was a problem hiding this comment.
Actually, this is in dist/, so presumably it's intended to work on older perls (though it doesn't at this point), so perhaps you can't depend on the 26 year old C99.
| The provider may also offer C<rand>, C<rand01>, and | ||
| C<rand01_callback> methods as convenience interfaces. These are ordinary | ||
| module methods; Perl's built-in C<rand> uses C<rand_uv> directly. |
There was a problem hiding this comment.
What do these other methods do?
6212566 to
083f2c4
Compare
Historically the random number generator used by perl has evolved over
time, in the early days it was the system RNG, then later we rolled our
own version drand48 for cross platform stability. But these days
drand48 has a bunch of flaws so it would be nice to be able to use
something better.
However this is not so easy. Most code just calls rand() because it is
the easiest thing to do. Most people dont bother adding support for
their own hook, or burdening their consumers with having to provide an
RNG, especially as there is no standard for doing so. Some code maybe
ends up with support for an RNG hook, like List::Util did, but IMO such
cases are a rarity, and a proliferation of slightly different hooks for
this would not be helpful.
In theory one can override CORE::rand() and CORE::srand() before a
module is loaded, and thus change the RNG, but the reality is there is
no good way to overload srand()/rand() in another packages namespace.
(So when people say "just put your pet RNG on CPAN" it is kind of a cop
out, that RNG is likely to never really be useful as it just wont get
used by most code.)
Similarly we can't really solve this problem with a new builtin function
that implements a new rng. Such a builtin wouldn't be used by anything
and would only be useful to new code that was written to use it. It
would have the same problems as the current rand() and it would not be
surprising if in 10 years when the PCG has been replaced by something
else we will be back to square one, but now with two problems. :-)
At the same time we don't really want to change Perls internal code to
use something other than drand48(). There is lots of code that expects
that doing srand(1234) will produce test results that look a specific
way, so changing cores RNG would cause all kinds of unfortunate drama
through the ecosystem.
This patch takes a different approach. Let the user hook rand() to use
an alternative, so that the user can choose which algorith, and how it
was seeded, etc. In this model ${^RNG} is used as a hook for
rand() and srand(). If ${^RNG} contains an object then it uses the
objects rand_uv() and srand() methods to do its thing. If it
contains an unblessed subref then it calls it with no arguments for
rand() and with arguments for srand() (srand(undef) is defined to be
the same as srand()).
This is wired into the macro Drand01() which despite its name and
appearance is actually a pTHX style macro, but which actually hides the
fact by access what looks like a global var, but is actually an
interpreter var under threads, thus hiding that it does operate on aTHX
just not directly. This allows us to redirect so that modules like
List::Util will still respect the hook even though they bypass pp_rand()
for most of their random calls.
I thought about using ${^HOOKS}{RNG} instead of ${^RNG}, but for the
time I decided to just use ${^RNG} as it feels better to do
somethinglike:
local ${^RNG} = RNG::PCG->new();
than it does to do:
local ${^HOOKS}{RNG}
and it allows the rng internals to avoid the hash lookup. Although
currently it is not optimizing the ${^RNG} package var fetch, and the
var is not magic in any way so the ${^RNG} hook is not being stored in
an interpreter var so the current implementation could be sped up we
wished. IMO it is preferable to keep this as simple as possible.
As part of this patch I have added a new dist/RNG which contains XS code
to implement the two dimensional PCG-XSH-RR 64/128, which allows for 128
bit state with only 64 bit or 32 operations. This seems to be the state
of the art RNG these days.
I also implemented an RNG::SHA which uses SHA256 to produce the stream
of random values, most just as a proof of concept.
I chose to make RNG use a UV as its base currency, so rand() calls
${^RNG}->rand_uv() which returns a UV, which is then converted into
whatever the user requested. I did it this way so that any floating
point operations or conversion to floating point operations could be
performed by the C code. It feels like a more flexible foundation
than using a float. /me waves hands.
Modules like List::Util which bind to Drand01() at the C level still
call the callback, so even without using List::Util's hook you can
override it just like any other perl code.
This effectively means that every piece of code that uses rand() to
do randomness now has the same capability as was hand hacked in
List::Util long ago. On callback to rull them all sort of thing. :-)
083f2c4 to
176404c
Compare
Historically the random number generator used by perl has evolved over time, in the early days it was the system RNG, then later we rolled our own version drand48 for cross platform stability. But these days drand48 has a bunch of flaws so it would be nice to be able to use something better.
However this is not so easy. Most code just calls rand() because it is the easiest thing to do. Most people dont bother adding support for their own hook, or burdening their user with providing an RNG, especially as there is no standard for doing so. Some code maybe ends up with support for an RNG hook, like List::Util did, but IMO such cases are a rarity, and a proliferation of slightly different hooks for this would not be helpful.
In theory one can override CORE::rand() and CORE::srand() before a module is loaded, and thus change the RNG, but the reality is there is no good way to overload srand()/rand() in another packages namespace. (So when people say "just put your pet RNG on CPAN" it is kind of a cop out, that RNG is likely to never really be useful as it just wont get used by most code.)
At the same time we dont really want to change Perls internal code to use something other than drand48(). There is lots of code that expects that doing srand(1234) will produce test results that look a specific way, so changing cores RNG would cause all kinds of unfortunate drama through the ecosystem.
This patch takes a different approach: ${^RNG} is used as a hook for rand() and srand(). If ${^RNG} contains an object then it uses the objects rand_uv() and srand() methods to do its thing. If it contains an unblessed subref then it calls it with no arguments for rand() and with arguments for srand() (srand(undef) is defined to be the same as srand()).
This is wired into the macro Drand01() which despite its name and appearance is actually a pTHX style macro, but which actually hides the fact by access what looks like a global var, but is actually an interpreter var under threads, thus hiding that it does operate on aTHX just not directly. This allows us to redirect so that modules like List::Util will still respect the hook even though they bypass pp_rand() for most of their random calls.
I thought about using ${^HOOKS}{RNG} instead of ${^RNG}, but for the time I decided to just use ${^RNG} as it feels better to do somethinglike:
than it does to do:
and it allows the rng internals to avoid the hash lookup. Although currently it is not optimizing the ${^RNG} package var fetch, and the var is not magic in any way so the ${^RNG} hook is not being stored in an interpreter var so the current implementation could be sped up we wished. IMO it is preferable to keep this as simple as possible.
As part of this patch I have added a new dist/RNG which contains XS code to implement the two dimensional PCG-XSH-RR 64/128, which allows for 128 bit state with only 64 bit or 32 operations. This seems to be the state of the art RNG these days.
I also implemented an RNG::SHA which uses SHA256 to produce the stream of random values.
I chose to make RNG use a UV as its base currency, so rand() calls ${^RNG}->rand_uv() which returns a UV, which is then converted into whatever the user requested. I did it this way so that any floating point operations or conversion to floating point operations could be performed by the C code. It feels like a more flexible foundation than using a float.
Modules like List::Util which bind to Drand01() at the C level still call the callback, so even without using List::Util's hook you can override it just like any other perl code.
TODO: fill description here