random: allow writes to /dev/urandom to influence fast init

For as far back as I can tell, writing to /dev/urandom or /dev/random
will put entropy into the pool, but won't immediately use it, and won't
credit it either. Instead, crediting is controlled by the ioctls
RNDADDTOENTCNT and RNDADDENTROPY. If, however, this happens during early
boot before the input pool is ready, we have a dangerous situation with
seed files as commonly used by shell scripts:

  dd if=seedfile of=/dev/urandom # write seed into pool
  dd if=/dev/urandom of=seedfile count=1 bs=32 # read new seed for next boot

Since the entropy in seedfile isn't credited there, this won't cause the
RNG to transition from crng_init=0 to crng_init=2, and so when we make a
new seedfile for next boot, we'll still be getting crng_init=0-quality
randomness, which may well regress from the original seedfile.

I fixed a related issue in systemd with
<https://github.com/systemd/systemd/commit/da2862ef06f22fc8d31dafced6d2d6dc14f2ee0b>,
which is a clean way of doing it, but it doesn't really help with
existing shell scripts. And the behavior here remains bad and
surprising.

So this patch fixes the issue by including /dev/urandom writes as part
of the "fast init", but not crediting it as part of the fast init
counter. This is more or less exactly what's already done for
kernel-sourced entropy whose quality we don't know, when we use
add_device_randomness(), which both contributes to the input pool and to
the fast init key.

There is one caveat to consider, which is what happens if the user
additionally calls RNDADDTOENTCNT after having written to /dev/urandom,
expecting to credit that write. That might give way to this pathological
pattern:

   - write(urandom_fd, &single_byte, 1);
   - ioctl(urandom_fd, RNDADDTOENTCNT, 8);
   - attacker brute forces that single byte.
   - write(urandom_fd, &single_byte, 1);
   - ioctl(urandom_fd, RNDADDTOENTCNT, 8);
   - attacker brute forces that single byte.
   - write(urandom_fd, &single_byte, 1);
   - ioctl(urandom_fd, RNDADDTOENTCNT, 8);
   - attacker brute forces that single byte.
   - write(urandom_fd, &single_byte, 1);
   - ioctl(urandom_fd, RNDADDTOENTCNT, 8);
   - attacker brute forces that single byte.
   - write(urandom_fd, &single_byte, 1);
   - ioctl(urandom_fd, RNDADDTOENTCNT, 8);
   - attacker brute forces that single byte.
   - ...

After this goes 32 times, the crng has now initialized, except an
attacker was able to bruteforce the whole state, making for a sort of
boot time "premature next" problem.

This is bad, but the case above is pretty pathological. Part of this
stems from the fact that /dev/urandom write + RNDADDTOENTCNT is a poor
interface, because it's unclear whether the crediting will follow the
writing, whereas we know in the add_device_randomness() case that we
_won't_ credit it, so we don't have to worry about that.

The better interface for userspace is RNDADDENTROPY, which takes the
input buffer and the entropy credit all at once, so we can make the
right decision. For the RNDADDENTROPY, we do not take part in fast init
if entropy is being credited.

And perhaps we might consider attempting to deprecate RNDADDTOENTCNT at
some point in the future.

Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Dominik Brodowski <linux@dominikbrodowski.net>
Cc: Theodore Ts'o <tytso@mit.edu>
Cc: Jann Horn <jannh@google.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
This isn't a "perfect" solution, and the of entropy accounting sort of
points both to problems there and to how the "fast init" phase fits in
to the overall design. So I'll think this over a bit for a few days, and
maybe it might lead to more improvements in fast init handling later.

 drivers/char/random.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

On Tue, Mar 22, 2022 at 12:15 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> @@ -1507,6 +1507,8 @@ static int write_pool(const char __user *ubuf, size_t count)
>                 }
>                 count -= len;
>                 ubuf += len;
> +               if (unlikely(crng_init == 0 && !will_credit))
> +                       crng_pre_init_inject(block, len, false);
>                 mix_pool_bytes(block, len);
>                 cond_resched();
>         }

