[PATCH v7 1/3] efi_loader: Add check for event log passed from firmware
Ilias Apalodimas
ilias.apalodimas at linaro.org
Fri Nov 26 21:58:23 CET 2021
Hi Heinrich,
> > +}
> > > +
> > > +/**
> > > + * parse_specid_event() - Parse and verify the specID Event in the eventlog
> > > + *
> > > + * @dev: udevice
> > > + * @buffer: Pointer to the start of the eventlog
> > > + * @log_size: Size of the eventlog
> > > + * @pos: [in] Offset of specID event in the eventlog buffer
> > > + * [out] Return offset of the next event in the buffer
> > > + * after the specID
> > > + * @digest_list: list of digests in the event
> > > + *
> > > + * Return: status code
> > > + * @pos Offset in the eventlog where the specID event ends
> > > + * @digest_list: list of digests in the event
> > > + */
> > > +static efi_status_t parse_specid_event(struct udevice *dev, void *buffer,
> > > + u32 log_size, u32 *pos,
> > > + struct tpml_digest_values *digest_list)
> > > +{
> > > + struct tcg_efi_spec_id_event *spec_event;
> > > + struct tcg_pcr_event *event_header = (struct tcg_pcr_event *)buffer;
> > > + size_t spec_event_size;
> > > + u32 active = 0, supported = 0, pcr_count = 0, alg_count = 0;
> > > + u32 spec_active = 0;
> > > + u16 hash_alg, hash_sz;
> > > + u8 vendor_sz;
> > > + int err, i;
> > > +
> > > + if (*pos >= log_size || (*pos + sizeof(*spec_event)) > log_size)
> > > + return EFI_COMPROMISED_DATA;
> > > +
> > > + /* Check specID event data */
> > > + spec_event = (struct tcg_efi_spec_id_event *)((uintptr_t)buffer + *pos);
> > > + /* Check for signature */
> > > + if (memcmp(spec_event->signature, TCG_EFI_SPEC_ID_EVENT_SIGNATURE_03,
> > > + sizeof(TCG_EFI_SPEC_ID_EVENT_SIGNATURE_03))) {
> > > + log_err("specID Event: Signature mismatch\n");
> > > + return EFI_COMPROMISED_DATA;
> > > + }
> > > +
> > > + if (spec_event->spec_version_minor !=
> > > + TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MINOR_TPM2 ||
> > > + spec_event->spec_version_major !=
> > > + TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MAJOR_TPM2)
> > > + return EFI_COMPROMISED_DATA;
> > > +
> > > + if (spec_event->number_of_algorithms > MAX_HASH_COUNT ||
> > > + spec_event->number_of_algorithms < 1) {
> > > + log_err("specID Event: Number of algorithms incorrect\n");
> > > + return EFI_COMPROMISED_DATA;
> > > + }
> > > +
> > > + alg_count = spec_event->number_of_algorithms;
> > > +
> > > + err = tpm2_get_pcr_info(dev, &supported, &active, &pcr_count);
> > > + if (err)
> > > + return EFI_DEVICE_ERROR;
> > > +
> > > + digest_list->count = 0;
> > > + /*
> > > + * We have to take care that the sequence of algorithms that we record
> > > + * in digest_list matches the sequence in eventlog.
> > > + */
> > > + for (i = 0; i < alg_count; i++) {
> > > + hash_alg =
> > > + get_unaligned_le16(&spec_event->digest_sizes[i].algorithm_id);
> > > + hash_sz =
> > > + get_unaligned_le16(&spec_event->digest_sizes[i].digest_size);
This is unused. Do we need it ?
> > > +
> > > + if (!(supported & alg_to_mask(hash_alg))) {
> > > + log_err("specID Event: Unsupported algorithm\n");
> > > + return EFI_COMPROMISED_DATA;
> > > + }
> > > + digest_list->digests[digest_list->count++].hash_alg = hash_alg;
> > > +
> > > + spec_active |= alg_to_mask(hash_alg);
> > > + }
> > > +
> > > + /*
> > > + * TCG specification expects the event log to have hashes for all
> > > + * active PCR's
> > > + */
> > > + if (spec_active != active) {
> > > + /*
> > > + * Previous stage bootloader should know all the active PCR's
> > > + * and use them in the Eventlog.
> > > + */
> > > + log_err("specID Event: All active hash alg not present\n");
> > > + return EFI_COMPROMISED_DATA;
> > > + }
> > > +
> > > + /*
> > > + * the size of the spec event and placement of vendor_info_size
> > > + * depends on supported algoriths
> > > + */
> > > + spec_event_size =
> > > + offsetof(struct tcg_efi_spec_id_event, digest_sizes) +
> > > + alg_count * sizeof(spec_event->digest_sizes[0]);
> > > +
> > > + if (*pos + spec_event_size >= log_size)
> > > + return EFI_COMPROMISED_DATA;
> > > +
> > > + vendor_sz = *(uint8_t *)((uintptr_t)buffer + *pos + spec_event_size);
> > > +
> > > + spec_event_size += sizeof(vendor_sz) + vendor_sz;
> > > + *pos += spec_event_size;
> > > +
> > > + if (get_unaligned_le32(&event_header->event_size) != spec_event_size) {
> > > + log_err("specID event: header event size mismatch\n");
> > > + /* Right way to handle this can be to call SetActive PCR's */
> > > + return EFI_COMPROMISED_DATA;
> > > + }
> > > +
> > > + return EFI_SUCCESS;
> > > +}
> > > +
> > > +/**
> > > + * tcg2_parse_event() - Parse the event in the eventlog
> > > + *
> > > + * @dev: udevice
> > > + * @buffer: Pointer to the start of the eventlog
> > > + * @log_size: Size of the eventlog
> > > + * @offset: [in] Offset of the event in the eventlog buffer
> > > + * [out] Return offset of the next event in the buffer
> > > + * @digest_list: list of digests in the event
> > > + * @pcr Index of the PCR in the event
> > > + *
> > > + * Return: status code
> > > + */
> > > +static efi_status_t tcg2_parse_event(struct udevice *dev, void *buffer,
> > > + u32 log_size, u32 *offset,
> > > + struct tpml_digest_values *digest_list,
> > > + u32 *pcr)
> > > +{
> > > + struct tcg_pcr_event2 *event = NULL;
> > > + u32 event_type, count, size, event_size;
> > > + size_t pos;
> > > +
> > > + event_size = tcg_event_final_size(digest_list);
> > > + if (*offset >= log_size || *offset + event_size > log_size) {
> > > + log_err("Event exceeds log size\n");
> > > + return EFI_COMPROMISED_DATA;
> > > + }
> > > +
> > > + event = (struct tcg_pcr_event2 *)((uintptr_t)buffer + *offset);
> > > + *pcr = get_unaligned_le32(&event->pcr_index);
> > > + event_type = get_unaligned_le32(&event->event_type);
This seems unused?
> > > +
> > > + /* get the count */
> > > + count = get_unaligned_le32(&event->digests.count);
> > > + if (count != digest_list->count)
> > > + return EFI_COMPROMISED_DATA;
> > > +
> > > + pos = offsetof(struct tcg_pcr_event2, digests);
> > > + pos += offsetof(struct tpml_digest_values, digests);
> > > +
> > > + for (int i = 0; i < digest_list->count; i++) {
> > > + u16 alg;
> > > + u16 hash_alg = digest_list->digests[i].hash_alg;
> > > + u8 *digest = (u8 *)&digest_list->digests[i].digest;
> > > +
> > > + alg = get_unaligned_le16((void *)((uintptr_t)event + pos));
> > > +
> > > + if (alg != hash_alg)
> > > + return EFI_COMPROMISED_DATA;
> > > +
> > > + pos += offsetof(struct tpmt_ha, digest);
> > > + memcpy(digest, (void *)((uintptr_t)event + pos), alg_to_len(hash_alg));
> > > + pos += alg_to_len(hash_alg);
> > > + }
> > > +
> > > + size = get_unaligned_le32((void *)((uintptr_t)event + pos));
> > > + event_size += size;
> > > + pos += sizeof(u32); /* tcg_pcr_event2 event_size*/
> > > + pos += size;
> > > +
> > > + /* make sure the calculated buffer is what we checked against */
> > > + if (pos != event_size)
> > > + return EFI_COMPROMISED_DATA;
> > > +
> > > + if (pos > log_size)
> > > + return EFI_COMPROMISED_DATA;
> > > +
> > > + *offset += pos;
> > > +
> > > + return EFI_SUCCESS;
> > > +}
> > > +
> > > +/**
> > > + * tcg2_get_fw_eventlog() - Get the eventlog address and size
> > > + *
> > > + * If the previous firmware has passed some eventlog, this function get it's
> > > + * location and check for it's validity.
> > > + *
> > > + * @dev: udevice
> > > + * @log_buffer: eventlog address
> > > + * @log_sz: eventlog size
> > > + *
> > > + * Return: status code
> > > + */
> > > +static efi_status_t tcg2_get_fw_eventlog(struct udevice *dev, void *log_buffer,
> > > + size_t *log_sz)
> > > +{
> > > + struct tpml_digest_values digest_list;
> > > + void *buffer;
> > > + efi_status_t ret;
> > > + u32 pcr, pos;
> > > + u64 base;
> > > + u32 sz;
> > > +
> > > + ret = platform_get_eventlog(dev, &base, &sz);
> > > + if (ret != EFI_SUCCESS)
> > > + return ret;
> > > +
> > > + if (sz > TPM2_EVENT_LOG_SIZE)
> > > + return EFI_VOLUME_FULL;
> > > +
> > > + buffer = (void *)base;
>
> This leads to a build failure on 32bit:
>
> +lib/efi_loader/efi_tcg2.c: In function 'tcg2_get_fw_eventlog':
> +lib/efi_loader/efi_tcg2.c:1512:18: error: cast to pointer from integer
> of different size [-Werror=int-to-pointer-cast]
> + 1512 | buffer = (void *)base;
> + | ^
> +cc1: all
>
> I will correct this when merging.
>
> Best regards
>
> Heinrich
>
Don't merge this yet. The report from cppcheck is introduced by this
patchset. I'd prefer getting a v8 with those fixed.
> > > + pos = 0;
> > > + /* Parse the eventlog to check for its validity */
> > > + ret = parse_event_log_header(buffer, sz, &pos);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = parse_specid_event(dev, buffer, sz, &pos, &digest_list);
> > > + if (ret) {
> > > + log_err("Error parsing SPEC ID Event\n");
> > > + return ret;
> > > + }
> > > +
> > > + while (pos < sz) {
> > > + ret = tcg2_parse_event(dev, buffer, sz, &pos, &digest_list,
> > > + &pcr);
> > > + if (ret) {
> > > + log_err("Error parsing event\n");
> > > + return ret;
> > > + }
> > > + }
> > > +
> > > + memcpy(log_buffer, buffer, sz);
> > > + *log_sz = sz;
> > > +
> > > + return ret;
> > > +}
> > > +
> > > /**
> > > * create_specid_event() - Create the first event in the eventlog
> > > *
> > > @@ -1312,69 +1628,6 @@ out:
> > > return ret;
> > > }
> > >
> > > -/**
> > > - * efi_init_event_log() - initialize an eventlog
> > > - */
> > > -static efi_status_t efi_init_event_log(void)
> > > -{
> > > - /*
> > > - * vendor_info_size is currently set to 0, we need to change the length
> > > - * and allocate the flexible array member if this changes
> > > - */
> > > - struct tcg_pcr_event *event_header = NULL;
> > > - struct udevice *dev;
> > > - size_t spec_event_size;
> > > - efi_status_t ret;
> > > -
> > > - ret = platform_get_tpm2_device(&dev);
> > > - if (ret != EFI_SUCCESS)
> > > - goto out;
> > > -
> > > - ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, TPM2_EVENT_LOG_SIZE,
> > > - (void **)&event_log.buffer);
> > > - if (ret != EFI_SUCCESS)
> > > - goto out;
> > > -
> > > - /*
> > > - * initialize log area as 0xff so the OS can easily figure out the
> > > - * last log entry
> > > - */
> > > - memset(event_log.buffer, 0xff, TPM2_EVENT_LOG_SIZE);
> > > - event_log.pos = 0;
> > > - event_log.last_event_size = 0;
> > > - event_log.get_event_called = false;
> > > - event_log.ebs_called = false;
> > > - event_log.truncated = false;
> > > -
> > > - /*
> > > - * The log header is defined to be in SHA1 event log entry format.
> > > - * Setup event header
> > > - */
> > > - event_header = (struct tcg_pcr_event *)event_log.buffer;
> > > - put_unaligned_le32(0, &event_header->pcr_index);
> > > - put_unaligned_le32(EV_NO_ACTION, &event_header->event_type);
> > > - memset(&event_header->digest, 0, sizeof(event_header->digest));
> > > - ret = create_specid_event(dev, (void *)((uintptr_t)event_log.buffer + sizeof(*event_header)),
> > > - &spec_event_size);
> > > - if (ret != EFI_SUCCESS)
> > > - goto free_pool;
> > > - put_unaligned_le32(spec_event_size, &event_header->event_size);
> > > - event_log.pos = spec_event_size + sizeof(*event_header);
> > > - event_log.last_event_size = event_log.pos;
> > > -
> > > - ret = create_final_event();
> > > - if (ret != EFI_SUCCESS)
> > > - goto free_pool;
> > > -
> > > -out:
> > > - return ret;
> > > -
> > > -free_pool:
> > > - efi_free_pool(event_log.buffer);
> > > - event_log.buffer = NULL;
> > > - return ret;
> > > -}
> > > -
> > > /**
> > > * tcg2_measure_event() - common function to add event log and extend PCR
> > > *
> > > @@ -1427,6 +1680,93 @@ static efi_status_t efi_append_scrtm_version(struct udevice *dev)
> > > return ret;
> > > }
> > >
> > > +/**
> > > + * efi_init_event_log() - initialize an eventlog
> > > + *
> > > + * Return: status code
> > > + */
> > > +static efi_status_t efi_init_event_log(void)
> > > +{
> > > + /*
> > > + * vendor_info_size is currently set to 0, we need to change the length
> > > + * and allocate the flexible array member if this changes
> > > + */
> > > + struct tcg_pcr_event *event_header = NULL;
> > > + struct udevice *dev;
> > > + size_t spec_event_size;
> > > + efi_status_t ret;
> > > +
> > > + ret = platform_get_tpm2_device(&dev);
> > > + if (ret != EFI_SUCCESS)
> > > + return ret;
> > > +
> > > + ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, TPM2_EVENT_LOG_SIZE,
> > > + (void **)&event_log.buffer);
> > > + if (ret != EFI_SUCCESS)
> > > + return ret;
> > > +
> > > + /*
> > > + * initialize log area as 0xff so the OS can easily figure out the
> > > + * last log entry
> > > + */
> > > + memset(event_log.buffer, 0xff, TPM2_EVENT_LOG_SIZE);
> > > +
> > > + /*
> > > + * The log header is defined to be in SHA1 event log entry format.
> > > + * Setup event header
> > > + */
> > > + event_header = (struct tcg_pcr_event *)event_log.buffer;
> > > + event_log.pos = 0;
> > > + event_log.last_event_size = 0;
> > > + event_log.get_event_called = false;
> > > + event_log.ebs_called = false;
> > > + event_log.truncated = false;
> > > +
> > > + /*
> > > + * Check if earlier firmware have passed any eventlog. Different
> > > + * platforms can use different ways to do so.
> > > + */
> > > + ret = tcg2_get_fw_eventlog(dev, event_log.buffer, &event_log.pos);
> > > + /*
> > > + * If earlier firmware hasn't passed any eventlog, go ahead and
> > > + * create the eventlog header.
> > > + */
> > > + if (ret == EFI_NOT_FOUND) {
> > > + put_unaligned_le32(0, &event_header->pcr_index);
> > > + put_unaligned_le32(EV_NO_ACTION, &event_header->event_type);
> > > + memset(&event_header->digest, 0, sizeof(event_header->digest));
> > > + ret = create_specid_event(dev,
> > > + (void *)((uintptr_t)event_log.buffer +
> > > + sizeof(*event_header)),
> > > + &spec_event_size);
> > > + if (ret != EFI_SUCCESS)
> > > + goto free_pool;
> > > + put_unaligned_le32(spec_event_size, &event_header->event_size);
> > > + event_log.pos = spec_event_size + sizeof(*event_header);
> > > + event_log.last_event_size = event_log.pos;
> > > +
> > > + /*
> > > + * Add SCRTM version to the log if previous firmmware
> > > + * doesn't pass an eventlog.
> > > + */
> > > + ret = efi_append_scrtm_version(dev);
> > > + }
> > > +
> > > + if (ret != EFI_SUCCESS)
> > > + goto free_pool;
> > > +
> > > + ret = create_final_event();
> > > + if (ret != EFI_SUCCESS)
> > > + goto free_pool;
> > > +
> > > + return ret;
> > > +
> > > +free_pool:
> > > + efi_free_pool(event_log.buffer);
> > > + event_log.buffer = NULL;
> > > + return ret;
> > > +}
> > > +
> > > /**
> > > * tcg2_measure_variable() - add variable event log and extend PCR
> > > *
> > > @@ -1963,12 +2303,6 @@ efi_status_t efi_tcg2_register(void)
> > > if (ret != EFI_SUCCESS)
> > > goto fail;
> > >
> > > - ret = efi_append_scrtm_version(dev);
> > > - if (ret != EFI_SUCCESS) {
> > > - tcg2_uninit();
> > > - goto fail;
> > > - }
> > > -
> > > ret = efi_add_protocol(efi_root, &efi_guid_tcg2_protocol,
> > > (void *)&efi_tcg2_protocol);
> > > if (ret != EFI_SUCCESS) {
> > > --
> > > 2.25.1
> > >
> >
> > Tested-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> >
Cheers
/Ilias
More information about the U-Boot
mailing list