From owner-freebsd-hackers@FreeBSD.ORG Fri Jul 18 16:21:24 2014 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 2A76D70F for ; Fri, 18 Jul 2014 16:21:24 +0000 (UTC) Received: from mail-wg0-x22c.google.com (mail-wg0-x22c.google.com [IPv6:2a00:1450:400c:c00::22c]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id ACA5224D9 for ; Fri, 18 Jul 2014 16:21:23 +0000 (UTC) Received: by mail-wg0-f44.google.com with SMTP id m15so3673584wgh.27 for ; Fri, 18 Jul 2014 09:21:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:subject :content-type; bh=SZzQRmbsaRl+u79C/ZN+IwAxrN2k7U1GH3XFQIN8qVk=; b=ZIs+Pw4RVtcHOk2XF8Ag4J0jtJdzNPtfP6n4mOE9G6W+HCC5un0SCZjCuKGKG5eBVQ dQ2A6pJ/mhX0c/P6VhG9wJEBXkzgTshZOy3VNIEjr/3h/cOegi872ZWDDmQBemPiHsfI C9dNaOfx1pLtnGl+QaT06IxqDHo7EQdfIxfTwAYNiEBkkp+DBFRvsoOU9HRX3B1uz9+T HSISZIBAKiAGAWkpKExncbUUulK2vxGqHhqDZLiCeQg1lJkrZoq9av9J7oWpUEOdyzS5 Rq3DqCElkNPWM6LV0xoQ3LtuQEWQoX3SwJ9f7ci10fd096usJo8ZD7DUWOc/pdUxLCMu JPWg== X-Received: by 10.180.80.225 with SMTP id u1mr7332662wix.69.1405700481947; Fri, 18 Jul 2014 09:21:21 -0700 (PDT) Received: from [192.168.178.121] (p5B36FDC5.dip0.t-ipconnect.de. [91.54.253.197]) by mx.google.com with ESMTPSA id eo4sm8283658wid.4.2014.07.18.09.21.20 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 18 Jul 2014 09:21:21 -0700 (PDT) Message-ID: <53C9497E.9020001@gmail.com> Date: Fri, 18 Jul 2014 18:21:18 +0200 From: =?ISO-8859-1?Q?Jan_Kokem=FCller?= User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: freebsd-hackers@freebsd.org Subject: Fixing fork detection of arc4random by implementing INHERIT_ZERO for minherit? Content-Type: multipart/mixed; boundary="------------090701060408080708060506" X-Mailman-Approved-At: Fri, 18 Jul 2014 16:40:25 +0000 X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 18 Jul 2014 16:21:24 -0000 This is a multi-part message in MIME format. --------------090701060408080708060506 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Hi, the issue mentioned at https://www.agwa.name/blog/post/libressls_prng_is_unsafe_on_linux also affects FreeBSDs arc4random implementation as its fork detection relies on changing pids only. Under FreeBSD, LibreSSL uses arc4random directly, and relies on it to be 100% fork safe. They don't provide a way to stir the RNG manually. I've brought this up with the LibreSSL developers, who think it's a bug in the OS (https://github.com/libressl-portable/portable/issues/17). I've tried to implement INHERIT_ZERO for minherit to make arc4random fork safe (patches attached). It seems to work fine so far, but I'm really no expert on FreeBSDs VM system. Also, the arc4random functions should probably be updated to use something more secure like ChaCha20 instead of RC4 (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=182610). Cheers, Jan --------------090701060408080708060506 Content-Type: text/x-patch; name="minherit-inherit-zero.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="minherit-inherit-zero.patch" diff --git a/sys/sys/mman.h b/sys/sys/mman.h index e89bee3..e3cb967 100644 --- a/sys/sys/mman.h +++ b/sys/sys/mman.h @@ -43,6 +43,7 @@ #define INHERIT_SHARE 0 #define INHERIT_COPY 1 #define INHERIT_NONE 2 +#define INHERIT_ZERO 3 #endif /* diff --git a/sys/vm/vm.h b/sys/vm/vm.h index d87495d..0eb7568 100644 --- a/sys/vm/vm.h +++ b/sys/vm/vm.h @@ -68,6 +68,7 @@ typedef char vm_inherit_t; /* inheritance codes */ #define VM_INHERIT_SHARE ((vm_inherit_t) 0) #define VM_INHERIT_COPY ((vm_inherit_t) 1) #define VM_INHERIT_NONE ((vm_inherit_t) 2) +#define VM_INHERIT_ZERO ((vm_inherit_t) 3) #define VM_INHERIT_DEFAULT VM_INHERIT_COPY typedef u_char vm_prot_t; /* protection codes */ diff --git a/sys/vm/vm_map.c b/sys/vm/vm_map.c index 0713af8..a8c86ef 100644 --- a/sys/vm/vm_map.c +++ b/sys/vm/vm_map.c @@ -2229,6 +2229,7 @@ vm_map_inherit(vm_map_t map, vm_offset_t start, vm_offset_t end, case VM_INHERIT_NONE: case VM_INHERIT_COPY: case VM_INHERIT_SHARE: + case VM_INHERIT_ZERO: break; default: return (KERN_INVALID_ARGUMENT); @@ -3324,6 +3325,7 @@ vmspace_fork(struct vmspace *vm1, vm_ooffset_t *fork_charge) break; case VM_INHERIT_COPY: + case VM_INHERIT_ZERO: /* * Clone the entry and link into the map. */ @@ -3341,8 +3343,10 @@ vmspace_fork(struct vmspace *vm1, vm_ooffset_t *fork_charge) vm_map_entry_link(new_map, new_map->header.prev, new_entry); vmspace_map_entry_forked(vm1, vm2, new_entry); - vm_map_copy_entry(old_map, new_map, old_entry, - new_entry, fork_charge); + if (old_entry->inheritance == VM_INHERIT_COPY) { + vm_map_copy_entry(old_map, new_map, old_entry, + new_entry, fork_charge); + } break; } old_entry = old_entry->next; --------------090701060408080708060506 Content-Type: text/x-patch; name="arc4random-fork-safety-POC.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="arc4random-fork-safety-POC.patch" diff --git a/lib/libc/gen/arc4random.c b/lib/libc/gen/arc4random.c index 59e410b..8dab016 100644 --- a/lib/libc/gen/arc4random.c +++ b/lib/libc/gen/arc4random.c @@ -38,6 +38,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include #include @@ -77,7 +78,7 @@ static pthread_mutex_t arc4random_mtx = PTHREAD_MUTEX_INITIALIZER; static int rs_initialized; static struct arc4_stream rs; static pid_t arc4_stir_pid; -static int arc4_count; +static int* arc4_count; extern int __sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp, size_t newlen); @@ -90,6 +91,12 @@ arc4_init(void) { int n; + if ((arc4_count = mmap(NULL, sizeof(int), PROT_READ|PROT_WRITE, + MAP_ANON|MAP_PRIVATE, -1, 0)) == MAP_FAILED) + abort(); + if (minherit(arc4_count, sizeof(int), INHERIT_ZERO) == -1) + abort(); + for (n = 0; n < 256; n++) rs.s[n] = n; rs.i = 0; @@ -174,7 +181,7 @@ arc4_stir(void) */ for (i = 0; i < 1024; i++) (void)arc4_getbyte(); - arc4_count = 1600000; + *arc4_count = 1600000; } static void @@ -182,7 +189,7 @@ arc4_stir_if_needed(void) { pid_t pid = getpid(); - if (arc4_count <= 0 || !rs_initialized || arc4_stir_pid != pid) + if (arc4_count == NULL || *arc4_count <= 0 || !rs_initialized || arc4_stir_pid != pid) { arc4_stir_pid = pid; arc4_stir(); @@ -237,7 +244,8 @@ arc4random(void) { u_int32_t val; _ARC4_LOCK(); - arc4_count -= 4; + if (arc4_count) + *arc4_count -= 4; arc4_stir_if_needed(); val = arc4_getword(); _ARC4_UNLOCK(); @@ -251,7 +259,7 @@ arc4random_buf(void *_buf, size_t n) _ARC4_LOCK(); arc4_stir_if_needed(); while (n--) { - if (--arc4_count <= 0) + if (--*arc4_count <= 0) arc4_stir(); buf[n] = arc4_getbyte(); } --------------090701060408080708060506--