Re: [Buildroot] [PATCH v2] package/urandom-scripts: hash old seed with new seed when saving

["Yann E. MORIN" <yann.morin.1998@free.fr>]

Jason, All,

+Peter: candidate for backporting as a security fix

On 2022-03-23 14:07 -0600, Jason A. Donenfeld spake thusly:
> 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>

Applied to master, thanks.

Regards,
Yann E. MORIN.

> ---
>  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

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot