[Buildroot] [PATCH] package/rng-tools: make jitterentropy conditional

[Buildroot] [PATCH] package/rng-tools: make jitterentropy conditional

[Matt Weber]

The update of rng-tools from 5 to 6.7 introduced a change where
the jitterentropy library was enabled by default instead of
returning a special 66 return code to hangle the case of no
hwrng. This patch reverts that change and allows a user to
select when to enable the jitterentropy source. The bug
documents an issue of when a hwrng is enabled with jitterentropy
there is a longer boot time.

Fixes:
https://bugs.busybox.net/show_bug.cgi?id=12511

Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
---
 package/rng-tools/Config.in    | 1 -
 package/rng-tools/rng-tools.mk | 9 ++++++++-
 package/rng-tools/rngd.service | 1 +
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/package/rng-tools/Config.in b/package/rng-tools/Config.in
index 71514260e6..ddcc221d06 100644
--- a/package/rng-tools/Config.in
+++ b/package/rng-tools/Config.in
@@ -4,7 +4,6 @@ config BR2_PACKAGE_RNG_TOOLS
 	# pthread_setaffinity_np
 	depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL
 	select BR2_PACKAGE_ARGP_STANDALONE if BR2_TOOLCHAIN_USES_UCLIBC || BR2_TOOLCHAIN_USES_MUSL
-	select BR2_PACKAGE_JITTERENTROPY_LIBRARY
 	# For rdrand & darn ligcrypt is required and it's not obvious to users
 	select BR2_PACKAGE_LIBGCRYPT if BR2_i386 || BR2_x86_64 || BR2_powerpc64le
 	select BR2_PACKAGE_LIBSYSFS
diff --git a/package/rng-tools/rng-tools.mk b/package/rng-tools/rng-tools.mk
index 274079044c..e458fbb8eb 100644
--- a/package/rng-tools/rng-tools.mk
+++ b/package/rng-tools/rng-tools.mk
@@ -8,7 +8,7 @@ RNG_TOOLS_VERSION = 6.8
 RNG_TOOLS_SITE = $(call github,nhorman,$(RNG_TOOLS_NAME),v$(RNG_TOOLS_VERSION))
 RNG_TOOLS_LICENSE = GPL-2.0
 RNG_TOOLS_LICENSE_FILES = COPYING
-RNG_TOOLS_DEPENDENCIES = libsysfs jitterentropy-library host-pkgconf
+RNG_TOOLS_DEPENDENCIES = libsysfs host-pkgconf
 # From git
 RNG_TOOLS_AUTORECONF = YES
 
@@ -29,6 +29,13 @@ else
 RNG_TOOLS_CONF_OPTS += --without-libgcrypt
 endif
 
+ifeq ($(BR2_PACKAGE_JITTERENTROPY_LIBRARY),y)
+RNG_TOOLS_DEPENDENCIES += jitterentropy-library
+RNG_TOOLS_CONF_OPTS += --enable-jitterentropy
+else
+RNG_TOOLS_CONF_OPTS += --disable-jitterentropy
+endif
+
 define RNG_TOOLS_INSTALL_INIT_SYSV
 	$(INSTALL) -D -m 755 package/rng-tools/S21rngd \
 		$(TARGET_DIR)/etc/init.d/S21rngd
diff --git a/package/rng-tools/rngd.service b/package/rng-tools/rngd.service
index 11386d1e5d..c0bcffe59e 100644
--- a/package/rng-tools/rngd.service
+++ b/package/rng-tools/rngd.service
@@ -3,6 +3,7 @@ Description=Hardware RNG Entropy Gatherer Daemon
 
 [Service]
 ExecStart=/usr/sbin/rngd -f $DAEMON_ARGS
+SuccessExitStatus=66
 EnvironmentFile=-/etc/default/rngd
 
 [Install]
-- 
2.17.1

[Buildroot] [PATCH] package/rng-tools: make jitterentropy conditional

[Thomas Petazzoni]