Ugh. I hate that whole crng_pre_init_inject() dance.

We already mix the data into the input_pool with that 'mix_pool_bytes()' call.

So what I think the real fix is, is to just make urandom_read() use
the input_pool data directly for initializing the state.

IOW, why isn't the patch along the lines of just making
crng_make_state() take the data from the input pool instead, when
crng_ready() isn't set?

As a broken example patch, something like the appended (except that
doesn't build, because 'input_pool' is declared later)?

So take this purely as a conceptual patch, not a real patch.

(Yeah, I think this also means that code that currently does that

                crng_pre_init_inject(pool, sizeof(pool), true);
                mix_pool_bytes(pool, sizeof(pool));

should do those two operations in the reverse order, so that the input
pool is always updated before that crng_pre_init_inject() dance).

Maybe I'm missing something. But it seems kind of silly to use
base_crng AT ALL before crng_ready(). Why not use the pool we have
that *is* actually updated (that 'input_pool')?

                Linus

@@ -374,19 +374,14 @@ static void crng_make_state(u32
chacha_state[CHACHA_STATE_WORDS],
        /*
         * For the fast path, we check whether we're ready, unlocked first, and
         * then re-check once locked later. In the case where we're really not
-        * ready, we do fast key erasure with the base_crng directly, because
-        * this is what crng_pre_init_inject() mutates during early init.
+        * ready, we do fast key erasure with the input pool directly.
         */
        if (!crng_ready()) {
-               bool ready;
-
-               spin_lock_irqsave(&base_crng.lock, flags);
-               ready = crng_ready();
-               if (!ready)
-                       crng_fast_key_erasure(base_crng.key, chacha_state,
-                                             random_data, random_data_len);
-               spin_unlock_irqrestore(&base_crng.lock, flags);
-               if (!ready)
+               spin_lock_irqsave(&input_pool.lock, flags);
+               crng_fast_key_erasure(input_pool.key, chacha_state,
+                                     random_data, random_data_len);
+               spin_unlock_irqrestore(&input_pool.lock, flags);
+               if (!crng_ready())
                        return;
        }

Hey Linus,

On Tue, Mar 22, 2022 at 2:42 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Tue, Mar 22, 2022 at 12:15 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >
> > @@ -1507,6 +1507,8 @@ static int write_pool(const char __user *ubuf, size_t count)
> >                 }
> >                 count -= len;
> >                 ubuf += len;
> > +               if (unlikely(crng_init == 0 && !will_credit))
> > +                       crng_pre_init_inject(block, len, false);
> >                 mix_pool_bytes(block, len);
> >                 cond_resched();
> >         }
>
> Ugh. I hate that whole crng_pre_init_inject() dance.

Yea it's not great, but it's a helluva lot better than what was there
before 5.18, when there were two different ways of mixing into the
crng key, both of which were insecure. At least now it's a hash
function. As I mentioned in that pull request, I tried to shore up the
existing design as much as I could without departing from it in any
large fundamental ways. It may well be time to revisit the pre init
situation though.

Maybe it'd help if I described what the goals are of the current
approach and how it attempts to accomplish that.

The chacha key that's used for random bytes is separate from the input
pool, which is where entropy gets dumped. The reason for that
separation is because it's important that we decide when and under
what conditions to extract from the input pool into a new chacha key.
If we do it too often or prematurely, then we risk updating the chacha
key with very few new bits of entropy. If there aren't enough new
bits, and an attacker knows the old state (say, because the system
just booted), then it's trivial to bruteforce those new bits, since
read access to /dev/urandom is unprivileged. That's very bad.

For that reason, we only reseed when we've collected 256 bits of
entropy, and at a schedule of uptime/2 for the first 10 minutes, and
then every 5 minutes after that.

The question, here, is what to do before we've collected 256 bits of
entropy. 80 bits is still better than 0, for example. The existing
scheme (from Ted) is that we maintain a state variable, crng_init.
When crng_init=0, we direct all entropy into the chacha key directly.
If that entropy is creditable, then we account for up to 64 bytes of
input total, regardless of how many "bits" of entropy we would
otherwise be crediting were it not pre-init phase. That's kinda weird,
but the motivating mantra has always been, "fast init is garbage, but
it's better than nothing." Then, once we hit 64 bytes of fast init
accounting, we transition to crng_init=1. At this stage, we're putting
bytes into the normal input pool and crediting it as usual, not
updating the crng key directly anymore with injection, and the crng is
still blocked. When we hit 256 bits of accounted entropy from the
crng_init=1 stage, then we're ready to go, and we transition to
crng_init=2.

So this patch helps with the crng_init=0 case. It doesn't address the
crng_init=1 case, when the bytes written to /dev/urandom won't be
directly injected as they are for crng_init=0. In any case, the one
thing we're trying to preserve in all of this is that when we do
transition to crng_init=2, it's with a pool that contains 256 bits of
entropy that haven't previously been exposed anywhere, so that they
can't be brute forced.

With that as background, to answer your question:

> Maybe I'm missing something. But it seems kind of silly to use
> base_crng AT ALL before crng_ready(). Why not use the pool we have
> that *is* actually updated (that 'input_pool')?

In this case, base_crng.key is really just doubling as a separate
pool. The "pre init pool", the "fast init pool", the "super terrible
but at least not zero pool", the "all bets are off pool", ... whatever
you want to call it. Why a separate pool for pre init? Because the
real input pool needs to accumulate 256 bits of entropy before it's
safe to use.

Your suggestion is to instead not have a separate pool, but perhaps
just do separate accounting. That might work to some degree, but the
devil is in the details, and that sounds a lot harder and messier to
code. For example, you wrote:

> +               crng_fast_key_erasure(input_pool.key, chacha_state,
> +                                     random_data, random_data_len);

Except there is no input_pool.key, because the input_pool's state is
actually an un-finalized hash function. So what you wind up with
instead is calling extract_entropy() on every read. But then you have
to decide a certain point when you stop doing that, so it can
accumulate 256 bits prior to exposure, and things quickly get
extremely messy. So I don't think your suggestion improves much.

The long term solution to this stuff might be doing away with the
entropy accounting entirely, as I suggested in the other thread. The
shorter term solution, though, might be reimagining how the
crng_init=1/2 states work to begin with. (And the shortest-term "fix"
is this patch, which while not perfect is at least something.)

Just sort of spitballing what the shorter term solution might be, we
could do something exponential, where we get rid of the
pre_init_inject stuff entirely, but call `crng_reseed(force=true)` at
1 bit, 2 bits, 4 bits, 8 bits, 16 bits, 32 bits, ... 256 bits, the
same way we do now with the time. (This is somewhat similar to what NT
does, fwiw.) A downside is that the gap between, say, 64 bits and 128
bits is much "longer" than what we have now with pre_init_inject.

While the above might be an improvement, that doesn't help with our
/dev/urandom problem, though. Perhaps for that we could have some
messy heuristic, like `if (capable(CAP_SYS_ADMIN) && !crng_ready())
crng_reseed(force=true);`. That's sort of ugly too, but it would help
with the /dev/urandom shellscript seeders.

I'll think on it some more, but these two spitball ideas together
might result in a nice simplification by eliminating the fast pool
entirely. Happy to hear more ideas from you too if the above inspires
anything.

Jason

