diff mbox series

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

Message ID 20211007062340.72207-4-takahiro.akashi@linaro.org
State Accepted
Commit 7a6fb28c8e4b03bc37b05936ae5fa4c16c278520
Headers show
Series efi_loader: capsule: improve capsule authentication support | expand

Commit Message

AKASHI Takahiro Oct. 7, 2021, 6:23 a.m. UTC
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@linaro.org>

---
 lib/efi_loader/efi_capsule.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

-- 
2.33.0

Comments

Ilias Apalodimas Oct. 8, 2021, 7:25 p.m. UTC | #1
Akashi-san,

On Thu, Oct 07, 2021 at 03:23:32PM +0900, AKASHI Takahiro 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"")


Yep I noticed the same thing a few days ago and told Heinrich, I'd fix
that. Thanks!

> Signed-off-by: AKASHI Takahiro <takahiro.akashi@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)

> +{

> +	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

> 


Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Simon Glass Oct. 15, 2021, 12:40 a.m. UTC | #2
Hi Takahiro,

On Thu, 7 Oct 2021 at 00:25, AKASHI Takahiro <takahiro.akashi@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@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?

> +{

> +       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 Oct. 20, 2021, 8:18 a.m. UTC | #3
Hi Simon,

2021年10月15日(金) 9:40 Simon Glass <sjg@chromium.org>:
>

> Hi Takahiro,

>

> On Thu, 7 Oct 2021 at 00:25, AKASHI Takahiro <takahiro.akashi@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@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?)

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 Ozog Oct. 20, 2021, 9:08 a.m. UTC | #4
Le mer. 20 oct. 2021 à 10:18, Masami Hiramatsu <masami.hiramatsu@linaro.org>
a écrit :

> Hi Simon,

>

> 2021年10月15日(金) 9:40 Simon Glass <sjg@chromium.org>:

> >

> > Hi Takahiro,

> >

> > On Thu, 7 Oct 2021 at 00:25, AKASHI Takahiro <takahiro.akashi@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@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@linaro.org | Skype: ffozog
Simon Glass Oct. 20, 2021, 1:39 p.m. UTC | #5
Hi Masami,

On Wed, 20 Oct 2021 at 02:18, Masami Hiramatsu
<masami.hiramatsu@linaro.org> wrote:
>

> Hi Simon,

>

> 2021年10月15日(金) 9:40 Simon Glass <sjg@chromium.org>:

> >

> > Hi Takahiro,

> >

> > On Thu, 7 Oct 2021 at 00:25, AKASHI Takahiro <takahiro.akashi@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@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?)


Are we talking about a public key (which I assume from the function
naming) or some secret key. What is an auth key?

As I understand it, the public key should be provided by the platform
in devicetree that U-Boot uses, by whatever prior stage has access to
the key.

Regards,
Simon
AKASHI Takahiro Oct. 25, 2021, 5:14 a.m. UTC | #6
On Wed, Oct 20, 2021 at 07:39:37AM -0600, Simon Glass wrote:
> Hi Masami,

> 

> On Wed, 20 Oct 2021 at 02:18, Masami Hiramatsu

> <masami.hiramatsu@linaro.org> wrote:

> >

> > Hi Simon,

> >

> > 2021年10月15日(金) 9:40 Simon Glass <sjg@chromium.org>:

> > >

> > > Hi Takahiro,

> > >

> > > On Thu, 7 Oct 2021 at 00:25, AKASHI Takahiro <takahiro.akashi@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@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?)

> 

> Are we talking about a public key (which I assume from the function

> naming) or some secret key. What is an auth key?


Surely, a public key (more strictly, x509 certificate under the current
implementation)

> As I understand it, the public key should be provided by the platform

> in devicetree that U-Boot uses, by whatever prior stage has access to

> the key.


