From: Eric Biggers <ebiggers@kernel.org>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
Theodore Ts'o <tytso@mit.edu>,
Dominik Brodowski <linux@dominikbrodowski.net>
Subject: Re: [PATCH v2] random: reseed more often immediately after booting
Date: Sat, 12 Mar 2022 11:44:40 -0800 [thread overview]
Message-ID: <Yiz4KBqaxURu/6mZ@sol.localdomain> (raw)
In-Reply-To: <CAHmME9ohyKKX4Qg_dyGq36MxFkhBoVQYYgs8uUoCfBkJNqfX7Q@mail.gmail.com>
On Thu, Mar 10, 2022 at 01:59:05PM -0700, Jason A. Donenfeld wrote:
> > However, one thing that seems a bit odd is that this method can result in two
> > reseeds with very little time in between. For example, if no one is using the
> > RNG from second 40-78, but then it is used in seconds 79-80, then it will be
> > reseeded at both seconds 79 and 80 if there is entropy available.
>
> I've been sort of going back and forth on this. I think the idea is
> that there are two relative time measurements. The ordinary one we use
> is time since last reseeding. But at boot, we're trying to account for
> the fact that entropy is coming in relative to the init process of the
> system, which means we need it to be relative to boot time rather than
> relative to the last reseeding. As you point out, this is a little
> wonky with how things are now, because we only ever reseed on usage.
> To get closer to what I have in mind, we could reseed in a timer, so
> that it hits it exactly on the 5,10,20,40,etc dot. But that seems a
> bit cumbersome and maybe unnecessary. The effect of the behavior of
> this v1 you pointed out is:
>
> - We might reseed at 79, and then fail to reseed at 80. Consequence:
> we lose 1 second of entropy that could have made it for that try.
> - We might reseed at 79, and then also reseed at 80 too. Consequence:
> that's a fairly quick refresh, but on the other hand, we're still
> requiring 256 bit credits, so maybe not so bad, and if we've got so
> much entropy coming in during that small period of time, maybe it
> really isn't a concern.
>
> So I'm not sure either of these cases really matter that much.
>
> > Perhaps the condition should still be:
> >
> > time_after(jiffies, READ_ONCE(base_crng.birth) + interval);
> >
> > ... as it is in the non-early case, but where 'interval' is a function of
> > 'uptime' rather than always CRNG_RESEED_INTERVAL? Maybe something like:
> >
> > interval = CRNG_RESEED_INTERVAL;
> > if (uptime < 2 * CRNG_RESEED_INTERVAL / HZ)
> > interval = max(5, uptime / 2) * HZ;
>
> I'd experimented with things like that, for example making it exponential:
>
> static bool early_boot = true;
> unsigned long interval = CRNG_RESEED_INTERVAL;
>
> if (unlikely(READ_ONCE(early_boot))) {
> unsigned int uptime = min_t(u64, INT_MAX, ktime_get_seconds());
> if (uptime >= CRNG_RESEED_INTERVAL / HZ)
> WRITE_ONCE(early_boot, false);
> else
> interval = (5U << fls(uptime / 5)) * HZ;
> }
> return time_after(jiffies, READ_ONCE(base_crng.birth) + interval);
>
> But the whole thing of combining relative-to-last-reseed with
> relative-to-boottime seems really strange. I'm not quite sure what
> that's supposed to represent, whereas what I have in v1 is
> "exponentially increasing intervals from boottime" which is fairly
> easy to understand.
I don't think it's strange. Maybe it seems strange because of how you wrote it
('interval = (5U << fls(uptime / 5)) * HZ'), where the reseed interval suddenly
jumps from X to 2*X seconds. The version I suggested is 'interval = max(5,
uptime / 2) * HZ', which is smoother. It's simply saying that the reseed
interval increases as the uptime increases, which seems to be what we want.
(Bounded by [5*HZ, CRNG_RESEED_INTERVAL], of course.)
Your current patch interacts badly with how crng_reseed() is a no-op if fewer
than 256 entropy bits are available. Suppose that random bytes are requested at
seconds 10 and 15, and at second 10 there are 200 entropy bits available and at
second 15 there are 256. With your current patch, the CRNG wouldn't be reseeded
before either request, since the no-op reseed at second 10 would consume the
only attempt for the whole interval of 10-19. I.e. the request at second 10
would actually be preventing the CRNG from being reseeded when it should be.
Using an interval since the actual last reseed time (base_crng.birth) avoids
this quirk, as well the one I mentioned before where two reseeds can be done
with very little time in between.
What you have now is still better than the status quo, but I'm not sure it's the
best way.
- Eric
next prev parent reply other threads:[~2022-03-12 19:44 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-09 15:26 [PATCH] random: reseed more often immediately after booting Jason A. Donenfeld
2022-03-09 19:18 ` [PATCH v2] " Jason A. Donenfeld
2022-03-10 4:57 ` Eric Biggers
2022-03-10 20:59 ` Jason A. Donenfeld
2022-03-10 21:10 ` Jason A. Donenfeld
2022-03-12 19:44 ` Eric Biggers [this message]
2022-03-13 0:35 ` Jason A. Donenfeld
2022-03-13 1:00 ` Eric Biggers
2022-03-13 1:29 ` Jason A. Donenfeld
2022-03-13 1:41 ` [PATCH v4] " Jason A. Donenfeld
2022-03-13 3:39 ` Eric Biggers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Yiz4KBqaxURu/6mZ@sol.localdomain \
--to=ebiggers@kernel.org \
--cc=Jason@zx2c4.com \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@dominikbrodowski.net \
--cc=tytso@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox