From owner-svn-src-all@FreeBSD.ORG Tue Nov 15 05:49:24 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A9D54106566B; Tue, 15 Nov 2011 05:49:24 +0000 (UTC) (envelope-from das@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 99C388FC0A; Tue, 15 Nov 2011 05:49:24 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id pAF5nOZM083031; Tue, 15 Nov 2011 05:49:24 GMT (envelope-from das@svn.freebsd.org) Received: (from das@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id pAF5nOu8083029; Tue, 15 Nov 2011 05:49:24 GMT (envelope-from das@svn.freebsd.org) Message-Id: <201111150549.pAF5nOu8083029@svn.freebsd.org> From: David Schultz Date: Tue, 15 Nov 2011 05:49:24 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r227520 - head/lib/libc/gen X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 15 Nov 2011 05:49:24 -0000 Author: das Date: Tue Nov 15 05:49:24 2011 New Revision: 227520 URL: http://svn.freebsd.org/changeset/base/227520 Log: Further reduce diffs with OpenBSD's arc4random. The main functional change here is to ensure that when a process forks after arc4random is seeded, the parent and child don't observe the same random sequence. OpenBSD's fix introduces some additional overhead in the form of a getpid() call. This could be improved upon, e.g., by setting a flag in fork(), if it proves to be a problem. This was discussed with secteam (simon, csjp, rwatson) in 2008, shortly prior to my going out of town and forgetting all about it. The conclusion was that the problem with forks is worrisome, but it doesn't appear to have introduced an actual vulnerability for any known programs. The only significant remaining difference between our arc4random and OpenBSD's is in how we seed the generator in arc4_stir(). Modified: head/lib/libc/gen/arc4random.c Modified: head/lib/libc/gen/arc4random.c ============================================================================== --- head/lib/libc/gen/arc4random.c Tue Nov 15 05:45:46 2011 (r227519) +++ head/lib/libc/gen/arc4random.c Tue Nov 15 05:49:24 2011 (r227520) @@ -33,11 +33,13 @@ __FBSDID("$FreeBSD$"); #include "namespace.h" -#include -#include -#include #include +#include +#include #include +#include +#include +#include #include #include "libc_private.h" @@ -71,9 +73,9 @@ static pthread_mutex_t arc4random_mtx = _pthread_mutex_unlock(&arc4random_mtx); \ } while (0) -static struct arc4_stream rs; static int rs_initialized; -static int rs_stired; +static struct arc4_stream rs; +static pid_t arc4_stir_pid; static int arc4_count; static inline u_int8_t arc4_getbyte(void); @@ -117,6 +119,10 @@ arc4_stir(void) u_char rnd[KEYSIZE]; } rdat; + if (!rs_initialized) { + arc4_init(); + rs_initialized = 1; + } fd = _open(RANDOMDEV, O_RDONLY, 0); done = 0; if (fd >= 0) { @@ -141,6 +147,18 @@ arc4_stir(void) arc4_count = 1600000; } +static void +arc4_stir_if_needed(void) +{ + pid_t pid = getpid(); + + if (arc4_count <= 0 || !rs_initialized || arc4_stir_pid != pid) + { + arc4_stir_pid = pid; + arc4_stir(); + } +} + static inline u_int8_t arc4_getbyte(void) { @@ -166,31 +184,11 @@ arc4_getword(void) return val; } -static void -arc4_check_init(void) -{ - if (!rs_initialized) { - arc4_init(); - rs_initialized = 1; - } -} - -static inline void -arc4_check_stir(void) -{ - if (!rs_stired || arc4_count <= 0) { - arc4_stir(); - rs_stired = 1; - } -} - void arc4random_stir(void) { _ARC4_LOCK(); - arc4_check_init(); arc4_stir(); - rs_stired = 1; _ARC4_UNLOCK(); } @@ -198,8 +196,8 @@ void arc4random_addrandom(u_char *dat, int datlen) { _ARC4_LOCK(); - arc4_check_init(); - arc4_check_stir(); + if (!rs_initialized) + arc4_stir(); arc4_addrandom(dat, datlen); _ARC4_UNLOCK(); } @@ -209,10 +207,9 @@ arc4random(void) { u_int32_t val; _ARC4_LOCK(); - arc4_check_init(); - arc4_check_stir(); - val = arc4_getword(); arc4_count -= 4; + arc4_stir_if_needed(); + val = arc4_getword(); _ARC4_UNLOCK(); return val; } @@ -222,11 +219,11 @@ arc4random_buf(void *_buf, size_t n) { u_char *buf = (u_char *)_buf; _ARC4_LOCK(); - arc4_check_init(); + arc4_stir_if_needed(); while (n--) { - arc4_check_stir(); + if (--arc4_count <= 0) + arc4_stir(); buf[n] = arc4_getbyte(); - arc4_count--; } _ARC4_UNLOCK(); }