| Message ID | 20230519103214.1239656-6-masahisa.kojima@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | FMP versioning support | expand |
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@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? > + 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
On Tue, 23 May 2023 at 06:36, Ilias Apalodimas <ilias.apalodimas@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@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
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) { + 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; }
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@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(-)