Skip to content
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

Secure random documentation #32957

Closed
wants to merge 1 commit into from

Conversation

chethega
Copy link
Contributor

One possible resolution for #32954 is to simply document that people should use Random.RandomDevice().

This is not just "improve some docs", it is an actual recommendation, and a commitment that we will care about security in our wrapper for the OS random.

Hence, this needs more review than usual docfixes.

To start with, the windows, linux, freebsd, openbsd, netbsd, macos all promise secure random. Are there any crazy supported systems that are known to fail here?

Am I understanding right that libuv ensures that this is thread-safe? What happens on older julia versions if multiple threads read from the same random device? Good (all works), fail (segfault) or catastrophic fail (same random emitted multiple times)?

Then there is the buffering by libuv. Are there any issues?

@chethega
Copy link
Contributor Author

Ok, we cannot use this. On 1.1 linux:

julia> using Random
julia> arr=zeros(UInt64, 10);
julia> rd=Random.RandomDevice();
julia> Threads.@threads for i=1:10
       arr[i]=rand(rd, UInt64)
       end
julia> arr
10-element Array{UInt64,1}:
 0xaf99dce00b214c73
 0xafd98286cacb8afd
 0x0d7dcbe7143a3a71
 0x95c456bac0ddad46
 0xc89533178f0991ad
 0x0d7dcbe7143a3a71
 0xb99af106d5b27194
 0xafd98286cacb8afd
 0x455eacdc763a93e4
 0xc89533178f0991ad

Whatever we recommend here, people will use it on old julia versions as well. And this is catastrophic failure, far worse than a segfault.

Any ideas what else to recommend?

@ViralBShah ViralBShah added the domain:docs This change adds or pertains to documentation label Aug 19, 2019
@ViralBShah ViralBShah added the domain:randomness Random number generation and the Random stdlib label Aug 19, 2019
@JeffBezanson
Copy link
Sponsor Member

On v1.3+ that should work. So maybe the idea of adding a new name const CSRNG = RandomDevice() is ok? We could also add a shim for older versions where CSRNG adds a lock.

@PallHaraldsson
Copy link
Contributor

PallHaraldsson commented Aug 19, 2019

Can the boilerplate text be shortened?

Could gen_nonce(len=128) be defined, to only recommend calling it?

I assume 128-bit is the current recommendation, and I'm also thinking will that always be enough?

It seems 0xafd98286cacb8afd repeated is the problem; because of rand non-threadsafe, so you do not want to recommend using that anyway... [The repeated 0xaf for 0xaf99dce00b214c73 is I guess unrelated, just statistically likely and not to be worried about.]

I think the unlimited version is always ok (and Windows doesn't have a choice, I assume it's non-urandom-like). I.e. I only see the non-bool version for non-Windows:

RandomDevice(unlimited::Bool=true) = new(open(unlimited ? "/dev/urandom" : "/dev/random"), unlimited)

const CSRNG = RandomDevice() takes this choice away from you, but it may not be important to have, at least for get_nonce().

https://linux.die.net/man/4/urandom

As a general rule, /dev/urandom should be used for everything except long-lived GPG/SSL/SSH keys.

https://en.wikipedia.org/wiki//dev/random

A counterpart to /dev/random is /dev/urandom ("unlimited"[5]/non-blocking random source[4]) which reuses the internal pool to produce more pseudo-random bits. This means that the call will not block, but the output may contain less entropy than the corresponding read from /dev/random. While /dev/urandom is still intended as a pseudorandom number generator suitable for most cryptographic purposes, the authors of the corresponding man page note that, theoretically, there may exist an as-yet-unpublished attack on the algorithm used by /dev/urandom, and that users concerned about such an attack should use /dev/random instead.[4] However such an attack is unlikely to come into existence, because once the entropy pool is unpredictable it doesn't leak security by a reduced number of bits.[6]

Also interesting: https://en.wikipedia.org/wiki/Lavarand

@StefanKarpinski
Copy link
Sponsor Member

This is nothing specific to RandomDevice: neither I/O nor random number generation are thread safe prior to 1.3. I'm not sure why the example code here attempts to use threads with both I/O and random number generation. If you don't try to do this on many threads then it works just fine.

It's also not terribly verbose or biolerplateful to get a securely filled random array:

julia> rand(RandomDevice(), UInt64, 10)
10-element Array{UInt64,1}:
 0x9546c9a14fd83fdf
 0x08e6ce584f58e258
 0xf5b9c89362242c2a
 0x202adcafcc12be2e
 0x11340a32242058ff
 0xe642d98d0486a785
 0xd89e2721abb45e72
 0xd7768c51b766fcc7
 0x15ddfa425e52c436
 0xb57b1c3245a6bad8

@chethega
Copy link
Contributor Author

Could gen_nonce(len=128) be defined, to only recommend calling it?

Sure we could. But secure random is relevant for more than just gen_nonce, also e.g. randstring(CSPRNG, 32).

It seems 0xafd98286cacb8afd repeated is the problem; because of rand non-threadsafe, so you do not want to recommend using that anyway...

Exactly. The secure random has three aspects:

  1. What we provide and recommend going forward (i.e. from 1.4 onwards, possibly from 1.3 onwards depending on what we recommend and whether we make an exception for the feature freeze that has already passed)
  2. What we backport explicitly, in terms of documentation.
  3. What we backport implicitly.

