Linux-Doc Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/3] random: add random.rng_seed to bootconfig entry
@ 2020-02-14  6:10 Masami Hiramatsu
  2020-02-14  6:10 ` [PATCH 1/3] bootconfig: Support non-ascii characters in value Masami Hiramatsu
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Masami Hiramatsu @ 2020-02-14  6:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-team, Mark Salyzyn, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Richard Henderson, Mark Brown, Kees Cook,
	Hsin-Yi Wang, Vasily Gorbik, Andrew Morton, Masami Hiramatsu,
	Steven Rostedt, Mike Rapoport, Arvind Sankar, Dominik Brodowski,
	Thomas Gleixner, Alexander Potapenko, Jonathan Corbet,
	Mauro Carvalho Chehab, Josh Poimboeuf, Pawan Gupta,
	Juergen Gross, Rob Herring, linux-doc

Hi,

The following series is bootconfig based implementation of
the rng_seed option patch originally from Mark Salyzyn.
Note that I removed unrelated command line fixes from this
series.

To complete the support of UTF-8 for rng_seed, I added [1/3]
to support non-ascii chars on the value (user can use 0x80-
0xff at the value of bootconfig).

For [3/3], I updated to use bootconfig (xbc_find_value)
instead of command line. Also move the documentation under
Documentation/admin-guide/bootconfig.

Thank you,

---

Mark Salyzyn (2):
      random: rng-seed source is utf-8
      random: add random.rng_seed= bootconfig option

Masami Hiramatsu (1):
      bootconfig: Support non-ascii characters in value


 Documentation/admin-guide/bootconfig/random.rst |   21 ++++++++++++
 drivers/char/Kconfig                            |    1 +
 drivers/char/random.c                           |   10 +++++-
 fs/proc/bootconfig.c                            |    4 ++
 include/linux/random.h                          |    7 ++++
 init/main.c                                     |   41 ++++++++++++++++-------
 lib/bootconfig.c                                |    2 +
 tools/bootconfig/samples/good-non-ascii.bconf   |    1 +
 8 files changed, 73 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/admin-guide/bootconfig/random.rst
 create mode 100644 tools/bootconfig/samples/good-non-ascii.bconf

-- 
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/3] bootconfig: Support non-ascii characters in value
  2020-02-14  6:10 [PATCH 0/3] random: add random.rng_seed to bootconfig entry Masami Hiramatsu
@ 2020-02-14  6:10 ` Masami Hiramatsu
  2020-02-14  6:10 ` [PATCH 2/3] random: rng-seed source is utf-8 Masami Hiramatsu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 17+ messages in thread
From: Masami Hiramatsu @ 2020-02-14  6:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-team, Mark Salyzyn, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Richard Henderson, Mark Brown, Kees Cook,
	Hsin-Yi Wang, Vasily Gorbik, Andrew Morton, Masami Hiramatsu,
	Steven Rostedt, Mike Rapoport, Arvind Sankar, Dominik Brodowski,
	Thomas Gleixner, Alexander Potapenko, Jonathan Corbet,
	Mauro Carvalho Chehab, Josh Poimboeuf, Pawan Gupta,
	Juergen Gross, Rob Herring, linux-doc

Support non-ascii (u8 code > 128) characters in the
value data. This will allow user to pass utf-8 data
as a value in a bootconfig. (Note that ascii non-
printable characters are still not allowed.)

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 lib/bootconfig.c                              |    2 +-
 tools/bootconfig/samples/good-non-ascii.bconf |    1 +
 2 files changed, 2 insertions(+), 1 deletion(-)
 create mode 100644 tools/bootconfig/samples/good-non-ascii.bconf

diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 3ea601a2eba5..9f15b8bef3a0 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -466,7 +466,7 @@ static int __init __xbc_parse_value(char **__v, char **__n)
 	}
 	p = v - 1;
 	while ((c = *++p)) {
-		if (!isprint(c) && !isspace(c))
+		if (c >= 0 && !isprint(c) && !isspace(c))
 			return xbc_parse_error("Non printable value", p);
 		if (quotes) {
 			if (c != quotes)
diff --git a/tools/bootconfig/samples/good-non-ascii.bconf b/tools/bootconfig/samples/good-non-ascii.bconf
new file mode 100644
index 000000000000..b6cb4c24fad6
--- /dev/null
+++ b/tools/bootconfig/samples/good-non-ascii.bconf
@@ -0,0 +1 @@
+hello.japanese = "こんにちは"


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 2/3] random: rng-seed source is utf-8
  2020-02-14  6:10 [PATCH 0/3] random: add random.rng_seed to bootconfig entry Masami Hiramatsu
  2020-02-14  6:10 ` [PATCH 1/3] bootconfig: Support non-ascii characters in value Masami Hiramatsu
