Received-SPF: softfail (google.com: domain of transitioning linux-crypto-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
User-Agent: Cyrus-JMAP/3.5.0-alpha0-4778-g14fba9972e-fm-20220217.001-g14fba997
Mime-Version: 1.0
Message-Id: <6e117393-9c2f-441c-9c72-62c209643622@www.fastmail.com>
In-Reply-To: <20220217162848.303601-1-Jason@zx2c4.com>
References: <20220217162848.303601-1-Jason@zx2c4.com>
Date:   Mon, 21 Feb 2022 10:01:37 -0800
From:   "Andy Lutomirski" <luto@kernel.org>
To:     "Jason A. Donenfeld" <Jason@zx2c4.com>,
        "Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
        "Linux Crypto Mailing List" <linux-crypto@vger.kernel.org>,
        linux-arch@vger.kernel.org
Cc:     "Dinh Nguyen" <dinguyen@kernel.org>,
        "Nick Hu" <nickhu@andestech.com>,
        "Max Filippov" <jcmvbkbc@gmail.com>,
        "Palmer Dabbelt" <palmer@dabbelt.com>,
        "David S . Miller" <davem@davemloft.net>,
        "Yoshinori Sato" <ysato@users.sourceforge.jp>,
        "Michal Simek" <monstr@monstr.eu>,
        "Borislav Petkov" <bp@alien8.de>, "Guo Ren" <guoren@kernel.org>,
        "Geert Uytterhoeven" <geert@linux-m68k.org>,
        "Joshua Kinard" <kumba@gentoo.org>,
        "David Laight" <David.Laight@aculab.com>,
        "Dominik Brodowski" <linux@dominikbrodowski.net>,
        "Eric Biggers" <ebiggers@google.com>,
        "Ard Biesheuvel" <ardb@kernel.org>,
        "Arnd Bergmann" <arnd@arndb.de>,
        "Thomas Gleixner" <tglx@linutronix.de>,
        "Kees Cook" <keescook@chromium.org>,
        "Lennart Poettering" <mzxreary@0pointer.de>,
        "Konstantin Ryabitsev" <konstantin@linuxfoundation.org>,
        "Linus Torvalds" <torvalds@linux-foundation.org>,
        "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
        "Theodore Ts'o" <tytso@mit.edu>
Subject: Re: [PATCH v1] random: block in /dev/urandom
Content-Type: text/plain
Precedence: bulk

On Thu, Feb 17, 2022, at 8:28 AM, 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".
>

This patch is 100% about a historical mistake.  Way back when (not actually that long ago), there were two usable interfaces to the random number generator: /dev/random and /dev/urandom.  /dev/random was, at least in principle, secure, but it blocked unnecessarily and was, therefore, incredibly slow.  It was totally unsuitable for repeated use by any sort of server.  /dev/urandom didn't block but was insecure if called too early.  *But* urandom was also the correct interface to get best-effort-i-need-them-right-now random bits.  The actual semantics that general crypography users wanted were not available.

Fast forward to today.  /dev/random has the correct semantics for cryptographic purposes.  getrandom() also has the correct semantics for cryptographic purposes and is reliable as such -- it is guaranteed to either not exist or to DTRT.  And best-effort users can use GRND_INSECURE or /dev/urandom.

If we imagine that every user program we care about uses GRND_INSECURE for best-effort and /dev/random or getrandom() without GRND_INSECURE for cryptography, then we're in great shape and this patch is irrelevant.

But we don't get to rely on that.  New kernels are supposed to be compatible with old userspace.  And with *old* userspace, we do not know whether /dev/urandom users want cryptographically secure output or whether they want insecure output.  And there is this window during boot that lasts, supposedly, up to 1 second, there is a massive difference. [0]

So, sorry, this patch is an ABI break.  You're reinterpreting any program that wanted best-effort randomness right after boot as wanting cryptographic randomness, this can delay boot by up to a second [0], and that's more than enough delay to be considered a break.

So I don't like this without a stronger justification and a clearer compatibility story.  I could *maybe* get on board if you had a urandom=insecure boot option to switch back to the old behavior and a very clear message like "random: startup of %s is delayed. Set urandom=insecure for faster boot if you do not need cryptographically secure urandom during boot", but I don't think this patch is okay otherwise.

Or we stick with the status quo and make the warning clearer.  "random: %s us using insecure urandom output.  Fix it to use getrandom() or /dev/rando as appropriate."

[0] I just booted 5.16 in a Skylake -rdrand,-rdseed VM and it took 1.14 seconds to initialize.