With implicit backporting I mean the following: People will read the newest docs, copy-paste into their application that runs on 1.0, and something happens. This something should either be good or fail loud and hard. Having copy-pastable code that leads to silent catastrophic failure on wrong julia versions would be suboptimal.

One could consider including/exporting const CSPRNG = RandomDevice() on 1.3 (or 1.4 if feature-freeze makes no exception for single-line definitions), and backporting documentation that recommends

import Random
@static if isdefined(Random, CSPRNG)
    gen_nonce() = rand(Random.CSPRNG, UInt128)
else
    const CSPRNGs = [Random.RandomDevice() for i=1:Threads.nthreads()]
    gen_nonce() = rand(CSPRNGs[Threads.threadid()], UInt128)
end

and potentially also backport the initialization (const CSPRNGs = [RandomDevice() for i=1:Threads.nthreads()] in the Random module) to 1.0, 1.1, 1.2. We could also consider backporting locks on RandomDevice; I don't know whether that is too big a change for bugfix releases.

Then one should also explain the pitfalls: Do not to call bad_gen_nonce()=rand(Random.RandomDevice(), UInt128), because it will run out of file descriptors on unix-like systems, and beware of const CSPRNG = Random.RandomDevice() because of multithreading. And maybe also warn that the random stream is buffered (with natural implications for random state after snapshot resumption of virtual machines).

This is nothing specific to RandomDevice: neither I/O nor random number generation are thread safe prior to 1.3. I'm not sure why the example code here attempts to use threads with both I/O and random number generation. If you don't try to do this on many threads then it works just fine.

I am aware. The example I posted is terrible and wrong code, and people who write such a thing should be ashamed. It's just that, for the specific issue of crypto/security, I'd like to be more misuse resistant. A security-footgun is worse than a harmless-bug-footgun.

@rfourquet
Copy link
Member

rfourquet commented Aug 20, 2019

As a side note, there is already an open PR for a global RandomDevice() object (called RANDOM_DEVICE instead of CSPRNG), see #27936. It was more tricky to implement than a simple const CSRNG = RandomDevice(), I guess because of how stdlib modules are precompiled (but I can't recall the details).

@rfourquet
Copy link
Member

Another interesting link for /dev/urandom vs /dev/random is https://www.2uo.de/myths-about-urandom. The take-away (AFAICT) is: always use urandom except if you need random numbers so early in the machine boot process that there are'nt enough produced entropy bits yet (like less than 128 bits). I think by the time julia is started, we are safe on this front, and I believe it would fine to remove the unlimited keyword from RandomDevice, which is not documented.

@@ -26,6 +26,11 @@ unbounded integers, the interval must be specified (e.g. `rand(big.(1:6))`).
Additionally, normal and exponential distributions are implemented for some `AbstractFloat` and
`Complex` types, see [`randn`](@ref) and [`randexp`](@ref) for details.

!!! note
The `MersenneTwister` used by unqualified `rand` or `randstring` calls is not suitable for
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be not clear for everyone what is meant by "unqualified" here, what about:

The MersenneTwister object used implicitly by unqualified rand or randstring calls is ...

`bad_gen_nonce() = rand(Random.RandomDevice(), UInt128)`, in order to not cause performance degradation and an eventual
`ERROR: SystemError: opening file "/dev/urandom": Too many open files`.

The `RandomDevice` reads and caches `/dev/urandom` on Unix-derived systems, and uses appropriate `Ntsecapi.h` calls on Windows.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will trust you on this fact concerning Windows, but I infer from the name of the file that it's a CSPRNG?

@@ -66,6 +71,18 @@ Random.MersenneTwister
Random.RandomDevice
```

## Cryptographically secure random numbers

The Julia standard library does not provide a native cryptographically secure random number generator. For most applications,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By "native", you mean written in Julia? I find it's easy to interpret this sentence as "you can't get CSPRNG" from the Random module, which seems to be contradicted in the next sentence...


The Julia standard library does not provide a native cryptographically secure random number generator. For most applications,
we recommend to use the operating system `RandomDevice()`. This should be initialized via a module-level
`const SECURE_RANDOM = Random.RandomDevice()` and subsequently used via e.g. `gen_nonce() = rand(SECURE_RANDOM, UInt128)`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only by the name SECURE_RANDOM we can infer that indeed RandomDevice is secure, it should be stated explicitly.

@rfourquet
Copy link
Member

What about: pushing RandomDevice into deprecation, and recommend only the singleton RANDOM_DEVICE (or CSPRNG). Then make it thread-safe, either by a lock (managed by the core rand methods), or by having as many "singletons" as threads?

I don't know the performance impact of using a lock, relatively to the cost of generating numbers for RandomDevice. But anyway, this RNG doesn't typically require high performance.

@ViralBShah
Copy link
Member

I feel that this is something a package needs to address - having cryptographically secure random numbers. Saying what Julia's RNGs do not provide (or any other software) is usually a long list. I don't think we need to spell it out in the Julia documentation itself.

However, a package that provided cryptographically secure random numbers could talk at length about these issues.

@ViralBShah ViralBShah closed this Apr 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:docs This change adds or pertains to documentation domain:randomness Random number generation and the Random stdlib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants