[PATCH v6 5/8] efi_loader: check lowest supported version

[Masahisa Kojima]

On Tue, 23 May 2023 at 06:36, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> On Fri, May 19, 2023 at 07:32:11PM +0900, Masahisa Kojima wrote:
> > The FMP Payload Header which EDK II capsule generation scripts
> > insert has a firmware version.
> > This commit reads the lowest supported version stored in the
> > device tree, then check if the firmware version in FMP payload header
> > of the ongoing capsule is equal or greater than the
> > lowest supported version. If the firmware version is lower than
> > lowest supported version, capsule update will not be performed.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
> > ---
> > Changes in v6:
> > - get aligned to the latest implementation
> >
> > Changes in v5:
> > - newly implement the device tree based versioning
> >
> > Changes in v4:
> > - use log_err() instead of printf()
> >
> > Changes in v2:
> > - add error message when the firmware version is lower than
> >   lowest supported version
> >
> >  lib/efi_loader/efi_firmware.c | 19 ++++++++++++++++++-
> >  1 file changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> > index 00cf9a088a..7cd0016765 100644
> > --- a/lib/efi_loader/efi_firmware.c
> > +++ b/lib/efi_loader/efi_firmware.c
> > @@ -424,7 +424,8 @@ static void efi_firmware_get_fw_version(const void **p_image,
> >   * @image_index              Image index
> >   * @state            Pointer to fmp state
> >   *
> > - * Verify the capsule file
> > + * Verify the capsule authentication and check if the fw_version
> > + * is equal or greater than the lowest supported version.
> >   *
> >   * Return:           status code
> >   */
> > @@ -434,11 +435,27 @@ efi_status_t efi_firmware_verify_image(const void **p_image,
> >                                      u8 image_index,
> >                                      struct fmp_state *state)
> >  {
> > +     u32 lsv;
> >       efi_status_t ret;
> > +     efi_guid_t *image_type_id;
> >
> >       ret = efi_firmware_capsule_authenticate(p_image, p_image_size);
> >       efi_firmware_get_fw_version(p_image, p_image_size, state);
> >
> > +     /* check lowest_supported_version if capsule authentication passes */
> > +     if (ret == EFI_SUCCESS) {
>
> What's the point of this here?  Can;'t we move this check right after
> efi_firmware_capsule_authenticate() and return a security violation if that
> failed?

Yes, I agree.

Thanks,
Masahisa Kojima

>
> > +             image_type_id = efi_firmware_get_image_type_id(image_index);
> > +             if (!image_type_id)
> > +                     return EFI_INVALID_PARAMETER;
> > +
> > +             efi_firmware_get_lsv_from_dtb(image_index, image_type_id, &lsv);
> > +             if (state->fw_version < lsv) {
> > +                     log_err("Firmware version %u too low. Expecting >= %u. Aborting update\n",
> > +                             state->fw_version, lsv);
> > +                     return EFI_INVALID_PARAMETER;
> > +             }
> > +     }
> > +
> >       return ret;
> >  }
> >
> > --
> > 2.17.1
> >
>
> Thanks
> /Ilias