| Message ID | 20180803131903.24303-1-blomqvist.janne@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Use getentropy() for seeding PRNG | expand |
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 (&b[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
On Fri, Aug 3, 2018 at 4:28 PM, Jakub Jelinek <jakub@redhat.com> 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 (&b[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 (&b[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?
> On Aug 3, 2018, at 9:19 AM, Janne Blomqvist <blomqvist.janne@gmail.com> 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
PING On Fri, Aug 3, 2018 at 5:05 PM, Janne Blomqvist <blomqvist.janne@gmail.com> wrote: > On Fri, Aug 3, 2018 at 4:28 PM, Jakub Jelinek <jakub@redhat.com> 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 (&b[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 (&b[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 >
On Fri, Aug 3, 2018 at 9:19 AM Janne Blomqvist <blomqvist.janne@gmail.com> 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
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 (&b[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
On Mon, Aug 13, 2018 at 5:36 PM, Fritz Reese <fritzoreese@gmail.com> wrote: > On Fri, Aug 3, 2018 at 9:19 AM Janne Blomqvist > <blomqvist.janne@gmail.com> 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.
On Mon, Aug 13, 2018 at 4:12 PM Janne Blomqvist <blomqvist.janne@gmail.com> wrote: > > On Mon, Aug 13, 2018 at 5:36 PM, Fritz Reese <fritzoreese@gmail.com> wrote: >> >> On Fri, Aug 3, 2018 at 9:19 AM Janne Blomqvist >> <blomqvist.janne@gmail.com> 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
Hi Janne, > PING > > On Fri, Aug 3, 2018 at 5:05 PM, Janne Blomqvist <blomqvist.janne@gmail.com> > wrote: > >> On Fri, Aug 3, 2018 at 4:28 PM, Jakub Jelinek <jakub@redhat.com> 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 (&b[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 (&b[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 <sys/random.h> 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
On Tue, Aug 14, 2018 at 11:18 PM, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote: > Hi Janne, > > > PING > > > > On Fri, Aug 3, 2018 at 5:05 PM, Janne Blomqvist < > blomqvist.janne@gmail.com> > > wrote: > > > >> On Fri, Aug 3, 2018 at 4:28 PM, Jakub Jelinek <jakub@redhat.com> 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 (&b[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 (&b[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 <sys/random.h> 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!
diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac index bf6d3634dda..900c7466dec 100644 --- a/libgfortran/configure.ac +++ b/libgfortran/configure.ac @@ -312,7 +312,8 @@ if test "${hardwire_newlib:-0}" -eq 1; then fi else AC_CHECK_FUNCS_ONCE(getrusage times mkstemp strtof strtold snprintf \ - ftruncate chsize chdir getlogin gethostname kill link symlink sleep ttyname \ + ftruncate chsize chdir getentropy getlogin gethostname kill link symlink \ + sleep ttyname \ alarm access fork setmode fcntl \ gettimeofday stat fstat lstat getpwuid vsnprintf dup \ getcwd localtime_r gmtime_r getpwuid_r ttyname_r clock_gettime \ diff --git a/libgfortran/intrinsics/random.c b/libgfortran/intrinsics/random.c index 234c5ff95fd..1f47f32188b 100644 --- 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 (&b[i]); return buflen; +#elif defined(HAVE_GETENTROPY) + return getentropy (buf, 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. */ int flags = O_RDONLY; #ifdef O_CLOEXEC flags |= O_CLOEXEC;