add system random to stdlib: std/sysrand

[ringabout]

Cryptographically secure pseudorandom number generator

https://en.wikipedia.org/wiki/Cryptographically_secure_pseudorandom_number_generator

Many popular and mature programming languages put system random on their standard library

• Python
  https://www.python.org/dev/peps/pep-0506
  https://www.python.org/dev/peps/pep-0524
  https://docs.python.org/3/library/os.html#os.urandom
  https://docs.python.org/3/library/secrets.html

• Rust
  https://github.com/rust-lang/rust/blob/master/library/std/src/sys/unix/rand.rs

• Go
  https://golang.org/pkg/crypto/rand
  https://sourcegraph.com/github.com/golang/go/-/blob/src/internal/syscall/unix/getrandom_linux.go

• JS(WebAPI)
  https://developer.mozilla.org/en-US/docs/Web/API/Crypto/getRandomValues

• Nodejs
  https://nodejs.org/api/crypto.html#crypto_crypto_randomfillsync_buffer_offset_size

• Crystal
  https://github.com/crystal-lang/crystal/blob/master/src/random/secure.cr

• Zig
  https://github.com/ziglang/zig/blob/master/lib/std/crypto/tlcsprng.zig

• Java/Scala/Kotlin
  https://docs.oracle.com/javase/7/docs/api/java/security/SecureRandom.html

• C#
  https://docs.microsoft.com/en-us/dotnet/api/system.security.cryptography.rngcryptoserviceprovider?view=net-5.0

• PHP 7
  random_bytes — Generates cryptographically secure pseudo-random bytes
  https://www.php.net/manual/en/function.random-bytes.php

• Ruby
  https://ruby-doc.org/stdlib-2.5.1/libdoc/securerandom/rdoc/SecureRandom.html

• Dart
  https://api.dart.dev/stable/2.10.5/dart-math/Random/Random.secure.html

• Erlang
  https://erlang.org/doc/man/crypto.html#strong_rand_bytes-1

---

libsodium
https://github.com/jedisct1/libsodium/blob/master/src/libsodium/randombytes/sysrandom/randombytes_sysrandom.c

blog:
https://paragonie.com/blog/2016/05/how-generate-secure-random-numbers-in-various-programming-languages

implementation:

future work

• refs add system random to stdlib: std/sysrand #16459 (comment)

It need not hold up this PR being merged, but if this is merge-worthy then I think it is probably better to use this instead of a high resolution clock in lib/pure/random.nim:randomize.

oids can also benefits from this PR(use a secure seed).

If stdlib can accept UUID, the module will benefit from this PR too.

It can also be used to generate salts, keys or initialization vectors.

[timotheecour]

LGTM; @Araq how about we merge this now, and we can do followup PR's to fix the remaining bugs; since it's a new module it's fine IMO to do this so we can make faster progress (maybe with a note: experimental module, unstable API)

[Clyybber]

LGTM; @Araq how about we merge this now, and we can do followup PR's to fix the remaining bugs; since it's a new module it's fine IMO to do this so we can make faster progress (maybe with a note: experimental module, unstable API)

We cannot merge something with potential security issues/bugs like this, especially when it should cryptographically secure. Let's fix the bugs instead of rushing.

[timotheecour]

We cannot merge something with potential security issues/bugs like this, especially when it should cryptographically secure. Let's fix the bugs instead of rushing.

What I'm arguing for here is to allow iterating in a more effective way on new modules (or even new APIs in existing modules), including this one. This could be as simple as:

# in std/sysrand.nim
when not defined(nimExperimentalSysrand): static: doAssert "experimental module, unstable API"
# rest of code ...

so that:

• this module remains hidden by default, but people can easily try it in devel with explicit flag -d:nimExperimentalSysrand; more eyeballs => higher likelyhood to find bugs/design issues
• more than 1 person can easily contribute to it
• no need to constantly rebase long running PR's and resolve conflicts since they'd be merged faster
• we can more freely iterate without fear of breaking changes for regular devel users

Unlike the proposal discussed in nim-lang/RFCs#310, there'd be no need for std2 namespace, and no need for introducing an experimental namespace (which has the issues of causing problems when module is moved), since the only difference once module is stabilized is that the flag -d:nimExperimentalFoo would not be needed anymore;

This is essentially a lightweight form of --experimental:foo that can be used for new (non-trivial) modules/API's.
(refs timotheecour#575)

[dom96]

Beautiful, a perfect example of something that should be in the stdlib.

I'm mostly writing this because I feel like I very often reject PRs for new modules to the stdlib, it's nice to accept a case that I think does deserve to be in the stdlib :)

The API is also solid and docs look good. Implementation we can always easily improve as long as API doesn't need to change so IMO we can merge this now tbh.

[dom96]

We cannot merge something with potential security issues/bugs like this, especially when it should cryptographically secure. Let's fix the bugs instead of rushing.

Yes, we can. We just need to make it clear that the module is new, all this needs is a warning in the docs, something like:

Warning: This module was added in Nim 1.6. If you are using it for crypto purposes then keep in mind that so far this has not been audited by any security professionals, as such may not be secure.

In fact, we should keep this warning until a reputable company does audit it and then link to the audit in the docs.

[timotheecour]

Yes, we can. We just need to make it clear that the module is new, all this needs is a warning in the docs, something like:

indeed. This is exactly what timotheecour#575 is about: avoiding analysis paralysis where a PR slowly collects bitrot over time, and needs constant rebasing. More eyes = more bugfixes.
We can hide this behind when defined(nimExperimentalSysrandom) in a next PR to allow fixing API in meantime as needed, until it stabilizes.

reputable company does audit it and then link to the audit in the docs

it can't hurt but I'm not sure if it buys you much in practice beyond a marketing argument; this module depends on some parts of stdlib (system.nim) and compiler for correctness; that one-time audit wouldn't cover those, and even if it did it wouldn't guard against regressions.

[euantorano]

This is completely broken on NetBSD at the moment (and likely Dragonfly, though I haven’t tested there yet).

Note that there are a few existing Nimble packages that aim to achieve the same thing, like:

• https://github.com/euantorano/sysrandom.nim/blob/master/src/sysrandom.nim
• https://github.com/cheatfate/nimcrypto/blob/master/nimcrypto/sysrand.nim

I would almost always base this kind of code on the fantastic libsodium, who’s approach can be found here: https://github.com/jedisct1/libsodium/blob/master/src/libsodium/randombytes/sysrandom/randombytes_sysrandom.c

I’m very happy to see this module get added to the stdlib though :)

[timotheecour]

@euantorano PR welcome; also regarding running netbsd in CI

for netbsd, see also this comment #16459 (comment) which has some potential solutions

[ringabout]

see 18c24eb#r47295760

[euantorano]

@euantorano PR welcome; also regarding running netbsd in CI

for netbsd, see also this comment #16459 (comment) which has some potential solutions

I have some nightly tests running here already: http://jenkins.euantorano.co.uk/job/Nim/124/console - it just runs on a schedule rather than on every PR/commit which is good enough for now until we get some of the failures fixed.

see 18c24eb#r47295760

Yep, looks like it. It was broken when I commented, likely due to the nature of it running nightly.