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@xxxxxxxxxxxxxxxxxxxx> Cc: Guenter Roeck <linux@xxxxxxxxxxxx> Cc: Dominik Brodowski <linux@xxxxxxxxxxxxxxxxxxxx> Cc: Theodore Ts'o <tytso@xxxxxxx> Cc: Jann Horn <jannh@xxxxxxxxxx> Signed-off-by: Jason A. Donenfeld <Jason@xxxxxxxxx> --- 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(-) diff --git a/drivers/char/random.c b/drivers/char/random.c index 706f08edf0dc..7b7f5e6596c2 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -1493,7 +1493,7 @@ static __poll_t random_poll(struct file *file, poll_table *wait) return mask; } -static int write_pool(const char __user *ubuf, size_t count) +static int write_pool(const char __user *ubuf, size_t count, bool will_credit) { size_t len; int ret = 0; @@ -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(); } @@ -1521,7 +1523,13 @@ static ssize_t random_write(struct file *file, const char __user *buffer, { int ret; - ret = write_pool(buffer, count); + /* + * We set "will_credit" to false here, which might be wrong if the + * user subsequently calls RNDADDTOENTCNT. But it accounts for the + * more common case of shell scripts writing to /dev/urandom and + * then immediately reading back a seed file. + */ + ret = write_pool(buffer, count, false); if (ret) return ret; @@ -1584,7 +1592,7 @@ static long random_ioctl(struct file *f, unsigned int cmd, unsigned long arg) return -EINVAL; if (get_user(size, p++)) return -EFAULT; - retval = write_pool((const char __user *)p, size); + retval = write_pool((const char __user *)p, size, ent_count > 0); if (retval < 0) return retval; credit_entropy_bits(ent_count); -- 2.35.1
