Re: [PATCH v6 5/8] efi_loader: check lowest supported version
On Tue, 23 May 2023 at 06:36, Ilias Apalodimas
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
> > ---
> > 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
> > * @statePointer 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,
> > );
> > + 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
Re: [PATCH v6 5/8] efi_loader: check lowest supported version
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
> ---
> 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
> * @statePointer 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?
> + 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, );
> + 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
[PATCH v6 5/8] efi_loader: check lowest supported version
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
---
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_indexImage 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) {
+ 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, );
+ 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
