Re: Is fork() hook ever possible?

On Tue, Sep 16, 2008, Andrey Chernov wrote:
> I need some sort of fork() hook to detect that pid is changed to re-stir 
> ar4random() after that (in the child), simple flag variable with 
> child's pid is needed.
> 
> Currently OpenBSD does almost that checking getpid() every time 
> arc4random() called, but it is very slow way to use getpid() syscall 
> repeatedly, about 12-15 times slower than just arc4random() without 
> getpid().

Do you have a compelling example of why microoptimizing
arc4random() is an urgent matter? Most apps don't need
cryptographic-quality randomness thousands of times a second in
the critical path, and OpenBSD has been surviving just fine with
the calls to getpid().

The patches I sent you essentially upgrade to the OpenBSD source
for arc4random() and try to minimize the diffs against them. (A
comparison of the resultant sources to OpenBSD's is below.) This
fixes several bugs, including:

1. silently using insecure entropy sources for jailed apps where
   /dev/random is unavailable
2. buffer sloppiness
3. spurious restir checks
4. the fork() problem you describe

secteam_at_ already agreed to the idea of solving the fork problem as
in OpenBSD over a month ago. I really do not understand why we
need to keep trying to outdo OpenBSD with our own variant of
arc4random(). They addressed the fork() issue more than 5 years
ago and in general, they seem to be winning in terms of
correctness.

If getpid() really winds up being a serious problem, we can solve
it in a principled way, e.g., by having the kernel fault in a
read-only page containing the current process pid, and having
libc's getpid() read it. I know Windows has a facility like this
that they use for a number of things, and ISTR that Linux recently
introduced one, too. The bottom line is that we shouldn't solve
the problem with hacks in arc4random(), and we shouldn't try to
solve it at all until it's proven to be a real problem.


--- /usr/ob/src/lib/libc/crypt/arc4random.c	2008-06-03 20:50:23.000000000 -0400
+++ arc4random.c	2008-08-16 15:14:59.000000000 -0400
_at__at_ -34,21 +34,22 _at__at_
  * RC4 is a registered trademark of RSA Laboratories.
  */
 
+#include <sys/cdefs.h>
+__FBSDID("$FreeBSD: head/lib/libc/gen/arc4random.c 181261 2008-08-03 20:15:22Z ache $");
+
+#include "namespace.h"
 #include <fcntl.h>
 #include <limits.h>
 #include <stdlib.h>
 #include <unistd.h>
+#include <pthread.h>
 #include <sys/types.h>
 #include <sys/param.h>
 #include <sys/time.h>
 #include <sys/sysctl.h>
-#include "thread_private.h"
 
-#ifdef __GNUC__
-#define inline __inline
-#else				/* !__GNUC__ */
-#define inline
-#endif				/* !__GNUC__ */
+#include "libc_private.h"
+#include "un-namespace.h"
 
 struct arc4_stream {
 	u_int8_t i;
_at__at_ -56,6 +57,21 _at__at_
 	u_int8_t s[256];
 };
 
+static pthread_mutex_t	arc4random_mtx = PTHREAD_MUTEX_INITIALIZER;
+
+#define	RANDOMDEV	"/dev/urandom"
+#define	_ARC4_LOCK()						\
+	do {							\
+		if (__isthreaded)				\
+			_pthread_mutex_lock(&arc4random_mtx);	\
+	} while (0)
+
+#define	_ARC4_UNLOCK()						\
+	do {							\
+		if (__isthreaded)				\
+			_pthread_mutex_unlock(&arc4random_mtx);	\
+	} while (0)
+
 static int rs_initialized;
 static struct arc4_stream rs;
 static pid_t arc4_stir_pid;
_at__at_ -114,9 +130,9 _at__at_
 
 	/*
 	 * Discard early keystream, as per recommendations in:
-	 * http://www.wisdom.weizmann.ac.il/~itsik/RC4/Papers/Rc4_ksa.ps
+	 * "(Not So) Random Shuffles of RC4" by Ilya Mironov.
 	 */
-	for (i = 0; i < 256; i++)
+	for (i = 0; i < 1024; i++)
 		(void)arc4_getbyte();
 	arc4_count = 1600000;
 }
_at__at_ -135,6 +151,7 _at__at_
 	return (rs.s[(si + sj) & 0xff]);
 }
 
+#if 0
 u_int8_t
 __arc4_getbyte(void)
 {
_at__at_ -147,6 +164,7 _at__at_
 	_ARC4_UNLOCK();
 	return val;
 }
+#endif
 
 static inline u_int32_t
 arc4_getword(void)