Hi Jason!
On Sun, 10 Apr 2022 23:20:34 +0200
"Jason A. Donenfeld" <[email protected]> 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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>. 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 <[email protected]>
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 <[email protected]>
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
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
