[PATCH 1/3] efi_loader: Add check for event log passed from firmware

[PATCH 1/3] efi_loader: Add check for event log passed from firmware

[Ruchika Gupta]

Platforms may have support to measure their initial firmware components
and pass the event log to u-boot. The event log address can be passed
in property tpm_event_log_addr and tpm_event_log_size of the tpm node.
Platforms may choose their own specific mechanism to do so. A weak
function is added to check if even log has been passed to u-boot
from earlier firmware components. If available, the eventlog is parsed
to check for its correctness and further event logs are appended to the
passed log.

Signed-off-by: Ruchika Gupta <ruchika.gupta@linaro.org>
---
 lib/efi_loader/efi_tcg2.c | 312 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 296 insertions(+), 16 deletions(-)

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 74f0bef239..c97766eae3 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -276,6 +276,45 @@ __weak efi_status_t platform_get_tpm2_device(struct udevice **dev)
 	return EFI_NOT_FOUND;
 }
 
+/**
+ * platform_get_eventlog() - retrieve the eventlog address and size
+ *
+ * This function retrieves the eventlog address and size if the underlying
+ * firmware has done some measurements and passed them.
+ *
+ * This function may be overridden based on platform specific method of
+ * passing the eventlog address and size.
+ *
+ * @dev:	udevice
+ * @addr:	eventlog address
+ * @sz:		eventlog size
+ * Return:	status code
+ */
+__weak efi_status_t platform_get_eventlog(struct udevice *dev, u64 *addr,
+					  u32 *sz)
+{
+	const u64 *basep;
+	const u32 *sizep;
+
+	basep = dev_read_prop(dev, "tpm_event_log_addr", NULL);
+	if (!basep)
+		return EFI_NOT_FOUND;
+
+	*addr = be64_to_cpup((__force __be64 *)basep);
+
+	sizep = dev_read_prop(dev, "tpm_event_log_size", NULL);
+	if (!sizep)
+		return EFI_NOT_FOUND;
+
+	*sz = be32_to_cpup((__force __be32 *)sizep);
+	if (*sz == 0) {
+		log_debug("event log empty\n");
+		return EFI_NOT_FOUND;
+	}
+
+	return EFI_SUCCESS;
+}
+
 /**
  * tpm2_get_max_command_size() - get the supported max command size
  *
@@ -1107,6 +1146,205 @@ static const struct efi_tcg2_protocol efi_tcg2_protocol = {
 	.get_result_of_set_active_pcr_banks = efi_tcg2_get_result_of_set_active_pcr_banks,
 };
 
+/**
+ * parse_event_log_header() -  Parse and verify the event log header fields
+ *
+ * @buffer:			Pointer to the event header
+ * @size:			Size of the eventlog
+ * @pos:			Position in buffer after event log header
+ *
+ * Return:	status code
+ */
+efi_status_t parse_event_log_header(void *buffer, u32 size, u32 *pos)
+{
+	struct tcg_pcr_event *event_header = (struct tcg_pcr_event *)buffer;
+	int i = 0;
+
+	if (size < sizeof(*event_header))
+		return EFI_COMPROMISED_DATA;
+
+	if (get_unaligned_le32(&event_header->pcr_index) != 0 ||
+	    get_unaligned_le32(&event_header->event_type) != EV_NO_ACTION)
+		return EFI_COMPROMISED_DATA;
+
+	for (i = 0; i < sizeof(event_header->digest); i++) {
+		if (event_header->digest[i] != 0)
+			return EFI_COMPROMISED_DATA;
+	}
+
+	*pos += sizeof(*event_header);
+
+	return EFI_SUCCESS;
+}
+
+/**
+ * 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:		Offset in the evenlog where specID event starts
+ *
+ * Return:		status code
+ * @pos			Offset in the eventlog where the specID event ends
+ * @digest_list:	list of digests in the event
+ */
+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;
+
+	/* 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 may need to worry about the order of algs in this structure as
+	 * subsequent entries in event should be in same order
+	 */
+	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);
+
+		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 spec 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]);
+
+	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;
+}
+
+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;
+
+	if (*offset > log_size)
+		return EFI_COMPROMISED_DATA;
+
+	event = (struct tcg_pcr_event2 *)((uintptr_t)buffer + *offset);
+
+	*pcr = get_unaligned_le32(&event->pcr_index);
+
+	event_size = tcg_event_final_size(digest_list);
+
+	if (*offset + event_size > log_size) {
+		log_err("Event exceeds log size\n");
+		return EFI_COMPROMISED_DATA;
+	}
+
+	event_type = get_unaligned_le32(&event->event_type);
+
+	/* 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;
+}
+
 /**
  * create_specid_event() - Create the first event in the eventlog
  *
@@ -1241,16 +1479,19 @@ out:
 /**
  * efi_init_event_log() - initialize an eventlog
  */
-static efi_status_t efi_init_event_log(void)
+static efi_status_t efi_init_event_log(struct udevice *dev)
 {
 	/*
 	 * 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;
+	struct tpml_digest_values digest_list;
 	size_t spec_event_size;
 	efi_status_t ret;
+	u32 pcr, pos;
+	u64 base;
+	u32 sz;
 
 	ret = platform_get_tpm2_device(&dev);
 	if (ret != EFI_SUCCESS)
@@ -1266,26 +1507,65 @@ static efi_status_t efi_init_event_log(void)
 	 * 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.truncated = false;
 
 	/*
-	 * The log header is defined to be in SHA1 event log entry format.
-	 * Setup event header
+	 * Check if earlier firmware have passed any eventlog. Different
+	 * platforms can use different ways to do so
 	 */
-	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 = platform_get_eventlog(dev, &base, &sz);
+	if (ret == EFI_SUCCESS && sz < TPM2_EVENT_LOG_SIZE) {
+		void *buffer = (void *)base;
+
+		pos = 0;
+		/* Parse the eventlog to check for its validity */
+		ret = parse_event_log_header(buffer, sz, &pos);
+		if (ret || pos > sz) {
+			ret = EFI_COMPROMISED_DATA;
+			goto free_pool;
+		}
+
+		ret = parse_specid_event(dev, buffer, sz, &pos, &digest_list);
+		if (ret || pos > sz) {
+			log_err("Error parsing SPEC ID Event\n");
+			ret = EFI_COMPROMISED_DATA;
+			goto free_pool;
+		}
+
+		while (pos < sz) {
+			ret = tcg2_parse_event(dev, buffer, sz, &pos,
+					       &digest_list, &pcr);
+			if (ret) {
+				log_err("Error parsing event\n");
+				goto free_pool;
+			}
+		}
+
+		memcpy(event_log.buffer, buffer, sz);
+		event_log.pos = sz;
+	} else {
+		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)
@@ -1664,7 +1944,7 @@ efi_status_t efi_tcg2_register(void)
 		return EFI_SUCCESS;
 	}
 
-	ret = efi_init_event_log();
+	ret = efi_init_event_log(dev);
 	if (ret != EFI_SUCCESS)
 		goto fail;
 
-- 
2.25.1

[PATCH] efi_loader: fix FinalEvents table if an EFI uses GetEventLog

[Ruchika Gupta]

---
 lib/efi_loader/efi_tcg2.c | 90 ++++++++++++++++++++++++++-------------
 1 file changed, 61 insertions(+), 29 deletions(-)

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 189e4a5ba5..215f4b2b04 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -34,6 +34,7 @@ struct event_log_buffer {
 	size_t final_pos; /* final events config table position */
 	size_t last_event_size;
 	bool get_event_called;
+	bool ebs_called;
 	bool truncated;
 };
 
@@ -186,39 +187,29 @@ static efi_status_t tcg2_pcr_extend(struct udevice *dev, u32 pcr_index,
 	return EFI_SUCCESS;
 }
 
-/* tcg2_agile_log_append - Append an agile event to out eventlog
+/* put_event - Append an agile event to an eventlog
  *
  * @pcr_index:		PCR index
  * @event_type:		type of event added
  * @digest_list:	list of digest algorithms to add
  * @size:		size of event
  * @event:		event to add
+ * @log:		log buffer to append the event
  *
- * @Return: status code
  */
-static efi_status_t tcg2_agile_log_append(u32 pcr_index, u32 event_type,
-					  struct tpml_digest_values *digest_list,
-					  u32 size, u8 event[])
+static void put_event(u32 pcr_index, u32 event_type,
+		      struct tpml_digest_values *digest_list, u32 size,
+		      u8 event[], void *log)
 {
-	void *log = (void *)((uintptr_t)event_log.buffer + event_log.pos);
 	size_t pos;
 	size_t i;
 	u32 event_size;
 
-	if (event_log.get_event_called)
-		log = (void *)((uintptr_t)event_log.final_buffer +
-			       event_log.final_pos);
-
 	/*
 	 * size refers to the length of event[] only, we need to check against
 	 * the final tcg_pcr_event2 size
 	 */
 	event_size = size + tcg_event_final_size(digest_list);
-	if (event_log.pos + event_size > TPM2_EVENT_LOG_SIZE ||
-	    event_log.final_pos + event_size > TPM2_EVENT_LOG_SIZE) {
-		event_log.truncated = true;
-		return EFI_VOLUME_FULL;
-	}
 
 	put_unaligned_le32(pcr_index, log);
 	pos = offsetof(struct tcg_pcr_event2, event_type);
@@ -242,25 +233,64 @@ static efi_status_t tcg2_agile_log_append(u32 pcr_index, u32 event_type,
 	memcpy((void *)((uintptr_t)log + pos), event, size);
 	pos += size;
 
-	/* make sure the calculated buffer is what we checked against */
+	/*
+	 * make sure the calculated buffer is what we checked against
+	 * This check should never fail.  It checks the code above is
+	 * calculating the right length for the event we are adding
+	 * */
 	if (pos != event_size)
-		return EFI_INVALID_PARAMETER;
+		log_err("Appending to the EventLog failed\n");
 
-	/* if GetEventLog hasn't been called update the normal log */
-	if (!event_log.get_event_called) {
-		event_log.pos += pos;
-		event_log.last_event_size = pos;
-	} else {
-	/* if GetEventLog has been called update config table log */
-		struct efi_tcg2_final_events_table *final_event;
+}
 
-		final_event =
-			(struct efi_tcg2_final_events_table *)(event_log.final_buffer);
-		final_event->number_of_events++;
-		event_log.final_pos += pos;
+/* tcg2_agile_log_append - Append an agile event to an eventlog
+ *
+ * @pcr_index:		PCR index
+ * @event_type:		type of event added
+ * @digest_list:	list of digest algorithms to add
+ * @size:		size of event
+ * @event:		event to add
+ * @log:		log buffer to append the event
+ *
+ * @Return: status code
+ */
+static efi_status_t tcg2_agile_log_append(u32 pcr_index, u32 event_type,
+					  struct tpml_digest_values *digest_list,
+					  u32 size, u8 event[])
+{
+	void *log = (void *)((uintptr_t)event_log.buffer + event_log.pos);
+	u32 event_size = size + tcg_event_final_size(digest_list);
+	struct efi_tcg2_final_events_table *final_event;
+	efi_status_t ret = EFI_SUCCESS;
+
+	/* if ExitBootServices hasn't been called update the normal log */
+	if (!event_log.ebs_called) {
+		if (event_log.truncated ||
+		    event_log.pos + event_size > TPM2_EVENT_LOG_SIZE) {
+			event_log.truncated = true;
+			return EFI_VOLUME_FULL;
+		}
+		put_event(pcr_index, event_type, digest_list, size, event, log);
+		event_log.pos += event_size;
+		event_log.last_event_size = event_size;
 	}
 
-	return EFI_SUCCESS;
+	if (!event_log.get_event_called)
+		return ret;
+
+	/* if GetEventLog has been called update FinalEventLog as well */
+	if (event_log.final_pos + event_size > TPM2_EVENT_LOG_SIZE)
+		return EFI_VOLUME_FULL;
+
+	log = (void *)((uintptr_t)event_log.final_buffer + event_log.final_pos);
+	put_event(pcr_index, event_type, digest_list, size, event, log);
+
+	final_event =
+		(struct efi_tcg2_final_events_table *)event_log.final_buffer;
+	final_event->number_of_events++;
+	event_log.final_pos += event_size;
+
+	return ret;
 }
 
 /**
@@ -1303,6 +1333,7 @@ static efi_status_t efi_init_event_log(void)
 	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;
 
 	/*
@@ -1792,6 +1823,7 @@ efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context)
 
 	EFI_ENTRY("%p, %p", event, context);
 
+	event_log.ebs_called = true;
 	ret = platform_get_tpm2_device(&dev);
 	if (ret != EFI_SUCCESS)
 		goto out;
-- 
2.25.1

[PATCH 2/3] tpm: use more algorithms than sha256 on pcr_read

[Ruchika Gupta]

The current tpm2_pcr_read is hardcoded using SHA256. Make the
actual command to TPM configurable to use wider range of algorithms.
The current command line is kept as is i.e limited to SHA-256 only.

Signed-off-by: Ruchika Gupta <ruchika.gupta@linaro.org>
---
 cmd/tpm-v2.c     |  3 ++-
 include/tpm-v2.h |  3 ++-
 lib/tpm-v2.c     | 12 ++++++++----
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/cmd/tpm-v2.c b/cmd/tpm-v2.c
index daae91100a..4ea5f9f094 100644
--- a/cmd/tpm-v2.c
+++ b/cmd/tpm-v2.c
@@ -151,7 +151,8 @@ static int do_tpm_pcr_read(struct cmd_tbl *cmdtp, int flag, int argc,
 
 	data = map_sysmem(simple_strtoul(argv[2], NULL, 0), 0);
 
-	rc = tpm2_pcr_read(dev, index, priv->pcr_select_min, data, &updates);
+	rc = tpm2_pcr_read(dev, index, priv->pcr_select_min, TPM2_ALG_SHA256,
+			   data, TPM2_DIGEST_LEN, &updates);
 	if (!rc) {
 		printf("PCR #%u content (%u known updates):\n", index, updates);
 		print_byte_string(data, TPM2_DIGEST_LEN);
diff --git a/include/tpm-v2.h b/include/tpm-v2.h
index e6b68769f3..07dfaa64fb 100644
--- a/include/tpm-v2.h
+++ b/include/tpm-v2.h
@@ -518,7 +518,8 @@ u32 tpm2_nv_write_value(struct udevice *dev, u32 index, const void *data,
  * @return code of the operation
  */
 u32 tpm2_pcr_read(struct udevice *dev, u32 idx, unsigned int idx_min_sz,
-		  void *data, unsigned int *updates);
+		  u32 algorithm, void *data, u32 digest_len,
+		  unsigned int *updates);
 
 /**
  * Issue a TPM2_GetCapability command.  This implementation is limited
diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
index 235f8c20d4..9f86eab814 100644
--- a/lib/tpm-v2.c
+++ b/lib/tpm-v2.c
@@ -254,7 +254,8 @@ u32 tpm2_nv_write_value(struct udevice *dev, u32 index, const void *data,
 }
 
 u32 tpm2_pcr_read(struct udevice *dev, u32 idx, unsigned int idx_min_sz,
-		  void *data, unsigned int *updates)
+		  u32 algorithm, void *data, u32 digest_len,
+		  unsigned int *updates)
 {
 	u8 idx_array_sz = max(idx_min_sz, DIV_ROUND_UP(idx, 8));
 	u8 command_v2[COMMAND_BUFFER_SIZE] = {
@@ -264,7 +265,7 @@ u32 tpm2_pcr_read(struct udevice *dev, u32 idx, unsigned int idx_min_sz,
 
 		/* TPML_PCR_SELECTION */
 		tpm_u32(1),			/* Number of selections */
-		tpm_u16(TPM2_ALG_SHA256),	/* Algorithm of the hash */
+		tpm_u16(algorithm),		/* Algorithm of the hash */
 		idx_array_sz,			/* Array size for selection */
 		/* bitmap(idx)			   Selected PCR bitmap */
 	};
