From e4c1c48eebd072b2fa6f27fd9b7baa731350ebe8 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Mon, 29 Jun 2020 11:49:58 +0200 Subject: efi_loader: fix incorrect use of EFI_EXIT() efi_get_variable_common() does not use EFI_ENTRY(). So we should not use EFI_EXIT() either. Fixes: 767f6eeb01d3 ("efi_loader: variable: support variable authentication") Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_variable.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/efi_loader/efi_variable.c') diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 6271dbcf41f..c262cb5972b 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -606,7 +606,7 @@ static efi_status_t efi_get_variable_common(u16 *variable_name, u32 attr; if (!variable_name || !vendor || !data_size) - return EFI_EXIT(EFI_INVALID_PARAMETER); + return EFI_INVALID_PARAMETER; ret = efi_to_native(&native_name, variable_name, vendor); if (ret) -- cgit v1.2.3 From ce3c3865b0c321116e59437abec96da3745ba619 Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Tue, 9 Jun 2020 14:09:34 +0900 Subject: efi_loader: variable: replace debug to EFI_PRINT Just for style consistency, replace all the uses of debug() to EFI_PRINT in efi_variable.c. Signed-off-by: AKASHI Takahiro Reviewed-by: Heinrich Schuchardt --- lib/efi_loader/efi_variable.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) (limited to 'lib/efi_loader/efi_variable.c') diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index c262cb5972b..74a9c65402a 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -243,7 +243,8 @@ static efi_status_t efi_transfer_secure_state(enum efi_secure_mode mode) { efi_status_t ret; - debug("Switching secure state from %d to %d\n", efi_secure_mode, mode); + EFI_PRINT("Switching secure state from %d to %d\n", efi_secure_mode, + mode); if (mode == EFI_MODE_DEPLOYED) { ret = efi_set_secure_state(1, 0, 0, 1); @@ -394,16 +395,16 @@ static struct pkcs7_message *efi_variable_parse_signature(const void *buf, * TODO: * The header should be composed in a more refined manner. */ - debug("Makeshift prefix added to authentication data\n"); + EFI_PRINT("Makeshift prefix added to authentication data\n"); ebuflen = sizeof(pkcs7_hdr) + buflen; if (ebuflen <= 0x7f) { - debug("Data is too short\n"); + EFI_PRINT("Data is too short\n"); return NULL; } ebuf = malloc(ebuflen); if (!ebuf) { - debug("Out of memory\n"); + EFI_PRINT("Out of memory\n"); return NULL; } @@ -527,7 +528,7 @@ static efi_status_t efi_variable_authenticate(u16 *variable, auth->auth_info.hdr.dwLength - sizeof(auth->auth_info)); if (!var_sig) { - debug("Parsing variable's signature failed\n"); + EFI_PRINT("Parsing variable's signature failed\n"); goto err; } @@ -558,14 +559,14 @@ static efi_status_t efi_variable_authenticate(u16 *variable, /* verify signature */ if (efi_signature_verify_with_sigdb(regs, var_sig, truststore, NULL)) { - debug("Verified\n"); + EFI_PRINT("Verified\n"); } else { if (truststore2 && efi_signature_verify_with_sigdb(regs, var_sig, truststore2, NULL)) { - debug("Verified\n"); + EFI_PRINT("Verified\n"); } else { - debug("Verifying variable's signature failed\n"); + EFI_PRINT("Verifying variable's signature failed\n"); goto err; } } @@ -640,7 +641,7 @@ static efi_status_t efi_get_variable_common(u16 *variable_name, } if (!data) { - debug("Variable with no data shouldn't exist.\n"); + EFI_PRINT("Variable with no data shouldn't exist.\n"); return EFI_INVALID_PARAMETER; } @@ -659,7 +660,7 @@ static efi_status_t efi_get_variable_common(u16 *variable_name, } if (!data) { - debug("Variable with no data shouldn't exist.\n"); + EFI_PRINT("Variable with no data shouldn't exist.\n"); return EFI_INVALID_PARAMETER; } @@ -940,8 +941,8 @@ static efi_status_t efi_set_variable_common(u16 *variable_name, /* authentication is mandatory */ if (!(attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)) { - debug("%ls: AUTHENTICATED_WRITE_ACCESS required\n", - variable_name); + EFI_PRINT("%ls: AUTHENTICATED_WRITE_ACCESS required\n", + variable_name); ret = EFI_INVALID_PARAMETER; goto err; } @@ -970,7 +971,7 @@ static efi_status_t efi_set_variable_common(u16 *variable_name, if (attributes & (EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS | EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)) { - debug("Secure boot is not configured\n"); + EFI_PRINT("Secure boot is not configured\n"); ret = EFI_INVALID_PARAMETER; goto err; } -- cgit v1.2.3 From 33f183f68b76226a1053694418d2c283371bee72 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 1 Jul 2020 12:44:00 +0200 Subject: efi_loader: add missing validation of timestamp The UEFI specification requires that when UEFI variables are set using time based authentication we have to check that unused fields of the timestamp are zero Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_variable.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'lib/efi_loader/efi_variable.c') diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 74a9c65402a..f9a0efd427c 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -481,11 +481,15 @@ static efi_status_t efi_variable_authenticate(u16 *variable, if (guidcmp(&auth->auth_info.cert_type, &efi_guid_cert_type_pkcs7)) goto err; + memcpy(×tamp, &auth->time_stamp, sizeof(timestamp)); + if (timestamp.pad1 || timestamp.nanosecond || timestamp.timezone || + timestamp.daylight || timestamp.pad2) + goto err; + *data += sizeof(auth->time_stamp) + auth->auth_info.hdr.dwLength; *data_size -= (sizeof(auth->time_stamp) + auth->auth_info.hdr.dwLength); - memcpy(×tamp, &auth->time_stamp, sizeof(timestamp)); memset(&tm, 0, sizeof(tm)); tm.tm_year = timestamp.year; tm.tm_mon = timestamp.month; -- cgit v1.2.3 From cb7116030aff44f48f29bdc3bd7ed22f7ad74bb9 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 1 Jul 2020 15:32:47 +0200 Subject: efi_loader: time based authentication When overwriting an existing time base authenticated variable we should compare to the preceding time value and not to the start of the epoch. Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_variable.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) (limited to 'lib/efi_loader/efi_variable.c') diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index f9a0efd427c..4d49fd60dcb 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -35,7 +35,8 @@ static u8 efi_vendor_keys; static efi_status_t efi_get_variable_common(u16 *variable_name, const efi_guid_t *vendor, u32 *attributes, - efi_uintn_t *data_size, void *data); + efi_uintn_t *data_size, void *data, + u64 *timep); static efi_status_t efi_set_variable_common(u16 *variable_name, const efi_guid_t *vendor, @@ -309,7 +310,7 @@ static efi_status_t efi_init_secure_state(void) size = 0; ret = efi_get_variable_common(L"PK", &efi_global_variable_guid, - NULL, &size, NULL); + NULL, &size, NULL, NULL); if (ret == EFI_BUFFER_TOO_SMALL) { if (IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) mode = EFI_MODE_USER; @@ -601,7 +602,8 @@ static efi_status_t efi_variable_authenticate(u16 *variable, static efi_status_t efi_get_variable_common(u16 *variable_name, const efi_guid_t *vendor, u32 *attributes, - efi_uintn_t *data_size, void *data) + efi_uintn_t *data_size, void *data, + u64 *timep) { char *native_name; efi_status_t ret; @@ -626,6 +628,9 @@ static efi_status_t efi_get_variable_common(u16 *variable_name, val = parse_attr(val, &attr, &time); + if (timep) + *timep = time; + in_size = *data_size; if ((s = prefix(val, "(blob)"))) { @@ -709,7 +714,7 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name, data_size, data); ret = efi_get_variable_common(variable_name, vendor, attributes, - data_size, data); + data_size, data, NULL); return EFI_EXIT(ret); } @@ -905,7 +910,7 @@ static efi_status_t efi_set_variable_common(u16 *variable_name, old_size = 0; attr = 0; ret = efi_get_variable_common(variable_name, vendor, &attr, - &old_size, NULL); + &old_size, NULL, &time); append = !!(attributes & EFI_VARIABLE_APPEND_WRITE); attributes &= ~(u32)EFI_VARIABLE_APPEND_WRITE; delete = !append && (!data_size || !attributes); @@ -996,7 +1001,7 @@ static efi_status_t efi_set_variable_common(u16 *variable_name, goto err; } ret = efi_get_variable_common(variable_name, vendor, - &attr, &old_size, old_data); + &attr, &old_size, old_data, NULL); if (ret != EFI_SUCCESS) goto err; } else { -- cgit v1.2.3 From 3a92f85f2121bb76061758e2830a5a1572729bcf Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 30 Jun 2020 12:02:14 +0200 Subject: efi_loader: rtc_mktime() called twice Don't call rtc_mktime() twice with the same argument in efi_variable_authenticate(). Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_variable.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/efi_loader/efi_variable.c') diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 4d49fd60dcb..efaba869ef2 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -577,7 +577,7 @@ static efi_status_t efi_variable_authenticate(u16 *variable, } /* finished checking */ - *time = rtc_mktime(&tm); + *time = new_time; ret = EFI_SUCCESS; err: -- cgit v1.2.3