Please ignore this patch as I mistakenly sent out v1.
-Takahiro Akashi
On Tue, Jul 20, 2021 at 02:44:42PM +0900, AKASHI Takahiro wrote:
> If capsule authentication is disabled and yet a capsule file is signed,
> its signature must be removed from image data to flush.
> Otherwise, the firmware will be corrupted after update.
>
> Fixes: 04be98bd6bcf ("efi: capsule: Add support for uefi capsule
> authentication")
> Signed-off-by: AKASHI Takahiro <[email protected]>
> ---
> lib/efi_loader/efi_capsule.c | 70 +++++++++++++++++++++++++++++-------
> 1 file changed, 57 insertions(+), 13 deletions(-)
>
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index b0dffd3ac9ce..5d156c730faa 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -206,6 +206,39 @@ skip:
> return NULL;
> }
>
> +/**
> + * efi_remove_auth_hdr - remove authentication data from image
> + * @image: Pointer to pointer to Image
> + * @image_size: Pointer to Image size
> + *
> + * Remove the authentication data from image if possible.
> + * Update @image and @image_size.
> + *
> + * Return: status code
> + */
> +static efi_status_t efi_remove_auth_hdr(void **image, efi_uintn_t
> *image_size)
> +{
> + struct efi_firmware_image_authentication *auth_hdr;
> + efi_status_t ret = EFI_INVALID_PARAMETER;
> +
> + auth_hdr = (struct efi_firmware_image_authentication *)*image;
> + if (*image_size < sizeof(*auth_hdr))
> + goto out;
> +
> + if (auth_hdr->auth_info.hdr.dwLength <=
> + offsetof(struct win_certificate_uefi_guid, cert_data))
> + goto out;
> +
> + *image = (uint8_t *)*image + sizeof(auth_hdr->monotonic_count) +
> + auth_hdr->auth_info.hdr.dwLength;
> + *image_size = *image_size - auth_hdr->auth_info.hdr.dwLength -
> + sizeof(auth_hdr->monotonic_count);
> +
> + ret = EFI_SUCCESS;
> +out:
> + return ret;
> +}
> +
> #if defined(CONFIG_EFI_CAPSULE_AUTHENTICATE)
>
> #if defined(CONFIG_EFI_PKEY_DTB_EMBED)
> @@ -271,21 +304,15 @@ efi_status_t efi_capsule_authenticate(const void
> *capsule, efi_uintn_t capsule_s
> if (capsule == NULL || capsule_size == 0)
> goto out;
>
> - auth_hdr = (struct efi_firmware_image_authentication *)capsule;
> - if (capsule_size < sizeof(*auth_hdr))
> - goto out;
> -
> - if (auth_hdr->auth_info.hdr.dwLength <=
> - offsetof(struct win_certificate_uefi_guid, cert_data))
> + *image = (uint8_t *)capsule;
> + *image_size = capsule_size;
> + if (efi_remove_auth_hdr(image, image_size) != EFI_SUCCESS)
> goto out;
>
> + auth_hdr = (struct efi_firmware_image_authentication *)capsule;
> if (guidcmp(&auth_hdr->auth_info.cert_type, &efi_guid_cert_type_pkcs7))
> goto out;
>
> - *image = (uint8_t *)capsule + sizeof(auth_hdr->monotonic_count) +
> - auth_hdr->auth_info.hdr.dwLength;
> - *image_size = capsule_size - auth_hdr->auth_info.hdr.dwLength -
> - sizeof(auth_hdr->monotonic_count);
> memcpy(&monotonic_count, &auth_hdr->monotonic_count,
> sizeof(monotonic_count));
>
> @@ -367,7 +394,7 @@ static efi_status_t efi_capsule_update_firmware(
> {
> struct efi_firmware_management_capsule_header *capsule;
> struct efi_firmware_management_capsule_image_header *image;
> - size_t capsule_size;
> + size_t capsule_size, image_binary_size;
> void *image_binary, *vendor_code;
> efi_handle_t *handles;
> efi_uintn_t no_handles;
> @@ -429,13 +456,30 @@ static efi_status_t efi_capsule_update_firmware(
> }
>
> /* do update */
> + if (IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE) &&
> + !(image->image_capsule_support &
> + CAPSULE_SUPPORT_AUTHENTICATION)) {
> + /* no signature */
> + ret = EFI_SECURITY_VIOLATION;
> + goto out;
> + }
> +
> image_binary = (void *)image + sizeof(*image);
> - vendor_code = image_binary + image->update_image_size;
> + image_binary_size = image->update_image_size;
> + vendor_code = image_binary + image_binary_size;
> + if (!IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE) &&
> + (image->image_capsule_support &
> + CAPSULE_SUPPORT_AUTHENTICATION)) {
> + ret = efi_remove_auth_hdr(&image_binary,
> + &image_binary_size);
> + if (ret != EFI_SUCCESS)
> + goto out;
> + }
>
> abort_reason = NULL;
> ret = EFI_CALL(fmp->set_image(fmp, image->update_image_index,
> image_binary,
> - image->update_image_size,
> + image_binary_size,
> vendor_code, NULL,
> &abort_reason));
> if (ret != EFI_SUCCESS) {
> --
> 2.31.0
>
