Expose ThreadRng internal as ThreadRngCore

[LiosK]

This commit refactors ThreadRng to separate its internal PRNG and reseeding logic as ThreadRngCore so the new type could be exposed to public in the future for those who want to copy ThreadRng behavior with an independent state or in a Send + Sync context.

See also #1748

[LiosK]

Design question: there would be two ways to expose the ThreadRng internal:

• As ThreadRngCore that implements new() and reseed() with hard-coded integration with SysRng just as ThreadRng does.
• As ReseedingStdRng that implements SeedableRng allowing users to re-use ThreadRng's logic with different seed source.

This PR chose the former because it is simpler, clearer, and focused on the needs to copy ThreadRng behavior with an independent state or in a Send + Sync context. With the latter, we have to decide which feature, std_rng or thread_rng, to expose this under, and we will need further discussion about what we want to do with the new type.

[newpavlov]

Personally, I don't think we should do this. IIUC your main motivation is aws/s2n-quic#2986, I left an alternative suggestion in the comment there.

Also, if your goal is to make the type public, it should be done as part of this PR.

[LiosK]

Hi, I am not from aws/s2n-quic. I just found it relevant but should have opened a new issue. ThreadRngCore is ready for pub, but I didn't do so because it's just the beginning of discussion.

The motivation behind this is to provide users with ability to reuse ThreadRng behavior with an independent state or in a Send or Sync context. ReseedingRng used to be a tool for that but wasn't convenient at all as we needed to pull ChaChaCore from rand_chacha, set parameters properly, and monitor rand for any updates to ThreadRng. This process becomes further complicated after ReseedingRng removal.

With a more convenient type, more users might be interested in reusing ThreadRng behavior than the ReseedingRng users. I would even expect ThreadRngCore covers a substantial portion of StdRng use cases. What do you think?

[newpavlov]

As I wrote in the original issue, we could re-introduce ReseedingRng, but I think it's better to do in a separate crate instead of rand.

Moreover, I consider reliance on reseeding RNG to be an anti-pattern more often than not (i.e. users should use either a "stateless" RNG like SysRng/ThreadRng or a good CSPRNG seeded once from a secure source), so IMO it's fine to keep reseeding RNGs it somewhat "inconvenient".

[tarcieri]

Reseeding RNGs give you forward secrecy over ones which use a persistent seed/key

[newpavlov]

Sure, but if you want forward secrecy then it's better to select a CSPRNG which provides it out of the box instead of relying on the reseeding hack.

[LiosK]

Reseeding is controversial, and AFAIK it's just a nice-to-have feature, but because it's nice to have, ThreadRng employs it and many users might wish to. Honestly, I can't find a specific use case where StdRng is preferred if ThreadRngCore is available in Send/Sync situations.

Also, I am fully convinced that ReseedingRng should go away, but this PR has a very difference complexity/utility profile.

[dhardy]

Why call this ThreadRngCore? Sure, this is the "core" of ThreadRng, but that's a backwards way to define the capabilities it has.

Relative to ReseedingRng, this fixes the reseeding source (fine; why bother with anything other than SysRng), the core generator (ChaCha12Core should be sufficient, so probably also fine) and the reseeding interval (here selected to occasionally reseed without affecting performance much).

Also, I am fully convinced that ReseedingRng should go away, but this PR has a very difference complexity/utility profile.

The complexity that this does have is that there is another piece of API which must be specified. You haven't made this pub or written docs for it which are arguably the most important part of such an addition.

So, why should people use this? The reason ThreadRng is the way it is is because we wanted to (continue to) provide a fast, easily usable randomness source while also somewhat limiting the risks of mis-use, for example should some application fork while using ThreadRng to generate keys, or to allow an early-boot application getting a week seed to eventually recover (which was a theoretical issue at one point, but shouldn't be an issue now). Most applications shouldn't have these requirements and could just use SysRng or StdRng or a non-CS PRNG.

[newpavlov]

whereas it allows users to utilize the same reseeding generator where Send or Sync is required

I don't think it's a sufficient motivation for this addition. As I wrote in the s2n-quic issue, I think it's better to use a ZST type representing ThreadRng instead.

Ideally, rand::ThreadRng should also be a ZST type, but we are unlikely to do it in the near future for efficiency reasons, at least until #[thread_local] and 'thread are stabilized.

[LiosK]

Thank you very much for your inputs. I added minimum viable doc to ThreadRngCore. I can expand this to rngs/mod.rs and others as discussion evolves. Defining this in relation to ThreadRng clarifies the role and minimizes the need for documentation. Also, if most use cases of ReseedingRng were to re-produce ThreadRng (according to #1721), ThreadRngCore will be the exact thing ReseedingRng users (including I) were looking for.

So, why should people use this?

The answer is likely to be the same as the reason why ThreadRng does reseeding. ThreadRng provides (1) thread locality, (2) reseeding, and (3) fixed seed source on top of StdRng, and I assume 2 and 3 are to provide a potentially securer default (whereas 1 seems more towards convenience) when the actual use case, quality of seed sources, and knowledge of the user in RNGs are all unknown. Similarly, as a writer of libraries depending on rand, I would like to provide users with a potentially securer default because the actual use case, quality of seed sources, and knowledge of the user in RNGs are all unknown. The thread locality is a blocker here because potentially forcing time/space-costly initialization per thread upon launch/fork is not a nice default where I do not know how many threads users are going to use.

ThreadRng looks to me to mix two functionalities (1 vs. 2 and 3), and I see a potential boundary of responsibilities here. This is a more theoretical background of this proposal.