From owner-freebsd-current@FreeBSD.ORG Tue Sep 16 20:16:38 2008 Return-Path: Delivered-To: current@FreeBSD.ORG Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E1BD21065677 for ; Tue, 16 Sep 2008 20:16:38 +0000 (UTC) (envelope-from das@FreeBSD.ORG) Received: from zim.MIT.EDU (ZIM.MIT.EDU [18.95.3.101]) by mx1.freebsd.org (Postfix) with ESMTP id B53DB8FC1B for ; Tue, 16 Sep 2008 20:16:38 +0000 (UTC) (envelope-from das@FreeBSD.ORG) Received: from zim.MIT.EDU (localhost [127.0.0.1]) by zim.MIT.EDU (8.14.3/8.14.2) with ESMTP id m8GKJWg1060037; Tue, 16 Sep 2008 16:19:32 -0400 (EDT) (envelope-from das@FreeBSD.ORG) Received: (from das@localhost) by zim.MIT.EDU (8.14.3/8.14.2/Submit) id m8GKJWKL060036; Tue, 16 Sep 2008 16:19:32 -0400 (EDT) (envelope-from das@FreeBSD.ORG) Date: Tue, 16 Sep 2008 16:19:32 -0400 From: David Schultz To: Andrey Chernov , current@FreeBSD.ORG Message-ID: <20080916201932.GA59781@zim.MIT.EDU> Mail-Followup-To: Andrey Chernov , current@freebsd.org References: <20080916140319.GA34447@nagual.pp.ru> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20080916140319.GA34447@nagual.pp.ru> Cc: Subject: Re: Is fork() hook ever possible? X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Sep 2008 20:16:39 -0000 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@ 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 @@ -34,21 +34,22 @@ * RC4 is a registered trademark of RSA Laboratories. */ +#include +__FBSDID("$FreeBSD: head/lib/libc/gen/arc4random.c 181261 2008-08-03 20:15:22Z ache $"); + +#include "namespace.h" #include #include #include #include +#include #include #include #include #include -#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; @@ -56,6 +57,21 @@ 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; @@ -114,9 +130,9 @@ /* * 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; } @@ -135,6 +151,7 @@ return (rs.s[(si + sj) & 0xff]); } +#if 0 u_int8_t __arc4_getbyte(void) { @@ -147,6 +164,7 @@ _ARC4_UNLOCK(); return val; } +#endif static inline u_int32_t arc4_getword(void)