On Wed,  5 Feb 2020 12:26:08 -0600
Matt Weber <matthew.weber@rockwellcollins.com> wrote:

> The update of rng-tools from 5 to 6.7 introduced a change where
> the jitterentropy library was enabled by default instead of
> returning a special 66 return code to hangle the case of no
> hwrng. This patch reverts that change and allows a user to
> select when to enable the jitterentropy source. The bug
> documents an issue of when a hwrng is enabled with jitterentropy
> there is a longer boot time.

I don't understand this story of the 66 return code. Could you explain
a bit more ?

> index 11386d1e5d..c0bcffe59e 100644
> --- a/package/rng-tools/rngd.service
> +++ b/package/rng-tools/rngd.service
> @@ -3,6 +3,7 @@ Description=Hardware RNG Entropy Gatherer Daemon
>  
>  [Service]
>  ExecStart=/usr/sbin/rngd -f $DAEMON_ARGS
> +SuccessExitStatus=66

Will this work even if jitterentropy support is enabled ?

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] package/rng-tools: make jitterentropy conditional

[Matthew Weber]

Thomas,

On Wed, Feb 5, 2020 at 1:33 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> On Wed,  5 Feb 2020 12:26:08 -0600
> Matt Weber <matthew.weber@rockwellcollins.com> wrote:
>
> > The update of rng-tools from 5 to 6.7 introduced a change where
> > the jitterentropy library was enabled by default instead of
> > returning a special 66 return code to hangle the case of no
> > hwrng. This patch reverts that change and allows a user to
> > select when to enable the jitterentropy source. The bug
> > documents an issue of when a hwrng is enabled with jitterentropy
> > there is a longer boot time.
>
> I don't understand this story of the 66 return code. Could you explain
> a bit more ?

In Buildroot commit 22cb51e1 the systemd support addition included the
successcode change based on the Fedora bugfix for ignoring if no hwrng
is present (https://bugzilla.redhat.com/show_bug.cgi?id=892178).

>
> > index 11386d1e5d..c0bcffe59e 100644
> > --- a/package/rng-tools/rngd.service
> > +++ b/package/rng-tools/rngd.service
> > @@ -3,6 +3,7 @@ Description=Hardware RNG Entropy Gatherer Daemon
> >
> >  [Service]
> >  ExecStart=/usr/sbin/rngd -f $DAEMON_ARGS
> > +SuccessExitStatus=66
>
> Will this work even if jitterentropy support is enabled ?

My understanding is that you'd never hit this return case when
jitterentropy support built in as it would always init the entropy
pool using that library as a source.  Thus the problem of the boot
delay even when hwrng is present.

Matt

[Buildroot] [PATCH] package/rng-tools: make jitterentropy conditional

[Matthew Weber]

Thomas,


On Wed, Feb 5, 2020 at 2:46 PM Matthew Weber
<matthew.weber@rockwellcollins.com> wrote:
>
> Thomas,
>
> On Wed, Feb 5, 2020 at 1:33 PM Thomas Petazzoni
> <thomas.petazzoni@bootlin.com> wrote:
> >
> > On Wed,  5 Feb 2020 12:26:08 -0600
> > Matt Weber <matthew.weber@rockwellcollins.com> wrote:
> >
> > > The update of rng-tools from 5 to 6.7 introduced a change where
> > > the jitterentropy library was enabled by default instead of
> > > returning a special 66 return code to hangle the case of no
> > > hwrng. This patch reverts that change and allows a user to
> > > select when to enable the jitterentropy source. The bug
> > > documents an issue of when a hwrng is enabled with jitterentropy
> > > there is a longer boot time.
> >
> > I don't understand this story of the 66 return code. Could you explain
> > a bit more ?
>
> In Buildroot commit 22cb51e1 the systemd support addition included the
> successcode change based on the Fedora bugfix for ignoring if no hwrng
> is present (https://bugzilla.redhat.com/show_bug.cgi?id=892178).

Ryan Barnett noticed that rng-tools dropped the special return code.
I guess we could just drop support for this special case as well....
https://github.com/nhorman/rng-tools/blob/v6.8/rngd.c#L805

>
> >
> > > index 11386d1e5d..c0bcffe59e 100644
> > > --- a/package/rng-tools/rngd.service
> > > +++ b/package/rng-tools/rngd.service
> > > @@ -3,6 +3,7 @@ Description=Hardware RNG Entropy Gatherer Daemon
> > >
> > >  [Service]
> > >  ExecStart=/usr/sbin/rngd -f $DAEMON_ARGS
> > > +SuccessExitStatus=66
> >
> > Will this work even if jitterentropy support is enabled ?
>
> My understanding is that you'd never hit this return case when
> jitterentropy support built in as it would always init the entropy
> pool using that library as a source.  Thus the problem of the boot
> delay even when hwrng is present.
>
> Matt



-- 

Matthew Weber | Associate Director Software Engineer | Commercial Avionics

COLLINS AEROSPACE

400 Collins Road NE, Cedar Rapids, Iowa 52498, USA

Tel: +1 319 295 7349 | FAX: +1 319 263 6099

matthew.weber at collins.com | collinsaerospace.com



CONFIDENTIALITY WARNING: This message may contain proprietary and/or
privileged information of Collins Aerospace and its affiliated
companies. If you are not the intended recipient, please 1) Do not
disclose, copy, distribute or use this message or its contents. 2)
Advise the sender by return email. 3) Delete all copies (including all
attachments) from your computer. Your cooperation is greatly
appreciated.


