Re: [PATCH v4 2/6] tpm: Support boot measurements

Eddie James
2023-02-02T16:25:01Z

On 1/26/23 01:51, Ilias Apalodimas wrote:
> Hi Eddie,
>
> Thanks for the cleanup! Unfortunately this doesn't compile with EFI
> selected, but in general it looks pretty good.

Thanks, yes I forgot to remove tcg2_pcr_read

>
> On Wed, Jan 25, 2023 at 11:18:06AM -0600, Eddie James wrote:
>> Add TPM2 functions to support boot measurement. This includes
>> starting up the TPM, initializing/appending the event log, and
>> measuring the U-Boot version. Much of the code was used in the
>> EFI subsystem, so remove it there and use the common functions.
>>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> ---
>> include/efi_tcg2.h | 44 --
>> include/tpm-v2.h | 254 ++++++++++
>> lib/efi_loader/efi_tcg2.c | 975 +++-----------------------------------
>> lib/tpm-v2.c | 799 +++++++++++++++++++++++++++++++
>> 4 files changed, 1129 insertions(+), 943 deletions(-)
>>
>> HR_NV_INDEX = TPM_HT_NV_INDEX << HR_SHIFT,
>> };
>>
> [...]
>
>> }
>>
>> +}
>> +
>> +int tcg2_measure_data(struct udevice *dev, struct tcg2_event_log *elog,
>> + u32 pcr_index, u32 size, const u8 *data, u32 event_type,
>> + u32 event_size, const u8 *event)
>> +{
>> + struct tpml_digest_values digest_list;
>> + int rc;
>> +
>> + rc = tcg2_create_digest(dev, data, size, &digest_list);
>> + if (rc)
>> + return rc;
>> +
>> + rc = tcg2_pcr_extend(dev, pcr_index, &digest_list);
>> + if (rc)
>> + return rc;
>> +
>> + return tcg2_log_append_check(elog, pcr_index, event_type, &digest_list,
>> + event_size, event);
>> +}
>> +
>> +int tcg2_measure_event(struct udevice *dev, struct tcg2_event_log *elog,
>> + u32 pcr_index, u32 event_type, u32 size,
>> + const u8 *event)
>> +{
>> + struct tpml_digest_values digest_list;
>> + int rc;
>> +
>> + rc = tcg2_create_digest(dev, event, size, &digest_list);
>> + if (rc)
>> + return rc;
>> +
>> + rc = tcg2_pcr_extend(dev, pcr_index, &digest_list);
>> + if (rc)
>> + return rc;
>> +
>> + return tcg2_log_append_check(elog, pcr_index, event_type, &digest_list,
>> + size, event);
>> +}
> The only difference between these 2 is the measured data. Can't we make
> one function? If data = NULL we could just measure the event

Ok, yes that should work. I'll probably add a macro for
tcg2_measure_event that fills in NULL and 0 for data and size for
convenience.

>
>> +
>> +int tcg2_init_log(struct udevice *dev, struct tcg2_event_log *elog)
>> +{
>> + int rc;
>> +
>> + rc = tcg2_platform_get_log(dev, (void **)&elog->log, &elog->log_size);
>> + if (rc)
>> + return rc;
>> +
>> + elog->log_position = 0;
>> + elog->found = false;
>> + rc = tcg2_log_parse(dev, elog);
>> + if (rc)
>> + return rc;
>> +
>> + if (!elog->found) {
>> + rc = tcg2_log_init(dev, elog);
>> + if (rc)
>> + return rc;
> You can get rid of this if and just return rc on the function

Ack.

>
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +int tcg2_measurement_init(struct udevice **dev, struct tcg2_event_log *elog)
>> +{
>> + int rc;
>> +
>> + rc = tcg2_platform_get_tpm2(dev);
>> + if (rc)
>> + return rc;
>> +
>> + rc = tpm_init(*dev);
>> + if (rc)
>> + return rc;
>> +
>> + rc = tpm2_startup(*dev, TPM2_SU_CLEAR);
>> + if (rc) {
>> + tcg2_platform_startup_error(*dev, rc);
>> + return rc;
>> + }
>> +
>> + rc = tpm2_self_test(*dev, TPMI_YES);
>> + if (rc)
>> + printf("%s: self test error, continuing.\n", __func__);
> This is the correct init sequence of the TPM. I was trying to solve
> something similar a few days ago [0]. I haven't merged that patch yet,
> but I will send a new version that also calls tpm_init() and doesn't exit
> on -EBUSY. Can you use that patch instead of open coding the init
> sequence?

Yes I'd be happy to use it. I'll just base my series on yours.

> Also we should be bailing out on selftest errors?'

Maybe so, yes. Whatever behavior your auto startup function implements
will be fine, I'm sure.

>
> The problem here is that with U-Boot's lazy binding we might need to init
> the TPM in other subsystems and at the very least we can have a function
> doing that for us reliably.
>
>> +
>> + rc = tcg2_init_log(*dev, elog);
>> + if (rc) {
>> + tcg2_measurement_term(*dev, elog, true);
>> + return rc;
>> + }
>> +
>> + rc = tcg2_measure_event(*dev, elog, 0, EV_S_CRTM_VERSION,
>> + strlen(version_string) + 1,
>> + (u8 *)version_string);
>> + if (rc) {
>> + tcg2_measurement_term(*dev, elog, true);
>> + return rc;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +void tcg2_measurement_term(struct udevice *dev, struct tcg2_event_log *elog,
>> + bool error)
>> +{
>> + u32 event = error ? 0x1 : 0xffffffff;
>> + int i;
>> +
>> + for (i = 0; i < 8; ++i)
>> + tcg2_measure_event(dev, elog, i, EV_SEPARATOR, sizeof(event),
>> + (const u8 *)&event);
> Is the separator event at the end of the eventlog described in the spec?

Yes.
https://trustedcomputinggroup.org/wp-content/uploads/TCG_ServerManagDomainFWProfile_r1p00_pub.pdf

See section 7.1.1

>
>> +
>> + if (elog->log)
>> + unmap_physmem(elog->log, MAP_NOCACHE);
>> +}
>> +
>> +__weak int tcg2_platform_get_log(struct udevice *dev, void **addr, u32 *size)
>> +{
>> + const __be32 *addr_prop;
>> + const __be32 *size_prop;
>> + int asize;
>> + int ssize;
>> +
>> + *addr = NULL;
>> + *size = 0;
>> +
>> + addr_prop = dev_read_prop(dev, "linux,sml-base", &asize);
>> + if (!addr_prop)
>> + addr_prop = dev_read_prop(dev, "tpm_event_log_addr", &asize);
>> + size_prop = dev_read_prop(dev, "linux,sml-size", &ssize);
>> + if (!size_prop)
>> + size_prop = dev_read_prop(dev, "tpm_event_log_size", &ssize);
>> + if (addr_prop && size_prop) {
>> + u64 a = of_read_number(addr_prop, asize / sizeof(__be32));
>> + u64 s = of_read_number(size_prop, ssize / sizeof(__be32));
>> +
>> + *addr = map_physmem(a, s, MAP_NOCACHE);
>> + *size = (u32)s;
>> + } else {
>> + struct ofnode_phandle_args args;
>> + phys_addr_t a;
>> + phys_size_t s;
>> +
>> + if (dev_read_phandle_with_args(dev, "memory-region", NULL, 0,
>> + 0, &args))
>> + return -ENODEV;
>> +
>> + a = ofnode_get_addr_size(args.node, "reg", &s);
>> + if (a == FDT_ADDR_T_NONE)
>> + return -ENOMEM;
>> +
>> + *addr = map_physmem(a, s, MAP_NOCACHE);
>> + *size = (u32)s;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +__weak int tcg2_platform_get_tpm2(struct udevice **dev)
>> +{
>> + for_each_tpm_device(*dev) {
>> + if (tpm_get_version(*dev) == TPM_V2)
>> + return 0;
>> + }
>> +
>> + return -ENODEV;
>> +}
>> +
>> +__weak void tcg2_platform_startup_error(struct udevice *dev, int rc) {}
>> +
>> u32 tpm2_startup(struct udevice *dev, enum tpm2_startup_types mode)
>> {
>> const u8 command_v2[12] = {
>> @@ -342,6 +1016,131 @@ u32 tpm2_get_capability(struct udevice *dev, u32 capability, u32 property,
>> return 0;
>> }
>>
>> +static int tpm2_get_num_pcr(struct udevice *dev, u32 *num_pcr)
>> +{
>> + u8 response[(sizeof(struct tpms_capability_data) -
>> + offsetof(struct tpms_capability_data, data))];
>> + u32 properties_offset =
>> + offsetof(struct tpml_tagged_tpm_property, tpm_property) +
>> + offsetof(struct tpms_tagged_property, value);
>> + u32 ret;
>> +
>> + memset(response, 0, sizeof(response));
>> + ret = tpm2_get_capability(dev, TPM2_CAP_TPM_PROPERTIES,
>> + TPM2_PT_PCR_COUNT, response, 1);
>> + if (ret)
>> + return ret;
>> +
>> + *num_pcr = get_unaligned_be32(response + properties_offset);
>> + if (*num_pcr > TPM2_MAX_PCRS) {
>> + printf("%s: too many pcrs: %u\n", __func__, *num_pcr);
>> + return -E2BIG;
>> + }
>> +
>> + return 0;
>> +}
>> +
> [...]
>
> [0] https://lore.kernel.org/u-boot/20230125144851.532154-1-ilias.apalodimas@linaro.org/
>
> Thanks
> /Ilias