mbox series

[v4,0/2] eal: choose initial PRNG seed source at runtime

Message ID 20200422234255.7066-1-dg@adax.com (mailing list archive)
Headers
Series eal: choose initial PRNG seed source at runtime |

Message

Dan Gora April 22, 2020, 11:42 p.m. UTC
  Hi All,

The following patches updates the rte_random subsystem to dynamically find
the best source of the initial seed to the PRNG at run time.

The first patch enables dynamic checking for the rdseed instruction and
removes the requirement for it on the execution system.  It also ensures
that the code to use the rdseed instruction is generated, even if the host
compilation system does not support it (on x86 systems).

The second patch emulates the getentropy() glibc function by reading bytes
from /dev/urandom.  This removes an unnecessary dependency on glibc 2.25.

v4:  Note that emulating getentropy by reading from /dev/urandom should
never fail, so now the question is if we should just remove the rdseed
method entirely since it's x86 only?  Should we remove the fallback to
rte_get_tsc_cycles()?

Thanks
Dan

-----
v2:
* Fix patch apply issue.
* dlclose() handle if dlsym() fails in __rte_getentropy().

v3:
* Fix error checking of dlsym() in __rte_getentropy().
* Style changes recommended by Mattias.

v4:
* Replace dlopen/dlsym method with reading from /dev/urandom.
* Try rdseed method before getentropy() method since the
  latter should never fail.



Dan Gora (2):
  eal: check for rdseed at run time for random seed
  eal: emulate glibc getentropy for initial random seed

 config/x86/meson.build             | 11 +++--
 lib/librte_eal/common/rte_random.c | 79 ++++++++++++++++++++++++------
 lib/librte_eal/meson.build         |  3 --
 mk/rte.cpuflags.mk                 |  9 +++-
 4 files changed, 79 insertions(+), 23 deletions(-)
  

Comments

Mattias Rönnblom June 29, 2020, 9:32 a.m. UTC | #1
On 2020-04-23 01:42, Dan Gora wrote:
> Hi All,
>
> The following patches updates the rte_random subsystem to dynamically find
> the best source of the initial seed to the PRNG at run time.
>
> The first patch enables dynamic checking for the rdseed instruction and
> removes the requirement for it on the execution system.  It also ensures
> that the code to use the rdseed instruction is generated, even if the host
> compilation system does not support it (on x86 systems).
>
> The second patch emulates the getentropy() glibc function by reading bytes
> from /dev/urandom.  This removes an unnecessary dependency on glibc 2.25.
>
> v4:  Note that emulating getentropy by reading from /dev/urandom should
> never fail, so now the question is if we should just remove the rdseed
> method entirely since it's x86 only?  Should we remove the fallback to
> rte_get_tsc_cycles()?
>
> Thanks
> Dan
>
> -----
> v2:
> * Fix patch apply issue.
> * dlclose() handle if dlsym() fails in __rte_getentropy().
>
> v3:
> * Fix error checking of dlsym() in __rte_getentropy().
> * Style changes recommended by Mattias.
>
> v4:
> * Replace dlopen/dlsym method with reading from /dev/urandom.
> * Try rdseed method before getentropy() method since the
>    latter should never fail.


I see no reason to prefer rdseed over the kernel.


>
>
> Dan Gora (2):
>    eal: check for rdseed at run time for random seed
>    eal: emulate glibc getentropy for initial random seed
>
>   config/x86/meson.build             | 11 +++--
>   lib/librte_eal/common/rte_random.c | 79 ++++++++++++++++++++++++------
>   lib/librte_eal/meson.build         |  3 --
>   mk/rte.cpuflags.mk                 |  9 +++-
>   4 files changed, 79 insertions(+), 23 deletions(-)
>
  
Dan Gora June 29, 2020, 6:01 p.m. UTC | #2
On Mon, Jun 29, 2020 at 6:32 AM Mattias Rönnblom
<mattias.ronnblom@ericsson.com> wrote:
>
> On 2020-04-23 01:42, Dan Gora wrote:
> > Hi All,
> >
> > The following patches updates the rte_random subsystem to dynamically find
> > the best source of the initial seed to the PRNG at run time.
> >
> > The first patch enables dynamic checking for the rdseed instruction and
> > removes the requirement for it on the execution system.  It also ensures
> > that the code to use the rdseed instruction is generated, even if the host
> > compilation system does not support it (on x86 systems).
> >
> > The second patch emulates the getentropy() glibc function by reading bytes
> > from /dev/urandom.  This removes an unnecessary dependency on glibc 2.25.
> >
> > v4:  Note that emulating getentropy by reading from /dev/urandom should
> > never fail, so now the question is if we should just remove the rdseed
> > method entirely since it's x86 only?  Should we remove the fallback to
> > rte_get_tsc_cycles()?
> >
> > Thanks
> > Dan
> >
> > -----
> > v2:
> > * Fix patch apply issue.
> > * dlclose() handle if dlsym() fails in __rte_getentropy().
> >
> > v3:
> > * Fix error checking of dlsym() in __rte_getentropy().
> > * Style changes recommended by Mattias.
> >
> > v4:
> > * Replace dlopen/dlsym method with reading from /dev/urandom.
> > * Try rdseed method before getentropy() method since the
> >    latter should never fail.
>
>
> I see no reason to prefer rdseed over the kernel.

Let me give you a good reason.  The reason that this change was done...

Since rte_getentropy() is now available on all systems (with these
patches) and cannot fail. rdseed can either 1) go before
rte_getentropy() or 2) be completely removed.

