-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
std.rand: set DefaultCsprng to Gimli, and require a larger seed #6622
Conversation
There are still major issues with our current RNG interface. If RNG states have to be passed around, they will inevitably end up being copied. Identical output streams will be generated. That can be catastrophic. There is also currently no thread safety. That can be easily and immediately fixed, though. They are not When secure random numbers are required, maybe we currently shouldn't rely on applications to take care of the points above. That seems very dangerous. In C, at least on BSD, macOS and WASI (libc), the problem was solved with In Rust, Would it be worth having something similar? We currently have |
On the fact that applications shouldn't have to seed a CSPRNG, see this example code in a recent issue #6384 (comment) var seed = @intCast(u64, std.time.timestamp());
var csprng = std.rand.DefaultCsprng.init(seed); Timestamps, especially with a second granularity, are very predictable. Doing this in real-world applications is unfortunately still very common. While that can be totally fine for a regular RNG (to run Monte-Carlo simulations or whatever - and we still want manual seeding here), it is definitely not to generate any kind of unpredictable material. Zig could provide some safety here by having a default, secure, fast, global, thread-safe, fork-safe CSPRNG just like But as pointed out by @daurnimator , how would that work in a freestanding environment? Could an application define its own global CSPRNG? |
These deserve to be in an issue rather than be hidden away in a PR
This is another case of #591 for which we have #3803
Fork safety is really hard and both OS/architecture specific; I'm not sure we can provide a general solution here.
This is what zig should be implementing. When linking against libc (which is the stable interface on BSDs) we would straight up use For a static linux we should even be able to do it all without any syscalls: use
That would be a good addition IMO. |
`DefaultCsprng` is documented as a cryptographically secure RNG. While `ISAAC` is a CSPRNG, the variant we have, `ISAAC64` is not. A 64 bit seed is a bit small to satisfy that claim. We also saw it being used with the current date as a seed, that also defeats the point of a CSPRNG. Set `DefaultCsprng` to `Gimli` instead of `ISAAC64`, rename the parameter from `init_s` to `secret_seed` + add a comment to clarify what kind of seed is expected here. Instead of directly touching the internals of the Gimli implementation (which can change/be architecture-specific), add an `init()` function to the state. Our Gimli-based CSPRNG was also not backtracking resistant. Gimli is a permutation; it can be reverted. So, if the state was ever leaked, future secrets, but also all the previously generated ones could be recovered. Clear the rate after a squeeze in order to prevent this. Finally, a dumb test was added just to exercise `DefaultCsprng` since we don't use it anywhere.
Detecting forks can be complicated, but not so much if forking is done using functions from the Zig standard library. It may be reasonable to not provide much guarantees about forks being made by other means. |
Pinning is indeed a great proposal! It will indeed solve that issue, as well as for anything that shouldn't be cloned. |
pinning should nip this in the bud
I'd like to do this via wrapping the API, not by baking in thread safety into the random interface.
I don't recognize general purpose use of fork() as a valid use case. Redis tries to be clever and uses it as a way to dump a backup of data while still running, but ultimately it's a footgun because the copy-on-write nature ends up nondeterministically requiring up to double the memory of a machine already and intentionally at memory capacity. I've seen this cause an outage in production. It's such a strange and dubious API that I would even say someone who uses fork() without execve() is intentionally trying to copy their state, including random number generators. If they don't want that they shouldn't use fork().
I support the policy of making the path of least resistance the robust and secure way of doing things.
I'm in favor of a per-thread CSPRNG available for general use.
We could follow the pattern set by std.log, panic, and event loop, where there is a convenient default, but the root source file has the option to override the global CSPRNG. On freestanding targets this would have the effect of making use of it be a compile error, however providing the implementation in the root source file would resolve the errors. |
https://github.com/ziglang/zig/blob/master/lib/std/fs.zig has 2 callsites of std.crypto.getrandom that would benefit from this abstraction.
|
DefaultCsprng
is documented as a cryptographically secure RNG.While
ISAAC
is a CSPRNG, the variant we have,ISAAC64
is not. A 64 bit seed is a bit small to satisfy that claim.We also saw it being used with the current date as a seed, that also defeats the point of a CSPRNG.
Set
DefaultCsprng
toGimli
instead ofISAAC64
, rename the parameter frominit_s
tosecret_seed
+ add a comment to clarify what kind of seed is expected here.Instead of directly touching the internals of the Gimli implementation (which can change/be architecture-specific), add an
init()
function to the state.Our Gimli-based CSPRNG was also not backtracking resistant. Gimli is a permutation; it can be reversed. If the state was ever leaked, future secrets, but also all the previously generated ones could be recovered. Clear the rate after a squeeze in order to prevent this.
Finally, a dumb test was added just to exercise
DefaultCsprng
since we don't use it anywhere.