ref: use SmallRng

[joshuarli]

Alternative to #118.

My understanding is that SmallRng is much smaller and cheaper to initialize than the thread_rng, which is a thread-local ThreadRng (StdRng, with the added overhead of ReseedingRng because it needs to be secure). SmallRng is also faster than StdRng. This is all because SmallRng is not a CSPRNG.

It's worth noting that unreleased rand at this time also shakes rand_chacha from the dependency tree, which is really nice. I believe rand's planning on releasing 0.8, so if this PR looks good, I'd recommend blocking on that. Issues: rust-random/rand#965, rust-random/rand#938.

[Stebalien]

Nice!

[taiki-e]

Suggested change

	.sample_iter(&Alphanumeric)
	.take(rand_len)
	.collect::<String>()
	.sample_iter(Alphanumeric)
	.take(rand_len)
	.map(char::from)
	.collect::<String>()

[Stebalien]

That's still going to allocate, unfortunately.

[taiki-e]

I'll look for a way to avoid the allocation.

EDIT: See #119 (comment)

[Stebalien]

In addition to the compile issue above, I'm a bit concerned about calling getrandom every time. I assume that'll slow things down significantly.

[Stebalien]

One solution would be to put a smallrng in a thread-local variable.

[joshuarli]

In addition to the compile issue above, I'm a bit concerned about calling getrandom every time. I assume that'll slow things down significantly.

Oh yeah, I don't remember considering performance at the time at all. You can also close this if you'd like.

[taiki-e]

We can avoid allocation by using char::encode_utf8.
https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=421dfa2eebe631075a03b06e08c6e7cf

Suggested change

	let small_rng = SmallRng::from_entropy();
	buf.push(small_rng
	.sample_iter(&Alphanumeric)
	.take(rand_len)
	.collect::<String>()
	);
	let mut tmp = [0; 1];
	let small_rng = SmallRng::from_entropy();
	small_rng
	.sample_iter(Alphanumeric)
	.take(rand_len)
	.for_each(|c| buf.push((c as char).encode_utf8(&mut tmp)));

Can we probably resolve performance issues in combination with putting a smallrng in a thread-local variable?

[Stebalien]

Merged in #141.

[jwgarber]

I have concerns about using a non-CSPRNG to generate temporary file names. For example, the Cert C Coding standard on temporary files warns that

Privileged programs that create temporary files in world-writable directories can be exploited to overwrite protected system files. An attacker who can predict the name of a file created by a privileged program can create a symbolic link (with the same name as the file used by the program) to point to a protected system file. Unless the privileged program is coded securely, the program will follow the symbolic link instead of opening or creating the file that it is supposed to be using. As a result, a protected system file to which the symbolic link points can be overwritten when the program is executed [HP 2003]. Unprivileged programs can be similarly exploited to overwrite protected user files.

This OWASP page also goes into great detail of the potential security implications of using temporary files with predictable names.

The documentation for SmallRng states that it is not a good choice when security against predictions is important (which is the case here), and recommends using StdRng instead, which is cryptographically secure and thus unpredictable. Considering that we do not know all use cases where this crate will be used, I suggest we err on the side of caution and use StdRng or ThreadRng to generate the file names.

[Stebalien]

Yeah, fair enough. Using StdRng is probably overkill but the filenames should be unpredictable.

I'm going to try to make using insecure rand opt-in... but I'm having some trouble with the feature specification.