Re: [PATCH] random: add rng-seed= command line option

From: "Theodore Y. Ts'o" <tytso@mit.edu>
To: Mark Salyzyn <salyzyn@android.com>
Cc: linux-kernel@vger.kernel.org, kernel-team@android.com,
	Arnd Bergmann <arnd@arndb.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Richard Henderson <richard.henderson@linaro.org>,
	Mark Brown <broonie@kernel.org>,
	Kees Cook <keescook@chromium.org>,
	Hsin-Yi Wang <hsinyi@chromium.org>,
	Vasily Gorbik <gor@linux.ibm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	"Steven Rostedt (VMware)" <rostedt@goodmis.org>,
	Mike Rapoport <rppt@linux.ibm.com>,
	Arvind Sankar <nivedita@alum.mit.edu>,
	Dominik Brodowski <linux@dominikbrodowski.net>,
	Thomas Gleixner <tglx@linutronix.de>,
	Alexander Potapenko <glider@google.com>
Subject: Re: [PATCH] random: add rng-seed= command line option
Date: Fri, 7 Feb 2020 19:49:22 -0500	[thread overview]
Message-ID: <20200208004922.GE122530@mit.edu> (raw)
In-Reply-To: <d35bacd4-ba3f-335d-85c4-57e87abd8e9a@android.com>

On Fri, Feb 07, 2020 at 09:49:17AM -0800, Mark Salyzyn wrote:
> > > It is preferred to add rng-seed to the Device Tree, but some
> > > platforms do not have this option, so this adds the ability to
> > > provide some command-line-limited data to the entropy through this
> > > alternate mechanism.  Expect all 8 bits to be used, but must exclude
> > > space to be accounted in the command line.
> > "all 8 bits"?
> 
> Command line (and Device Tree for that matter) can provide 8-bits of data,
> and specifically for the command line as long as they skip space and nul
> characters, we will be stripping the content out of the command line because
> we strip it from view, so that no one gets hot and bothered.

What wasn't obvious to me initially (and should be clearly documented
in the commit description as well as elsewhere) is that we are already
adding the entire boot command-line string using
"add_device_randomness()" and so what this commit is doing is simply
counting the length of xxx in "rng_seed=xxx" and assuming that those
bytes are 100% entropy and simply crediting the trusted entropy by
length of xxx.  If xxx happened to be a hex string, or worse, was
hard-coded in /etc/grub.conf as "rng_seed=supercalifragilisticexpialidocious"
with this commit (and CONFIG_RANDOM_TRUST_BOOTLOADER), it would assume
that it is safe to credit the boot command line has having sufficient
entropy to fully initialize the CRNG.

> I expected this to be contentious, this is why I call it out. Does _anyone_
> have a disagreement with allowing raw data (minus nul and space characters)
> to be part of the rng-seed?

There are two parts of this that might be controverisial.  The first
is that there isn't actually *fully* eight bits; it's really
log_2(254) bits per caharacter, since NUL and SPC aren't valid.

The second is that we're treating rng_seed as being magic, and if
someone tries to pass in something like rng_seed=0x7932dca76b51
because they didn't understand how rng_seed was going to work, it
would be surprising.

My preference would be to pass in the random seed *not* on the
command-line at all, but as a separate parameter which is passed to
the bootloader, just as we pass in the device-tree, the initrd and the
command-line as separate things.  The problem is that how we pass in
extra boot parameters is architecture specific, and how we might do it
for x86 is different than for arm64.  So yeah, it's a bit more
inconvenient to do things that way; but I think it's also much
cleaner.

					- Ted