On July 1, 2022 10:58:08 AM PDT, "Jason A. Donenfeld" <[email protected]> wrote:
>Currently the only way x86 can get an early boot RNG seed is via EFI,
>which is generally always used now for physical machines, but is very
>rarely used in VMs, especially VMs that are optimized for starting
>"instantaneously", such as Firecracker's MicroVM. For tiny fast booting
>VMs, EFI is not something you generally need or want.
>
>Rather, here we want the ability for the image loader or firmware to
>pass a single random seed, exactly as device tree platforms do with the
>"rng-seed" property. Additionally, this is something that bootloaders
>can append, with their own seed file management, which is something
>every other major OS ecosystem has that we do not (yet).
>
>This patch adds SETUP_RNG_SEED, similar to the other seven SETUP_*
>entries that are parsed from the e820 setup table. It also takes care to
>zero out the seed immediately after using, in order to retain forward
>secrecy. This all takes about 7 trivial lines of code.
>
>Then, on kexec_file_load(), a new fresh seed is generated and passed to
>the next kernel, just as is done on device tree architectures when
>using kexec. And, importantly, I've tested that QEMU is able to properly
>pass SETUP_RNG_SEED as well, making this work for every step of the way.
>This code too is pretty straight forward.
>
>Together these measures ensure that VMs and nested kexec()'d kernels
>always are getting a proper boot time RNG seed at the earliest possible
>stage from their parents:
>
> - Host [already has strongly initialized RNG]
> - QEMU [passes fresh seed in e820 SETUP_RNG_SEED field]
> - Linux [uses parent's seed and gathers entropy of its own]
> - kexec [passes this in e820 SETUP_RNG_SEED field]
> - Linux [uses parent's seed and gathers entropy of its own]
> - kexec [passes this in e820 SETUP_RNG_SEED field]
> - Linux [uses parent's seed and gathers entropy of its own]
> - kexec [passes this in e820 SETUP_RNG_SEED field]
> - ...
>
>I've verified in several scenarios that this works quite well from a
>host kernel to QEMU and down inwards, mixing and matching loaders, with
>every layer providing a seed to the next.
>
>Signed-off-by: Jason A. Donenfeld <[email protected]>
>---
>Another day, another patch revision. If you haven't noticed, I'm quite
>determined to make this work well and robustly. At some point I'd of
>course appreciate some review. Either way, I've got every intention of
>plugging away at this until it's perfect.
>
>Changes v4->v5:
>- Populate field when loading bzimages for kexec, just like device tree
> platforms do.
>Changes v3->v4:
>- Zero out data after using, for forward secrecy.
>Changes v2->v3:
>- Actually memmap the right area with the random bytes in it. This
> worked before because of page sizes, but the code wasn't right. Now
> it's right.
>Changes v1->v2:
>- Fix small typo of data_len -> data->len.
>
> arch/x86/include/uapi/asm/bootparam.h | 1 +
> arch/x86/kernel/kexec-bzimage64.c | 36 ++++++++++++++++++++++++---
> arch/x86/kernel/setup.c | 8 ++++++
> 3 files changed, 42 insertions(+), 3 deletions(-)
>
>diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
>index bea5cdcdf532..a60676b8d1d4 100644
>--- a/arch/x86/include/uapi/asm/bootparam.h
>+++ b/arch/x86/include/uapi/asm/bootparam.h
>@@ -11,6 +11,7 @@
> #define SETUP_APPLE_PROPERTIES 5
> #define SETUP_JAILHOUSE 6
> #define SETUP_CC_BLOB 7
>+#define SETUP_RNG_SEED 8
>
> #define SETUP_INDIRECT (1<<31)
>
>diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
>index 170d0fd68b1f..13b2c55ebbf0 100644
>--- a/arch/x86/kernel/kexec-bzimage64.c
>+++ b/arch/x86/kernel/kexec-bzimage64.c
>@@ -18,6 +18,7 @@
> #include <linux/mm.h>
> #include <linux/efi.h>
> #include <linux/verification.h>
>+#include <linux/random.h>
>
> #include <asm/bootparam.h>
> #include <asm/setup.h>
>@@ -110,6 +111,27 @@ static int setup_e820_entries(struct boot_params *params)
> return 0;
> }
>
>+enum { RNG_SEED_LENGTH = 32 };
>+
>+static void
>+add_rng_seed_setup_data(struct boot_params *params,
>+ unsigned long params_load_addr,
>+ unsigned int rng_seed_setup_data_offset)
>+{
>+ struct setup_data *sd = (void *)params + rng_seed_setup_data_offset;
>+ unsigned long setup_data_phys;
>+
>+ if (!rng_is_initialized())
>+ return;
>+
>+ sd->type = SETUP_RNG_SEED;
>+ sd->len = RNG_SEED_LENGTH;
>+ get_random_bytes(sd->data, RNG_SEED_LENGTH);
>+ setup_data_phys = params_load_addr + rng_seed_setup_data_offset;
>+ sd->next = params->hdr.setup_data;
>+ params->hdr.setup_data = setup_data_phys;
>+}
>+
> #ifdef CONFIG_EFI
> static int setup_efi_info_memmap(struct boot_params *params,
> unsigned long params_load_addr,
>@@ -190,7 +212,8 @@ static int
> setup_boot_parameters(struct kimage *image, struct boot_params *params,
> unsigned long params_load_addr,
> unsigned int efi_map_offset, unsigned int efi_map_sz,
>- unsigned int efi_setup_data_offset)
>+ unsigned int efi_setup_data_offset,
>+ unsigned int rng_seed_setup_data_offset)
> {
> unsigned int nr_e820_entries;
> unsigned long long mem_k, start, end;
>@@ -242,6 +265,8 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
> }
> }
>
>+ add_rng_seed_setup_data(params, params_load_addr,
>+ rng_seed_setup_data_offset);
> #ifdef CONFIG_EFI
> /* Setup EFI state */
> setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
>@@ -337,6 +362,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> void *stack;
> unsigned int setup_hdr_offset = offsetof(struct boot_params, hdr);
> unsigned int efi_map_offset, efi_map_sz, efi_setup_data_offset;
>+ unsigned int rng_seed_setup_data_offset;
> struct kexec_buf kbuf = { .image = image, .buf_max = ULONG_MAX,
> .top_down = true };
> struct kexec_buf pbuf = { .image = image, .buf_min = MIN_PURGATORY_ADDR,
>@@ -401,13 +427,16 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> params_cmdline_sz = ALIGN(params_cmdline_sz, 16);
> kbuf.bufsz = params_cmdline_sz + ALIGN(efi_map_sz, 16) +
> sizeof(struct setup_data) +
>- sizeof(struct efi_setup_data);
>+ sizeof(struct efi_setup_data) +
>+ sizeof(struct setup_data) +
>+ RNG_SEED_LENGTH;
>
> params = kzalloc(kbuf.bufsz, GFP_KERNEL);
> if (!params)
> return ERR_PTR(-ENOMEM);
> efi_map_offset = params_cmdline_sz;
> efi_setup_data_offset = efi_map_offset + ALIGN(efi_map_sz, 16);
>+ rng_seed_setup_data_offset = efi_setup_data_offset + sizeof(struct efi_setup_data);
>
> /* Copy setup header onto bootparams. Documentation/x86/boot.rst */
> setup_header_size = 0x0202 + kernel[0x0201] - setup_hdr_offset;
>@@ -490,7 +519,8 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
>
> ret = setup_boot_parameters(image, params, bootparam_load_addr,
> efi_map_offset, efi_map_sz,
>- efi_setup_data_offset);
>+ efi_setup_data_offset,
>+ rng_seed_setup_data_offset);
> if (ret)
> goto out_free_params;
>
>diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
>index bd6c6fd373ae..6c807a4ae141 100644
>--- a/arch/x86/kernel/setup.c
>+++ b/arch/x86/kernel/setup.c
>@@ -23,6 +23,7 @@
> #include <linux/usb/xhci-dbgp.h>
> #include <linux/static_call.h>
> #include <linux/swiotlb.h>
>+#include <linux/random.h>
>
> #include <uapi/linux/mount.h>
>
>@@ -355,6 +356,13 @@ static void __init parse_setup_data(void)
> case SETUP_EFI:
> parse_efi_setup(pa_data, data_len);
> break;
>+ case SETUP_RNG_SEED:
>+ data = early_memremap(pa_data, data_len);
>+ add_bootloader_randomness(data->data, data->len);
>+ memzero_explicit(data->data, data->len);
>+ memzero_explicit(&data->len, sizeof(data->len));
>+ early_memunmap(data, data_len);
>+ break;
> default:
> break;
> }
Please correct the incredibly confusing title of this patch.
The setup_data linked list has nothing to do with memory types (e820), except that memory types is one kind of data that can be passed on by this mechanism. This title makes it sound like you are passing random data in as a memory type, which could make some bizarre sense if it were, say, some kind of ring buffer preconfigured by the BIOS/VMM/boot loader to be continually overwritten with new random data.
Hi Peter,
On Fri, Jul 01, 2022 at 11:25:49AM -0700, H. Peter Anvin wrote:
> Please correct the incredibly confusing title of this patch.
>
> The setup_data linked list has nothing to do with memory types (e820),
> except that memory types is one kind of data that can be passed on by
> this mechanism. This title makes it sound like you are passing random
> data in as a memory type, which could make some bizarre sense if it
> were, say, some kind of ring buffer preconfigured by the BIOS/VMM/boot
> loader to be continually overwritten with new random data.
Yea that is pretty confusing. Alright, I'll retitle that to something
like:
x86/setup: Allow passing RNG seeds via setup data
Hopefully that matches more, considering the function that does the work
is called "parse_setup_data".
(I'll wait some more time before posting yet another v+1 with that
change, in case others have more feedback.)
Jason
On July 1, 2022 11:42:03 AM PDT, "Jason A. Donenfeld" <[email protected]> wrote:
>Hi Peter,
>
>On Fri, Jul 01, 2022 at 11:25:49AM -0700, H. Peter Anvin wrote:
>> Please correct the incredibly confusing title of this patch.
>>
>> The setup_data linked list has nothing to do with memory types (e820),
>> except that memory types is one kind of data that can be passed on by
>> this mechanism. This title makes it sound like you are passing random
>> data in as a memory type, which could make some bizarre sense if it
>> were, say, some kind of ring buffer preconfigured by the BIOS/VMM/boot
>> loader to be continually overwritten with new random data.
>
>Yea that is pretty confusing. Alright, I'll retitle that to something
>like:
>
> x86/setup: Allow passing RNG seeds via setup data
>
>Hopefully that matches more, considering the function that does the work
>is called "parse_setup_data".
>
>(I'll wait some more time before posting yet another v+1 with that
>change, in case others have more feedback.)
>
>Jason
We usually refer to it as setup_data (with the underscore) to indicate that it is a name and not a description.