All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] efi: consume random seed provided by loader
@ 2022-09-19 16:09 Ard Biesheuvel
  2022-09-19 16:09 ` [PATCH 1/3] efi: random: reduce seed size to 32 bytes Ard Biesheuvel
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2022-09-19 16:09 UTC (permalink / raw)
  To: linux-efi
  Cc: Ard Biesheuvel, Ilias Apalodimas, Jason A . Donenfeld,
	Lennart Poettering, Herbert Xu

Getting a random seed into the kernel very early is important for data
structures that rely on the random number generator at initialization
time, but hardware based RNGs may not become available until much later
in the boot.

For this reason, the EFI stub will currently invoke the EFI RNG protocol
to get a random seed from the hardware before tearing down the EFI boot
services and performing the low level boot of the kernel proper. The
generated seed is passed via a EFI configuration table, which is
available very early, and so the random number generator comes up much
earlier as well.

Any boot stage preceding the EFI stub can install configuration tables,
so we can decide to expose the same mechanism to other loaders.  This
allows, e.g., systemd-boot to pass the seed it keeps in a file on the
ESP without having to rely on PID #1 dd'ing it into /dev/random, which
is much too late to be useful.

As suggested by Jason, let's use blake2s to combine seeds obtained via
the config table and via the protocol if both are available, and throw
in a personalization string for good measure.

Older kernels will simply supersede the bootloader provided seed, unless
the RNG protocol is not available, in which case the bootloader seed
will be forwarded untouched if one is present. This should not be an
issue, but let's reduce the seed size to blake2's output size, and
switch to the correct memory type in separate changes so they can be
backported.

Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Lennart Poettering <lennart@poettering.net>
Cc: Herbert Xu <herbert@gondor.apana.org.au>

Ard Biesheuvel (3):
  efi: random: reduce seed size to 32 bytes
  efi: random: Use 'ACPI reclaim' memory for random seed
  efi: random: combine bootloader provided RNG seed with RNG protocol
    output

 drivers/firmware/efi/efi.c             |  2 +-
 drivers/firmware/efi/libstub/Makefile  |  4 +
 drivers/firmware/efi/libstub/efistub.h |  2 +
 drivers/firmware/efi/libstub/random.c  | 83 ++++++++++++--------
 include/linux/efi.h                    |  4 +-
 lib/crypto/blake2s-generic.c           |  2 +
 lib/crypto/blake2s.c                   |  7 +-
 7 files changed, 64 insertions(+), 40 deletions(-)

-- 
2.35.1


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

* [PATCH 1/3] efi: random: reduce seed size to 32 bytes
  2022-09-19 16:09 [PATCH 0/3] efi: consume random seed provided by loader Ard Biesheuvel
@ 2022-09-19 16:09 ` Ard Biesheuvel
  2022-09-19 16:27   ` Jason A. Donenfeld
  2022-09-19 16:09 ` [PATCH 2/3] efi: random: Use 'ACPI reclaim' memory for random seed Ard Biesheuvel
  2022-09-19 16:09 ` [PATCH 3/3] efi: random: combine bootloader provided RNG seed with RNG protocol output Ard Biesheuvel
  2 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2022-09-19 16:09 UTC (permalink / raw)
  To: linux-efi
  Cc: Ard Biesheuvel, Ilias Apalodimas, Jason A . Donenfeld,
	Lennart Poettering, Herbert Xu, stable

We no longer need at least 64 bytes of random seed to permit the early
crng init to complete. The RNG is now based on Blake2s, so reduce the
EFI seed size to the Blake2s hash size, which is sufficient for our
purposes.

Cc: <stable@vger.kernel.org> # v4.14+
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/efi.c | 2 +-
 include/linux/efi.h        | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index e4080ad96089..06b0755f32a2 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -606,7 +606,7 @@ int __init efi_config_parse_tables(const efi_config_table_t *config_tables,
 
 		seed = early_memremap(efi_rng_seed, sizeof(*seed));
 		if (seed != NULL) {
-			size = READ_ONCE(seed->size);
+			size = min(seed->size, EFI_RANDOM_SEED_SIZE);
 			early_memunmap(seed, sizeof(*seed));
 		} else {
 			pr_err("Could not map UEFI random seed!\n");
diff --git a/include/linux/efi.h b/include/linux/efi.h
index d2b84c2fec39..7b015508c773 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1195,7 +1195,7 @@ efi_status_t efi_random_get_seed(void);
 	arch_efi_call_virt_teardown();					\
 })
 
-#define EFI_RANDOM_SEED_SIZE		64U
+#define EFI_RANDOM_SEED_SIZE		32U // BLAKE2S_HASH_SIZE
 
 struct linux_efi_random_seed {
 	u32	size;
-- 
2.35.1


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

* [PATCH 2/3] efi: random: Use 'ACPI reclaim' memory for random seed
  2022-09-19 16:09 [PATCH 0/3] efi: consume random seed provided by loader Ard Biesheuvel
  2022-09-19 16:09 ` [PATCH 1/3] efi: random: reduce seed size to 32 bytes Ard Biesheuvel