@@ -283,10 +284,13 @@ u32 tpm2_pcr_read(struct udevice *dev, u32 idx, unsigned int idx_min_sz,
 	if (ret)
 		return ret;
 
+	if (digest_len > response_len)
+		return TPM_LIB_ERROR;
+
 	if (unpack_byte_string(response, response_len, "ds",
 			       10, &counter,
-			       response_len - TPM2_DIGEST_LEN, data,
-			       TPM2_DIGEST_LEN))
+			       response_len - digest_len, data,
+			       digest_len))
 		return TPM_LIB_ERROR;
 
 	if (updates)
-- 
2.25.1

[PATCH 3/3] efi_loader: Extend PCR's for firmware measurements

[Ruchika Gupta]

Firmwares before U-Boot may be capable of doing tpm measurements
and passing them to U-Boot in the form of eventlog. However there
may be scenarios where the firmwares don't have TPM driver and
are not capable of extending the measurements in the PCRs. To
cater to such platforms, read the PCR0 to determine if the
previous firmwares have extended the PCR0. If not, then extend
the PCR's as the eventlog is parsed.

Signed-off-by: Ruchika Gupta <ruchika.gupta@linaro.org>
---
 lib/efi_loader/efi_tcg2.c | 86 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index c97766eae3..cbd0c7d224 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -178,6 +178,43 @@ static efi_status_t tcg2_pcr_extend(struct udevice *dev, u32 pcr_index,
 	return EFI_SUCCESS;
 }
 
+/* tcg2_pcr_read - Read PCRs for a TPM2 device for a given tpml_digest_values
+ *
+ * @dev:		device
+ * @digest_list:	list of digest algorithms to extend
+ *
+ * @Return: status code
+ */
+static efi_status_t tcg2_pcr_read(struct udevice *dev, u32 pcr_index,
+				    struct tpml_digest_values *digest_list)
+{
+	struct tpm_chip_priv *priv;
+	unsigned int updates, pcr_select_min;
+	u32 rc;
+	size_t i;
+
+	priv = dev_get_uclass_priv(dev);
+	if (!priv)
+		return EFI_DEVICE_ERROR;
+
+	pcr_select_min = priv->pcr_select_min;
+
+	for (i = 0; i < digest_list->count; i++) {
+		u16 hash_alg = digest_list->digests[i].hash_alg;
+		u8 *digest = (u8 *)&digest_list->digests[i].digest;
+
+		rc = tpm2_pcr_read(dev, pcr_index, pcr_select_min,
+				    hash_alg, digest, alg_to_len(hash_alg),
+				    &updates);
+		if (rc) {
+			EFI_PRINT("Failed to read PCR\n");
+			return EFI_DEVICE_ERROR;
+		}
+	}
+
+	return EFI_SUCCESS;
+}
+
 /* tcg2_agile_log_append - Append an agile event to out eventlog
  *
  * @pcr_index:		PCR index
@@ -1488,10 +1525,12 @@ static efi_status_t efi_init_event_log(struct udevice *dev)
 	struct tcg_pcr_event *event_header = NULL;
 	struct tpml_digest_values digest_list;
 	size_t spec_event_size;
+	bool extend_pcr = false;
 	efi_status_t ret;
 	u32 pcr, pos;
 	u64 base;
 	u32 sz;
+	int i;
 
 	ret = platform_get_tpm2_device(&dev);
 	if (ret != EFI_SUCCESS)
@@ -1541,6 +1580,26 @@ static efi_status_t efi_init_event_log(struct udevice *dev)
 			goto free_pool;
 		}
 
+		ret = tcg2_pcr_read(dev, 0, &digest_list);
+		if (ret) {
+			log_err("Error reading PCR 0\n");
+			goto free_pool;
+		}
+
+		/*
+		 * If PCR0 is 0, previous firmware didn't have the capability
+		 * to extend the PCR. In this scenario, extend the PCR as
+		 * the eventlog is parsed.
+		 */
+		for (i = 0; i < digest_list.count; i++) {
+			u8 buffer[TPM2_DIGEST_LEN] =  { 0 };
+			u16 hash_alg = digest_list.digests[i].hash_alg;
+
+			if (!memcmp((u8 *)&digest_list.digests[i].digest,
+				    buffer, alg_to_len(hash_alg)))
+				extend_pcr = true;
+		}
+
 		while (pos < sz) {
 			ret = tcg2_parse_event(dev, buffer, sz, &pos,
 					       &digest_list, &pcr);
@@ -1548,6 +1607,33 @@ static efi_status_t efi_init_event_log(struct udevice *dev)
 				log_err("Error parsing event\n");
 				goto free_pool;
 			}
+
+			if (pcr != 0) {
+				/*
+				 * Eventlog passed by firmware should extend
+				 * PCR0 only.
+				 */
+				log_err("Invalid PCR\n");
+				goto free_pool;
+			}
+
+			if (extend_pcr) {
+				ret = tcg2_pcr_extend(dev, pcr, &digest_list);
+				if (ret != EFI_SUCCESS) {
+					log_err("Error in extending PCR\n");
+					goto free_pool;
+				}
+
+				/* Clear the digest for next event */
+				for (i = 0; i < digest_list.count; i++) {
+					u16 hash_alg =
+						digest_list.digests[i].hash_alg;
+					u8 *digest =
+					   (u8 *)&digest_list.digests[i].digest;
+
+					memset(digest, 0, alg_to_len(hash_alg));
+				}
+			}
 		}
 
 		memcpy(event_log.buffer, buffer, sz);
-- 
2.25.1

Re: [PATCH] efi_loader: fix FinalEvents table if an EFI uses GetEventLog

[Ruchika Gupta]

Please ignore this patch. Sent by mistake.

On Thu, 18 Nov 2021 at 11:48, Ruchika Gupta <ruchika.gupta@linaro.org>
wrote:

> ---
>  lib/efi_loader/efi_tcg2.c | 90 ++++++++++++++++++++++++++-------------
>  1 file changed, 61 insertions(+), 29 deletions(-)
>
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 189e4a5ba5..215f4b2b04 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -34,6 +34,7 @@ struct event_log_buffer {
>         size_t final_pos; /* final events config table position */
>         size_t last_event_size;
>         bool get_event_called;
> +       bool ebs_called;
>         bool truncated;
>  };
>
> @@ -186,39 +187,29 @@ static efi_status_t tcg2_pcr_extend(struct udevice
> *dev, u32 pcr_index,
>         return EFI_SUCCESS;
>  }
>
> -/* tcg2_agile_log_append - Append an agile event to out eventlog
> +/* put_event - Append an agile event to an eventlog
>   *
>   * @pcr_index:         PCR index
>   * @event_type:                type of event added
>   * @digest_list:       list of digest algorithms to add
>   * @size:              size of event
>   * @event:             event to add
> + * @log:               log buffer to append the event
>   *
> - * @Return: status code
>   */
> -static efi_status_t tcg2_agile_log_append(u32 pcr_index, u32 event_type,
> -                                         struct tpml_digest_values
> *digest_list,
> -                                         u32 size, u8 event[])
> +static void put_event(u32 pcr_index, u32 event_type,
> +                     struct tpml_digest_values *digest_list, u32 size,
> +                     u8 event[], void *log)
>  {
> -       void *log = (void *)((uintptr_t)event_log.buffer + event_log.pos);
>         size_t pos;
>         size_t i;
>         u32 event_size;
>
> -       if (event_log.get_event_called)
> -               log = (void *)((uintptr_t)event_log.final_buffer +
> -                              event_log.final_pos);
> -
>         /*
>          * size refers to the length of event[] only, we need to check
> against
>          * the final tcg_pcr_event2 size
>          */
>         event_size = size + tcg_event_final_size(digest_list);
> -       if (event_log.pos + event_size > TPM2_EVENT_LOG_SIZE ||
> -           event_log.final_pos + event_size > TPM2_EVENT_LOG_SIZE) {
> -               event_log.truncated = true;
> -               return EFI_VOLUME_FULL;
> -       }
>
>         put_unaligned_le32(pcr_index, log);
>         pos = offsetof(struct tcg_pcr_event2, event_type);
> @@ -242,25 +233,64 @@ static efi_status_t tcg2_agile_log_append(u32
> pcr_index, u32 event_type,
>         memcpy((void *)((uintptr_t)log + pos), event, size);
>         pos += size;
>
> -       /* make sure the calculated buffer is what we checked against */
> +       /*
> +        * make sure the calculated buffer is what we checked against
> +        * This check should never fail.  It checks the code above is
> +        * calculating the right length for the event we are adding
> +        * */
>         if (pos != event_size)
> -               return EFI_INVALID_PARAMETER;
> +               log_err("Appending to the EventLog failed\n");
>
> -       /* if GetEventLog hasn't been called update the normal log */
> -       if (!event_log.get_event_called) {
> -               event_log.pos += pos;
> -               event_log.last_event_size = pos;
> -       } else {
> -       /* if GetEventLog has been called update config table log */
> -               struct efi_tcg2_final_events_table *final_event;
> +}
>
> -               final_event =
> -                       (struct efi_tcg2_final_events_table
> *)(event_log.final_buffer);
> -               final_event->number_of_events++;
> -               event_log.final_pos += pos;
> +/* tcg2_agile_log_append - Append an agile event to an eventlog
> + *
> + * @pcr_index:         PCR index
> + * @event_type:                type of event added
> + * @digest_list:       list of digest algorithms to add
> + * @size:              size of event
> + * @event:             event to add
> + * @log:               log buffer to append the event
> + *
> + * @Return: status code
> + */
> +static efi_status_t tcg2_agile_log_append(u32 pcr_index, u32 event_type,
> +                                         struct tpml_digest_values
> *digest_list,
> +                                         u32 size, u8 event[])
> +{
> +       void *log = (void *)((uintptr_t)event_log.buffer + event_log.pos);
> +       u32 event_size = size + tcg_event_final_size(digest_list);
> +       struct efi_tcg2_final_events_table *final_event;
> +       efi_status_t ret = EFI_SUCCESS;
> +
> +       /* if ExitBootServices hasn't been called update the normal log */
> +       if (!event_log.ebs_called) {
> +               if (event_log.truncated ||
> +                   event_log.pos + event_size > TPM2_EVENT_LOG_SIZE) {
> +                       event_log.truncated = true;
> +                       return EFI_VOLUME_FULL;
> +               }
> +               put_event(pcr_index, event_type, digest_list, size, event,
> log);
> +               event_log.pos += event_size;
> +               event_log.last_event_size = event_size;
>         }
>
> -       return EFI_SUCCESS;
> +       if (!event_log.get_event_called)
> +               return ret;
> +
> +       /* if GetEventLog has been called update FinalEventLog as well */
> +       if (event_log.final_pos + event_size > TPM2_EVENT_LOG_SIZE)
> +               return EFI_VOLUME_FULL;
> +
> +       log = (void *)((uintptr_t)event_log.final_buffer +
> event_log.final_pos);
> +       put_event(pcr_index, event_type, digest_list, size, event, log);
> +
> +       final_event =
> +               (struct efi_tcg2_final_events_table
> *)event_log.final_buffer;
> +       final_event->number_of_events++;
> +       event_log.final_pos += event_size;
> +
> +       return ret;
>  }
>
>  /**
> @@ -1303,6 +1333,7 @@ static efi_status_t efi_init_event_log(void)
>         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;
>
>         /*
> @@ -1792,6 +1823,7 @@ efi_tcg2_notify_exit_boot_services(struct efi_event
> *event, void *context)
>
>         EFI_ENTRY("%p, %p", event, context);
>
> +       event_log.ebs_called = true;
>         ret = platform_get_tpm2_device(&dev);
>         if (ret != EFI_SUCCESS)
>                 goto out;
> --
> 2.25.1
>
>

Fwd: [PATCH 1/3] efi_loader: Add check for event log passed from firmware

[Heinrich Schuchardt]

Hello Ilias, hello Masahisa,

could you, please, review the patch available at

https://patchwork.ozlabs.org/project/uboot/patch/20211118061751.3334620-1-ruchika.gupta@linaro.org/

Best regards

Heinrich

On 11/18/21 07:17, Ruchika Gupta wrote:
> Platforms may have support to measure their initial firmware components
> and pass the event log to u-boot. The event log address can be passed
> in property tpm_event_log_addr and tpm_event_log_size of the tpm node.
> Platforms may choose their own specific mechanism to do so. A weak
> function is added to check if even log has been passed to u-boot
> from earlier firmware components. If available, the eventlog is parsed
> to check for its correctness and further event logs are appended to the
> passed log.
>
> Signed-off-by: Ruchika Gupta <ruchika.gupta@linaro.org>
> ---
>   lib/efi_loader/efi_tcg2.c | 312 ++++++++++++++++++++++++++++++++++++--
>   1 file changed, 296 insertions(+), 16 deletions(-)
>
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 74f0bef239..c97766eae3 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -276,6 +276,45 @@ __weak efi_status_t platform_get_tpm2_device(struct udevice **dev)
>   	return EFI_NOT_FOUND;
>   }
>
> +/**
> + * platform_get_eventlog() - retrieve the eventlog address and size
> + *
> + * This function retrieves the eventlog address and size if the underlying
> + * firmware has done some measurements and passed them.
> + *
> + * This function may be overridden based on platform specific method of
> + * passing the eventlog address and size.
> + *
> + * @dev:	udevice
> + * @addr:	eventlog address
> + * @sz:		eventlog size
> + * Return:	status code
> + */
> +__weak efi_status_t platform_get_eventlog(struct udevice *dev, u64 *addr,
> +					  u32 *sz)
> +{
> +	const u64 *basep;
> +	const u32 *sizep;
> +
> +	basep = dev_read_prop(dev, "tpm_event_log_addr", NULL);
> +	if (!basep)
> +		return EFI_NOT_FOUND;
> +
> +	*addr = be64_to_cpup((__force __be64 *)basep);
> +
> +	sizep = dev_read_prop(dev, "tpm_event_log_size", NULL);
> +	if (!sizep)
> +		return EFI_NOT_FOUND;
> +
> +	*sz = be32_to_cpup((__force __be32 *)sizep);
> +	if (*sz == 0) {
> +		log_debug("event log empty\n");
> +		return EFI_NOT_FOUND;
> +	}
> +
> +	return EFI_SUCCESS;
> +}
> +
>   /**
>    * tpm2_get_max_command_size() - get the supported max command size
>    *
> @@ -1107,6 +1146,205 @@ static const struct efi_tcg2_protocol efi_tcg2_protocol = {
>   	.get_result_of_set_active_pcr_banks = efi_tcg2_get_result_of_set_active_pcr_banks,
>   };
>
> +/**
> + * parse_event_log_header() -  Parse and verify the event log header fields
> + *
> + * @buffer:			Pointer to the event header
> + * @size:			Size of the eventlog
> + * @pos:			Position in buffer after event log header
> + *
> + * Return:	status code
> + */
> +efi_status_t parse_event_log_header(void *buffer, u32 size, u32 *pos)
> +{
> +	struct tcg_pcr_event *event_header = (struct tcg_pcr_event *)buffer;
> +	int i = 0;
> +
> +	if (size < sizeof(*event_header))
> +		return EFI_COMPROMISED_DATA;
> +
> +	if (get_unaligned_le32(&event_header->pcr_index) != 0 ||
> +	    get_unaligned_le32(&event_header->event_type) != EV_NO_ACTION)
> +		return EFI_COMPROMISED_DATA;
> +
> +	for (i = 0; i < sizeof(event_header->digest); i++) {
> +		if (event_header->digest[i] != 0)
> +			return EFI_COMPROMISED_DATA;
> +	}
> +
> +	*pos += sizeof(*event_header);
> +
> +	return EFI_SUCCESS;
> +}
> +
> +/**
> + * 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:		Offset in the evenlog where specID event starts
> + *
> + * Return:		status code
> + * @pos			Offset in the eventlog where the specID event ends
> + * @digest_list:	list of digests in the event
> + */
> +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;
> +
> +	/* 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 may need to worry about the order of algs in this structure as
> +	 * subsequent entries in event should be in same order
> +	 */
> +	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);
> +
> +		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 spec 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]);
> +
> +	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;
> +}
> +
> +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;
> +
> +	if (*offset > log_size)
> +		return EFI_COMPROMISED_DATA;
> +
> +	event = (struct tcg_pcr_event2 *)((uintptr_t)buffer + *offset);
> +
> +	*pcr = get_unaligned_le32(&event->pcr_index);
> +
> +	event_size = tcg_event_final_size(digest_list);
> +
> +	if (*offset + event_size > log_size) {
> +		log_err("Event exceeds log size\n");
> +		return EFI_COMPROMISED_DATA;
> +	}
> +
> +	event_type = get_unaligned_le32(&event->event_type);
> +
> +	/* 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;
> +}
> +
>   /**
>    * create_specid_event() - Create the first event in the eventlog
>    *
> @@ -1241,16 +1479,19 @@ out:
>   /**
>    * efi_init_event_log() - initialize an eventlog
>    */
> -static efi_status_t efi_init_event_log(void)
> +static efi_status_t efi_init_event_log(struct udevice *dev)
>   {
>   	/*
>   	 * 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;
> +	struct tpml_digest_values digest_list;
>   	size_t spec_event_size;
>   	efi_status_t ret;
> +	u32 pcr, pos;
> +	u64 base;
> +	u32 sz;
>
>   	ret = platform_get_tpm2_device(&dev);
>   	if (ret != EFI_SUCCESS)
> @@ -1266,26 +1507,65 @@ static efi_status_t efi_init_event_log(void)
>   	 * 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.truncated = false;
>
>   	/*
> -	 * The log header is defined to be in SHA1 event log entry format.
> -	 * Setup event header
> +	 * Check if earlier firmware have passed any eventlog. Different
> +	 * platforms can use different ways to do so
>   	 */
> -	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 = platform_get_eventlog(dev, &base, &sz);
> +	if (ret == EFI_SUCCESS && sz < TPM2_EVENT_LOG_SIZE) {
> +		void *buffer = (void *)base;
> +
> +		pos = 0;
> +		/* Parse the eventlog to check for its validity */
> +		ret = parse_event_log_header(buffer, sz, &pos);
> +		if (ret || pos > sz) {
> +			ret = EFI_COMPROMISED_DATA;
> +			goto free_pool;
> +		}
> +
> +		ret = parse_specid_event(dev, buffer, sz, &pos, &digest_list);
> +		if (ret || pos > sz) {
> +			log_err("Error parsing SPEC ID Event\n");
> +			ret = EFI_COMPROMISED_DATA;
> +			goto free_pool;
> +		}
> +
> +		while (pos < sz) {
> +			ret = tcg2_parse_event(dev, buffer, sz, &pos,
> +					       &digest_list, &pcr);
> +			if (ret) {
> +				log_err("Error parsing event\n");
> +				goto free_pool;
> +			}
> +		}
> +
> +		memcpy(event_log.buffer, buffer, sz);
> +		event_log.pos = sz;
> +	} else {
> +		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)
> @@ -1664,7 +1944,7 @@ efi_status_t efi_tcg2_register(void)
>   		return EFI_SUCCESS;
>   	}
>
> -	ret = efi_init_event_log();
> +	ret = efi_init_event_log(dev);
>   	if (ret != EFI_SUCCESS)
>   		goto fail;
>
>

