ISAAC rng needs to detect forks, change its seed

[apoelstra]

The Isaac CSPRNG shipped in the standard library does not detect forking, which means that if a process forks, the result will be two processes with RNG's with identical state, a very dangerous situation.

IIRC the stdlib does not support forking, but it's certainly possible by interfacing with POSIX code. Maybe it will require changes to the task model to support. (I know that is on the docket, so something to consider..)

This is also a problem for the pending Fortuna PR in rust-crypto, which won't be accepted until it's dealt with... so I'm bugging it here in the hopes that you guys will know how to fix it :)

[thestinger]

Forking at all with the standard library has completely undefined behaviour. There are bigger problems than the task-local RNG so I don't really think it can be considered a bug in isolation. Rust's standard library isn't really a library, it's a managed framework that runs your code and you have to obey certain rules.

See #9568

[huonw]

The task-local RNG does reseed itself from the OS every few kilobytes, so this is a problem after forking, but only a temporary one.

[l0kod]

It may be handy to be able to register handlers (from external libraries like rust-crypto) to be called (automatically or manually) after a fork occurred. The #6930 could had helped.
This way, it will be possible to add a PRNG "refresher" among other future handlers.

[l0kod]

The task-local RNG does reseed itself from the OS every few kilobytes, so this is a problem after forking, but only a temporary one.

This is an important security issue.

Some quick search for recent CVEs:
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2014-0016
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2014-0017
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2013-1900
http://bugs.python.org/issue18747

[l0kod]

An interesting post on this subject: https://blog.0xbadc0de.be/archives/218

The LibreSSL team fixed that bug by using the register_atfork() function, that gets a callback called after every fork. This function is part of POSIX and is portable enough, and better, it ensures that every fork() is detected in the right way. This is not perfect (as wrote Andrew) and an explicit way to reset the entropy pool would be welcome.

The #9568 should be reopened.

[l0kod]

cc @sfackler https://github.com/sfackler/rust-openssl
cc @elly https://github.com/elly/rustcrypto
cc @relistan https://github.com/relistan/cryptorust

[l0kod]

@sfackler, CVE-2013-1900 is about PostgreSQL as well (https://github.com/sfackler/rust-postgres)

[huonw]

This RNG should not be used for cryptographically sensitive tasks; OsRng is the correct approach, and is fork-safe, if/when the rest of the stdlib is. cf. http://doc.rust-lang.org/master/std/rand/#cryptographic-security

An application that requires an entropy source for cryptographic purposes must use OsRng, which reads randomness from the source that the operating system provides (e.g. /dev/urandom on Unixes or CryptGenRandom() on Windows). The other random number generators provided by this module are not suitable for such purposes.

[thestinger]

@l0kod: Rust's runtime doesn't support fork, it's that simple. The boundary between safe and unsafe code takes care of this already. It's not a security issue.

[l0kod]

Rust's runtime doesn't support fork, it's that simple. The boundary between safe and unsafe code takes care of this already. It's not a security issue.

It's not a security issue for pure safe code but it may be needed to use an (unsafe) fork when Rust's runtime doesn't implement all required features (e.g. OS specific syscall like clone(2)).
This is why I think it should be useful to be able to centralize post-fork handlers to (manually) call after unsafe fork-like. Otherwise, there is no way (other than dig in all libraries code used) to known what should be done to cleanly re-initialize the process context.

However, this manual solution doesn't address the problem of foreign code (binding) using fork (neither the developer ignoring this issue). The #9568 should.

[thestinger]

A library making use of fork is incorrect. Use of fork beyond a simple fork + exec with known to be safe calls requires careful integration into the application and every library it's using. It's memory unsafe for more than one reason. Also, if you use clone to manually create threads / processes, then it's your fault when stuff breaks. That's completely unsupported by glibc and other C libraries - you need to write everything from the ground up if you want to do that.

[thestinger]

Forking with the runtime is known to be unsupported. There are dozens of reasons why it is broken / completely memory unsafe. If you want it to be supported, then file an RFC about supporting it as a whole. You'll need to come up with a clear design for making it work properly. An individual problem resulting from the choice not to support fork isn't a bug. I closed the earlier bug I filed because I didn't have any realistic answers to @brson's question about how we can actually make this safe as a whole.

[apoelstra]

Does this continue to be problematic with the runtime changes?

[apoelstra]

Does this continue to be problematic with the runtime changes?

[alexcrichton]

@apoelstra this is still applicable to libstd, yes, because the default HashMap is still seeded from an RNG which is itself just seeded from the OS. When forking the seed of that thread-local RNG doesn't change.

[apoelstra]

@alexcrichton can we reopen this issue? The above discussion landed on "well forking is going to cause nasal daemons anyway because the runtime doesn't support it" but now we don't have a runtime and I expect that forking can be done without trouble .... except for this nasty RNG problem.

[huonw]

I think calling not reseeding here "nasty" is a bit of an overstatement: the random numbers are just used by the HashMap to mitigate HashDoS attacks, they are not exposed elsewhere. There has even been discussion of using the same seeds for all HashMaps in an program, which has basically the same result as this does when forking.

[Gankra]

I agree that this isn't an issue for HashMap itself (or rather, it's not a problem for SipHash), but the original CVE's cited were for reseeding OpenSSL's entropy pool (for generating private keys, presumably).

That said: Rust doesn't have official RNG anymore. It's all in the rand library. As far as I can tell, nothing has changed and your argument that OsRng should be robust to forks should hold. Presumably, any security library would use that.

[alexcrichton]

@huonw that's an excellent point actually! @apoelstra in light of that I believe this issue would be most appropriately against the rand crate rather than this repo (as this isn't actually any issue either way in libstd today)