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.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;
}
