[0/5] Add support for embedding public key in platform's dtb

Message ID 20210407115335.8615-1-sughosh.ganu@linaro.org
Headers show
Series
  • Add support for embedding public key in platform's dtb
Related show

Message

Sughosh Ganu April 7, 2021, 11:53 a.m.
Patch 1 fixes an issue of selection of IMAGE_SIGN_INFO config option
when capsule authentication is enabled.

Patch 2 add two config symbols, EFI_PKEY_DTB_EMBED and EFI_PKEY_FILE
which are used for enabling embedding of the public key in the dtb,
and specifying the esl file name.

Patch 3 moves efi_capsule_auth_enabled as a weak function, which can
be used as a default mechanism for checking if capsule authentication
has been enabled.

Patch 4 adds a default weak function for retrieving the public key
from the platform's dtb.

Patch 5 adds the functionality to embed the esl file into the
platform's dtb during the platform build.

I have tested this functionality on the STM32MP157C DK2 board.

[1] - https://lists.denx.de/pipermail/u-boot/2021-March/442867.html 

Sughosh Ganu (5):
  efi_loader: Kconfig: Select IMAGE_SIGN_INFO when capsule
    authentication is enabled
  efi_loader: Kconfig: Add symbols for embedding the public key into the
    platform's dtb
  efi_capsule: Add a weak function to check whether capsule
    authentication is enabled
  efi_capsule: Add a weak function to get the public key needed for
    capsule authentication
  Makefile: Add provision for embedding public key in platform's dtb

 Makefile                              | 10 ++++++
 board/emulation/common/qemu_capsule.c |  6 ----
 lib/efi_loader/Kconfig                | 16 ++++++++++
 lib/efi_loader/efi_capsule.c          | 44 ++++++++++++++++++++++++---
 4 files changed, 66 insertions(+), 10 deletions(-)

-- 
2.17.1

Comments

Simon Glass April 7, 2021, 4:14 p.m. | #1
Hi,

On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>

> Patch 1 fixes an issue of selection of IMAGE_SIGN_INFO config option

> when capsule authentication is enabled.

>

> Patch 2 add two config symbols, EFI_PKEY_DTB_EMBED and EFI_PKEY_FILE

> which are used for enabling embedding of the public key in the dtb,

> and specifying the esl file name.

>

> Patch 3 moves efi_capsule_auth_enabled as a weak function, which can

> be used as a default mechanism for checking if capsule authentication

> has been enabled.

>

> Patch 4 adds a default weak function for retrieving the public key

> from the platform's dtb.

>

> Patch 5 adds the functionality to embed the esl file into the

> platform's dtb during the platform build.

>

> I have tested this functionality on the STM32MP157C DK2 board.

>

> [1] - https://lists.denx.de/pipermail/u-boot/2021-March/442867.html

>

> Sughosh Ganu (5):

>   efi_loader: Kconfig: Select IMAGE_SIGN_INFO when capsule

>     authentication is enabled

>   efi_loader: Kconfig: Add symbols for embedding the public key into the

>     platform's dtb

>   efi_capsule: Add a weak function to check whether capsule

>     authentication is enabled

>   efi_capsule: Add a weak function to get the public key needed for

>     capsule authentication

>   Makefile: Add provision for embedding public key in platform's dtb

>

>  Makefile                              | 10 ++++++

>  board/emulation/common/qemu_capsule.c |  6 ----

>  lib/efi_loader/Kconfig                | 16 ++++++++++

>  lib/efi_loader/efi_capsule.c          | 44 ++++++++++++++++++++++++---

>  4 files changed, 66 insertions(+), 10 deletions(-)

>

> --

> 2.17.1

>


We need to rethink the use of weak functions for this sort of thing,
or we will end up with an unnavigable mess at some point. If we need
to adjust the flow of boot, let's adjust the flow rather than adding
hooks everywhere.

Regards,
Simon
Sughosh Ganu April 8, 2021, 6:53 a.m. | #2
hi Simon,

On Wed, 7 Apr 2021 at 21:44, Simon Glass <sjg@chromium.org> wrote:

> Hi,

>

> On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:

> >

> > Patch 1 fixes an issue of selection of IMAGE_SIGN_INFO config option

> > when capsule authentication is enabled.

> >

> > Patch 2 add two config symbols, EFI_PKEY_DTB_EMBED and EFI_PKEY_FILE

> > which are used for enabling embedding of the public key in the dtb,

> > and specifying the esl file name.

> >

> > Patch 3 moves efi_capsule_auth_enabled as a weak function, which can

> > be used as a default mechanism for checking if capsule authentication

> > has been enabled.

> >

> > Patch 4 adds a default weak function for retrieving the public key

> > from the platform's dtb.

> >

> > Patch 5 adds the functionality to embed the esl file into the

> > platform's dtb during the platform build.

> >

> > I have tested this functionality on the STM32MP157C DK2 board.

> >

> > [1] - https://lists.denx.de/pipermail/u-boot/2021-March/442867.html

> >

> > Sughosh Ganu (5):

> >   efi_loader: Kconfig: Select IMAGE_SIGN_INFO when capsule

> >     authentication is enabled

> >   efi_loader: Kconfig: Add symbols for embedding the public key into the

> >     platform's dtb

> >   efi_capsule: Add a weak function to check whether capsule

