[PATCH v2 2/6] efi_loader: add secure boot variable measurement
Heinrich Schuchardt
xypron.glpk at gmx.de
Thu Jul 15 08:44:48 CEST 2021
On 7/14/21 3:00 PM, Masahisa Kojima wrote:
> TCG PC Client PFP spec requires to measure the secure
> boot policy before validating the UEFI image.
> This commit adds the secure boot variable measurement
> of "SecureBoot", "PK", "KEK", "db" and "dbx".
>
> Note that this implementation assumes that secure boot
> variables are pre-configured and not be set/updated in runtime.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
> ---
>
> Changes in v2:
> - missing null check for getting variable data
> - some minor fix for readability
>
> include/efi_tcg2.h | 20 ++++++
> lib/efi_loader/efi_tcg2.c | 139 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 159 insertions(+)
>
> diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
> index bcfb98168a..8d7b77c087 100644
> --- a/include/efi_tcg2.h
> +++ b/include/efi_tcg2.h
> @@ -142,6 +142,26 @@ struct efi_tcg2_final_events_table {
> struct tcg_pcr_event2 event[];
> };
>
> +/**
> + * struct tdUEFI_VARIABLE_DATA
Please, make this
struct struct_name - Brief description
Cf.
https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation
> + * @variable_name: The vendorGUID parameter in the
> + * GetVariable() API.
> + * @unicode_name_length: The length in CHAR16 of the Unicode name of
> + * the variable.
> + * @variable_data_length: The size of the variable data.
> + * @unicode_name: The CHAR16 unicode name of the variable
> + * without NULL-terminator.
> + * @variable_data: The data parameter of the efi variable
> + * in the GetVariable() API.
> + */
> +struct efi_tcg2_uefi_variable_data {
> + efi_guid_t variable_name;
> + u64 unicode_name_length;
> + u64 variable_data_length;
> + u16 unicode_name[1];
> + u8 variable_data[1];
> +};
> +
> struct efi_tcg2_protocol {
> efi_status_t (EFIAPI * get_capability)(struct efi_tcg2_protocol *this,
> struct efi_tcg2_boot_service_capability *capability);
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 1319a8b378..12db6f6b7c 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -78,6 +78,19 @@ static const struct digest_info hash_algo_list[] = {
> },
> };
>
> +struct variable_info {
> + u16 *name;
> + const efi_guid_t *guid;
> +};
> +
> +static struct variable_info secure_variables[] = {
> + {L"SecureBoot", &efi_global_variable_guid},
> + {L"PK", &efi_global_variable_guid},
> + {L"KEK", &efi_global_variable_guid},
> + {L"db", &efi_guid_image_security_database},
> + {L"dbx", &efi_guid_image_security_database},
> +};
> +
> #define MAX_HASH_COUNT ARRAY_SIZE(hash_algo_list)
>
> /**
> @@ -1264,6 +1277,39 @@ free_pool:
> return ret;
> }
>
> +/**
> + * tcg2_measure_event() - common function to add event log and extend PCR
> + *
> + * @dev: TPM device
> + * @pcr_index: PCR index
> + * @event_type: type of event added
> + * @size: event size
> + * @event: event data
> + *
> + * Return: status code
> + */
> +static efi_status_t EFIAPI
> +tcg2_measure_event(struct udevice *dev, u32 pcr_index, u32 event_type,
> + u32 size, u8 event[])
> +{
> + struct tpml_digest_values digest_list;
> + efi_status_t ret;
> +
> + ret = tcg2_create_digest(event, size, &digest_list);
> + if (ret != EFI_SUCCESS)
> + goto out;
> +
> + ret = tcg2_pcr_extend(dev, pcr_index, &digest_list);
> + if (ret != EFI_SUCCESS)
> + goto out;
> +
> + ret = tcg2_agile_log_append(pcr_index, event_type, &digest_list,
> + size, event);
> +
> +out:
> + return ret;
> +}
> +
> /**
> * efi_append_scrtm_version - Append an S-CRTM EV_S_CRTM_VERSION event on the
> * eventlog and extend the PCRs
> @@ -1294,6 +1340,92 @@ out:
> return ret;
> }
>
> +/**
> + * tcg2_measure_variable() - add variable event log and extend PCR
> + *
> + * @dev: TPM device
> + * @pcr_index: PCR index
> + * @event_type: type of event added
> + * @var_name: variable name
> + * @guid: guid
> + * @data_size: variable data size
> + * @data: variable data
> + *
> + * Return: status code
> + */
> +static efi_status_t tcg2_measure_variable(struct udevice *dev, u32 pcr_index,
> + u32 event_type, u16 *var_name,
> + const efi_guid_t *guid,
> + efi_uintn_t data_size, u8 *data)
> +{
> + u32 event_size;
> + efi_status_t ret;
> + struct efi_tcg2_uefi_variable_data *event;
> +
> + event_size = sizeof(event->variable_name) +
> + sizeof(event->unicode_name_length) +
> + sizeof(event->variable_data_length) +
> + (u16_strlen(var_name) * sizeof(u16)) + data_size;
> + event = malloc(event_size);
> + if (!event)
> + return EFI_OUT_OF_RESOURCES;
> +
> + guidcpy(&event->variable_name, guid);
> + event->unicode_name_length = u16_strlen(var_name);
> + event->variable_data_length = data_size;
> + memcpy(event->unicode_name, var_name,
> + (event->unicode_name_length * sizeof(u16)));
> + memcpy((u16 *)event->unicode_name + event->unicode_name_length,
> + data, data_size);
> + ret = tcg2_measure_event(dev, pcr_index, event_type, event_size,
> + (u8 *)event);
> + free(event);
> + return ret;
> +}
> +
> +/**
> + * tcg2_measure_secure_boot_variable() - measure secure boot variables
> + *
> + * @dev: TPM device
> + *
> + * Return: status code
> + */
> +static efi_status_t tcg2_measure_secure_boot_variable(struct udevice *dev)
> +{
> + u8 *data;
> + efi_uintn_t data_size;
> + u32 count, i;
> + efi_status_t ret;
> +
> + count = ARRAY_SIZE(secure_variables);
> + for (i = 0; i < count; i++) {
> + data = efi_get_var(secure_variables[i].name,
> + secure_variables[i].guid,
> + &data_size);
> + if (data == NULL) {
> + log_info("%ls not found\n", secure_variables[i].name);
log_debug() seems adequate here as this function will be called even if
the board is not yet provisioned.
> + continue;
> + }
> +
> + ret = tcg2_measure_variable(dev, 7,
> + EV_EFI_VARIABLE_DRIVER_CONFIG,
> + secure_variables[i].name,
> + secure_variables[i].guid,
> + data_size, (u8 *)data);
> + free(data);
> + if (ret != EFI_SUCCESS)
> + goto error;
> + }
> +
> + /*
> + * TODO: add DBT and DBR measurement support when u-boot supports
"dbt" and "dbr" are lower case.
> + * these variables.
> + */
Adding them to secure_variables[] now would not harm.
Best regards
Heinrich
> +
> +error:
> + return ret;
> +}
> +
> /**
> * efi_tcg2_register() - register EFI_TCG2_PROTOCOL
> *
> @@ -1328,6 +1460,13 @@ efi_status_t efi_tcg2_register(void)
> tcg2_uninit();
> goto fail;
> }
> +
> + ret = tcg2_measure_secure_boot_variable(dev);
> + if (ret != EFI_SUCCESS) {
> + tcg2_uninit();
> + goto fail;
> + }
> +
> return ret;
>
> fail:
>
More information about the U-Boot
mailing list