[PATCH v4 03/11] efi_loader: capsule: add back efi_get_public_key_data()

[François Ozog]

Le mer. 20 oct. 2021 à 10:18, Masami Hiramatsu <masami.hiramatsu at linaro.org>
a écrit :

> Hi Simon,
>
> 2021年10月15日(金) 9:40 Simon Glass <sjg at chromium.org>:
> >
> > Hi Takahiro,
> >
> > On Thu, 7 Oct 2021 at 00:25, AKASHI Takahiro <takahiro.akashi at linaro.org>
> wrote:
> > >
> > > The commit 47a25e81d35c ("Revert "efi_capsule: Move signature from DTB
> to
> > > .rodata"") failed to revert the removal of efi_get_public_key_data().
> > >
> > > Add back this function and move it under lib/efi_loader so that other
> > > platforms can utilize it. It is now declared as a weak function so that
> > > it can be replaced with a platform-specific implementation.
> > >
> > > Fixes: 47a25e81d35c ("Revert "efi_capsule: Move signature from DTB to
> > >         .rodata"")
> > > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> > > ---
> > >  lib/efi_loader/efi_capsule.c | 36 ++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 36 insertions(+)
> > >
> > > diff --git a/lib/efi_loader/efi_capsule.c
> b/lib/efi_loader/efi_capsule.c
> > > index b75e4bcba1a9..44f5da61a9be 100644
> > > --- a/lib/efi_loader/efi_capsule.c
> > > +++ b/lib/efi_loader/efi_capsule.c
> > > @@ -11,15 +11,20 @@
> > >  #include <common.h>
> > >  #include <efi_loader.h>
> > >  #include <efi_variable.h>
> > > +#include <env.h>
> > > +#include <fdtdec.h>
> > >  #include <fs.h>
> > >  #include <malloc.h>
> > >  #include <mapmem.h>
> > >  #include <sort.h>
> > > +#include <asm/global_data.h>
> > >
> > >  #include <crypto/pkcs7.h>
> > >  #include <crypto/pkcs7_parser.h>
> > >  #include <linux/err.h>
> > >
> > > +DECLARE_GLOBAL_DATA_PTR;
> > > +
> > >  const efi_guid_t efi_guid_capsule_report = EFI_CAPSULE_REPORT_GUID;
> > >  static const efi_guid_t efi_guid_firmware_management_capsule_id =
> > >                 EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
> > > @@ -251,6 +256,37 @@ out:
> > >  }
> > >
> > >  #if defined(CONFIG_EFI_CAPSULE_AUTHENTICATE)
> > > +int __weak efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
> >
> > I don't think this should be weak. What other way is there of handling
> > this and why would it be platform-specific?
>
> I have a question about the current design of the capsule auth key.
> If the platform has its own key-storage, how can the platform use the
> platform specific storage? Does such platform load the key from the storage
> and generate the dtb node in the platform initialization code? (or
> device driver?)

it depends on what the capsule contains.

If the capsule contains SCP firmware or secure firmware or TAs, U-Boot may
not be even allowed to see the key.
If the capsule contains U-Boot itself it may be again outside scope of
U-Boot because that may be secure firmware that verifies the signature. We
may allow U-Boot to update itself but the final say is the secure firmware
that may prevent the boot.
If the capsule contains device firmware then it may depend on the device:
secure device U-Boot can do anything, otherwise then it is to be decided by
U-Boot.


>
> Thank you,
>
>
> >
> > > +{
> > > +       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) {
> > > +               log_err("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) {
> > > +               log_err("Unable to get capsule-key value\n");
> > > +               *pkey = NULL;
> > > +               *pkey_len = 0;
> > > +
> > > +               return -FDT_ERR_NOTFOUND;
> > > +       }
> > > +
> > > +       *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)
> > > --
> > > 2.33.0
> > >
> >
> > Regards,
> > Simon
>
>
>
> --
> Masami Hiramatsu
>
-- 
François-Frédéric Ozog | *Director Business Development*
T: +33.67221.6485
francois.ozog at linaro.org | Skype: ffozog