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

Sun Jun 16 10:35:38 CEST 2024

On 6/13/24 21:04, Jon Humphreys wrote:
> 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");

For warnings we use log_warning().

A warning might be adequate if capsules signatures were not checked
(EFI_CAPSULE_AUTHENTICATE=n). We don't need a new Kconfig symbol here.
Most users will not remark this message as it scrolls away when rebooting.

In some cases it might make sense to require a manual confirmation
before applying an unsigned capsule.

Please, consider that capsule authentication only provides extra
security if the same changes could not be done directly from the OS.

Why would you show the new messages on all existing boards supporting
capsule updates?

corstone1000_defconfig
imx8mm-cl-iot-gate-optee_defconfig
imx8mm-cl-iot-gate_defconfig
imx8mp_rsb3720a1_4G_defconfig
imx8mp_rsb3720a1_6G_defconfig
kontron-sl-mx8mm_defconfig
rock-4c-plus-rk3399_defconfig
rock-4se-rk3399_defconfig
rock-pi-4-rk3399_defconfig
rock-pi-4c-rk3399_defconfig
sandbox_defconfig
sandbox_flattree_defconfig
synquacer_developerbox_defconfig
xilinx_zynqmp_kria_defconfig
xilinx_zynqmp_virt_defconfig

>>> +               log_info("WARNING: Please check https://docs.u-boot.org/en/latest/develop/uefi/uefi.html#porting-capsule-update-to-new-boards\n");

We can expect a developer to know where to find documentation.

Best regards

Heinrich

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