Summary: | [patch] arc4random(3): replace RC4 with ChaCha20, follow OpenBSD | ||
---|---|---|---|
Product: | Base System | Reporter: | Christian Weisgerber <naddy> |
Component: | kern | Assignee: | Xin LI <delphij> |
Status: | Closed FIXED | ||
Severity: | Affects Many People | CC: | cse.cem, david.carlier, delphij, des, emaste, grahamperrin, holger, jan.kokemueller, op |
Priority: | Normal | Flags: | des:
mfc-stable11?
des: mfc-stable10? |
Version: | CURRENT | ||
Hardware: | Any | ||
OS: | Any | ||
Attachments: |
Description
Christian Weisgerber
2013-10-03 20:00:00 UTC
Responsible Changed From-To: freebsd-bugs->secteam Over to maintainer(s). A slightly different version is proposed here https://github.com/HardenedBSD/hardenedBSD/blob/efe9fbb405647b6e223a621a6ff4c36a6d237071/lib/libc/gen/arc4random.c with you can notice use mmap INHERIT_ZERO flag which does not exist yet https://github.com/HardenedBSD/hardenedBSD/commit/a222f4ff2a22b7b937dde3756dcc3633ca1e9eb9 Created attachment 147975 [details]
Proposed patch
Created attachment 147976 [details]
Proposed patch
Created attachment 149068 [details]
Updated patch
This represents the lastest change.
Created attachment 149507 [details]
Updated patch
Please ignore this patch until the VM part not cleaned up. I'll take a look at this. Note that this is libc's arc4random(3), not libkern's arc4random(9), which is completely different code. It would be a good idea to merge the two. (In reply to Dag-Erling Smørgrav from comment #8) Please note that we need https://reviews.freebsd.org/D427 landed. Holy FSM, I thought that had been committed ages ago. What's the holdup? There was also forkdepth() proposed (https://people.freebsd.org/~kib/misc/forkdepth.1.patch). Does that approach have any downsides compared to mmap with INHERIT_ZERO? To me, forkdepth() looks like the cleaner API. VDSOs are orthogonal to INHERIT_ZERO. A commit references this bug: Author: delphij Date: Tue Mar 14 17:10:43 UTC 2017 New revision: 315272 URL: https://svnweb.freebsd.org/changeset/base/315272 Log: Implement INHERIT_ZERO for minherit(2). INHERIT_ZERO is an OpenBSD feature. When a page is marked as such, it would be zeroed upon fork(). This would be used in new arc4random(3) functions. PR: 182610 Reviewed by: kib (earlier version) MFC after: 1 month Differential Revision: https://reviews.freebsd.org/D427 Changes: head/lib/libc/sys/minherit.2 head/sys/sys/mman.h head/sys/vm/vm.h head/sys/vm/vm_map.c (In reply to Dag-Erling Smørgrav from comment #10) There were some discussion and I got distracted from this by some other work :( By the way, I actually have some Chacha20 stuff on github at https://github.com/freebsd/freebsd/compare/master...delphij:featurefork/chacha20 back in 2014, and some features may still be interesting (e.g. multithread support; compatibility shims for arc4random_stir()), so feel free to take them if needed. A commit references this bug: Author: delphij Date: Wed May 31 05:10:03 UTC 2017 New revision: 319277 URL: https://svnweb.freebsd.org/changeset/base/319277 Log: MFC r315272, r315370 r315272: Implement INHERIT_ZERO for minherit(2). INHERIT_ZERO is an OpenBSD feature. When a page is marked as such, it would be zeroed upon fork(). This would be used in new arc4random(3) functions. PR: 182610 Reviewed by: kib (earlier version) Differential Revision: https://reviews.freebsd.org/D427 r315370: The adj_free and max_free values of new_entry will be calculated and assigned by subsequent vm_map_entry_link(), therefore, remove the pointless copying. Submitted by: alc Changes: _U stable/11/ stable/11/lib/libc/sys/minherit.2 stable/11/sys/sys/mman.h stable/11/sys/vm/vm.h stable/11/sys/vm/vm_map.c A commit references this bug: Author: delphij Date: Wed May 31 05:11:28 UTC 2017 New revision: 319278 URL: https://svnweb.freebsd.org/changeset/base/319278 Log: MFC r315272, r315370 r315272: Implement INHERIT_ZERO for minherit(2). INHERIT_ZERO is an OpenBSD feature. When a page is marked as such, it would be zeroed upon fork(). This would be used in new arc4random(3) functions. PR: 182610 Reviewed by: kib (earlier version) Differential Revision: https://reviews.freebsd.org/D427 r315370: The adj_free and max_free values of new_entry will be calculated and assigned by subsequent vm_map_entry_link(), therefore, remove the pointless copying. Submitted by: alc Changes: _U stable/10/ stable/10/lib/libc/sys/minherit.2 stable/10/sys/sys/mman.h stable/10/sys/vm/vm.h stable/10/sys/vm/vm_map.c Created attachment 189567 [details]
patch to make userspace arc4random fork safe
Any updates on this? In the meantime I've been using this patch that at least makes arc4random(3) fork safe.
Created attachment 196272 [details]
Proposed patch updated to latest OpenBSD version
Proposed patch. I'll create a review for this as well.
(In reply to Xin LI from comment #18) https://reviews.freebsd.org/D16760 A commit references this bug: Author: delphij Date: Sat Aug 18 06:20:46 UTC 2018 New revision: 337997 URL: https://svnweb.freebsd.org/changeset/base/337997 Log: Split arc4random_uniform into it's own file and sync with OpenBSD. PR: 182610 Obtained from: OpenBSD MFC after: 2 weeks Changes: head/lib/libc/gen/Makefile.inc head/lib/libc/gen/arc4random.c head/lib/libc/gen/arc4random_uniform.c Created attachment 196337 [details]
Folded chacha20 implementation with the kernel one and various other fixes
Created attachment 196342 [details]
Remove uses of arc4random_addrandom too and remove manual pages for deprecated interfaces
A commit references this bug: Author: delphij Date: Sun Aug 19 17:40:53 UTC 2018 New revision: 338059 URL: https://svnweb.freebsd.org/changeset/base/338059 Log: Update userland arc4random() with OpenBSD's Chacha20 based arc4random(). ObsoleteFiles.inc: Remove manual pages for arc4random_addrandom(3) and arc4random_stir(3). contrib/ntp/lib/isc/random.c: contrib/ntp/sntp/libevent/evutil_rand.c: Eliminate in-tree usage of arc4random_addrandom(). crypto/heimdal/lib/roken/rand.c: crypto/openssh/config.h: Eliminate in-tree usage of arc4random_stir(). include/stdlib.h: Remove arc4random_stir() and arc4random_addrandom() prototypes, provide temporary shims for transistion period. lib/libc/gen/Makefile.inc: Hook arc4random-compat.c to build, add hint for Chacha20 source for kernel, and remove arc4random_addrandom(3) and arc4random_stir(3) links. lib/libc/gen/arc4random.c: Adopt OpenBSD arc4random.c,v 1.54 with bare minimum changes, use the sys/crypto/chacha20 implementation of keystream. lib/libc/gen/Symbol.map: Remove arc4random_stir and arc4random_addrandom interfaces. lib/libc/gen/arc4random.h: Adopt OpenBSD arc4random.h,v 1.4 but provide _ARC4_LOCK of our own. lib/libc/gen/arc4random.3: Adopt OpenBSD arc4random.3,v 1.35 but keep FreeBSD r114444 and r118247. lib/libc/gen/arc4random-compat.c: Compatibility shims for arc4random_stir and arc4random_addrandom functions to preserve ABI. Log once when called but do nothing otherwise. lib/libc/gen/getentropy.c: lib/libc/include/libc_private.h: Fold __arc4_sysctl into getentropy.c (renamed to arnd_sysctl). Remove from libc_private.h as a result. sys/crypto/chacha20/chacha.c: sys/crypto/chacha20/chacha.h: Make it possible to use the kernel implementation in libc. PR: 182610 Reviewed by: cem, markm Obtained from: OpenBSD Relnotes: yes Differential Revision: https://reviews.freebsd.org/D16760 Changes: head/ObsoleteFiles.inc head/contrib/ntp/lib/isc/random.c head/contrib/ntp/sntp/libevent/evutil_rand.c head/crypto/heimdal/lib/roken/rand.c head/crypto/openssh/config.h head/include/stdlib.h head/lib/libc/gen/Makefile.inc head/lib/libc/gen/Symbol.map head/lib/libc/gen/arc4random-compat.c head/lib/libc/gen/arc4random.3 head/lib/libc/gen/arc4random.c head/lib/libc/gen/arc4random.h head/lib/libc/gen/getentropy.c head/lib/libc/include/libc_private.h head/sys/crypto/chacha20/chacha.c head/sys/crypto/chacha20/chacha.h |