Hello Jason,
Am Fri, Apr 29, 2022 at 07:04:47PM +0200 schrieb Jason A. Donenfeld:
> Rather than having getrandom() be called in a loop that handles EINTR --
> which would require more code bloat -- we just limit the maximum seed
> size to 256 bytes, which the kernel guarantees won't be interrupted.
> Additionally document the flock() and fsync() usage so that somebody
> doesn't remove it. Apparently busybox developers like to remove things
> they don't understand with no regards to security implications, so Denys
> suggested I leave some comments here.
>
> Cc: Denys Vlasenko <[email protected]>
> Cc: Bernhard Reutner-Fischer <[email protected]>
> Signed-off-by: Jason A. Donenfeld <[email protected]>
> ---
> util-linux/seedrng.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/util-linux/seedrng.c b/util-linux/seedrng.c
> index c42274759..5ee6197e3 100644
> --- a/util-linux/seedrng.c
> +++ b/util-linux/seedrng.c
> @@ -56,7 +56,7 @@
>
> enum {
> MIN_SEED_LEN = SHA256_OUTSIZE,
> - MAX_SEED_LEN = 512
> + MAX_SEED_LEN = 256 /* Maximum size of getrandom() call without EINTR. */
> };
>
> static size_t determine_optimal_seed_len(void)
> @@ -142,6 +142,8 @@ static int seed_from_file_if_exists(const char *filename,
> int dfd, bool credit,
> bb_perror_msg("can't%s seed", " read");
> return -1;
> }
> + /* The fsync() here is necessary for safety here, so that power being
> pulled
> + * at the wrong moment doesn't result in the seed being used twice by
> accident. */
nit: "here" is used twice in the same sentence.
Greets
Alex
> if ((unlink(filename) < 0 || fsync(dfd) < 0) && seed_len) {
> bb_perror_msg("can't%s seed", " remove");
> return -1;
> @@ -190,6 +192,8 @@ int seedrng_main(int argc UNUSED_PARAM, char *argv[])
> if (mkdir(seed_dir, 0700) < 0 && errno != EEXIST)
> bb_perror_msg_and_die("can't %s seed directory", "create");
> dfd = open(seed_dir, O_DIRECTORY | O_RDONLY);
> + /* The flock() here is absolutely necessary, as the consistency of this
> + * program breaks down with concurrent uses. */
> if (dfd < 0 || flock(dfd, LOCK_EX) < 0)
> bb_perror_msg_and_die("can't %s seed directory", "lock");
> xfchdir(dfd);
> @@ -222,6 +226,8 @@ int seedrng_main(int argc UNUSED_PARAM, char *argv[])
>
> printf("Saving %u bits of %screditable seed for next boot\n",
> (unsigned)new_seed_len * 8, new_seed_creditable ? "" : "non-");
> fd = open(NON_CREDITABLE_SEED_NAME, O_WRONLY | O_CREAT | O_TRUNC, 0400);
> + /* The fsync() here is necessary to ensure the data is written to disk
> before
> + * we attempt to make it creditable. */
> if (fd < 0 || full_write(fd, new_seed, new_seed_len) !=
> (ssize_t)new_seed_len || fsync(fd) < 0) {
> bb_perror_msg("can't%s seed", " write");
> return program_ret | (1 << 4);
> --
> 2.35.1
>
> _______________________________________________
> busybox mailing list
> [email protected]
> http://lists.busybox.net/mailman/listinfo/busybox
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