Re: [PATCH 2/3] tpm: use more algorithms than sha256 on pcr_read

[Ilias Apalodimas]

Hi Ruchika, 

[...]
> -		  void *data, unsigned int *updates);
> +		  u32 algorithm, void *data, u32 digest_len,

This goes into a tpm_u16() call.  It doesn't break anything currently, but
shouldn't we define this as u16 ?

> +		  unsigned int *updates);
>  
>  /**
>   * Issue a TPM2_GetCapability command.  This implementation is limited
> diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
> index 235f8c20d4..9f86eab814 100644
> --- a/lib/tpm-v2.c
> +++ b/lib/tpm-v2.c
> @@ -254,7 +254,8 @@ u32 tpm2_nv_write_value(struct udevice *dev, u32 index, const void *data,
>  }
>  
>  u32 tpm2_pcr_read(struct udevice *dev, u32 idx, unsigned int idx_min_sz,
> -		  void *data, unsigned int *updates)
> +		  u32 algorithm, void *data, u32 digest_len,
> +		  unsigned int *updates)
>  {
>  	u8 idx_array_sz = max(idx_min_sz, DIV_ROUND_UP(idx, 8));
>  	u8 command_v2[COMMAND_BUFFER_SIZE] = {
> @@ -264,7 +265,7 @@ u32 tpm2_pcr_read(struct udevice *dev, u32 idx, unsigned int idx_min_sz,
>  
>  		/* TPML_PCR_SELECTION */
>  		tpm_u32(1),			/* Number of selections */
> -		tpm_u16(TPM2_ALG_SHA256),	/* Algorithm of the hash */
> +		tpm_u16(algorithm),		/* Algorithm of the hash */
>  		idx_array_sz,			/* Array size for selection */
>  		/* bitmap(idx)			   Selected PCR bitmap */
>  	};
> @@ -283,10 +284,13 @@ u32 tpm2_pcr_read(struct udevice *dev, u32 idx, unsigned int idx_min_sz,
>  	if (ret)
>  		return ret;
>  
> +	if (digest_len > response_len)
> +		return TPM_LIB_ERROR;
> +
>  	if (unpack_byte_string(response, response_len, "ds",
>  			       10, &counter,
> -			       response_len - TPM2_DIGEST_LEN, data,
> -			       TPM2_DIGEST_LEN))
> +			       response_len - digest_len, data,
> +			       digest_len))
>  		return TPM_LIB_ERROR;
>  
>  	if (updates)
> -- 
> 2.25.1
> 


Cheers
/Ilias

Re: [PATCH 1/3] efi_loader: Add check for event log passed from firmware

[Ilias Apalodimas]

Hi Ruchika, 
>  

[...]

> +static efi_status_t efi_init_event_log(struct udevice *dev)
>  {
>  	/*
>  	 * 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;
> +	struct tpml_digest_values digest_list;
>  	size_t spec_event_size;
>  	efi_status_t ret;
> +	u32 pcr, pos;
> +	u64 base;
> +	u32 sz;
>  
>  	ret = platform_get_tpm2_device(&dev);
>  	if (ret != EFI_SUCCESS)
> @@ -1266,26 +1507,65 @@ static efi_status_t efi_init_event_log(void)
>  	 * 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.truncated = false;
>  
>  	/*
> -	 * The log header is defined to be in SHA1 event log entry format.
> -	 * Setup event header
> +	 * Check if earlier firmware have passed any eventlog. Different
> +	 * platforms can use different ways to do so
>  	 */
> -	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 = platform_get_eventlog(dev, &base, &sz);

I think we can refactor this slightly and make it easier to read. 
Can we merge the logic of getting + validating the eventlog in 
platform_get_eventlog(). Then just return EFI_XXXXX and continue from
there.

> +	if (ret == EFI_SUCCESS && sz < TPM2_EVENT_LOG_SIZE) {
> +		void *buffer = (void *)base;
> +
> +		pos = 0;
> +		/* Parse the eventlog to check for its validity */
> +		ret = parse_event_log_header(buffer, sz, &pos);
> +		if (ret || pos > sz) {
> +			ret = EFI_COMPROMISED_DATA;
> +			goto free_pool;
> +		}
> +
> +		ret = parse_specid_event(dev, buffer, sz, &pos, &digest_list);
> +		if (ret || pos > sz) {
> +			log_err("Error parsing SPEC ID Event\n");
> +			ret = EFI_COMPROMISED_DATA;
> +			goto free_pool;
> +		}
> +
> +		while (pos < sz) {
> +			ret = tcg2_parse_event(dev, buffer, sz, &pos,
> +					       &digest_list, &pcr);
> +			if (ret) {
> +				log_err("Error parsing event\n");
> +				goto free_pool;
> +			}
> +		}
> +
> +		memcpy(event_log.buffer, buffer, sz);
> +		event_log.pos = sz;
> +	} else {
> +		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)
> @@ -1664,7 +1944,7 @@ efi_status_t efi_tcg2_register(void)
>  		return EFI_SUCCESS;
>  	}
>  
> -	ret = efi_init_event_log();
> +	ret = efi_init_event_log(dev);
>  	if (ret != EFI_SUCCESS)
>  		goto fail;
>  
> -- 
> 2.25.1
> 


Thanks
/Ilias

Re: [PATCH 3/3] efi_loader: Extend PCR's for firmware measurements

[Ilias Apalodimas]

On Thu, Nov 18, 2021 at 11:47:51AM +0530, Ruchika Gupta wrote:
> Firmwares before U-Boot may be capable of doing tpm measurements
> and passing them to U-Boot in the form of eventlog. However there
> may be scenarios where the firmwares don't have TPM driver and
> are not capable of extending the measurements in the PCRs. To
> cater to such platforms, read the PCR0 to determine if the
> previous firmwares have extended the PCR0. If not, then extend
> the PCR's as the eventlog is parsed.
> 
> Signed-off-by: Ruchika Gupta <ruchika.gupta@linaro.org>
> ---
>  lib/efi_loader/efi_tcg2.c | 86 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 86 insertions(+)
> 
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index c97766eae3..cbd0c7d224 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -178,6 +178,43 @@ static efi_status_t tcg2_pcr_extend(struct udevice *dev, u32 pcr_index,
>  	return EFI_SUCCESS;
>  }
>  
> +/* tcg2_pcr_read - Read PCRs for a TPM2 device for a given tpml_digest_values
> + *
> + * @dev:		device
> + * @digest_list:	list of digest algorithms to extend
> + *
> + * @Return: status code
> + */
> +static efi_status_t tcg2_pcr_read(struct udevice *dev, u32 pcr_index,
> +				    struct tpml_digest_values *digest_list)
> +{
> +	struct tpm_chip_priv *priv;
> +	unsigned int updates, pcr_select_min;
> +	u32 rc;
> +	size_t i;
> +
> +	priv = dev_get_uclass_priv(dev);
> +	if (!priv)
> +		return EFI_DEVICE_ERROR;
> +
> +	pcr_select_min = priv->pcr_select_min;
> +
> +	for (i = 0; i < digest_list->count; i++) {
> +		u16 hash_alg = digest_list->digests[i].hash_alg;
> +		u8 *digest = (u8 *)&digest_list->digests[i].digest;
> +
> +		rc = tpm2_pcr_read(dev, pcr_index, pcr_select_min,
> +				    hash_alg, digest, alg_to_len(hash_alg),
> +				    &updates);
> +		if (rc) {
> +			EFI_PRINT("Failed to read PCR\n");
> +			return EFI_DEVICE_ERROR;
> +		}
> +	}
> +
> +	return EFI_SUCCESS;
> +}
> +
>  /* tcg2_agile_log_append - Append an agile event to out eventlog
>   *
>   * @pcr_index:		PCR index
> @@ -1488,10 +1525,12 @@ static efi_status_t efi_init_event_log(struct udevice *dev)
>  	struct tcg_pcr_event *event_header = NULL;
>  	struct tpml_digest_values digest_list;
>  	size_t spec_event_size;
> +	bool extend_pcr = false;
>  	efi_status_t ret;
>  	u32 pcr, pos;
>  	u64 base;
>  	u32 sz;
> +	int i;
>  
>  	ret = platform_get_tpm2_device(&dev);
>  	if (ret != EFI_SUCCESS)
> @@ -1541,6 +1580,26 @@ static efi_status_t efi_init_event_log(struct udevice *dev)
>  			goto free_pool;
>  		}
>  
> +		ret = tcg2_pcr_read(dev, 0, &digest_list);
> +		if (ret) {
> +			log_err("Error reading PCR 0\n");
> +			goto free_pool;
> +		}
> +
> +		/*
> +		 * If PCR0 is 0, previous firmware didn't have the capability
> +		 * to extend the PCR. In this scenario, extend the PCR as
> +		 * the eventlog is parsed.
> +		 */
> +		for (i = 0; i < digest_list.count; i++) {
> +			u8 buffer[TPM2_DIGEST_LEN] =  { 0 };
> +			u16 hash_alg = digest_list.digests[i].hash_alg;
> +
> +			if (!memcmp((u8 *)&digest_list.digests[i].digest,
> +				    buffer, alg_to_len(hash_alg)))
> +				extend_pcr = true;
> +		}
> +
>  		while (pos < sz) {
>  			ret = tcg2_parse_event(dev, buffer, sz, &pos,
>  					       &digest_list, &pcr);
> @@ -1548,6 +1607,33 @@ static efi_status_t efi_init_event_log(struct udevice *dev)
>  				log_err("Error parsing event\n");
>  				goto free_pool;
>  			}
> +
> +			if (pcr != 0) {
> +				/*
> +				 * Eventlog passed by firmware should extend
> +				 * PCR0 only.

I don't get this comment.  The previous stage bootloader could extend any
PCR (0-7) as long as it adheres to the PC client spec right?
I think we just need the logic below here,  were we decide to start with an
empty eventlog if the prior stage boot loader doesn't expose one.

> +				 */
> +				log_err("Invalid PCR\n");
> +				goto free_pool;
> +			}
> +
> +			if (extend_pcr) {
> +				ret = tcg2_pcr_extend(dev, pcr, &digest_list);
> +				if (ret != EFI_SUCCESS) {
> +					log_err("Error in extending PCR\n");
> +					goto free_pool;
> +				}
> +
> +				/* Clear the digest for next event */
> +				for (i = 0; i < digest_list.count; i++) {
> +					u16 hash_alg =
> +						digest_list.digests[i].hash_alg;
> +					u8 *digest =
> +					   (u8 *)&digest_list.digests[i].digest;
> +
> +					memset(digest, 0, alg_to_len(hash_alg));
> +				}
> +			}
>  		}
>  
>  		memcpy(event_log.buffer, buffer, sz);
> -- 
> 2.25.1
> 


Cheers
/Ilias

Re: [PATCH] efi_loader: fix FinalEvents table if an EFI uses GetEventLog

[Ilias Apalodimas]

Hi Heinrich,

