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

[Jon Humphreys]

Ilias Apalodimas <ilias.apalodimas at linaro.org> writes:

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

Hi Ilias.

The Kconfig option is not there to ensure a capsule key exists, but rather
to make sure a developer, that is porting uboot to their board and is
starting from a reference board in upstream, doesn't continue using the
development board dummy key and instead creates their own (private one).

The Kconfig option is used to force a board developer to acknowledge they
have followed the board porting steps (which talks about creating your own
key) with an explict action of disabling the config to remove the warning
message.

Hope that helps.

Jon

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