Re: [PATCH v2] efi_loader: Remove authentication header in GetVariable()

[Heinrich Schuchardt]

Am 14. Mai 2026 13:01:08 MESZ schrieb Aswin Murugan <aswin.murugan at oss.qualcomm.com>:
>Hi Ilias,
>
>We are attempting to pre-seed ubootefi.var created using the *efivar.py* tool for authenticated EFI variables.
>Currently, efivar.py writes variables including the full authentication descriptor (timestamp + WIN_CERTIFICATE + payload) into ubootefi.var.

For authorization timestamps we have a field in the variable header in the file format.

We should verify that SetVariable only sets the header field and strips the authentication header to keep the efivar.py and SetVariable in sync.

I think this is done here
https://github.com/trini/u-boot/blob/36d4c653580824b16574560b21d4401614d8b68e/lib/efi_loader/efi_variable.c#L88

Do you intend to provide a patch for efivar.py?

Best regards

Heinrich

>
>As per the CI tests shared, it stores the authenticated EFI variables using setenv
>When authenticated variables are set via U‑Boot using setenv -e -at, the flow goes through SetVariable(), where the firmware:
>
> * parses and validates the authentication descriptor
> * removes the authentication header
> * stores only the variable payload
>
>
>Due to this mismatch, variables created with efivar.py differ from those created via U‑Boot:
>
>GetVariable() returns descriptor + payload instead of payload-only, this leads to inconsistent and non‑compliant variable behavior
>
>Fix Proposed:
>
>Strip the authentication descriptor within efivar.py during variable write, so that only the payload is stored in ubootefi.var, matching U‑Boot SetVariable() behavior. Please confirm if there are any concerns with this approach.
>
>Regards,
>Aswin
>
>On 5/13/2026 11:54 AM, Ilias Apalodimas wrote:
>> Hi Aswin,
>> 
>> On Tue, 12 May 2026 at 22:42, Aswin Murugan
>> <aswin.murugan at oss.qualcomm.com> wrote:
>>> Currently, efi_get_variable_mem() returns the raw stored format for
>>> authenticated variables, which includes the EFI_VARIABLE_AUTHENTICATION_2
>>> descriptor . This causes image authentication failures when reading
>>> secure boot variables as callers receive wrong data containing
>>> authentication headers instead of the actual variable payload.
>> What are 'the callers'? The EFI security related variables are usually
>> read and consumed by the firmware itself during secure boot. I don't
>> remember the code on top of my head, but i think we end up storing the
>> auth info because it also contains timestamps etc.
>> 
>>> The UEFI Specification Section 8.2.6(EFI_VARIABLE_AUTHENTICATION_2 descriptor)
>>> explicitly states:
>>> "The authentication descriptor is not part of the variable data and is
>>> not returned by subsequent calls to GetVariable()."
>> We can strip the headers during GetVariable, but you need more changes
>> apart from the one here. The EFI secure boot tests fail with the
>> change above [1].
>> 
>>> This patch implements spec-compliant behavior by detecting variables with
>>> the EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS attribute and
>>> stripping the authentication descriptor before returning data to callers.
>> [1] https://source.denx.de/u-boot/custodians/u-boot-tpm/-/pipelines/30086
>> 
>> Regards
>> /Ilias
>>> Signed-off-by: Aswin Murugan <aswin.murugan at oss.qualcomm.com>
>>> ---
>>> Changes in v2:
>>> - Enhanced commit message with explicit UEFI spec reference
>>> Link to v1: https://lore.kernel.org/u-boot/20250725123653.513252-1-aswin.murugan@oss.qualcomm.com/
>>> ---
>>>   lib/efi_loader/efi_var_mem.c | 44 +++++++++++++++++++++++++++++-------
>>>   1 file changed, 36 insertions(+), 8 deletions(-)
>>> 
>>> diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c
>>> index 8d5f99f4870..edf4522e51f 100644
>>> --- a/lib/efi_loader/efi_var_mem.c
>>> +++ b/lib/efi_loader/efi_var_mem.c
>>> @@ -336,9 +336,10 @@ 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;
>>>          struct efi_var_entry *var;
>>>          u16 *pdata;
>>> +       void *var_data;
>>> 
>>>          if (!variable_name || !vendor || !data_size)
>>>                  return EFI_INVALID_PARAMETER;
>>> @@ -362,19 +363,46 @@ 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)) {
>>> +                       struct efi_time *timestamp;
>>> +                       struct win_certificate_uefi_guid *cert;
>>> +                       efi_uintn_t auth_size;
>>> +
>>> +                       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;
>>>   }
>>> --
>>> 2.34.1
>>>