Bug 237869 - is_random_seeded should perform pre_read for unseeded case
Summary: is_random_seeded should perform pre_read for unseeded case
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: Conrad Meyer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-05-13 07:55 UTC by Xin LI
Modified: 2019-05-13 19:36 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Xin LI freebsd_committer freebsd_triage 2019-05-13 07:55:01 UTC
I noticed the following on a recent -CURRENT amd64 system, this message on boot:

arc4random: WARNING: initial seeding bypassed the cryptographic random device because it was not yet seeded and the knob 'bypass_before_seeding' was enabled.

Despite /boot/entropy is preloaded.  It looks like it was related to r346358.

On my system, the call path is roughly:

vnet_domain_init_inet_vnet_init -> arc4random()

At the time, the entropy device have already seen /boot/entropy through random_harvestq_prime(), however, because nobody have read from entropy device, the pre_read method was never called, therefore the device would report itself as unseeded, even though it already have enough entropy to proceed.

(By the way, r346292 should be reverted now that r346358 is landed).

It looks like is_random_seeded should do something like:

if (__predict_false(!p_random_alg_context->ra_seeded())) {
    p_random_alg_context->ra_pre_read();
    return (p_random_alg_context->ra_seeded());
}
return (true);

Instead of its current form.

Note that random_infra.c needs similar treatment as well; the current form is broken by the way.
Comment 1 Conrad Meyer freebsd_committer freebsd_triage 2019-05-13 15:18:13 UTC
https://reviews.freebsd.org/D20239
Comment 2 Conrad Meyer freebsd_committer freebsd_triage 2019-05-13 15:25:20 UTC
(In reply to Xin LI from comment #0)
> (By the way, r346292 should be reverted now that r346358 is landed).

r346358 only adds a knob, defaulting to on, to bypass seeded random; I'm not sure it entirely obviates r346292.  It might plausibly be useful to disable random bypass in general but allow it for the extremely-early stack cookie system, in case /boot/entropy was unavailable.  But maybe not.
Comment 3 commit-hook freebsd_committer freebsd_triage 2019-05-13 19:35:45 UTC
A commit references this bug:

Author: cem
Date: Mon May 13 19:35:36 UTC 2019
New revision: 347546
URL: https://svnweb.freebsd.org/changeset/base/347546

Log:
  Fortuna: Fix false negatives in is_random_seeded()

  (1) We may have had sufficient entropy to consider Fortuna seeded, but the
  random_fortuna_seeded() function would produce a false negative if
  fs_counter was still zero.  This condition could arise after
  random_harvestq_prime() processed the /boot/entropy file and before any
  read-type operation invoked "pre_read()."  Fortuna's fs_counter variable is
  only incremented (if certain conditions are met) by reseeding, which is
  invoked by random_fortuna_pre_read().

  is_random_seeded(9) was introduced in r346282, but the function was unused
  prior to r346358, which introduced this regression.  The regression broke
  initial seeding of arc4random(9) and broke periodic reseeding[A], until something
  other than arc4random(9) invoked read_random(9) or read_random_uio(9) directly.
  (Such as userspace getrandom(2) or read(2) of /dev/random.  By default,
  /etc/rc.d/random does this during multiuser start-up.)

  (2) The conditions under which Fortuna will reseed (including initial seeding)
  are: (a) sufficient "entropy" (by sheer byte count; default 64) is collected
  in the zeroth pool (of 32 pools), and (b) it has been at least 100ms since
  the last reseed (to prevent trivial DoS; part of FS&K design).  Prior to
  this revision, initial seeding might have been prevented if the reseed
  function was invoked during the first 100ms of boot.

  This revision addresses both of these issues.  If random_fortuna_seeded()
  observes a zero fs_counter, it invokes random_fortuna_pre_read() and checks
  again.  This addresses the problem where entropy actually was sufficient,
  but nothing had attempted a read -> pre_read yet.

  The second change is to disable the 100ms reseed guard when Fortuna has
  never been seeded yet (fs_lasttime == 0).  The guard is intended to prevent
  gratuitous subsequent reseeds, not initial seeding!

  Machines running CURRENT between r346358 and this revision are encouraged to
  refresh when possible.  Keys generated by userspace with /dev/random or
  getrandom(9) during this timeframe are safe, but any long-term session keys
  generated by kernel arc4random consumers are potentially suspect.

  [A]: Broken in the sense that is_random_seeded(9) false negatives would cause
  arc4random(9) to (re-)seed with weak entropy (SHA256(cyclecount ||
  FreeBSD_version)).

  PR:		237869
  Reported by:	delphij, dim
  Reviewed by:	delphij
  Approved by:	secteam(delphij)
  X-MFC-With:	r346282, r346358 (if ever)
  Security:	yes
  Sponsored by:	Dell EMC Isilon
  Differential Revision:	https://reviews.freebsd.org/D20239

Changes:
  head/sys/dev/random/fortuna.c