Hi Lennart,
On Mon, Feb 14, 2022 at 9:53 AM Lennart Poettering <mzxreary@xxxxxxxxxxx> wrote:
So, systemd uses (potentially half-initialized) /dev/urandom for
seeding its hash tables. For that its kinda OK if the random values
have low entropy initially, as we'll automatically reseed when too
many hash collisions happen, and then use a newer (and thus hopefully
better) seed, again acquired through /dev/urandom. i.e. if the seeds
are initially not good enough to thwart hash collision attacks, once
the hash table are actually attacked we'll replace the seeds with
someting better. For that all we need is that the random pool
eventually gets better, that's all.
So for that usecase /dev/urandom behaving the way it so far does is
kinda nice.
Oh that's an interesting point. But that sounds to me like the problem
with this patch is not that it makes /dev/urandom block (its primary
purpose) but that it also removes GRND_INSECURE (a distraction). So
perhaps an improved patch would be something like the below, which
changes /dev/urandom for new kernels but doesn't remove GRND_INSECURE.
Then your hash tables could continue to use GRND_INSECURE and all would
be well. (And for kernels without getrandom(), they'd just fall back to
/dev/urandom like normal which would have old semantics, so works.)
Jason
---------8<-----------------8<-------------------------------
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index cc296f0823bd..9f586025dbe6 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -707,7 +707,7 @@ static const struct memdev {
[5] = { "zero", 0666, &zero_fops, FMODE_NOWAIT },
[7] = { "full", 0666, &full_fops, 0 },
[8] = { "random", 0666, &random_fops, 0 },
- [9] = { "urandom", 0666, &urandom_fops, 0 },
+ [9] = { "urandom", 0666, &random_fops, 0 },
#ifdef CONFIG_PRINTK
[11] = { "kmsg", 0644, &kmsg_fops, 0 },
#endif
diff --git a/drivers/char/random.c b/drivers/char/random.c
index ce199af9bc56..ae4400c48b2f 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -89,8 +89,6 @@ static LIST_HEAD(random_ready_list);
/* Control how we warn userspace. */
static struct ratelimit_state unseeded_warning =
RATELIMIT_STATE_INIT("warn_unseeded_randomness", HZ, 3);
-static struct ratelimit_state urandom_warning =
- RATELIMIT_STATE_INIT("warn_urandom_randomness", HZ, 3);
static int ratelimit_disable __read_mostly;
module_param_named(ratelimit_disable, ratelimit_disable, int, 0644);
MODULE_PARM_DESC(ratelimit_disable, "Disable random ratelimit suppression");
@@ -336,11 +334,6 @@ static void crng_reseed(void)
unseeded_warning.missed);
unseeded_warning.missed = 0;
}
- if (urandom_warning.missed) {
- pr_notice("%d urandom warning(s) missed due to ratelimiting\n",
- urandom_warning.missed);
- urandom_warning.missed = 0;
- }
}
}
@@ -993,10 +986,8 @@ int __init rand_initialize(void)
pr_notice("crng init done (trusting CPU's manufacturer)\n");
}
- if (ratelimit_disable) {
- urandom_warning.interval = 0;
+ if (ratelimit_disable)
unseeded_warning.interval = 0;
- }
return 0;
}
@@ -1387,20 +1378,17 @@ static void try_to_generate_entropy(void)
* getrandom(2) is the primary modern interface into the RNG and should
* be used in preference to anything else.
*
- * Reading from /dev/random has the same functionality as calling
- * getrandom(2) with flags=0. In earlier versions, however, it had
- * vastly different semantics and should therefore be avoided, to
- * prevent backwards compatibility issues.
- *
- * Reading from /dev/urandom has the same functionality as calling
- * getrandom(2) with flags=GRND_INSECURE. Because it does not block
- * waiting for the RNG to be ready, it should not be used.
+ * Reading from /dev/random and /dev/urandom both the same effect as
+ * calling getrandom(2) with flags=0. In earlier versions, however,
+ * they each had vastly different semantics and should therefore be
+ * avoided to prevent backwards compatibility issues.
*
* Writing to either /dev/random or /dev/urandom adds entropy to
* the input pool but does not credit it.
*
- * Polling on /dev/random indicates when the RNG is initialized, on
- * the read side, and when it wants new entropy, on the write side.
+ * Polling on /dev/random or /dev/urandom indicates when the RNG
+ * is initialized, on the read side, and when it wants new entropy,
+ * on the write side.
*
* Both /dev/random and /dev/urandom have the same set of ioctls for
* adding entropy, getting the entropy count, zeroing the count, and
@@ -1485,21 +1473,6 @@ static ssize_t random_write(struct file *file, const char __user *buffer,
return (ssize_t)count;
}
-static ssize_t urandom_read(struct file *file, char __user *buf, size_t nbytes,
- loff_t *ppos)
-{
- static int maxwarn = 10;
-
- if (!crng_ready() && maxwarn > 0) {
- maxwarn--;
- if (__ratelimit(&urandom_warning))
- pr_notice("%s: uninitialized urandom read (%zd bytes read)\n",
- current->comm, nbytes);
- }
-
- return get_random_bytes_user(buf, nbytes);
-}
-
static ssize_t random_read(struct file *file, char __user *buf, size_t nbytes,
loff_t *ppos)
{
@@ -1586,15 +1559,6 @@ const struct file_operations random_fops = {
.llseek = noop_llseek,
};
-const struct file_operations urandom_fops = {
- .read = urandom_read,
- .write = random_write,
- .unlocked_ioctl = random_ioctl,
- .compat_ioctl = compat_ptr_ioctl,
- .fasync = random_fasync,
- .llseek = noop_llseek,
-};
-
/********************************************************************
*