@ 2020-02-14  6:10 ` Masami Hiramatsu
  2020-02-14 18:14   ` Hsin-Yi Wang
  2020-02-14 19:58   ` Rob Herring
  2020-02-14  6:10 ` [PATCH 3/3] random: add random.rng_seed= bootconfig option Masami Hiramatsu
  2020-02-14 13:49 ` [PATCH 0/3] random: add random.rng_seed to bootconfig entry Rob Herring
  3 siblings, 2 replies; 17+ messages in thread
From: Masami Hiramatsu @ 2020-02-14  6:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-team, Mark Salyzyn, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Richard Henderson, Mark Brown, Kees Cook,
	Hsin-Yi Wang, Vasily Gorbik, Andrew Morton, Masami Hiramatsu,
	Steven Rostedt, Mike Rapoport, Arvind Sankar, Dominik Brodowski,
	Thomas Gleixner, Alexander Potapenko, Jonathan Corbet,
	Mauro Carvalho Chehab, Josh Poimboeuf, Pawan Gupta,
	Juergen Gross, Rob Herring, linux-doc

From: Mark Salyzyn <salyzyn@android.com>

commit 428826f5358c922dc378830a1717b682c0823160
("fdt: add support for rng-seed") makes the assumption that the data
in rng-seed is binary, when it is typically constructed of utf-8
characters which has a bitness of roughly 6 to give appropriate
credit due for the entropy.

Fixes: 428826f5358c ("fdt: add support for rng-seed")
Signed-off-by: Mark Salyzyn <salyzyn@android.com>
Cc: linux-kernel@vger.kernel.org
Cc: kernel-team@android.com
Cc: Kees Cook <keescook@chromium.org>
Cc: Theodore Y. Ts'o <tytso@mit.edu>
---
 drivers/char/random.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index c7f9584de2c8..ee21a6a584b1 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -2306,7 +2306,7 @@ EXPORT_SYMBOL_GPL(add_hwgenerator_randomness);
 void add_bootloader_randomness(const void *buf, unsigned int size)
 {
 	if (IS_ENABLED(CONFIG_RANDOM_TRUST_BOOTLOADER))
-		add_hwgenerator_randomness(buf, size, size * 8);
+		add_hwgenerator_randomness(buf, size, size * 6);
 	else
 		add_device_randomness(buf, size);
 }


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 3/3] random: add random.rng_seed= bootconfig option
  2020-02-14  6:10 [PATCH 0/3] random: add random.rng_seed to bootconfig entry Masami Hiramatsu
  2020-02-14  6:10 ` [PATCH 1/3] bootconfig: Support non-ascii characters in value Masami Hiramatsu
  2020-02-14  6:10 ` [PATCH 2/3] random: rng-seed source is utf-8 Masami Hiramatsu
@ 2020-02-14  6:10 ` Masami Hiramatsu
  2020-02-14 13:49 ` [PATCH 0/3] random: add random.rng_seed to bootconfig entry Rob Herring
  3 siblings, 0 replies; 17+ messages in thread
From: Masami Hiramatsu @ 2020-02-14  6:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-team, Mark Salyzyn, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Richard Henderson, Mark Brown, Kees Cook,
	Hsin-Yi Wang, Vasily Gorbik, Andrew Morton, Masami Hiramatsu,
	Steven Rostedt, Mike Rapoport, Arvind Sankar, Dominik Brodowski,
	Thomas Gleixner, Alexander Potapenko, Jonathan Corbet,
	Mauro Carvalho Chehab, Josh Poimboeuf, Pawan Gupta,
	Juergen Gross, Rob Herring, linux-doc

From: Mark Salyzyn <salyzyn@android.com>

A followup to commit 428826f5358c922dc378830a1717b682c0823160
("fdt: add support for rng-seed") to extend what was started
with Open Firmware (OF or Device Tree) parsing, but also add
it to the bootconfig.

If CONFIG_RANDOM_TRUST_BOOTLOADER is set, then feed the
random.rng_seed bootconfig data length as added trusted
entropy.

Always erase view of the random.rng_seed option from
/proc/bootconfig to prevent leakage to applications or modules,
to eliminate any attack vector.  Note that initcall embedded
code still have a chance to see it, but that will be unsafe
at different level.

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 bootconfig-limited data to the entropy through this
alternate mechanism.  Expect on average 6 bits of useful entropy
per character.

Signed-off-by: Mark Salyzyn <salyzyn@android.com>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: linux-kernel@vger.kernel.org
Cc: kernel-team@android.com
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Hsin-Yi Wang <hsinyi@chromium.org>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Cc: Mike Rapoport <rppt@linux.ibm.com>
Cc: Arvind Sankar <nivedita@alum.mit.edu>
Cc: Dominik Brodowski <linux@dominikbrodowski.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Alexander Potapenko <glider@google.com>
---
v4
- Use bootconfig instead of command line
- Move the documentation under Documentation/admin-guide/bootconfig/.
v3
- Add Documentation (all other new v2 patches unchanged)

v2
- Split into four bite sized patches.
- Correct spelling in commit message.
- rng-seed is assumed to be utf-8, so correct both to 6 bits/character
  of collected entropy.
- Move entropy collection to a static __always_inline helper function.
---
 Documentation/admin-guide/bootconfig/random.rst |   21 ++++++++++++
 drivers/char/Kconfig                            |    1 +
 drivers/char/random.c                           |    8 ++++
 fs/proc/bootconfig.c                            |    4 ++
 include/linux/random.h                          |    7 ++++
 init/main.c                                     |   41 ++++++++++++++++-------
 6 files changed, 70 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/admin-guide/bootconfig/random.rst

diff --git a/Documentation/admin-guide/bootconfig/random.rst b/Documentation/admin-guide/bootconfig/random.rst
new file mode 100644
index 000000000000..d4ee513c5136
--- /dev/null
+++ b/Documentation/admin-guide/bootconfig/random.rst
@@ -0,0 +1,21 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===============================
+The Random Subsystem Bootconfig
+===============================
+
+The keys start with "random." configures random number generator subsystem.
+
+Options
+=======
+
+random.rng_seed
+  Provide a trusted seed for the kernel's CRNG. Seed only trusted if
+  CONFIG_RANDOM_TRUST_BOOTLOADER=y.  After collection, this option is not
+  shown in /proc/bootconfig.
+  The seed is given a weight of 6 bits per character with the assumption that
+  it is a printable utf8 string.  It is expected that the supplier of the
+  seed, typically a bootloader or virtualization, will supply a new random
+  seed for each kernel instance.
+  A fixed serial number is typically not appropriate for security features
+  like ASLR.
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 ee21a6a584b1..83c77306e18e 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..6d1a819f2df4 100644
--- a/fs/proc/bootconfig.c
+++ b/fs/proc/bootconfig.c
@@ -8,6 +8,7 @@
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 #include <linux/bootconfig.h>
+#include <linux/random.h>
 #include <linux/slab.h>
 
 static char *saved_boot_config;
@@ -36,6 +37,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..d0e5a95b4182 100644
--- a/init/main.c
+++ b/init/main.c
@@ -776,6 +776,34 @@ 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,
+		 * 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) {
+			add_device_randomness(rng_seed, strlen(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 +915,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();


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/3] random: add random.rng_seed to bootconfig entry
  2020-02-14  6:10 [PATCH 0/3] random: add random.rng_seed to bootconfig entry Masami Hiramatsu
                   ` (2 preceding siblings ...)
  2020-02-14  6:10 ` [PATCH 3/3] random: add random.rng_seed= bootconfig option Masami Hiramatsu
@ 2020-02-14 13:49 ` Rob Herring
  2020-02-14 17:00   ` Mark Salyzyn
  3 siblings, 1 reply; 17+ messages in thread
From: Rob Herring @ 2020-02-14 13:49 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: linux-kernel, Android Kernel Team, Mark Salyzyn,
	Theodore Ts'o, Arnd Bergmann, Greg Kroah-Hartman,
	Richard Henderson, Mark Brown, Kees Cook, Hsin-Yi Wang,
	Vasily Gorbik, Andrew Morton, Steven Rostedt, Mike Rapoport,
	Arvind Sankar, Dominik Brodowski, Thomas Gleixner,
	Alexander Potapenko, Jonathan Corbet, Mauro Carvalho Chehab,
	Josh Poimboeuf, Pawan Gupta, Juergen Gross,
	Linux Doc Mailing List

On Fri, Feb 14, 2020 at 12:10 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> Hi,
>
> The following series is bootconfig based implementation of
> the rng_seed option patch originally from Mark Salyzyn.
> Note that I removed unrelated command line fixes from this
> series.

Why do we need this? There's already multiple other ways to pass
random seed and this doesn't pass the "too complex for the command
line" argument you had for needing bootconfig.

Rob

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/3] random: add random.rng_seed to bootconfig entry
  2020-02-14 13:49 ` [PATCH 0/3] random: add random.rng_seed to bootconfig entry Rob Herring
@ 2020-02-14 17:00   ` Mark Salyzyn
  2020-02-14 18:14     ` Rob Herring
  0 siblings, 1 reply; 17+ messages in thread
From: Mark Salyzyn @ 2020-02-14 17:00 UTC (permalink / raw)
  To: Rob Herring, Masami Hiramatsu
  Cc: linux-kernel, Android Kernel Team, Theodore Ts'o,
	Arnd Bergmann, Greg Kroah-Hartman, Richard Henderson, Mark Brown,
	Kees Cook, Hsin-Yi Wang, Vasily Gorbik, Andrew Morton,
	Steven Rostedt, Mike Rapoport, Arvind Sankar, Dominik Brodowski,
	Thomas Gleixner, Alexander Potapenko, Jonathan Corbet,
	Mauro Carvalho Chehab, Josh Poimboeuf, Pawan Gupta,
	Juergen Gross, Linux Doc Mailing List

On 2/14/20 5:49 AM, Rob Herring wrote:
> On Fri, Feb 14, 2020 at 12:10 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>> Hi,
>>
>> The following series is bootconfig based implementation of
>> the rng_seed option patch originally from Mark Salyzyn.
>> Note that I removed unrelated command line fixes from this
>> series.
> Why do we need this? There's already multiple other ways to pass
> random seed and this doesn't pass the "too complex for the command
> line" argument you had for needing bootconfig.
>
> Rob

Android is the use case I can vouch for. But also KVM.

Android Cuttlefish is an emulated device used extensively in the testing 
and development infrastructure for In-house, partner, and system and 
application developers for Android. There is no bootloader, per-se. 
Because of the Android GKI distribution, there is also no rng virtual 
driver built in, it is loaded later as a module, too late for many 
aspects of KASLR and networking. There is no Device Tree, it does 
however have access to the content of the initrd image, and to the 
command line for the kernel. The only convenient way to get early 
entropy is going to have to be one of those two places.

In addition, 2B Android devices on the planet, especially in light of 
the Android GKI distribution were everything that is vendor created is 
in a module, needs a way to collect early entropy prior to module load 
and pass it to the kernel. Yes, they do have access to the recently 
added Device Tree approach, and we expect them to use it, as I have an 
active backport for the mechanism into the Android 4.19 and 5.4 kernels. 
There may also be some benefit to allowing the 13000 different 
bootloaders an option to use bootconfig as a way of propagating the much 
needed entropy to their kernels. I could make a case to also allow them 
command line as another option to relieve their development stress to 
deliver product, but we can stop there. Regardless, this early entropy 
has the benefit of greatly improving security and precious boot time.

Sincerely -- Mark Salyzyn


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] random: rng-seed source is utf-8
  2020-02-14  6:10 ` [PATCH 2/3] random: rng-seed source is utf-8 Masami Hiramatsu
@ 2020-02-14 18:14   ` Hsin-Yi Wang
  2020-02-14 19:58   ` Rob Herring
  1 sibling, 0 replies; 17+ messages in thread
From: Hsin-Yi Wang @ 2020-02-14 18:14 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: lkml, kernel-team, Mark Salyzyn, Theodore Ts'o,
	Arnd Bergmann, Greg Kroah-Hartman, Richard Henderson, Mark Brown,
	Kees Cook, Vasily Gorbik, Andrew Morton, Steven Rostedt,
	Mike Rapoport, Arvind Sankar, Dominik Brodowski, Thomas Gleixner,
	Alexander Potapenko, Jonathan Corbet, Mauro Carvalho Chehab,
	Josh Poimboeuf, Pawan Gupta, Juergen Gross, Rob Herring,
	linux-doc

On Fri, Feb 14, 2020 at 2:10 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> From: Mark Salyzyn <salyzyn@android.com>
>
> commit 428826f5358c922dc378830a1717b682c0823160
> ("fdt: add support for rng-seed") makes the assumption that the data
> in rng-seed is binary, when it is typically constructed of utf-8
> characters which has a bitness of roughly 6 to give appropriate
> credit due for the entropy.
>
> Fixes: 428826f5358c ("fdt: add support for rng-seed")
> Signed-off-by: Mark Salyzyn <salyzyn@android.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: kernel-team@android.com
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Theodore Y. Ts'o <tytso@mit.edu>
> ---
>  drivers/char/random.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index c7f9584de2c8..ee21a6a584b1 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -2306,7 +2306,7 @@ EXPORT_SYMBOL_GPL(add_hwgenerator_randomness);
>  void add_bootloader_randomness(const void *buf, unsigned int size)
>  {
>         if (IS_ENABLED(CONFIG_RANDOM_TRUST_BOOTLOADER))
> -               add_hwgenerator_randomness(buf, size, size * 8);
> +               add_hwgenerator_randomness(buf, size, size * 6);
Hi,

In the next patch, entropy is added by
+                       add_device_randomness(rng_seed, strlen(rng_seed));
+                       credit_trusted_entropy_bits(strlen(rng_seed) * 6);

If the add_bootloader_randomness() function is only used for dt, do we
need to shorten the credit bits?

In dt-schema[1] we stated that this is a uint8 array, and dt is able
to generate this. It doesn't need to avoid using space for parameter
splitting.

For some device, asking for random number is time consuming. Shorten
the credit length makes it have to generate longer seed for dt to meet
the CRNG_INIT_CNT_THRESH threshold.

[1] https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L55

Thanks

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/3] random: add random.rng_seed to bootconfig entry
  2020-02-14 17:00   ` Mark Salyzyn
@ 2020-02-14 18:14     ` Rob Herring
  2020-02-14 18:31       ` Mark Salyzyn
  2020-02-15  0:17       ` Masami Hiramatsu
  0 siblings, 2 replies; 17+ messages in thread
From: Rob Herring @ 2020-02-14 18:14 UTC (permalink / raw)
  To: Mark Salyzyn
  Cc: Masami Hiramatsu, linux-kernel, Android Kernel Team,
	Theodore Ts'o, Arnd Bergmann, Greg Kroah-Hartman,
	Richard Henderson, Mark Brown, Kees Cook, Hsin-Yi Wang,
	Vasily Gorbik, Andrew Morton, Steven Rostedt, Mike Rapoport,
	Arvind Sankar, Dominik Brodowski, Thomas Gleixner,
	Alexander Potapenko, Jonathan Corbet, Mauro Carvalho Chehab,
	Josh Poimboeuf, Pawan Gupta, Juergen Gross,
	Linux Doc Mailing List

On Fri, Feb 14, 2020 at 11:00 AM Mark Salyzyn <salyzyn@android.com> wrote:
>
> On 2/14/20 5:49 AM, Rob Herring wrote:
> > On Fri, Feb 14, 2020 at 12:10 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >> Hi,
> >>
> >> The following series is bootconfig based implementation of
> >> the rng_seed option patch originally from Mark Salyzyn.
> >> Note that I removed unrelated command line fixes from this
> >> series.
> > Why do we need this? There's already multiple other ways to pass
> > random seed and this doesn't pass the "too complex for the command
> > line" argument you had for needing bootconfig.
> >
> > Rob
>
> Android is the use case I can vouch for. But also KVM.
>
> Android Cuttlefish is an emulated device used extensively in the testing
> and development infrastructure for In-house, partner, and system and
> application developers for Android. There is no bootloader, per-se.
> Because of the Android GKI distribution, there is also no rng virtual
> driver built in, it is loaded later as a module, too late for many
> aspects of KASLR and networking. There is no Device Tree, it does
> however have access to the content of the initrd image, and to the
> command line for the kernel. The only convenient way to get early
> entropy is going to have to be one of those two places.

I'm familiar with Cuttlefish somewhat. Guess who got virtio-gpu
working on Android[1]. :) I assume DT doesn't work for you because you
need x86 builds, but doesn't QEMU use UEFI in that case which also has
a mechanism for passing entropy.

To clarify my question: Why do we need random seed in bootconfig
rather than just the kernel command line? I'm not understanding why
things changed from your original patch.

> In addition, 2B Android devices on the planet, especially in light of
> the Android GKI distribution were everything that is vendor created is
> in a module, needs a way to collect early entropy prior to module load
> and pass it to the kernel. Yes, they do have access to the recently
> added Device Tree approach, and we expect them to use it, as I have an
> active backport for the mechanism into the Android 4.19 and 5.4 kernels.
> There may also be some benefit to allowing the 13000 different
> bootloaders an option to use bootconfig as a way of propagating the much
> needed entropy to their kernels. I could make a case to also allow them
> command line as another option to relieve their development stress to
> deliver product, but we can stop there. Regardless, this early entropy
> has the benefit of greatly improving security and precious boot time.

We're going to update 13000 bootloaders to understand bootconfig
rather than use the infrastructure already in place (DT and/or command
line)?

bootconfig is an ftrace feature only IMO. If it's more than that, I
imagine there will be some opinions about that. Adding new
bootloader-kernel interfaces is painful and not something to just add
without much review.

Rob

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/3] random: add random.rng_seed to bootconfig entry
  2020-02-14 18:14     ` Rob Herring
@ 2020-02-14 18:31       ` Mark Salyzyn
  2020-02-15  0:17       ` Masami Hiramatsu
  1 sibling, 0 replies; 17+ messages in thread
From: Mark Salyzyn @ 2020-02-14 18:31 UTC (permalink / raw)
  To: Rob Herring
  Cc: Masami Hiramatsu, linux-kernel, Android Kernel Team,
	Theodore Ts'o, Arnd Bergmann, Greg Kroah-Hartman,
	Richard Henderson, Mark Brown, Kees Cook, Hsin-Yi Wang,
	Vasily Gorbik, Andrew Morton, Steven Rostedt, Mike Rapoport,
	Arvind Sankar, Dominik Brodowski, Thomas Gleixner,
	Alexander Potapenko, Jonathan Corbet, Mauro Carvalho Chehab,
	Josh Poimboeuf, Pawan Gupta, Juergen Gross,
	Linux Doc Mailing List

On 2/14/20 10:14 AM, Rob Herring wrote:
> On Fri, Feb 14, 2020 at 11:00 AM Mark Salyzyn <salyzyn@android.com> wrote:
>> On 2/14/20 5:49 AM, Rob Herring wrote:
>>> On Fri, Feb 14, 2020 at 12:10 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>>>> Hi,
>>>>
>>>> The following series is bootconfig based implementation of
>>>> the rng_seed option patch originally from Mark Salyzyn.
>>>> Note that I removed unrelated command line fixes from this
>>>> series.
>>> Why do we need this? There's already multiple other ways to pass
>>> random seed and this doesn't pass the "too complex for the command
>>> line" argument you had for needing bootconfig.
>>>
>>> Rob
>> Android is the use case I can vouch for. But also KVM.
. . .
> I'm familiar with Cuttlefish somewhat. Guess who got virtio-gpu
> working on Android[1]. :) I assume DT doesn't work for you because you
> need x86 builds, but doesn't QEMU use UEFI in that case which also has
> a mechanism for passing entropy.
IDK, will have to ask the Cuttlefish Team why UEFI not being used, will 
get back to you.
>
> To clarify my question: Why do we need random seed in bootconfig
> rather than just the kernel command line? I'm not understanding why
> things changed from your original patch.

Command line was identified as the simplest for them to implement 
generically for the x86 and arm64 Cuttlefish instances and hence my 
original patch series.

However, it also is limited in the size of the entropy string that can 
be provided, so we flipped a coin and decided to accept the bootconfig 
mechanism as a viable alternative; that BTW appeared to be simpler to 
implement (mainly because rubbing out the entropy command line argument 
is not easy).

-- Mark


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] random: rng-seed source is utf-8
  2020-02-14  6:10 ` [PATCH 2/3] random: rng-seed source is utf-8 Masami Hiramatsu
  2020-02-14 18:14   ` Hsin-Yi Wang
@ 2020-02-14 19:58   ` Rob Herring
  2020-02-14 22:47     ` Theodore Y. Ts'o
  1 sibling, 1 reply; 17+ messages in thread
From: Rob Herring @ 2020-02-14 19:58 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: linux-kernel, Android Kernel Team, Mark Salyzyn,
	Theodore Ts'o, Arnd Bergmann, Greg Kroah-Hartman,
	Richard Henderson, Mark Brown, Kees Cook, Hsin-Yi Wang,
	Vasily Gorbik, Andrew Morton, Steven Rostedt, Mike Rapoport,
	Arvind Sankar, Dominik Brodowski, Thomas Gleixner,
	Alexander Potapenko, Jonathan Corbet, Mauro Carvalho Chehab,
	Josh Poimboeuf, Pawan Gupta, Juergen Gross,
	Linux Doc Mailing List

On Fri, Feb 14, 2020 at 12:10 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> From: Mark Salyzyn <salyzyn@android.com>
>
> commit 428826f5358c922dc378830a1717b682c0823160
> ("fdt: add support for rng-seed") makes the assumption that the data
> in rng-seed is binary, when it is typically constructed of utf-8

Typically? Why is that?

> characters which has a bitness of roughly 6 to give appropriate
> credit due for the entropy.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] random: rng-seed source is utf-8
  2020-02-14 19:58   ` Rob Herring
@ 2020-02-14 22:47     ` Theodore Y. Ts'o
  2020-02-14 22:55       ` Mark Salyzyn
  0 siblings, 1 reply; 17+ messages in thread
From: Theodore Y. Ts'o @ 2020-02-14 22:47 UTC (permalink / raw)
  To: Rob Herring
  Cc: Masami Hiramatsu, linux-kernel, Android Kernel Team,
	Mark Salyzyn, Arnd Bergmann, Greg Kroah-Hartman,
	Richard Henderson, Mark Brown, Kees Cook, Hsin-Yi Wang,
	Vasily Gorbik, Andrew Morton, Steven Rostedt, Mike Rapoport,
	Arvind Sankar, Dominik Brodowski, Thomas Gleixner,
	Alexander Potapenko, Jonathan Corbet, Mauro Carvalho Chehab,
	Josh Poimboeuf, Pawan Gupta, Juergen Gross,
	Linux Doc Mailing List

On Fri, Feb 14, 2020 at 01:58:35PM -0600, Rob Herring wrote:
> On Fri, Feb 14, 2020 at 12:10 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >
> > From: Mark Salyzyn <salyzyn@android.com>
> >
> > commit 428826f5358c922dc378830a1717b682c0823160
> > ("fdt: add support for rng-seed") makes the assumption that the data
> > in rng-seed is binary, when it is typically constructed of utf-8
> 
> Typically? Why is that?
> 
> > characters which has a bitness of roughly 6 to give appropriate
> > credit due for the entropy.

This is why I really think what gets specified via the boot command
line, or bootconfig, should specify the bits of entropy and the
entropy seed *separately*, so it can be specified explicitly, instead
of assuming that *everyone knows* that rng-seed is either (a) a binary
string, or (b) utf-8, or (c) a hex string.  The fact is, everyone does
*not* know, or everyone will have a different implementation, which
everyone will say is *obviously* the only way to go....

	      	     		     	  - Ted

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] random: rng-seed source is utf-8
  2020-02-14 22:47     ` Theodore Y. Ts'o
@ 2020-02-14 22:55       ` Mark Salyzyn
  2020-02-15  0:53         ` Theodore Y. Ts'o
  0 siblings, 1 reply; 17+ messages in thread
From: Mark Salyzyn @ 2020-02-14 22:55 UTC (permalink / raw)
  To: Theodore Y. Ts'o, Rob Herring
  Cc: Masami Hiramatsu, linux-kernel, Android Kernel Team,
	Arnd Bergmann, Greg Kroah-Hartman, Richard Henderson, Mark Brown,
	Kees Cook, Hsin-Yi Wang, Vasily Gorbik, Andrew Morton,
	Steven Rostedt, Mike Rapoport, Arvind Sankar, Dominik Brodowski,
	Thomas Gleixner, Alexander Potapenko, Jonathan Corbet,
	Mauro Carvalho Chehab, Josh Poimboeuf, Pawan Gupta,
	Juergen Gross, Linux Doc Mailing List

On 2/14/20 2:47 PM, Theodore Y. Ts'o wrote:
> On Fri, Feb 14, 2020 at 01:58:35PM -0600, Rob Herring wrote:
>> On Fri, Feb 14, 2020 at 12:10 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>>> From: Mark Salyzyn <salyzyn@android.com>
>>>
>>> commit 428826f5358c922dc378830a1717b682c0823160
>>> ("fdt: add support for rng-seed") makes the assumption that the data
>>> in rng-seed is binary, when it is typically constructed of utf-8
>> Typically? Why is that?
>>
>>> characters which has a bitness of roughly 6 to give appropriate
>>> credit due for the entropy.
> This is why I really think what gets specified via the boot command
> line, or bootconfig, should specify the bits of entropy and the
> entropy seed *separately*, so it can be specified explicitly, instead
> of assuming that *everyone knows* that rng-seed is either (a) a binary
> string, or (b) utf-8, or (c) a hex string.  The fact is, everyone does
> *not* know, or everyone will have a different implementation, which
> everyone will say is *obviously* the only way to go....
>
> 	      	     		     	  - Ted

Given that the valid option are between 4 (hex), 6 (utf-8) or 8 
(binary), we can either split the difference and accept 6; or take a 
pass at the values and determine which of the set they belong to 
[0-9a-fA-F], [!-~] or [\000-\377]  nor need to separately specify.

-- Mark


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/3] random: add random.rng_seed to bootconfig entry
  2020-02-14 18:14     ` Rob Herring
  2020-02-14 18:31       ` Mark Salyzyn
@ 2020-02-15  0:17       ` Masami Hiramatsu
  1 sibling, 0 replies; 17+ messages in thread
From: Masami Hiramatsu @ 2020-02-15  0:17 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Salyzyn, Masami Hiramatsu, linux-kernel,
	Android Kernel Team, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Richard Henderson, Mark Brown, Kees Cook,
	Hsin-Yi Wang, Vasily Gorbik, Andrew Morton, Steven Rostedt,
	Mike Rapoport, Arvind Sankar, Dominik Brodowski, Thomas Gleixner,
	Alexander Potapenko, Jonathan Corbet, Mauro Carvalho Chehab,
	Josh Poimboeuf, Pawan Gupta, Juergen Gross,
	Linux Doc Mailing List

Hi Rob,

On Fri, 14 Feb 2020 12:14:53 -0600
Rob Herring <robh@kernel.org> wrote:

> To clarify my question: Why do we need random seed in bootconfig
> rather than just the kernel command line? I'm not understanding why
> things changed from your original patch.

I recommended to use it in the previous thread, because of simplicity.
Since it has to hide from userspace and modules, it needs to modify
kernel command line. But the bootconfig can make it simple, and it
also architecture independent.

> > In addition, 2B Android devices on the planet, especially in light of
> > the Android GKI distribution were everything that is vendor created is
> > in a module, needs a way to collect early entropy prior to module load
> > and pass it to the kernel. Yes, they do have access to the recently
> > added Device Tree approach, and we expect them to use it, as I have an
> > active backport for the mechanism into the Android 4.19 and 5.4 kernels.

FYI, I backported bootconfig with boot-time tracer for 4.19 stable kernel
recently.

https://github.com/mhiramat/linux/commits/ftrace-boottrace-4.19

You can check what commits are related.

> > There may also be some benefit to allowing the 13000 different
> > bootloaders an option to use bootconfig as a way of propagating the much
> > needed entropy to their kernels. I could make a case to also allow them
> > command line as another option to relieve their development stress to
> > deliver product, but we can stop there. Regardless, this early entropy
> > has the benefit of greatly improving security and precious boot time.
> 
> We're going to update 13000 bootloaders to understand bootconfig
> rather than use the infrastructure already in place (DT and/or command
> line)?
> 
> bootconfig is an ftrace feature only IMO. If it's more than that, I
> imagine there will be some opinions about that. Adding new
> bootloader-kernel interfaces is painful and not something to just add
> without much review.

The bootconfig itself is designed as a generic feature. I had tried to use
devicetree, but that was rejected. Thus I made it as a "software
configuration tree" (but far simpler.)
 
If you have any review comment on the bootconfig, always welcome!
Seriously, I would like to have more comments. I want to make it better.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] random: rng-seed source is utf-8
  2020-02-14 22:55       ` Mark Salyzyn
@ 2020-02-15  0:53         ` Theodore Y. Ts'o
  2020-02-18 16:01           ` Mark Salyzyn
  0 siblings, 1 reply; 17+ messages in thread
From: Theodore Y. Ts'o @ 2020-02-15  0:53 UTC (permalink / raw)
  To: Mark Salyzyn
  Cc: Rob Herring, Masami Hiramatsu, linux-kernel, Android Kernel Team,
	Arnd Bergmann, Greg Kroah-Hartman, Richard Henderson, Mark Brown,
	Kees Cook, Hsin-Yi Wang, Vasily Gorbik, Andrew Morton,
	Steven Rostedt, Mike Rapoport, Arvind Sankar, Dominik Brodowski,
	Thomas Gleixner, Alexander Potapenko, Jonathan Corbet,
	Mauro Carvalho Chehab, Josh Poimboeuf, Pawan Gupta,
	Juergen Gross, Linux Doc Mailing List

On Fri, Feb 14, 2020 at 02:55:36PM -0800, Mark Salyzyn wrote:
> > This is why I really think what gets specified via the boot command
> > line, or bootconfig, should specify the bits of entropy and the
> > entropy seed *separately*, so it can be specified explicitly, instead
> > of assuming that *everyone knows* that rng-seed is either (a) a binary
> > string, or (b) utf-8, or (c) a hex string.  The fact is, everyone does
> > *not* know, or everyone will have a different implementation, which
> > everyone will say is *obviously* the only way to go....
> > 
> Given that the valid option are between 4 (hex), 6 (utf-8) or 8 (binary), we
> can either split the difference and accept 6; or take a pass at the values
> and determine which of the set they belong to [0-9a-fA-F], [!-~] or
> [\000-\377]  nor need to separately specify.

So let's split this up into separate issues.  First of all, from an
architectural issue, I really think we need to change
add_bootloader_randomness() in drivers/char/random.c so it looks like this:

void add_bootloader_randomness(const void *buf, unsigned int size, unsigned int entropy_bits)

That's because this is a general function that could be used by any
number of bootloaders.  For example, for the UEFI bootloader, it can
use the UEFI call that will return binary bits.  Some other bootloader
might use utf-8, etc.  So it would be an abstraction violation to have
code in drivers/char/random.c make assumption about how a particular
bootloader might be behaving.

The second question is we are going to be parsing an rng_seed
parameter it shows up in bootconfig or in the boot command line, how
do we decide how many bits of entropy it actually has.  The advantage
of using the boot command line is we don't need to change the rest of
the bootloader ecosystem.  But that's also a massive weakness, since
apparently some people are already using it, and perhaps not in the
same way.

So what I'd really prefer is if we use something new, and we define it
in a way that makes as close as possible to "impossible to misuse".
(See Rusty Russell's API design manifesto[1]).  So I'm going to
propose something different.  Let's use something new, say
entropy_seed_hex, and let's say that it *must* be a hex string:

    entropy_seed_hex=7337db91a4824e3480ba6d2dfaa60bec

If it is not a valid hex string, it gets zero entropy credit.

I don't think we really need to worry about efficient encoding of the
seed, since 256 bits is only 64 characters using a hex string.  And
whether it's 32 characters or 64 characters, the max command line
string is 32k, so it's probably not worth it to try to do something
more complex.  (And only 128 bits is needed to declare the CRNG to be
fully initialized, in which case we're talking about 16 characters
versus 32 charaters.)

[1] http://sweng.the-davies.net/Home/rustys-api-design-manifesto

						- Ted


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] random: rng-seed source is utf-8
  2020-02-15  0:53         ` Theodore Y. Ts'o
@ 2020-02-18 16:01           ` Mark Salyzyn
  2020-02-18 16:52             ` Hsin-Yi Wang
  2020-02-18 17:14             ` Theodore Y. Ts'o
  0 siblings, 2 replies; 17+ messages in thread
From: Mark Salyzyn @ 2020-02-18 16:01 UTC (permalink / raw)
  To: Theodore Y. Ts'o
  Cc: Rob Herring, Masami Hiramatsu, linux-kernel, Android Kernel Team,
	Arnd Bergmann, Greg Kroah-Hartman, Richard Henderson, Mark Brown,
	Kees Cook, Hsin-Yi Wang, Vasily Gorbik, Andrew Morton,
	Steven Rostedt, Mike Rapoport, Arvind Sankar, Dominik Brodowski,
	Thomas Gleixner, Alexander Potapenko, Jonathan Corbet,
	Mauro Carvalho Chehab, Josh Poimboeuf, Pawan Gupta,
	Juergen Gross, Linux Doc Mailing List

On 2/14/20 4:53 PM, Theodore Y. Ts'o wrote:
> On Fri, Feb 14, 2020 at 02:55:36PM -0800, Mark Salyzyn wrote:
>>> This is why I really think what gets specified via the boot command
>>> line, or bootconfig, should specify the bits of entropy and the
>>> entropy seed *separately*, so it can be specified explicitly, instead
>>> of assuming that *everyone knows* that rng-seed is either (a) a binary
>>> string, or (b) utf-8, or (c) a hex string.  The fact is, everyone does
>>> *not* know, or everyone will have a different implementation, which
>>> everyone will say is *obviously* the only way to go....
>>>
>> Given that the valid option are between 4 (hex), 6 (utf-8) or 8 (binary), we
>> can either split the difference and accept 6; or take a pass at the values
>> and determine which of the set they belong to [0-9a-fA-F], [!-~] or
>> [\000-\377]  nor need to separately specify.
> So let's split this up into separate issues.  First of all, from an
> architectural issue, I really think we need to change
> add_bootloader_randomness() in drivers/char/random.c so it looks like this:
>
> void add_bootloader_randomness(const void *buf, unsigned int size, unsigned int entropy_bits)
>
> That's because this is a general function that could be used by any
> number of bootloaders.  For example, for the UEFI bootloader, it can
> use the UEFI call that will return binary bits.  Some other bootloader
> might use utf-8, etc.  So it would be an abstraction violation to have
> code in drivers/char/random.c make assumption about how a particular
> bootloader might be behaving.
>
> The second question is we are going to be parsing an rng_seed
> parameter it shows up in bootconfig or in the boot command line, how
> do we decide how many bits of entropy it actually has.  The advantage
> of using the boot command line is we don't need to change the rest of
> the bootloader ecosystem.  But that's also a massive weakness, since
> apparently some people are already using it, and perhaps not in the
> same way.
>
> So what I'd really prefer is if we use something new, and we define it
> in a way that makes as close as possible to "impossible to misuse".
> (See Rusty Russell's API design manifesto[1]).  So I'm going to
> propose something different.  Let's use something new, say
> entropy_seed_hex, and let's say that it *must* be a hex string:
>
>      entropy_seed_hex=7337db91a4824e3480ba6d2dfaa60bec
>
> If it is not a valid hex string, it gets zero entropy credit.
>
> I don't think we really need to worry about efficient encoding of the
> seed, since 256 bits is only 64 characters using a hex string.  An
> whether it's 32 characters or 64 characters, the max command line
> string is 32k, so it's probably not worth it to try to do something
> more complex.  (And only 128 bits is needed to declare the CRNG to be
> fully initialized, in which case we're talking about 16 characters
> versus 32 charaters.)
>
> [1] http://sweng.the-davies.net/Home/rustys-api-design-manifesto
>
> 						- Ted
>
I am additionally concerned about add_bootloader_randomness() because it 
is possible for it to sleep because of add_hwgenerator_randomness() as 
once it hits the entropy threshold. As-is it can not be used inside 
start_kernel() because the sleep would result in a kernel panic, and I 
suspect its use inside early_init_dt_scan_chosen() for the commit "fdt: 
add support for rng-seed" might also be problematic since it is 
effectively called underneath start_kernel() is it not?

If add_bootloader_randomness was rewritten to call 
add_device_randomness() always, and when trusted also called the 
functionality of the new credit_trusted_entropy_bits (no longer needing 
to be exported if so), then the function could be used in both 
start_kernel() and early_init_dt_scan_chosen().


-- Mark


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] random: rng-seed source is utf-8
  2020-02-18 16:01           ` Mark Salyzyn
@ 2020-02-18 16:52             ` Hsin-Yi Wang
  2020-02-18 17:14             ` Theodore Y. Ts'o
  1 sibling, 0 replies; 17+ messages in thread
From: Hsin-Yi Wang @ 2020-02-18 16:52 UTC (permalink / raw)
  To: Mark Salyzyn
  Cc: Theodore Y. Ts'o, Rob Herring, Masami Hiramatsu,
	linux-kernel, Android Kernel Team, Arnd Bergmann,
	Greg Kroah-Hartman, Richard Henderson, Mark Brown, Kees Cook,
	Vasily Gorbik, Andrew Morton, Steven Rostedt, Mike Rapoport,
	Arvind Sankar, Dominik Brodowski, Thomas Gleixner,
	Alexander Potapenko, Jonathan Corbet, Mauro Carvalho Chehab,
	Josh Poimboeuf, Pawan Gupta, Juergen Gross,
	Linux Doc Mailing List

On Wed, Feb 19, 2020 at 12:01 AM Mark Salyzyn <salyzyn@android.com> wrote:
>
> On 2/14/20 4:53 PM, Theodore Y. Ts'o wrote:
> > On Fri, Feb 14, 2020 at 02:55:36PM -0800, Mark Salyzyn wrote:
> >>> This is why I really think what gets specified via the boot command
> >>> line, or bootconfig, should specify the bits of entropy and the
> >>> entropy seed *separately*, so it can be specified explicitly, instead
> >>> of assuming that *everyone knows* that rng-seed is either (a) a binary
> >>> string, or (b) utf-8, or (c) a hex string.  The fact is, everyone does
> >>> *not* know, or everyone will have a different implementation, which
> >>> everyone will say is *obviously* the only way to go....
> >>>
> >> Given that the valid option are between 4 (hex), 6 (utf-8) or 8 (binary), we
> >> can either split the difference and accept 6; or take a pass at the values
> >> and determine which of the set they belong to [0-9a-fA-F], [!-~] or
> >> [\000-\377]  nor need to separately specify.
> > So let's split this up into separate issues.  First of all, from an
> > architectural issue, I really think we need to change
> > add_bootloader_randomness() in drivers/char/random.c so it looks like this:
> >
> > void add_bootloader_randomness(const void *buf, unsigned int size, unsigned int entropy_bits)
> >
> > That's because this is a general function that could be used by any
> > number of bootloaders.  For example, for the UEFI bootloader, it can
> > use the UEFI call that will return binary bits.  Some other bootloader
> > might use utf-8, etc.  So it would be an abstraction violation to have
> > code in drivers/char/random.c make assumption about how a particular
> > bootloader might be behaving.
> >
> > The second question is we are going to be parsing an rng_seed
> > parameter it shows up in bootconfig or in the boot command line, how
> > do we decide how many bits of entropy it actually has.  The advantage
> > of using the boot command line is we don't need to change the rest of
> > the bootloader ecosystem.  But that's also a massive weakness, since
> > apparently some people are already using it, and perhaps not in the
> > same way.
> >
> > So what I'd really prefer is if we use something new, and we define it
> > in a way that makes as close as possible to "impossible to misuse".
> > (See Rusty Russell's API design manifesto[1]).  So I'm going to
> > propose something different.  Let's use something new, say
> > entropy_seed_hex, and let's say that it *must* be a hex string:
> >
> >      entropy_seed_hex=7337db91a4824e3480ba6d2dfaa60bec
> >
> > If it is not a valid hex string, it gets zero entropy credit.
> >
> > I don't think we really need to worry about efficient encoding of the
> > seed, since 256 bits is only 64 characters using a hex string.  An
> > whether it's 32 characters or 64 characters, the max command line
> > string is 32k, so it's probably not worth it to try to do something
> > more complex.  (And only 128 bits is needed to declare the CRNG to be
> > fully initialized, in which case we're talking about 16 characters
> > versus 32 charaters.)
> >
> > [1] http://sweng.the-davies.net/Home/rustys-api-design-manifesto
> >
> >                                               - Ted
> >
> I am additionally concerned about add_bootloader_randomness() because it
> is possible for it to sleep because of add_hwgenerator_randomness() as
> once it hits the entropy threshold. As-is it can not be used inside
> start_kernel() because the sleep would result in a kernel panic, and I
> suspect its use inside early_init_dt_scan_chosen() for the commit "fdt:
> add support for rng-seed" might also be problematic since it is
> effectively called underneath start_kernel() is it not?
>
> If add_bootloader_randomness was rewritten to call
> add_device_randomness() always, and when trusted also called the
> functionality of the new credit_trusted_entropy_bits (no longer needing
> to be exported if so), then the function could be used in both
> start_kernel() and early_init_dt_scan_chosen().
>
I tested 64 bytes rng-seed previously so didn't hit the threshold that
makes it suspend. Thanks for pointing this out.
+1 for changing the add_bootloader_randomness() function as you
suggested to avoid this issue. But besides credit_entropy_bits(), they
are also different on crng_init (crng_fast_load/crng_slow_load).

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] random: rng-seed source is utf-8
  2020-02-18 16:01           ` Mark Salyzyn
  2020-02-18 16:52             ` Hsin-Yi Wang
@ 2020-02-18 17:14             ` Theodore Y. Ts'o
  1 sibling, 0 replies; 17+ messages in thread