From: Jason A. Donenfeld
> Sent: 22 March 2022 19:15
> 
> For as far back as I can tell, writing to /dev/urandom or /dev/random
> will put entropy into the pool, but won't immediately use it, and won't
> credit it either. Instead, crediting is controlled by the ioctls
> RNDADDTOENTCNT and RNDADDENTROPY. If, however, this happens during early
> boot before the input pool is ready, we have a dangerous situation with
> seed files as commonly used by shell scripts:
> 
>   dd if=seedfile of=/dev/urandom # write seed into pool
>   dd if=/dev/urandom of=seedfile count=1 bs=32 # read new seed for next boot
> 
> Since the entropy in seedfile isn't credited there, this won't cause the
> RNG to transition from crng_init=0 to crng_init=2, and so when we make a
> new seedfile for next boot, we'll still be getting crng_init=0-quality
> randomness, which may well regress from the original seedfile.

Never mind scripts that try to immediately save a new seedfile [1].

What about code run by later startup scripts that wants random numbers.
They really do want the seedfile data to be used.
If it isn't used then they are likely to get very weak random numbers.

You can't really expect startup scripts to be issuing ioctl requests.

[1] I suspect the initial 'save' is just there to ensure that the
random numbers don't exactly repeat if the system crashes.
The seedfile will be written again during normal shutdown.


	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

On Tue, Mar 22, 2022 at 01:14:36PM -0600, Jason A. Donenfeld wrote:
> So this patch fixes the issue by including /dev/urandom writes as part
> of the "fast init", but not crediting it as part of the fast init
> counter. This is more or less exactly what's already done for
> kernel-sourced entropy whose quality we don't know, when we use
> add_device_randomness(), which both contributes to the input pool and to
> the fast init key.

One of the big issues with /dev/urandom writes is that *anyone*,
including malicious user space, can force specific bytes to be mixed
in.  That's the source of the reluctance to immediate use inputs from
writes into /dev/[u]random until there is a chance for it to be mixed
in with other entropy which is hopefully not under the control of
malicious userspace.

Now, I recognize that things are a bit special in early boot, and if
we have a malicious script running in a systemd unit script, we might
as well go home.  But something to consider is whether we want to do
soemthing special if the process writing to /dev/[u]random has
CAP_SYS_ADMIN, or some such.

> There is one caveat to consider, which is what happens if the user
> additionally calls RNDADDTOENTCNT after having written to /dev/urandom,
> expecting to credit that write. That might give way to this pathological
> pattern:

Yeah, no one should ever ver ever be using RNDADDTOENTCNT.  It's an
ioctl which requires root privilegs, and if it breaks, you get to keep
both pieces.

> The better interface for userspace is RNDADDENTROPY, which takes the
> input buffer and the entropy credit all at once, so we can make the
> right decision. For the RNDADDENTROPY, we do not take part in fast init
> if entropy is being credited.
> 
> And perhaps we might consider attempting to deprecate RNDADDTOENTCNT at
> some point in the future.

That would be a good idea.  :-)

						- Ted

On 23/03/2022 03.50, Jason A. Donenfeld wrote:

> - Since these seeding shell scripts have always been broken, because
>   this is how the rng has always been, rather than trying to bolt on a
>   very imperfect fix in the kernel for something that never worked
>   right, we could suggest shell scripts take the path that I implemented
>   for systemd:
>   https://github.com/systemd/systemd/commit/da2862ef06f22fc8d31dafced6d2d6dc14f2ee0b
>   In shell, this would look like:
> 
>     #!/bin/bash
>     cat seedfile > /dev/urandom
>     { cat seedfile; head -c 32 /dev/urandom; } | sha256sum | cut -d ' ' -f 1 > seedfile

Maybe stating the obvious, but in the interest of preventing
proliferation of more broken shell scripts: The tail of the above should
be spelled

  ...  > seedfile.tmp && mv seedfile.tmp seedfile

or seedfile would be truncated before cat had a chance to read it.

Rasmus

From: Jason A. Donenfeld
> Sent: 23 March 2022 04:48
...
> - Plenty of things are seeding the RNG correctly, and buildroot's
> shell script is just "doing it wrong".
> 
> On that last point, I should reiterate that buildroot's shell script
> still isn't actually initializing the RNG, despite what it says in its
> echo; there's never been a way to initialize the RNG from a shell
> script, without calling out to various special purpose ioctl-aware
> binaries.

Perhaps the very first write after boot could be assumed to
be valid initialisation data?
(On top of a few other tests.)

Then I need a patch against 5.10 :-)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Hi Alex,

On Thu, Mar 24, 2022 at 10:29 AM Alex Xu (Hello71) <alex_y_xu@yahoo.ca> wrote:
>
> Excerpts from Jason A. Donenfeld's message of March 23, 2022 11:18 pm:
> > Hi all,
> >
> > [...]
> >
> > In light of that conclusion, I'm going to work with every userspace
> > downstream I can find to help them fix their file-based seeding, if it
> > has bugs. I've started talking with the buildroot folks, and then I'll
> > speak with the OpenRC people (being a Gentoo dev, that should be easy
> > going). Systemd does the right thing already.
> >
> > I wrote a little utility for potential inclusion in
> > busybox/util-linux/whatever when it matures beyond its current age of
> > being half hour old:
> > - https://git.zx2c4.com/seedrng/about/
> > - https://git.zx2c4.com/seedrng/tree/seedrng.c
> > So I'll see what the buildroot people think of this and take it from there.
> >
> > The plus side of doing all this is that, if the efforts pan out, it
> > means there'll actually be proper seeding on devices that don't
> > currently do that, which then might lead to a better ecosystem and
> > less boot time blocking and all that jazz.
> >
> > Jason
> >
>
> The issue, in systemd developers' opinion, is that counting seed file
> towards entropy initialization potentially causes repeated RNG output if
> a system is cloned without resetting the seed file. This is discussed at
> length in https://github.com/systemd/systemd/pull/4513. A few years ago,
> I wrote most of a program to check machine ID, disk ID, DMI ID, and some
> other things in order to avoid this issue. Since then, systemd decided
> to store the random seed in EFI variables, I assume on the basis that
> machine cloning typically does not clone the EFI variables? In my
> opinion, since the same argument applies to machine ID, ssh keys, and
> any other persistent cryptographic (or even non-cryptographic) material,
> this falls outside the scope of random seeding and into a general
> machine cloning "sysprep"-like utility.

systemd's seed utility will credit a seed file if the seed file was
generated properly (e.g. after the RNG was initialized). For that they
use the user.random-seed-creditable xattr, which is a reasonable way of
deciding. If that attribute is present, it's credited; if it's not, it's
not. Here's their source:

        /* If we got this random seed data from getrandom() the data is suitable for crediting
         * entropy later on. Let's keep that in mind by setting an extended attribute. on the file */
        if (getrandom_worked)
                if (fsetxattr(seed_fd, "user.random-seed-creditable", "1", 1, 0) < 0)
                        log_full_errno(ERRNO_IS_NOT_SUPPORTED(errno) ? LOG_DEBUG : LOG_WARNING, errno,
                                      "Failed to mark seed file as creditable, ignoring: %m");

Since my seedrng.c is designed for more minimal systems (running
buildroot or openrc or whatever), which might not have xattrs available,
it distinguishes just based on the filename:

	if (new_seed_creditable && rename(NON_CREDITABLE_SEED, CREDITABLE_SEED) < 0) {
		fprintf(stderr, "ERROR: Unable to make new seed creditable: %s\n", strerror(errno));
		program_ret |= 1 << 6;
	}

It's no surprise that these are very similar; I've read systemd's
seeding logic and contributed a fix to it.

By the way, if you think something is different or wrong or whatever in
seedrng.c, please feel free to send me a patch for it. It already
received its first contribution this morning (from the buildroot
maintainer). Hopefully the code will reach a good settling point soon,
and then various projects that want it can just copy and paste it
verbatim into their environment, and tweak idiomatic things as needed.

