On Sat, 17 Jul 2021 at 14:35, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
[...]
obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o +obj-$(CONFIG_EFI_CAPSULE_AUTHENTICATE) += efi_capsule_key.o
We should give users another choice here to allow them to add their own solution for key storage. Or only enable this line if "CONFIG_EFI_CAPSULE_KEY_PATH" != null?
Actually once you enable the capsule authentication compilation fails now if a file is not provided, with a message asking for the use to provide a valid filepath. I'd prefer leaving it as is and once we get a hardware that can override the embedded key, we can add an extra Kconfig option.
obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_firmware.o obj-y += efi_console.o obj-y += efi_device_path.o diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index b878e71438b8..50e93cad4ee5 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -16,6 +16,7 @@ #include <mapmem.h> #include <sort.h>
+#include <asm/sections.h> #include <crypto/pkcs7.h> #include <crypto/pkcs7_parser.h> #include <linux/err.h> @@ -222,12 +223,23 @@ skip: const efi_guid_t efi_guid_capsule_root_cert_guid = EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
+int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
static?
yea
Once we do get support for hardware stored signatures, this can be changed to a __weak function.
+{
const void *blob = __efi_capsule_sig_begin;const int len = __efi_capsule_sig_end - __efi_capsule_sig_begin;It seems that the length can be calculated at compile time.
Yea but you still need the __efi_capsule_sig_begin. What's the proposal here? Replace __efi_capsule_sig_end with the size on the .S file?
*pkey = (void *)blob;*pkey_len = len;return 0;+}
efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_size, void **image, efi_uintn_t *image_size) { u8 *buf; int ret;
void *fdt_pkey, *pkey;
void *stored_pkey, *pkey; efi_uintn_t pkey_len; uint64_t monotonic_count; struct efi_signature_store *truststore;@@ -286,7 +298,7 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s goto out; }
ret = efi_get_public_key_data(&fdt_pkey, &pkey_len);
ret = efi_get_public_key_data(&stored_pkey, &pkey_len); if (ret < 0) goto out;@@ -294,7 +306,7 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s if (!pkey) goto out;
memcpy(pkey, fdt_pkey, pkey_len);
memcpy(pkey, stored_pkey, pkey_len); truststore = efi_build_signature_store(pkey, pkey_len); if (!truststore) goto out;diff --git a/lib/efi_loader/efi_capsule_key.S b/lib/efi_loader/efi_capsule_key.S new file mode 100644 index 000000000000..f7047a42e39d --- /dev/null +++ b/lib/efi_loader/efi_capsule_key.S @@ -0,0 +1,8 @@
Should we have "#include <config.h>" here?
Hmm maybe. Compiling didn't cause any problems, but it seems we can add that include
Otherwise it looks good.
-Takahiro Akashi
Thanks /Ilias
+.section .rodata.capsule_key.init,"a" +.balign 16 +.global __efi_capsule_sig_begin +__efi_capsule_sig_begin: +.incbin CONFIG_EFI_CAPSULE_KEY_PATH +__efi_capsule_sig_end: +.global __efi_capsule_sig_end
+.balign 16
2.32.0.rc0
-- Masami Hiramatsu