Use getentropy() for seeding PRNG

Message ID 20180803131903.24303-1-blomqvist.janne@gmail.com
State New
Headers show
Series
  • Use getentropy() for seeding PRNG
Related show

Commit Message

Janne Blomqvist Aug. 3, 2018, 1:19 p.m.
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?

2018-08-03  Janne Blomqvist  <jb@gcc.gnu.org>

	* configure.ac: Check for getentropy.
	* intrinsics/random.c (getosrandom): Use getentropy if available.
	* config.h.in: Regenerated.
	* configure: Regenerated.
---
 libgfortran/configure.ac        | 3 ++-
 libgfortran/intrinsics/random.c | 7 ++-----
 2 files changed, 4 insertions(+), 6 deletions(-)

Comments

Jakub Jelinek Aug. 3, 2018, 1:28 p.m. | #1
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
Janne Blomqvist Aug. 3, 2018, 2:05 p.m. | #2
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?
Paul Koning Aug. 3, 2018, 2:48 p.m. | #3
> 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
Janne Blomqvist Aug. 13, 2018, 10:12 a.m. | #4
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
>
Fritz Reese Aug. 13, 2018, 2:36 p.m. | #5
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
Jakub Jelinek Aug. 13, 2018, 8:07 p.m. | #6
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
Janne Blomqvist Aug. 13, 2018, 8:11 p.m. | #7
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.
Fritz Reese Aug. 14, 2018, 1:59 p.m. | #8
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
Rainer Orth Aug. 14, 2018, 8:18 p.m. | #9
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
Janne Blomqvist Aug. 14, 2018, 8:29 p.m. | #10
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!

Patch

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;