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

[Aswin Murugan]

On 7/25/2025 6:56 PM, Heinrich Schuchardt wrote:
> 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?
> In the current implementation of efi_signature_store()
> https://source.denx.de/u-boot/u-boot/-/blob/master/lib/efi_loader/efi_signature.c#L809,
> the db variable is retrieved using efi_get_var() and passed directly to
> efi_build_signature_store(). However, for authenticated variables, the 
> data
> returned by efi_get_var() includes the authentication header.
> efi_build_signature_store() expects the data pointer to begin at the start
> of the EFI Signature List, not the authentication header. This 
> mismatch leads
> to a parsing error, as the header is misinterpreted as part of the 
> signature
> list, resulting in an invalid signature list format.
>
> How can the error be reproduced?
> Created the ubootefi.var through tools/efivar.py and enabled 
> EFI_VARIABLES_PRESEED for secure boot,
> during image authentication, wrong siglist format error was observed.
>
>> 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?
> I belive that stripping the auth header during variable read might not 
> violate the spec.
>
> Is tests/test_efi_secboot/test_authvar.py still passing with your patch?
> Yes, it passed
>>
>> 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;
>>   }
>