Re: [PATCH] Use getentropy() for seeding PRNG
On Tue, Aug 14, 2018 at 11:18 PM, Rainer Orth wrote: > Hi Janne, > > > PING > > > > On Fri, Aug 3, 2018 at 5:05 PM, Janne Blomqvist < > blomqvist.ja...@gmail.com> > > wrote: > > > >> On Fri, Aug 3, 2018 at 4:28 PM, Jakub Jelinek wrote: > >> > >>> On Fri, Aug 03, 2018 at 04:19:03PM +0300, Janne Blomqvist wrote: > >>> > --- a/libgfortran/intrinsics/random.c > >>> > +++ b/libgfortran/intrinsics/random.c > >>> > @@ -309,12 +309,9 @@ getosrandom (void *buf, size_t buflen) > >>> >for (size_t i = 0; i < buflen / sizeof (unsigned int); i++) > >>> > rand_s ([i]); > >>> >return buflen; > >>> > +#elif defined(HAVE_GETENTROPY) > >>> > + return getentropy (buf, buflen); > >>> > #else > >>> > >>> I wonder if it wouldn't be better to use getentropy only if it is > >>> successful > >>> and fall back to whatever you were doing before. > >>> > >>> E.g. on Linux, I believe getentropy in glibc just uses the getrandom > >>> syscall, which has only been introduced in Linux kernel 3.17. > >>> > >> > >> Yes, that is my understanding as well. > >> > >> > >>> So, if somebody is running glibc 2.25 or later on kernel < 3.17, it > will > >>> fail, even though reads from /dev/urandom could work. > >>> > >> > >> Hmm, fair enough. So replace the random.c part of the patch with > >> > >> diff --git a/libgfortran/intrinsics/random.c b/libgfortran/intrinsics/ > >> random.c > >> index 234c5ff95fd..229fa6995c0 100644 > >> --- a/libgfortran/intrinsics/random.c > >> +++ b/libgfortran/intrinsics/random.c > >> @@ -310,11 +310,10 @@ getosrandom (void *buf, size_t buflen) > >> rand_s ([i]); > >>return buflen; > >> #else > >> - /* > >> - TODO: When glibc adds a wrapper for the getrandom() system call > >> - on Linux, one could use that. > >> - > >> - TODO: One could use getentropy() on OpenBSD. */ > >> +#ifdef HAVE_GETENTROPY > >> + if (getentropy (buf, buflen) == 0) > >> +return 0; > >> +#endif > >>int flags = O_RDONLY; > >> #ifdef O_CLOEXEC > >>flags |= O_CLOEXEC; > >> > >> > >> > >> Just to be sure, I regtested this slightly modified patch as well. Ok > for > >> trunk? > > the patch broke Solaris 11.3+ bootstrap: > > /vol/gcc/src/hg/trunk/local/libgfortran/intrinsics/random.c: In function > 'getosrandom': > /vol/gcc/src/hg/trunk/local/libgfortran/intrinsics/random.c:314:7: error: > implicit declaration of function 'getentropy'; did you mean 'get_nprocs'? > [-Werror=implicit-function-declaration] > 314 | if (getentropy (buf, buflen) == 0) > | ^~ > | get_nprocs > > > According to the manpage, one needs to include to get the > getentropy declaration. > > Fixed as follows. Bootstraps on i386-pc-solaris2.11, > i386-pc-solaris2.10 (which lacks getentropy), sparc-sun-solaris2.11, and > x86_64-pc-linux-gnu still running, but all already into make check. > > Ok for mainline? > > Rainer > > Sorry about that, I wasn't aware that Solaris also has added getentropy. Patch looks good, Ok for trunk! -- Janne Blomqvist
Re: [PATCH] Use getentropy() for seeding PRNG
Hi Janne, > PING > > On Fri, Aug 3, 2018 at 5:05 PM, Janne Blomqvist > wrote: > >> On Fri, Aug 3, 2018 at 4:28 PM, Jakub Jelinek wrote: >> >>> On Fri, Aug 03, 2018 at 04:19:03PM +0300, Janne Blomqvist wrote: >>> > --- a/libgfortran/intrinsics/random.c >>> > +++ b/libgfortran/intrinsics/random.c >>> > @@ -309,12 +309,9 @@ getosrandom (void *buf, size_t buflen) >>> >for (size_t i = 0; i < buflen / sizeof (unsigned int); i++) >>> > rand_s ([i]); >>> >return buflen; >>> > +#elif defined(HAVE_GETENTROPY) >>> > + return getentropy (buf, buflen); >>> > #else >>> >>> I wonder if it wouldn't be better to use getentropy only if it is >>> successful >>> and fall back to whatever you were doing before. >>> >>> E.g. on Linux, I believe getentropy in glibc just uses the getrandom >>> syscall, which has only been introduced in Linux kernel 3.17. >>> >> >> Yes, that is my understanding as well. >> >> >>> So, if somebody is running glibc 2.25 or later on kernel < 3.17, it will >>> fail, even though reads from /dev/urandom could work. >>> >> >> Hmm, fair enough. So replace the random.c part of the patch with >> >> diff --git a/libgfortran/intrinsics/random.c b/libgfortran/intrinsics/ >> random.c >> index 234c5ff95fd..229fa6995c0 100644 >> --- a/libgfortran/intrinsics/random.c >> +++ b/libgfortran/intrinsics/random.c >> @@ -310,11 +310,10 @@ getosrandom (void *buf, size_t buflen) >> rand_s ([i]); >>return buflen; >> #else >> - /* >> - TODO: When glibc adds a wrapper for the getrandom() system call >> - on Linux, one could use that. >> - >> - TODO: One could use getentropy() on OpenBSD. */ >> +#ifdef HAVE_GETENTROPY >> + if (getentropy (buf, buflen) == 0) >> +return 0; >> +#endif >>int flags = O_RDONLY; >> #ifdef O_CLOEXEC >>flags |= O_CLOEXEC; >> >> >> >> Just to be sure, I regtested this slightly modified patch as well. Ok for >> trunk? the patch broke Solaris 11.3+ bootstrap: /vol/gcc/src/hg/trunk/local/libgfortran/intrinsics/random.c: In function 'getosrandom': /vol/gcc/src/hg/trunk/local/libgfortran/intrinsics/random.c:314:7: error: implicit declaration of function 'getentropy'; did you mean 'get_nprocs'? [-Werror=implicit-function-declaration] 314 | if (getentropy (buf, buflen) == 0) | ^~ | get_nprocs According to the manpage, one needs to include to get the getentropy declaration. Fixed as follows. Bootstraps on i386-pc-solaris2.11, i386-pc-solaris2.10 (which lacks getentropy), sparc-sun-solaris2.11, and x86_64-pc-linux-gnu still running, but all already into make check. Ok for mainline? Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University 2018-08-14 Rainer Orth * configure.ac: Check for . * configure, config.h.in: Regenerate. * intrinsics/random.c [HAVE_SYS_RANDOM_H]: Include . # HG changeset patch # Parent 8cc4b3b19e87eb3221d688c25887efd199d44f89 Include for getentropy on Solaris diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac --- a/libgfortran/configure.ac +++ b/libgfortran/configure.ac @@ -275,8 +275,9 @@ AC_TYPE_UINTPTR_T AC_CHECK_TYPES([ptrdiff_t]) # check header files (we assume C89 is available, so don't check for that) -AC_CHECK_HEADERS_ONCE(unistd.h sys/time.h sys/times.h sys/resource.h \ -sys/types.h sys/stat.h sys/wait.h floatingpoint.h ieeefp.h fenv.h fptrap.h \ +AC_CHECK_HEADERS_ONCE(unistd.h sys/random.h sys/time.h sys/times.h \ +sys/resource.h sys/types.h sys/stat.h sys/wait.h \ +floatingpoint.h ieeefp.h fenv.h fptrap.h \ fpxcp.h pwd.h complex.h xlocale.h) GCC_HEADER_STDINT(gstdint.h) diff --git a/libgfortran/intrinsics/random.c b/libgfortran/intrinsics/random.c --- a/libgfortran/intrinsics/random.c +++ b/libgfortran/intrinsics/random.c @@ -37,6 +37,9 @@ see the files COPYING3 and COPYING.RUNTI #include #include #include "time_1.h" +#ifdef HAVE_SYS_RANDOM_H +#include +#endif #ifdef __MINGW32__ #define HAVE_GETPID 1
Re: [PATCH] Use getentropy() for seeding PRNG
On Mon, Aug 13, 2018 at 4:12 PM Janne Blomqvist wrote: > > On Mon, Aug 13, 2018 at 5:36 PM, Fritz Reese wrote: >> >> On Fri, Aug 3, 2018 at 9:19 AM Janne Blomqvist >> wrote: >> > >> > The getentropy function, found on Linux, OpenBSD, and recently also >> > FreeBSD, can be used to get random bytes to initialize the PRNG. It >> > is similar to the traditional way of reading from /dev/urandom, but >> > being a system call rather than a special file, it doesn't suffer from >> > problems like running out of file descriptors, or failure when running >> > in a container where /dev/urandom is not available. >> > >> > Regtested on x86_64-pc-linux-gnu, Ok for trunk? >> >> Actually, getentropy() is similar to reading from /dev/random, where >> getrandom() is similar to reading from /dev/urandom. > > > No, getentropy is similar to getrandom with the flags argument == 0. Which is > similar to reading /dev/urandom, except that just after boot if enough > entropy hasn't yet been gathered, it may block instead of returning some > not-quite-random data. But once it has been initialized, it will never block > again. > > I agree that reading from /dev/random is overkill, but this patch isn't doing > the equivalent of that. > Fair enough, I misread the documentation on getentropy(). Then I concur with Jakub, patch looks OK. Fritz
Re: [PATCH] Use getentropy() for seeding PRNG
On Mon, Aug 13, 2018 at 5:36 PM, Fritz Reese wrote: > On Fri, Aug 3, 2018 at 9:19 AM Janne Blomqvist > wrote: > > > > The getentropy function, found on Linux, OpenBSD, and recently also > > FreeBSD, can be used to get random bytes to initialize the PRNG. It > > is similar to the traditional way of reading from /dev/urandom, but > > being a system call rather than a special file, it doesn't suffer from > > problems like running out of file descriptors, or failure when running > > in a container where /dev/urandom is not available. > > > > Regtested on x86_64-pc-linux-gnu, Ok for trunk? > > Actually, getentropy() is similar to reading from /dev/random, where > getrandom() is similar to reading from /dev/urandom. No, getentropy is similar to getrandom with the flags argument == 0. Which is similar to reading /dev/urandom, except that just after boot if enough entropy hasn't yet been gathered, it may block instead of returning some not-quite-random data. But once it has been initialized, it will never block again. I agree that reading from /dev/random is overkill, but this patch isn't doing the equivalent of that. > Since the > original behavior of getosrandom() is to read from /dev/urandom, I > think it is better to use getrandom() for consistent semantics. > > Furthermore, getentropy() may block to achieve an appropriate degree > of randomness, since it is intended for secure use. The only time this might happen is just after boot, after that the entropy never drains (in contrast to /dev/random). So unless you're planning to write an init daemon in Fortran, this shouldn't matter. -- Janne Blomqvist
Re: [PATCH] Use getentropy() for seeding PRNG
On Mon, Aug 13, 2018 at 01:12:12PM +0300, Janne Blomqvist wrote: > PING LGTM. > > diff --git a/libgfortran/intrinsics/random.c b/libgfortran/intrinsics/ > > random.c > > index 234c5ff95fd..229fa6995c0 100644 > > --- a/libgfortran/intrinsics/random.c > > +++ b/libgfortran/intrinsics/random.c > > @@ -310,11 +310,10 @@ getosrandom (void *buf, size_t buflen) > > rand_s ([i]); > >return buflen; > > #else > > - /* > > - TODO: When glibc adds a wrapper for the getrandom() system call > > - on Linux, one could use that. > > - > > - TODO: One could use getentropy() on OpenBSD. */ > > +#ifdef HAVE_GETENTROPY > > + if (getentropy (buf, buflen) == 0) > > +return 0; > > +#endif > >int flags = O_RDONLY; > > #ifdef O_CLOEXEC > >flags |= O_CLOEXEC; > > > > > > > > Just to be sure, I regtested this slightly modified patch as well. Ok for > > trunk? > > > > -- > > Janne Blomqvist > > Jakub
Re: [PATCH] Use getentropy() for seeding PRNG
On Fri, Aug 3, 2018 at 9:19 AM Janne Blomqvist wrote: > > The getentropy function, found on Linux, OpenBSD, and recently also > FreeBSD, can be used to get random bytes to initialize the PRNG. It > is similar to the traditional way of reading from /dev/urandom, but > being a system call rather than a special file, it doesn't suffer from > problems like running out of file descriptors, or failure when running > in a container where /dev/urandom is not available. > > Regtested on x86_64-pc-linux-gnu, Ok for trunk? Actually, getentropy() is similar to reading from /dev/random, where getrandom() is similar to reading from /dev/urandom. Since the original behavior of getosrandom() is to read from /dev/urandom, I think it is better to use getrandom() for consistent semantics. Furthermore, getentropy() may block to achieve an appropriate degree of randomness, since it is intended for secure use. Of course such block time would hardly be noticeable for a one-time read of a thousand bits or so... but on principle I think we should provide a quick cheesy seed by default, and the user may provide his own seed if he wants expensive secure bits. Just my opinion. I am personally OK with the [second version of the] patch | sed s/getentropy/getrandom/g. Fritz
Re: [PATCH] Use getentropy() for seeding PRNG
PING On Fri, Aug 3, 2018 at 5:05 PM, Janne Blomqvist wrote: > On Fri, Aug 3, 2018 at 4:28 PM, Jakub Jelinek wrote: > >> On Fri, Aug 03, 2018 at 04:19:03PM +0300, Janne Blomqvist wrote: >> > --- a/libgfortran/intrinsics/random.c >> > +++ b/libgfortran/intrinsics/random.c >> > @@ -309,12 +309,9 @@ getosrandom (void *buf, size_t buflen) >> >for (size_t i = 0; i < buflen / sizeof (unsigned int); i++) >> > rand_s ([i]); >> >return buflen; >> > +#elif defined(HAVE_GETENTROPY) >> > + return getentropy (buf, buflen); >> > #else >> >> I wonder if it wouldn't be better to use getentropy only if it is >> successful >> and fall back to whatever you were doing before. >> >> E.g. on Linux, I believe getentropy in glibc just uses the getrandom >> syscall, which has only been introduced in Linux kernel 3.17. >> > > Yes, that is my understanding as well. > > >> So, if somebody is running glibc 2.25 or later on kernel < 3.17, it will >> fail, even though reads from /dev/urandom could work. >> > > Hmm, fair enough. So replace the random.c part of the patch with > > diff --git a/libgfortran/intrinsics/random.c b/libgfortran/intrinsics/ > random.c > index 234c5ff95fd..229fa6995c0 100644 > --- a/libgfortran/intrinsics/random.c > +++ b/libgfortran/intrinsics/random.c > @@ -310,11 +310,10 @@ getosrandom (void *buf, size_t buflen) > rand_s ([i]); >return buflen; > #else > - /* > - TODO: When glibc adds a wrapper for the getrandom() system call > - on Linux, one could use that. > - > - TODO: One could use getentropy() on OpenBSD. */ > +#ifdef HAVE_GETENTROPY > + if (getentropy (buf, buflen) == 0) > +return 0; > +#endif >int flags = O_RDONLY; > #ifdef O_CLOEXEC >flags |= O_CLOEXEC; > > > > Just to be sure, I regtested this slightly modified patch as well. Ok for > trunk? > > -- > Janne Blomqvist > -- Janne Blomqvist
Re: [PATCH] Use getentropy() for seeding PRNG
> On Aug 3, 2018, at 9:19 AM, Janne Blomqvist wrote: > > The getentropy function, found on Linux, OpenBSD, and recently also > FreeBSD, can be used to get random bytes to initialize the PRNG. It > is similar to the traditional way of reading from /dev/urandom, but > being a system call rather than a special file, it doesn't suffer from > problems like running out of file descriptors, or failure when running > in a container where /dev/urandom is not available. I don't understand why this is useful. getrandom, and /dev/random, are for strong (secure) RNGs. A PRNG is something entirely different. By saying we use entropy to seed it, we blur the distinction and create the false impression that the PRNG has security properties. It would be better to initialize with something more obviously insecure, like gettimeofday(). paul
Re: [PATCH] Use getentropy() for seeding PRNG
On Fri, Aug 3, 2018 at 4:28 PM, Jakub Jelinek wrote: > On Fri, Aug 03, 2018 at 04:19:03PM +0300, Janne Blomqvist wrote: > > --- a/libgfortran/intrinsics/random.c > > +++ b/libgfortran/intrinsics/random.c > > @@ -309,12 +309,9 @@ getosrandom (void *buf, size_t buflen) > >for (size_t i = 0; i < buflen / sizeof (unsigned int); i++) > > rand_s ([i]); > >return buflen; > > +#elif defined(HAVE_GETENTROPY) > > + return getentropy (buf, buflen); > > #else > > I wonder if it wouldn't be better to use getentropy only if it is > successful > and fall back to whatever you were doing before. > > E.g. on Linux, I believe getentropy in glibc just uses the getrandom > syscall, which has only been introduced in Linux kernel 3.17. > Yes, that is my understanding as well. > So, if somebody is running glibc 2.25 or later on kernel < 3.17, it will > fail, even though reads from /dev/urandom could work. > Hmm, fair enough. So replace the random.c part of the patch with diff --git a/libgfortran/intrinsics/random.c b/libgfortran/intrinsics/random.c index 234c5ff95fd..229fa6995c0 100644 --- a/libgfortran/intrinsics/random.c +++ b/libgfortran/intrinsics/random.c @@ -310,11 +310,10 @@ getosrandom (void *buf, size_t buflen) rand_s ([i]); return buflen; #else - /* - TODO: When glibc adds a wrapper for the getrandom() system call - on Linux, one could use that. - - TODO: One could use getentropy() on OpenBSD. */ +#ifdef HAVE_GETENTROPY + if (getentropy (buf, buflen) == 0) +return 0; +#endif int flags = O_RDONLY; #ifdef O_CLOEXEC flags |= O_CLOEXEC; Just to be sure, I regtested this slightly modified patch as well. Ok for trunk? -- Janne Blomqvist
Re: [PATCH] Use getentropy() for seeding PRNG
On Fri, Aug 03, 2018 at 04:19:03PM +0300, Janne Blomqvist wrote: > --- a/libgfortran/intrinsics/random.c > +++ b/libgfortran/intrinsics/random.c > @@ -309,12 +309,9 @@ getosrandom (void *buf, size_t buflen) >for (size_t i = 0; i < buflen / sizeof (unsigned int); i++) > rand_s ([i]); >return buflen; > +#elif defined(HAVE_GETENTROPY) > + return getentropy (buf, buflen); > #else I wonder if it wouldn't be better to use getentropy only if it is successful and fall back to whatever you were doing before. E.g. on Linux, I believe getentropy in glibc just uses the getrandom syscall, which has only been introduced in Linux kernel 3.17. So, if somebody is running glibc 2.25 or later on kernel < 3.17, it will fail, even though reads from /dev/urandom could work. > - /* > - TODO: When glibc adds a wrapper for the getrandom() system call > - on Linux, one could use that. > - > - TODO: One could use getentropy() on OpenBSD. */ >int flags = O_RDONLY; > #ifdef O_CLOEXEC >flags |= O_CLOEXEC; Jakub
