[PATCH] efi: capsule: Improve capsule update porting to new boards

[Ilias Apalodimas]

Hi Jon,

On Sat, 8 Jun 2024 at 02:44, Jonathan Humphreys <j-humphreys at ti.com> wrote:
>
> Add to the capsule update porting documentation that when capsule
> authentication is enabled (via EFI_CAPSULE_AUTHENTICATE), a public key
> certificate corresponding to the capsule signing key must be specified.
>
> Also add a config (EFI_CAPSULE_BOARD_INSECURE) that is set by default and
> causes a warning (info level) alerting a developer to consult this porting
> section to ensure the capsule update capability is secure for the platform.
> Once this section has been reviewed and confidence is established in the
> security of the capsule update implementation for this board, deselecting
> the EFI_CAPSULE_BOARD_INSECURE config will remove the warning.
>
> This config is to balance the desire to provide a reference implementation
> in upstream for capsule authentication but also provide some safeguards
> to prevent developers from overlooking the need to properly secure their
> implementation.
>
> Signed-off-by: Jonathan Humphreys <j-humphreys at ti.com>
> ---
>  doc/develop/uefi/uefi.rst    | 30 +++++++++++++++++++++++-------
>  lib/efi_loader/Kconfig       | 13 +++++++++++++
>  lib/efi_loader/efi_capsule.c |  5 +++++
>  3 files changed, 41 insertions(+), 7 deletions(-)
>
> diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
> index 985956ab85c..1c98c6e4ab4 100644
> --- a/doc/develop/uefi/uefi.rst
> +++ b/doc/develop/uefi/uefi.rst
> @@ -526,6 +526,7 @@ following command can be issued
>        --guid c1b629f1-ce0e-4894-82bf-f0a38387e630 \
>        optee.bin optee.capsule
>
> +.. _Enabling Capsule Authentication:
>
>  Enabling Capsule Authentication
>  *******************************
> @@ -631,17 +632,32 @@ where version.dtso looks like::
>  The properties of image-type-id and image-index must match the value
>  defined in the efi_fw_image array as image_type_id and image_index.
>
> -Porting Capsule Updates to new boards
> -*************************************
> +Porting Capsule Update to new boards
> +************************************
>
>  It is important, when using a reference board as a starting point for a custom
>  board, that certain steps are taken to properly support Capsule Updates.
>
> -Capsule GUIDs need to be unique for each firmware and board. That is, if two
> -firmwares are built from the same source but result in different binaries
> -because they are built for different boards, they should have different GUIDs.
> -Therefore it is important when creating support for a new board, new GUIDs are
> -defined in the board's header file.  *DO NOT* reuse capsule GUIDs.
> +Capsule GUIDs need to be unique for each firmware and board. That is, even
> +if two firmwares are built from the same source but result in different
> +binaries because they are built for different boards, they should have
> +different GUIDs. Therefore it is important when creating support for a new
> +board, new GUIDs are defined in the board's header file. *DO NOT* reuse
> +capsule GUIDs.
> +
> +When capsule authentication is enabled (via EFI_CAPSULE_AUTHENTICATE), it
> +is also very important to set the public key certificate corresponding to
> +the capsule signing key. See :ref:`Enabling Capsule Authentication` for
> +more information on signing capsules. *DO NOT* reuse the certificate file
> +from the reference board, which usually would be based on the development
> +board keys.
> +
> +By default, the EFI_CAPSULE_BOARD_INSECURE config is set to true and causes
> +a warning (info level) alerting a developer to consult this section to
> +ensure the capsule update capability is secure for the platform. Once this
> +section has been reviewed and confidence is established in the security of
> +the capsule update implementation for this board, deselecting
> +EFI_CAPSULE_BOARD_INSECURE config will remove the warning.
>
>  Executing the boot manager
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 23079a5709d..f4b1a20ee40 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -236,6 +236,19 @@ config EFI_CAPSULE_ON_DISK_EARLY
>           executed as part of U-Boot initialisation so that they will
>           surely take place whatever is set to distro_bootcmd.
>
> +config EFI_CAPSULE_BOARD_INSECURE
> +       bool "Permit insecure capsule-on-disk support"
> +       depends on EFI_HAVE_CAPSULE_SUPPORT
> +       default y
> +       help
> +         Permit insecure capsule update support and emit a warning (info level)
> +         when applying capsule updates, indicating that the configuration is
> +         insecure and including a link to the capsule update porting guide in
> +         U-Boot docs. Disabling this option removes the warning. A developer
> +         configuration doesn't need to be secure, but before making a product
> +         from the configuration, the intent is to developers alert of steps that
> +         must be taken.
> +
>  config EFI_CAPSULE_FIRMWARE
>         bool
>
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index 0937800e588..76ade2809d9 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -1329,6 +1329,11 @@ efi_status_t efi_launch_capsules(void)
>         if (!nfiles)
>                 return EFI_SUCCESS;
>
> +       if (IS_ENABLED(CONFIG_EFI_CAPSULE_BOARD_INSECURE)) {
> +               log_info("WARNING: EFI Capsules might be insecure!\n");
> +               log_info("WARNING: Please check https://docs.u-boot.org/en/latest/develop/uefi/uefi.html#porting-capsule-update-to-new-boards\n");
> +       }

I am fine with adding a warning on this, but why do we have to control
it via a Kconfig option? We can detect the capsule-key is missing from
the DT and print the same warning.

Regards
/Ilias
> +
>         /* Launch capsules */
>         for (i = 0, ++index; i < nfiles; i++, index++) {
>                 log_debug("Applying %ls\n", files[i]);
> --
> 2.34.1
>