Date: Fri, 18 Jul 2014 18:21:18 +0200 From: =?ISO-8859-1?Q?Jan_Kokem=FCller?= <jan.kokemueller@gmail.com> To: freebsd-hackers@freebsd.org Subject: Fixing fork detection of arc4random by implementing INHERIT_ZERO for minherit? Message-ID: <53C9497E.9020001@gmail.com>
next in thread | raw e-mail | index | archive | help
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 <stdlib.h> #include <unistd.h> #include <sys/types.h> +#include <sys/mman.h> #include <sys/param.h> #include <sys/sysctl.h> #include <sys/time.h> @@ -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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?53C9497E.9020001>