Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
To: yann.morin.1998@free.fr, buildroot@buildroot.org
Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>
Subject: [Buildroot] [PATCH v2] package/urandom-scripts: hash old seed with new seed when saving
Date: Wed, 23 Mar 2022 14:07:31 -0600	[thread overview]
Message-ID: <20220323200731.170409-1-Jason@zx2c4.com> (raw)
In-Reply-To: <CAHmME9oUAXNpjWF266m3mH5HvEvVWCthy64RHFye+mOktMegzg@mail.gmail.com>

Writing into /dev/urandom doesn't actually credit any entropy bits. And
while it adds that data to the entropy pool, it won't actually be
immediately used when reading from /dev/urandom subsequently. This is
how the kernel's /dev/urandom has always worked, unfortunately.

As a result of this behavior, which may be understandably surprising,
writing a good seed file into /dev/urandom and then saving a new seed
file immediately after is dangerous, because the new seed file may wind
up being entirely deterministic, even if the old seed file was quite
good.

This has been fixed in systemd with
<https://github.com/systemd/systemd/commit/da2862ef06f22fc8d31dafced6d2d6dc14f2ee0b>,
and fortunately it's possible to do the same thing in shell script here.
Specifically, instead of just saving new /dev/urandom output straight
up, we hash the new /dev/urandom together with the old seed, in order to
produce the new seed. This way the amount of entropy in the new seed
will stay the same or get better, but not appreciably regress.

At the same time, the pool size check in this script is useless. Writing
to /dev/urandom never credits bits anyway, so no matter what, writing
into /dev/urandom is useful and not harmful. There's also not much of a
point in seeding with more than 256 bits, which is what the hashing
operation above produces. So this commit removes the file size check.

As a final note, while this commit improves upon the status quo by
removing a vulnerability, this shell script still does not actually
initialize the RNG like it says it does. For initialization via a seed
file, the RNDADDENTROPY ioctl must be used.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 package/urandom-scripts/S20urandom | 39 +++++++++++++-----------------
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/package/urandom-scripts/S20urandom b/package/urandom-scripts/S20urandom
index e4fd125721..c6b2ebd48f 100644
--- a/package/urandom-scripts/S20urandom
+++ b/package/urandom-scripts/S20urandom
@@ -17,43 +17,38 @@ else
 	pool_size=512
 fi
 
-check_file_size() {
-	[ -f "$URANDOM_SEED" ] || return 1
-	# Try to read two blocks but exactly one will be read if the file has
-	# the correct size.
-	size=$(dd if="$URANDOM_SEED" bs="$pool_size" count=2 2> /dev/null | wc -c)
-	test "$size" -eq "$pool_size"
-}
-
 init_rng() {
-	if check_file_size; then
-		printf 'Initializing random number generator: '
-		dd if="$URANDOM_SEED" bs="$pool_size" of=/dev/urandom count=1 2> /dev/null
-		status=$?
-		if [ "$status" -eq 0 ]; then
-			echo "OK"
-		else
-			echo "FAIL"
-		fi
-		return "$status"
+	printf 'Initializing random number generator: '
+	dd if="$URANDOM_SEED" bs="$pool_size" of=/dev/urandom count=1 2> /dev/null
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
 	fi
+	return "$status"
 }
 
 save_random_seed() {
 	printf 'Saving random seed: '
-	if touch "$URANDOM_SEED" 2> /dev/null; then
+	status=1
+	if touch "$URANDOM_SEED.new" 2> /dev/null; then
 		old_umask=$(umask)
 		umask 077
-		dd if=/dev/urandom of="$URANDOM_SEED" bs="$pool_size" count=1 2> /dev/null
-		status=$?
+		dd if=/dev/urandom of="$URANDOM_SEED.tmp" bs="$pool_size" count=1 2> /dev/null
+		cat "$URANDOM_SEED" "$URANDOM_SEED.tmp" 2>/dev/null \
+			| sha256sum \
+			| cut -d ' ' -f 1 > "$URANDOM_SEED.new" && \
+		mv "$URANDOM_SEED.new" "$URANDOM_SEED" && status=0
+		rm -f "$URANDOM_SEED.tmp"
 		umask "$old_umask"
 		if [ "$status" -eq 0 ]; then
 			echo "OK"
 		else
 			echo "FAIL"
 		fi
+
 	else
-		status=$?
 		echo "SKIP (read-only file system detected)"
 	fi
 	return "$status"
-- 
2.35.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

  reply	other threads:[~2022-03-23 20:07 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-23  3:52 [Buildroot] [PATCH] package/urandom-scripts: hash old seed with new seed when saving Jason A. Donenfeld
2022-03-23  5:10 ` Jason A. Donenfeld
2022-03-23  8:43 ` Nicolas Cavallari
2022-03-23  9:13 ` Yann E. MORIN
2022-03-23 13:39   ` Nicolas Cavallari
2022-03-23 20:06   ` Jason A. Donenfeld
2022-03-23 20:07     ` Jason A. Donenfeld [this message]
2022-03-24  8:24       ` [Buildroot] [PATCH v2] " Yann E. MORIN
2022-03-24  9:15         ` David Laight
2022-03-24 10:09           ` Yann E. MORIN
2022-03-24 10:25             ` David Laight
2022-03-24 10:39               ` Yann E. MORIN
2022-03-24 13:06                 ` David Laight
2022-03-24 13:54           ` Jason A. Donenfeld
2022-03-24 14:31             ` David Laight
2022-03-24 14:39               ` Jason A. Donenfeld
2022-03-28 13:17         ` Peter Korsgaard
2022-04-15 10:54           ` Eugen.Hristev--- via buildroot
2022-04-15 12:25             ` Nicolas Cavallari
2022-04-16 11:12               ` Peter Korsgaard
2022-04-16 11:31                 ` [Buildroot] [PATCH] package/urandom-scripts: do not seed if initial seed doesn't exist Jason A. Donenfeld
2022-04-16 13:47                   ` Peter Korsgaard
2022-04-18 20:19                     ` Eugen.Hristev--- via buildroot
2022-04-18 20:36                       ` Jason A. Donenfeld
2022-04-19 10:23                         ` Eugen.Hristev--- via buildroot
2022-04-18 20:50                       ` Peter Korsgaard
2022-05-22 10:11                   ` Peter Korsgaard
2022-04-16  8:29             ` [Buildroot] [PATCH v2] package/urandom-scripts: hash old seed with new seed when saving Peter Korsgaard
2022-03-24  2:41     ` [Buildroot] [PATCH] " Jason A. Donenfeld

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220323200731.170409-1-Jason@zx2c4.com \
    --to=jason@zx2c4.com \
    --cc=buildroot@buildroot.org \
    --cc=yann.morin.1998@free.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox