Message-ID: <Yiz/wm3nlJ/wOo6n@sol.localdomain>
Date:   Sat, 12 Mar 2022 12:17:06 -0800
From:   Eric Biggers <ebiggers@...nel.org>
To:     "Jason A. Donenfeld" <Jason@...c4.com>
Cc:     linux-kernel@...r.kernel.org, linux-crypto@...r.kernel.org,
        linux-arch@...r.kernel.org, Dinh Nguyen <dinguyen@...nel.org>,
        Nick Hu <nickhu@...estech.com>,
        Max Filippov <jcmvbkbc@...il.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        "David S . Miller" <davem@...emloft.net>,
        Yoshinori Sato <ysato@...rs.sourceforge.jp>,
        Michal Simek <monstr@...str.eu>,
        Borislav Petkov <bp@...en8.de>, Guo Ren <guoren@...nel.org>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Joshua Kinard <kumba@...too.org>,
        David Laight <David.Laight@...lab.com>,
        Dominik Brodowski <linux@...inikbrodowski.net>,
        Ard Biesheuvel <ardb@...nel.org>,
        Arnd Bergmann <arnd@...db.de>,
        Thomas Gleixner <tglx@...utronix.de>,
        Andy Lutomirski <luto@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        Lennart Poettering <mzxreary@...inter.de>,
        Konstantin Ryabitsev <konstantin@...uxfoundation.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Theodore Ts'o <tytso@....edu>
Subject: Re: [PATCH v1] random: block in /dev/urandom

On Thu, Feb 17, 2022 at 05:28:48PM +0100, Jason A. Donenfeld wrote:
> This topic has come up countless times, and usually doesn't go anywhere.
> This time I thought I'd bring it up with a slightly narrower focus,
> updated for some developments over the last three years: we finally can
> make /dev/urandom always secure, in light of the fact that our RNG is
> now always seeded.
> 
> Ever since Linus' 50ee7529ec45 ("random: try to actively add entropy
> rather than passively wait for it"), the RNG does a haveged-style jitter
> dance around the scheduler, in order to produce entropy (and credit it)
> for the case when we're stuck in wait_for_random_bytes(). How ever you
> feel about the Linus Jitter Dance is beside the point: it's been there
> for three years and usually gets the RNG initialized in a second or so.
> 
> As a matter of fact, this is what happens currently when people use
> getrandom(). It's already there and working, and most people have been
> using it for years without realizing.
> 
> So, given that the kernel has grown this mechanism for seeding itself
> from nothing, and that this procedure happens pretty fast, maybe there's
> no point any longer in having /dev/urandom give insecure bytes. In the
> past we didn't want the boot process to deadlock, which was
> understandable. But now, in the worst case, a second goes by, and the
> problem is resolved. It seems like maybe we're finally at a point when
> we can get rid of the infamous "urandom read hole".
> 
> The one slight drawback is that the Linus Jitter Dance relies on random_
> get_entropy() being implemented. The first lines of try_to_generate_
> entropy() are:
> 
> 	stack.now = random_get_entropy();
> 	if (stack.now == random_get_entropy())
> 		return;
> 
> On most platforms, random_get_entropy() is simply aliased to get_cycles().
> The number of machines without a cycle counter or some other
> implementation of random_get_entropy() in 2022, which can also run a
> mainline kernel, and at the same time have a both broken and out of date
> userspace that relies on /dev/urandom never blocking at boot is thought
> to be exceedingly low. And to be clear: those museum pieces without
> cycle counters will continue to run Linux just fine, and even
> /dev/urandom will be operable just like before; the RNG just needs to be
> seeded first through the usual means, which should already be the case
> now.
> 
> On systems that really do want unseeded randomness, we already offer
> getrandom(GRND_INSECURE), which is in use by, e.g., systemd for seeding
> their hash tables at boot. Nothing in this commit would affect
> GRND_INSECURE, and it remains the means of getting those types of random
> numbers.
> 
> This patch goes a long way toward eliminating a long overdue userspace
> crypto footgun. After several decades of endless user confusion, we will
> finally be able to say, "use any single one of our random interfaces and
> you'll be fine. They're all the same. It doesn't matter." And that, I
> think, is really something. Finally all of those blog posts and
> disagreeing forums and contradictory articles will all become correct
> about whatever they happened to recommend, and along with it, a whole
> class of vulnerabilities eliminated.
> 
> With very minimal downside, we're finally in a position where we can
> make this change.
> 
> Cc: Dinh Nguyen <dinguyen@...nel.org>
> Cc: Nick Hu <nickhu@...estech.com>
> Cc: Max Filippov <jcmvbkbc@...il.com>
> Cc: Palmer Dabbelt <palmer@...belt.com>
> Cc: David S. Miller <davem@...emloft.net>
> Cc: Yoshinori Sato <ysato@...rs.sourceforge.jp>
> Cc: Michal Simek <monstr@...str.eu>
> Cc: Borislav Petkov <bp@...en8.de>
> Cc: Guo Ren <guoren@...nel.org>
> Cc: Geert Uytterhoeven <geert@...ux-m68k.org>
> Cc: Joshua Kinard <kumba@...too.org>
> Cc: David Laight <David.Laight@...lab.com>
> Cc: Dominik Brodowski <linux@...inikbrodowski.net>
> Cc: Eric Biggers <ebiggers@...gle.com>
> Cc: Ard Biesheuvel <ardb@...nel.org>
> Cc: Arnd Bergmann <arnd@...db.de>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Andy Lutomirski <luto@...nel.org>
> Cc: Kees Cook <keescook@...omium.org>
> Cc: Lennart Poettering <mzxreary@...inter.de>
> Cc: Konstantin Ryabitsev <konstantin@...uxfoundation.org>
> Cc: Linus Torvalds <torvalds@...ux-foundation.org>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Cc: Theodore Ts'o <tytso@....edu>
> Signed-off-by: Jason A. Donenfeld <Jason@...c4.com>
> ---
> Having learned that MIPS32 isn't affected by this (initially my largest
> worry), and then heartened today upon reading LWN's summary of our
> previous discussion ("it would seem there are no huge barriers to
> removing the final distinction between /dev/random and /dev/urandom"), I
> figured I'd go ahead and submit a v1 of this. It seems at least worth
> trying and seeing if somebody arrives with legitimate complaints. To
> that end I've also widened the CC list quite a bit.
> 
> Changes v0->v1:
> - We no longer touch GRND_INSECURE at all, in anyway. Lennart (and to an
>   extent, Andy) pointed out that getting insecure numbers immediately at
>   boot is still something that has legitimate use cases, so this patch
>   no longer touches that code.
> 
>  drivers/char/mem.c     |  2 +-
>  drivers/char/random.c  | 51 ++++++------------------------------------
>  include/linux/random.h |  2 +-
>  3 files changed, 9 insertions(+), 46 deletions(-)
> 

Just a small nit: the comments above rng_is_initialized() and
wait_for_random_bytes() still imply that /dev/urandom is nonblocking.

- Eric

Powered by blists - more mailing lists