| Message ID | 20210915051546.500-2-masahisa.kojima@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Enhance Measured Boot | expand |
Hi Kojima-san, On Wed, Sep 15, 2021 at 02:15:44PM +0900, Masahisa Kojima wrote: > TCG PC Client spec requires to measure the SMBIOS > table that contain static configuration information > (e.g. Platform Manufacturer Enterprise Number assigned by IANA, > platform model number, Vendor and Device IDs for each SMBIOS table). > > The device and environment dependent information such as > serial number is cleared to zero or space character for > the measurement. > > This commit also fixes the following compiler warning: > > lib/smbios-parser.c:59:39: warning: cast to pointer from integer of > different size [-Wint-to-pointer-cast] > const struct smbios_header *header = (struct smbios_header *)entry->struct_table_address; > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > --- > include/efi_loader.h | 2 + > include/efi_tcg2.h | 15 ++++ > include/smbios.h | 13 ++++ > lib/efi_loader/Kconfig | 1 + > lib/efi_loader/efi_boottime.c | 2 + > lib/efi_loader/efi_smbios.c | 2 - > lib/efi_loader/efi_tcg2.c | 84 ++++++++++++++++++++++ > lib/smbios-parser.c | 127 +++++++++++++++++++++++++++++++++- > 8 files changed, 243 insertions(+), 3 deletions(-) > > diff --git a/include/efi_loader.h b/include/efi_loader.h > index c440962fe5..13f0c24058 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -308,6 +308,8 @@ extern const efi_guid_t efi_guid_capsule_report; > extern const efi_guid_t efi_guid_firmware_management_protocol; > /* GUID for the ESRT */ > extern const efi_guid_t efi_esrt_guid; > +/* GUID of the SMBIOS table */ > +extern const efi_guid_t smbios_guid; > > extern char __efi_runtime_start[], __efi_runtime_stop[]; > extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[]; > diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h > index 5a1a36212e..da33f8a1d0 100644 > --- a/include/efi_tcg2.h > +++ b/include/efi_tcg2.h > @@ -215,6 +215,21 @@ struct efi_tcg2_uefi_variable_data { > u8 variable_data[1]; > }; > > +/** > + * struct tdUEFI_HANDOFF_TABLE_POINTERS2 - event log structure of SMBOIS tables > + * @table_description_size: size of table description > + * @table_description: table description > + * @number_of_tables: number of uefi configuration table > + * @table_entry: uefi configuration table entry > + */ > +#define SMBIOS_HANDOFF_TABLE_DESC "SmbiosTable" > +struct smbios_handoff_table_pointers2 { > + u8 table_description_size; > + u8 table_description[sizeof(SMBIOS_HANDOFF_TABLE_DESC)]; > + u64 number_of_tables; > + struct efi_configuration_table table_entry[1]; > +} __packed; Is this included in any other struct or array? If not the last member could be a flexible array member? > + > 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/include/smbios.h b/include/smbios.h > index aa6b6f3849..0c22c0c489 100644 > --- a/include/smbios.h > +++ b/include/smbios.h > @@ -292,4 +292,17 @@ int smbios_update_version(const char *version); > */ > int smbios_update_version_full(void *smbios_tab, const char *version); > > +/** > + * smbios_prepare_measurement() - Update smbios table for the measurement > + * > + * TCG specification requires to measure static configuration information. > + * This function clear the device dependent parameters such as > + * serial number for the measurement. > + * > + * @entry: pointer to a struct smbios_entry > + * @header: pointer to a struct smbios_header > + */ > +void smbios_prepare_measurement(const struct smbios_entry *entry, > + struct smbios_header *header); > + > #endif /* _SMBIOS_H_ */ > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > index dacc3b5881..ac1a281a36 100644 > --- a/lib/efi_loader/Kconfig > +++ b/lib/efi_loader/Kconfig > @@ -327,6 +327,7 @@ config EFI_TCG2_PROTOCOL > select SHA384 > select SHA512 > select HASH > + select SMBIOS_PARSER > help > Provide a EFI_TCG2_PROTOCOL implementation using the TPM hardware > of the platform. > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c > index f0283b539e..701e2212c8 100644 > --- a/lib/efi_loader/efi_boottime.c > +++ b/lib/efi_loader/efi_boottime.c > @@ -86,6 +86,8 @@ const efi_guid_t efi_guid_event_group_reset_system = > /* GUIDs of the Load File and Load File2 protocols */ > const efi_guid_t efi_guid_load_file_protocol = EFI_LOAD_FILE_PROTOCOL_GUID; > const efi_guid_t efi_guid_load_file2_protocol = EFI_LOAD_FILE2_PROTOCOL_GUID; > +/* GUID of the SMBIOS table */ > +const efi_guid_t smbios_guid = SMBIOS_TABLE_GUID; > > static efi_status_t EFIAPI efi_disconnect_controller( > efi_handle_t controller_handle, > diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c > index 2eb4cb1c1a..fc0b23397c 100644 > --- a/lib/efi_loader/efi_smbios.c > +++ b/lib/efi_loader/efi_smbios.c > @@ -13,8 +13,6 @@ > #include <mapmem.h> > #include <smbios.h> > > -static const efi_guid_t smbios_guid = SMBIOS_TABLE_GUID; > - > /* > * Install the SMBIOS table as a configuration table. > * > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c > index cb48919223..7f47998a55 100644 > --- a/lib/efi_loader/efi_tcg2.c > +++ b/lib/efi_loader/efi_tcg2.c > @@ -14,6 +14,7 @@ > #include <efi_tcg2.h> > #include <log.h> > #include <malloc.h> > +#include <smbios.h> > #include <version.h> > #include <tpm-v2.h> > #include <u-boot/hash-checksum.h> > @@ -1449,6 +1450,81 @@ error: > return ret; > } > > +/** > + * tcg2_measure_smbios() - measure smbios table > + * > + * @dev: TPM device > + * @entry: pointer to the smbios_entry structure > + * > + * Return: status code > + */ > +static efi_status_t > +tcg2_measure_smbios(struct udevice *dev, > + const struct smbios_entry *entry) > +{ > + efi_status_t ret; > + struct smbios_header *smbios_copy; > + struct smbios_handoff_table_pointers2 *event = NULL; > + u32 event_size; > + > + /* > + * TCG PC Client PFP Spec says > + * "SMBIOS structures that contain static configuration information > + * (e.g. Platform Manufacturer Enterprise Number assigned by IANA, > + * platform model number, Vendor and Device IDs for each SMBIOS table) > + * that is relevant to the security of the platform MUST be measured". > + * Device dependent parameters such as serial number are cleared to > + * zero or spaces for the measurement. > + */ > + event_size = sizeof(struct smbios_handoff_table_pointers2) + > + entry->struct_table_length - > + FIELD_SIZEOF(struct efi_configuration_table, table); Why do we have to subtract the efi_config_table table field here? > + event = calloc(1, event_size); > + if (!event) { > + ret = EFI_OUT_OF_RESOURCES; > + goto out; > + } > + > + event->table_description_size = sizeof(SMBIOS_HANDOFF_TABLE_DESC); > + memcpy(event->table_description, SMBIOS_HANDOFF_TABLE_DESC, > + sizeof(SMBIOS_HANDOFF_TABLE_DESC)); > + put_unaligned_le64(1, &event->number_of_tables); > + guidcpy(&event->table_entry[0].guid, &smbios_guid); > + smbios_copy = (struct smbios_header *)((uintptr_t)&event->table_entry[0].table); > + memcpy(&event->table_entry[0].table, > + (void *)((uintptr_t)entry->struct_table_address), > + entry->struct_table_length); > + > + smbios_prepare_measurement(entry, smbios_copy); > + > + ret = tcg2_measure_event(dev, 1, EV_EFI_HANDOFF_TABLES2, event_size, > + (u8 *)event); > + if (ret != EFI_SUCCESS) > + goto out; > + > +out: > + free(event); > + > + return ret; > +} > + > +/** > + * search_smbios_table() - search smbios table > + * > + * Return: pointer to the smbios table > + */ > +static void *search_smbios_table(void) Isn't find_smbios_table a better name? > +{ > + u32 i; > + > + for (i = 0; i < systab.nr_tables; i++) { > + if (!guidcmp(&smbios_guid, &systab.tables[i].guid)) > + return systab.tables[i].table; > + } > + > + return NULL; > +} > + > /** > * efi_tcg2_measure_efi_app_invocation() - measure efi app invocation > * > @@ -1460,6 +1536,7 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(void) > u32 pcr_index; > struct udevice *dev; > u32 event = 0; > + struct smbios_entry *entry; > > if (tcg2_efi_app_invoked) > return EFI_SUCCESS; > @@ -1485,6 +1562,13 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(void) > goto out; > } > > + entry = (struct smbios_entry *)search_smbios_table(); > + if (entry) { > + ret = tcg2_measure_smbios(dev, entry); > + if (ret != EFI_SUCCESS) > + goto out; > + } > + > tcg2_efi_app_invoked = true; > out: > return ret; > diff --git a/lib/smbios-parser.c b/lib/smbios-parser.c > index 34203f952c..5e0bd8f4ca 100644 > --- a/lib/smbios-parser.c > +++ b/lib/smbios-parser.c > @@ -56,7 +56,7 @@ static const struct smbios_header *next_header(const struct smbios_header *curr) > const struct smbios_header *smbios_header(const struct smbios_entry *entry, int type) > { > const unsigned int num_header = entry->struct_count; > - const struct smbios_header *header = (struct smbios_header *)entry->struct_table_address; > + const struct smbios_header *header = (struct smbios_header *)((uintptr_t)entry->struct_table_address); > > for (unsigned int i = 0; i < num_header; i++) { > if (header->type == type) > @@ -132,3 +132,128 @@ int smbios_update_version_full(void *smbios_tab, const char *version) > > return 0; > } > + > +struct smbios_filter_param { > + u32 offset; > + u32 size; > + bool is_string; > +}; > + > +struct smbios_filter_table { > + int type; > + struct smbios_filter_param *params; > + u32 count; > +}; > + > +struct smbios_filter_param smbios_type1_filter_params[] = { > + {offsetof(struct smbios_type1, serial_number), > + FIELD_SIZEOF(struct smbios_type1, serial_number), true}, > + {offsetof(struct smbios_type1, uuid), > + FIELD_SIZEOF(struct smbios_type1, uuid), false}, > + {offsetof(struct smbios_type1, wakeup_type), > + FIELD_SIZEOF(struct smbios_type1, wakeup_type), false}, > +}; > + > +struct smbios_filter_param smbios_type2_filter_params[] = { > + {offsetof(struct smbios_type2, serial_number), > + FIELD_SIZEOF(struct smbios_type2, serial_number), true}, > + {offsetof(struct smbios_type2, chassis_location), > + FIELD_SIZEOF(struct smbios_type2, chassis_location), false}, > +}; > + > +struct smbios_filter_param smbios_type3_filter_params[] = { > + {offsetof(struct smbios_type3, serial_number), > + FIELD_SIZEOF(struct smbios_type3, serial_number), true}, > + {offsetof(struct smbios_type3, asset_tag_number), > + FIELD_SIZEOF(struct smbios_type3, asset_tag_number), true}, > +}; > + > +struct smbios_filter_param smbios_type4_filter_params[] = { > + {offsetof(struct smbios_type4, serial_number), > + FIELD_SIZEOF(struct smbios_type4, serial_number), true}, > + {offsetof(struct smbios_type4, asset_tag), > + FIELD_SIZEOF(struct smbios_type4, asset_tag), true}, > + {offsetof(struct smbios_type4, part_number), > + FIELD_SIZEOF(struct smbios_type4, part_number), true}, > + {offsetof(struct smbios_type4, core_count), > + FIELD_SIZEOF(struct smbios_type4, core_count), false}, > + {offsetof(struct smbios_type4, core_enabled), > + FIELD_SIZEOF(struct smbios_type4, core_enabled), false}, > + {offsetof(struct smbios_type4, thread_count), > + FIELD_SIZEOF(struct smbios_type4, thread_count), false}, > + {offsetof(struct smbios_type4, core_count2), > + FIELD_SIZEOF(struct smbios_type4, core_count2), false}, > + {offsetof(struct smbios_type4, core_enabled2), > + FIELD_SIZEOF(struct smbios_type4, core_enabled2), false}, > + {offsetof(struct smbios_type4, thread_count), > + FIELD_SIZEOF(struct smbios_type4, thread_count2), false}, > + {offsetof(struct smbios_type4, voltage), > + FIELD_SIZEOF(struct smbios_type4, voltage), false}, > +}; > + > +struct smbios_filter_table smbios_filter_tables[] = { > + {SMBIOS_SYSTEM_INFORMATION, smbios_type1_filter_params, > + ARRAY_SIZE(smbios_type1_filter_params)}, > + {SMBIOS_BOARD_INFORMATION, smbios_type2_filter_params, > + ARRAY_SIZE(smbios_type2_filter_params)}, > + {SMBIOS_SYSTEM_ENCLOSURE, smbios_type3_filter_params, > + ARRAY_SIZE(smbios_type3_filter_params)}, > + {SMBIOS_PROCESSOR_INFORMATION, smbios_type4_filter_params, > + ARRAY_SIZE(smbios_type4_filter_params)}, > +}; > + > +static void clear_smbios_table(struct smbios_header *header, > + struct smbios_filter_param *filter, > + u32 count) > +{ > + u32 i; > + char *str; > + u8 string_id; > + > + for (i = 0; i < count; i++) { > + if (filter[i].is_string) { > + string_id = *((u8 *)header + filter[i].offset); > + if (string_id == 0) /* string is empty */ > + continue; > + /* > + * smbios_string() returns const value, but this > + * function needs to clear the string. > + */ > + str = (char *)smbios_string(header, string_id); > + if (!str) > + continue; > + /* string is cleared to space */ > + memset(str, ' ', strlen(str)); Isn't this undefined behavior ? If the area pointed to by whatever smbios_string() is ro that will crash? > + > + } else { > + memset((void *)((u8 *)header + filter[i].offset), > + 0, filter[i].size); > + } > + } > +} > + > +void smbios_prepare_measurement(const struct smbios_entry *entry, > + struct smbios_header *smbios_copy) > +{ > + u32 i, j; > + struct smbios_header *header; > + > + for (i = 0; i < ARRAY_SIZE(smbios_filter_tables); i++) { > + header = smbios_copy; > + for (j = 0; j < entry->struct_count; j++) { > + if (header->type == smbios_filter_tables[i].type) > + break; > + /* > + * next_header() returns the const value, but some > + * members need to be cleared for the measurement. > + */ > + header = (struct smbios_header *)next_header(header); > + } > + if (j >= entry->struct_count) > + continue; > + > + clear_smbios_table(header, > + smbios_filter_tables[i].params, > + smbios_filter_tables[i].count); > + } > +} > -- > 2.17.1 > Regards /Ilias
Hi Ilias, Thank you for the review. On Wed, 15 Sept 2021 at 17:37, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Hi Kojima-san, > > On Wed, Sep 15, 2021 at 02:15:44PM +0900, Masahisa Kojima wrote: > > TCG PC Client spec requires to measure the SMBIOS > > table that contain static configuration information > > (e.g. Platform Manufacturer Enterprise Number assigned by IANA, > > platform model number, Vendor and Device IDs for each SMBIOS table). > > > > The device and environment dependent information such as > > serial number is cleared to zero or space character for > > the measurement. > > > > This commit also fixes the following compiler warning: > > > > lib/smbios-parser.c:59:39: warning: cast to pointer from integer of > > different size [-Wint-to-pointer-cast] > > const struct smbios_header *header = (struct smbios_header *)entry->struct_table_address; > > > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > > --- > > include/efi_loader.h | 2 + > > include/efi_tcg2.h | 15 ++++ > > include/smbios.h | 13 ++++ > > lib/efi_loader/Kconfig | 1 + > > lib/efi_loader/efi_boottime.c | 2 + > > lib/efi_loader/efi_smbios.c | 2 - > > lib/efi_loader/efi_tcg2.c | 84 ++++++++++++++++++++++ > > lib/smbios-parser.c | 127 +++++++++++++++++++++++++++++++++- > > 8 files changed, 243 insertions(+), 3 deletions(-) > > > > diff --git a/include/efi_loader.h b/include/efi_loader.h > > index c440962fe5..13f0c24058 100644 > > --- a/include/efi_loader.h > > +++ b/include/efi_loader.h > > @@ -308,6 +308,8 @@ extern const efi_guid_t efi_guid_capsule_report; > > extern const efi_guid_t efi_guid_firmware_management_protocol; > > /* GUID for the ESRT */ > > extern const efi_guid_t efi_esrt_guid; > > +/* GUID of the SMBIOS table */ > > +extern const efi_guid_t smbios_guid; > > > > extern char __efi_runtime_start[], __efi_runtime_stop[]; > > extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[]; > > diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h > > index 5a1a36212e..da33f8a1d0 100644 > > --- a/include/efi_tcg2.h > > +++ b/include/efi_tcg2.h > > @@ -215,6 +215,21 @@ struct efi_tcg2_uefi_variable_data { > > u8 variable_data[1]; > > }; > > > > +/** > > + * struct tdUEFI_HANDOFF_TABLE_POINTERS2 - event log structure of SMBOIS tables > > + * @table_description_size: size of table description > > + * @table_description: table description > > + * @number_of_tables: number of uefi configuration table > > + * @table_entry: uefi configuration table entry > > + */ > > +#define SMBIOS_HANDOFF_TABLE_DESC "SmbiosTable" > > +struct smbios_handoff_table_pointers2 { > > + u8 table_description_size; > > + u8 table_description[sizeof(SMBIOS_HANDOFF_TABLE_DESC)]; > > + u64 number_of_tables; > > + struct efi_configuration_table table_entry[1]; > > +} __packed; > > Is this included in any other struct or array? If not the last member could > be a flexible array member? Yes, it can be a flexible array. > > > + > > 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/include/smbios.h b/include/smbios.h > > index aa6b6f3849..0c22c0c489 100644 > > --- a/include/smbios.h > > +++ b/include/smbios.h > > @@ -292,4 +292,17 @@ int smbios_update_version(const char *version); > > */ > > int smbios_update_version_full(void *smbios_tab, const char *version); > > > > +/** > > + * smbios_prepare_measurement() - Update smbios table for the measurement > > + * > > + * TCG specification requires to measure static configuration information. > > + * This function clear the device dependent parameters such as > > + * serial number for the measurement. > > + * > > + * @entry: pointer to a struct smbios_entry > > + * @header: pointer to a struct smbios_header > > + */ > > +void smbios_prepare_measurement(const struct smbios_entry *entry, > > + struct smbios_header *header); > > + > > #endif /* _SMBIOS_H_ */ > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > index dacc3b5881..ac1a281a36 100644 > > --- a/lib/efi_loader/Kconfig > > +++ b/lib/efi_loader/Kconfig > > @@ -327,6 +327,7 @@ config EFI_TCG2_PROTOCOL > > select SHA384 > > select SHA512 > > select HASH > > + select SMBIOS_PARSER > > help > > Provide a EFI_TCG2_PROTOCOL implementation using the TPM hardware > > of the platform. > > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c > > index f0283b539e..701e2212c8 100644 > > --- a/lib/efi_loader/efi_boottime.c > > +++ b/lib/efi_loader/efi_boottime.c > > @@ -86,6 +86,8 @@ const efi_guid_t efi_guid_event_group_reset_system = > > /* GUIDs of the Load File and Load File2 protocols */ > > const efi_guid_t efi_guid_load_file_protocol = EFI_LOAD_FILE_PROTOCOL_GUID; > > const efi_guid_t efi_guid_load_file2_protocol = EFI_LOAD_FILE2_PROTOCOL_GUID; > > +/* GUID of the SMBIOS table */ > > +const efi_guid_t smbios_guid = SMBIOS_TABLE_GUID; > > > > static efi_status_t EFIAPI efi_disconnect_controller( > > efi_handle_t controller_handle, > > diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c > > index 2eb4cb1c1a..fc0b23397c 100644 > > --- a/lib/efi_loader/efi_smbios.c > > +++ b/lib/efi_loader/efi_smbios.c > > @@ -13,8 +13,6 @@ > > #include <mapmem.h> > > #include <smbios.h> > > > > -static const efi_guid_t smbios_guid = SMBIOS_TABLE_GUID; > > - > > /* > > * Install the SMBIOS table as a configuration table. > > * > > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c > > index cb48919223..7f47998a55 100644 > > --- a/lib/efi_loader/efi_tcg2.c > > +++ b/lib/efi_loader/efi_tcg2.c > > @@ -14,6 +14,7 @@ > > #include <efi_tcg2.h> > > #include <log.h> > > #include <malloc.h> > > +#include <smbios.h> > > #include <version.h> > > #include <tpm-v2.h> > > #include <u-boot/hash-checksum.h> > > @@ -1449,6 +1450,81 @@ error: > > return ret; > > } > > > > +/** > > + * tcg2_measure_smbios() - measure smbios table > > + * > > + * @dev: TPM device > > + * @entry: pointer to the smbios_entry structure > > + * > > + * Return: status code > > + */ > > +static efi_status_t > > +tcg2_measure_smbios(struct udevice *dev, > > + const struct smbios_entry *entry) > > +{ > > + efi_status_t ret; > > + struct smbios_header *smbios_copy; > > + struct smbios_handoff_table_pointers2 *event = NULL; > > + u32 event_size; > > + > > + /* > > + * TCG PC Client PFP Spec says > > + * "SMBIOS structures that contain static configuration information > > + * (e.g. Platform Manufacturer Enterprise Number assigned by IANA, > > + * platform model number, Vendor and Device IDs for each SMBIOS table) > > + * that is relevant to the security of the platform MUST be measured". > > + * Device dependent parameters such as serial number are cleared to > > + * zero or spaces for the measurement. > > + */ > > + event_size = sizeof(struct smbios_handoff_table_pointers2) + > > + entry->struct_table_length - > > + FIELD_SIZEOF(struct efi_configuration_table, table); > > Why do we have to subtract the efi_config_table table field here? efi_config_table *table field is used to point the address of smbios table data, actual smbios table size is indicated by entry->struct_table_length. efi_config_table *table field itself is not used and we need to substract it. > > > + event = calloc(1, event_size); > > + if (!event) { > > + ret = EFI_OUT_OF_RESOURCES; > > + goto out; > > + } > > + > > + event->table_description_size = sizeof(SMBIOS_HANDOFF_TABLE_DESC); > > + memcpy(event->table_description, SMBIOS_HANDOFF_TABLE_DESC, > > + sizeof(SMBIOS_HANDOFF_TABLE_DESC)); > > + put_unaligned_le64(1, &event->number_of_tables); > > + guidcpy(&event->table_entry[0].guid, &smbios_guid); > > + smbios_copy = (struct smbios_header *)((uintptr_t)&event->table_entry[0].table); > > + memcpy(&event->table_entry[0].table, > > + (void *)((uintptr_t)entry->struct_table_address), > > + entry->struct_table_length); > > + > > + smbios_prepare_measurement(entry, smbios_copy); > > + > > + ret = tcg2_measure_event(dev, 1, EV_EFI_HANDOFF_TABLES2, event_size, > > + (u8 *)event); > > + if (ret != EFI_SUCCESS) > > + goto out; > > + > > +out: > > + free(event); > > + > > + return ret; > > +} > > + > > +/** > > + * search_smbios_table() - search smbios table > > + * > > + * Return: pointer to the smbios table > > + */ > > +static void *search_smbios_table(void) > > Isn't find_smbios_table a better name? I agree, my first choice was "find", but I finally chose "search". I will replace with "find". > > > +{ > > + u32 i; > > + > > + for (i = 0; i < systab.nr_tables; i++) { > > + if (!guidcmp(&smbios_guid, &systab.tables[i].guid)) > > + return systab.tables[i].table; > > + } > > + > > + return NULL; > > +} > > + > > /** > > * efi_tcg2_measure_efi_app_invocation() - measure efi app invocation > > * > > @@ -1460,6 +1536,7 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(void) > > u32 pcr_index; > > struct udevice *dev; > > u32 event = 0; > > + struct smbios_entry *entry; > > > > if (tcg2_efi_app_invoked) > > return EFI_SUCCESS; > > @@ -1485,6 +1562,13 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(void) > > goto out; > > } > > > > + entry = (struct smbios_entry *)search_smbios_table(); > > + if (entry) { > > + ret = tcg2_measure_smbios(dev, entry); > > + if (ret != EFI_SUCCESS) > > + goto out; > > + } > > + > > tcg2_efi_app_invoked = true; > > out: > > return ret; > > diff --git a/lib/smbios-parser.c b/lib/smbios-parser.c > > index 34203f952c..5e0bd8f4ca 100644 > > --- a/lib/smbios-parser.c > > +++ b/lib/smbios-parser.c > > @@ -56,7 +56,7 @@ static const struct smbios_header *next_header(const struct smbios_header *curr) > > const struct smbios_header *smbios_header(const struct smbios_entry *entry, int type) > > { > > const unsigned int num_header = entry->struct_count; > > - const struct smbios_header *header = (struct smbios_header *)entry->struct_table_address; > > + const struct smbios_header *header = (struct smbios_header *)((uintptr_t)entry->struct_table_address); > > > > for (unsigned int i = 0; i < num_header; i++) { > > if (header->type == type) > > @@ -132,3 +132,128 @@ int smbios_update_version_full(void *smbios_tab, const char *version) > > > > return 0; > > } > > + > > +struct smbios_filter_param { > > + u32 offset; > > + u32 size; > > + bool is_string; > > +}; > > + > > +struct smbios_filter_table { > > + int type; > > + struct smbios_filter_param *params; > > + u32 count; > > +}; > > + > > +struct smbios_filter_param smbios_type1_filter_params[] = { > > + {offsetof(struct smbios_type1, serial_number), > > + FIELD_SIZEOF(struct smbios_type1, serial_number), true}, > > + {offsetof(struct smbios_type1, uuid), > > + FIELD_SIZEOF(struct smbios_type1, uuid), false}, > > + {offsetof(struct smbios_type1, wakeup_type), > > + FIELD_SIZEOF(struct smbios_type1, wakeup_type), false}, > > +}; > > + > > +struct smbios_filter_param smbios_type2_filter_params[] = { > > + {offsetof(struct smbios_type2, serial_number), > > + FIELD_SIZEOF(struct smbios_type2, serial_number), true}, > > + {offsetof(struct smbios_type2, chassis_location), > > + FIELD_SIZEOF(struct smbios_type2, chassis_location), false}, > > +}; > > + > > +struct smbios_filter_param smbios_type3_filter_params[] = { > > + {offsetof(struct smbios_type3, serial_number), > > + FIELD_SIZEOF(struct smbios_type3, serial_number), true}, > > + {offsetof(struct smbios_type3, asset_tag_number), > > + FIELD_SIZEOF(struct smbios_type3, asset_tag_number), true}, > > +}; > > + > > +struct smbios_filter_param smbios_type4_filter_params[] = { > > + {offsetof(struct smbios_type4, serial_number), > > + FIELD_SIZEOF(struct smbios_type4, serial_number), true}, > > + {offsetof(struct smbios_type4, asset_tag), > > + FIELD_SIZEOF(struct smbios_type4, asset_tag), true}, > > + {offsetof(struct smbios_type4, part_number), > > + FIELD_SIZEOF(struct smbios_type4, part_number), true}, > > + {offsetof(struct smbios_type4, core_count), > > + FIELD_SIZEOF(struct smbios_type4, core_count), false}, > > + {offsetof(struct smbios_type4, core_enabled), > > + FIELD_SIZEOF(struct smbios_type4, core_enabled), false}, > > + {offsetof(struct smbios_type4, thread_count), > > + FIELD_SIZEOF(struct smbios_type4, thread_count), false}, > > + {offsetof(struct smbios_type4, core_count2), > > + FIELD_SIZEOF(struct smbios_type4, core_count2), false}, > > + {offsetof(struct smbios_type4, core_enabled2), > > + FIELD_SIZEOF(struct smbios_type4, core_enabled2), false}, > > + {offsetof(struct smbios_type4, thread_count), > > + FIELD_SIZEOF(struct smbios_type4, thread_count2), false}, > > + {offsetof(struct smbios_type4, voltage), > > + FIELD_SIZEOF(struct smbios_type4, voltage), false}, > > +}; > > + > > +struct smbios_filter_table smbios_filter_tables[] = { > > + {SMBIOS_SYSTEM_INFORMATION, smbios_type1_filter_params, > > + ARRAY_SIZE(smbios_type1_filter_params)}, > > + {SMBIOS_BOARD_INFORMATION, smbios_type2_filter_params, > > + ARRAY_SIZE(smbios_type2_filter_params)}, > > + {SMBIOS_SYSTEM_ENCLOSURE, smbios_type3_filter_params, > > + ARRAY_SIZE(smbios_type3_filter_params)}, > > + {SMBIOS_PROCESSOR_INFORMATION, smbios_type4_filter_params, > > + ARRAY_SIZE(smbios_type4_filter_params)}, > > +}; > > + > > +static void clear_smbios_table(struct smbios_header *header, > > + struct smbios_filter_param *filter, > > + u32 count) > > +{ > > + u32 i; > > + char *str; > > + u8 string_id; > > + > > + for (i = 0; i < count; i++) { > > + if (filter[i].is_string) { > > + string_id = *((u8 *)header + filter[i].offset); > > + if (string_id == 0) /* string is empty */ > > + continue; > > + /* > > + * smbios_string() returns const value, but this > > + * function needs to clear the string. > > + */ > > + str = (char *)smbios_string(header, string_id); > > + if (!str) > > + continue; > > + /* string is cleared to space */ > > + memset(str, ' ', strlen(str)); > > Isn't this undefined behavior ? If the area pointed to by whatever > smbios_string() is ro that will crash? Yes, this code is not good, I will fix. Same instance exists in smbios_prepare_measurement(). Thanks, Masahisa Kojima > > > + > > + } else { > > + memset((void *)((u8 *)header + filter[i].offset), > > + 0, filter[i].size); > > + } > > + } > > +} > > + > > +void smbios_prepare_measurement(const struct smbios_entry *entry, > > + struct smbios_header *smbios_copy) > > +{ > > + u32 i, j; > > + struct smbios_header *header; > > + > > + for (i = 0; i < ARRAY_SIZE(smbios_filter_tables); i++) { > > + header = smbios_copy; > > + for (j = 0; j < entry->struct_count; j++) { > > + if (header->type == smbios_filter_tables[i].type) > > + break; > > + /* > > + * next_header() returns the const value, but some > > + * members need to be cleared for the measurement. > > + */ > > + header = (struct smbios_header *)next_header(header); > > + } > > + if (j >= entry->struct_count) > > + continue; > > + > > + clear_smbios_table(header, > > + smbios_filter_tables[i].params, > > + smbios_filter_tables[i].count); > > + } > > +} > > -- > > 2.17.1 > > > > Regards > /Ilias
diff --git a/include/efi_loader.h b/include/efi_loader.h index c440962fe5..13f0c24058 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -308,6 +308,8 @@ extern const efi_guid_t efi_guid_capsule_report; extern const efi_guid_t efi_guid_firmware_management_protocol; /* GUID for the ESRT */ extern const efi_guid_t efi_esrt_guid; +/* GUID of the SMBIOS table */ +extern const efi_guid_t smbios_guid; extern char __efi_runtime_start[], __efi_runtime_stop[]; extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[]; diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h index 5a1a36212e..da33f8a1d0 100644 --- a/include/efi_tcg2.h +++ b/include/efi_tcg2.h @@ -215,6 +215,21 @@ struct efi_tcg2_uefi_variable_data { u8 variable_data[1]; }; +/** + * struct tdUEFI_HANDOFF_TABLE_POINTERS2 - event log structure of SMBOIS tables + * @table_description_size: size of table description + * @table_description: table description + * @number_of_tables: number of uefi configuration table + * @table_entry: uefi configuration table entry + */ +#define SMBIOS_HANDOFF_TABLE_DESC "SmbiosTable" +struct smbios_handoff_table_pointers2 { + u8 table_description_size; + u8 table_description[sizeof(SMBIOS_HANDOFF_TABLE_DESC)]; + u64 number_of_tables; + struct efi_configuration_table table_entry[1]; +} __packed; + 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/include/smbios.h b/include/smbios.h index aa6b6f3849..0c22c0c489 100644 --- a/include/smbios.h +++ b/include/smbios.h @@ -292,4 +292,17 @@ int smbios_update_version(const char *version); */ int smbios_update_version_full(void *smbios_tab, const char *version); +/** + * smbios_prepare_measurement() - Update smbios table for the measurement + * + * TCG specification requires to measure static configuration information. + * This function clear the device dependent parameters such as + * serial number for the measurement. + * + * @entry: pointer to a struct smbios_entry + * @header: pointer to a struct smbios_header + */ +void smbios_prepare_measurement(const struct smbios_entry *entry, + struct smbios_header *header); + #endif /* _SMBIOS_H_ */ diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index dacc3b5881..ac1a281a36 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -327,6 +327,7 @@ config EFI_TCG2_PROTOCOL select SHA384 select SHA512 select HASH + select SMBIOS_PARSER help Provide a EFI_TCG2_PROTOCOL implementation using the TPM hardware of the platform. diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index f0283b539e..701e2212c8 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -86,6 +86,8 @@ const efi_guid_t efi_guid_event_group_reset_system = /* GUIDs of the Load File and Load File2 protocols */ const efi_guid_t efi_guid_load_file_protocol = EFI_LOAD_FILE_PROTOCOL_GUID; const efi_guid_t efi_guid_load_file2_protocol = EFI_LOAD_FILE2_PROTOCOL_GUID; +/* GUID of the SMBIOS table */ +const efi_guid_t smbios_guid = SMBIOS_TABLE_GUID; static efi_status_t EFIAPI efi_disconnect_controller( efi_handle_t controller_handle, diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c index 2eb4cb1c1a..fc0b23397c 100644 --- a/lib/efi_loader/efi_smbios.c +++ b/lib/efi_loader/efi_smbios.c @@ -13,8 +13,6 @@ #include <mapmem.h> #include <smbios.h> -static const efi_guid_t smbios_guid = SMBIOS_TABLE_GUID; - /* * Install the SMBIOS table as a configuration table. * diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index cb48919223..7f47998a55 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -14,6 +14,7 @@ #include <efi_tcg2.h> #include <log.h> #include <malloc.h> +#include <smbios.h> #include <version.h> #include <tpm-v2.h> #include <u-boot/hash-checksum.h> @@ -1449,6 +1450,81 @@ error: return ret; } +/** + * tcg2_measure_smbios() - measure smbios table + * + * @dev: TPM device + * @entry: pointer to the smbios_entry structure + * + * Return: status code + */ +static efi_status_t +tcg2_measure_smbios(struct udevice *dev, + const struct smbios_entry *entry) +{ + efi_status_t ret; + struct smbios_header *smbios_copy; + struct smbios_handoff_table_pointers2 *event = NULL; + u32 event_size; + + /* + * TCG PC Client PFP Spec says + * "SMBIOS structures that contain static configuration information + * (e.g. Platform Manufacturer Enterprise Number assigned by IANA, + * platform model number, Vendor and Device IDs for each SMBIOS table) + * that is relevant to the security of the platform MUST be measured". + * Device dependent parameters such as serial number are cleared to + * zero or spaces for the measurement. + */ + event_size = sizeof(struct smbios_handoff_table_pointers2) + + entry->struct_table_length - + FIELD_SIZEOF(struct efi_configuration_table, table); + event = calloc(1, event_size); + if (!event) { + ret = EFI_OUT_OF_RESOURCES; + goto out; + } + + event->table_description_size = sizeof(SMBIOS_HANDOFF_TABLE_DESC); + memcpy(event->table_description, SMBIOS_HANDOFF_TABLE_DESC, + sizeof(SMBIOS_HANDOFF_TABLE_DESC)); + put_unaligned_le64(1, &event->number_of_tables); + guidcpy(&event->table_entry[0].guid, &smbios_guid); + smbios_copy = (struct smbios_header *)((uintptr_t)&event->table_entry[0].table); + memcpy(&event->table_entry[0].table, + (void *)((uintptr_t)entry->struct_table_address), + entry->struct_table_length); + + smbios_prepare_measurement(entry, smbios_copy); + + ret = tcg2_measure_event(dev, 1, EV_EFI_HANDOFF_TABLES2, event_size, + (u8 *)event); + if (ret != EFI_SUCCESS) + goto out; + +out: + free(event); + + return ret; +} + +/** + * search_smbios_table() - search smbios table + * + * Return: pointer to the smbios table + */ +static void *search_smbios_table(void) +{ + u32 i; + + for (i = 0; i < systab.nr_tables; i++) { + if (!guidcmp(&smbios_guid, &systab.tables[i].guid)) + return systab.tables[i].table; + } + + return NULL; +} + /** * efi_tcg2_measure_efi_app_invocation() - measure efi app invocation * @@ -1460,6 +1536,7 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(void) u32 pcr_index; struct udevice *dev; u32 event = 0; + struct smbios_entry *entry; if (tcg2_efi_app_invoked) return EFI_SUCCESS; @@ -1485,6 +1562,13 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(void) goto out; } + entry = (struct smbios_entry *)search_smbios_table(); + if (entry) { + ret = tcg2_measure_smbios(dev, entry); + if (ret != EFI_SUCCESS) + goto out; + } + tcg2_efi_app_invoked = true; out: return ret; diff --git a/lib/smbios-parser.c b/lib/smbios-parser.c index 34203f952c..5e0bd8f4ca 100644 --- a/lib/smbios-parser.c +++ b/lib/smbios-parser.c @@ -56,7 +56,7 @@ static const struct smbios_header *next_header(const struct smbios_header *curr) const struct smbios_header *smbios_header(const struct smbios_entry *entry, int type) { const unsigned int num_header = entry->struct_count; - const struct smbios_header *header = (struct smbios_header *)entry->struct_table_address; + const struct smbios_header *header = (struct smbios_header *)((uintptr_t)entry->struct_table_address); for (unsigned int i = 0; i < num_header; i++) { if (header->type == type) @@ -132,3 +132,128 @@ int smbios_update_version_full(void *smbios_tab, const char *version) return 0; } + +struct smbios_filter_param { + u32 offset; + u32 size; + bool is_string; +}; + +struct smbios_filter_table { + int type; + struct smbios_filter_param *params; + u32 count; +}; + +struct smbios_filter_param smbios_type1_filter_params[] = { + {offsetof(struct smbios_type1, serial_number), + FIELD_SIZEOF(struct smbios_type1, serial_number), true}, + {offsetof(struct smbios_type1, uuid), + FIELD_SIZEOF(struct smbios_type1, uuid), false}, + {offsetof(struct smbios_type1, wakeup_type), + FIELD_SIZEOF(struct smbios_type1, wakeup_type), false}, +}; + +struct smbios_filter_param smbios_type2_filter_params[] = { + {offsetof(struct smbios_type2, serial_number), + FIELD_SIZEOF(struct smbios_type2, serial_number), true}, + {offsetof(struct smbios_type2, chassis_location), + FIELD_SIZEOF(struct smbios_type2, chassis_location), false}, +}; + +struct smbios_filter_param smbios_type3_filter_params[] = { + {offsetof(struct smbios_type3, serial_number), + FIELD_SIZEOF(struct smbios_type3, serial_number), true}, + {offsetof(struct smbios_type3, asset_tag_number), + FIELD_SIZEOF(struct smbios_type3, asset_tag_number), true}, +}; + +struct smbios_filter_param smbios_type4_filter_params[] = { + {offsetof(struct smbios_type4, serial_number), + FIELD_SIZEOF(struct smbios_type4, serial_number), true}, + {offsetof(struct smbios_type4, asset_tag), + FIELD_SIZEOF(struct smbios_type4, asset_tag), true}, + {offsetof(struct smbios_type4, part_number), + FIELD_SIZEOF(struct smbios_type4, part_number), true}, + {offsetof(struct smbios_type4, core_count), + FIELD_SIZEOF(struct smbios_type4, core_count), false}, + {offsetof(struct smbios_type4, core_enabled), + FIELD_SIZEOF(struct smbios_type4, core_enabled), false}, + {offsetof(struct smbios_type4, thread_count), + FIELD_SIZEOF(struct smbios_type4, thread_count), false}, + {offsetof(struct smbios_type4, core_count2), + FIELD_SIZEOF(struct smbios_type4, core_count2), false}, + {offsetof(struct smbios_type4, core_enabled2), + FIELD_SIZEOF(struct smbios_type4, core_enabled2), false}, + {offsetof(struct smbios_type4, thread_count), + FIELD_SIZEOF(struct smbios_type4, thread_count2), false}, + {offsetof(struct smbios_type4, voltage), + FIELD_SIZEOF(struct smbios_type4, voltage), false}, +}; + +struct smbios_filter_table smbios_filter_tables[] = { + {SMBIOS_SYSTEM_INFORMATION, smbios_type1_filter_params, + ARRAY_SIZE(smbios_type1_filter_params)}, + {SMBIOS_BOARD_INFORMATION, smbios_type2_filter_params, + ARRAY_SIZE(smbios_type2_filter_params)}, + {SMBIOS_SYSTEM_ENCLOSURE, smbios_type3_filter_params, + ARRAY_SIZE(smbios_type3_filter_params)}, + {SMBIOS_PROCESSOR_INFORMATION, smbios_type4_filter_params, + ARRAY_SIZE(smbios_type4_filter_params)}, +}; + +static void clear_smbios_table(struct smbios_header *header, + struct smbios_filter_param *filter, + u32 count) +{ + u32 i; + char *str; + u8 string_id; + + for (i = 0; i < count; i++) { + if (filter[i].is_string) { + string_id = *((u8 *)header + filter[i].offset); + if (string_id == 0) /* string is empty */ + continue; + /* + * smbios_string() returns const value, but this + * function needs to clear the string. + */ + str = (char *)smbios_string(header, string_id); + if (!str) + continue; + /* string is cleared to space */ + memset(str, ' ', strlen(str)); + + } else { + memset((void *)((u8 *)header + filter[i].offset), + 0, filter[i].size); + } + } +} + +void smbios_prepare_measurement(const struct smbios_entry *entry, + struct smbios_header *smbios_copy) +{ + u32 i, j; + struct smbios_header *header; + + for (i = 0; i < ARRAY_SIZE(smbios_filter_tables); i++) { + header = smbios_copy; + for (j = 0; j < entry->struct_count; j++) { + if (header->type == smbios_filter_tables[i].type) + break; + /* + * next_header() returns the const value, but some + * members need to be cleared for the measurement. + */ + header = (struct smbios_header *)next_header(header); + } + if (j >= entry->struct_count) + continue; + + clear_smbios_table(header, + smbios_filter_tables[i].params, + smbios_filter_tables[i].count); + } +}
TCG PC Client spec requires to measure the SMBIOS table that contain static configuration information (e.g. Platform Manufacturer Enterprise Number assigned by IANA, platform model number, Vendor and Device IDs for each SMBIOS table). The device and environment dependent information such as serial number is cleared to zero or space character for the measurement. This commit also fixes the following compiler warning: lib/smbios-parser.c:59:39: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] const struct smbios_header *header = (struct smbios_header *)entry->struct_table_address; Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> --- include/efi_loader.h | 2 + include/efi_tcg2.h | 15 ++++ include/smbios.h | 13 ++++ lib/efi_loader/Kconfig | 1 + lib/efi_loader/efi_boottime.c | 2 + lib/efi_loader/efi_smbios.c | 2 - lib/efi_loader/efi_tcg2.c | 84 ++++++++++++++++++++++ lib/smbios-parser.c | 127 +++++++++++++++++++++++++++++++++- 8 files changed, 243 insertions(+), 3 deletions(-) -- 2.17.1