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

[draft] investigate solutions to immediate-reseeding-on-fork #1377

Closed
wants to merge 2 commits into from

Conversation

dhardy
Copy link
Member

@dhardy dhardy commented Jan 30, 2024

This is an investigation branch into potential solutions to #1362.

No fork protection

The simplest solution is simply to choose not provide fork protection.

GlobalRng

ThreadRng has thread-local state which cannot be reached by fork_handler. The simplest "solution" to this is to replace thread-local state with global state that can be, hence GlobalRng using a Mutex.

Perf. on a single thread is (unsurprisingly) fine, however holding onto a GlobalRng reference could easily cause dead-locking, making fn global_rng() -> GlobalRng a hazard.

Obviously this is not a good solution where high-performance generation is needed from multiple threads; in this case users would need to seed and store local generators. This is possibly the best option in such a case anyway.

Implication: global_rng is fine for occasional usage and/or in single-thread environments, but probably we would want to encourage users to construct a local RNG for any significant usage.

An additional advantage of replacing ThreadRng with this is less thread-local state usage.

Frequent polling of RESEEDING_RNG_FORK_COUNTER

The current fork handler works by checking RESEEDING_RNG_FORK_COUNTER only when generating a new block of random state. We could check this much more often (on every value generation), but this likely would have a large perf. hit.

Other solutions?

There are various approaches to fast thread-safe containers and consensus algorithms. We could possibly attempt to design an RNG along such lines, however this would be far from a trivial undertaking (especially reviewing for security) and still likely to severely under-perform existing RNGs.

Maybe there are other existing solutions we're overlooking?

@newpavlov
Copy link
Member

newpavlov commented Jan 30, 2024

Honestly, I am tempted to say that ThreadRng is not fork-safe by default and that applications which rely on fork and security of ThreadRng must manually reseed it in a forked thread using inherent method. Something like this:

let pid = unsafe { libc::fork() };
if pid == 0 {
    thread_rng().reseed();
}

We currently implement fork protection only for UNIX targets and I am not even sure it properly works on all #[cfg(all(unix, not(target_os = "emscripten")))] targets. At the very least, it's an inconsistent behavior.

Most Rust applications do not use fork at all, so I don't think it's worth to increase code complexity for a very niche case.

@dhardy
Copy link
Member Author

dhardy commented Feb 1, 2024

that applications which rely on fork and security of ThreadRng must manually reseed it in a forked thread using inherent method. Something like this:

Problem: such a call would need to appear immediately before every (important) usage of thread_rng, and that's hard to assure.

Most Rust applications do not use fork at all

Certainly, choosing not to provide fork protection is a valid choice.

@dhardy dhardy mentioned this pull request Feb 1, 2024
5 tasks
@newpavlov
Copy link
Member

newpavlov commented Feb 1, 2024

Problem: such a call would need to appear immediately before every (important) usage of thread_rng, and that's hard to assure.

Maybe I misunderstand something, but I don't think so? You only need to assure that ThreadRng is reseeded in a child process immediately after fork, before code in it gets to use its local instance of ThreadRng. Meanwhile, the parent process can continue to work without changes. In other words, you need to guard not thread_rng uses, but fork points. Remember that the child process starts only with one thread (it's a POSIX standard requirement).

We could mention pthread_atfork in ThreadRng docs with an example how users may add fork protection to their forking application.

@dhardy
Copy link
Member Author

dhardy commented Feb 1, 2024

Remember that the child process starts only with one thread (it's a POSIX standard requirement).

Ah, I didn't know this. Then, yes. In that case we could also fix the existing thread_rng to reseed immediately on fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants