Add `rand_core::InfallibleRng` marker trait

[newpavlov]

Implementers of the rand_core::InfallibleRng trait indicate that they will never return errors from the RngCore::try_fill_bytes method and will never panic on unwrapping rand_core::Error while calling other methods.

@ctz Does it address your concerns mentioned in this comment? If you have other concerns/suggestion, feel free to list them here or in a separate issue.

[newpavlov]

This PR got a bunch of unrelated formatting changes in the edited files, which were introduced automatically by my editor. Later we probably need to fix formatting for the whole project and add rustfmt check to CI.

[dhardy]

Later we probably need to fix formatting for the whole project and add rustfmt check to CI.

Yes; I was thinking try to resolve most PRs first. But we could also just go ahead...

[newpavlov]

I've cleaned up the formatting changes.

[dhardy]

Also, if @ctz doesn't appreciate the design of RngCore, he's not the only one. It's a compromise that I don't think anyone likes. There's a tracker of some suggestions here: #1261.

This PR adds something, yes, but is a more fundamental revision a better option?

[dhardy]

There is a much simpler alternative to this PR: require that all RngCore impls are infallible (or at least document possible panics).

Implication: we should drop OsRng. Since we no longer use this for seeding, I am not aware of any good reason to keep this. (We already removed ReadRng.)

We can also drop try_fill_bytes and all the stuff about converting between fallible and infallible cases.

Am I missing anything important?

[newpavlov]

I am open to a more global rework (e.g. it may be worth to remove BlockRng stuff and replace it with helper macros/functions). But I think it's better to do in a separate PR after merging this one, since it's not given that we will be able to finalize it for v0.9 release cycle.

There is a much simpler alternative to this PR: require that all RngCore impls are infallible (or at least document possible panics).

In my opinion, it's tantamount to sweeping the problem under the rug.

IO-based RNGs such as OsRng or hardware-based RNGs (not an unusual thing with cryptographic applications) can fail as any other IO. It's a simple truth of life. Imagine someone has misconfigured, or forgot to plug-in a HW RNG. It's better for a cryptographic application/library to return a sensible error, instead of unconditionally panicking outright. Even PRNGs may fail in some cases, e.g. ChaCha-based RNGs may return error on detected looping, which may happen in practice in the presence of seeking.

As for OsRng, it can be a preferred way of generating sensitive information such as cryptographic keys, especially considering that ThreadRng does not implement any protection against exposed state. You may say "then use getrandom directly", but it would be less flexible, e.g. we would not be able to use PRNGs for reproducible testing of such methods.

[dhardy]

You are right that such questions are beyond the scope of this PR, however we should at least determine whether we actually want trait InfallibleRng before merging this. Right, new issue then.

[newpavlov]

Closing in favor of #1424.