@ 2022-09-19 16:09 ` Ard Biesheuvel
  2022-09-19 16:09 ` [PATCH 3/3] efi: random: combine bootloader provided RNG seed with RNG protocol output Ard Biesheuvel
  2 siblings, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2022-09-19 16:09 UTC (permalink / raw)
  To: linux-efi
  Cc: Ard Biesheuvel, Ilias Apalodimas, Jason A . Donenfeld,
	Lennart Poettering, Herbert Xu, stable

EFI runtime services data is guaranteed to be preserved by the OS,
making it a suitable candidate for the EFI random seed table, which may
be passed to kexec kernels as well (after refreshing the seed), and so
we need to ensure that the memory is preserved without support from the
OS itself.

However, runtime services data is intended for allocations that are
relevant to the implementations of the runtime services themselves, and
so they are unmapped from the kernel linear map, and mapped into the EFI
page tables that are active while runtime service invocations are in
progress. None of this is needed for the RNG seed.

So let's switch to EFI 'ACPI reclaim' memory: in spite of the name,
there is nothing exclusively ACPI about it, it is simply a type of
allocation that carries firmware provided data which may or may not be
relevant to the OS, and it is left up to the OS to decide whether to
reclaim it after having consumed its contents.

Given that in Linux, we never reclaim these allocations, it is a good
choice for the EFI RNG seed, as the allocation is guaranteed to survive
kexec reboots.

One additional reason for changing this now is to align it with the
upcoming recommendation for EFI bootloader provided RNG seeds, which
must not use EFI runtime services code/data allocations.

Cc: <stable@vger.kernel.org> # v4.14+
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/random.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/random.c b/drivers/firmware/efi/libstub/random.c
index 24aa37535372..183dc5cdb8ed 100644
--- a/drivers/firmware/efi/libstub/random.c
+++ b/drivers/firmware/efi/libstub/random.c
@@ -75,7 +75,7 @@ efi_status_t efi_random_get_seed(void)
 	if (status != EFI_SUCCESS)
 		return status;
 