From: Theodore Y. Ts'o @ 2020-02-18 17:14 UTC (permalink / raw)
  To: Mark Salyzyn
  Cc: Rob Herring, Masami Hiramatsu, linux-kernel, Android Kernel Team,
	Arnd Bergmann, Greg Kroah-Hartman, Richard Henderson, Mark Brown,
	Kees Cook, Hsin-Yi Wang, Vasily Gorbik, Andrew Morton,
	Steven Rostedt, Mike Rapoport, Arvind Sankar, Dominik Brodowski,
	Thomas Gleixner, Alexander Potapenko, Jonathan Corbet,
	Mauro Carvalho Chehab, Josh Poimboeuf, Pawan Gupta,
	Juergen Gross, Linux Doc Mailing List

On Tue, Feb 18, 2020 at 08:01:51AM -0800, Mark Salyzyn wrote:
> I am additionally concerned about add_bootloader_randomness() because it is
> possible for it to sleep because of add_hwgenerator_randomness() as once it
> hits the entropy threshold. As-is it can not be used inside start_kernel()
> because the sleep would result in a kernel panic, and I suspect its use
> inside early_init_dt_scan_chosen() for the commit "fdt: add support for
> rng-seed" might also be problematic since it is effectively called
> underneath start_kernel() is it not?
> 
> If add_bootloader_randomness was rewritten to call add_device_randomness()
> always, and when trusted also called the functionality of the new
> credit_trusted_entropy_bits (no longer needing to be exported if so), then
> the function could be used in both start_kernel() and
> early_init_dt_scan_chosen().

That's a good point, and it's a bug in add_bootloader_randomness().
That should be easily fixed by simply having it call mix_pool_bytes()
and credit_entropy_bits() directly.  I'll create a patch...

    			  	     	  	   - Ted

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, back to index

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-14  6:10 [PATCH 0/3] random: add random.rng_seed to bootconfig entry Masami Hiramatsu
2020-02-14  6:10 ` [PATCH 1/3] bootconfig: Support non-ascii characters in value Masami Hiramatsu
2020-02-14  6:10 ` [PATCH 2/3] random: rng-seed source is utf-8 Masami Hiramatsu
2020-02-14 18:14   ` Hsin-Yi Wang
2020-02-14 19:58   ` Rob Herring
2020-02-14 22:47     ` Theodore Y. Ts'o
2020-02-14 22:55       ` Mark Salyzyn
2020-02-15  0:53         ` Theodore Y. Ts'o
2020-02-18 16:01           ` Mark Salyzyn
2020-02-18 16:52             ` Hsin-Yi Wang
2020-02-18 17:14             ` Theodore Y. Ts'o
2020-02-14  6:10 ` [PATCH 3/3] random: add random.rng_seed= bootconfig option Masami Hiramatsu
2020-02-14 13:49 ` [PATCH 0/3] random: add random.rng_seed to bootconfig entry Rob Herring
2020-02-14 17:00   ` Mark Salyzyn
2020-02-14 18:14     ` Rob Herring
2020-02-14 18:31       ` Mark Salyzyn
2020-02-15  0:17       ` Masami Hiramatsu

Linux-Doc Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-doc/0 linux-doc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-doc linux-doc/ https://lore.kernel.org/linux-doc \
		linux-doc@vger.kernel.org
	public-inbox-index linux-doc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-doc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git