Hi Jason! On Sun, 10 Apr 2022 23:20:34 +0200 "Jason A. Donenfeld" <ja...@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,
>From eb570afb9e1f550b589602f86269ecfe61c979a0 Mon Sep 17 00:00:00 2001 From: Bernhard Reutner-Fischer <rep.dot....@gmail.com> Date: Tue, 12 Apr 2022 20:11:21 +0200 Subject: [PATCH 1/8] seedrng: shrink read_new_seed: The indicator for ENOSYS would be if getrandom does not return the requested len _and_ errno is ENOSYS. Remove the ret check. seed_rng: just return the retval of the ioctl. seedrng_main: no point in looking at the uid, the command should fail in various ways if we are not allowed to do what we want. function old new delta seedrng_main 948 924 -24 seed_from_file_if_exists 410 383 -27 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-51) Total: -51 bytes --- util-linux/seedrng.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/util-linux/seedrng.c b/util-linux/seedrng.c index 5735dc059..d8120f7b3 100644 --- a/util-linux/seedrng.c +++ b/util-linux/seedrng.c @@ -95,7 +95,7 @@ static int read_new_seed(uint8_t *seed, size_t len, bool *is_creditable) if (ret == (ssize_t)len) { *is_creditable = true; return 0; - } else if (ret < 0 && errno == ENOSYS) { + } else if (/* ret < 0 && */ errno == ENOSYS) { struct pollfd random_fd = { .fd = open("/dev/random", O_RDONLY), .events = POLLIN @@ -133,12 +133,9 @@ static int seed_rng(uint8_t *seed, size_t len, bool credit) if (random_fd < 0) return -1; ret = ioctl(random_fd, RNDADDENTROPY, &req); - if (ret) - ret = -errno ? -errno : -EIO; if (ENABLE_FEATURE_CLEAN_UP) close(random_fd); - errno = -ret; - return ret ? -1 : 0; + return ret; } static int seed_from_file_if_exists(const char *filename, int dfd, bool credit, sha256_ctx_t *hash) @@ -202,7 +199,7 @@ int seedrng_main(int argc UNUSED_PARAM, char *argv[]) non_creditable_seed = concat_path_file(seed_dir, NON_CREDITABLE_SEED_NAME); umask(0077); - if (getuid()) + if (00 && getuid()) bb_simple_error_msg_and_die(bb_msg_you_must_be_root); if (mkdir(seed_dir, 0700) < 0 && errno != EEXIST) -- 2.35.1
>From 8f649ab23a31f21f08c29d039fc20a326fc18468 Mon Sep 17 00:00:00 2001 From: Bernhard Reutner-Fischer <rep.dot....@gmail.com> Date: Tue, 12 Apr 2022 20:11:21 +0200 Subject: [PATCH 2/8] seedrng: manually inline seed_rng seed_rng: inline into single caller. We can now remove a separate buffer and memcpy et al --- util-linux/seedrng.c | 56 ++++++++++++++++++-------------------------- 1 file changed, 23 insertions(+), 33 deletions(-) diff --git a/util-linux/seedrng.c b/util-linux/seedrng.c index d8120f7b3..e8f3b2a72 100644 --- a/util-linux/seedrng.c +++ b/util-linux/seedrng.c @@ -111,39 +111,18 @@ static int read_new_seed(uint8_t *seed, size_t len, bool *is_creditable) return -1; } -static int seed_rng(uint8_t *seed, size_t len, bool credit) +static int seed_from_file_if_exists(const char *filename, int dfd, bool credit, + sha256_ctx_t *hash) { + ssize_t seed_len; + int random_fd, ret; struct { int entropy_count; int buf_size; uint8_t buffer[MAX_SEED_LEN]; - } req = { - .entropy_count = credit ? len * 8 : 0, - .buf_size = len - }; - int random_fd, ret; + } req; - if (len > sizeof(req.buffer)) { - errno = EFBIG; - return -1; - } - memcpy(req.buffer, seed, len); - - random_fd = open("/dev/random", O_RDONLY); - if (random_fd < 0) - return -1; - ret = ioctl(random_fd, RNDADDENTROPY, &req); - if (ENABLE_FEATURE_CLEAN_UP) - close(random_fd); - return ret; -} - -static int seed_from_file_if_exists(const char *filename, int dfd, bool credit, sha256_ctx_t *hash) -{ - uint8_t seed[MAX_SEED_LEN]; - ssize_t seed_len; - - seed_len = open_read_close(filename, seed, sizeof(seed)); + seed_len = open_read_close(filename, req.buffer, sizeof(req.buffer)); if (seed_len < 0) { if (errno == ENOENT) return 0; @@ -155,16 +134,27 @@ static int seed_from_file_if_exists(const char *filename, int dfd, bool credit, return -1; } else if (!seed_len) return 0; + if (seed_len > sizeof(req.buffer)) { + return -1; + } - sha256_hash(hash, &seed_len, sizeof(seed_len)); - sha256_hash(hash, seed, seed_len); - - printf("Seeding %zd bits %s crediting\n", seed_len * 8, credit ? "and" : "without"); - if (seed_rng(seed, seed_len, credit) < 0) { + random_fd = open("/dev/random", O_RDONLY); + if (random_fd < 0) { bb_simple_perror_msg("unable to seed"); return -1; } - return 0; + + sha256_hash(hash, &seed_len, sizeof(seed_len)); + sha256_hash(hash, req.buffer, seed_len); + + printf("Seeding %zd bits %s crediting\n", seed_len * 8, credit ? "and" : "without"); + req.entropy_count = credit ? seed_len * 8 : 0; + req.buf_size = seed_len; + + ret = ioctl(random_fd, RNDADDENTROPY, &req); + if (ENABLE_FEATURE_CLEAN_UP) + close(random_fd); + return ret; } int seedrng_main(int argc, char *argv[]) MAIN_EXTERNALLY_VISIBLE; -- 2.35.1
>From f5ffb3d373ad3bbc337cf0b1c36f4a9ffd5d07c9 Mon Sep 17 00:00:00 2001 From: Bernhard Reutner-Fischer <rep.dot....@gmail.com> Date: Tue, 12 Apr 2022 20:11:21 +0200 Subject: [PATCH 3/8] seedrng: CSE seed_len * 8 function old new delta seed_from_file_if_exists 322 317 -5 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-5) Total: -5 bytes --- util-linux/seedrng.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/util-linux/seedrng.c b/util-linux/seedrng.c index e8f3b2a72..a4ec642a4 100644 --- a/util-linux/seedrng.c +++ b/util-linux/seedrng.c @@ -115,6 +115,7 @@ static int seed_from_file_if_exists(const char *filename, int dfd, bool credit, sha256_ctx_t *hash) { ssize_t seed_len; + unsigned seed_bits; int random_fd, ret; struct { int entropy_count; @@ -147,8 +148,10 @@ static int seed_from_file_if_exists(const char *filename, int dfd, bool credit, sha256_hash(hash, &seed_len, sizeof(seed_len)); sha256_hash(hash, req.buffer, seed_len); - printf("Seeding %zd bits %s crediting\n", seed_len * 8, credit ? "and" : "without"); - req.entropy_count = credit ? seed_len * 8 : 0; + seed_bits = seed_len * 8; + + printf("Seeding %zd bits %s crediting\n", seed_bits, credit ? "and" : "without"); + req.entropy_count = credit ? seed_bits : 0; req.buf_size = seed_len; ret = ioctl(random_fd, RNDADDENTROPY, &req); -- 2.35.1
>From 18e8a9995b207b63fee28846d5a1419569c377ff Mon Sep 17 00:00:00 2001 From: Bernhard Reutner-Fischer <rep.dot....@gmail.com> Date: Tue, 12 Apr 2022 20:11:21 +0200 Subject: [PATCH 4/8] seedrng: remove impossible condition We read only up to sizeof(req.buffer) so we will never read more. function old new delta seed_from_file_if_exists 317 307 -10 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-10) Total: -10 bytes --- util-linux/seedrng.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/util-linux/seedrng.c b/util-linux/seedrng.c index a4ec642a4..092ecfa13 100644 --- a/util-linux/seedrng.c +++ b/util-linux/seedrng.c @@ -135,9 +135,6 @@ static int seed_from_file_if_exists(const char *filename, int dfd, bool credit, return -1; } else if (!seed_len) return 0; - if (seed_len > sizeof(req.buffer)) { - return -1; - } random_fd = open("/dev/random", O_RDONLY); if (random_fd < 0) { -- 2.35.1
>From 198413a0a861b515dd4899feff5f92abe4926faa Mon Sep 17 00:00:00 2001 From: Bernhard Reutner-Fischer <rep.dot....@gmail.com> Date: Tue, 12 Apr 2022 20:11:21 +0200 Subject: [PATCH 5/8] seedrng: simplify seed_dir handling function old new delta seedrng_main 924 906 -18 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-18) Total: -18 bytes --- util-linux/seedrng.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/util-linux/seedrng.c b/util-linux/seedrng.c index 092ecfa13..fe84a2f98 100644 --- a/util-linux/seedrng.c +++ b/util-linux/seedrng.c @@ -160,7 +160,8 @@ static int seed_from_file_if_exists(const char *filename, int dfd, bool credit, int seedrng_main(int argc, char *argv[]) MAIN_EXTERNALLY_VISIBLE; int seedrng_main(int argc UNUSED_PARAM, char *argv[]) { - char *seed_dir, *creditable_seed, *non_creditable_seed; + const char *seed_dir = DEFAULT_SEED_DIR; + char *creditable_seed, *non_creditable_seed; int ret, fd = -1, dfd = -1, program_ret = 0; uint8_t new_seed[MAX_SEED_LEN]; size_t new_seed_len; @@ -182,8 +183,6 @@ int seedrng_main(int argc UNUSED_PARAM, char *argv[]) #endif opt = getopt32long(argv, "d:n", longopts, &seed_dir); - if (!(opt & OPT_d) || !seed_dir) - seed_dir = xstrdup(DEFAULT_SEED_DIR); skip_credit = opt & OPT_n; creditable_seed = concat_path_file(seed_dir, CREDITABLE_SEED_NAME); non_creditable_seed = concat_path_file(seed_dir, NON_CREDITABLE_SEED_NAME); -- 2.35.1
>From 9dbaa9031b749e0eb6842c9f240ee4de53754811 Mon Sep 17 00:00:00 2001 From: Bernhard Reutner-Fischer <rep.dot....@gmail.com> Date: Tue, 12 Apr 2022 20:11:21 +0200 Subject: [PATCH 6/8] seedrng: allow inlining seed_from_file_if_exists inline functions called once. function old new delta seedrng_main 906 1205 +299 seed_from_file_if_exists 307 - -307 ------------------------------------------------------------------------------ (add/remove: 0/1 grow/shrink: 1/0 up/down: 299/-307) Total: -8 bytes --- util-linux/seedrng.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/util-linux/seedrng.c b/util-linux/seedrng.c index fe84a2f98..c2d30d2e4 100644 --- a/util-linux/seedrng.c +++ b/util-linux/seedrng.c @@ -1,4 +1,5 @@ -// SPDX-License-Identifier: GPL-2.0 OR MIT +/* SPDX-License-Identifier: GPL-2.0 OR MIT */ +/* vi: set sw=4 ts=4: */ /* * Copyright (C) 2022 Jason A. Donenfeld <ja...@zx2c4.com>. All Rights Reserved. * @@ -169,7 +170,7 @@ int seedrng_main(int argc UNUSED_PARAM, char *argv[]) bool skip_credit = false; struct timespec timestamp = { 0 }; sha256_ctx_t hash; - + unsigned i; int opt; enum { OPT_d = (1 << 0), @@ -208,12 +209,15 @@ int seedrng_main(int argc UNUSED_PARAM, char *argv[]) clock_gettime(CLOCK_BOOTTIME, ×tamp); sha256_hash(&hash, ×tamp, sizeof(timestamp)); - 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); - if (ret < 0) - program_ret |= 1 << 2; + for (i = 0; i <= 1; i++) { + ret = seed_from_file_if_exists( + i == 0 ? non_creditable_seed : creditable_seed, + dfd, + /* i == 0 ? 0 : !skip_credit,*/ (!skip_credit) & i, + &hash); + if (ret < 0) + program_ret |= 1 << (1 + i); + } new_seed_len = determine_optimal_seed_len(); ret = read_new_seed(new_seed, new_seed_len, &new_seed_creditable); -- 2.35.1
>From 5fb367b85a64db03c2c454547417630e4d1291de Mon Sep 17 00:00:00 2001 From: Bernhard Reutner-Fischer <rep.dot....@gmail.com> Date: Tue, 12 Apr 2022 20:11:21 +0200 Subject: [PATCH 7/8] seedrng: cleanup strings function old new delta seedrng_main 1205 1240 +35 .rodata 108338 108206 -132 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 1/1 up/down: 35/-132) Total: -97 bytes --- util-linux/seedrng.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/util-linux/seedrng.c b/util-linux/seedrng.c index c2d30d2e4..176ff7669 100644 --- a/util-linux/seedrng.c +++ b/util-linux/seedrng.c @@ -128,18 +128,18 @@ static int seed_from_file_if_exists(const char *filename, int dfd, bool credit, if (seed_len < 0) { if (errno == ENOENT) return 0; - bb_simple_perror_msg("unable to read seed file"); + bb_perror_msg("unable to%s seed", " read"); return -1; } if ((unlink(filename) < 0 || fsync(dfd) < 0) && seed_len) { - bb_simple_perror_msg("unable to remove seed, so not seeding"); + bb_perror_msg("unable to%s seed", " remove"); return -1; } else if (!seed_len) return 0; random_fd = open("/dev/random", O_RDONLY); if (random_fd < 0) { - bb_simple_perror_msg("unable to seed"); + bb_perror_msg("unable to%s seed", ""); return -1; } @@ -193,11 +193,11 @@ int seedrng_main(int argc UNUSED_PARAM, char *argv[]) bb_simple_error_msg_and_die(bb_msg_you_must_be_root); if (mkdir(seed_dir, 0700) < 0 && errno != EEXIST) - bb_simple_perror_msg_and_die("unable to create seed directory"); + bb_perror_msg_and_die("unable to %s seed directory", "create"); dfd = open(seed_dir, O_DIRECTORY | O_RDONLY); if (dfd < 0 || flock(dfd, LOCK_EX) < 0) { - bb_simple_perror_msg("unable to lock seed directory"); + bb_perror_msg("unable to %s seed directory", "lock"); program_ret = 1; goto out; } @@ -222,7 +222,7 @@ int seedrng_main(int argc UNUSED_PARAM, char *argv[]) new_seed_len = determine_optimal_seed_len(); ret = read_new_seed(new_seed, new_seed_len, &new_seed_creditable); if (ret < 0) { - bb_simple_perror_msg("unable to read new seed"); + bb_perror_msg("unable to%s seed", " read new"); new_seed_len = SHA256_OUTSIZE; memset(new_seed, 0, SHA256_OUTSIZE); program_ret |= 1 << 3; @@ -231,10 +231,10 @@ int seedrng_main(int argc UNUSED_PARAM, char *argv[]) sha256_hash(&hash, new_seed, new_seed_len); sha256_end(&hash, new_seed + new_seed_len - SHA256_OUTSIZE); - printf("Saving %zu bits of %s seed for next boot\n", new_seed_len * 8, new_seed_creditable ? "creditable" : "non-creditable"); + printf("Saving %zu bits of %screditable seed for next boot\n", new_seed_len * 8, new_seed_creditable ? "" : "non-"); fd = open(non_creditable_seed, O_WRONLY | O_CREAT | O_TRUNC, 0400); if (fd < 0 || full_write(fd, new_seed, new_seed_len) != (ssize_t)new_seed_len || fsync(fd) < 0) { - bb_simple_perror_msg("unable to write seed file"); + bb_perror_msg("unable to%s seed", " write"); program_ret |= 1 << 4; goto out; } -- 2.35.1
>From eaafedb1478e4fece96270b54d34099b88daa8fe Mon Sep 17 00:00:00 2001 From: Bernhard Reutner-Fischer <rep.dot....@gmail.com> Date: Tue, 12 Apr 2022 20:11:21 +0200 Subject: [PATCH 8/8] seedrng: shrink a bit function old new delta seedrng_main 1240 1228 -12 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-12) Total: -12 bytes --- util-linux/seedrng.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/util-linux/seedrng.c b/util-linux/seedrng.c index 176ff7669..124c2c80c 100644 --- a/util-linux/seedrng.c +++ b/util-linux/seedrng.c @@ -69,7 +69,7 @@ #define CREDITABLE_SEED_NAME "seed.credit" #define NON_CREDITABLE_SEED_NAME "seed.no-credit" -enum seedrng_lengths { +enum { MIN_SEED_LEN = SHA256_OUTSIZE, MAX_SEED_LEN = 512 }; @@ -163,15 +163,15 @@ int seedrng_main(int argc UNUSED_PARAM, char *argv[]) { const char *seed_dir = DEFAULT_SEED_DIR; char *creditable_seed, *non_creditable_seed; - int ret, fd = -1, dfd = -1, program_ret = 0; + int fd = -1, dfd = -1, program_ret = 0; uint8_t new_seed[MAX_SEED_LEN]; size_t new_seed_len; bool new_seed_creditable; - bool skip_credit = false; - struct timespec timestamp = { 0 }; + bool skip_credit; + struct timespec timestamp; sha256_ctx_t hash; - unsigned i; int opt; + int i; enum { OPT_d = (1 << 0), OPT_n = (1 << 1) @@ -210,18 +210,17 @@ int seedrng_main(int argc UNUSED_PARAM, char *argv[]) sha256_hash(&hash, ×tamp, sizeof(timestamp)); for (i = 0; i <= 1; i++) { - ret = seed_from_file_if_exists( + /* skip credit for non_creditable seed file or if -n was given */ + if (seed_from_file_if_exists( i == 0 ? non_creditable_seed : creditable_seed, dfd, - /* i == 0 ? 0 : !skip_credit,*/ (!skip_credit) & i, - &hash); - if (ret < 0) + (!skip_credit) & i, + &hash) < 0) program_ret |= 1 << (1 + i); } new_seed_len = determine_optimal_seed_len(); - ret = read_new_seed(new_seed, new_seed_len, &new_seed_creditable); - if (ret < 0) { + if (read_new_seed(new_seed, new_seed_len, &new_seed_creditable)) { bb_perror_msg("unable to%s seed", " read new"); new_seed_len = SHA256_OUTSIZE; memset(new_seed, 0, SHA256_OUTSIZE); -- 2.35.1
_______________________________________________ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox