| Message ID | 20210921071931.3755-2-masahisa.kojima@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enhance Measured Boot | expand |
Hi Masahisa, On Tue, 21 Sept 2021 at 01:17, Masahisa Kojima <masahisa.kojima@linaro.org> 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 device- and environment-dependent > serial number is cleared to zero or space character for > the measurement. > > Existing smbios_string() function returns pointer to the string > with const qualifier, but exisintg use case is updating version > string and const qualifier must be removed. > This commit removes const qualifier from smbios_string() > return value and reuses to clear the strings 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> > --- > > Changes in v2: > - use flexible array for table_entry field > - modify funtion name to find_smbios_table() > - remove unnecessary const qualifier from smbios_string() > - create non-const version of next_header() > > include/efi_loader.h | 2 + > include/efi_tcg2.h | 15 ++++ > include/smbios.h | 17 +++- > 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 | 152 +++++++++++++++++++++++++++++++--- > 8 files changed, 261 insertions(+), 14 deletions(-) Where are the tests for this new code, please? Would it make sense to have a function that iterates through the data that does need to be hashed, instead? Regards, Simon
Hi Simon, On Wed, 22 Sept 2021 at 19:19, Simon Glass <sjg@chromium.org> wrote: > > Hi Masahisa, > > On Tue, 21 Sept 2021 at 01:17, Masahisa Kojima > <masahisa.kojima@linaro.org> 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 > > device- and environment-dependent > > > serial number is cleared to zero or space character for > > the measurement. > > > > Existing smbios_string() function returns pointer to the string > > with const qualifier, but exisintg use case is updating version > > string and const qualifier must be removed. > > This commit removes const qualifier from smbios_string() > > return value and reuses to clear the strings 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> > > --- > > > > Changes in v2: > > - use flexible array for table_entry field > > - modify funtion name to find_smbios_table() > > - remove unnecessary const qualifier from smbios_string() > > - create non-const version of next_header() > > > > include/efi_loader.h | 2 + > > include/efi_tcg2.h | 15 ++++ > > include/smbios.h | 17 +++- > > 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 | 152 +++++++++++++++++++++++++++++++--- > > 8 files changed, 261 insertions(+), 14 deletions(-) > > Where are the tests for this new code, please? We've mentioned this in the past. The sandbox TPM is very limited wrt tpm testing for the EFI TCG protocol. I did send TPM MMIO patches a while back [1]. This would allow us to test everything under QEMU, but you asked for *another* device to be part of the API I posted (apart from the MMIO). I've found some time and changed the tpm2 spi driver we have, but I can't test it yet, since I don't have a device for that. [1] https://lore.kernel.org/u-boot/20210707162604.84196-1-ilias.apalodimas@linaro.org/ Cheers /Ilias > > Would it make sense to have a function that iterates through the data > that does need to be hashed, instead? > > Regards, > Simon
Hi Ilias, On Thu, 23 Sept 2021 at 03:17, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Hi Simon, > > On Wed, 22 Sept 2021 at 19:19, Simon Glass <sjg@chromium.org> wrote: > > > > Hi Masahisa, > > > > On Tue, 21 Sept 2021 at 01:17, Masahisa Kojima > > <masahisa.kojima@linaro.org> 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 > > > > device- and environment-dependent > > > > > serial number is cleared to zero or space character for > > > the measurement. > > > > > > Existing smbios_string() function returns pointer to the string > > > with const qualifier, but exisintg use case is updating version > > > string and const qualifier must be removed. > > > This commit removes const qualifier from smbios_string() > > > return value and reuses to clear the strings 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> > > > --- > > > > > > Changes in v2: > > > - use flexible array for table_entry field > > > - modify funtion name to find_smbios_table() > > > - remove unnecessary const qualifier from smbios_string() > > > - create non-const version of next_header() > > > > > > include/efi_loader.h | 2 + > > > include/efi_tcg2.h | 15 ++++ > > > include/smbios.h | 17 +++- > > > 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 | 152 +++++++++++++++++++++++++++++++--- > > > 8 files changed, 261 insertions(+), 14 deletions(-) > > > > Where are the tests for this new code, please? > > We've mentioned this in the past. The sandbox TPM is very limited wrt > tpm testing for the EFI TCG protocol. So let's add some more features? If it helps, think of the sandbox TPM as test code, not an emulator. It is a very simple kind of emulator to allow tests to work. > I did send TPM MMIO patches a while back [1]. This would allow us to > test everything under QEMU, but you asked for *another* device to be > part of the API I posted (apart from the MMIO). I've found some time Yes that is because if you just add a new protocol you have not made anything better, just added one more way of doing things. > and changed the tpm2 spi driver we have, but I can't test it yet, > since I don't have a device for that. OK I think we are both going to get one. [..] Regards, SImon
Hi Simon, [...] > > > > - remove unnecessary const qualifier from smbios_string() > > > > - create non-const version of next_header() > > > > > > > > include/efi_loader.h | 2 + > > > > include/efi_tcg2.h | 15 ++++ > > > > include/smbios.h | 17 +++- > > > > 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 | 152 +++++++++++++++++++++++++++++++--- > > > > 8 files changed, 261 insertions(+), 14 deletions(-) > > > > > > Where are the tests for this new code, please? > > > > We've mentioned this in the past. The sandbox TPM is very limited wrt > > tpm testing for the EFI TCG protocol. > > So let's add some more features? If it helps, think of the sandbox TPM > as test code, not an emulator. It is a very simple kind of emulator to > allow tests to work. The amount of features needed to test EFI TCG are not minimal. Since I'll upstream the mmio tpm anyway, we'll just test TCG there. If someone wants to go ahead and make the sandbox TPM a TIS compliant device that covers the requirements of the EFI TCG, I am fine using it. > > > I did send TPM MMIO patches a while back [1]. This would allow us to > > test everything under QEMU, but you asked for *another* device to be > > part of the API I posted (apart from the MMIO). I've found some time > > Yes that is because if you just add a new protocol you have not made > anything better, just added one more way of doing things. Our perspective of 'better' seems to be different. I added a TIS API for any driver to use. I actually did 2 iterations of the driver. The first one was replicating all the code and you said 'why are we replicating code', which was done already in a bunch of drivers already... Then I added an API and a driver using it but you wanted to convert more *existing* drivers to the API before merging it. But the fact is that if anyone wants to add a new driver he has to code ~900 lines instead of the ~150 needed with the API in place, not to mention the duplication of bugs all over the place.... > > > and changed the tpm2 spi driver we have, but I can't test it yet, > > since I don't have a device for that. > > OK I think we are both going to get one. > > [..] > > Regards, > SImon Regards /Ilias
Hi Ilias, On Mon, 27 Sept 2021 at 02:52, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Hi Simon, > > [...] > > > > > > - remove unnecessary const qualifier from smbios_string() > > > > > - create non-const version of next_header() > > > > > > > > > > include/efi_loader.h | 2 + > > > > > include/efi_tcg2.h | 15 ++++ > > > > > include/smbios.h | 17 +++- > > > > > 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 | 152 +++++++++++++++++++++++++++++++--- > > > > > 8 files changed, 261 insertions(+), 14 deletions(-) > > > > > > > > Where are the tests for this new code, please? > > > > > > We've mentioned this in the past. The sandbox TPM is very limited wrt > > > tpm testing for the EFI TCG protocol. > > > > So let's add some more features? If it helps, think of the sandbox TPM > > as test code, not an emulator. It is a very simple kind of emulator to > > allow tests to work. > > The amount of features needed to test EFI TCG are not minimal. Since I'll > upstream the mmio tpm anyway, we'll just test TCG there. If someone wants > to go ahead and make the sandbox TPM a TIS compliant device that covers the > requirements of the EFI TCG, I am fine using it. Do you know how many features? There's 250 LOC in this patch. > > > > > > I did send TPM MMIO patches a while back [1]. This would allow us to > > > test everything under QEMU, but you asked for *another* device to be > > > part of the API I posted (apart from the MMIO). I've found some time > > > > Yes that is because if you just add a new protocol you have not made > > anything better, just added one more way of doing things. > > Our perspective of 'better' seems to be different. > > I added a TIS API for any driver to use. I actually did 2 iterations of > the driver. The first one was replicating all the code and you said 'why > are we replicating code', which was done already in a bunch of drivers > already... > Then I added an API and a driver using it but you wanted to convert more > *existing* drivers to the API before merging it. But the fact is that if > anyone wants to add a new driver he has to code ~900 lines instead of the > ~150 needed with the API in place, not to mention the duplication of bugs > all over the place.... It would be like adding a new filesystem in U-Boot with its own new framework for filesystems. It creates technical debt and we don't know if anyone will actually use it. https://xkcd.com/927/ I think your API is a great idea but we need some effort to migrate to it, to avoid the problem above. After all, who else is going to do it? Regards, Simon
Hi Simon, [...] > > > > We've mentioned this in the past. The sandbox TPM is very limited wrt > > > > tpm testing for the EFI TCG protocol. > > > > > > So let's add some more features? If it helps, think of the sandbox TPM > > > as test code, not an emulator. It is a very simple kind of emulator to > > > allow tests to work. > > > > The amount of features needed to test EFI TCG are not minimal. Since I'll > > upstream the mmio tpm anyway, we'll just test TCG there. If someone wants > > to go ahead and make the sandbox TPM a TIS compliant device that covers the > > requirements of the EFI TCG, I am fine using it. > > Do you know how many features? There's 250 LOC in this patch. I haven't checked for a while but back when I tested it tpm2_get_capability() was failing on a number of cases. The EFI TCG code expects: - TPM2_PT_MAX_COMMAND_SIZE - TPM2_PT_MAX_RESPONSE_SIZE - TPM2_PT_MANUFACTURER - TPM2_PT_PCR_COUNT - TPM2_CAP_PCRS when querying capabilities. Ideally we'd also want to extend more than 1 PCRs and verify that worked correctly. > > > > > > > > > > I did send TPM MMIO patches a while back [1]. This would allow us to > > > > test everything under QEMU, but you asked for *another* device to be > > > > part of the API I posted (apart from the MMIO). I've found some time > > > > > > Yes that is because if you just add a new protocol you have not made > > > anything better, just added one more way of doing things. > > > > Our perspective of 'better' seems to be different. > > > > I added a TIS API for any driver to use. I actually did 2 iterations of > > the driver. The first one was replicating all the code and you said 'why > > are we replicating code', which was done already in a bunch of drivers > > already... > > Then I added an API and a driver using it but you wanted to convert more > > *existing* drivers to the API before merging it. But the fact is that if > > anyone wants to add a new driver he has to code ~900 lines instead of the > > ~150 needed with the API in place, not to mention the duplication of bugs > > all over the place.... > > It would be like adding a new filesystem in U-Boot with its own new > framework for filesystems. It creates technical debt and we don't know > if anyone will actually use it. > > https://xkcd.com/927/ > > I think your API is a great idea but we need some effort to migrate to > it, to avoid the problem above. After all, who else is going to do it? I ordered the SPI TPM, so hopefully, I'll be able to have the MMIO and SPI drivers using it! Cheers /Ilias
On Thu, 23 Sept 2021 at 01:19, Simon Glass <sjg@chromium.org> wrote: > > Hi Masahisa, > > On Tue, 21 Sept 2021 at 01:17, Masahisa Kojima > <masahisa.kojima@linaro.org> 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 > > device- and environment-dependent > > > serial number is cleared to zero or space character for > > the measurement. > > > > Existing smbios_string() function returns pointer to the string > > with const qualifier, but exisintg use case is updating version > > string and const qualifier must be removed. > > This commit removes const qualifier from smbios_string() > > return value and reuses to clear the strings 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> > > --- > > > > Changes in v2: > > - use flexible array for table_entry field > > - modify funtion name to find_smbios_table() > > - remove unnecessary const qualifier from smbios_string() > > - create non-const version of next_header() > > > > include/efi_loader.h | 2 + > > include/efi_tcg2.h | 15 ++++ > > include/smbios.h | 17 +++- > > 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 | 152 +++++++++++++++++++++++++++++++--- > > 8 files changed, 261 insertions(+), 14 deletions(-) > > Where are the tests for this new code, please? > > Would it make sense to have a function that iterates through the data > that does need to be hashed, instead? I agree that it is straightforward if we iterates the data to be hashed. But the data NOT to be hashed is less than the data to be hashed, and this is what edk2 implements, we can easily compare with edk2. So I would like to keep current implementation. Thanks, Masahisa Kojima > > Regards, > Simon
Hi Ilias, On Tue, 28 Sept 2021 at 11:41, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Hi Simon, > > > [...] > > > > > We've mentioned this in the past. The sandbox TPM is very limited wrt > > > > > tpm testing for the EFI TCG protocol. > > > > > > > > So let's add some more features? If it helps, think of the sandbox TPM > > > > as test code, not an emulator. It is a very simple kind of emulator to > > > > allow tests to work. > > > > > > The amount of features needed to test EFI TCG are not minimal. Since I'll > > > upstream the mmio tpm anyway, we'll just test TCG there. If someone wants > > > to go ahead and make the sandbox TPM a TIS compliant device that covers the > > > requirements of the EFI TCG, I am fine using it. > > > > Do you know how many features? There's 250 LOC in this patch. > > I haven't checked for a while but back when I tested it > tpm2_get_capability() was failing on a number of cases. The EFI TCG > code expects: > - TPM2_PT_MAX_COMMAND_SIZE > - TPM2_PT_MAX_RESPONSE_SIZE > - TPM2_PT_MANUFACTURER > - TPM2_PT_PCR_COUNT > - TPM2_CAP_PCRS > when querying capabilities. Ideally we'd also want to extend more > than 1 PCRs and verify that worked correctly. That doesn't seem like much. As you say I already added support for multiple PCRs. I think we should do it and add a test. > > > > > > > > > > > > > > > I did send TPM MMIO patches a while back [1]. This would allow us to > > > > > test everything under QEMU, but you asked for *another* device to be > > > > > part of the API I posted (apart from the MMIO). I've found some time > > > > > > > > Yes that is because if you just add a new protocol you have not made > > > > anything better, just added one more way of doing things. > > > > > > Our perspective of 'better' seems to be different. > > > > > > I added a TIS API for any driver to use. I actually did 2 iterations of > > > the driver. The first one was replicating all the code and you said 'why > > > are we replicating code', which was done already in a bunch of drivers > > > already... > > > Then I added an API and a driver using it but you wanted to convert more > > > *existing* drivers to the API before merging it. But the fact is that if > > > anyone wants to add a new driver he has to code ~900 lines instead of the > > > ~150 needed with the API in place, not to mention the duplication of bugs > > > all over the place.... > > > > It would be like adding a new filesystem in U-Boot with its own new > > framework for filesystems. It creates technical debt and we don't know > > if anyone will actually use it. > > > > https://xkcd.com/927/ > > > > I think your API is a great idea but we need some effort to migrate to > > it, to avoid the problem above. After all, who else is going to do it? > > I ordered the SPI TPM, so hopefully, I'll be able to have the MMIO and > SPI drivers using it! Snap! Regards, Simon
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..85a032dbbd 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[]; +} __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..acfcbfe2ca 100644 --- a/include/smbios.h +++ b/include/smbios.h @@ -260,9 +260,9 @@ const struct smbios_header *smbios_header(const struct smbios_entry *entry, int * * @header: pointer to struct smbios_header * @index: string index - * @return: NULL or a valid const char pointer + * @return: NULL or a valid char pointer */ -const char *smbios_string(const struct smbios_header *header, int index); +char *smbios_string(const struct smbios_header *header, int index); /** * smbios_update_version() - Update the version string @@ -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 f4e9129a39..da68a219a3 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -328,6 +328,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..4f68f6dfd5 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) + + FIELD_SIZEOF(struct efi_configuration_table, guid) + + entry->struct_table_length; + 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; +} + +/** + * find_smbios_table() - find smbios table + * + * Return: pointer to the smbios table + */ +static void *find_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 *)find_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..596a967302 100644 --- a/lib/smbios-parser.c +++ b/lib/smbios-parser.c @@ -39,10 +39,8 @@ const struct smbios_entry *smbios_entry(u64 address, u32 size) return entry; } -static const struct smbios_header *next_header(const struct smbios_header *curr) +static u8 *find_next_header(u8 *pos) { - u8 *pos = ((u8 *)curr) + curr->length; - /* search for _double_ NULL bytes */ while (!((*pos == 0) && (*(pos + 1) == 0))) pos++; @@ -50,13 +48,27 @@ static const struct smbios_header *next_header(const struct smbios_header *curr) /* step behind the double NULL bytes */ pos += 2; - return (struct smbios_header *)pos; + return pos; +} + +static struct smbios_header *get_next_header(struct smbios_header *curr) +{ + u8 *pos = ((u8 *)curr) + curr->length; + + return (struct smbios_header *)find_next_header(pos); +} + +static const struct smbios_header *next_header(const struct smbios_header *curr) +{ + u8 *pos = ((u8 *)curr) + curr->length; + + return (struct smbios_header *)find_next_header(pos); } 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) @@ -68,8 +80,8 @@ const struct smbios_header *smbios_header(const struct smbios_entry *entry, int return NULL; } -static const char *string_from_smbios_table(const struct smbios_header *header, - int idx) +static char *string_from_smbios_table(const struct smbios_header *header, + int idx) { unsigned int i = 1; u8 *pos; @@ -86,10 +98,10 @@ static const char *string_from_smbios_table(const struct smbios_header *header, pos++; } - return (const char *)pos; + return (char *)pos; } -const char *smbios_string(const struct smbios_header *header, int index) +char *smbios_string(const struct smbios_header *header, int index) { if (!header) return NULL; @@ -109,7 +121,7 @@ int smbios_update_version_full(void *smbios_tab, const char *version) if (!hdr) return log_msg_ret("tab", -ENOENT); bios = (struct smbios_type0 *)hdr; - ptr = (char *)smbios_string(hdr, bios->bios_ver); + ptr = smbios_string(hdr, bios->bios_ver); if (!ptr) return log_msg_ret("str", -ENOMEDIUM); @@ -132,3 +144,123 @@ 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; + + str = smbios_string(header, string_id); + if (!str) + continue; + + /* string is cleared to space, keep '\0' terminator */ + 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; + + header = get_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. Existing smbios_string() function returns pointer to the string with const qualifier, but exisintg use case is updating version string and const qualifier must be removed. This commit removes const qualifier from smbios_string() return value and reuses to clear the strings 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> --- Changes in v2: - use flexible array for table_entry field - modify funtion name to find_smbios_table() - remove unnecessary const qualifier from smbios_string() - create non-const version of next_header() include/efi_loader.h | 2 + include/efi_tcg2.h | 15 ++++ include/smbios.h | 17 +++- 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 | 152 +++++++++++++++++++++++++++++++--- 8 files changed, 261 insertions(+), 14 deletions(-) -- 2.17.1