[PATCH v2 3/4] efi_capsule: Add a function to get the public key needed for capsule authentication

[Sughosh Ganu]

On Thu, 15 Apr 2021 at 01:08, Simon Glass <sjg at chromium.org> wrote:

> On Mon, 12 Apr 2021 at 16:06, Sughosh Ganu <sughosh.ganu at linaro.org>
> wrote:
> >
> > Define a function which would be used in the scenario where the
> > public key is stored on the platform's dtb. This dtb is concatenated
> > with the u-boot binary during the build process. Platforms which have
> > a different mechanism for getting the public key would define their
> > own platform specific function under a different Kconfig symbol.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > ---
> >
> > Changes since V1:
> > * Remove the weak function, and add the functionality to retrieve the
> >   public key under the config symbol CONFIG_EFI_PKEY_DTB_EMBED.
> >
> >  lib/efi_loader/efi_capsule.c | 43 +++++++++++++++++++++++++++++++-----
> >  1 file changed, 38 insertions(+), 5 deletions(-)
>
> Is this defined in a header file somewhere?
>

No, I will declare the function in a header. Will do so in the next version.


>
> >
> > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > index 2cc8f2dee0..d95e9377fe 100644
> > --- a/lib/efi_loader/efi_capsule.c
> > +++ b/lib/efi_loader/efi_capsule.c
> > @@ -14,10 +14,13 @@
> >  #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;
> > @@ -208,15 +211,45 @@ skip:
> >  const efi_guid_t efi_guid_capsule_root_cert_guid =
> >         EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
> >
> > -__weak int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
> > +#if defined(CONFIG_EFI_PKEY_DTB_EMBED)
>
> Can you drop the #ifdef ?
>

It will be good to keep the ifdef. This way, if some other platform wants
to define a function for getting the public key using a different, platform
specific method(for e.g. getting the keys from some read-only device like a
fuse), it will be possible to do so. Without the ifdef, this becomes the
only way to get the public key.


> > +int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
>
> pkey should really have a time...what is it?
>

This is really a public key certificate in an efi signature list(esl)
format. The parsing of the certificate and it's use for capsule
authentication is done by the same set of functions which perform image
authentication for the  uefi secure boot feature.


> >  {
> > -       /* The platform is supposed to provide
> > -        * a method for getting the public key
> > -        * stored in the form of efi signature
> > -        * list
> > +       /*
> > +        * This is a function for retrieving the public key from the
> > +        * platform's device tree. The platform's device tree has been
> > +        * concatenated with the u-boot binary.
> > +        * If a platform has a different mechanism to get the public
> > +        * key, it can define it's own kconfig symbol and define a
> > +        * function to retrieve the public key
> >          */
> > +       const void *fdt_blob = gd->fdt_blob;
> > +       const void *blob;
>
> prop? val? It is not a DT blob
>

Okay.


>
> > +       const char *cnode_name = "capsule-key";
> > +       const char *snode_name = "signature";
>
> I believe these FIT things are defined in image.h
>

These are based on the node names that are populated by the mkeficapsule
utility. If you don't have a strong opinion on this, I would like to keep
them as is. I can define macros for them.


>
> > +       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;
> > +       }
>
> Can you use the livetree API?
>

Can you please point me to the specific API that you are referring to.
Thanks.

-sughosh


>
> > +
> > +       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;
> >  }
> > +#endif /* CONFIG_EFI_PKEY_DTB_EMBED */
> >
> >  efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t
> capsule_size,
> >                                       void **image, efi_uintn_t
> *image_size)
> > --
> > 2.17.1
> >
>
> Regards,
> Simon
>