* [PATCH] efi_loader: capsule: add a debug message in case of no key
@ 2021-05-10 8:19 AKASHI Takahiro
2021-05-20 2:06 ` Heinrich Schuchardt
0 siblings, 1 reply; 6+ messages in thread
From: AKASHI Takahiro @ 2021-05-10 8:19 UTC (permalink / raw)
To: u-boot
It will probably be a common error case that a certificate (public key)
is not provided by the system while capsule authentication is enabled.
So add a debug message.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
lib/efi_loader/efi_capsule.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index 90893f85e22c..84ddaf50d13f 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -316,8 +316,10 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s
}
ret = efi_get_public_key_data(&fdt_pkey, &pkey_len);
- if (ret < 0)
+ if (ret < 0) {
+ debug("Public key/certificate not found\n");
goto out;
+ }
pkey = malloc(pkey_len);
if (!pkey)
--
2.31.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] efi_loader: capsule: add a debug message in case of no key
2021-05-10 8:19 [PATCH] efi_loader: capsule: add a debug message in case of no key AKASHI Takahiro
@ 2021-05-20 2:06 ` Heinrich Schuchardt
2021-07-20 2:13 ` AKASHI Takahiro
0 siblings, 1 reply; 6+ messages in thread
From: Heinrich Schuchardt @ 2021-05-20 2:06 UTC (permalink / raw)
To: u-boot
On 5/10/21 10:19 AM, AKASHI Takahiro wrote:
> It will probably be a common error case that a certificate (public key)
> is not provided by the system while capsule authentication is enabled.
> So add a debug message.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> lib/efi_loader/efi_capsule.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index 90893f85e22c..84ddaf50d13f 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -316,8 +316,10 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s
> }
>
> ret = efi_get_public_key_data(&fdt_pkey, &pkey_len);
> - if (ret < 0)
> + if (ret < 0) {
> + debug("Public key/certificate not found\n");
Currently the only implementation of efi_get_public_key_data() actually
providing keys is the one in board/emulation/common/qemu_capsule.c where
the user has to manually upload the esl file.
For future implementation it is preferable to build the public key data
into the U-Boot binary. If it is part of the build process then the only
error that could come up is that the public key data has the wrong format.
If we are using the weak implementation of efi_get_public_key_data() in
lib/efi_loader/efi_capsule.cwith CONFIG_EFI_CAPSULE_AUTHENTICATE=y, the
system is misconfigured. Do we need that weak implementation at all? I
would prefer to remove to get a build error.
I suggest that you add a log_err() message with above text into the
board/emulation/common/qemu_capsule.c implementation of
efi_get_public_key_data(). This way the user will see that he forgot a step.
Best regards
Heinrich
> goto out;
> + }
>
> pkey = malloc(pkey_len);
> if (!pkey)
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] efi_loader: capsule: add a debug message in case of no key
2021-05-20 2:06 ` Heinrich Schuchardt
@ 2021-07-20 2:13 ` AKASHI Takahiro
2021-07-20 6:39 ` Ilias Apalodimas
0 siblings, 1 reply; 6+ messages in thread
From: AKASHI Takahiro @ 2021-07-20 2:13 UTC (permalink / raw)
To: Heinrich Schuchardt; +Cc: sughosh.ganu, u-boot, Ilias Apalodimas, agraf
On Thu, May 20, 2021 at 04:06:12AM +0200, Heinrich Schuchardt wrote:
> On 5/10/21 10:19 AM, AKASHI Takahiro wrote:
> > It will probably be a common error case that a certificate (public key)
> > is not provided by the system while capsule authentication is enabled.
> > So add a debug message.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > lib/efi_loader/efi_capsule.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > index 90893f85e22c..84ddaf50d13f 100644
> > --- a/lib/efi_loader/efi_capsule.c
> > +++ b/lib/efi_loader/efi_capsule.c
> > @@ -316,8 +316,10 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s
> > }
> >
> > ret = efi_get_public_key_data(&fdt_pkey, &pkey_len);
> > - if (ret < 0)
> > + if (ret < 0) {
> > + debug("Public key/certificate not found\n");
>
> Currently the only implementation of efi_get_public_key_data() actually
> providing keys is the one in board/emulation/common/qemu_capsule.c where
> the user has to manually upload the esl file.
>
> For future implementation it is preferable to build the public key data
> into the U-Boot binary. If it is part of the build process then the only
> error that could come up is that the public key data has the wrong format.
Now Ilias posted a patch to embed a public key in the U-Boot binary.
But it won't be the only solution in the future and the system owners
may want to provide a key in their own way; hence, it might not be "part
of build process."
So I think that adding a message is still valid, even it should be
treated as an error message instead of a debug message to warn "users".
-Takahiro Akashi
> If we are using the weak implementation of efi_get_public_key_data() in
> lib/efi_loader/efi_capsule.cwith CONFIG_EFI_CAPSULE_AUTHENTICATE=y, the
> system is misconfigured. Do we need that weak implementation at all? I
> would prefer to remove to get a build error.
>
> I suggest that you add a log_err() message with above text into the
> board/emulation/common/qemu_capsule.c implementation of
> efi_get_public_key_data(). This way the user will see that he forgot a step.
>
> Best regards
>
> Heinrich
>
> > goto out;
> > + }
> >
> > pkey = malloc(pkey_len);
> > if (!pkey)
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] efi_loader: capsule: add a debug message in case of no key
2021-07-20 2:13 ` AKASHI Takahiro
@ 2021-07-20 6:39 ` Ilias Apalodimas
2021-07-20 6:48 ` AKASHI Takahiro
0 siblings, 1 reply; 6+ messages in thread
From: Ilias Apalodimas @ 2021-07-20 6:39 UTC (permalink / raw)
To: AKASHI Takahiro, Heinrich Schuchardt, sughosh.ganu, u-boot, agraf
Akashi-san,
On Tue, Jul 20, 2021 at 11:13:40AM +0900, AKASHI Takahiro wrote:
> On Thu, May 20, 2021 at 04:06:12AM +0200, Heinrich Schuchardt wrote:
> > On 5/10/21 10:19 AM, AKASHI Takahiro wrote:
> > > It will probably be a common error case that a certificate (public key)
> > > is not provided by the system while capsule authentication is enabled.
> > > So add a debug message.
> > >
> > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > ---
> > > lib/efi_loader/efi_capsule.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > > index 90893f85e22c..84ddaf50d13f 100644
> > > --- a/lib/efi_loader/efi_capsule.c
> > > +++ b/lib/efi_loader/efi_capsule.c
> > > @@ -316,8 +316,10 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s
> > > }
> > >
> > > ret = efi_get_public_key_data(&fdt_pkey, &pkey_len);
> > > - if (ret < 0)
> > > + if (ret < 0) {
> > > + debug("Public key/certificate not found\n");
> >
> > Currently the only implementation of efi_get_public_key_data() actually
> > providing keys is the one in board/emulation/common/qemu_capsule.c where
> > the user has to manually upload the esl file.
> >
> > For future implementation it is preferable to build the public key data
> > into the U-Boot binary. If it is part of the build process then the only
> > error that could come up is that the public key data has the wrong format.
>
> Now Ilias posted a patch to embed a public key in the U-Boot binary.
> But it won't be the only solution in the future and the system owners
> may want to provide a key in their own way; hence, it might not be "part
> of build process."
>
Correct. My patch intentionally leaves out that part and I hope someone
will need it and implement it.
> So I think that adding a message is still valid, even it should be
> treated as an error message instead of a debug message to warn "users".
Keep in mind that the makefile currently checks for the .esl file. if the
file is not found there's a compilation error, prompting the user to add a
valid file
Thanks
/Ilias
>
> -Takahiro Akashi
>
>
> > If we are using the weak implementation of efi_get_public_key_data() in
> > lib/efi_loader/efi_capsule.cwith CONFIG_EFI_CAPSULE_AUTHENTICATE=y, the
> > system is misconfigured. Do we need that weak implementation at all? I
> > would prefer to remove to get a build error.
> >
> > I suggest that you add a log_err() message with above text into the
> > board/emulation/common/qemu_capsule.c implementation of
> > efi_get_public_key_data(). This way the user will see that he forgot a step.
> >
> > Best regards
> >
> > Heinrich
> >
> > > goto out;
> > > + }
> > >
> > > pkey = malloc(pkey_len);
> > > if (!pkey)
> > >
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] efi_loader: capsule: add a debug message in case of no key
2021-07-20 6:39 ` Ilias Apalodimas
@ 2021-07-20 6:48 ` AKASHI Takahiro
2021-07-20 7:18 ` Ilias Apalodimas
0 siblings, 1 reply; 6+ messages in thread
From: AKASHI Takahiro @ 2021-07-20 6:48 UTC (permalink / raw)
To: Ilias Apalodimas; +Cc: Heinrich Schuchardt, sughosh.ganu, u-boot, agraf
On Tue, Jul 20, 2021 at 09:39:12AM +0300, Ilias Apalodimas wrote:
> Akashi-san,
>
> On Tue, Jul 20, 2021 at 11:13:40AM +0900, AKASHI Takahiro wrote:
> > On Thu, May 20, 2021 at 04:06:12AM +0200, Heinrich Schuchardt wrote:
> > > On 5/10/21 10:19 AM, AKASHI Takahiro wrote:
> > > > It will probably be a common error case that a certificate (public key)
> > > > is not provided by the system while capsule authentication is enabled.
> > > > So add a debug message.
> > > >
> > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > > ---
> > > > lib/efi_loader/efi_capsule.c | 4 +++-
> > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > > > index 90893f85e22c..84ddaf50d13f 100644
> > > > --- a/lib/efi_loader/efi_capsule.c
> > > > +++ b/lib/efi_loader/efi_capsule.c
> > > > @@ -316,8 +316,10 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s
> > > > }
> > > >
> > > > ret = efi_get_public_key_data(&fdt_pkey, &pkey_len);
> > > > - if (ret < 0)
> > > > + if (ret < 0) {
> > > > + debug("Public key/certificate not found\n");
> > >
> > > Currently the only implementation of efi_get_public_key_data() actually
> > > providing keys is the one in board/emulation/common/qemu_capsule.c where
> > > the user has to manually upload the esl file.
> > >
> > > For future implementation it is preferable to build the public key data
> > > into the U-Boot binary. If it is part of the build process then the only
> > > error that could come up is that the public key data has the wrong format.
> >
> > Now Ilias posted a patch to embed a public key in the U-Boot binary.
> > But it won't be the only solution in the future and the system owners
> > may want to provide a key in their own way; hence, it might not be "part
> > of build process."
> >
>
> Correct. My patch intentionally leaves out that part and I hope someone
> will need it and implement it.
>
> > So I think that adding a message is still valid, even it should be
> > treated as an error message instead of a debug message to warn "users".
>
>
> Keep in mind that the makefile currently checks for the .esl file. if the
> file is not found there's a compilation error, prompting the user to add a
> valid file
If your efi_get_public_key_data() is the only implementation in
the system, checking a return value (if ret < 0) is also meaningless.
-Takahiro Akashi
>
> Thanks
> /Ilias
> >
> > -Takahiro Akashi
> >
> >
> > > If we are using the weak implementation of efi_get_public_key_data() in
> > > lib/efi_loader/efi_capsule.cwith CONFIG_EFI_CAPSULE_AUTHENTICATE=y, the
> > > system is misconfigured. Do we need that weak implementation at all? I
> > > would prefer to remove to get a build error.
> > >
> > > I suggest that you add a log_err() message with above text into the
> > > board/emulation/common/qemu_capsule.c implementation of
> > > efi_get_public_key_data(). This way the user will see that he forgot a step.
> > >
> > > Best regards
> > >
> > > Heinrich
> > >
> > > > goto out;
> > > > + }
> > > >
> > > > pkey = malloc(pkey_len);
> > > > if (!pkey)
> > > >
> > >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] efi_loader: capsule: add a debug message in case of no key
2021-07-20 6:48 ` AKASHI Takahiro
@ 2021-07-20 7:18 ` Ilias Apalodimas
0 siblings, 0 replies; 6+ messages in thread
From: Ilias Apalodimas @ 2021-07-20 7:18 UTC (permalink / raw)
To: AKASHI Takahiro, Heinrich Schuchardt, sughosh.ganu, u-boot, agraf
> > > > > + debug("Public key/certificate not found\n");
[...]
> > > >
> > > > Currently the only implementation of efi_get_public_key_data() actually
> > > > providing keys is the one in board/emulation/common/qemu_capsule.c where
> > > > the user has to manually upload the esl file.
> > > >
> > > > For future implementation it is preferable to build the public key data
> > > > into the U-Boot binary. If it is part of the build process then the only
> > > > error that could come up is that the public key data has the wrong format.
> > >
> > > Now Ilias posted a patch to embed a public key in the U-Boot binary.
> > > But it won't be the only solution in the future and the system owners
> > > may want to provide a key in their own way; hence, it might not be "part
> > > of build process."
> > >
> >
> > Correct. My patch intentionally leaves out that part and I hope someone
> > will need it and implement it.
> >
> > > So I think that adding a message is still valid, even it should be
> > > treated as an error message instead of a debug message to warn "users".
> >
> >
> > Keep in mind that the makefile currently checks for the .esl file. if the
> > file is not found there's a compilation error, prompting the user to add a
> > valid file
>
> If your efi_get_public_key_data() is the only implementation in
> the system, checking a return value (if ret < 0) is also meaningless.
Yea but what I expect here, is to make it a __weak function in the future
and allow reading the key from hardware. So the check is there to ensure
that when we add other ways of reading the key we are doing the right thing
Thanks
/Ilias
>
> -Takahiro Akashi
>
>
> >
> > Thanks
> > /Ilias
> > >
> > > -Takahiro Akashi
> > >
> > >
> > > > If we are using the weak implementation of efi_get_public_key_data() in
> > > > lib/efi_loader/efi_capsule.cwith CONFIG_EFI_CAPSULE_AUTHENTICATE=y, the
> > > > system is misconfigured. Do we need that weak implementation at all? I
> > > > would prefer to remove to get a build error.
> > > >
> > > > I suggest that you add a log_err() message with above text into the
> > > > board/emulation/common/qemu_capsule.c implementation of
> > > > efi_get_public_key_data(). This way the user will see that he forgot a step.
> > > >
> > > > Best regards
> > > >
> > > > Heinrich
> > > >
> > > > > goto out;
> > > > > + }
> > > > >
> > > > > pkey = malloc(pkey_len);
> > > > > if (!pkey)
> > > > >
> > > >
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-07-20 7:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-10 8:19 [PATCH] efi_loader: capsule: add a debug message in case of no key AKASHI Takahiro
2021-05-20 2:06 ` Heinrich Schuchardt
2021-07-20 2:13 ` AKASHI Takahiro
2021-07-20 6:39 ` Ilias Apalodimas
2021-07-20 6:48 ` AKASHI Takahiro
2021-07-20 7:18 ` Ilias Apalodimas
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.