I still believe that some platform provider may want to save the key
in a *safer* storage, which should be at least read-only for non-authorized
writers. But if this issue (__weak or not) is the only blocking factor
for my entire patch series, I'm willing to drop __weak for now since
someone with production system may change it in the future when he has
a good reason for that :)

-Takahiro Akashi


> Regards,

> Simon
François Ozog Oct. 25, 2021, 6:28 a.m. UTC | #7
Le lun. 25 oct. 2021 à 07:14, AKASHI Takahiro <takahiro.akashi@linaro.org>
a écrit :

> On Wed, Oct 20, 2021 at 07:39:37AM -0600, Simon Glass wrote:

> > Hi Masami,

> >

> > On Wed, 20 Oct 2021 at 02:18, Masami Hiramatsu

> > <masami.hiramatsu@linaro.org> wrote:

> > >

> > > Hi Simon,

> > >

> > > 2021年10月15日(金) 9:40 Simon Glass <sjg@chromium.org>:

> > > >

> > > > Hi Takahiro,

> > > >

> > > > On Thu, 7 Oct 2021 at 00:25, AKASHI Takahiro <

> takahiro.akashi@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@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?)

> >

> > Are we talking about a public key (which I assume from the function

> > naming) or some secret key. What is an auth key?

>

> Surely, a public key (more strictly, x509 certificate under the current

> implementation)

>

> > As I understand it, the public key should be provided by the platform

> > in devicetree that U-Boot uses, by whatever prior stage has access to

> > the key.

>

> I still believe that some platform provider may want to save the key

> in a *safer* storage, which should be at least read-only for non-authorized

> writers.



indeed. And U-Boot may not be entitled at all to check the capsule
signature. For example updating SCP firmware with a key that is in secure
world and will never leave this world.


But if this issue (__weak or not) is the only blocking factor
> for my entire patch series, I'm willing to drop __weak for now since

> someone with production system may change it in the future when he has

> a good reason for that :)



If that need be….


>

> -Takahiro Akashi

>

>

> > Regards,

> > Simon

>

-- 
François-Frédéric Ozog | *Director Business Development*
T: +33.67221.6485
francois.ozog@linaro.org | Skype: ffozog
Masami Hiramatsu Oct. 25, 2021, 7:04 a.m. UTC | #8
Hi Francois,

2021年10月25日(月) 15:28 François Ozog <francois.ozog@linaro.org>:
>

>

>

> Le lun. 25 oct. 2021 à 07:14, AKASHI Takahiro <takahiro.akashi@linaro.org> a écrit :

>>

>> On Wed, Oct 20, 2021 at 07:39:37AM -0600, Simon Glass wrote:

>> > Hi Masami,

>> >

>> > On Wed, 20 Oct 2021 at 02:18, Masami Hiramatsu

>> > <masami.hiramatsu@linaro.org> wrote:

>> > >

>> > > Hi Simon,

>> > >

>> > > 2021年10月15日(金) 9:40 Simon Glass <sjg@chromium.org>:

>> > > >

>> > > > Hi Takahiro,

>> > > >

>> > > > On Thu, 7 Oct 2021 at 00:25, AKASHI Takahiro <takahiro.akashi@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@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?)

>> >

>> > Are we talking about a public key (which I assume from the function

>> > naming) or some secret key. What is an auth key?

>>

>> Surely, a public key (more strictly, x509 certificate under the current

>> implementation)

>>

>> > As I understand it, the public key should be provided by the platform

>> > in devicetree that U-Boot uses, by whatever prior stage has access to

>> > the key.

>>

>> I still believe that some platform provider may want to save the key

>> in a *safer* storage, which should be at least read-only for non-authorized

>> writers.

>

>

> indeed. And U-Boot may not be entitled at all to check the capsule signature. For example updating SCP firmware with a key that is in secure world and will never leave this world.


I think secure world firmware updates should be discussed in another
thread, like with FWU. At this point, the capsule signature will be
only authenticated by U-Boot, because we haven't passed the capsule
image to the secure world yet.

