Created attachment 148045 [details] Proposed patch it is a proposed simple FreeBSD version of the OpenBSD's
For reference https://github.com/HardenedBSD/hardenedBSD/commit/c5406ea8467a89f3335c60d84dd11701177485fd
I'm confused with this patch. OpenBSD implementation returns EIO for too big buffers, although apparently this was not the behaviour from the start. Previously it filled up to 256 bytes, but also indicated how many bytes were returned. Your patch fills up to 256 bytes and does not tell the caller how many bytes it got, which is a big no-no. We should stick to what is in OpenBSD, so please update the patch to return EIO when needed. memset you put there is buggy. explicit_zero or equivalent is needed and if we don't have that in the kernel one will need to be ported as well. Cannot comment on arc4rand usefulness for this purpose though.
Alright except that meanwhile I already made the change to use explicit_bzero in the github repo. Otherwise ok I ll update.
Created attachment 149067 [details] Updated patch As advised, it returns EIO if the len exceeds 256 bytes. Plus a light update of the related man page. For reference the original OpenBSD man page. http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man2/getentropy.2
I'm no crypto expert. Turns out FreeBSD's arc4rand uses RC4, while OpenBSD's version uses something based on ChaCha which is stronger than RC4. I don't think we should provide a syscall with weaker randomness than one provided with original implementation.
I did as well an updated related to arc4random RC4 => Chacha 20 https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=182610
This is a part of private conversation with Theo de Raadt: OP> Can we somehow prove / verify, that OpenBSD's implementation is fine? OP> I know, that chacha based arc4random much better than (a)rc4 based, TdR> It is not significantly better than rc4.
(In reply to Mateusz Guzik from comment #5) > I don't think we should provide a syscall with weaker randomness than one > provided with original implementation. I think that's the wrong way to look at it; if we have a weak CSPRNG that needs to be fixed, regardless of the mechanism by which userland obtains entropy from kernel. I believe it is worthwhile for us to add the getentropy syscall, but the approach we use today (the kern.arandom sysctl) is in practice largely equivalent.
Seems like it could be implemented as a thin shim around the sysctl, no? Why do we need a syscall for it?
Btw, our kernel random has used chacha20 since 2017 -- r317015. I realize comment #5 dates from 2014, though.
> I think that's the wrong way to look at it; if we have a weak CSPRNG that needs to be fixed, regardless of the mechanism by which userland obtains entropy from kernel. The way of looking at it was that if the compatibility mechanism is to be provided it has to be not weaker than OpenBSD equivalent. Entropy for the rest of the kernel is a very different issue. > Seems like it could be implemented as a thin shim around the sysctl, no? Why do we need a syscall for it? sysctls are very slow, but that may be a somewhat weak point given the nature of the request. given the abundance of free syscall numbers and the fact other systems (OpenBSD, Linux) do it as a syscall I don't see a good reason to do it differently.
That was quite a time ago, I forgot in the meantime ... Is it still interesting enough to be considered ?
(In reply to David Carlier from comment #12) Yes, it's worth having.
So I had a closer look, linux added getrandom which is a slightly more elaborate version and getentropy can be emulated on top. I don't see any downsides to doing the equivalent.
Alright I would not expect it would get any attraction in fact :-) curious now how it will end up.
I'll take.
A commit references this bug: Author: cem Date: Wed Mar 21 01:15:47 UTC 2018 New revision: 331279 URL: https://svnweb.freebsd.org/changeset/base/331279 Log: Implement getrandom(2) and getentropy(3) The general idea here is to provide userspace programs with well-defined sources of entropy, in a fashion that doesn't require opening a new file descriptor (ulimits) or accessing paths (/dev/urandom may be restricted by chroot or capsicum). getrandom(2) is the more general API, and comes from the Linux world. Since our urandom and random devices are identical, the GRND_RANDOM flag is ignored. getentropy(3) is added as a compatibility shim for the OpenBSD API. truss(1) support is included. Tests for both system calls are provided. Coverage is believed to be at least as comprehensive as LTP getrandom(2) test coverage. Additionally, instructions for running the LTP tests directly against FreeBSD are provided in the "Test Plan" section of the Differential revision linked below. (They pass, of course.) PR: 194204 Reported by: David CARLIER <david.carlier AT hardenedbsd.org> Discussed with: cperciva, delphij, jhb, markj Relnotes: maybe Differential Revision: https://reviews.freebsd.org/D14500 Changes: head/include/unistd.h head/lib/libc/gen/Makefile.inc head/lib/libc/gen/Symbol.map head/lib/libc/gen/getentropy.3 head/lib/libc/gen/getentropy.c head/lib/libc/sys/Makefile.inc head/lib/libc/sys/Symbol.map head/lib/libc/sys/getrandom.2 head/lib/libc/tests/gen/Makefile head/lib/libc/tests/gen/getentropy_test.c head/sys/compat/freebsd32/syscalls.master head/sys/conf/files head/sys/kern/sys_getrandom.c head/sys/kern/syscalls.master head/sys/sys/random.h head/tests/sys/kern/Makefile head/tests/sys/kern/sys_getrandom.c head/usr.bin/truss/syscalls.c