Bug 230870 - Deprecate Yarrow
Summary: Deprecate Yarrow
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Mark Murray
URL:
Keywords:
Depends on:
Blocks: 228911 228931
  Show dependency treegraph
 
Reported: 2018-08-24 20:42 UTC by Conrad Meyer
Modified: 2018-08-26 12:53 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Conrad Meyer freebsd_committer freebsd_triage 2018-08-24 20:42:01 UTC
I think we should deprecate Yarrow as soon as we possibly can.  I think it would be reasonable to gone_in(12) it, even, removing it from tree before stable/12.

At the very least, it should be gone_in(13)'d and removed after stable/12 branches.

We discussed this briefly on orthogonal devrandom bugs, and one concern raised was embedded systems may prefer the lower space usage of Yarrow.

In response to that, I quantified the difference in state size and came up with 962 bytes.  Do we have embedded systems today that would trade a weak devrandom for 962 bytes of memory?

I suspect not.  IIRC, the smallest system we can run on today is either 32 or 64 MB, and even that requires quite a bit of manual tweaking to get a minimal kernel and almost no userspace.  And on such a 32MB system, 962 bytes is 0.003%.  Is that small enough to be de minimis?  I think so.  If you can't run with 33553470 bytes of memory it is unlikely you will be able to run with 33554432.
Comment 1 Rodney W. Grimes freebsd_committer freebsd_triage 2018-08-24 20:49:18 UTC
This change is in the same class as the drm/drm2 issue we just went through, best that should be considered at this point is to request additions of gone_in(13) and start the deprecation procedure.

After code freeze is not the time to start deprecation.
Comment 2 Mark Murray freebsd_committer freebsd_triage 2018-08-24 22:49:52 UTC
(In reply to Conrad Meyer from comment #0)

This was advertised in random(4) in FreeBSD-11 already.

I have a tested patch ready to go for this. I'll put it on Phabricator tomorrow.

I saw the drm/drm2 discussion, so I won't be pulling triggers without consensus.
Comment 3 Conrad Meyer freebsd_committer freebsd_triage 2018-08-25 01:35:16 UTC
(In reply to Rodney W. Grimes from comment #1)
I'll lead with: I am ok with gone_in(13), and mentioned that explicitly in the description as-filed.

However, I disagree (somewhat) with the characterization as similar to drm2.

Granted, it is similar in that there hasn't been a formal deprecation announcement by the project.

However, in many ways this transition is less abrupt than drm2 to drm-next.

1. The replacement, Fortuna, is in tree.

2. The replacement, Fortuna, has been the default since 11-CURRENT and 11.0 release.

3. The Yarrow designers are also the Fortuna designers, and they recommend no one use Yarrow any more.  Fortuna was published as recently as 2003, while Yarrow dates to 1999.  I.e., the design has been deprecated by the designers, and has been for something like 15 years.

4. There is no benefit to Yarrow over Fortuna.  drm1 and drm2 provide some benefit at least one user each, but Yarrow cannot make the same claim.

Anyway, if the conclusion is still "keep it for 12," I'm fine with that.
Comment 4 Conrad Meyer freebsd_committer freebsd_triage 2018-08-25 01:47:00 UTC
(In reply to Mark Murray from comment #2)
> This was advertised in random(4) in FreeBSD-11 already.

Oh, I missed that!  Great!

                                                  The random_yarrow
     module is deprecated, and will be removed in FreeBSD 12. Use of the
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     Yarrow algorithm is not encouraged, but while still present in the kernel
     source, it can be selected with the options RANDOM_YARROW kernel option.

Boom, let's do it for 12.0.
Comment 5 Rodney W. Grimes freebsd_committer freebsd_triage 2018-08-25 03:29:34 UTC
(In reply to Mark Murray from comment #2)
Thats good that it is marked as such in 11.  With that additional information I do not see it as a major prolem with getting this gone_in(12).

(In reply to Conrad Meyer from comment #3)
My characterization as similar to drm was mainly about the fact that bringing this up after code freeze is not good.  I was also under the wrong impression it had not been marked in any way in 11, those 2 things combined are the major reason that drm in the end got reverted.

(In reply to Conrad Meyer from comment #4)
Given the new information I agree we should get it done now.
Comment 6 Gordon Tetlow freebsd_committer freebsd_triage 2018-08-25 03:36:35 UTC
In case there is need for formal documentation. security-officer approves.
Comment 7 Mark Murray freebsd_committer freebsd_triage 2018-08-25 08:32:03 UTC
(In reply to Gordon Tetlow from comment #6)

Tested patch here. Tweeks to docs/UPDATING to follow.
https://reviews.freebsd.org/D16898
Comment 8 Mark Murray freebsd_committer freebsd_triage 2018-08-25 10:57:43 UTC
(In reply to Gordon Tetlow from comment #6)

Thanks, Gordon!
Comment 9 commit-hook freebsd_committer freebsd_triage 2018-08-26 12:52:20 UTC
A commit references this bug:

Author: markm
Date: Sun Aug 26 12:51:54 UTC 2018
New revision: 338324
URL: https://svnweb.freebsd.org/changeset/base/338324

Log:
  Remove the Yarrow PRNG algorithm option in accordance with due notice
  given in random(4).

  This includes updating of the relevant man pages, and no-longer-used
  harvesting parameters.

  Ensure that the pseudo-unit-test still does something useful, now also
  with the "other" algorithm instead of Yarrow.

  PR:		230870
  Reviewed by:	cem
  Approved by:	so(delphij,gtetlow)
  Approved by:	re(marius)
  Differential Revision:	https://reviews.freebsd.org/D16898

Changes:
  head/UPDATING
  head/share/man/man4/random.4
  head/share/man/man9/random_harvest.9
  head/sys/arm/amlogic/aml8726/aml8726_rng.c
  head/sys/arm/broadcom/bcm2835/bcm2835_rng.c
  head/sys/conf/NOTES
  head/sys/conf/files
  head/sys/conf/options
  head/sys/dev/glxsb/glxsb.c
  head/sys/dev/hifn/hifn7751.c
  head/sys/dev/random/build.sh
  head/sys/dev/random/fortuna.c
  head/sys/dev/random/other_algorithm.c
  head/sys/dev/random/other_algorithm.h
  head/sys/dev/random/random_harvestq.c
  head/sys/dev/random/random_harvestq.h
  head/sys/dev/random/randomdev.c
  head/sys/dev/random/unit_test.c
  head/sys/dev/random/unit_test.h
  head/sys/dev/random/yarrow.c
  head/sys/dev/random/yarrow.h
  head/sys/dev/rndtest/rndtest.c
  head/sys/dev/safe/safe.c
  head/sys/dev/syscons/scmouse.c
  head/sys/dev/syscons/syscons.c
  head/sys/dev/ubsec/ubsec.c
  head/sys/dev/virtio/random/virtio_random.c
  head/sys/dev/vt/vt_core.c
  head/sys/dev/vt/vt_sysmouse.c
  head/sys/fs/tmpfs/tmpfs_subr.c
  head/sys/kern/kern_intr.c
  head/sys/kern/subr_bus.c
  head/sys/mips/cavium/octeon_rnd.c
  head/sys/modules/Makefile
  head/sys/modules/random_yarrow/Makefile
  head/sys/net/if_ethersubr.c
  head/sys/net/if_tun.c
  head/sys/netgraph/ng_iface.c
  head/sys/sys/random.h
  head/sys/ufs/ffs/ffs_inode.c
  head/sys/vm/uma_core.c
  head/tools/tools/sysdoc/tunables.mdoc