Jason

On Wed, Mar 23, 2022 at 09:18:16PM -0600, Jason A. Donenfeld wrote:
> In light of that conclusion, I'm going to work with every userspace
> downstream I can find to help them fix their file-based seeding, if it
> has bugs. I've started talking with the buildroot folks, and then I'll
> speak with the OpenRC people (being a Gentoo dev, that should be easy
> going). Systemd does the right thing already.
> 
> I wrote a little utility for potential inclusion in
> busybox/util-linux/whatever when it matures beyond its current age of
> being half hour old:
> - https://git.zx2c4.com/seedrng/about/
> - https://git.zx2c4.com/seedrng/tree/seedrng.c
> So I'll see what the buildroot people think of this and take it from there.
> 

The example in random(4) needs to be fixed too, right?

https://man7.org/linux/man-pages/man4/random.4.html

- Eric

Excerpts from Jason A. Donenfeld's message of March 24, 2022 1:20 pm:
> Hi Alex,
> 
> On Thu, Mar 24, 2022 at 10:29 AM Alex Xu (Hello71) <alex_y_xu@yahoo.ca> wrote:
>> The issue, in systemd developers' opinion, is that counting seed file
>> towards entropy initialization potentially causes repeated RNG output if
>> a system is cloned without resetting the seed file. This is discussed at
>> length in https://github.com/systemd/systemd/pull/4513. A few years ago,
>> I wrote most of a program to check machine ID, disk ID, DMI ID, and some
>> other things in order to avoid this issue. Since then, systemd decided
>> to store the random seed in EFI variables, I assume on the basis that
>> machine cloning typically does not clone the EFI variables? In my
>> opinion, since the same argument applies to machine ID, ssh keys, and
>> any other persistent cryptographic (or even non-cryptographic) material,
>> this falls outside the scope of random seeding and into a general
>> machine cloning "sysprep"-like utility.
> 
> systemd's seed utility will credit a seed file if the seed file was
> generated properly (e.g. after the RNG was initialized). For that they
> use the user.random-seed-creditable xattr, which is a reasonable way of
> deciding. If that attribute is present, it's credited; if it's not, it's
> not. Here's their source:
> 
>         /* If we got this random seed data from getrandom() the data is suitable for crediting
>          * entropy later on. Let's keep that in mind by setting an extended attribute. on the file */
>         if (getrandom_worked)
>                 if (fsetxattr(seed_fd, "user.random-seed-creditable", "1", 1, 0) < 0)
>                         log_full_errno(ERRNO_IS_NOT_SUPPORTED(errno) ? LOG_DEBUG : LOG_WARNING, errno,
>                                       "Failed to mark seed file as creditable, ignoring: %m");
> 
> Since my seedrng.c is designed for more minimal systems (running
> buildroot or openrc or whatever), which might not have xattrs available,
> it distinguishes just based on the filename:
> 
> 	if (new_seed_creditable && rename(NON_CREDITABLE_SEED, CREDITABLE_SEED) < 0) {
> 		fprintf(stderr, "ERROR: Unable to make new seed creditable: %s\n", strerror(errno));
> 		program_ret |= 1 << 6;
> 	}
> 
> It's no surprise that these are very similar; I've read systemd's
> seeding logic and contributed a fix to it.
> 
> By the way, if you think something is different or wrong or whatever in
> seedrng.c, please feel free to send me a patch for it. It already
> received its first contribution this morning (from the buildroot
> maintainer). Hopefully the code will reach a good settling point soon,
> and then various projects that want it can just copy and paste it
> verbatim into their environment, and tweak idiomatic things as needed.
> 
> Jason
> 

Right, but I'm saying that even if the seed file is good when you wrote 
it, it becomes bad if you copy it to multiple machines (e.g. by VM 
cloning). For various reasons, I think this is outside the scope of the 
random seed services to handle, but I think it's good to keep in mind.

Cheers,
Alex.

