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

[Mark Salyzyn]

On 2/13/20 5:16 PM, Masami Hiramatsu wrote:
> Hi Mark,
>
> On Thu, 13 Feb 2020 10:44:59 -0800
> Mark Salyzyn <salyzyn@xxxxxxxxxxx> wrote:
>
> > On 2/13/20 7:03 AM, Masami Hiramatsu wrote:
> > > On Thu, 13 Feb 2020 20:24:54 +0900
> > > Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:
> > >
> > > > > > 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.
> > > > > Hmm, if the boot loader could add on to the bootconfig that Masami just
> > > > > added, then it could add some "random" seed for each boot! The
> > > > > bootconfig is just an appended file at the end of the initrd.
> > > > Yeah, it is easy to add bootconfig support to a bootloader. It can add
> > > > a entropy number as "rng.seed=XXX" text after initrd image with size
> > > > and checksum. That is architecutre independent way to pass such hidden
> > > > parameter.
> > > > (hidden key must be filtered out when printing out the /proc/bootconfig,
> > > > but that is very easy too, just need a strncmp)
> > > And here is the patch to support "random.rng_seed = XXX" option by
> > > bootconfig. Now you can focus on what you want to do. No need to
> > > modify command line strings.
> > LGTM, our virtualized emulator (cuttlefish) folks believe they can do it
> > this way. Albeit keep in mind that there are _thousands_ of embedded
> > bootloaders out there that are comfortable updating DT and kernel
> > command line, but few that add boot configs, so this may add complexity.
> I see, since the bootconfig is a new feature, it will take a time to
> be supported widely. Even though, maybe they can use DT for that
> purpose.
No for cuttlefish and KVM, there is no DT. WE will backport to 4.19 and 
5.4 Android kernels to grab bootconfig.
> > For our use case that caused us to need this, the cuttlefish Android
> > emulated device, not a problem though.
> >
> > However, the entropy _data_ has not been added (see below)
> Oh, I missed that.
>
> > Could you please formally re-submit your patch mhiramet@ with a change
> > to push the _data_ as well to the entropy?
> Yes, I'll do.

Thanks!

> > -- Mark
> >
> > > BTW, if you think you need to pass UTF-8 code as a data, I'm happy to
> > > update the bootconfig to support it. Just for the safeness, I have
> > > limited it by isprint() || isspace().
> > >
> > > Thank you,
> > >
> > > diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
> > > index 26956c006987..43fbbd307204 100644
> > > --- a/drivers/char/Kconfig
> > > +++ b/drivers/char/Kconfig
> > > @@ -554,6 +554,7 @@ config RANDOM_TRUST_CPU
> > >    
> > >    config RANDOM_TRUST_BOOTLOADER
> > >    	bool "Trust the bootloader to initialize Linux's CRNG"
> > > +	select BOOT_CONFIG
> > >    	help
> > >    	Some bootloaders can provide entropy to increase the kernel's initial
> > >    	device randomness. Say Y here to assume the entropy provided by the
> > > diff --git a/drivers/char/random.c b/drivers/char/random.c
> > > index c7f9584de2c8..0ae33bbbd338 100644
> > > --- a/drivers/char/random.c
> > > +++ b/drivers/char/random.c
> > > @@ -2311,3 +2311,11 @@ void add_bootloader_randomness(const void *buf, unsigned int size)
> > >    		add_device_randomness(buf, size);
> > >    }
> > >    EXPORT_SYMBOL_GPL(add_bootloader_randomness);
> > > +
> > > +#if defined(CONFIG_RANDOM_TRUST_BOOTLOADER)
> > > +/* caller called add_device_randomness, but it is from a trusted source */
> > > +void __init credit_trusted_entropy_bits(unsigned int nbits)
> > > +{
> > > +	credit_entropy_bits(&input_pool, nbits);
> > > +}
> > > +#endif
> > > diff --git a/fs/proc/bootconfig.c b/fs/proc/bootconfig.c
> > > index 9955d75c0585..aace466c56ed 100644
> > > --- a/fs/proc/bootconfig.c
> > > +++ b/fs/proc/bootconfig.c
> > > @@ -36,6 +36,9 @@ static int __init copy_xbc_key_value_list(char *dst, size_t size)
> > >    		ret = xbc_node_compose_key(leaf, key, XBC_KEYLEN_MAX);
> > >    		if (ret < 0)
> > >    			break;
> > > +		/* For keeping security reason, remove randomness key */
> > > +		if (!strcmp(key, RANDOM_SEED_XBC_KEY))
> > > +			continue;
> > >    		ret = snprintf(dst, rest(dst, end), "%s = ", key);
> > >    		if (ret < 0)
> > >    			break;
> > > diff --git a/include/linux/random.h b/include/linux/random.h
> > > index d319f9a1e429..c8f41ab4f342 100644
> > > --- a/include/linux/random.h
> > > +++ b/include/linux/random.h
> > > @@ -20,6 +20,13 @@ struct random_ready_callback {
> > >    
> > >    extern void add_device_randomness(const void *, unsigned int);
> > >    extern void add_bootloader_randomness(const void *, unsigned int);
> > > +#if defined(CONFIG_RANDOM_TRUST_BOOTLOADER)
> > > +extern void __init credit_trusted_entropy_bits(unsigned int nbits);
> > > +#else
> > > +static inline void credit_trusted_entropy_bits(unsigned int nbits) {}
> > > +#endif
> > > +
> > > +#define RANDOM_SEED_XBC_KEY "random.rng_seed"
> > >    
> > >    #if defined(LATENT_ENTROPY_PLUGIN) && !defined(__CHECKER__)
> > >    static inline void add_latent_entropy(void)
> > > diff --git a/init/main.c b/init/main.c
> > > index f95b014a5479..6c3f51bc76d5 100644
> > > --- a/init/main.c
> > > +++ b/init/main.c
> > > @@ -776,6 +776,32 @@ void __init __weak arch_call_rest_init(void)
> > >    	rest_init();
> > >    }
> > >    
> > > +static __always_inline void __init collect_entropy(const char *command_line)
> > > +{
> > > +	/*
> > > +	 * For best initial stack canary entropy, prepare it after:
> > > +	 * - setup_arch() for any UEFI RNG entropy and boot cmdline access
> > > +	 * - timekeeping_init() for ktime entropy used in rand_initialize()
> > > +	 * - rand_initialize() to get any arch-specific entropy like RDRAND
> > > +	 * - add_latent_entropy() to get any latent entropy
> > > +	 * - adding command line entropy
> > > +	 */
> > > +	rand_initialize();
> > > +	add_latent_entropy();
> > > +	add_device_randomness(command_line, strlen(command_line));
> > > +	if (IS_BUILTIN(CONFIG_RANDOM_TRUST_BOOTLOADER)) {
> > > +		/*
> > > +		 * Added bootconfig device randomness above,
> > This part is incorrect, the rng_seed collected below was _not_ added to
> > the device_randomness.
> >
> > add_device_randomness(rng_seed, strlen(rng_seed)) needs to be pushed
> > below along with the credit.
> OK, as same as above command_line, I'll add that.
>
> Thank you,
>
> > > +		 * now add entropy credit for just random.rng_seed=<data>
> > > +		 */
> > > +		const char *rng_seed = xbc_find_value(RANDOM_SEED_XBC_KEY, NULL);
> > > +
> > > +		if (rng_seed)
> > > +			credit_trusted_entropy_bits(strlen(rng_seed) * 6);
> > > +	}
> > > +	boot_init_stack_canary();
> > > +}
> > > +
> > >    asmlinkage __visible void __init start_kernel(void)
> > >    {
> > >    	char *command_line;
> > > @@ -887,18 +913,7 @@ asmlinkage __visible void __init start_kernel(void)
> > >    	softirq_init();
> > >    	timekeeping_init();
> > >    
> > > -	/*
> > > -	 * For best initial stack canary entropy, prepare it after:
> > > -	 * - setup_arch() for any UEFI RNG entropy and boot cmdline access
> > > -	 * - timekeeping_init() for ktime entropy used in rand_initialize()
> > > -	 * - rand_initialize() to get any arch-specific entropy like RDRAND
> > > -	 * - add_latent_entropy() to get any latent entropy
> > > -	 * - adding command line entropy
> > > -	 */
> > > -	rand_initialize();
> > > -	add_latent_entropy();
> > > -	add_device_randomness(command_line, strlen(command_line));
> > > -	boot_init_stack_canary();
> > > +	collect_entropy(command_line);
> > >    
> > >    	time_init();
> > >    	printk_safe_init();
-- MArk