| Message ID | 20190819071602.139014-3-hsinyi@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add support for rng-seed | expand |
On Mon, Aug 19, 2019 at 03:16:04PM +0800, Hsin-Yi Wang wrote: > Introducing a chosen node, rng-seed, which is an entropy that can be > passed to kernel called very early to increase initial device > randomness. Bootloader should provide this entropy and the value is > read from /chosen/rng-seed in DT. So it's really cool that you've sent out this patch set. I've been wanting this for all platforms / architectures for quite a while. Question --- are you willing to guarantee that the booloader can be trusted enough that you *know* the entropy being provided by the bootloader to be secure? If so, we could let fdt.c use a different interface, perhaps add_hwgenerator_randomness(), which allows the bootloader to transfer trusted entropy for the purposes of initializing the crng and entropy accounting for /dev/random. One of the questions is how do we make sure the boot loader is actually secure, but given that we have to trust the boot loader for various trusted boot use cases, it seems reasonable to do that. What do you think? - Ted
Hi Ted, Thanks for raising this question. For UEFI based system, they have a config table that carries rng seed and can be passed to device randomness. However, they also use add_device_randomness (not sure if it's the same reason that they can't guarantee _all_ bootloader can be trusted) This patch is to let DT based system also have similar features, which can make initial random number stronger. (We only care initial situation here, since more entropy would be added to kernel as time goes on ) Conservatively, we can use add_device_randomness() as well, which would pass buffer to crng_slow_load() instead of crng_fast_load(). But I think we should trust bootloader here. Whoever wants to use this feature should make sure their bootloader can pass valid (random enough) seeds. If they are not sure, they can just don't add the property to DT.
On Tue, 20 Aug 2019 at 10:43, Hsin-Yi Wang <hsinyi@chromium.org> wrote: > > Hi Ted, > > Thanks for raising this question. > > For UEFI based system, they have a config table that carries rng seed > and can be passed to device randomness. However, they also use > add_device_randomness (not sure if it's the same reason that they > can't guarantee _all_ bootloader can be trusted) The config table is actually a Linux invention: it is populated by the EFI stub code (which is part of the kernel) based on the output of a call into the EFI_RNG_PROTOCOL, which is defined in the UEFI spec, but optional and not widely available. I have opted for add_device_randomness() since there is no way to establish the quality level of the output of EFI_RNG_PROTOCOL, and so it is currently only used to prevent the bootup state of the entropy pool to be too predictable, and the output does not contribute to the entropy estimate kept by the RNG core. > This patch is to let DT based system also have similar features, which > can make initial random number stronger. (We only care initial > situation here, since more entropy would be added to kernel as time > goes on ) > > Conservatively, we can use add_device_randomness() as well, which > would pass buffer to crng_slow_load() instead of crng_fast_load(). > But I think we should trust bootloader here. Whoever wants to use this > feature should make sure their bootloader can pass valid (random > enough) seeds. If they are not sure, they can just don't add the > property to DT. It is the firmware that adds the property to the DT, not the user.
Then we'd still use add_device_randomness() in case that bootloader provides weak entropy. On Tue, Aug 20, 2019 at 7:14 PM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > On Tue, 20 Aug 2019 at 10:43, Hsin-Yi Wang <hsinyi@chromium.org> wrote: > > > > Hi Ted, > > > > Thanks for raising this question. > > > > For UEFI based system, they have a config table that carries rng seed > > and can be passed to device randomness. However, they also use > > add_device_randomness (not sure if it's the same reason that they > > can't guarantee _all_ bootloader can be trusted) > > The config table is actually a Linux invention: it is populated by the > EFI stub code (which is part of the kernel) based on the output of a > call into the EFI_RNG_PROTOCOL, which is defined in the UEFI spec, but > optional and not widely available. > > I have opted for add_device_randomness() since there is no way to > establish the quality level of the output of EFI_RNG_PROTOCOL, and so > it is currently only used to prevent the bootup state of the entropy > pool to be too predictable, and the output does not contribute to the > entropy estimate kept by the RNG core. > > > > This patch is to let DT based system also have similar features, which > > can make initial random number stronger. (We only care initial > > situation here, since more entropy would be added to kernel as time > > goes on ) > > > > Conservatively, we can use add_device_randomness() as well, which > > would pass buffer to crng_slow_load() instead of crng_fast_load(). > > But I think we should trust bootloader here. Whoever wants to use this > > feature should make sure their bootloader can pass valid (random > > enough) seeds. If they are not sure, they can just don't add the > > property to DT. > > It is the firmware that adds the property to the DT, not the user.
On Wed, 21 Aug 2019 at 08:57, Hsin-Yi Wang <hsinyi@chromium.org> wrote: > > Then we'd still use add_device_randomness() in case that bootloader > provides weak entropy. > (please don't top post) Whether to trust the firmware provided entropy is a policy decision, and typically, we try to avoid dictating policy in the kernel, and instead, we try to provide a sane default but give the user control over it. So in this case, we should probably introduce add_firmware_randomness() with a Kconfig/cmdline option pair to decide whether it should be trusted or not (or reuse the one we have for trusting RDRAND etc) > On Tue, Aug 20, 2019 at 7:14 PM Ard Biesheuvel > <ard.biesheuvel@linaro.org> wrote: > > > > On Tue, 20 Aug 2019 at 10:43, Hsin-Yi Wang <hsinyi@chromium.org> wrote: > > > > > > Hi Ted, > > > > > > Thanks for raising this question. > > > > > > For UEFI based system, they have a config table that carries rng seed > > > and can be passed to device randomness. However, they also use > > > add_device_randomness (not sure if it's the same reason that they > > > can't guarantee _all_ bootloader can be trusted) > > > > The config table is actually a Linux invention: it is populated by the > > EFI stub code (which is part of the kernel) based on the output of a > > call into the EFI_RNG_PROTOCOL, which is defined in the UEFI spec, but > > optional and not widely available. > > > > I have opted for add_device_randomness() since there is no way to > > establish the quality level of the output of EFI_RNG_PROTOCOL, and so > > it is currently only used to prevent the bootup state of the entropy > > pool to be too predictable, and the output does not contribute to the > > entropy estimate kept by the RNG core. > > > > > > > This patch is to let DT based system also have similar features, which > > > can make initial random number stronger. (We only care initial > > > situation here, since more entropy would be added to kernel as time > > > goes on ) > > > > > > Conservatively, we can use add_device_randomness() as well, which > > > would pass buffer to crng_slow_load() instead of crng_fast_load(). > > > But I think we should trust bootloader here. Whoever wants to use this > > > feature should make sure their bootloader can pass valid (random > > > enough) seeds. If they are not sure, they can just don't add the > > > property to DT. > > > > It is the firmware that adds the property to the DT, not the user.
On Wed, Aug 21, 2019 at 09:39:28AM +0300, Ard Biesheuvel wrote: > > Whether to trust the firmware provided entropy is a policy decision, > and typically, we try to avoid dictating policy in the kernel, and > instead, we try to provide a sane default but give the user control > over it. > > So in this case, we should probably introduce > add_firmware_randomness() with a Kconfig/cmdline option pair to decide > whether it should be trusted or not (or reuse the one we have for > trusting RDRAND etc) I'd call it add_bootloader_randomness(), since we are trusting the *bootloader*; it's the bootloader which is vouching for the security / validity of the passed-in entropy. Furthermore, the bootloader on some architectures might be fetching directly from some secure element. And for that reason, I'd use a different Kconfig/cmdline option pair than the one used for trusting CPU-provided randomness. - Ted
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index 9cdf14b9aaab..97a75996993c 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -24,6 +24,7 @@ #include <linux/debugfs.h> #include <linux/serial_core.h> #include <linux/sysfs.h> +#include <linux/random.h> #include <asm/setup.h> /* for COMMAND_LINE_SIZE */ #include <asm/page.h> @@ -1044,6 +1045,7 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname, { int l; const char *p; + const void *rng_seed; pr_debug("search \"chosen\", depth: %d, uname: %s\n", depth, uname); @@ -1078,6 +1080,14 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname, pr_debug("Command line is: %s\n", (char*)data); + rng_seed = of_get_flat_dt_prop(node, "rng-seed", &l); + if (rng_seed && l > 0) { + add_device_randomness(rng_seed, l); + + /* try to clear seed so it won't be found. */ + fdt_nop_property(initial_boot_params, node, "rng-seed"); + } + /* break now */ return 1; } @@ -1166,8 +1176,6 @@ bool __init early_init_dt_verify(void *params) /* Setup flat device-tree pointer */ initial_boot_params = params; - of_fdt_crc32 = crc32_be(~0, initial_boot_params, - fdt_totalsize(initial_boot_params)); return true; } @@ -1197,6 +1205,8 @@ bool __init early_init_dt_scan(void *params) return false; early_init_dt_scan_nodes(); + of_fdt_crc32 = crc32_be(~0, initial_boot_params, + fdt_totalsize(initial_boot_params)); return true; }