Any export restricted material should be shared using my
matthew.weber at corp.rockwellcollins.com address.

[Buildroot] [PATCH] package/rng-tools: make jitterentropy conditional

[Peter Korsgaard]

>>>>> "Matthew" == Matthew Weber <matthew.weber@rockwellcollins.com> writes:

 > Thomas,
 > On Wed, Feb 5, 2020 at 2:46 PM Matthew Weber
 > <matthew.weber@rockwellcollins.com> wrote:
 >> 
 >> Thomas,
 >> 
 >> On Wed, Feb 5, 2020 at 1:33 PM Thomas Petazzoni
 >> <thomas.petazzoni@bootlin.com> wrote:
 >> >
 >> > On Wed,  5 Feb 2020 12:26:08 -0600
 >> > Matt Weber <matthew.weber@rockwellcollins.com> wrote:
 >> >
 >> > > The update of rng-tools from 5 to 6.7 introduced a change where
 >> > > the jitterentropy library was enabled by default instead of
 >> > > returning a special 66 return code to hangle the case of no
 >> > > hwrng. This patch reverts that change and allows a user to
 >> > > select when to enable the jitterentropy source. The bug
 >> > > documents an issue of when a hwrng is enabled with jitterentropy
 >> > > there is a longer boot time.
 >> >
 >> > I don't understand this story of the 66 return code. Could you explain
 >> > a bit more ?
 >> 
 >> In Buildroot commit 22cb51e1 the systemd support addition included the
 >> successcode change based on the Fedora bugfix for ignoring if no hwrng
 >> is present (https://bugzilla.redhat.com/show_bug.cgi?id=892178).

 > Ryan Barnett noticed that rng-tools dropped the special return code.
 > I guess we could just drop support for this special case as well....
 > https://github.com/nhorman/rng-tools/blob/v6.8/rngd.c#L805

Has that Fedora patch ever been included upstream? I don't seem to find
it in the history.

I btw see there is a 6.9 release with more bugfixes, care to send a
patch bumping the version?

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] package/rng-tools: make jitterentropy conditional

[Matthew Weber]

Peter,