-	status = efi_bs_call(allocate_pool, EFI_RUNTIME_SERVICES_DATA,
+	status = efi_bs_call(allocate_pool, EFI_ACPI_RECLAIM_MEMORY,
 			     sizeof(*seed) + EFI_RANDOM_SEED_SIZE,
 			     (void **)&seed);
 	if (status != EFI_SUCCESS)
-- 
2.35.1


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

* [PATCH 3/3] efi: random: combine bootloader provided RNG seed with RNG protocol output
  2022-09-19 16:09 [PATCH 0/3] efi: consume random seed provided by loader Ard Biesheuvel
  2022-09-19 16:09 ` [PATCH 1/3] efi: random: reduce seed size to 32 bytes Ard Biesheuvel
  2022-09-19 16:09 ` [PATCH 2/3] efi: random: Use 'ACPI reclaim' memory for random seed Ard Biesheuvel
@ 2022-09-19 16:09 ` Ard Biesheuvel
  2022-09-19 16:28   ` Jason A. Donenfeld
                     ` (2 more replies)
  2 siblings, 3 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2022-09-19 16:09 UTC (permalink / raw)
  To: linux-efi
  Cc: Ard Biesheuvel, Ilias Apalodimas, Jason A . Donenfeld,
	Lennart Poettering, Herbert Xu

Instead of blindly creating the EFI random seed configuration table if
the RNG protocol is implemented and works, check whether such a EFI
configuration table was provided by an earlier boot stage and if so,
combine its contents with a Linux specific personalization string, and
if available, mix in the output of the RNG protocol as well.

This can be used for, e.g., systemd-boot, to pass an additional seed to
Linux in a way that can be consumed by the kernel very early. In that
case, the following definitions should be used to pass the seed to the
EFI stub:

  struct linux_efi_random_seed {
          u32     size; // of the 'seed' array in bytes
          u8      seed[];
  };

The memory for the struct must be allocated as EFI_ACPI_RECLAIM_MEMORY
pool memory, and the address of the struct in memory should be installed
as a EFI configuration table using the following GUID:

LINUX_EFI_RANDOM_SEED_TABLE_GUID        1ce1e5bc-7ceb-42f2-81e5-8aadf180f57b

Note that doing so is safe even on kernels that were built without this
patch applied, but the seed will simply be overwritten with a seed
derived from the EFI RNG protocol, if available. The recommended seed
size is 32 bytes, anything beyond that is mixed in but not reflected in
the final seed size.

Suggested-by: "Jason A. Donenfeld" <Jason@zx2c4.com>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/Makefile  |  4 +
 drivers/firmware/efi/libstub/efistub.h |  2 +
 drivers/firmware/efi/libstub/random.c  | 79 ++++++++++++--------
 include/linux/efi.h                    |  2 -
 lib/crypto/blake2s-generic.c           |  2 +
 lib/crypto/blake2s.c                   |  7 +-
 6 files changed, 60 insertions(+), 36 deletions(-)

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index d0537573501e..3b3c67001566 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -55,6 +55,7 @@ KCOV_INSTRUMENT			:= n
 lib-y				:= efi-stub-helper.o gop.o secureboot.o tpm.o \
 				   file.o mem.o random.o randomalloc.o pci.o \
 				   skip_spaces.o lib-cmdline.o lib-ctype.o \
+				   libcrypto-blake2s.o libcrypto-blake2s-generic.o \
 				   alignedmem.o relocate.o vsprintf.o
 
 # include the stub's generic dependencies from lib/ when building for ARM/arm64
@@ -63,6 +64,9 @@ efi-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
 $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE
 	$(call if_changed_rule,cc_o_c)
 
+$(obj)/libcrypto-%.o: $(srctree)/lib/crypto/%.c FORCE
+	$(call if_changed_rule,cc_o_c)
+
 lib-$(CONFIG_EFI_GENERIC_STUB)	+= efi-stub.o fdt.o string.o \
 				   $(patsubst %.c,lib-%.o,$(efi-deps-y))
 
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index b0ae0a454404..fc32897de5e8 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -873,6 +873,8 @@ efi_status_t efi_get_random_bytes(unsigned long size, u8 *out);
 efi_status_t efi_random_alloc(unsigned long size, unsigned long align,
 			      unsigned long *addr, unsigned long random_seed);
 
+void efi_random_get_seed(void);
+
 efi_status_t check_platform_features(void);
 
 void *get_efi_config_table(efi_guid_t guid);
diff --git a/drivers/firmware/efi/libstub/random.c b/drivers/firmware/efi/libstub/random.c
index 183dc5cdb8ed..87fcd73ef615 100644
--- a/drivers/firmware/efi/libstub/random.c
+++ b/drivers/firmware/efi/libstub/random.c
@@ -5,6 +5,7 @@
 
 #include <linux/efi.h>
 #include <asm/efi.h>
+#include <crypto/blake2s.h>
 
 #include "efistub.h"
 
@@ -49,60 +50,74 @@ efi_status_t efi_get_random_bytes(unsigned long size, u8 *out)
 	return efi_call_proto(rng, get_rng, NULL, size, out);
 }
 
+static char const pstr[] = "Linux EFI Stub RNG Seed Label v1";
+
 /**
  * efi_random_get_seed() - provide random seed as configuration table
  *
  * The EFI_RNG_PROTOCOL is used to read random bytes. These random bytes are
  * saved as a configuration table which can be used as entropy by the kernel
  * for the initialization of its pseudo random number generator.
- *
- * If the EFI_RNG_PROTOCOL is not available or there are not enough random bytes
- * available, the configuration table will not be installed and an error code
- * will be returned.
- *
- * Return:	status code
  */
-efi_status_t efi_random_get_seed(void)
+void efi_random_get_seed(void)
 {
 	efi_guid_t rng_proto = EFI_RNG_PROTOCOL_GUID;
 	efi_guid_t rng_algo_raw = EFI_RNG_ALGORITHM_RAW;
 	efi_guid_t rng_table_guid = LINUX_EFI_RANDOM_SEED_TABLE_GUID;
 	efi_rng_protocol_t *rng = NULL;
 	struct linux_efi_random_seed *seed = NULL;
+	struct blake2s_state state;
+	unsigned int total_len = 0;
 	efi_status_t status;
 
-	status = efi_bs_call(locate_protocol, &rng_proto, NULL, (void **)&rng);
-	if (status != EFI_SUCCESS)
-		return status;
+	// grab the EFI RNG protocol, if it exists
+	efi_bs_call(locate_protocol, &rng_proto, NULL, (void **)&rng);
 
-	status = efi_bs_call(allocate_pool, EFI_ACPI_RECLAIM_MEMORY,
-			     sizeof(*seed) + EFI_RANDOM_SEED_SIZE,
-			     (void **)&seed);
-	if (status != EFI_SUCCESS)
-		return status;
+	// grab the seed provided by the previous boot stages
+	seed = get_efi_config_table(LINUX_EFI_RANDOM_SEED_TABLE_GUID);
+
+	// if neither exists, there is little we can do
+	if (!seed && !rng)
+		return;
+
+	blake2s_init(&state, EFI_RANDOM_SEED_SIZE);
+	blake2s_update(&state, pstr, sizeof(pstr) - 1);
+
+	if (seed) {
+		blake2s_update(&state, (void *)&seed->size, sizeof(seed->size));
+		blake2s_update(&state, seed->bits, seed->size);
+		total_len += seed->size;
+	}
 
-	status = efi_call_proto(rng, get_rng, &rng_algo_raw,
-				 EFI_RANDOM_SEED_SIZE, seed->bits);
+	if (rng) {
+		const int sz = EFI_RANDOM_SEED_SIZE;
+		u8 bits[EFI_RANDOM_SEED_SIZE];
 
-	if (status == EFI_UNSUPPORTED)
-		/*
-		 * Use whatever algorithm we have available if the raw algorithm
-		 * is not implemented.
-		 */
-		status = efi_call_proto(rng, get_rng, NULL,
-					EFI_RANDOM_SEED_SIZE, seed->bits);
+		status = efi_call_proto(rng, get_rng, &rng_algo_raw, sz, bits);
+		if (status == EFI_UNSUPPORTED)
+			/*
+			 * Use whatever algorithm we have available if the raw algorithm
+			 * is not implemented.
+			 */
+			status = efi_call_proto(rng, get_rng, NULL, sz, bits);
 
+		if (status == EFI_SUCCESS) {
+			blake2s_update(&state, (void *)&sz, sizeof(sz));
+			blake2s_update(&state, bits, sz);
+			total_len += sz;
+		}
+	}
+
+	status = efi_bs_call(allocate_pool, EFI_ACPI_RECLAIM_MEMORY,
+			     sizeof(*seed) + EFI_RANDOM_SEED_SIZE,
+			     (void **)&seed);
 	if (status != EFI_SUCCESS)
-		goto err_freepool;
+		return;
 
-	seed->size = EFI_RANDOM_SEED_SIZE;
+	blake2s_final(&state, seed->bits);
+	seed->size = min(total_len, EFI_RANDOM_SEED_SIZE);
 	status = efi_bs_call(install_configuration_table, &rng_table_guid, seed);
 	if (status != EFI_SUCCESS)
-		goto err_freepool;
-
-	return EFI_SUCCESS;
+		efi_warn("Failed to install LINUX_EFI_RANDOM_SEED_TABLE_GUID config table\n");
 
-err_freepool:
-	efi_bs_call(free_pool, seed);
-	return status;
 }
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 7b015508c773..95542148d9d7 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1142,8 +1142,6 @@ void efi_check_for_embedded_firmwares(void);
 static inline void efi_check_for_embedded_firmwares(void) { }
 #endif
 
-efi_status_t efi_random_get_seed(void);
-
 #define arch_efi_call_virt(p, f, args...)	((p)->f(args))
 
 /*
diff --git a/lib/crypto/blake2s-generic.c b/lib/crypto/blake2s-generic.c
index 75ccb3e633e6..5d16b5a59618 100644
--- a/lib/crypto/blake2s-generic.c
+++ b/lib/crypto/blake2s-generic.c
@@ -110,6 +110,8 @@ void blake2s_compress_generic(struct blake2s_state *state, const u8 *block,
 
 EXPORT_SYMBOL(blake2s_compress_generic);
 
+#ifdef MODULE
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("BLAKE2s hash function");
 MODULE_AUTHOR("Jason A. Donenfeld <Jason@zx2c4.com>");
+#endif
diff --git a/lib/crypto/blake2s.c b/lib/crypto/blake2s.c
index 98e688c6d891..dbae6d83027b 100644
--- a/lib/crypto/blake2s.c
+++ b/lib/crypto/blake2s.c
@@ -58,15 +58,18 @@ void blake2s_final(struct blake2s_state *state, u8 *out)
 }
 EXPORT_SYMBOL(blake2s_final);
 
+#if !defined(CONFIG_CRYPTO_MANAGER_DISABLE_TESTS) && !defined(__DISABLE_EXPORTS)
 static int __init blake2s_mod_init(void)
 {
-	if (!IS_ENABLED(CONFIG_CRYPTO_MANAGER_DISABLE_TESTS) &&
-	    WARN_ON(!blake2s_selftest()))
+	if (WARN_ON(!blake2s_selftest()))
 		return -ENODEV;
 	return 0;
 }
 
 module_init(blake2s_mod_init);
+#endif
+#ifdef MODULE
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("BLAKE2s hash function");
 MODULE_AUTHOR("Jason A. Donenfeld <Jason@zx2c4.com>");
+#endif
-- 
2.35.1


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

* Re: [PATCH 1/3] efi: random: reduce seed size to 32 bytes
  2022-09-19 16:09 ` [PATCH 1/3] efi: random: reduce seed size to 32 bytes Ard Biesheuvel
@ 2022-09-19 16:27   ` Jason A. Donenfeld
  0 siblings, 0 replies; 10+ messages in thread
From: Jason A. Donenfeld @ 2022-09-19 16:27 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-efi, Ilias Apalodimas, Lennart Poettering, Herbert Xu, stable

On Mon, Sep 19, 2022 at 6:09 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> We no longer need at least 64 bytes of random seed to permit the early
> crng init to complete. The RNG is now based on Blake2s, so reduce the
> EFI seed size to the Blake2s hash size, which is sufficient for our
> purposes.
>
> Cc: <stable@vger.kernel.org> # v4.14+
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>

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

* Re: [PATCH 3/3] efi: random: combine bootloader provided RNG seed with RNG protocol output
  2022-09-19 16:09 ` [PATCH 3/3] efi: random: combine bootloader provided RNG seed with RNG protocol output Ard Biesheuvel
@ 2022-09-19 16:28   ` Jason A. Donenfeld
  2022-09-19 16:35   ` Jason A. Donenfeld
  2022-09-20 16:28   ` Lennart Poettering
  2 siblings, 0 replies; 10+ messages in thread
From: Jason A. Donenfeld @ 2022-09-19 16:28 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-efi, Ilias Apalodimas, Lennart Poettering, Herbert Xu

On Mon, Sep 19, 2022 at 6:09 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> Instead of blindly creating the EFI random seed configuration table if
> the RNG protocol is implemented and works, check whether such a EFI
> configuration table was provided by an earlier boot stage and if so,
> combine its contents with a Linux specific personalization string, and
> if available, mix in the output of the RNG protocol as well.
>
> This can be used for, e.g., systemd-boot, to pass an additional seed to
> Linux in a way that can be consumed by the kernel very early. In that
> case, the following definitions should be used to pass the seed to the
> EFI stub:
>
>   struct linux_efi_random_seed {
>           u32     size; // of the 'seed' array in bytes
>           u8      seed[];
>   };
>
> The memory for the struct must be allocated as EFI_ACPI_RECLAIM_MEMORY
> pool memory, and the address of the struct in memory should be installed
> as a EFI configuration table using the following GUID:
>
> LINUX_EFI_RANDOM_SEED_TABLE_GUID        1ce1e5bc-7ceb-42f2-81e5-8aadf180f57b
>
> Note that doing so is safe even on kernels that were built without this
> patch applied, but the seed will simply be overwritten with a seed
> derived from the EFI RNG protocol, if available. The recommended seed
> size is 32 bytes, anything beyond that is mixed in but not reflected in
> the final seed size.
>
> Suggested-by: "Jason A. Donenfeld" <Jason@zx2c4.com>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>

(And I suppose you can trim those quotation marks in the suggested-by
tag, since it's a git trailer rather than an email header.)

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

* Re: [PATCH 3/3] efi: random: combine bootloader provided RNG seed with RNG protocol output
  2022-09-19 16:09 ` [PATCH 3/3] efi: random: combine bootloader provided RNG seed with RNG protocol output Ard Biesheuvel
  2022-09-19 16:28   ` Jason A. Donenfeld
@ 2022-09-19 16:35   ` Jason A. Donenfeld
  2022-09-20 16:28   ` Lennart Poettering
  2 siblings, 0 replies; 10+ messages in thread
From: Jason A. Donenfeld @ 2022-09-19 16:35 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-efi, Ilias Apalodimas, Lennart Poettering, Herbert Xu

On Mon, Sep 19, 2022 at 6:09 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> Instead of blindly creating the EFI random seed configuration table if
> the RNG protocol is implemented and works, check whether such a EFI
> configuration table was provided by an earlier boot stage and if so,
> combine its contents with a Linux specific personalization string, and
> if available, mix in the output of the RNG protocol as well.
>
> This can be used for, e.g., systemd-boot, to pass an additional seed to
> Linux in a way that can be consumed by the kernel very early. In that
> case, the following definitions should be used to pass the seed to the
> EFI stub:
>
>   struct linux_efi_random_seed {
>           u32     size; // of the 'seed' array in bytes
>           u8      seed[];
>   };
>
> The memory for the struct must be allocated as EFI_ACPI_RECLAIM_MEMORY
> pool memory, and the address of the struct in memory should be installed
> as a EFI configuration table using the following GUID:
>
> LINUX_EFI_RANDOM_SEED_TABLE_GUID        1ce1e5bc-7ceb-42f2-81e5-8aadf180f57b
>
> Note that doing so is safe even on kernels that were built without this
> patch applied, but the seed will simply be overwritten with a seed
> derived from the EFI RNG protocol, if available. The recommended seed
> size is 32 bytes, anything beyond that is mixed in but not reflected in
> the final seed size.
>
> Suggested-by: "Jason A. Donenfeld" <Jason@zx2c4.com>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  drivers/firmware/efi/libstub/Makefile  |  4 +
>  drivers/firmware/efi/libstub/efistub.h |  2 +
>  drivers/firmware/efi/libstub/random.c  | 79 ++++++++++++--------
>  include/linux/efi.h                    |  2 -
>  lib/crypto/blake2s-generic.c           |  2 +
>  lib/crypto/blake2s.c                   |  7 +-
>  6 files changed, 60 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index d0537573501e..3b3c67001566 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -55,6 +55,7 @@ KCOV_INSTRUMENT                       := n
>  lib-y                          := efi-stub-helper.o gop.o secureboot.o tpm.o \
>                                    file.o mem.o random.o randomalloc.o pci.o \
>                                    skip_spaces.o lib-cmdline.o lib-ctype.o \
> +                                  libcrypto-blake2s.o libcrypto-blake2s-generic.o \
>                                    alignedmem.o relocate.o vsprintf.o
>
>  # include the stub's generic dependencies from lib/ when building for ARM/arm64
> @@ -63,6 +64,9 @@ efi-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
>  $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE
>         $(call if_changed_rule,cc_o_c)
>
> +$(obj)/libcrypto-%.o: $(srctree)/lib/crypto/%.c FORCE
> +       $(call if_changed_rule,cc_o_c)
> +
>  lib-$(CONFIG_EFI_GENERIC_STUB) += efi-stub.o fdt.o string.o \
>                                    $(patsubst %.c,lib-%.o,$(efi-deps-y))
>
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index b0ae0a454404..fc32897de5e8 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -873,6 +873,8 @@ efi_status_t efi_get_random_bytes(unsigned long size, u8 *out);
>  efi_status_t efi_random_alloc(unsigned long size, unsigned long align,
>                               unsigned long *addr, unsigned long random_seed);
>
> +void efi_random_get_seed(void);
> +
>  efi_status_t check_platform_features(void);
>
>  void *get_efi_config_table(efi_guid_t guid);
> diff --git a/drivers/firmware/efi/libstub/random.c b/drivers/firmware/efi/libstub/random.c
> index 183dc5cdb8ed..87fcd73ef615 100644
> --- a/drivers/firmware/efi/libstub/random.c
> +++ b/drivers/firmware/efi/libstub/random.c
> @@ -5,6 +5,7 @@
>
>  #include <linux/efi.h>
>  #include <asm/efi.h>
> +#include <crypto/blake2s.h>
>
>  #include "efistub.h"
>
> @@ -49,60 +50,74 @@ efi_status_t efi_get_random_bytes(unsigned long size, u8 *out)
>         return efi_call_proto(rng, get_rng, NULL, size, out);
>  }
>
> +static char const pstr[] = "Linux EFI Stub RNG Seed Label v1";
> +
>  /**
>   * efi_random_get_seed() - provide random seed as configuration table
>   *
>   * The EFI_RNG_PROTOCOL is used to read random bytes. These random bytes are
>   * saved as a configuration table which can be used as entropy by the kernel
>   * for the initialization of its pseudo random number generator.
> - *
> - * If the EFI_RNG_PROTOCOL is not available or there are not enough random bytes
> - * available, the configuration table will not be installed and an error code
> - * will be returned.
> - *
> - * Return:     status code
>   */
> -efi_status_t efi_random_get_seed(void)
> +void efi_random_get_seed(void)
>  {
>         efi_guid_t rng_proto = EFI_RNG_PROTOCOL_GUID;
>         efi_guid_t rng_algo_raw = EFI_RNG_ALGORITHM_RAW;
>         efi_guid_t rng_table_guid = LINUX_EFI_RANDOM_SEED_TABLE_GUID;
>         efi_rng_protocol_t *rng = NULL;
>         struct linux_efi_random_seed *seed = NULL;
> +       struct blake2s_state state;
> +       unsigned int total_len = 0;
>         efi_status_t status;
>
> -       status = efi_bs_call(locate_protocol, &rng_proto, NULL, (void **)&rng);
> -       if (status != EFI_SUCCESS)
> -               return status;
> +       // grab the EFI RNG protocol, if it exists
> +       efi_bs_call(locate_protocol, &rng_proto, NULL, (void **)&rng);
>
> -       status = efi_bs_call(allocate_pool, EFI_ACPI_RECLAIM_MEMORY,
> -                            sizeof(*seed) + EFI_RANDOM_SEED_SIZE,
> -                            (void **)&seed);
> -       if (status != EFI_SUCCESS)
> -               return status;
> +       // grab the seed provided by the previous boot stages
> +       seed = get_efi_config_table(LINUX_EFI_RANDOM_SEED_TABLE_GUID);
> +
> +       // if neither exists, there is little we can do
> +       if (!seed && !rng)
> +               return;
> +
> +       blake2s_init(&state, EFI_RANDOM_SEED_SIZE);
> +       blake2s_update(&state, pstr, sizeof(pstr) - 1);
> +
> +       if (seed) {
> +               blake2s_update(&state, (void *)&seed->size, sizeof(seed->size));
> +               blake2s_update(&state, seed->bits, seed->size);

Since this is never freed, and also just for hygiene, add:

memzero_explicit(seed, sizeof(seed));

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

* Re: [PATCH 3/3] efi: random: combine bootloader provided RNG seed with RNG protocol output
  2022-09-19 16:09 ` [PATCH 3/3] efi: random: combine bootloader provided RNG seed with RNG protocol output Ard Biesheuvel
  2022-09-19 16:28   ` Jason A. Donenfeld
  2022-09-19 16:35   ` Jason A. Donenfeld
@ 2022-09-20 16:28   ` Lennart Poettering
  2022-09-20 16:35     ` Ard Biesheuvel
  2022-09-20 16:41     ` Jason A. Donenfeld
  2 siblings, 2 replies; 10+ messages in thread
From: Lennart Poettering @ 2022-09-20 16:28 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-efi, Ilias Apalodimas, Jason A . Donenfeld, Herbert Xu

On Mo, 19.09.22 18:09, Ard Biesheuvel (ardb@kernel.org) wrote:

Heya!

Looks excellent!

I was wondering though, shouldn't the memory the seed data is stored
in be zeroed out when you dispose of it, just for safety?

> +	if (rng) {
> +		const int sz = EFI_RANDOM_SEED_SIZE;
> +		u8 bits[EFI_RANDOM_SEED_SIZE];
>
> -	if (status == EFI_UNSUPPORTED)
> -		/*
> -		 * Use whatever algorithm we have available if the raw algorithm
> -		 * is not implemented.
> -		 */
> -		status = efi_call_proto(rng, get_rng, NULL,
> -					EFI_RANDOM_SEED_SIZE, seed->bits);
> +		status = efi_call_proto(rng, get_rng, &rng_algo_raw, sz, bits);
> +		if (status == EFI_UNSUPPORTED)
> +			/*
> +			 * Use whatever algorithm we have available if the raw algorithm
> +			 * is not implemented.
> +			 */
> +			status = efi_call_proto(rng, get_rng, NULL, sz, bits);
>
> +		if (status == EFI_SUCCESS) {
> +			blake2s_update(&state, (void *)&sz, sizeof(sz));
> +			blake2s_update(&state, bits, sz);
So, here, shouldn't bitſ[] be zeroed out?

> -	seed->size = EFI_RANDOM_SEED_SIZE;
> +	blake2s_final(&state, seed->bits);

And here, shouldn't the state struct be zeroed out? (or does
blake2s_final() do that implicitly?

Looks excellent otherwise!

Will-be-used-by: systemd
Reviewed-by: Lennart Poettering <mzxreary@0pointer.net>

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

* Re: [PATCH 3/3] efi: random: combine bootloader provided RNG seed with RNG protocol output
  2022-09-20 16:28   ` Lennart Poettering
@ 2022-09-20 16:35     ` Ard Biesheuvel
  2022-09-20 16:41     ` Jason A. Donenfeld
  1 sibling, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2022-09-20 16:35 UTC (permalink / raw)
  To: Lennart Poettering
  Cc: linux-efi, Ilias Apalodimas, Jason A . Donenfeld, Herbert Xu

On Tue, 20 Sept 2022 at 18:28, Lennart Poettering
<lennart@poettering.net> wrote:
>
> On Mo, 19.09.22 18:09, Ard Biesheuvel (ardb@kernel.org) wrote:
>
> Heya!
>
> Looks excellent!
>
> I was wondering though, shouldn't the memory the seed data is stored
> in be zeroed out when you dispose of it, just for safety?
>
> > +     if (rng) {
> > +             const int sz = EFI_RANDOM_SEED_SIZE;
> > +             u8 bits[EFI_RANDOM_SEED_SIZE];
> >
> > -     if (status == EFI_UNSUPPORTED)
> > -             /*
> > -              * Use whatever algorithm we have available if the raw algorithm
> > -              * is not implemented.
> > -              */
> > -             status = efi_call_proto(rng, get_rng, NULL,
> > -                                     EFI_RANDOM_SEED_SIZE, seed->bits);
> > +             status = efi_call_proto(rng, get_rng, &rng_algo_raw, sz, bits);
> > +             if (status == EFI_UNSUPPORTED)
> > +                     /*
> > +                      * Use whatever algorithm we have available if the raw algorithm
> > +                      * is not implemented.
> > +                      */
> > +                     status = efi_call_proto(rng, get_rng, NULL, sz, bits);
> >
> > +             if (status == EFI_SUCCESS) {
> > +                     blake2s_update(&state, (void *)&sz, sizeof(sz));
> > +                     blake2s_update(&state, bits, sz);
> So, here, shouldn't bitſ[] be zeroed out?
>
> > -     seed->size = EFI_RANDOM_SEED_SIZE;
> > +     blake2s_final(&state, seed->bits);
>
> And here, shouldn't the state struct be zeroed out? (or does
> blake2s_final() do that implicitly?
>

Yes, Jason pointed that out as well. (Twice, actually :-))

> Looks excellent otherwise!
>
> Will-be-used-by: systemd
> Reviewed-by: Lennart Poettering <mzxreary@0pointer.net>

Thanks, much appreciated.

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

* Re: [PATCH 3/3] efi: random: combine bootloader provided RNG seed with RNG protocol output
  2022-09-20 16:28   ` Lennart Poettering
  2022-09-20 16:35     ` Ard Biesheuvel
@ 2022-09-20 16:41     ` Jason A. Donenfeld
  1 sibling, 0 replies; 10+ messages in thread
From: Jason A. Donenfeld @ 2022-09-20 16:41 UTC (permalink / raw)
  To: Lennart Poettering
  Cc: Ard Biesheuvel, linux-efi, Ilias Apalodimas, Herbert Xu

On Tue, Sep 20, 2022 at 6:28 PM Lennart Poettering
<lennart@poettering.net> wrote:
> I was wondering though, shouldn't the memory the seed data is stored
> in be zeroed out when you dispose of it, just for safety?

I mentioned the same. I think Ard is gonna handle that for v2, in
addition to freeing the prior seed's allocation.

> > +     blake2s_final(&state, seed->bits);
>
> And here, shouldn't the state struct be zeroed out? (or does
> blake2s_final() do that implicitly?

In this case, blake2s_final does it implicitly.

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

end of thread, other threads:[~2022-09-20 16:42 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-19 16:09 [PATCH 0/3] efi: consume random seed provided by loader Ard Biesheuvel
2022-09-19 16:09 ` [PATCH 1/3] efi: random: reduce seed size to 32 bytes Ard Biesheuvel
2022-09-19 16:27   ` Jason A. Donenfeld
2022-09-19 16:09 ` [PATCH 2/3] efi: random: Use 'ACPI reclaim' memory for random seed Ard Biesheuvel
2022-09-19 16:09 ` [PATCH 3/3] efi: random: combine bootloader provided RNG seed with RNG protocol output Ard Biesheuvel
2022-09-19 16:28   ` Jason A. Donenfeld
2022-09-19 16:35   ` Jason A. Donenfeld
2022-09-20 16:28   ` Lennart Poettering
2022-09-20 16:35     ` Ard Biesheuvel
2022-09-20 16:41     ` Jason A. Donenfeld

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.