> >     authentication is enabled

> >   efi_capsule: Add a weak function to get the public key needed for

> >     capsule authentication

> >   Makefile: Add provision for embedding public key in platform's dtb

> >

> >  Makefile                              | 10 ++++++

> >  board/emulation/common/qemu_capsule.c |  6 ----

> >  lib/efi_loader/Kconfig                | 16 ++++++++++

> >  lib/efi_loader/efi_capsule.c          | 44 ++++++++++++++++++++++++---

> >  4 files changed, 66 insertions(+), 10 deletions(-)

> >

> > --

> > 2.17.1

> >

>

> We need to rethink the use of weak functions for this sort of thing,

> or we will end up with an unnavigable mess at some point. If we need

> to adjust the flow of boot, let's adjust the flow rather than adding

> hooks everywhere.



There are two weak functions being added. One is for retrieving the public
key to be used for the capsule authentication, and the other is for
checking for whether capsule authentication has been enabled. The reason
why a weak function is needed is because platforms can have other
mechanisms for retrieval of the public key or for testing if capsule
authentication has been enabled.

If we consider the case of public key retrieval, the majority of platforms
would be built with the device tree concatenated with the u-boot binary.
The weak function would cater to all of those platforms -- having a weak
function would mean that we are not required to repeat the same
functionality for every platform that uses the same mechanism for
extracting the public key. However, there would be platforms where the
public key is obtained through a different mechanism which is platform
specific. Those platforms would have to define their own function to get
the public key. Same for checking whether capsule authentication feature
has been enabled or not.

-sughosh
Heinrich Schuchardt April 8, 2021, 8:41 a.m. | #3
On 08.04.21 08:53, Sughosh Ganu wrote:
> hi Simon,

>

> On Wed, 7 Apr 2021 at 21:44, Simon Glass <sjg@chromium.org

> <mailto:sjg@chromium.org>> wrote:

>

>     Hi,

>

>     On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu <sughosh.ganu@linaro.org

>     <mailto:sughosh.ganu@linaro.org>> wrote:

>     >

>     > Patch 1 fixes an issue of selection of IMAGE_SIGN_INFO config option

>     > when capsule authentication is enabled.

>     >

>     > Patch 2 add two config symbols, EFI_PKEY_DTB_EMBED and EFI_PKEY_FILE

>     > which are used for enabling embedding of the public key in the dtb,

>     > and specifying the esl file name.

>     >

>     > Patch 3 moves efi_capsule_auth_enabled as a weak function, which can

>     > be used as a default mechanism for checking if capsule authentication

>     > has been enabled.

>     >

>     > Patch 4 adds a default weak function for retrieving the public key

>     > from the platform's dtb.

>     >

>     > Patch 5 adds the functionality to embed the esl file into the

>     > platform's dtb during the platform build.

>     >

>     > I have tested this functionality on the STM32MP157C DK2 board.

>     >

>     > [1] -

>     https://lists.denx.de/pipermail/u-boot/2021-March/442867.html

>     <https://lists.denx.de/pipermail/u-boot/2021-March/442867.html>

>     >

>     > Sughosh Ganu (5):

>     >   efi_loader: Kconfig: Select IMAGE_SIGN_INFO when capsule

>     >     authentication is enabled

>     >   efi_loader: Kconfig: Add symbols for embedding the public key

>     into the

>     >     platform's dtb

>     >   efi_capsule: Add a weak function to check whether capsule

>     >     authentication is enabled

>     >   efi_capsule: Add a weak function to get the public key needed for

>     >     capsule authentication

>     >   Makefile: Add provision for embedding public key in platform's dtb

>     >

>     >  Makefile                              | 10 ++++++

>     >  board/emulation/common/qemu_capsule.c |  6 ----

>     >  lib/efi_loader/Kconfig                | 16 ++++++++++

>     >  lib/efi_loader/efi_capsule.c          | 44

>     ++++++++++++++++++++++++---

>     >  4 files changed, 66 insertions(+), 10 deletions(-)

>     >

>     > --

>     > 2.17.1

>     >

>

>     We need to rethink the use of weak functions for this sort of thing,

>     or we will end up with an unnavigable mess at some point. If we need

>     to adjust the flow of boot, let's adjust the flow rather than adding

>     hooks everywhere.

>

>

> There are two weak functions being added. One is for retrieving the

> public key to be used for the capsule authentication, and the other is

> for checking for whether capsule authentication has been enabled. The

> reason why a weak function is needed is because platforms can have other

> mechanisms for retrieval of the public key or for testing if capsule

> authentication has been enabled.

>

> If we consider the case of public key retrieval, the majority of

> platforms would be built with the device tree concatenated with the

> u-boot binary. The weak function would cater to all of those platforms

> -- having a weak function would mean that we are not required to repeat

> the same functionality for every platform that uses the same mechanism

> for extracting the public key. However, there would be platforms where

> the public key is obtained through a different mechanism which is

> platform specific. Those platforms would have to define their own

> function to get the public key. Same for checking whether capsule

> authentication feature has been enabled or not.

>

> -sughosh


Hello Sughosh,

Could you, please, explain why there could be a need to use public keys
for capsule verification that are not compiled into U-Boot. I cannot see
how this would increase security.