On Sat, Feb 8, 2020 at 1:13 PM Peter Korsgaard <peter@korsgaard.com> wrote:
>
> >>>>> "Matthew" == Matthew Weber <matthew.weber@rockwellcollins.com> writes:
>
>  > Thomas,
>  > On Wed, Feb 5, 2020 at 2:46 PM Matthew Weber
>  > <matthew.weber@rockwellcollins.com> wrote:
>  >>
>  >> Thomas,
>  >>
>  >> On Wed, Feb 5, 2020 at 1:33 PM Thomas Petazzoni
>  >> <thomas.petazzoni@bootlin.com> wrote:
>  >> >
>  >> > On Wed,  5 Feb 2020 12:26:08 -0600
>  >> > Matt Weber <matthew.weber@rockwellcollins.com> wrote:
>  >> >
>  >> > > The update of rng-tools from 5 to 6.7 introduced a change where
>  >> > > the jitterentropy library was enabled by default instead of
>  >> > > returning a special 66 return code to hangle the case of no
>  >> > > hwrng. This patch reverts that change and allows a user to
>  >> > > select when to enable the jitterentropy source. The bug
>  >> > > documents an issue of when a hwrng is enabled with jitterentropy
>  >> > > there is a longer boot time.
>  >> >
>  >> > I don't understand this story of the 66 return code. Could you explain
>  >> > a bit more ?
>  >>
>  >> In Buildroot commit 22cb51e1 the systemd support addition included the
>  >> successcode change based on the Fedora bugfix for ignoring if no hwrng
>  >> is present (https://bugzilla.redhat.com/show_bug.cgi?id=892178).
>
>  > Ryan Barnett noticed that rng-tools dropped the special return code.
>  > I guess we could just drop support for this special case as well....
>  > https://github.com/nhorman/rng-tools/blob/v6.8/rngd.c#L805
>
> Has that Fedora patch ever been included upstream? I don't seem to find
> it in the history.

It doesn't look like it.  I'm wondering if we should just drop this
systemd unit return behavior and put a comment in the service file
about enabling an entropy package if this service fails without a
hardware rng present.  Seems too complicated to try and cover this
special case.

>
> I btw see there is a 6.9 release with more bugfixes, care to send a
> patch bumping the version?

I'll review the one that was just sent.  I also ping'd the original
bug report to see if they could test with this bump as there are a
number of minor jitterentropy related fixes.
http://patchwork.ozlabs.org/patch/1235396/

Matt

[Buildroot] [PATCH] package/rng-tools: make jitterentropy conditional

[Yegor Yefremov]

Hi Matt,

On Tue, Feb 11, 2020 at 4:06 PM Matthew Weber
<matthew.weber@rockwellcollins.com> wrote:
>
> Peter,
>
>
> On Sat, Feb 8, 2020 at 1:13 PM Peter Korsgaard <peter@korsgaard.com> wrote:
> >
> > >>>>> "Matthew" == Matthew Weber <matthew.weber@rockwellcollins.com> writes:
> >
> >  > Thomas,
> >  > On Wed, Feb 5, 2020 at 2:46 PM Matthew Weber
> >  > <matthew.weber@rockwellcollins.com> wrote:
> >  >>
> >  >> Thomas,
> >  >>
> >  >> On Wed, Feb 5, 2020 at 1:33 PM Thomas Petazzoni
> >  >> <thomas.petazzoni@bootlin.com> wrote:
> >  >> >
> >  >> > On Wed,  5 Feb 2020 12:26:08 -0600
> >  >> > Matt Weber <matthew.weber@rockwellcollins.com> wrote:
> >  >> >
> >  >> > > The update of rng-tools from 5 to 6.7 introduced a change where
> >  >> > > the jitterentropy library was enabled by default instead of
> >  >> > > returning a special 66 return code to hangle the case of no
> >  >> > > hwrng. This patch reverts that change and allows a user to
> >  >> > > select when to enable the jitterentropy source. The bug
> >  >> > > documents an issue of when a hwrng is enabled with jitterentropy
> >  >> > > there is a longer boot time.
> >  >> >
> >  >> > I don't understand this story of the 66 return code. Could you explain
> >  >> > a bit more ?
> >  >>
> >  >> In Buildroot commit 22cb51e1 the systemd support addition included the
> >  >> successcode change based on the Fedora bugfix for ignoring if no hwrng
> >  >> is present (https://bugzilla.redhat.com/show_bug.cgi?id=892178).
> >
> >  > Ryan Barnett noticed that rng-tools dropped the special return code.
> >  > I guess we could just drop support for this special case as well....
> >  > https://github.com/nhorman/rng-tools/blob/v6.8/rngd.c#L805
> >
> > Has that Fedora patch ever been included upstream? I don't seem to find
> > it in the history.
>
> It doesn't look like it.  I'm wondering if we should just drop this
> systemd unit return behavior and put a comment in the service file
> about enabling an entropy package if this service fails without a
> hardware rng present.  Seems too complicated to try and cover this
> special case.
>
> >
> > I btw see there is a 6.9 release with more bugfixes, care to send a
> > patch bumping the version?
>
> I'll review the one that was just sent.  I also ping'd the original
> bug report to see if they could test with this bump as there are a
> number of minor jitterentropy related fixes.
> http://patchwork.ozlabs.org/patch/1235396/

We had similar issue today with am335x (kernel 5.4.x). Bumping
rng-tools to v6.9 helped.

The next bump will be more interesting as rng-tools moves to openssl
instead of libgcrypt and this dependency is mandatory.

Yegor

[Buildroot] [PATCH] package/rng-tools: make jitterentropy conditional

[Matthew Weber]

Yegor,


On Tue, Feb 11, 2020 at 9:22 AM Yegor Yefremov
<yegorslists@googlemail.com> wrote:
>
> Hi Matt,
>
> On Tue, Feb 11, 2020 at 4:06 PM Matthew Weber
> <matthew.weber@rockwellcollins.com> wrote:
> >
> > Peter,
> >
> >
> > On Sat, Feb 8, 2020 at 1:13 PM Peter Korsgaard <peter@korsgaard.com> wrote:
> > >
> > > >>>>> "Matthew" == Matthew Weber <matthew.weber@rockwellcollins.com> writes:
> > >
> > >  > Thomas,
> > >  > On Wed, Feb 5, 2020 at 2:46 PM Matthew Weber
> > >  > <matthew.weber@rockwellcollins.com> wrote:
> > >  >>
> > >  >> Thomas,
> > >  >>
> > >  >> On Wed, Feb 5, 2020 at 1:33 PM Thomas Petazzoni
> > >  >> <thomas.petazzoni@bootlin.com> wrote:
> > >  >> >
> > >  >> > On Wed,  5 Feb 2020 12:26:08 -0600
> > >  >> > Matt Weber <matthew.weber@rockwellcollins.com> wrote:
> > >  >> >
> > >  >> > > The update of rng-tools from 5 to 6.7 introduced a change where
> > >  >> > > the jitterentropy library was enabled by default instead of
> > >  >> > > returning a special 66 return code to hangle the case of no
> > >  >> > > hwrng. This patch reverts that change and allows a user to
> > >  >> > > select when to enable the jitterentropy source. The bug
> > >  >> > > documents an issue of when a hwrng is enabled with jitterentropy
> > >  >> > > there is a longer boot time.
> > >  >> >
> > >  >> > I don't understand this story of the 66 return code. Could you explain
> > >  >> > a bit more ?
> > >  >>
> > >  >> In Buildroot commit 22cb51e1 the systemd support addition included the
> > >  >> successcode change based on the Fedora bugfix for ignoring if no hwrng
> > >  >> is present (https://bugzilla.redhat.com/show_bug.cgi?id=892178).
> > >
> > >  > Ryan Barnett noticed that rng-tools dropped the special return code.
> > >  > I guess we could just drop support for this special case as well....
> > >  > https://github.com/nhorman/rng-tools/blob/v6.8/rngd.c#L805
> > >
> > > Has that Fedora patch ever been included upstream? I don't seem to find
> > > it in the history.
> >
> > It doesn't look like it.  I'm wondering if we should just drop this
> > systemd unit return behavior and put a comment in the service file
> > about enabling an entropy package if this service fails without a
> > hardware rng present.  Seems too complicated to try and cover this
> > special case.
> >
> > >
> > > I btw see there is a 6.9 release with more bugfixes, care to send a
> > > patch bumping the version?
> >
> > I'll review the one that was just sent.  I also ping'd the original
> > bug report to see if they could test with this bump as there are a
> > number of minor jitterentropy related fixes.
> > http://patchwork.ozlabs.org/patch/1235396/
>
> We had similar issue today with am335x (kernel 5.4.x). Bumping
> rng-tools to v6.9 helped.
>

Thank you for that feedback, I'll make this patch is superseded as the
bump resolved the bug.  I'll add a note with this patchwork thread in
the bug report as well.

Matt

[Buildroot] [PATCH] package/rng-tools: make jitterentropy conditional

[Thomas Petazzoni]

Hello Matt,

On Thu, 13 Feb 2020 11:07:36 -0600
Matthew Weber <matthew.weber@rockwellcollins.com> wrote:

> > We had similar issue today with am335x (kernel 5.4.x). Bumping
> > rng-tools to v6.9 helped.
> 
> Thank you for that feedback, I'll make this patch is superseded as the
> bump resolved the bug.  I'll add a note with this patchwork thread in
> the bug report as well.

As I was going through open bug reports, I looked again at
https://bugs.busybox.net/show_bug.cgi?id=12511, and I disagree with the
conclusion of discarding this patch "package/rng-tools: make
jitterentropy conditional".

If I understand correctly:

 - If you have a hardware RNG, the jitterentropy library is not
   necessary.

 - If you don't have a hardware RNG, rng-tools will error out, unless
   jitterentropy support is enabled.

So, we really want to make the jitterentropy support in rng-tools
optional, and leave it up to the user to enable it if (s)he has no
hardware RNG available. And if there's no hardware RNG and no
jitterentropy support, rng-tools will error out, the service will fail
to start, and that's good. So the trick with the special 66 return code
is not necessary.

Do you agree ?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] package/rng-tools: make jitterentropy conditional

[Matthew Weber]

Thomas,


On Mon, May 18, 2020 at 2:49 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Matt,
>
> On Thu, 13 Feb 2020 11:07:36 -0600
> Matthew Weber <matthew.weber@rockwellcollins.com> wrote:
>
> > > We had similar issue today with am335x (kernel 5.4.x). Bumping
> > > rng-tools to v6.9 helped.
> >
> > Thank you for that feedback, I'll make this patch is superseded as the
> > bump resolved the bug.  I'll add a note with this patchwork thread in
> > the bug report as well.
>
> As I was going through open bug reports, I looked again at
> https://bugs.busybox.net/show_bug.cgi?id=12511, and I disagree with the
> conclusion of discarding this patch "package/rng-tools: make
> jitterentropy conditional".
>
> If I understand correctly:
>
>  - If you have a hardware RNG, the jitterentropy library is not
>    necessary.
>

Right,  jitterentropy  is a fall back if there isn't a hardware RNG
setup.  The performance impacts of having it enabled when there is a
hardware RNG were improved between versions and that was the
motivation to drop this patch as it didn't seem to still be a bug
(having it enabled when it isn't really needed).

>  - If you don't have a hardware RNG, rng-tools will error out, unless
>    jitterentropy support is enabled.

Correct.

>
> So, we really want to make the jitterentropy support in rng-tools
> optional, and leave it up to the user to enable it if (s)he has no
> hardware RNG available. And if there's no hardware RNG and no
> jitterentropy support, rng-tools will error out, the service will fail
> to start, and that's good. So the trick with the special 66 return code
> is not necessary.
>
> Do you agree ?

I agree, we should drop that rngd.service change but still keep the
option where a user could add the jitterentropy library if they need
it.

Regards,
Matt