Re: [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.muru...@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;
  }