libottery icon indicating copy to clipboard operation
libottery copied to clipboard

fork() safety

Open zackw opened this issue 10 years ago • 4 comments

The global state object should be automatically invalidated on the child side of a fork, to ensure that the child does not produce the same random stream as the parent from then on (see http://emboss.github.io/blog/2013/08/21/openssl-prng-is-not-really-fork-safe/ for discussion of this issue wrt OpenSSL).

The obvious way to do this is with pthread_atfork; conveniently, ottery_wipe already has the right signature for that. It does raise a bunch of portability issues, but I don't see a better approach.

Dealing with this for application-allocated ottery_state objects is probably up to the application, but we should at least document the problem.

zackw avatar Aug 28 '13 15:08 zackw

Well, libottery currently does a "gitpid() != st->pid" check, which doesn't have the same flaw as openssl's "MD_Update(...cur_pid)" operation. No matter how many times you fork a process, the child's pid will never be the same as the parent's.

There is a possible flaw here, though: if we fork, and then the parent dies, and then the child forks again without invoking any ottery functions in the meantime, we could wind up with the same PID as the original process, which would keep us from noticing that the PID had changed.

There's an old-ish branch "topic/pthread_atfork" that tries to do the obvious pthread_atfork workaround. I'm not completely happy with it, but I do think we should take some approach like that at some point.

nmathewson avatar Aug 28 '13 18:08 nmathewson

The branch topic/pthread_atfork_v2 now has some sketched-out work here, but I don't much care for it. It uses both an atfork-based implementation and a getpid-based implementation, but doing both of these slows down small requests by more than I'd like... and this is before making the atfork-counter thread-safe. (Atomic, not locked, I hope!) If we can really trust the pthread_atfork thing, we could get the time back, but I'm worried to do so without a lot of thinking and testing.

nmathewson avatar Sep 10 '13 07:09 nmathewson

I don't see any pthread_atfork_v2 branch...?

zackw avatar Sep 10 '13 19:09 zackw

oops. Should be there now.

nmathewson avatar Sep 10 '13 20:09 nmathewson