[PATCH v1] efi_loader: Remove authentication header in efi variable data

[Heinrich Schuchardt]

On 25.07.25 14:36, Aswin Murugan wrote:
> efi_get_variable_mem() returned auth header along with ESL data for
> authenticated variables like PK, KEK, and DB. This caused issues in
> later stages where the auth header was misinterpreted as ESL data,

Which later stages are you relating to?
How can the error be reproduced?

> leading to parsing failures, the efi_get_variable_mem() is expected
> to return esl data alone.
> Added a check to remove the auth header if the variable has the
> EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS attribute. Now,
> only the ESL data is returned from the function.

Please, have a look at chapter 8.2.1 GetVariable in the UEFI spec.

When the attribute EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS is set, 
the Data buffer shall be interpreted as follows:
// NOTE: “||” indicates concatenation.
// Example: EFI_VARIABLE_AUTHENTICATION_3_TIMESTAMP_TYPE
EFI_VARIABLE_AUTHENTICATION_3 || EFI_TIME || 
EFI_VARIABLE_AUTHENTICATION_3_CERT_ID || Data
// Example: EFI_VARIABLE_AUTHENTICATION_3_NONCE_TYPE
EFI_VARIABLE_AUTHENTICATION_3 || EFI_VARIABLE_AUTHENTICATION_3_NONCE || 
EFI_VARIABLE_AUTHENTICATION_3_CERT_ID || Data

Isn't removing the authentication header violating the EFI specification?

Is tests/test_efi_secboot/test_authvar.py still passing with your patch?

> 
> Signed-off-by: Aswin Murugan <aswin.murugan at oss.qualcomm.com>
> ---
>   lib/efi_loader/efi_var_mem.c | 42 +++++++++++++++++++++++++++++-------
>   1 file changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c
> index 31180df9e3a..f73daf8bdc9 100644
> --- a/lib/efi_loader/efi_var_mem.c
> +++ b/lib/efi_loader/efi_var_mem.c
> @@ -309,9 +309,12 @@ efi_get_variable_mem(const u16 *variable_name, const efi_guid_t *vendor,
>   		     u32 *attributes, efi_uintn_t *data_size, void *data,
>   		     u64 *timep, u32 mask)
>   {
> -	efi_uintn_t old_size;
> +	efi_uintn_t old_size, var_data_size, auth_size;
>   	struct efi_var_entry *var;
>   	u16 *pdata;
> +	void *var_data;
> +	struct efi_time *timestamp;
> +	struct win_certificate_uefi_guid *cert;

Please, move variable definitions to the innermost code block where they 
are used.

Best regards

Heinrich

>   
>   	if (!variable_name || !vendor || !data_size)
>   		return EFI_INVALID_PARAMETER;
> @@ -335,19 +338,42 @@ efi_get_variable_mem(const u16 *variable_name, const efi_guid_t *vendor,
>   	if (!u16_strcmp(variable_name, vtf))
>   		return efi_var_collect_mem(data, data_size, EFI_VARIABLE_NON_VOLATILE);
>   
> +	for (pdata = var->name; *pdata; ++pdata)
> +		;
> +	++pdata;
> +
> +	var_data = (void *)pdata;
> +	var_data_size = var->length;
> +
> +	/* Handle EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS */
> +	if (var->attr & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) {
> +		if (var_data_size > sizeof(struct efi_time)) {
> +			timestamp = (struct efi_time *)var_data;
> +			cert = (struct win_certificate_uefi_guid *)(timestamp + 1);
> +			auth_size = sizeof(struct efi_time) + cert->hdr.dwLength;
> +			if (var_data_size > auth_size) {
> +				var_data = (u8 *)var_data + auth_size;
> +				var_data_size -= auth_size;
> +			}
> +		}
> +	}
> +	/*
> +	 * EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS type is old & not
> +	 * expected for auth variables
> +	 */
> +	else if (var->attr & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS)
> +		return EFI_INVALID_PARAMETER;
> +
>   	old_size = *data_size;
> -	*data_size = var->length;
> -	if (old_size < var->length)
> +	*data_size = var_data_size;
> +
> +	if (old_size < var_data_size)
>   		return EFI_BUFFER_TOO_SMALL;
>   
>   	if (!data)
>   		return EFI_INVALID_PARAMETER;
>   
> -	for (pdata = var->name; *pdata; ++pdata)
> -		;
> -	++pdata;
> -
> -	efi_memcpy_runtime(data, pdata, var->length);
> +	efi_memcpy_runtime(data, var_data, var_data_size);
>   
>   	return EFI_SUCCESS;
>   }