I cannot imagine any scenario where you would want to allow switching
off capsule authentication if it has been built into U-Boot.

Best regards

Heinrich
Sughosh Ganu April 8, 2021, 10:10 a.m. | #4
hi Heinrich,

On Thu, 8 Apr 2021 at 14:17, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

> On 08.04.21 08:53, Sughosh Ganu wrote:

> > hi Simon,

> >

> > On Wed, 7 Apr 2021 at 21:44, Simon Glass <sjg@chromium.org

> > <mailto:sjg@chromium.org>> wrote:

> >

> >     Hi,

> >

> >     On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu <sughosh.ganu@linaro.org

> >     <mailto:sughosh.ganu@linaro.org>> wrote:

> >     >

> >     > Patch 1 fixes an issue of selection of IMAGE_SIGN_INFO config

> option

> >     > when capsule authentication is enabled.

> >     >

> >     > Patch 2 add two config symbols, EFI_PKEY_DTB_EMBED and

> EFI_PKEY_FILE

> >     > which are used for enabling embedding of the public key in the dtb,

> >     > and specifying the esl file name.

> >     >

> >     > Patch 3 moves efi_capsule_auth_enabled as a weak function, which

> can

> >     > be used as a default mechanism for checking if capsule

> authentication

> >     > has been enabled.

> >     >

> >     > Patch 4 adds a default weak function for retrieving the public key

> >     > from the platform's dtb.

> >     >

> >     > Patch 5 adds the functionality to embed the esl file into the

> >     > platform's dtb during the platform build.

> >     >

> >     > I have tested this functionality on the STM32MP157C DK2 board.

> >     >

> >     > [1] -

> >     https://lists.denx.de/pipermail/u-boot/2021-March/442867.html

> >     <https://lists.denx.de/pipermail/u-boot/2021-March/442867.html>

> >     >

> >     > Sughosh Ganu (5):

> >     >   efi_loader: Kconfig: Select IMAGE_SIGN_INFO when capsule

> >     >     authentication is enabled

> >     >   efi_loader: Kconfig: Add symbols for embedding the public key

> >     into the

> >     >     platform's dtb

> >     >   efi_capsule: Add a weak function to check whether capsule

> >     >     authentication is enabled

> >     >   efi_capsule: Add a weak function to get the public key needed for

> >     >     capsule authentication

> >     >   Makefile: Add provision for embedding public key in platform's

> dtb

> >     >

> >     >  Makefile                              | 10 ++++++

> >     >  board/emulation/common/qemu_capsule.c |  6 ----

> >     >  lib/efi_loader/Kconfig                | 16 ++++++++++

> >     >  lib/efi_loader/efi_capsule.c          | 44

> >     ++++++++++++++++++++++++---

> >     >  4 files changed, 66 insertions(+), 10 deletions(-)

> >     >

> >     > --

> >     > 2.17.1

> >     >

> >

> >     We need to rethink the use of weak functions for this sort of thing,

> >     or we will end up with an unnavigable mess at some point. If we need

> >     to adjust the flow of boot, let's adjust the flow rather than adding

> >     hooks everywhere.

> >

> >

> > There are two weak functions being added. One is for retrieving the

> > public key to be used for the capsule authentication, and the other is

> > for checking for whether capsule authentication has been enabled. The

> > reason why a weak function is needed is because platforms can have other

> > mechanisms for retrieval of the public key or for testing if capsule

> > authentication has been enabled.

> >

> > If we consider the case of public key retrieval, the majority of

> > platforms would be built with the device tree concatenated with the

> > u-boot binary. The weak function would cater to all of those platforms

> > -- having a weak function would mean that we are not required to repeat

> > the same functionality for every platform that uses the same mechanism

> > for extracting the public key. However, there would be platforms where

> > the public key is obtained through a different mechanism which is

> > platform specific. Those platforms would have to define their own

> > function to get the public key. Same for checking whether capsule

> > authentication feature has been enabled or not.

> >

> > -sughosh

>

> Hello Sughosh,

>

> Could you, please, explain why there could be a need to use public keys

> for capsule verification that are not compiled into U-Boot. I cannot see

> how this would increase security.

>


With the changes that have been made in the Makefile(patch 5/5), the public
key is now embedded into the platform's dtb, and subsequently this dtb is
concatenated with the u-boot binary to create a single u-boot.bin image.
This image can then be verified during the trusted boot flow to check
against any kind of tampering. This takes care of your concern of not
having the public key separately on the disk, which makes it open to
tampering, with the public key now embedded as part of the u-boot image.
You had suggested embedding the public key as part of the u-boot image. I
have embedded it within the platform's dtb which is part of the u-boot
image. This becomes a generic solution which is platform and architecture
agnostic. I believe concatenating the platform's dtb with the u-boot binary
is the standard flow for production images.

I cannot imagine any scenario where you would want to allow switching
> off capsule authentication if it has been built into U-Boot.

>


This is only an additional knob for any user who might want to perform a
capsule update without authentication -- with this additional knob, the
user can use the same image for updating a capsule which does not have an
authentication header. The user would not be required to recompile the
image to turn off CONFIG_EFI_CAPSULE_AUTHENTICATE config option. But if you
don't see this necessary, i can remove this additional check. In that case,
the capsule will be authenticated when CONFIG_EFI_CAPSULE_AUTHENTICATE is
enabled.

