Add devices+config to make random/urandom behave like BSD

[alex]

TODO:

• Actually compile this
• Actually run this
• consistently able to boot
• Get a kernel friend to review this for kernelyness
• Get several friends to review this to make sure I didn't fuck up
• name it something better than BSD
• Learn how to LKML
• Bribe someone to LKML on my behalf

BUGS:

• /dev/bsd_urandom exists ("probably from udev" - @geofft); make that not exist, we just want it to be an option for mknod, not actually be created.
• Error in systemd urandom dump/restore

cc: @nelhage @gnprice

[geofft]

This doesn't seem quite right - I think you want

#if CONFIG_BSD_RANDOM
     [8] = { "random", 0666, &bsd_urandom_fops, 0 },
     [9] = { "urandom", 0666, &bsd_urandom_fops, 0 },
#else
     [8] = { "random", 0666, &random_fops, 0 },
     [9] = { "urandom", 0666, &urandom_fops, 0 },
#endif
     [13] = { "bsd_urandom", 0666, &bsd_urandom_fops, 0 }

[alex]

So you think the names are significant, and not just "decorative", or they are decorative but changing them is ~backwards incompatible.

[geofft]

The latter. The Kconfig option is

"When "y", the character devices for /dev/random and /dev/urandom behave as they do on BSDs"

which I think is how the code should behave. (Also I think your code as written would attempt to create two bsd_urandoms? Maybe I'm misunderstanding this line of code.)

[alex]

If I understand this correctly, this file doesn't create the actual /dev/urandom and /dev/random, it just internally registers the internal things so when you mknod 1 8 /dev/random and mknod 1 9 /dev/urandom it knows where to find the file_operations corresponding to them. Will change the the names to still be random/urandom though.

[geofft]

It seems like this is causing udevd to hang in early boot, reading /dev/urandom before it's initialized:

(gdb) bt
#0  0x00007fb4bf9beac0 in __read_nocancel () at ../sysdeps/unix/syscall-template.S:81
#1  0x0000558f7c2ee92f in __read_alias () at /usr/include/x86_64-linux-gnu/bits/unistd.h:44
#2  loop_read (fd=fd@entry=9, buf=buf@entry=0x558f7c50a910 <current>, nbytes=nbytes@entry=16, do_poll=do_poll@entry=true)
    at ../src/shared/util.c:2121
#3  0x0000558f7c2e2615 in dev_urandom (p=p@entry=0x558f7c50a910 <current>, n=n@entry=16) at ../src/shared/util.c:2399
#4  0x0000558f7c2e5341 in random_bytes (n=16, p=0x558f7c50a910 <current>) at ../src/shared/util.c:2413
#5  get_hash_key (hash_key=hash_key@entry=0x558f7c50d240 "", reuse_is_ok=reuse_is_ok@entry=true) at ../src/shared/hashmap.c:183
#6  0x0000558f7c2e6174 in hashmap_new (hash_func=<optimized out>, compare_func=<optimized out>) at ../src/shared/hashmap.c:223
#7  0x0000558f7c2eb0a3 in conf_files_list_strv_internal (root=<optimized out>, dirs=<optimized out>, suffix=<optimized out>,
    strv=<optimized out>) at ../src/shared/conf-files.c:112
#8  conf_files_list_strv.constprop.59 (strv=0x9, suffix=0x558f7c2f9dea ".link", dirs=0x558f7c50b6a0, root=<optimized out>)
    at ../src/shared/conf-files.c:146
#9  0x0000558f7c2f02a4 in link_config_load (ctx=<optimized out>) at ../src/udev/net/link-config.c:227
#10 builtin_net_setup_link_init.lto_priv.133 (udev=0x9) at ../src/udev/udev-builtin-net_setup_link.c:76
#11 0x0000558f7c2da331 in udev_builtin_init (udev=0x9) at ../src/udev/udev-builtin.c:64
#12 0x0000558f7c2d3861 in main (argc=2085662944, argv=0x3) at ../src/udev/udevd.c:1274

And you have little hope of the pool initializing on its own without udevd going, because the good sources of entropy are devices that udev initializes. If I'm reading this right, it's using a randomized hashmap to load a config file, which is generally a reasonable life decision (if you're going to have only one hashmap, make it a randomized one), but isn't actually necessary in early boot since there's no attacker-controlled data. We should see if there's a simple way to get udevd to not bother with the randomization.

[alex]

Plan of action:

• Modify udevd.c to take an additional --early-boot-no-random arg, which will make the hashtables not try to seed themselves with quality entropy, since that's unnecessary.
• Where does this arg get passed?

Dunno if the systemd/udev team will be interested in this patch?

[alex]

@geofft Found a slightly simpler patch. Not sure what things need to happen to apply this and run with the modified kernel?

diff --git a/src/basic/random-util.c b/src/basic/random-util.c
index 2f468db..590a749 100644
--- a/src/basic/random-util.c
+++ b/src/basic/random-util.c
@@ -78,11 +78,11 @@ int dev_urandom(void *p, size_t n) {
                         return -ENODATA;
         }

-        fd = open("/dev/urandom", O_RDONLY|O_CLOEXEC|O_NOCTTY);
+        fd = open("/dev/urandom", O_RDONLY|O_CLOEXEC|O_NOCTTY|O_NONBLOCK);
         if (fd < 0)
                 return errno == ENOENT ? -ENOSYS : -errno;

-        return loop_read_exact(fd, p, n, true);
+        return loop_read_exact(fd, p, n, false);
 }

 void initialize_srand(void) {

[alex]

I think this patch should be non-controversial. Here's why:

• the true -> false change is do_poll, which basically says whether to poll on EAGAIN when you read. With current /dev/urandom, EAGAIN will never be returned.
• O_NONBLOCK also does nothing on the current kernel.

So basically, these are no-ops on the current kernel, with this kernel patch however, it will lead to a fallback to the srand generation method. Which is suboptimal, but can only ever happen when you dont' have the entropy in the urandom pool to do the right thing anyways.