>> But if this issue (__weak or not) is the only blocking factor

>> for my entire patch series, I'm willing to drop __weak for now since

>> someone with production system may change it in the future when he has

>> a good reason for that :)

>

>

> If that need be….


Agreed.

Thank you,

>

>>

>>

>> -Takahiro Akashi

>>

>>

>> > Regards,

>> > Simon

>

> --

> François-Frédéric Ozog | Director Business Development

> T: +33.67221.6485

> francois.ozog@linaro.org | Skype: ffozog

>



--
Masami Hiramatsu
François Ozog Oct. 25, 2021, 7:14 a.m. UTC | #9
Le lun. 25 oct. 2021 à 09:05, Masami Hiramatsu <masami.hiramatsu@linaro.org>
a écrit :

> Hi Francois,

>

> 2021年10月25日(月) 15:28 François Ozog <francois.ozog@linaro.org>:

> >

> >

> >

> > Le lun. 25 oct. 2021 à 07:14, AKASHI Takahiro <

> takahiro.akashi@linaro.org> a écrit :

> >>

> >> On Wed, Oct 20, 2021 at 07:39:37AM -0600, Simon Glass wrote:

> >> > Hi Masami,

> >> >

> >> > On Wed, 20 Oct 2021 at 02:18, Masami Hiramatsu

> >> > <masami.hiramatsu@linaro.org> wrote:

> >> > >

> >> > > Hi Simon,

> >> > >

> >> > > 2021年10月15日(金) 9:40 Simon Glass <sjg@chromium.org>:

> >> > > >

> >> > > > Hi Takahiro,

> >> > > >

> >> > > > On Thu, 7 Oct 2021 at 00:25, AKASHI Takahiro <

> takahiro.akashi@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@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?)

> >> >

> >> > Are we talking about a public key (which I assume from the function

> >> > naming) or some secret key. What is an auth key?

> >>

> >> Surely, a public key (more strictly, x509 certificate under the current

> >> implementation)

> >>

> >> > As I understand it, the public key should be provided by the platform

> >> > in devicetree that U-Boot uses, by whatever prior stage has access to

> >> > the key.

> >>

> >> I still believe that some platform provider may want to save the key

> >> in a *safer* storage, which should be at least read-only for

> non-authorized

> >> writers.

> >

> >

> > indeed. And U-Boot may not be entitled at all to check the capsule

> signature. For example updating SCP firmware with a key that is in secure

> world and will never leave this world.

>

> I think secure world firmware updates should be discussed in another

> thread, like with FWU. At this point, the capsule signature will be

> only authenticated by U-Boot, because we haven't passed the capsule

> image to the secure world yet.


i took a wrong example. The choice of authentication is to be done by the
capsule driver designer and is outside scope of U-Boot. And the key may be
in a separate storage or even the driver may invoke secure world for the
authentication (FF-A API or other platform specific). U-Boot may have a
capsule driver to update U-Boot components such as external env file. The
location of the key can be in a u-boot specific device tree.


>

> >> But if this issue (__weak or not) is the only blocking factor

> >> for my entire patch series, I'm willing to drop __weak for now since

> >> someone with production system may change it in the future when he has

> >> a good reason for that :)

> >

> >

> > If that need be….

>

> Agreed.

>

> Thank you,

>

> >

> >>

> >>

> >> -Takahiro Akashi

> >>

> >>

> >> > Regards,

> >> > Simon

> >

> > --

> > François-Frédéric Ozog | Director Business Development

> > T: +33.67221.6485

> > francois.ozog@linaro.org | Skype: ffozog

> >

>

>

> --

> Masami Hiramatsu

>

-- 
François-Frédéric Ozog | *Director Business Development*
T: +33.67221.6485
francois.ozog@linaro.org | Skype: ffozog
Simon Glass Oct. 25, 2021, 3:18 p.m. UTC | #10
Hi François,

