[PATCH 1/3 v2] efi_capsule: Move signature from DTB to .rodata
Sughosh Ganu
sughosh.ganu at linaro.org
Tue Jul 20 15:31:57 CEST 2021
hi Simon,
On Tue, 20 Jul 2021 at 18:20, Ilias Apalodimas <ilias.apalodimas at linaro.org>
wrote:
> Hi Simon,
> On Tue, 20 Jul 2021 at 15:33, Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Ilias,
> >
> > On Sat, 17 Jul 2021 at 08:27, Ilias Apalodimas
> > <ilias.apalodimas at linaro.org> wrote:
> > >
> > > The capsule signature is now part of our DTB. This is problematic
> when a
> > > user is allowed to change/fixup that DTB from U-Boots command line
> since he
> > > can overwrite the signature as well.
> >
> > Just to repeat my question since it looks like I didn't get a response
> > on the last patch:
> >
> > Do you mean with the 'fdt' command?
> > >
> > If you mean the FDT fixups, they happen to a different DT, the one
> > being passed to Linux.
>
> In some platforms the key is derived from the relocated DTB, which we
> can overwrite. But I'll let Sughosh who figured it out explain the
> details.
>
On platforms where the dtb is concatenated with the u-boot image, using
CONFIG_OF_SEPARATE, the fdt is also getting relocated to the main memory.
We retrieve the public key from this dtb. By default, the fdtcontroladdr
env variable is getting set to this relocated dtb address -- this address
can also be accessed using the bdinfo command. Thus the public key can be
modified before attempting the capsule update. Which is the reason why
Ilias is moving the public key to the embedded rodata section.
-sughosh
> >
> > > So Instead of adding the key on the DTB, embed it in the u-boot binary
> it
> > > self as part of it's .rodata. This assumes that the U-Boot binary we
> load
> > > is authenticated by a previous boot stage loader.
> >
> > As I mentioned, this means you need to build U-Boot from source and
> > include the key. I don't think that is a good idea at all. Signing
> > should be a separate step from building.
>
> You need this key (which is an .esl cert) to authenticate the capsule
> you are going to apply. You *sign* the capsules with an external
> application (like GenerateCapsule provided by edk2 and we can also
> extend uboot's mkeficapsule for that). So we aren't signing anything
> here
>
> Thanks
> /Ilias
>
> /Ilias
> >
> >
> > - Simon
> >
> > >
> > > Reviewed-by: Masami Hiramatsu <masami.hiramatsu at linaro.org>
> > > Tested-by: Masami Hiramatsu <masami.hiramatsu at linaro.org>
> > > Tested-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > > ---
> > > changes since v1:
> > > - added static keyword on efi_get_public_key_data()
> > > - added missing config.h on efi_capsule_key.S
> > >
> > > board/emulation/common/Makefile | 1 -
> > > board/emulation/common/qemu_capsule.c | 43 ---------------------------
> > > include/asm-generic/sections.h | 2 ++
> > > lib/efi_loader/Kconfig | 7 +++++
> > > lib/efi_loader/Makefile | 8 +++++
> > > lib/efi_loader/efi_capsule.c | 18 +++++++++--
> > > lib/efi_loader/efi_capsule_key.S | 17 +++++++++++
> > > 7 files changed, 49 insertions(+), 47 deletions(-)
> > > delete mode 100644 board/emulation/common/qemu_capsule.c
> > > create mode 100644 lib/efi_loader/efi_capsule_key.S
> > >
> > > diff --git a/board/emulation/common/Makefile
> b/board/emulation/common/Makefile
> > > index 7ed447a69dce..c5b452e7e341 100644
> > > --- a/board/emulation/common/Makefile
> > > +++ b/board/emulation/common/Makefile
> > > @@ -2,4 +2,3 @@
> > >
> > > obj-$(CONFIG_SYS_MTDPARTS_RUNTIME) += qemu_mtdparts.o
> > > obj-$(CONFIG_SET_DFU_ALT_INFO) += qemu_dfu.o
> > > -obj-$(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT) += qemu_capsule.o
> > > diff --git a/board/emulation/common/qemu_capsule.c
> b/board/emulation/common/qemu_capsule.c
> > > deleted file mode 100644
> > > index 6b8a87022a4c..000000000000
> > > --- a/board/emulation/common/qemu_capsule.c
> > > +++ /dev/null
> > > @@ -1,43 +0,0 @@
> > > -// SPDX-License-Identifier: GPL-2.0+
> > > -/*
> > > - * Copyright (c) 2020 Linaro Limited
> > > - */
> > > -
> > > -#include <common.h>
> > > -#include <efi_api.h>
> > > -#include <efi_loader.h>
> > > -#include <env.h>
> > > -#include <fdtdec.h>
> > > -#include <asm/global_data.h>
> > > -
> > > -DECLARE_GLOBAL_DATA_PTR;
> > > -
> > > -int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
> > > -{
> > > - const void *fdt_blob = gd->fdt_blob;
> > > - const void *blob;
> > > - const char *cnode_name = "capsule-key";
> > > - const char *snode_name = "signature";
> > > - int sig_node;
> > > - int len;
> > > -
> > > - sig_node = fdt_subnode_offset(fdt_blob, 0, snode_name);
> > > - if (sig_node < 0) {
> > > - EFI_PRINT("Unable to get signature node offset\n");
> > > - return -FDT_ERR_NOTFOUND;
> > > - }
> > > -
> > > - blob = fdt_getprop(fdt_blob, sig_node, cnode_name, &len);
> > > -
> > > - if (!blob || len < 0) {
> > > - EFI_PRINT("Unable to get capsule-key value\n");
> > > - *pkey = NULL;
> > > - *pkey_len = 0;
> > > - return -FDT_ERR_NOTFOUND;
> > > - }
> > > -
> > > - *pkey = (void *)blob;
> > > - *pkey_len = len;
> > > -
> > > - return 0;
> > > -}
> > > diff --git a/include/asm-generic/sections.h
> b/include/asm-generic/sections.h
> > > index 267f1db73f23..ec992b0c2e3f 100644
> > > --- a/include/asm-generic/sections.h
> > > +++ b/include/asm-generic/sections.h
> > > @@ -27,6 +27,8 @@ extern char __efi_helloworld_begin[];
> > > extern char __efi_helloworld_end[];
> > > extern char __efi_var_file_begin[];
> > > extern char __efi_var_file_end[];
> > > +extern char __efi_capsule_sig_begin[];
> > > +extern char __efi_capsule_sig_end[];
> > >
> > > /* Private data used by of-platdata devices/uclasses */
> > > extern char __priv_data_start[], __priv_data_end[];
> > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > > index 156b39152112..cf6ff2d537f4 100644
> > > --- a/lib/efi_loader/Kconfig
> > > +++ b/lib/efi_loader/Kconfig
> > > @@ -213,6 +213,13 @@ config EFI_CAPSULE_AUTHENTICATE
> > > Select this option if you want to enable capsule
> > > authentication
> > >
> > > +config EFI_CAPSULE_KEY_PATH
> > > + string "Path to .esl cert for capsule authentication"
> > > + depends on EFI_CAPSULE_AUTHENTICATE
> > > + help
> > > + Provide the EFI signature list (esl) certificate used for
> capsule
> > > + authentication
> > > +
> > > config EFI_DEVICE_PATH_TO_TEXT
> > > bool "Device path to text protocol"
> > > default y
> > > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> > > index fd344cea29b0..9b369430e258 100644
> > > --- a/lib/efi_loader/Makefile
> > > +++ b/lib/efi_loader/Makefile
> > > @@ -20,11 +20,19 @@ always += helloworld.efi
> > > targets += helloworld.o
> > > endif
> > >
> > > +ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y)
> > > +EFI_CAPSULE_KEY_PATH := $(subst $\",,$(CONFIG_EFI_CAPSULE_KEY_PATH))
> > > +ifeq ("$(wildcard $(EFI_CAPSULE_KEY_PATH))","")
> > > +$(error .esl cerificate not found. Configure your
> CONFIG_EFI_CAPSULE_KEY_PATH)
> > > +endif
> > > +endif
> > > +
> > > obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
> > > obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o
> > > obj-y += efi_boottime.o
> > > obj-y += efi_helper.o
> > > obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
> > > +obj-$(CONFIG_EFI_CAPSULE_AUTHENTICATE) += efi_capsule_key.o
> > > 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..1900a938c140 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;
> > >
> > > +static int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
> > > +{
> > > + const void *blob = __efi_capsule_sig_begin;
> > > + const int len = __efi_capsule_sig_end -
> __efi_capsule_sig_begin;
> > > +
> > > + *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..58f00b8e4bcb
> > > --- /dev/null
> > > +++ b/lib/efi_loader/efi_capsule_key.S
> > > @@ -0,0 +1,17 @@
> > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > +/*
> > > + * .esl cert for capsule authentication
> > > + *
> > > + * Copyright (c) 2021, Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > > + */
> > > +
> > > +#include <config.h>
> > > +
> > > +.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.31.1
> > >
>
More information about the U-Boot
mailing list