[PATCH v8] seedrng: import SeedRNG utility for kernel RNG seed files

Tue Apr 12 18:37:31 UTC 2022

Hi Jason!

On Sun, 10 Apr 2022 23:20:34 +0200
"Jason A. Donenfeld" <Jason at zx2c4.com> wrote:

> +static int read_new_seed(uint8_t *seed, size_t len, bool *is_creditable)
> +{
> +	ssize_t ret;
> +
> +	*is_creditable = false;
> +	ret = getrandom(seed, len, GRND_NONBLOCK);
> +	if (ret == (ssize_t)len) {
> +		*is_creditable = true;
> +		return 0;
> +	} else if (ret < 0 && errno == ENOSYS) {
> +		struct pollfd random_fd = {
> +			.fd = open("/dev/random", O_RDONLY),
> +			.events = POLLIN
> +		};
> +		if (random_fd.fd < 0)
> +			return -1;
> +		*is_creditable = safe_poll(&random_fd, 1, 0) == 1;
> +		close(random_fd.fd);
> +	} else if (getrandom(seed, len, GRND_INSECURE) == (ssize_t)len)
> +		return 0;
> +	if (open_read_close("/dev/urandom", seed, len) == (ssize_t)len)
> +		return 0;
> +	return -1;
> +}

> +int seedrng_main(int argc, char *argv[]) MAIN_EXTERNALLY_VISIBLE;
> +int seedrng_main(int argc UNUSED_PARAM, char *argv[])
> +{

> +	bool new_seed_creditable;
> +	bool skip_credit = false;
> +	struct timespec timestamp = { 0 };
> +	sha256_ctx_t hash;
> +
> +	int opt;
> +	enum {
> +		OPT_d = (1 << 0),
> +		OPT_n = (1 << 1)
> +	};
> +#if ENABLE_LONG_OPTS
> +	static const char longopts[] ALIGN1 =
> +		"seed-dir\0"	Required_argument	"d"
> +		"skip-credit\0"	No_argument		"n"
> +		;
> +#endif
> +
> +	opt = getopt32long(argv, "d:n", longopts, &seed_dir);

> +	skip_credit = opt & OPT_n;

> +	ret = seed_from_file_if_exists(non_creditable_seed, dfd, false, &hash);
> +	if (ret < 0)
> +		program_ret |= 1 << 1;
> +	ret = seed_from_file_if_exists(creditable_seed, dfd, !skip_credit, &hash);

> +	printf("Saving %zu bits of %s seed for next boot\n", new_seed_len * 8, new_seed_creditable ? "creditable" : "non-creditable");

> +	if (new_seed_creditable && rename(non_creditable_seed, creditable_seed) < 0) {

I'm a bit surprised that even if i give -n the seed is moved to
seed.credit. The next boot/run will find the now creditable seed and
happily add it, IIUC, despite i wanted it to not be credited?
Is this intentional?

PS: And i can literally hear some security expert to
 mknod -m 0666 /dev/random c 1 3;# a /dev/zero, doesn't block much
and then run around complaining that we credited all those zeros ;)
So a paranoid impl would check for them to be c1,8 and c1,9 before
trusting them and crediting, i suppose. But i'd rather want to avoid
these checks in busybox since that's a bit too much bloat. But i
thought i'd note it for your other, bigger impls, fwiw. But you
certainly have that check in there already anyway..

PPS: I'm attaching some fiddle on top of your v8 which would give a
relative savings of
function                                             old     new   delta
seedrng_main                                         948    1228    +280
.rodata                                           108338  108206    -132
seed_from_file_if_exists                             410       -    -410
------------------------------------------------------------------------------
(add/remove: 0/1 grow/shrink: 1/1 up/down: 280/-542)         Total: -262 bytes

$ size */seedrng.o*
   text	   data	    bss	    dec	    hex	filename
   1900	      0	      0	   1900	    76c	util-linux/seedrng.o.v8
   1658	      0	      0	   1658	    67a	util-linux/seedrng.o

I still have to see if we can make the construction of the two seed
file names a bit smaller. And the headers should be pruned.

thanks and cheers,
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-seedrng-shrink.patch
Type: text/x-patch
Size: 2228 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20220412/585178e6/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-seedrng-manually-inline-seed_rng.patch
Type: text/x-patch
Size: 2788 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20220412/585178e6/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-seedrng-CSE-seed_len-8.patch
Type: text/x-patch
Size: 1553 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20220412/585178e6/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-seedrng-remove-impossible-condition.patch
Type: text/x-patch
Size: 1097 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20220412/585178e6/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-seedrng-simplify-seed_dir-handling.patch
Type: text/x-patch
Size: 1640 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20220412/585178e6/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-seedrng-allow-inlining-seed_from_file_if_exists.patch
Type: text/x-patch
Size: 2121 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20220412/585178e6/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0007-seedrng-cleanup-strings.patch
Type: text/x-patch
Size: 3255 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20220412/585178e6/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0008-seedrng-shrink-a-bit.patch
Type: text/x-patch
Size: 2396 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20220412/585178e6/attachment-0007.bin>