What do you recommend?

thanks
dan
  
Dan Gora June 29, 2020, 6:04 p.m. UTC | #3
On Mon, Jun 29, 2020 at 3:01 PM Dan Gora <dg@adax.com> wrote:
>
> On Mon, Jun 29, 2020 at 6:32 AM Mattias Rönnblom
> <mattias.ronnblom@ericsson.com> wrote:
> >
> > On 2020-04-23 01:42, Dan Gora wrote:
> > > Hi All,
> > >
> > > The following patches updates the rte_random subsystem to dynamically find
> > > the best source of the initial seed to the PRNG at run time.
> > >
> > > The first patch enables dynamic checking for the rdseed instruction and
> > > removes the requirement for it on the execution system.  It also ensures
> > > that the code to use the rdseed instruction is generated, even if the host
> > > compilation system does not support it (on x86 systems).
> > >
> > > The second patch emulates the getentropy() glibc function by reading bytes
> > > from /dev/urandom.  This removes an unnecessary dependency on glibc 2.25.
> > >
> > > v4:  Note that emulating getentropy by reading from /dev/urandom should
> > > never fail, so now the question is if we should just remove the rdseed
> > > method entirely since it's x86 only?  Should we remove the fallback to
> > > rte_get_tsc_cycles()?
> > >
> > > Thanks
> > > Dan
> > >
> > > -----
> > > v2:
> > > * Fix patch apply issue.
> > > * dlclose() handle if dlsym() fails in __rte_getentropy().
> > >
> > > v3:
> > > * Fix error checking of dlsym() in __rte_getentropy().
> > > * Style changes recommended by Mattias.
> > >
> > > v4:
> > > * Replace dlopen/dlsym method with reading from /dev/urandom.
> > > * Try rdseed method before getentropy() method since the
> > >    latter should never fail.
> >
> >
> > I see no reason to prefer rdseed over the kernel.

It even says why right here:

> > > * Try rdseed method before getentropy() method since the
> > >    latter should never fail.

*******since the latter should never fail*******

Did  you see that part?

thanks
dan
  
Mattias Rönnblom June 29, 2020, 9:05 p.m. UTC | #4
On 2020-06-29 20:01, Dan Gora wrote:
> On Mon, Jun 29, 2020 at 6:32 AM Mattias Rönnblom
> <mattias.ronnblom@ericsson.com> wrote:
>> On 2020-04-23 01:42, Dan Gora wrote:
>>> Hi All,
>>>
>>> The following patches updates the rte_random subsystem to dynamically find
>>> the best source of the initial seed to the PRNG at run time.
>>>
>>> The first patch enables dynamic checking for the rdseed instruction and
>>> removes the requirement for it on the execution system.  It also ensures
>>> that the code to use the rdseed instruction is generated, even if the host
>>> compilation system does not support it (on x86 systems).
>>>
>>> The second patch emulates the getentropy() glibc function by reading bytes
>>> from /dev/urandom.  This removes an unnecessary dependency on glibc 2.25.
>>>
>>> v4:  Note that emulating getentropy by reading from /dev/urandom should
>>> never fail, so now the question is if we should just remove the rdseed
>>> method entirely since it's x86 only?  Should we remove the fallback to
>>> rte_get_tsc_cycles()?
>>>
>>> Thanks
>>> Dan
>>>
>>> -----
>>> v2:
>>> * Fix patch apply issue.
>>> * dlclose() handle if dlsym() fails in __rte_getentropy().
>>>
>>> v3:
>>> * Fix error checking of dlsym() in __rte_getentropy().
>>> * Style changes recommended by Mattias.
>>>
>>> v4:
>>> * Replace dlopen/dlsym method with reading from /dev/urandom.
>>> * Try rdseed method before getentropy() method since the
>>>     latter should never fail.
>>
>> I see no reason to prefer rdseed over the kernel.
> Let me give you a good reason.  The reason that this change was done...
>
> Since rte_getentropy() is now available on all systems (with these
> patches) and cannot fail. rdseed can either 1) go before
> rte_getentropy() or 2) be completely removed.
>

It's unlikely to fail, and if it does something is probably seriously 
wrong with your system. You also seem to think it might fail, since you 
take great care of setting errno and having non-zero return code in 
__rte_getentropy().


 From what I recall, it was Bruce that suggested rdseed should be 
included as one of the sources. I have no opinion on that particular 
subject, other than I think kernel-originated randomness should have 
priority.
  
Dan Gora June 29, 2020, 9:14 p.m. UTC | #5
On Mon, Jun 29, 2020 at 6:06 PM Mattias Rönnblom
<mattias.ronnblom@ericsson.com> wrote:
>
> It's unlikely to fail, and if it does something is probably seriously
> wrong with your system. You also seem to think it might fail, since you
> take great care of setting errno and having non-zero return code in
> __rte_getentropy().

Well, no, I don't personally think that it will fail and certainly
even if a read call to /dev/random fails, the whole function will not
fail.  As I said, I was trying to emulate the glibc getentropy and all
its semantics.

>  From what I recall, it was Bruce that suggested rdseed should be
> included as one of the sources. I have no opinion on that particular
> subject, other than I think kernel-originated randomness should have
> priority.

In light of new attacks on rdseed, it should probably just be removed, IMHO.
https://www.phoronix.com/scan.php?page=article&item=crosstalk-srbds-vulnerability&num=1

thanks
d