[PATCH v2 1/4] efi_loader: capsule: Remove the check for capsule_authentication_enabled environment variable

[AKASHI Takahiro]

On Mon, Apr 12, 2021 at 08:35:23PM +0530, Sughosh Ganu wrote:
> The current capsule authentication code checks if the environment
> variable capsule_authentication_enabled is set, for authenticating the
> capsule. This is in addition to the check for the config symbol
> CONFIG_EFI_CAPSULE_AUTHENTICATE. Remove the check for the environment
> variable. The capsule will now be authenticated if the config symbol
> is set.
> 
> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> ---
> 
> Changes since V1:
> * As pointed out by Heinrich in the review, remove the extra check of
>   the env variable 'capsule_authentication_enabled'for authenticating
>   the capsule. The capsule authentication will now be done based on
>   whether the corresponding config symbol is enabled.
> 
>  board/emulation/common/qemu_capsule.c | 6 ------
>  lib/efi_loader/efi_firmware.c         | 5 ++---
>  2 files changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/board/emulation/common/qemu_capsule.c b/board/emulation/common/qemu_capsule.c
> index 5cb461d52b..6b8a87022a 100644
> --- a/board/emulation/common/qemu_capsule.c
> +++ b/board/emulation/common/qemu_capsule.c
> @@ -41,9 +41,3 @@ int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
>  
>  	return 0;
>  }
> -
> -bool efi_capsule_auth_enabled(void)
> -{
> -	return env_get("capsule_authentication_enabled") != NULL ?
> -		true : false;
> -}
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index 7a3cca2793..a1b88dbfc2 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -190,7 +190,7 @@ static efi_status_t efi_get_dfu_info(
>  				IMAGE_ATTRIBUTE_IMAGE_UPDATABLE;
>  
>  		/* Check if the capsule authentication is enabled */
> -		if (env_get("capsule_authentication_enabled"))
> +		if (IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE))
>  			image_info[0].attributes_setting |=
>  				IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED;
>  
> @@ -421,8 +421,7 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
>  		return EFI_EXIT(EFI_INVALID_PARAMETER);
>  
>  	/* Authenticate the capsule if authentication enabled */
> -	if (IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE) &&
> -	    env_get("capsule_authentication_enabled")) {
> +	if (IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE)) {

This change is not enough;
1. When a *signed* capsule file is applied on U-Boot with
EFI_CAPSULE_AUTHENTICATE disabled, the "authenticode" data
must be excluded from the data to write.
In other words, the offset and the size in a capsule file, 
image & image_size, must also be updated before writing even
if the authentication is not performed.

Otherwise, wrong data will be stored.

2. UEFI specification requires that the authentication must be
performed only if IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED is
set on the image (image type or ESRT?).
We must always check with the attribute.

-Takahiro Akashi

>  		capsule_payload = NULL;
>  		capsule_payload_size = 0;
>  		status = efi_capsule_authenticate(image, image_size,
> -- 
> 2.17.1
>