On Tue, Mar 22, 2022 at 01:14:36PM -0600, Jason A. Donenfeld wrote:
> For as far back as I can tell, writing to /dev/urandom or /dev/random
> will put entropy into the pool, but won't immediately use it, and won't
> credit it either.

Did you check kernels v4.7 and earlier?  It looks like this actually changed in
v4.8 when the ChaCha20 CRNG was introduced.  v4.7 would mix the data written to
/dev/{u,}random into {non,}blocking_pool, which would immediately be reflected
in reads from /dev/{u,}random, sys_getrandom(), and get_random_bytes().  Writes
to /dev/{u,}random didn't affect the input_pool, which was separate.

Was the change in behavior in v4.8 a regression?

- Eric

Hi Eric,

On 3/24/22, Eric Biggers <ebiggers@kernel.org> wrote:
> On Tue, Mar 22, 2022 at 01:14:36PM -0600, Jason A. Donenfeld wrote:
>> For as far back as I can tell, writing to /dev/urandom or /dev/random
>> will put entropy into the pool, but won't immediately use it, and won't
>> credit it either.
>
> Did you check kernels v4.7 and earlier?  It looks like this actually changed
> in
> v4.8 when the ChaCha20 CRNG was introduced.  v4.7 would mix the data written
> to
> /dev/{u,}random into {non,}blocking_pool, which would immediately be
> reflected
> in reads from /dev/{u,}random, sys_getrandom(), and get_random_bytes().
> Writes
> to /dev/{u,}random didn't affect the input_pool, which was separate.

Oh, I suppose you might be right, actually, that v4.7 and below would
hash the non blocking pool, and let /dev/urandom write directly into
it, as something distinct from the input pool. This changed with v4.8,
6 years ago, and now there are no LTS kernels that old, with most
small devices even having vendor kernels v4.9+. v4.8 apparently did
this while fixing a more extreme vulnerability of allowing
unprivileged users to bruteforce input bytes (in addition to allowing
unbounded unprivileged lock contention). Of those who have been
seeding via /dev/random, the ones who additionally issued the ioctl to
credit those bits haven't been affected since the crediting solved the
issue by invoking a reseeding. And those who didn't issue the ioctl
never had their RNG initialize in the first place, causing getrandom()
to block until entropy was collected from elsewhere, until it was
safe, so the harm there was minimal. So it's not great, but it's not
horrific either, and I still think the cons strongly outweigh the pros
in trying to change the behavior from what it is now.

Jason

Hi!
> > On Tue, Mar 22, 2022 at 01:14:36PM -0600, Jason A. Donenfeld wrote:
> >> For as far back as I can tell, writing to /dev/urandom or /dev/random
> >> will put entropy into the pool, but won't immediately use it, and won't
> >> credit it either.
> >
> > Did you check kernels v4.7 and earlier?  It looks like this actually changed
> > in
> > v4.8 when the ChaCha20 CRNG was introduced.  v4.7 would mix the data written
> > to
> > /dev/{u,}random into {non,}blocking_pool, which would immediately be
> > reflected
> > in reads from /dev/{u,}random, sys_getrandom(), and get_random_bytes().
> > Writes
> > to /dev/{u,}random didn't affect the input_pool, which was separate.
> 
> Oh, I suppose you might be right, actually, that v4.7 and below would
> hash the non blocking pool, and let /dev/urandom write directly into
> it, as something distinct from the input pool. This changed with v4.8,
> 6 years ago, and now there are no LTS kernels that old, with most
> small devices even having vendor kernels v4.9+. v4.8 apparently did

We are still maintaining 4.4 for -cip project, and people running android probably still 
maintain that, too.

> this while fixing a more extreme vulnerability of allowing unprivileged users to 
> bruteforce input bytes (in addition to allowing unbounded unprivileged lock contention). 

I assume this got fixed during the 4.4-stable series?

Best regards,
										Pavel