On Wed, Nov 17, 2021 at 11:01:55AM +0100, Heinrich Schuchardt wrote:
> On 11/17/21 10:10, Ilias Apalodimas wrote:
> > As described in the TCG spec [1] in sections 7.1.1 and 7.1.2 the FinalEvent
> > table should include events after GetEventLog has been called.  This
> > currently works for us as long as the kernel is the only EFI application
> > calling that.  Specifically we only implement what's described in 7.1.1.
> > 
> > So refactor the code a bit and support EFI application calling GetEventLog.
> > Events will now be logged in both the EventLog and FinalEvent table as long
> > as ExitBootServices haven't been invoked.
> > 
> > [1] https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specification-rev13-160330final.pdf
> > 
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> >   lib/efi_loader/efi_tcg2.c | 90 ++++++++++++++++++++++++++-------------
> >   1 file changed, 61 insertions(+), 29 deletions(-)
> > 
> > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> > index 189e4a5ba59c..215f4b2b04b8 100644
> > --- a/lib/efi_loader/efi_tcg2.c
> > +++ b/lib/efi_loader/efi_tcg2.c
> > @@ -34,6 +34,7 @@ struct event_log_buffer {
> >   	size_t final_pos; /* final events config table position */
> >   	size_t last_event_size;
> >   	bool get_event_called;
> > +	bool ebs_called;
> 
> Please, add documentation for the elements of the structure. Not every
> reader will be aware of ebs_called referring to ExitBootServices().

Sure

> 
> >   	bool truncated;
> >   };
> > 
> > @@ -186,39 +187,29 @@ static efi_status_t tcg2_pcr_extend(struct udevice *dev, u32 pcr_index,
> >   	return EFI_SUCCESS;
> > 

[...]

> >   /**
> > @@ -1303,6 +1333,7 @@ static efi_status_t efi_init_event_log(void)
> >   	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;
> > 
> >   	/*
> > @@ -1792,6 +1823,7 @@ efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context)
> > 
> >   	EFI_ENTRY("%p, %p", event, context);
> 
> This is called in EFI_EVENT_GROUP_EXIT_BOOT_SERVICES.
> 
> This implies that whatever happens in
> EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES is measured normally. Does
> this conform to the TCG2 standard?

Yes I think so.  My understanding of 7.1.2 diagram in the spec is:
- Log all events to the EventLog buffer if GetEventLog() hasn't been called
- Log all events to the EventLog buffer *and* the FinalEvent config table if 
  GetEventLog() has been called
- If you are in EBS(), you don't know if the firmware has cleaned up the
  EventLog buffer, so log these events in the FinalEvent config table only.
> 
> > 
> > +	event_log.ebs_called = true;
> 
> How should a failed call to ExitBootServices() be handled?
> E.g. invalid memory map?

Good question.  We also have efi_tcg2_notify_exit_boot_services_failed().
If the EventLog buffer hasn't been destroyed from memory we can switch the 
ebs_called = false?

Cheers
/Ilias
> 
> Best regards
> 
> Heinrich
> 
> >   	ret = platform_get_tpm2_device(&dev);
> >   	if (ret != EFI_SUCCESS)
> >   		goto out;
> > 
> 

Re: [PATCH] efi_loader: fix FinalEvents table if an EFI uses GetEventLog

[Heinrich Schuchardt]

On 11/17/21 10:10, Ilias Apalodimas wrote:
> As described in the TCG spec [1] in sections 7.1.1 and 7.1.2 the FinalEvent
> table should include events after GetEventLog has been called.  This
> currently works for us as long as the kernel is the only EFI application
> calling that.  Specifically we only implement what's described in 7.1.1.
>
> So refactor the code a bit and support EFI application calling GetEventLog.
> Events will now be logged in both the EventLog and FinalEvent table as long
> as ExitBootServices haven't been invoked.
>
> [1] https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specification-rev13-160330final.pdf
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>   lib/efi_loader/efi_tcg2.c | 90 ++++++++++++++++++++++++++-------------
>   1 file changed, 61 insertions(+), 29 deletions(-)
>
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 189e4a5ba59c..215f4b2b04b8 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -34,6 +34,7 @@ struct event_log_buffer {
>   	size_t final_pos; /* final events config table position */
>   	size_t last_event_size;
>   	bool get_event_called;
> +	bool ebs_called;

Please, add documentation for the elements of the structure. Not every
reader will be aware of ebs_called referring to ExitBootServices().

>   	bool truncated;
>   };
>
> @@ -186,39 +187,29 @@ static efi_status_t tcg2_pcr_extend(struct udevice *dev, u32 pcr_index,
>   	return EFI_SUCCESS;
>   }
>
> -/* tcg2_agile_log_append - Append an agile event to out eventlog
> +/* put_event - Append an agile event to an eventlog
>    *
>    * @pcr_index:		PCR index
>    * @event_type:		type of event added
>    * @digest_list:	list of digest algorithms to add
>    * @size:		size of event
>    * @event:		event to add
> + * @log:		log buffer to append the event
>    *
> - * @Return: status code
>    */
> -static efi_status_t tcg2_agile_log_append(u32 pcr_index, u32 event_type,
> -					  struct tpml_digest_values *digest_list,
> -					  u32 size, u8 event[])
> +static void put_event(u32 pcr_index, u32 event_type,
> +		      struct tpml_digest_values *digest_list, u32 size,
> +		      u8 event[], void *log)
>   {
> -	void *log = (void *)((uintptr_t)event_log.buffer + event_log.pos);
>   	size_t pos;
>   	size_t i;
>   	u32 event_size;
>
> -	if (event_log.get_event_called)
> -		log = (void *)((uintptr_t)event_log.final_buffer +
> -			       event_log.final_pos);
> -
>   	/*
>   	 * size refers to the length of event[] only, we need to check against
>   	 * the final tcg_pcr_event2 size
>   	 */
>   	event_size = size + tcg_event_final_size(digest_list);
> -	if (event_log.pos + event_size > TPM2_EVENT_LOG_SIZE ||
> -	    event_log.final_pos + event_size > TPM2_EVENT_LOG_SIZE) {
> -		event_log.truncated = true;
> -		return EFI_VOLUME_FULL;
> -	}
>
>   	put_unaligned_le32(pcr_index, log);
>   	pos = offsetof(struct tcg_pcr_event2, event_type);
> @@ -242,25 +233,64 @@ static efi_status_t tcg2_agile_log_append(u32 pcr_index, u32 event_type,
>   	memcpy((void *)((uintptr_t)log + pos), event, size);
>   	pos += size;
>
> -	/* make sure the calculated buffer is what we checked against */
> +	/*
> +	 * make sure the calculated buffer is what we checked against
> +	 * This check should never fail.  It checks the code above is
> +	 * calculating the right length for the event we are adding
> +	 * */
>   	if (pos != event_size)
> -		return EFI_INVALID_PARAMETER;
> +		log_err("Appending to the EventLog failed\n");
>
> -	/* if GetEventLog hasn't been called update the normal log */
> -	if (!event_log.get_event_called) {
> -		event_log.pos += pos;
> -		event_log.last_event_size = pos;
> -	} else {
> -	/* if GetEventLog has been called update config table log */
> -		struct efi_tcg2_final_events_table *final_event;
> +}
>
> -		final_event =
> -			(struct efi_tcg2_final_events_table *)(event_log.final_buffer);
> -		final_event->number_of_events++;
> -		event_log.final_pos += pos;
> +/* tcg2_agile_log_append - Append an agile event to an eventlog
> + *
> + * @pcr_index:		PCR index
> + * @event_type:		type of event added
> + * @digest_list:	list of digest algorithms to add
> + * @size:		size of event
> + * @event:		event to add
> + * @log:		log buffer to append the event
> + *
> + * @Return: status code
> + */
> +static efi_status_t tcg2_agile_log_append(u32 pcr_index, u32 event_type,
> +					  struct tpml_digest_values *digest_list,
> +					  u32 size, u8 event[])
> +{
> +	void *log = (void *)((uintptr_t)event_log.buffer + event_log.pos);
> +	u32 event_size = size + tcg_event_final_size(digest_list);
> +	struct efi_tcg2_final_events_table *final_event;
> +	efi_status_t ret = EFI_SUCCESS;
> +
> +	/* if ExitBootServices hasn't been called update the normal log */
> +	if (!event_log.ebs_called) {
> +		if (event_log.truncated ||
> +		    event_log.pos + event_size > TPM2_EVENT_LOG_SIZE) {
> +			event_log.truncated = true;
> +			return EFI_VOLUME_FULL;
> +		}
> +		put_event(pcr_index, event_type, digest_list, size, event, log);
> +		event_log.pos += event_size;
> +		event_log.last_event_size = event_size;
>   	}
>
> -	return EFI_SUCCESS;
> +	if (!event_log.get_event_called)
> +		return ret;
> +
> +	/* if GetEventLog has been called update FinalEventLog as well */
> +	if (event_log.final_pos + event_size > TPM2_EVENT_LOG_SIZE)
> +		return EFI_VOLUME_FULL;
> +
> +	log = (void *)((uintptr_t)event_log.final_buffer + event_log.final_pos);
> +	put_event(pcr_index, event_type, digest_list, size, event, log);
> +
> +	final_event =
> +		(struct efi_tcg2_final_events_table *)event_log.final_buffer;
> +	final_event->number_of_events++;
> +	event_log.final_pos += event_size;
> +
> +	return ret;
>   }
>
>   /**
> @@ -1303,6 +1333,7 @@ static efi_status_t efi_init_event_log(void)
>   	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;
>
>   	/*
> @@ -1792,6 +1823,7 @@ efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context)
>
>   	EFI_ENTRY("%p, %p", event, context);

This is called in EFI_EVENT_GROUP_EXIT_BOOT_SERVICES.

This implies that whatever happens in
EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES is measured normally. Does
this conform to the TCG2 standard?

>
> +	event_log.ebs_called = true;

How should a failed call to ExitBootServices() be handled?
E.g. invalid memory map?

Best regards

Heinrich

>   	ret = platform_get_tpm2_device(&dev);
>   	if (ret != EFI_SUCCESS)
>   		goto out;
>

[PATCH] efi_loader: fix FinalEvents table if an EFI uses GetEventLog

[Ilias Apalodimas]

As described in the TCG spec [1] in sections 7.1.1 and 7.1.2 the FinalEvent
table should include events after GetEventLog has been called.  This
currently works for us as long as the kernel is the only EFI application
calling that.  Specifically we only implement what's described in 7.1.1.

So refactor the code a bit and support EFI application calling GetEventLog.
Events will now be logged in both the EventLog and FinalEvent table as long
as ExitBootServices haven't been invoked.

[1] https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specification-rev13-160330final.pdf

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 lib/efi_loader/efi_tcg2.c | 90 ++++++++++++++++++++++++++-------------
 1 file changed, 61 insertions(+), 29 deletions(-)

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 189e4a5ba59c..215f4b2b04b8 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -34,6 +34,7 @@ struct event_log_buffer {
 	size_t final_pos; /* final events config table position */
 	size_t last_event_size;
 	bool get_event_called;
+	bool ebs_called;
 	bool truncated;
 };
 
@@ -186,39 +187,29 @@ static efi_status_t tcg2_pcr_extend(struct udevice *dev, u32 pcr_index,
 	return EFI_SUCCESS;
 }
 
-/* tcg2_agile_log_append - Append an agile event to out eventlog
+/* put_event - Append an agile event to an eventlog
  *
  * @pcr_index:		PCR index
  * @event_type:		type of event added
  * @digest_list:	list of digest algorithms to add
  * @size:		size of event
  * @event:		event to add
+ * @log:		log buffer to append the event
  *
- * @Return: status code
  */
-static efi_status_t tcg2_agile_log_append(u32 pcr_index, u32 event_type,
-					  struct tpml_digest_values *digest_list,
-					  u32 size, u8 event[])
+static void put_event(u32 pcr_index, u32 event_type,
+		      struct tpml_digest_values *digest_list, u32 size,
+		      u8 event[], void *log)
 {
-	void *log = (void *)((uintptr_t)event_log.buffer + event_log.pos);
 	size_t pos;
 	size_t i;
 	u32 event_size;
 
-	if (event_log.get_event_called)
-		log = (void *)((uintptr_t)event_log.final_buffer +
-			       event_log.final_pos);
-
 	/*
 	 * size refers to the length of event[] only, we need to check against
 	 * the final tcg_pcr_event2 size
 	 */
 	event_size = size + tcg_event_final_size(digest_list);
-	if (event_log.pos + event_size > TPM2_EVENT_LOG_SIZE ||
-	    event_log.final_pos + event_size > TPM2_EVENT_LOG_SIZE) {
-		event_log.truncated = true;
-		return EFI_VOLUME_FULL;
-	}
 
 	put_unaligned_le32(pcr_index, log);
 	pos = offsetof(struct tcg_pcr_event2, event_type);
@@ -242,25 +233,64 @@ static efi_status_t tcg2_agile_log_append(u32 pcr_index, u32 event_type,
 	memcpy((void *)((uintptr_t)log + pos), event, size);
 	pos += size;
 
-	/* make sure the calculated buffer is what we checked against */
+	/*
+	 * make sure the calculated buffer is what we checked against
+	 * This check should never fail.  It checks the code above is
+	 * calculating the right length for the event we are adding
+	 * */
 	if (pos != event_size)
-		return EFI_INVALID_PARAMETER;
+		log_err("Appending to the EventLog failed\n");
 
-	/* if GetEventLog hasn't been called update the normal log */
-	if (!event_log.get_event_called) {
-		event_log.pos += pos;
-		event_log.last_event_size = pos;
-	} else {
-	/* if GetEventLog has been called update config table log */
-		struct efi_tcg2_final_events_table *final_event;
+}
 
-		final_event =
-			(struct efi_tcg2_final_events_table *)(event_log.final_buffer);
-		final_event->number_of_events++;
-		event_log.final_pos += pos;
+/* tcg2_agile_log_append - Append an agile event to an eventlog
+ *
+ * @pcr_index:		PCR index
+ * @event_type:		type of event added
+ * @digest_list:	list of digest algorithms to add
+ * @size:		size of event
+ * @event:		event to add
+ * @log:		log buffer to append the event
+ *
+ * @Return: status code
+ */
+static efi_status_t tcg2_agile_log_append(u32 pcr_index, u32 event_type,
+					  struct tpml_digest_values *digest_list,
+					  u32 size, u8 event[])
+{
+	void *log = (void *)((uintptr_t)event_log.buffer + event_log.pos);
+	u32 event_size = size + tcg_event_final_size(digest_list);
+	struct efi_tcg2_final_events_table *final_event;
+	efi_status_t ret = EFI_SUCCESS;
+
+	/* if ExitBootServices hasn't been called update the normal log */
+	if (!event_log.ebs_called) {
+		if (event_log.truncated ||
+		    event_log.pos + event_size > TPM2_EVENT_LOG_SIZE) {
+			event_log.truncated = true;
+			return EFI_VOLUME_FULL;
+		}
+		put_event(pcr_index, event_type, digest_list, size, event, log);
+		event_log.pos += event_size;
+		event_log.last_event_size = event_size;
 	}
 
-	return EFI_SUCCESS;
+	if (!event_log.get_event_called)
+		return ret;
+
+	/* if GetEventLog has been called update FinalEventLog as well */
+	if (event_log.final_pos + event_size > TPM2_EVENT_LOG_SIZE)
+		return EFI_VOLUME_FULL;
+
+	log = (void *)((uintptr_t)event_log.final_buffer + event_log.final_pos);
+	put_event(pcr_index, event_type, digest_list, size, event, log);
+
+	final_event =
+		(struct efi_tcg2_final_events_table *)event_log.final_buffer;
+	final_event->number_of_events++;
+	event_log.final_pos += event_size;
+
+	return ret;
 }
 
 /**
@@ -1303,6 +1333,7 @@ static efi_status_t efi_init_event_log(void)
 	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;
 
 	/*
@@ -1792,6 +1823,7 @@ efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context)
 
 	EFI_ENTRY("%p, %p", event, context);
 
+	event_log.ebs_called = true;
 	ret = platform_get_tpm2_device(&dev);
 	if (ret != EFI_SUCCESS)
 		goto out;
-- 
2.33.1