On Mon, 25 Oct 2021 at 01:14, François Ozog <francois.ozog@linaro.org> wrote:
>

>

>

> Le lun. 25 oct. 2021 à 09:05, Masami Hiramatsu <masami.hiramatsu@linaro.org> a écrit :

>>

>> Hi Francois,

>>

>> 2021年10月25日(月) 15:28 François Ozog <francois.ozog@linaro.org>:

>> >

>> >

>> >

>> > Le lun. 25 oct. 2021 à 07:14, AKASHI Takahiro <takahiro.akashi@linaro.org> a écrit :

>> >>

>> >> On Wed, Oct 20, 2021 at 07:39:37AM -0600, Simon Glass wrote:

>> >> > Hi Masami,

>> >> >

>> >> > On Wed, 20 Oct 2021 at 02:18, Masami Hiramatsu

>> >> > <masami.hiramatsu@linaro.org> wrote:

>> >> > >

>> >> > > Hi Simon,

>> >> > >

>> >> > > 2021年10月15日(金) 9:40 Simon Glass <sjg@chromium.org>:

>> >> > > >

>> >> > > > Hi Takahiro,

>> >> > > >

>> >> > > > On Thu, 7 Oct 2021 at 00:25, AKASHI Takahiro <takahiro.akashi@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@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?)

>> >> >

>> >> > Are we talking about a public key (which I assume from the function

>> >> > naming) or some secret key. What is an auth key?

>> >>

>> >> Surely, a public key (more strictly, x509 certificate under the current

>> >> implementation)

>> >>

>> >> > As I understand it, the public key should be provided by the platform

>> >> > in devicetree that U-Boot uses, by whatever prior stage has access to

>> >> > the key.

>> >>

>> >> I still believe that some platform provider may want to save the key

>> >> in a *safer* storage, which should be at least read-only for non-authorized

>> >> writers.

>> >

>> >

>> > indeed. And U-Boot may not be entitled at all to check the capsule signature. For example updating SCP firmware with a key that is in secure world and will never leave this world.

>>

>> I think secure world firmware updates should be discussed in another

>> thread, like with FWU. At this point, the capsule signature will be

>> only authenticated by U-Boot, because we haven't passed the capsule

>> image to the secure world yet.

>

> i took a wrong example. The choice of authentication is to be done by the capsule driver designer and is outside scope of U-Boot. And the key may be in a separate storage or even the driver may invoke secure world for the authentication (FF-A API or other platform specific). U-Boot may have a capsule driver to update U-Boot components such as external env file. The location of the key can be in a u-boot specific device tree.


There is no such thing, in practice. There is just one devicetree. We
need to resolve this as we are still not aligned on this, after many
months. I think in fact there is difference of opinion about the
nature of the firmware image itself. I see it is a combined whole,
where the various blobs are an unfortunate result of firmware
fragmentation and must be stitched together with binman, so they can
each see the full picture to the extent needed.

>

>>

>>

>> >> But if this issue (__weak or not) is the only blocking factor

>> >> for my entire patch series, I'm willing to drop __weak for now since

>> >> someone with production system may change it in the future when he has

>> >> a good reason for that :)

>> >

>> >

>> > If that need be….

>>

>> Agreed.

>>


Yes please


- Simon

>>

>> Thank you,

>>

>> >

>> >>

>> >>

>> >> -Takahiro Akashi

>> >>

>> >>

>> >> > Regards,

>> >> > Simon

>> >

>> > --

>> > François-Frédéric Ozog | Director Business Development

>> > T: +33.67221.6485

>> > francois.ozog@linaro.org | Skype: ffozog

>> >

>>

>>

>> --

>> Masami Hiramatsu

>

> --

> François-Frédéric Ozog | Director Business Development

> T: +33.67221.6485

> francois.ozog@linaro.org | Skype: ffozog

>
diff mbox series

Patch

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)
+{
+	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)