-sughosh
Heinrich Schuchardt April 8, 2021, 11:21 a.m. | #5
On 08.04.21 12:10, Sughosh Ganu wrote:
> hi Heinrich,

>

> On Thu, 8 Apr 2021 at 14:17, Heinrich Schuchardt <xypron.glpk@gmx.de

> <mailto:xypron.glpk@gmx.de>> wrote:

>

>     On 08.04.21 08:53, Sughosh Ganu wrote:

>     > hi Simon,

>     >

>     > On Wed, 7 Apr 2021 at 21:44, Simon Glass <sjg@chromium.org

>     <mailto:sjg@chromium.org>

>     > <mailto:sjg@chromium.org <mailto:sjg@chromium.org>>> wrote:

>     >

>     >     Hi,

>     >

>     >     On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu

>     <sughosh.ganu@linaro.org <mailto:sughosh.ganu@linaro.org>

>     >     <mailto:sughosh.ganu@linaro.org

>     <mailto:sughosh.ganu@linaro.org>>> wrote:

>     >     >

>     >     > Patch 1 fixes an issue of selection of IMAGE_SIGN_INFO

>     config option

>     >     > when capsule authentication is enabled.

>     >     >

>     >     > Patch 2 add two config symbols, EFI_PKEY_DTB_EMBED and

>     EFI_PKEY_FILE

>     >     > which are used for enabling embedding of the public key in

>     the dtb,

>     >     > and specifying the esl file name.

>     >     >

>     >     > Patch 3 moves efi_capsule_auth_enabled as a weak function,

>     which can

>     >     > be used as a default mechanism for checking if capsule

>     authentication

>     >     > has been enabled.

>     >     >

>     >     > Patch 4 adds a default weak function for retrieving the

>     public key

>     >     > from the platform's dtb.

>     >     >

>     >     > Patch 5 adds the functionality to embed the esl file into the

>     >     > platform's dtb during the platform build.

>     >     >

>     >     > I have tested this functionality on the STM32MP157C DK2 board.

>     >     >

>     >     > [1] -

>     >     https://lists.denx.de/pipermail/u-boot/2021-March/442867.html

>     <https://lists.denx.de/pipermail/u-boot/2021-March/442867.html>

>     >     <https://lists.denx.de/pipermail/u-boot/2021-March/442867.html

>     <https://lists.denx.de/pipermail/u-boot/2021-March/442867.html>>

>     >     >

>     >     > Sughosh Ganu (5):

>     >     >   efi_loader: Kconfig: Select IMAGE_SIGN_INFO when capsule

>     >     >     authentication is enabled

>     >     >   efi_loader: Kconfig: Add symbols for embedding the public key

>     >     into the

>     >     >     platform's dtb

>     >     >   efi_capsule: Add a weak function to check whether capsule

>     >     >     authentication is enabled

>     >     >   efi_capsule: Add a weak function to get the public key

>     needed for

>     >     >     capsule authentication

>     >     >   Makefile: Add provision for embedding public key in

>     platform's dtb

>     >     >

>     >     >  Makefile                              | 10 ++++++

>     >     >  board/emulation/common/qemu_capsule.c |  6 ----

>     >     >  lib/efi_loader/Kconfig                | 16 ++++++++++

>     >     >  lib/efi_loader/efi_capsule.c          | 44

>     >     ++++++++++++++++++++++++---

>     >     >  4 files changed, 66 insertions(+), 10 deletions(-)

>     >     >

>     >     > --

>     >     > 2.17.1

>     >     >

>     >

>     >     We need to rethink the use of weak functions for this sort of

>     thing,

>     >     or we will end up with an unnavigable mess at some point. If

>     we need

>     >     to adjust the flow of boot, let's adjust the flow rather than

>     adding

>     >     hooks everywhere.

>     >

>     >

>     > There are two weak functions being added. One is for retrieving the

>     > public key to be used for the capsule authentication, and the other is

>     > for checking for whether capsule authentication has been enabled. The

>     > reason why a weak function is needed is because platforms can have

>     other

>     > mechanisms for retrieval of the public key or for testing if capsule

>     > authentication has been enabled.

>     >

>     > If we consider the case of public key retrieval, the majority of

>     > platforms would be built with the device tree concatenated with the

>     > u-boot binary. The weak function would cater to all of those platforms

>     > -- having a weak function would mean that we are not required to

>     repeat

>     > the same functionality for every platform that uses the same mechanism

>     > for extracting the public key. However, there would be platforms where

>     > the public key is obtained through a different mechanism which is

>     > platform specific. Those platforms would have to define their own

>     > function to get the public key. Same for checking whether capsule

>     > authentication feature has been enabled or not.

>     >

>     > -sughosh

>

>     Hello Sughosh,

>

>     Could you, please, explain why there could be a need to use public keys

>     for capsule verification that are not compiled into U-Boot. I cannot see

>     how this would increase security.

>

>

> With the changes that have been made in the Makefile(patch 5/5), the

> public key is now embedded into the platform's dtb, and subsequently

> this dtb is concatenated with the u-boot binary to create a single

> u-boot.bin image. This image can then be verified during the trusted

> boot flow to check against any kind of tampering. This takes care of

> your concern of not having the public key separately on the disk, which

> makes it open to tampering, with the public key now embedded as part of

> the u-boot image. You had suggested embedding the public key as part of

> the u-boot image. I have embedded it within the platform's dtb which is

> part of the u-boot image. This becomes a generic solution which is

> platform and architecture agnostic. I believe concatenating the

> platform's dtb with the u-boot binary is the standard flow for

> production images.


Embedding the key in the device-tree is fine. I am just trying to
understand why you need the extensibility via a weak function.

>

>     I cannot imagine any scenario where you would want to allow switching

>     off capsule authentication if it has been built into U-Boot.

>

>

> This is only an additional knob for any user who might want to perform a

> capsule update without authentication -- with this additional knob, the

> user can use the same image for updating a capsule which does not have

> an authentication header. The user would not be required to recompile

> the image to turn off CONFIG_EFI_CAPSULE_AUTHENTICATE config option. But

> if you don't see this necessary, i can remove this additional check. In

> that case, the capsule will be authenticated when

> CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled.


I would prefer to reduce the number of "knobs" that you have to check
when rolling out secure firmware.

Best regards

Heinrich
Sughosh Ganu April 8, 2021, 11:48 a.m. | #6
On Thu, 8 Apr 2021 at 16:51, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

> On 08.04.21 12:10, Sughosh Ganu wrote:

> > hi Heinrich,

> >

> > On Thu, 8 Apr 2021 at 14:17, Heinrich Schuchardt <xypron.glpk@gmx.de

> > <mailto:xypron.glpk@gmx.de>> wrote:

> >

> >     On 08.04.21 08:53, Sughosh Ganu wrote:

> >     > hi Simon,

> >     >

> >     > On Wed, 7 Apr 2021 at 21:44, Simon Glass <sjg@chromium.org

> >     <mailto:sjg@chromium.org>

> >     > <mailto:sjg@chromium.org <mailto:sjg@chromium.org>>> wrote:

> >     >

> >     >     Hi,

> >     >

> >     >     On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu

> >     <sughosh.ganu@linaro.org <mailto:sughosh.ganu@linaro.org>

> >     >     <mailto:sughosh.ganu@linaro.org

> >     <mailto:sughosh.ganu@linaro.org>>> wrote:

> >     >     >

> >     >     > Patch 1 fixes an issue of selection of IMAGE_SIGN_INFO

> >     config option

> >     >     > when capsule authentication is enabled.

> >     >     >

> >     >     > Patch 2 add two config symbols, EFI_PKEY_DTB_EMBED and

> >     EFI_PKEY_FILE

> >     >     > which are used for enabling embedding of the public key in

> >     the dtb,

> >     >     > and specifying the esl file name.

> >     >     >

> >     >     > Patch 3 moves efi_capsule_auth_enabled as a weak function,

> >     which can

> >     >     > be used as a default mechanism for checking if capsule

> >     authentication

> >     >     > has been enabled.

> >     >     >

> >     >     > Patch 4 adds a default weak function for retrieving the

> >     public key

> >     >     > from the platform's dtb.

> >     >     >

> >     >     > Patch 5 adds the functionality to embed the esl file into the

> >     >     > platform's dtb during the platform build.

> >     >     >

> >     >     > I have tested this functionality on the STM32MP157C DK2

> board.

> >     >     >

> >     >     > [1] -

> >     >     https://lists.denx.de/pipermail/u-boot/2021-March/442867.html

> >     <https://lists.denx.de/pipermail/u-boot/2021-March/442867.html>

> >     >     <https://lists.denx.de/pipermail/u-boot/2021-March/442867.html

> >     <https://lists.denx.de/pipermail/u-boot/2021-March/442867.html>>

> >     >     >

> >     >     > Sughosh Ganu (5):

> >     >     >   efi_loader: Kconfig: Select IMAGE_SIGN_INFO when capsule

> >     >     >     authentication is enabled

> >     >     >   efi_loader: Kconfig: Add symbols for embedding the public

> key

> >     >     into the

> >     >     >     platform's dtb

> >     >     >   efi_capsule: Add a weak function to check whether capsule

> >     >     >     authentication is enabled

> >     >     >   efi_capsule: Add a weak function to get the public key

> >     needed for

> >     >     >     capsule authentication

> >     >     >   Makefile: Add provision for embedding public key in

> >     platform's dtb

> >     >     >

> >     >     >  Makefile                              | 10 ++++++

> >     >     >  board/emulation/common/qemu_capsule.c |  6 ----

> >     >     >  lib/efi_loader/Kconfig                | 16 ++++++++++

> >     >     >  lib/efi_loader/efi_capsule.c          | 44

> >     >     ++++++++++++++++++++++++---

> >     >     >  4 files changed, 66 insertions(+), 10 deletions(-)

> >     >     >

> >     >     > --

> >     >     > 2.17.1

> >     >     >

> >     >

> >     >     We need to rethink the use of weak functions for this sort of

> >     thing,

> >     >     or we will end up with an unnavigable mess at some point. If

> >     we need

> >     >     to adjust the flow of boot, let's adjust the flow rather than

> >     adding

> >     >     hooks everywhere.

> >     >

> >     >

> >     > There are two weak functions being added. One is for retrieving the

> >     > public key to be used for the capsule authentication, and the

> other is

> >     > for checking for whether capsule authentication has been enabled.

> The

> >     > reason why a weak function is needed is because platforms can have

> >     other

> >     > mechanisms for retrieval of the public key or for testing if

> capsule

> >     > authentication has been enabled.

> >     >

> >     > If we consider the case of public key retrieval, the majority of

> >     > platforms would be built with the device tree concatenated with the

> >     > u-boot binary. The weak function would cater to all of those

> platforms

> >     > -- having a weak function would mean that we are not required to

> >     repeat

> >     > the same functionality for every platform that uses the same

> mechanism

> >     > for extracting the public key. However, there would be platforms

> where

> >     > the public key is obtained through a different mechanism which is

> >     > platform specific. Those platforms would have to define their own

> >     > function to get the public key. Same for checking whether capsule

> >     > authentication feature has been enabled or not.

> >     >

> >     > -sughosh

> >

> >     Hello Sughosh,

> >

> >     Could you, please, explain why there could be a need to use public

> keys

> >     for capsule verification that are not compiled into U-Boot. I cannot

> see

> >     how this would increase security.

> >

> >

> > With the changes that have been made in the Makefile(patch 5/5), the

> > public key is now embedded into the platform's dtb, and subsequently

> > this dtb is concatenated with the u-boot binary to create a single

> > u-boot.bin image. This image can then be verified during the trusted

> > boot flow to check against any kind of tampering. This takes care of

> > your concern of not having the public key separately on the disk, which

> > makes it open to tampering, with the public key now embedded as part of

> > the u-boot image. You had suggested embedding the public key as part of

> > the u-boot image. I have embedded it within the platform's dtb which is

> > part of the u-boot image. This becomes a generic solution which is

> > platform and architecture agnostic. I believe concatenating the

> > platform's dtb with the u-boot binary is the standard flow for

> > production images.

>

> Embedding the key in the device-tree is fine. I am just trying to

> understand why you need the extensibility via a weak function.

>


This is to provide flexibility for any platform that might have a different
mechanism of passing/retrieving the public key. Some platforms might have
their dtb passed from an earlier stage firmware, like tf-a. Or there could
be a read-only device like a fuse which houses the keys to be used. Having
a weak default would allow such platforms to implement a platform specific
function to retrieve the public key. So if there is no technical
disadvantage of having a weak default, I think keeping this flexibility for
platforms would be good.


>

> >

> >     I cannot imagine any scenario where you would want to allow switching

> >     off capsule authentication if it has been built into U-Boot.

> >

> >

> > This is only an additional knob for any user who might want to perform a

> > capsule update without authentication -- with this additional knob, the

> > user can use the same image for updating a capsule which does not have

> > an authentication header. The user would not be required to recompile

> > the image to turn off CONFIG_EFI_CAPSULE_AUTHENTICATE config option. But

> > if you don't see this necessary, i can remove this additional check. In

> > that case, the capsule will be authenticated when

> > CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled.

>

> I would prefer to reduce the number of "knobs" that you have to check

> when rolling out secure firmware.

>


Okay. I will remove this extra check in the next version. Whether the
platform authenticates the capsule or not would then depend solely on the
config option.

-sughosh
Simon Glass April 8, 2021, 11:55 p.m. | #7
Hi Sughosh,

On Thu, 8 Apr 2021 at 18:53, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>

> hi Simon,

>

> On Wed, 7 Apr 2021 at 21:44, Simon Glass <sjg@chromium.org> wrote:

>>

>> Hi,

>>

>> On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:

>> >

>> > Patch 1 fixes an issue of selection of IMAGE_SIGN_INFO config option

>> > when capsule authentication is enabled.

>> >

>> > Patch 2 add two config symbols, EFI_PKEY_DTB_EMBED and EFI_PKEY_FILE

>> > which are used for enabling embedding of the public key in the dtb,

>> > and specifying the esl file name.

>> >

>> > Patch 3 moves efi_capsule_auth_enabled as a weak function, which can

>> > be used as a default mechanism for checking if capsule authentication

>> > has been enabled.

>> >

>> > Patch 4 adds a default weak function for retrieving the public key

>> > from the platform's dtb.

>> >

>> > Patch 5 adds the functionality to embed the esl file into the

>> > platform's dtb during the platform build.

>> >

>> > I have tested this functionality on the STM32MP157C DK2 board.

>> >

>> > [1] - https://lists.denx.de/pipermail/u-boot/2021-March/442867.html

>> >

>> > Sughosh Ganu (5):

>> >   efi_loader: Kconfig: Select IMAGE_SIGN_INFO when capsule

>> >     authentication is enabled

>> >   efi_loader: Kconfig: Add symbols for embedding the public key into the

>> >     platform's dtb

>> >   efi_capsule: Add a weak function to check whether capsule

>> >     authentication is enabled

>> >   efi_capsule: Add a weak function to get the public key needed for

>> >     capsule authentication

>> >   Makefile: Add provision for embedding public key in platform's dtb

>> >

>> >  Makefile                              | 10 ++++++

>> >  board/emulation/common/qemu_capsule.c |  6 ----

>> >  lib/efi_loader/Kconfig                | 16 ++++++++++

>> >  lib/efi_loader/efi_capsule.c          | 44 ++++++++++++++++++++++++---

>> >  4 files changed, 66 insertions(+), 10 deletions(-)

>> >

>> > --

>> > 2.17.1

>> >

>>

>> We need to rethink the use of weak functions for this sort of thing,

>> or we will end up with an unnavigable mess at some point. If we need

>> to adjust the flow of boot, let's adjust the flow rather than adding

>> hooks everywhere.

>

>

> There are two weak functions being added. One is for retrieving the public key to be used for the capsule authentication, and the other is for checking for whether capsule authentication has been enabled. The reason why a weak function is needed is because platforms can have other mechanisms for retrieval of the public key or for testing if capsule authentication has been enabled.

>

> If we consider the case of public key retrieval, the majority of platforms would be built with the device tree concatenated with the u-boot binary. The weak function would cater to all of those platforms -- having a weak function would mean that we are not required to repeat the same functionality for every platform that uses the same mechanism for extracting the public key. However, there would be platforms where the public key is obtained through a different mechanism which is platform specific. Those platforms would have to define their own function to get the public key. Same for checking whether capsule authentication feature has been enabled or not.

>

> -sughosh


Yes, I get it, but if this is to be a critical feature and part of the
grand new design for verified boot using UEFI, surely we should be
building a new front door, not digging a tunnel in the bathroom :-)

We can either use drivers with driver model, or perhaps have a Kconfig
that enables calling the function (so we get a link error if not
provided). Or if there will be more than one handler, a linker_list.

Perhaps it is time to consider a 'hook' system, with a command to let
us see what hooks are active for any particular event?

Regards,
Simon
Sughosh Ganu April 9, 2021, 11:27 a.m. | #8
hi Simon,

On Fri, 9 Apr 2021 at 05:26, Simon Glass <sjg@chromium.org> wrote:

> Hi Sughosh,

>

> On Thu, 8 Apr 2021 at 18:53, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:

> >

> > hi Simon,

> >

> > On Wed, 7 Apr 2021 at 21:44, Simon Glass <sjg@chromium.org> wrote:

> >>

> >> Hi,

> >>

> >> On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu <sughosh.ganu@linaro.org>

> wrote:

> >> >

> >> > Patch 1 fixes an issue of selection of IMAGE_SIGN_INFO config option

> >> > when capsule authentication is enabled.

> >> >

> >> > Patch 2 add two config symbols, EFI_PKEY_DTB_EMBED and EFI_PKEY_FILE

> >> > which are used for enabling embedding of the public key in the dtb,

> >> > and specifying the esl file name.

> >> >

> >> > Patch 3 moves efi_capsule_auth_enabled as a weak function, which can

> >> > be used as a default mechanism for checking if capsule authentication

> >> > has been enabled.

> >> >

> >> > Patch 4 adds a default weak function for retrieving the public key

> >> > from the platform's dtb.

> >> >

> >> > Patch 5 adds the functionality to embed the esl file into the

> >> > platform's dtb during the platform build.

> >> >

> >> > I have tested this functionality on the STM32MP157C DK2 board.

> >> >

> >> > [1] - https://lists.denx.de/pipermail/u-boot/2021-March/442867.html

> >> >

> >> > Sughosh Ganu (5):

> >> >   efi_loader: Kconfig: Select IMAGE_SIGN_INFO when capsule

> >> >     authentication is enabled

> >> >   efi_loader: Kconfig: Add symbols for embedding the public key into

> the

> >> >     platform's dtb

> >> >   efi_capsule: Add a weak function to check whether capsule

> >> >     authentication is enabled

> >> >   efi_capsule: Add a weak function to get the public key needed for

> >> >     capsule authentication

> >> >   Makefile: Add provision for embedding public key in platform's dtb

> >> >

> >> >  Makefile                              | 10 ++++++

> >> >  board/emulation/common/qemu_capsule.c |  6 ----

> >> >  lib/efi_loader/Kconfig                | 16 ++++++++++

> >> >  lib/efi_loader/efi_capsule.c          | 44

> ++++++++++++++++++++++++---

> >> >  4 files changed, 66 insertions(+), 10 deletions(-)

> >> >

> >> > --

> >> > 2.17.1

> >> >

> >>

> >> We need to rethink the use of weak functions for this sort of thing,

> >> or we will end up with an unnavigable mess at some point. If we need

> >> to adjust the flow of boot, let's adjust the flow rather than adding

> >> hooks everywhere.

> >

> >

> > There are two weak functions being added. One is for retrieving the

> public key to be used for the capsule authentication, and the other is for

> checking for whether capsule authentication has been enabled. The reason

> why a weak function is needed is because platforms can have other

> mechanisms for retrieval of the public key or for testing if capsule

> authentication has been enabled.

> >

> > If we consider the case of public key retrieval, the majority of

> platforms would be built with the device tree concatenated with the u-boot

> binary. The weak function would cater to all of those platforms -- having a

> weak function would mean that we are not required to repeat the same

> functionality for every platform that uses the same mechanism for

> extracting the public key. However, there would be platforms where the

> public key is obtained through a different mechanism which is platform

> specific. Those platforms would have to define their own function to get

> the public key. Same for checking whether capsule authentication feature

> has been enabled or not.

> >

> > -sughosh

>

> Yes, I get it, but if this is to be a critical feature and part of the

> grand new design for verified boot using UEFI, surely we should be

> building a new front door, not digging a tunnel in the bathroom :-)

>

> We can either use drivers with driver model, or perhaps have a Kconfig

> that enables calling the function (so we get a link error if not

> provided). Or if there will be more than one handler, a linker_list.

>


I will use the Kconfig symbol for selecting the function to use for
retrieving the public key. So, in the current scenario, the function would
be under the CONFIG_EFI_PKEY_DTB_EMBED symbol. This should cater to the
majority of the cases. If some platform wants to use a different method to
get the public key, it would need to define it's own function.

-sughosh


>

> Perhaps it is time to consider a 'hook' system, with a command to let

> us see what hooks are active for any particular event?

>

> Regards,

> Simon

>
Simon Glass April 9, 2021, 8:58 p.m. | #9
Hi Sughosh,

On Fri, 9 Apr 2021 at 23:27, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>

> hi Simon,

>

> On Fri, 9 Apr 2021 at 05:26, Simon Glass <sjg@chromium.org> wrote:

>>

>> Hi Sughosh,

>>

>> On Thu, 8 Apr 2021 at 18:53, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:

>> >

>> > hi Simon,

>> >

>> > On Wed, 7 Apr 2021 at 21:44, Simon Glass <sjg@chromium.org> wrote:

>> >>

>> >> Hi,

>> >>

>> >> On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:

>> >> >

>> >> > Patch 1 fixes an issue of selection of IMAGE_SIGN_INFO config option

>> >> > when capsule authentication is enabled.

>> >> >

>> >> > Patch 2 add two config symbols, EFI_PKEY_DTB_EMBED and EFI_PKEY_FILE

>> >> > which are used for enabling embedding of the public key in the dtb,

>> >> > and specifying the esl file name.

>> >> >

>> >> > Patch 3 moves efi_capsule_auth_enabled as a weak function, which can

>> >> > be used as a default mechanism for checking if capsule authentication

>> >> > has been enabled.

>> >> >

>> >> > Patch 4 adds a default weak function for retrieving the public key

>> >> > from the platform's dtb.

>> >> >

>> >> > Patch 5 adds the functionality to embed the esl file into the

>> >> > platform's dtb during the platform build.

>> >> >

>> >> > I have tested this functionality on the STM32MP157C DK2 board.

>> >> >

>> >> > [1] - https://lists.denx.de/pipermail/u-boot/2021-March/442867.html

>> >> >

>> >> > Sughosh Ganu (5):

>> >> >   efi_loader: Kconfig: Select IMAGE_SIGN_INFO when capsule

>> >> >     authentication is enabled

>> >> >   efi_loader: Kconfig: Add symbols for embedding the public key into the

>> >> >     platform's dtb

>> >> >   efi_capsule: Add a weak function to check whether capsule

>> >> >     authentication is enabled

>> >> >   efi_capsule: Add a weak function to get the public key needed for

>> >> >     capsule authentication

>> >> >   Makefile: Add provision for embedding public key in platform's dtb

>> >> >

>> >> >  Makefile                              | 10 ++++++

>> >> >  board/emulation/common/qemu_capsule.c |  6 ----

>> >> >  lib/efi_loader/Kconfig                | 16 ++++++++++

>> >> >  lib/efi_loader/efi_capsule.c          | 44 ++++++++++++++++++++++++---

>> >> >  4 files changed, 66 insertions(+), 10 deletions(-)

>> >> >

>> >> > --

>> >> > 2.17.1

>> >> >

>> >>

>> >> We need to rethink the use of weak functions for this sort of thing,

>> >> or we will end up with an unnavigable mess at some point. If we need

>> >> to adjust the flow of boot, let's adjust the flow rather than adding

>> >> hooks everywhere.

>> >

>> >

>> > There are two weak functions being added. One is for retrieving the public key to be used for the capsule authentication, and the other is for checking for whether capsule authentication has been enabled. The reason why a weak function is needed is because platforms can have other mechanisms for retrieval of the public key or for testing if capsule authentication has been enabled.

>> >

>> > If we consider the case of public key retrieval, the majority of platforms would be built with the device tree concatenated with the u-boot binary. The weak function would cater to all of those platforms -- having a weak function would mean that we are not required to repeat the same functionality for every platform that uses the same mechanism for extracting the public key. However, there would be platforms where the public key is obtained through a different mechanism which is platform specific. Those platforms would have to define their own function to get the public key. Same for checking whether capsule authentication feature has been enabled or not.

>> >

>> > -sughosh

>>

>> Yes, I get it, but if this is to be a critical feature and part of the

>> grand new design for verified boot using UEFI, surely we should be

>> building a new front door, not digging a tunnel in the bathroom :-)

>>

>> We can either use drivers with driver model, or perhaps have a Kconfig

>> that enables calling the function (so we get a link error if not

>> provided). Or if there will be more than one handler, a linker_list.

>

>

> I will use the Kconfig symbol for selecting the function to use for retrieving the public key. So, in the current scenario, the function would be under the CONFIG_EFI_PKEY_DTB_EMBED symbol. This should cater to the majority of the cases. If some platform wants to use a different method to get the public key, it would need to define it's own function.


I wonder why another method would be needed, but if it, then someone
can turn your thing into a choice, I suppose.

Regards,
Simon