[PATCH v2 0/6] add measurement support

[PATCH v2 0/6] add measurement support

[Masahisa Kojima]

This patch series add the support of measurement
descibed in TCG PC Client PFP spec(Version 1.05 Revision 23).

Eventlog generated with this patch series are tested on
the aarch64 based machine(Socionext Developerbox) and fTPM
running on OP-TEE.
The eventlog result is almost same result as the one
generated by edk2 running on the Devloperbox and Secure96.

This patch series does not cover all measurement requirements
described in TCG spec, the remaining items will be supported
in the future.
Major missing items in TCG PC Client PFP spec:
 1) If the secure boot variables are updated after they are
   initially measured in PCR[7] and before ExitBootServices()
   has completed, the platform MAY be restarted OR the variables
   MUST be remeasured into PCR[7].
 2) SMBIOS structure measurement
 3) "DeployedMode" and "AuditMode" measurement
 4) EV_EFI_GPT_EVENT event
 5) Measurement of U-boot itself. I assume U-boot measurement will be done
    by the former firmware such as trusted firmware.

Masahisa Kojima (6):
  efi_loader: increase eventlog buffer size
  efi_loader: add secure boot variable measurement
  efi_loader: add boot variable measurement
  efi_loader: add ExitBootServices() measurement
  efi_loader: refactor efi_append_scrtm_version()
  efi_loader: add comment for efi_tcg2.h

 include/efi_loader.h          |   5 +
 include/efi_tcg2.h            |  71 ++++++++
 include/tpm-v2.h              |  18 +-
 lib/efi_loader/Kconfig        |   2 +-
 lib/efi_loader/efi_boottime.c |  25 +++
 lib/efi_loader/efi_tcg2.c     | 331 +++++++++++++++++++++++++++++++++-
 6 files changed, 444 insertions(+), 8 deletions(-)

-- 
2.17.1

[PATCH v2 1/6] efi_loader: increase eventlog buffer size

[Masahisa Kojima]

TCG PC Client PFP spec says "The Log Area Minimum Length
for the TCG event log MUST be at least 64KB." in ACPI chapter.
This commit increase the buffer size to 64KB.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---

Changes in v2:
- increase buffer size to 64KB, it follows the minimum size
  requirement stated in TCG spec

 lib/efi_loader/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 156b391521..20edac6932 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -326,7 +326,7 @@ config EFI_TCG2_PROTOCOL
 config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE
 	int "EFI_TCG2_PROTOCOL EventLog size"
 	depends on EFI_TCG2_PROTOCOL
-	default 4096
+	default 65536
 	help
 		Define the size of the EventLog for EFI_TCG2_PROTOCOL. Note that
 		this is going to be allocated twice. One for the eventlog it self
-- 
2.17.1

[PATCH v2 2/6] efi_loader: add secure boot variable measurement

[Masahisa Kojima]

TCG PC Client PFP spec requires to measure the secure
boot policy before validating the UEFI image.
This commit adds the secure boot variable measurement
of "SecureBoot", "PK", "KEK", "db" and "dbx".

Note that this implementation assumes that secure boot
variables are pre-configured and not be set/updated in runtime.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---

Changes in v2:
- missing null check for getting variable data
- some minor fix for readability

 include/efi_tcg2.h        |  20 ++++++
 lib/efi_loader/efi_tcg2.c | 139 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 159 insertions(+)

diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
index bcfb98168a..8d7b77c087 100644
--- a/include/efi_tcg2.h
+++ b/include/efi_tcg2.h
@@ -142,6 +142,26 @@ struct efi_tcg2_final_events_table {
 	struct tcg_pcr_event2 event[];
 };
 
+/**
+ * struct tdUEFI_VARIABLE_DATA
+ * @variable_name:		The vendorGUID parameter in the
+ *				GetVariable() API.
+ * @unicode_name_length:	The length in CHAR16 of the Unicode name of
+ *				the variable.
+ * @variable_data_length:	The size of the variable data.
+ * @unicode_name:		The CHAR16 unicode name of the variable
+ *				without NULL-terminator.
+ * @variable_data:		The data parameter of the efi variable
+ *				in the GetVariable() API.
+ */
+struct efi_tcg2_uefi_variable_data {
+	efi_guid_t variable_name;
+	u64 unicode_name_length;
+	u64 variable_data_length;
+	u16 unicode_name[1];
+	u8 variable_data[1];
+};
+
 struct efi_tcg2_protocol {
 	efi_status_t (EFIAPI * get_capability)(struct efi_tcg2_protocol *this,
 					       struct efi_tcg2_boot_service_capability *capability);
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 1319a8b378..12db6f6b7c 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -78,6 +78,19 @@ static const struct digest_info hash_algo_list[] = {
 	},
 };
 
+struct variable_info {
+	u16		*name;
+	const efi_guid_t	*guid;
+};
+
+static struct variable_info secure_variables[] = {
+	{L"SecureBoot", &efi_global_variable_guid},
+	{L"PK", &efi_global_variable_guid},
+	{L"KEK", &efi_global_variable_guid},
+	{L"db", &efi_guid_image_security_database},
+	{L"dbx", &efi_guid_image_security_database},
+};
+
 #define MAX_HASH_COUNT ARRAY_SIZE(hash_algo_list)
 
 /**
@@ -1264,6 +1277,39 @@ free_pool:
 	return ret;
 }
 
+/**
+ * tcg2_measure_event() - common function to add event log and extend PCR
+ *
+ * @dev:		TPM device
+ * @pcr_index:		PCR index
+ * @event_type:		type of event added
+ * @size:		event size
+ * @event:		event data
+ *
+ * Return:	status code
+ */
+static efi_status_t EFIAPI
+tcg2_measure_event(struct udevice *dev, u32 pcr_index, u32 event_type,
+		   u32 size, u8 event[])
+{
+	struct tpml_digest_values digest_list;
+	efi_status_t ret;
+
+	ret = tcg2_create_digest(event, size, &digest_list);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	ret = tcg2_pcr_extend(dev, pcr_index, &digest_list);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	ret = tcg2_agile_log_append(pcr_index, event_type, &digest_list,
+				    size, event);
+
+out:
+	return ret;
+}
+
 /**
  * efi_append_scrtm_version - Append an S-CRTM EV_S_CRTM_VERSION event on the
  *			      eventlog and extend the PCRs
@@ -1294,6 +1340,92 @@ out:
 	return ret;
 }
 
+/**
+ * tcg2_measure_variable() - add variable event log and extend PCR
+ *
+ * @dev:		TPM device
+ * @pcr_index:		PCR index
+ * @event_type:		type of event added
+ * @var_name:		variable name
+ * @guid:		guid
+ * @data_size:		variable data size
+ * @data:		variable data
+ *
+ * Return:	status code
+ */
+static efi_status_t tcg2_measure_variable(struct udevice *dev, u32 pcr_index,
+					  u32 event_type, u16 *var_name,
+					  const efi_guid_t *guid,
+					  efi_uintn_t data_size, u8 *data)
+{
+	u32 event_size;
+	efi_status_t ret;
+	struct efi_tcg2_uefi_variable_data *event;
+
+	event_size = sizeof(event->variable_name) +
+		     sizeof(event->unicode_name_length) +
+		     sizeof(event->variable_data_length) +
+		     (u16_strlen(var_name) * sizeof(u16)) + data_size;
+	event = malloc(event_size);
+	if (!event)
+		return EFI_OUT_OF_RESOURCES;
+
+	guidcpy(&event->variable_name, guid);
+	event->unicode_name_length = u16_strlen(var_name);
+	event->variable_data_length = data_size;
+	memcpy(event->unicode_name, var_name,
+	       (event->unicode_name_length * sizeof(u16)));
+	memcpy((u16 *)event->unicode_name + event->unicode_name_length,
+	       data, data_size);
+	ret = tcg2_measure_event(dev, pcr_index, event_type, event_size,
+				 (u8 *)event);
+	free(event);
+	return ret;
+}
+
+/**
+ * tcg2_measure_secure_boot_variable() - measure secure boot variables
+ *
+ * @dev:	TPM device
+ *
+ * Return:	status code
+ */
+static efi_status_t tcg2_measure_secure_boot_variable(struct udevice *dev)
+{
+	u8 *data;
+	efi_uintn_t data_size;
+	u32 count, i;
+	efi_status_t ret;
+
+	count = ARRAY_SIZE(secure_variables);
+	for (i = 0; i < count; i++) {
+		data = efi_get_var(secure_variables[i].name,
+				   secure_variables[i].guid,
+				   &data_size);
+		if (data == NULL) {
+			log_info("%ls not found\n", secure_variables[i].name);
+			continue;
+		}
+
+		ret = tcg2_measure_variable(dev, 7,
+					    EV_EFI_VARIABLE_DRIVER_CONFIG,
+					    secure_variables[i].name,
+					    secure_variables[i].guid,
+					    data_size, (u8 *)data);
+		free(data);
+		if (ret != EFI_SUCCESS)
+			goto error;
+	}
+
+	/*
+	 * TODO: add DBT and DBR measurement support when u-boot supports
+	 * these variables.
+	 */
+
+error:
+	return ret;
+}
+
 /**
  * efi_tcg2_register() - register EFI_TCG2_PROTOCOL
  *
@@ -1328,6 +1460,13 @@ efi_status_t efi_tcg2_register(void)
 		tcg2_uninit();
 		goto fail;
 	}
+
+	ret = tcg2_measure_secure_boot_variable(dev);
+	if (ret != EFI_SUCCESS) {
+		tcg2_uninit();
+		goto fail;
+	}
+
 	return ret;
 
 fail:
-- 
2.17.1

[PATCH v2 3/6] efi_loader: add boot variable measurement

[Masahisa Kojima]

TCG PC Client PFP spec requires to measure "Boot####"
and "BootOrder" variables, EV_SEPARATOR event prior
to the Ready to Boot invocation.
Since u-boot does not implement Ready to Boot event,
these measurements are performed when efi_start_image() is called.

TCG spec also requires to measure "Calling EFI Application from
Boot Option" for each boot attempt, and "Returning from EFI
Application from Boot Option" if a boot device returns control
back to the Boot Manager.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---

Changes in v2:
- use efi_create_indexed_name() for "Boot####" variable

 include/efi_loader.h          |   4 ++
 include/tpm-v2.h              |  18 ++++-
 lib/efi_loader/efi_boottime.c |  20 ++++++
 lib/efi_loader/efi_tcg2.c     | 121 ++++++++++++++++++++++++++++++++++
 4 files changed, 162 insertions(+), 1 deletion(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index b81180cfda..703b675950 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -407,6 +407,10 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size);
 efi_status_t efi_init_variables(void);
 /* Notify ExitBootServices() is called */
 void efi_variables_boot_exit_notify(void);
+/* Measure efi application invocation */
+efi_status_t EFIAPI efi_tcg2_measure_efi_app_invocation(void);
+/* Measure efi application exit */
+efi_status_t EFIAPI efi_tcg2_measure_efi_app_exit(void);
 /* Called by bootefi to initialize root node */
 efi_status_t efi_root_node_register(void);
 /* Called by bootefi to initialize runtime */
diff --git a/include/tpm-v2.h b/include/tpm-v2.h
index 247b386967..325c73006e 100644
--- a/include/tpm-v2.h
+++ b/include/tpm-v2.h
@@ -73,7 +73,7 @@ struct udevice;
 /*
  * event types, cf.
  * "TCG PC Client Platform Firmware Profile Specification", Family "2.0"
- * rev 1.04, June 3, 2019
+ * Level 00 Version 1.05 Revision 23, May 7, 2021
  */
 #define EV_EFI_EVENT_BASE			((u32)0x80000000)
 #define EV_EFI_VARIABLE_DRIVER_CONFIG		((u32)0x80000001)
@@ -85,8 +85,24 @@ struct udevice;
 #define EV_EFI_ACTION				((u32)0x80000007)
 #define EV_EFI_PLATFORM_FIRMWARE_BLOB		((u32)0x80000008)
 #define EV_EFI_HANDOFF_TABLES			((u32)0x80000009)
+#define EV_EFI_PLATFORM_FIRMWARE_BLOB2		((u32)0x8000000A)
+#define EV_EFI_HANDOFF_TABLES2			((u32)0x8000000B)
+#define EV_EFI_VARIABLE_BOOT2			((u32)0x8000000C)
 #define EV_EFI_HCRTM_EVENT			((u32)0x80000010)
 #define EV_EFI_VARIABLE_AUTHORITY		((u32)0x800000E0)
+#define EV_EFI_SPDM_FIRMWARE_BLOB		((u32)0x800000E1)
+#define EV_EFI_SPDM_FIRMWARE_CONFIG		((u32)0x800000E2)
+
+#define EFI_CALLING_EFI_APPLICATION         \
+	"Calling EFI Application from Boot Option"
+#define EFI_RETURNING_FROM_EFI_APPLICATION  \
+	"Returning from EFI Application from Boot Option"
+#define EFI_EXIT_BOOT_SERVICES_INVOCATION   \
+	"Exit Boot Services Invocation"
+#define EFI_EXIT_BOOT_SERVICES_FAILED       \
+	"Exit Boot Services Returned with Failure"
+#define EFI_EXIT_BOOT_SERVICES_SUCCEEDED    \
+	"Exit Boot Services Returned with Success"
 
 /* TPMS_TAGGED_PROPERTY Structure */
 struct tpms_tagged_property {
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index f6d5ba05e3..2914800c56 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -2993,6 +2993,16 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,
 	image_obj->exit_status = &exit_status;
 	image_obj->exit_jmp = &exit_jmp;
 
+	if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
+		if (image_obj->image_type == IMAGE_SUBSYSTEM_EFI_APPLICATION) {
+			ret = efi_tcg2_measure_efi_app_invocation();
+			if (ret != EFI_SUCCESS) {
+				EFI_PRINT("tcg2 measurement fails(0x%lx)\n",
+					  ret);
+			}
+		}
+	}
+
 	/* call the image! */
 	if (setjmp(&exit_jmp)) {
 		/*
@@ -3251,6 +3261,16 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle,
 	    exit_status != EFI_SUCCESS)
 		efi_delete_image(image_obj, loaded_image_protocol);
 
+	if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
+		if (image_obj->image_type == IMAGE_SUBSYSTEM_EFI_APPLICATION) {
+			ret = efi_tcg2_measure_efi_app_exit();
+			if (ret != EFI_SUCCESS) {
+				EFI_PRINT("tcg2 measurement fails(0x%lx)\n",
+					  ret);
+			}
+		}
+	}
+
 	/* Make sure entry/exit counts for EFI world cross-overs match */
 	EFI_EXIT(exit_status);
 
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 12db6f6b7c..d59fc5a890 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -35,6 +35,7 @@ struct event_log_buffer {
 };
 
 static struct event_log_buffer event_log;
+static bool tcg2_efi_app_invoked;
 /*
  * When requesting TPM2_CAP_TPM_PROPERTIES the value is on a standard offset.
  * Since the current tpm2_get_capability() response buffers starts at
@@ -1383,6 +1384,126 @@ static efi_status_t tcg2_measure_variable(struct udevice *dev, u32 pcr_index,
 	return ret;
 }
 
+/**
+ * tcg2_measure_boot_variable() - measure boot variables
+ *
+ * @dev:	TPM device
+ *
+ * Return:	status code
+ */
+static efi_status_t tcg2_measure_boot_variable(struct udevice *dev)
+{
+	u16 *boot_order;
+	u16 *boot_index;
+	u16 var_name[] = L"BootOrder";
+	u16 boot_name[] = L"Boot####";
+	u8 *bootvar;
+	efi_uintn_t var_data_size;
+	u32 count, i;
+	efi_status_t ret;
+
+	boot_order = efi_get_var(var_name, &efi_global_variable_guid,
+				 &var_data_size);
+	if (!boot_order) {
+		log_info("BootOrder not defined\n");
+		ret = EFI_NOT_FOUND;
+		goto error;
+	}
+
+	ret = tcg2_measure_variable(dev, 1, EV_EFI_VARIABLE_BOOT2, var_name,
+				    &efi_global_variable_guid, var_data_size,
+				    (u8 *)boot_order);
+	if (ret != EFI_SUCCESS)
+		goto error;
+
+	count = var_data_size / sizeof(*boot_order);
+	boot_index = boot_order;
+	for (i = 0; i < count; i++) {
+		efi_create_indexed_name(boot_name, sizeof(boot_name), "Boot", *boot_index++);
+
+		bootvar = efi_get_var(boot_name, &efi_global_variable_guid,
+				      &var_data_size);
+
+		if (!bootvar) {
+			log_info("%ls not found\n", boot_name);
+			continue;
+		}
+
+		ret = tcg2_measure_variable(dev, 1, EV_EFI_VARIABLE_BOOT2,
+					    boot_name,
+					    &efi_global_variable_guid,
+					    var_data_size, bootvar);
+		free(bootvar);
+		if (ret != EFI_SUCCESS)
+			goto error;
+	}
+
+error:
+	free(boot_order);
+	return ret;
+}
+
+/**
+ * efi_tcg2_measure_efi_app_invocation() - measure efi app invocation
+ *
+ * Return:	status code
+ */
+efi_status_t EFIAPI efi_tcg2_measure_efi_app_invocation(void)
+{
+	efi_status_t ret;
+	u32 pcr_index;
+	struct udevice *dev;
+	u32 event = 0;
+
+	if (tcg2_efi_app_invoked)
+		return EFI_SUCCESS;
+
+	ret = platform_get_tpm2_device(&dev);
+	if (ret != EFI_SUCCESS)
+		return ret;
+
+	ret = tcg2_measure_boot_variable(dev);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	ret = tcg2_measure_event(dev, 4, EV_EFI_ACTION,
+				 strlen(EFI_CALLING_EFI_APPLICATION),
+				 (u8 *)EFI_CALLING_EFI_APPLICATION);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	for (pcr_index = 0; pcr_index <= 7; pcr_index++) {
+		ret = tcg2_measure_event(dev, pcr_index, EV_SEPARATOR,
+					 sizeof(event), (u8 *)&event);
+		if (ret != EFI_SUCCESS)
+			goto out;
+	}
+
+	tcg2_efi_app_invoked = true;
+out:
+	return ret;
+}
+
+/**
+ * efi_tcg2_measure_efi_app_exit() - measure efi app exit
+ *
+ * Return:	status code
+ */
+efi_status_t EFIAPI efi_tcg2_measure_efi_app_exit(void)
+{
+	efi_status_t ret;
+	struct udevice *dev;
+
+	ret = platform_get_tpm2_device(&dev);
+	if (ret != EFI_SUCCESS)
+		return ret;
+
+	ret = tcg2_measure_event(dev, 4, EV_EFI_ACTION,
+				 strlen(EFI_RETURNING_FROM_EFI_APPLICATION),
+				 (u8 *)EFI_RETURNING_FROM_EFI_APPLICATION);
+	return ret;
+}
+
 /**
  * tcg2_measure_secure_boot_variable() - measure secure boot variables
  *
-- 
2.17.1

[PATCH v2 4/6] efi_loader: add ExitBootServices() measurement

[Masahisa Kojima]

TCG PC Client PFP spec requires to measure
"Exit Boot Services Invocation" if ExitBootServices() is invoked.
Depending upon the return code from the ExitBootServices() call,
"Exit Boot Services Returned with Success" or "Exit Boot Services
Returned with Failure" is also measured.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---

Changes in v2:
- use strlen instead of sizeof, event log for EV_EFI_ACTION string
  shall not include NUL terminator

 include/efi_loader.h          |  1 +
 lib/efi_loader/efi_boottime.c |  5 +++
 lib/efi_loader/efi_tcg2.c     | 70 +++++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index 703b675950..355fd184bc 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -407,6 +407,7 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size);
 efi_status_t efi_init_variables(void);
 /* Notify ExitBootServices() is called */
 void efi_variables_boot_exit_notify(void);
+efi_status_t efi_tcg2_notify_exit_boot_services_failed(void);
 /* Measure efi application invocation */
 efi_status_t EFIAPI efi_tcg2_measure_efi_app_invocation(void);
 /* Measure efi application exit */
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 2914800c56..6e07ef65bc 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -2181,6 +2181,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
 	efi_set_watchdog(0);
 	WATCHDOG_RESET();
 out:
+	if (ret != EFI_SUCCESS) {
+		if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL))
+			efi_tcg2_notify_exit_boot_services_failed();
+	}
+
 	return EFI_EXIT(ret);
 }
 
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index d59fc5a890..32e3818af4 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -1504,6 +1504,67 @@ efi_status_t EFIAPI efi_tcg2_measure_efi_app_exit(void)
 	return ret;
 }
 
+/**
+ * efi_tcg2_notify_exit_boot_services() - ExitBootService callback
+ *
+ * @event:	callback event
+ * @context:	callback context
+ */
+static void EFIAPI
+efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context)
+{
+	efi_status_t ret;
+	struct udevice *dev;
+
+	EFI_ENTRY("%p, %p", event, context);
+
+	ret = platform_get_tpm2_device(&dev);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	ret = tcg2_measure_event(dev, 5, EV_EFI_ACTION,
+				 strlen(EFI_EXIT_BOOT_SERVICES_INVOCATION),
+				 (u8 *)EFI_EXIT_BOOT_SERVICES_INVOCATION);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	ret = tcg2_measure_event(dev, 5, EV_EFI_ACTION,
+				 strlen(EFI_EXIT_BOOT_SERVICES_SUCCEEDED),
+				 (u8 *)EFI_EXIT_BOOT_SERVICES_SUCCEEDED);
+
+out:
+	EFI_EXIT(ret);
+}
+
+/**
+ * efi_tcg2_notify_exit_boot_services_failed()
+ *  - notify ExitBootServices() is failed
+ *
+ * Return:	status code
+ */
+efi_status_t efi_tcg2_notify_exit_boot_services_failed(void)
+{
+	struct udevice *dev;
+	efi_status_t ret;
+
+	ret = platform_get_tpm2_device(&dev);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	ret = tcg2_measure_event(dev, 5, EV_EFI_ACTION,
+				 strlen(EFI_EXIT_BOOT_SERVICES_INVOCATION),
+				 (u8 *)EFI_EXIT_BOOT_SERVICES_INVOCATION);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	ret = tcg2_measure_event(dev, 5, EV_EFI_ACTION,
+				 strlen(EFI_EXIT_BOOT_SERVICES_FAILED),
+				 (u8 *)EFI_EXIT_BOOT_SERVICES_FAILED);
+
+out:
+	return ret;
+}
+
 /**
  * tcg2_measure_secure_boot_variable() - measure secure boot variables
  *
@@ -1558,6 +1619,7 @@ efi_status_t efi_tcg2_register(void)
 {
 	efi_status_t ret = EFI_SUCCESS;
 	struct udevice *dev;
+	struct efi_event *event;
 
 	ret = platform_get_tpm2_device(&dev);
 	if (ret != EFI_SUCCESS) {
@@ -1582,6 +1644,14 @@ efi_status_t efi_tcg2_register(void)
 		goto fail;
 	}
 
+	ret = efi_create_event(EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK,
+			       efi_tcg2_notify_exit_boot_services, NULL,
+			       NULL, &event);
+	if (ret != EFI_SUCCESS) {
+		tcg2_uninit();
+		goto fail;
+	}
+
 	ret = tcg2_measure_secure_boot_variable(dev);
 	if (ret != EFI_SUCCESS) {
 		tcg2_uninit();
-- 
2.17.1

[PATCH v2 5/6] efi_loader: refactor efi_append_scrtm_version()

[Masahisa Kojima]

Refactor efi_append_scrtm_version() to use common
function for adding eventlog and extending PCR.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---

Changes in v2:
 no update

 lib/efi_loader/efi_tcg2.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 32e3818af4..564ac2255a 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -1321,23 +1321,11 @@ out:
  */
 static efi_status_t efi_append_scrtm_version(struct udevice *dev)
 {
-	struct tpml_digest_values digest_list;
 	u8 ver[] = U_BOOT_VERSION_STRING;
-	const int pcr_index = 0;
 	efi_status_t ret;
 
-	ret = tcg2_create_digest(ver, sizeof(ver), &digest_list);
-	if (ret != EFI_SUCCESS)
-		goto out;
+	ret = tcg2_measure_event(dev, 0, EV_S_CRTM_VERSION, sizeof(ver), ver);
 
-	ret = tcg2_pcr_extend(dev, pcr_index, &digest_list);
-	if (ret != EFI_SUCCESS)
-		goto out;
-
-	ret = tcg2_agile_log_append(pcr_index, EV_S_CRTM_VERSION, &digest_list,
-				    sizeof(ver), ver);
-
-out:
 	return ret;
 }
 
-- 
2.17.1

[PATCH v2 6/6] efi_loader: add comment for efi_tcg2.h

[Masahisa Kojima]

This commit adds the comment of the TCG Specification
efi_tcg2.h file refers, and comment for the structure.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---

Changes in v2:
- newly create commit from v2

 include/efi_tcg2.h | 51 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
index 8d7b77c087..25613caa19 100644
--- a/include/efi_tcg2.h
+++ b/include/efi_tcg2.h
@@ -3,6 +3,13 @@
  * Defines data structures and APIs that allow an OS to interact with UEFI
  * firmware to query information about the device
  *
+ * This file refers the following TCG specification.
+ *  - TCG PC Client Platform Firmware Profile Specification
+ *    https://trustedcomputinggroup.org/resource/pc-client-specific-platform-firmware-profile-specification/
+ *
+ *  - TCG EFI Protocol Specification
+ *    https://trustedcomputinggroup.org/resource/tcg-efi-protocol-specification/
+ *
  * Copyright (c) 2020, Linaro Limited
  */
 
@@ -36,11 +43,23 @@ typedef u32 efi_tcg_event_log_bitmap;
 typedef u32 efi_tcg_event_log_format;
 typedef u32 efi_tcg_event_algorithm_bitmap;
 
+/**
+ * struct tdEFI_TCG2_VERSION
+ * @major:	major version
+ * @minor:	minor version
+ */
 struct efi_tcg2_version {
 	u8 major;
 	u8 minor;
 };
 
+/**
+ * struct tdEFI_TCG2_EVENT_HEADER
+ * @header_size:	size of the event header
+ * @header_version:	header version
+ * @pcr_index:		index of the PCR that is extended
+ * @event_type:		type of the event that is extended
+ */
 struct efi_tcg2_event_header {
 	u32 header_size;
 	u16 header_version;
@@ -48,12 +67,27 @@ struct efi_tcg2_event_header {
 	u32 event_type;
 } __packed;
 
+/**
+ * struct tdEFI_TCG2_EVENT
+ * @size:	total size of the event including the size component, the header
+ *		and the event data
+ * @header:	event header
+ * @event:	event to add
+ */
 struct efi_tcg2_event {
 	u32 size;
 	struct efi_tcg2_event_header header;
 	u8 event[];
 } __packed;
 
+/**
+ * struct tdUEFI_IMAGE_LOAD_EVENT
+ * @image_location_in_memory:	image address
+ * @image_length_in_memory:	image size
+ * @image_link_time_address:	image link time address
+ * @length_of_device_path:	devive path size
+ * @device_path:		device path
+ */
 struct uefi_image_load_event {
 	efi_physical_addr_t image_location_in_memory;
 	u64 image_length_in_memory;
@@ -62,6 +96,23 @@ struct uefi_image_load_event {
 	struct efi_device_path device_path[];
 };
 
+/**
+ * struct tdEFI_TCG2_BOOT_SERVICE_CAPABILITY
+ * @size:			allocated size of the structure
+ * @structure_version:		version of this structure
+ * @protocol_version:		version of the EFI TCG2 protocol.
+ * @hash_algorithm_bitmap:	supported hash algorithms
+ * @supported_event_logs:	bitmap of supported event log formats
+ * @tpm_present_flag:		false = TPM not present
+ * @max_command_size:		max size (in bytes) of a command
+ *				that can be sent to the TPM
+ * @max_response_size:		max size (in bytes) of a response that
+ *				can be provided by the TPM
+ * @manufacturer_id:		4-byte Vendor ID
+ * @number_of_pcr_banks:	maximum number of PCR banks
+ * @active_pcr_banks:		bitmap of currently active
+ *				PCR banks (hashing algorithms).
+ */
 struct efi_tcg2_boot_service_capability {
 	u8 size;
 	struct efi_tcg2_version structure_version;
-- 
2.17.1

Re: [PATCH v2 1/6] efi_loader: increase eventlog buffer size

[Heinrich Schuchardt]

On 7/14/21 3:00 PM, Masahisa Kojima wrote:
> TCG PC Client PFP spec says "The Log Area Minimum Length
> for the TCG event log MUST be at least 64KB." in ACPI chapter.
> This commit increase the buffer size to 64KB.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

> ---
>
> Changes in v2:
> - increase buffer size to 64KB, it follows the minimum size
>    requirement stated in TCG spec
>
>   lib/efi_loader/Kconfig | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 156b391521..20edac6932 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -326,7 +326,7 @@ config EFI_TCG2_PROTOCOL
>   config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE
>   	int "EFI_TCG2_PROTOCOL EventLog size"
>   	depends on EFI_TCG2_PROTOCOL
> -	default 4096
> +	default 65536
>   	help
>   		Define the size of the EventLog for EFI_TCG2_PROTOCOL. Note that
>   		this is going to be allocated twice. One for the eventlog it self
>

Re: [PATCH v2 2/6] efi_loader: add secure boot variable measurement

[Heinrich Schuchardt]

On 7/14/21 3:00 PM, Masahisa Kojima wrote:
> TCG PC Client PFP spec requires to measure the secure
> boot policy before validating the UEFI image.
> This commit adds the secure boot variable measurement
> of "SecureBoot", "PK", "KEK", "db" and "dbx".
>
> Note that this implementation assumes that secure boot
> variables are pre-configured and not be set/updated in runtime.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
>
> Changes in v2:
> - missing null check for getting variable data
> - some minor fix for readability
>
>   include/efi_tcg2.h        |  20 ++++++
>   lib/efi_loader/efi_tcg2.c | 139 ++++++++++++++++++++++++++++++++++++++
>   2 files changed, 159 insertions(+)
>
> diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
> index bcfb98168a..8d7b77c087 100644
> --- a/include/efi_tcg2.h
> +++ b/include/efi_tcg2.h
> @@ -142,6 +142,26 @@ struct efi_tcg2_final_events_table {
>   	struct tcg_pcr_event2 event[];
>   };
>
> +/**
> + * struct tdUEFI_VARIABLE_DATA

Please, make this

struct struct_name - Brief description

Cf.
https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation

> + * @variable_name:		The vendorGUID parameter in the
> + *				GetVariable() API.
> + * @unicode_name_length:	The length in CHAR16 of the Unicode name of
> + *				the variable.
> + * @variable_data_length:	The size of the variable data.
> + * @unicode_name:		The CHAR16 unicode name of the variable
> + *				without NULL-terminator.
> + * @variable_data:		The data parameter of the efi variable
> + *				in the GetVariable() API.
> + */
> +struct efi_tcg2_uefi_variable_data {
> +	efi_guid_t variable_name;
> +	u64 unicode_name_length;
> +	u64 variable_data_length;
> +	u16 unicode_name[1];
> +	u8 variable_data[1];
> +};
> +
>   struct efi_tcg2_protocol {
>   	efi_status_t (EFIAPI * get_capability)(struct efi_tcg2_protocol *this,
>   					       struct efi_tcg2_boot_service_capability *capability);
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 1319a8b378..12db6f6b7c 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -78,6 +78,19 @@ static const struct digest_info hash_algo_list[] = {
>   	},
>   };
>
> +struct variable_info {
> +	u16		*name;
> +	const efi_guid_t	*guid;
> +};
> +
> +static struct variable_info secure_variables[] = {
> +	{L"SecureBoot", &efi_global_variable_guid},
> +	{L"PK", &efi_global_variable_guid},
> +	{L"KEK", &efi_global_variable_guid},
> +	{L"db", &efi_guid_image_security_database},
> +	{L"dbx", &efi_guid_image_security_database},
> +};
> +
>   #define MAX_HASH_COUNT ARRAY_SIZE(hash_algo_list)
>
>   /**
> @@ -1264,6 +1277,39 @@ free_pool:
>   	return ret;
>   }
>
> +/**
> + * tcg2_measure_event() - common function to add event log and extend PCR
> + *
> + * @dev:		TPM device
> + * @pcr_index:		PCR index
> + * @event_type:		type of event added
> + * @size:		event size
> + * @event:		event data
> + *
> + * Return:	status code
> + */
> +static efi_status_t EFIAPI
> +tcg2_measure_event(struct udevice *dev, u32 pcr_index, u32 event_type,
> +		   u32 size, u8 event[])
> +{
> +	struct tpml_digest_values digest_list;
> +	efi_status_t ret;
> +
> +	ret = tcg2_create_digest(event, size, &digest_list);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +	ret = tcg2_pcr_extend(dev, pcr_index, &digest_list);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +	ret = tcg2_agile_log_append(pcr_index, event_type, &digest_list,
> +				    size, event);
> +
> +out:
> +	return ret;
> +}
> +
>   /**
>    * efi_append_scrtm_version - Append an S-CRTM EV_S_CRTM_VERSION event on the
>    *			      eventlog and extend the PCRs
> @@ -1294,6 +1340,92 @@ out:
>   	return ret;
>   }
>
> +/**
> + * tcg2_measure_variable() - add variable event log and extend PCR
> + *
> + * @dev:		TPM device
> + * @pcr_index:		PCR index
> + * @event_type:		type of event added
> + * @var_name:		variable name
> + * @guid:		guid
> + * @data_size:		variable data size
> + * @data:		variable data
> + *
> + * Return:	status code
> + */
> +static efi_status_t tcg2_measure_variable(struct udevice *dev, u32 pcr_index,
> +					  u32 event_type, u16 *var_name,
> +					  const efi_guid_t *guid,
> +					  efi_uintn_t data_size, u8 *data)
> +{
> +	u32 event_size;
> +	efi_status_t ret;
> +	struct efi_tcg2_uefi_variable_data *event;
> +
> +	event_size = sizeof(event->variable_name) +
> +		     sizeof(event->unicode_name_length) +
> +		     sizeof(event->variable_data_length) +
> +		     (u16_strlen(var_name) * sizeof(u16)) + data_size;
> +	event = malloc(event_size);
> +	if (!event)
> +		return EFI_OUT_OF_RESOURCES;
> +
> +	guidcpy(&event->variable_name, guid);
> +	event->unicode_name_length = u16_strlen(var_name);
> +	event->variable_data_length = data_size;
> +	memcpy(event->unicode_name, var_name,
> +	       (event->unicode_name_length * sizeof(u16)));
> +	memcpy((u16 *)event->unicode_name + event->unicode_name_length,
> +	       data, data_size);
> +	ret = tcg2_measure_event(dev, pcr_index, event_type, event_size,
> +				 (u8 *)event);
> +	free(event);
> +	return ret;
> +}
> +
> +/**
> + * tcg2_measure_secure_boot_variable() - measure secure boot variables
> + *
> + * @dev:	TPM device
> + *
> + * Return:	status code
> + */
> +static efi_status_t tcg2_measure_secure_boot_variable(struct udevice *dev)
> +{
> +	u8 *data;
> +	efi_uintn_t data_size;
> +	u32 count, i;
> +	efi_status_t ret;
> +
> +	count = ARRAY_SIZE(secure_variables);
> +	for (i = 0; i < count; i++) {
> +		data = efi_get_var(secure_variables[i].name,
> +				   secure_variables[i].guid,
> +				   &data_size);
> +		if (data == NULL) {
> +			log_info("%ls not found\n", secure_variables[i].name);

log_debug() seems adequate here as this function will be called even if
the board is not yet provisioned.

> +			continue;
> +		}
> +
> +		ret = tcg2_measure_variable(dev, 7,
> +					    EV_EFI_VARIABLE_DRIVER_CONFIG,
> +					    secure_variables[i].name,
> +					    secure_variables[i].guid,
> +					    data_size, (u8 *)data);
> +		free(data);
> +		if (ret != EFI_SUCCESS)
> +			goto error;
> +	}
> +
> +	/*
> +	 * TODO: add DBT and DBR measurement support when u-boot supports

"dbt" and "dbr" are lower case.

> +	 * these variables.
> +	 */

Adding them to secure_variables[] now would not harm.

Best regards

Heinrich

> +
> +error:
> +	return ret;
> +}
> +
>   /**
>    * efi_tcg2_register() - register EFI_TCG2_PROTOCOL
>    *
> @@ -1328,6 +1460,13 @@ efi_status_t efi_tcg2_register(void)
>   		tcg2_uninit();
>   		goto fail;
>   	}
> +
> +	ret = tcg2_measure_secure_boot_variable(dev);
> +	if (ret != EFI_SUCCESS) {
> +		tcg2_uninit();
> +		goto fail;
> +	}
> +
>   	return ret;
>
>   fail:
>

Re: [PATCH v2 3/6] efi_loader: add boot variable measurement

[Heinrich Schuchardt]

On 7/14/21 3:00 PM, Masahisa Kojima wrote:
> TCG PC Client PFP spec requires to measure "Boot####"
> and "BootOrder" variables, EV_SEPARATOR event prior
> to the Ready to Boot invocation.
> Since u-boot does not implement Ready to Boot event,
> these measurements are performed when efi_start_image() is called.
>
> TCG spec also requires to measure "Calling EFI Application from
> Boot Option" for each boot attempt, and "Returning from EFI
> Application from Boot Option" if a boot device returns control
> back to the Boot Manager.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
>
> Changes in v2:
> - use efi_create_indexed_name() for "Boot####" variable
>
>   include/efi_loader.h          |   4 ++
>   include/tpm-v2.h              |  18 ++++-
>   lib/efi_loader/efi_boottime.c |  20 ++++++
>   lib/efi_loader/efi_tcg2.c     | 121 ++++++++++++++++++++++++++++++++++
>   4 files changed, 162 insertions(+), 1 deletion(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index b81180cfda..703b675950 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -407,6 +407,10 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size);
>   efi_status_t efi_init_variables(void);
>   /* Notify ExitBootServices() is called */
>   void efi_variables_boot_exit_notify(void);
> +/* Measure efi application invocation */
> +efi_status_t EFIAPI efi_tcg2_measure_efi_app_invocation(void);
> +/* Measure efi application exit */
> +efi_status_t EFIAPI efi_tcg2_measure_efi_app_exit(void);
>   /* Called by bootefi to initialize root node */
>   efi_status_t efi_root_node_register(void);
>   /* Called by bootefi to initialize runtime */
> diff --git a/include/tpm-v2.h b/include/tpm-v2.h
> index 247b386967..325c73006e 100644
> --- a/include/tpm-v2.h
> +++ b/include/tpm-v2.h
> @@ -73,7 +73,7 @@ struct udevice;
>   /*
>    * event types, cf.
>    * "TCG PC Client Platform Firmware Profile Specification", Family "2.0"
> - * rev 1.04, June 3, 2019
> + * Level 00 Version 1.05 Revision 23, May 7, 2021
>    */
>   #define EV_EFI_EVENT_BASE			((u32)0x80000000)
>   #define EV_EFI_VARIABLE_DRIVER_CONFIG		((u32)0x80000001)
> @@ -85,8 +85,24 @@ struct udevice;
>   #define EV_EFI_ACTION				((u32)0x80000007)
>   #define EV_EFI_PLATFORM_FIRMWARE_BLOB		((u32)0x80000008)
>   #define EV_EFI_HANDOFF_TABLES			((u32)0x80000009)
> +#define EV_EFI_PLATFORM_FIRMWARE_BLOB2		((u32)0x8000000A)
> +#define EV_EFI_HANDOFF_TABLES2			((u32)0x8000000B)
> +#define EV_EFI_VARIABLE_BOOT2			((u32)0x8000000C)
>   #define EV_EFI_HCRTM_EVENT			((u32)0x80000010)
>   #define EV_EFI_VARIABLE_AUTHORITY		((u32)0x800000E0)
> +#define EV_EFI_SPDM_FIRMWARE_BLOB		((u32)0x800000E1)
> +#define EV_EFI_SPDM_FIRMWARE_CONFIG		((u32)0x800000E2)
> +
> +#define EFI_CALLING_EFI_APPLICATION         \
> +	"Calling EFI Application from Boot Option"
> +#define EFI_RETURNING_FROM_EFI_APPLICATION  \
> +	"Returning from EFI Application from Boot Option"
> +#define EFI_EXIT_BOOT_SERVICES_INVOCATION   \
> +	"Exit Boot Services Invocation"
> +#define EFI_EXIT_BOOT_SERVICES_FAILED       \
> +	"Exit Boot Services Returned with Failure"
> +#define EFI_EXIT_BOOT_SERVICES_SUCCEEDED    \
> +	"Exit Boot Services Returned with Success"
>
>   /* TPMS_TAGGED_PROPERTY Structure */
>   struct tpms_tagged_property {
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index f6d5ba05e3..2914800c56 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -2993,6 +2993,16 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,
>   	image_obj->exit_status = &exit_status;
>   	image_obj->exit_jmp = &exit_jmp;
>
> +	if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
> +		if (image_obj->image_type == IMAGE_SUBSYSTEM_EFI_APPLICATION) {
> +			ret = efi_tcg2_measure_efi_app_invocation();
> +			if (ret != EFI_SUCCESS) {
> +				EFI_PRINT("tcg2 measurement fails(0x%lx)\n",
> +					  ret);

This will only show when enabling debug messages.
Should this be log_warning()?

> +			}
> +		}
> +	}
> +
>   	/* call the image! */
>   	if (setjmp(&exit_jmp)) {
>   		/*
> @@ -3251,6 +3261,16 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle,
>   	    exit_status != EFI_SUCCESS)
>   		efi_delete_image(image_obj, loaded_image_protocol);
>
> +	if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
> +		if (image_obj->image_type == IMAGE_SUBSYSTEM_EFI_APPLICATION) {
> +			ret = efi_tcg2_measure_efi_app_exit();
> +			if (ret != EFI_SUCCESS) {
> +				EFI_PRINT("tcg2 measurement fails(0x%lx)\n",
> +					  ret);

ditto

> +			}
> +		}
> +	}
> +
>   	/* Make sure entry/exit counts for EFI world cross-overs match */
>   	EFI_EXIT(exit_status);
>
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 12db6f6b7c..d59fc5a890 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -35,6 +35,7 @@ struct event_log_buffer {
>   };
>
>   static struct event_log_buffer event_log;
> +static bool tcg2_efi_app_invoked;
>   /*
>    * When requesting TPM2_CAP_TPM_PROPERTIES the value is on a standard offset.
>    * Since the current tpm2_get_capability() response buffers starts at
> @@ -1383,6 +1384,126 @@ static efi_status_t tcg2_measure_variable(struct udevice *dev, u32 pcr_index,
>   	return ret;
>   }
>
> +/**
> + * tcg2_measure_boot_variable() - measure boot variables
> + *
> + * @dev:	TPM device
> + *
> + * Return:	status code
> + */
> +static efi_status_t tcg2_measure_boot_variable(struct udevice *dev)
> +{
> +	u16 *boot_order;
> +	u16 *boot_index;
> +	u16 var_name[] = L"BootOrder";
> +	u16 boot_name[] = L"Boot####";
> +	u8 *bootvar;
> +	efi_uintn_t var_data_size;
> +	u32 count, i;
> +	efi_status_t ret;
> +
> +	boot_order = efi_get_var(var_name, &efi_global_variable_guid,
> +				 &var_data_size);
> +	if (!boot_order) {
> +		log_info("BootOrder not defined\n");

The user should know it anyway. Nothing noteworthy in the context of
measurement.

> +		ret = EFI_NOT_FOUND;
> +		goto error;
> +	}
> +
> +	ret = tcg2_measure_variable(dev, 1, EV_EFI_VARIABLE_BOOT2, var_name,
> +				    &efi_global_variable_guid, var_data_size,
> +				    (u8 *)boot_order);
> +	if (ret != EFI_SUCCESS)
> +		goto error;
> +
> +	count = var_data_size / sizeof(*boot_order);
> +	boot_index = boot_order;
> +	for (i = 0; i < count; i++) {
> +		efi_create_indexed_name(boot_name, sizeof(boot_name), "Boot", *boot_index++);
> +
> +		bootvar = efi_get_var(boot_name, &efi_global_variable_guid,
> +				      &var_data_size);

Does measurement of Boot#### and BootOrder imply for measured boot that
a board will not boot into Windows if I change the boot option for Debian?

Best regards

Heinrich

> +
> +		if (!bootvar) {
> +			log_info("%ls not found\n", boot_name);
> +			continue;
> +		}
> +
> +		ret = tcg2_measure_variable(dev, 1, EV_EFI_VARIABLE_BOOT2,
> +					    boot_name,
> +					    &efi_global_variable_guid,
> +					    var_data_size, bootvar);
> +		free(bootvar);
> +		if (ret != EFI_SUCCESS)
> +			goto error;
> +	}
> +
> +error:
> +	free(boot_order);
> +	return ret;
> +}
> +
> +/**
> + * efi_tcg2_measure_efi_app_invocation() - measure efi app invocation
> + *
> + * Return:	status code
> + */
> +efi_status_t EFIAPI efi_tcg2_measure_efi_app_invocation(void)
> +{
> +	efi_status_t ret;
> +	u32 pcr_index;
> +	struct udevice *dev;
> +	u32 event = 0;
> +
> +	if (tcg2_efi_app_invoked)
> +		return EFI_SUCCESS;
> +
> +	ret = platform_get_tpm2_device(&dev);
> +	if (ret != EFI_SUCCESS)
> +		return ret;
> +
> +	ret = tcg2_measure_boot_variable(dev);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +	ret = tcg2_measure_event(dev, 4, EV_EFI_ACTION,
> +				 strlen(EFI_CALLING_EFI_APPLICATION),
> +				 (u8 *)EFI_CALLING_EFI_APPLICATION);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +	for (pcr_index = 0; pcr_index <= 7; pcr_index++) {
> +		ret = tcg2_measure_event(dev, pcr_index, EV_SEPARATOR,
> +					 sizeof(event), (u8 *)&event);
> +		if (ret != EFI_SUCCESS)
> +			goto out;
> +	}
> +
> +	tcg2_efi_app_invoked = true;
> +out:
> +	return ret;
> +}
> +
> +/**
> + * efi_tcg2_measure_efi_app_exit() - measure efi app exit
> + *
> + * Return:	status code
> + */
> +efi_status_t EFIAPI efi_tcg2_measure_efi_app_exit(void)
> +{
> +	efi_status_t ret;
> +	struct udevice *dev;
> +
> +	ret = platform_get_tpm2_device(&dev);
> +	if (ret != EFI_SUCCESS)
> +		return ret;
> +
> +	ret = tcg2_measure_event(dev, 4, EV_EFI_ACTION,
> +				 strlen(EFI_RETURNING_FROM_EFI_APPLICATION),
> +				 (u8 *)EFI_RETURNING_FROM_EFI_APPLICATION);
> +	return ret;
> +}
> +
>   /**
>    * tcg2_measure_secure_boot_variable() - measure secure boot variables
>    *
>

Re: [PATCH v2 3/6] efi_loader: add boot variable measurement

[Masahisa Kojima]

On Thu, 15 Jul 2021 at 15:59, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
>
>
> On 7/14/21 3:00 PM, Masahisa Kojima wrote:
> > TCG PC Client PFP spec requires to measure "Boot####"
> > and "BootOrder" variables, EV_SEPARATOR event prior
> > to the Ready to Boot invocation.
> > Since u-boot does not implement Ready to Boot event,
> > these measurements are performed when efi_start_image() is called.
> >
> > TCG spec also requires to measure "Calling EFI Application from
> > Boot Option" for each boot attempt, and "Returning from EFI
> > Application from Boot Option" if a boot device returns control
> > back to the Boot Manager.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > ---
> >
> > Changes in v2:
> > - use efi_create_indexed_name() for "Boot####" variable
> >
> >   include/efi_loader.h          |   4 ++
> >   include/tpm-v2.h              |  18 ++++-
> >   lib/efi_loader/efi_boottime.c |  20 ++++++
> >   lib/efi_loader/efi_tcg2.c     | 121 ++++++++++++++++++++++++++++++++++
> >   4 files changed, 162 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index b81180cfda..703b675950 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -407,6 +407,10 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size);
> >   efi_status_t efi_init_variables(void);
> >   /* Notify ExitBootServices() is called */
> >   void efi_variables_boot_exit_notify(void);
> > +/* Measure efi application invocation */
> > +efi_status_t EFIAPI efi_tcg2_measure_efi_app_invocation(void);
> > +/* Measure efi application exit */
> > +efi_status_t EFIAPI efi_tcg2_measure_efi_app_exit(void);
> >   /* Called by bootefi to initialize root node */
> >   efi_status_t efi_root_node_register(void);
> >   /* Called by bootefi to initialize runtime */
> > diff --git a/include/tpm-v2.h b/include/tpm-v2.h
> > index 247b386967..325c73006e 100644
> > --- a/include/tpm-v2.h
> > +++ b/include/tpm-v2.h
> > @@ -73,7 +73,7 @@ struct udevice;
> >   /*
> >    * event types, cf.
> >    * "TCG PC Client Platform Firmware Profile Specification", Family "2.0"
> > - * rev 1.04, June 3, 2019
> > + * Level 00 Version 1.05 Revision 23, May 7, 2021
> >    */
> >   #define EV_EFI_EVENT_BASE                   ((u32)0x80000000)
> >   #define EV_EFI_VARIABLE_DRIVER_CONFIG               ((u32)0x80000001)
> > @@ -85,8 +85,24 @@ struct udevice;
> >   #define EV_EFI_ACTION                               ((u32)0x80000007)
> >   #define EV_EFI_PLATFORM_FIRMWARE_BLOB               ((u32)0x80000008)
> >   #define EV_EFI_HANDOFF_TABLES                       ((u32)0x80000009)
> > +#define EV_EFI_PLATFORM_FIRMWARE_BLOB2               ((u32)0x8000000A)
> > +#define EV_EFI_HANDOFF_TABLES2                       ((u32)0x8000000B)
> > +#define EV_EFI_VARIABLE_BOOT2                        ((u32)0x8000000C)
> >   #define EV_EFI_HCRTM_EVENT                  ((u32)0x80000010)
> >   #define EV_EFI_VARIABLE_AUTHORITY           ((u32)0x800000E0)
> > +#define EV_EFI_SPDM_FIRMWARE_BLOB            ((u32)0x800000E1)
> > +#define EV_EFI_SPDM_FIRMWARE_CONFIG          ((u32)0x800000E2)
> > +
> > +#define EFI_CALLING_EFI_APPLICATION         \
> > +     "Calling EFI Application from Boot Option"
> > +#define EFI_RETURNING_FROM_EFI_APPLICATION  \
> > +     "Returning from EFI Application from Boot Option"
> > +#define EFI_EXIT_BOOT_SERVICES_INVOCATION   \
> > +     "Exit Boot Services Invocation"
> > +#define EFI_EXIT_BOOT_SERVICES_FAILED       \
> > +     "Exit Boot Services Returned with Failure"
> > +#define EFI_EXIT_BOOT_SERVICES_SUCCEEDED    \
> > +     "Exit Boot Services Returned with Success"
> >
> >   /* TPMS_TAGGED_PROPERTY Structure */
> >   struct tpms_tagged_property {
> > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> > index f6d5ba05e3..2914800c56 100644
> > --- a/lib/efi_loader/efi_boottime.c
> > +++ b/lib/efi_loader/efi_boottime.c
> > @@ -2993,6 +2993,16 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,
> >       image_obj->exit_status = &exit_status;
> >       image_obj->exit_jmp = &exit_jmp;
> >
> > +     if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
> > +             if (image_obj->image_type == IMAGE_SUBSYSTEM_EFI_APPLICATION) {
> > +                     ret = efi_tcg2_measure_efi_app_invocation();
> > +                     if (ret != EFI_SUCCESS) {
> > +                             EFI_PRINT("tcg2 measurement fails(0x%lx)\n",
> > +                                       ret);
>
> This will only show when enabling debug messages.
> Should this be log_warning()?
>
> > +                     }
> > +             }
> > +     }
> > +
> >       /* call the image! */
> >       if (setjmp(&exit_jmp)) {
> >               /*
> > @@ -3251,6 +3261,16 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle,
> >           exit_status != EFI_SUCCESS)
> >               efi_delete_image(image_obj, loaded_image_protocol);
> >
> > +     if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
> > +             if (image_obj->image_type == IMAGE_SUBSYSTEM_EFI_APPLICATION) {
> > +                     ret = efi_tcg2_measure_efi_app_exit();
> > +                     if (ret != EFI_SUCCESS) {
> > +                             EFI_PRINT("tcg2 measurement fails(0x%lx)\n",
> > +                                       ret);
>
> ditto
>
> > +                     }
> > +             }
> > +     }
> > +
> >       /* Make sure entry/exit counts for EFI world cross-overs match */
> >       EFI_EXIT(exit_status);
> >
> > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> > index 12db6f6b7c..d59fc5a890 100644
> > --- a/lib/efi_loader/efi_tcg2.c
> > +++ b/lib/efi_loader/efi_tcg2.c
> > @@ -35,6 +35,7 @@ struct event_log_buffer {
> >   };
> >
> >   static struct event_log_buffer event_log;
> > +static bool tcg2_efi_app_invoked;
> >   /*
> >    * When requesting TPM2_CAP_TPM_PROPERTIES the value is on a standard offset.
> >    * Since the current tpm2_get_capability() response buffers starts at
> > @@ -1383,6 +1384,126 @@ static efi_status_t tcg2_measure_variable(struct udevice *dev, u32 pcr_index,
> >       return ret;
> >   }
> >
> > +/**
> > + * tcg2_measure_boot_variable() - measure boot variables
> > + *
> > + * @dev:     TPM device
> > + *
> > + * Return:   status code
> > + */
> > +static efi_status_t tcg2_measure_boot_variable(struct udevice *dev)
> > +{
> > +     u16 *boot_order;
> > +     u16 *boot_index;
> > +     u16 var_name[] = L"BootOrder";
> > +     u16 boot_name[] = L"Boot####";
> > +     u8 *bootvar;
> > +     efi_uintn_t var_data_size;
> > +     u32 count, i;
> > +     efi_status_t ret;
> > +
> > +     boot_order = efi_get_var(var_name, &efi_global_variable_guid,
> > +                              &var_data_size);
> > +     if (!boot_order) {
> > +             log_info("BootOrder not defined\n");
>
> The user should know it anyway. Nothing noteworthy in the context of
> measurement.
>
> > +             ret = EFI_NOT_FOUND;
> > +             goto error;
> > +     }
> > +
> > +     ret = tcg2_measure_variable(dev, 1, EV_EFI_VARIABLE_BOOT2, var_name,
> > +                                 &efi_global_variable_guid, var_data_size,
> > +                                 (u8 *)boot_order);
> > +     if (ret != EFI_SUCCESS)
> > +             goto error;
> > +
> > +     count = var_data_size / sizeof(*boot_order);
> > +     boot_index = boot_order;
> > +     for (i = 0; i < count; i++) {
> > +             efi_create_indexed_name(boot_name, sizeof(boot_name), "Boot", *boot_index++);
> > +
> > +             bootvar = efi_get_var(boot_name, &efi_global_variable_guid,
> > +                                   &var_data_size);
>
> Does measurement of Boot#### and BootOrder imply for measured boot that
> a board will not boot into Windows if I change the boot option for Debian?

Basically, measured boot does not affect system will boot or not.
Measured boot records the boot configuration and boot sequence by extending
eventlog and PCRs.
After boot , Attestation process will decide system behavior depending
on the PCRs.

Thanks,
Masahisa Kojima

>
> Best regards
>
> Heinrich
>
> > +
> > +             if (!bootvar) {
> > +                     log_info("%ls not found\n", boot_name);
> > +                     continue;
> > +             }
> > +
> > +             ret = tcg2_measure_variable(dev, 1, EV_EFI_VARIABLE_BOOT2,
> > +                                         boot_name,
> > +                                         &efi_global_variable_guid,
> > +                                         var_data_size, bootvar);
> > +             free(bootvar);
> > +             if (ret != EFI_SUCCESS)
> > +                     goto error;
> > +     }
> > +
> > +error:
> > +     free(boot_order);
> > +     return ret;
> > +}
> > +
> > +/**
> > + * efi_tcg2_measure_efi_app_invocation() - measure efi app invocation
> > + *
> > + * Return:   status code
> > + */
> > +efi_status_t EFIAPI efi_tcg2_measure_efi_app_invocation(void)
> > +{
> > +     efi_status_t ret;
> > +     u32 pcr_index;
> > +     struct udevice *dev;
> > +     u32 event = 0;
> > +
> > +     if (tcg2_efi_app_invoked)
> > +             return EFI_SUCCESS;
> > +
> > +     ret = platform_get_tpm2_device(&dev);
> > +     if (ret != EFI_SUCCESS)
> > +             return ret;
> > +
> > +     ret = tcg2_measure_boot_variable(dev);
> > +     if (ret != EFI_SUCCESS)
> > +             goto out;
> > +
> > +     ret = tcg2_measure_event(dev, 4, EV_EFI_ACTION,
> > +                              strlen(EFI_CALLING_EFI_APPLICATION),
> > +                              (u8 *)EFI_CALLING_EFI_APPLICATION);
> > +     if (ret != EFI_SUCCESS)
> > +             goto out;
> > +
> > +     for (pcr_index = 0; pcr_index <= 7; pcr_index++) {
> > +             ret = tcg2_measure_event(dev, pcr_index, EV_SEPARATOR,
> > +                                      sizeof(event), (u8 *)&event);
> > +             if (ret != EFI_SUCCESS)
> > +                     goto out;
> > +     }
> > +
> > +     tcg2_efi_app_invoked = true;
> > +out:
> > +     return ret;
> > +}
> > +
> > +/**
> > + * efi_tcg2_measure_efi_app_exit() - measure efi app exit
> > + *
> > + * Return:   status code
> > + */
> > +efi_status_t EFIAPI efi_tcg2_measure_efi_app_exit(void)
> > +{
> > +     efi_status_t ret;
> > +     struct udevice *dev;
> > +
> > +     ret = platform_get_tpm2_device(&dev);
> > +     if (ret != EFI_SUCCESS)
> > +             return ret;
> > +
> > +     ret = tcg2_measure_event(dev, 4, EV_EFI_ACTION,
> > +                              strlen(EFI_RETURNING_FROM_EFI_APPLICATION),
> > +                              (u8 *)EFI_RETURNING_FROM_EFI_APPLICATION);
> > +     return ret;
> > +}
> > +
> >   /**
> >    * tcg2_measure_secure_boot_variable() - measure secure boot variables
> >    *
> >

Re: [PATCH v2 2/6] efi_loader: add secure boot variable measurement

[Simon Glass]

Hi,

On Wed, 14 Jul 2021 at 06:59, Masahisa Kojima
<masahisa.kojima@linaro.org> wrote:
>
> TCG PC Client PFP spec requires to measure the secure
> boot policy before validating the UEFI image.
> This commit adds the secure boot variable measurement
> of "SecureBoot", "PK", "KEK", "db" and "dbx".
>
> Note that this implementation assumes that secure boot
> variables are pre-configured and not be set/updated in runtime.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
>
> Changes in v2:
> - missing null check for getting variable data
> - some minor fix for readability
>
>  include/efi_tcg2.h        |  20 ++++++
>  lib/efi_loader/efi_tcg2.c | 139 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 159 insertions(+)

It looks like this code should be in lib/tpm or similar as much of it
is not specific to EFI?

Regards,
Simon

Re: [PATCH v2 2/6] efi_loader: add secure boot variable measurement

[Masahisa Kojima]

Hi Simon,

On Wed, 21 Jul 2021 at 03:34, Simon Glass <sjg@chromium.org> wrote:
>
> Hi,
>
> On Wed, 14 Jul 2021 at 06:59, Masahisa Kojima
> <masahisa.kojima@linaro.org> wrote:
> >
> > TCG PC Client PFP spec requires to measure the secure
> > boot policy before validating the UEFI image.
> > This commit adds the secure boot variable measurement
> > of "SecureBoot", "PK", "KEK", "db" and "dbx".
> >
> > Note that this implementation assumes that secure boot
> > variables are pre-configured and not be set/updated in runtime.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > ---
> >
> > Changes in v2:
> > - missing null check for getting variable data
> > - some minor fix for readability
> >
> >  include/efi_tcg2.h        |  20 ++++++
> >  lib/efi_loader/efi_tcg2.c | 139 ++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 159 insertions(+)
>
> It looks like this code should be in lib/tpm or similar as much of it
> is not specific to EFI?

Yes, it is not directly related to EFI, but I think very small amount
of code will
be moved to lib/tpm or similar place.
lib/efi_loader/efi_tcg2.c currently implement two specs,
TCG EFI Protocol spec and TCG PC Client PFP spec.
There are many duplication in these specs, I think it is difficult to split
lib/efi_loader/efi_tcg2.c file into separate file.
In addition, efi tcg2 eventlog is currently created and initialized
during the efi init.

The major purpose of my patch series is extending measurement support,
I would like to implement these measurement in efi_tcg2.c for now.

In near future, u-boot must consider to support eventlog handoff from former
firmware such as trusted firmware, so current eventlog buffer preparation
in efi init will be modified. Then I would like to discuss implementation of
lib/efi_loader/efi_tcg2.c at that time.

Thanks,
Masahisa Kojima

>
> Regards,
> Simon

Re: [PATCH v2 2/6] efi_loader: add secure boot variable measurement

[Simon Glass]

Hi Masahisa,

On Tue, 20 Jul 2021 at 19:51, Masahisa Kojima
<masahisa.kojima@linaro.org> wrote:
>
> Hi Simon,
>
> On Wed, 21 Jul 2021 at 03:34, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi,
> >
> > On Wed, 14 Jul 2021 at 06:59, Masahisa Kojima
> > <masahisa.kojima@linaro.org> wrote:
> > >
> > > TCG PC Client PFP spec requires to measure the secure
> > > boot policy before validating the UEFI image.
> > > This commit adds the secure boot variable measurement
> > > of "SecureBoot", "PK", "KEK", "db" and "dbx".
> > >
> > > Note that this implementation assumes that secure boot
> > > variables are pre-configured and not be set/updated in runtime.
> > >
> > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > > ---
> > >
> > > Changes in v2:
> > > - missing null check for getting variable data
> > > - some minor fix for readability
> > >
> > >  include/efi_tcg2.h        |  20 ++++++
> > >  lib/efi_loader/efi_tcg2.c | 139 ++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 159 insertions(+)
> >
> > It looks like this code should be in lib/tpm or similar as much of it
> > is not specific to EFI?
>
> Yes, it is not directly related to EFI, but I think very small amount
> of code will
> be moved to lib/tpm or similar place.
> lib/efi_loader/efi_tcg2.c currently implement two specs,
> TCG EFI Protocol spec and TCG PC Client PFP spec.
> There are many duplication in these specs, I think it is difficult to split
> lib/efi_loader/efi_tcg2.c file into separate file.
> In addition, efi tcg2 eventlog is currently created and initialized
> during the efi init.
>
> The major purpose of my patch series is extending measurement support,
> I would like to implement these measurement in efi_tcg2.c for now.
>
> In near future, u-boot must consider to support eventlog handoff from former
> firmware such as trusted firmware, so current eventlog buffer preparation
> in efi init will be modified. Then I would like to discuss implementation of
> lib/efi_loader/efi_tcg2.c at that time.

OK thank you. Let's see how it goes.

Regards,
Simon

Re: [PATCH v2 2/6] efi_loader: add secure boot variable measurement

[Masahisa Kojima]

On Thu, 15 Jul 2021 at 15:44, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
>
>
> On 7/14/21 3:00 PM, Masahisa Kojima wrote:
> > TCG PC Client PFP spec requires to measure the secure
> > boot policy before validating the UEFI image.
> > This commit adds the secure boot variable measurement
> > of "SecureBoot", "PK", "KEK", "db" and "dbx".
> >
> > Note that this implementation assumes that secure boot
> > variables are pre-configured and not be set/updated in runtime.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > ---
> >
> > Changes in v2:
> > - missing null check for getting variable data
> > - some minor fix for readability
> >
> >   include/efi_tcg2.h        |  20 ++++++
> >   lib/efi_loader/efi_tcg2.c | 139 ++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 159 insertions(+)
> >
> > diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
> > index bcfb98168a..8d7b77c087 100644
> > --- a/include/efi_tcg2.h
> > +++ b/include/efi_tcg2.h
> > @@ -142,6 +142,26 @@ struct efi_tcg2_final_events_table {
> >       struct tcg_pcr_event2 event[];
> >   };
> >
> > +/**
> > + * struct tdUEFI_VARIABLE_DATA
>
> Please, make this
>
> struct struct_name - Brief description
>
> Cf.
> https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation
>
> > + * @variable_name:           The vendorGUID parameter in the
> > + *                           GetVariable() API.
> > + * @unicode_name_length:     The length in CHAR16 of the Unicode name of
> > + *                           the variable.
> > + * @variable_data_length:    The size of the variable data.
> > + * @unicode_name:            The CHAR16 unicode name of the variable
> > + *                           without NULL-terminator.
> > + * @variable_data:           The data parameter of the efi variable
> > + *                           in the GetVariable() API.
> > + */
> > +struct efi_tcg2_uefi_variable_data {
> > +     efi_guid_t variable_name;
> > +     u64 unicode_name_length;
> > +     u64 variable_data_length;
> > +     u16 unicode_name[1];
> > +     u8 variable_data[1];
> > +};
> > +
> >   struct efi_tcg2_protocol {
> >       efi_status_t (EFIAPI * get_capability)(struct efi_tcg2_protocol *this,
> >                                              struct efi_tcg2_boot_service_capability *capability);
> > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> > index 1319a8b378..12db6f6b7c 100644
> > --- a/lib/efi_loader/efi_tcg2.c
> > +++ b/lib/efi_loader/efi_tcg2.c
> > @@ -78,6 +78,19 @@ static const struct digest_info hash_algo_list[] = {
> >       },
> >   };
> >
> > +struct variable_info {
> > +     u16             *name;
> > +     const efi_guid_t        *guid;
> > +};
> > +
> > +static struct variable_info secure_variables[] = {
> > +     {L"SecureBoot", &efi_global_variable_guid},
> > +     {L"PK", &efi_global_variable_guid},
> > +     {L"KEK", &efi_global_variable_guid},
> > +     {L"db", &efi_guid_image_security_database},
> > +     {L"dbx", &efi_guid_image_security_database},
> > +};
> > +
> >   #define MAX_HASH_COUNT ARRAY_SIZE(hash_algo_list)
> >
> >   /**
> > @@ -1264,6 +1277,39 @@ free_pool:
> >       return ret;
> >   }
> >
> > +/**
> > + * tcg2_measure_event() - common function to add event log and extend PCR
> > + *
> > + * @dev:             TPM device
> > + * @pcr_index:               PCR index
> > + * @event_type:              type of event added
> > + * @size:            event size
> > + * @event:           event data
> > + *
> > + * Return:   status code
> > + */
> > +static efi_status_t EFIAPI
> > +tcg2_measure_event(struct udevice *dev, u32 pcr_index, u32 event_type,
> > +                u32 size, u8 event[])
> > +{
> > +     struct tpml_digest_values digest_list;
> > +     efi_status_t ret;
> > +
> > +     ret = tcg2_create_digest(event, size, &digest_list);
> > +     if (ret != EFI_SUCCESS)
> > +             goto out;
> > +
> > +     ret = tcg2_pcr_extend(dev, pcr_index, &digest_list);
> > +     if (ret != EFI_SUCCESS)
> > +             goto out;
> > +
> > +     ret = tcg2_agile_log_append(pcr_index, event_type, &digest_list,
> > +                                 size, event);
> > +
> > +out:
> > +     return ret;
> > +}
> > +
> >   /**
> >    * efi_append_scrtm_version - Append an S-CRTM EV_S_CRTM_VERSION event on the
> >    *                        eventlog and extend the PCRs
> > @@ -1294,6 +1340,92 @@ out:
> >       return ret;
> >   }
> >
> > +/**
> > + * tcg2_measure_variable() - add variable event log and extend PCR
> > + *
> > + * @dev:             TPM device
> > + * @pcr_index:               PCR index
> > + * @event_type:              type of event added
> > + * @var_name:                variable name
> > + * @guid:            guid
> > + * @data_size:               variable data size
> > + * @data:            variable data
> > + *
> > + * Return:   status code
> > + */
> > +static efi_status_t tcg2_measure_variable(struct udevice *dev, u32 pcr_index,
> > +                                       u32 event_type, u16 *var_name,
> > +                                       const efi_guid_t *guid,
> > +                                       efi_uintn_t data_size, u8 *data)
> > +{
> > +     u32 event_size;
> > +     efi_status_t ret;
> > +     struct efi_tcg2_uefi_variable_data *event;
> > +
> > +     event_size = sizeof(event->variable_name) +
> > +                  sizeof(event->unicode_name_length) +
> > +                  sizeof(event->variable_data_length) +
> > +                  (u16_strlen(var_name) * sizeof(u16)) + data_size;
> > +     event = malloc(event_size);
> > +     if (!event)
> > +             return EFI_OUT_OF_RESOURCES;
> > +
> > +     guidcpy(&event->variable_name, guid);
> > +     event->unicode_name_length = u16_strlen(var_name);
> > +     event->variable_data_length = data_size;
> > +     memcpy(event->unicode_name, var_name,
> > +            (event->unicode_name_length * sizeof(u16)));
> > +     memcpy((u16 *)event->unicode_name + event->unicode_name_length,
> > +            data, data_size);
> > +     ret = tcg2_measure_event(dev, pcr_index, event_type, event_size,
> > +                              (u8 *)event);
> > +     free(event);
> > +     return ret;
> > +}
> > +
> > +/**
> > + * tcg2_measure_secure_boot_variable() - measure secure boot variables
> > + *
> > + * @dev:     TPM device
> > + *
> > + * Return:   status code
> > + */
> > +static efi_status_t tcg2_measure_secure_boot_variable(struct udevice *dev)
> > +{
> > +     u8 *data;
> > +     efi_uintn_t data_size;
> > +     u32 count, i;
> > +     efi_status_t ret;
> > +
> > +     count = ARRAY_SIZE(secure_variables);
> > +     for (i = 0; i < count; i++) {
> > +             data = efi_get_var(secure_variables[i].name,
> > +                                secure_variables[i].guid,
> > +                                &data_size);
> > +             if (data == NULL) {
> > +                     log_info("%ls not found\n", secure_variables[i].name);
>
> log_debug() seems adequate here as this function will be called even if
> the board is not yet provisioned.

Sorry but my v1 patch was correct, NULL checking for data is not required.
According to the TCG2 Client PFP spec, "PK", "KEK", "db" and "dbx" variables
must be measured even if these variables are empty.
So I will remove this NULL checking in v3 patch.

>
> > +                     continue;
> > +             }
> > +
> > +             ret = tcg2_measure_variable(dev, 7,
> > +                                         EV_EFI_VARIABLE_DRIVER_CONFIG,
> > +                                         secure_variables[i].name,
> > +                                         secure_variables[i].guid,
> > +                                         data_size, (u8 *)data);
> > +             free(data);
> > +             if (ret != EFI_SUCCESS)
> > +                     goto error;
> > +     }
> > +
> > +     /*
> > +      * TODO: add DBT and DBR measurement support when u-boot supports
>
> "dbt" and "dbr" are lower case.
>
> > +      * these variables.
> > +      */
>
> Adding them to secure_variables[] now would not harm.

Yes, I will add "dbt" and "dbr".
"dbt" and "dbr" must be measured if present and not empty according to TCG spec,
unlike "PK", "KEK", "db" and "dbx"(it must be measured even if they are empty).

Thanks,
Masahisa Kojima

>
> Best regards
>
> Heinrich
>
> > +
> > +error:
> > +     return ret;
> > +}
> > +
> >   /**
> >    * efi_tcg2_register() - register EFI_TCG2_PROTOCOL
> >    *
> > @@ -1328,6 +1460,13 @@ efi_status_t efi_tcg2_register(void)
> >               tcg2_uninit();
> >               goto fail;
> >       }
> > +
> > +     ret = tcg2_measure_secure_boot_variable(dev);
> > +     if (ret != EFI_SUCCESS) {
> > +             tcg2_uninit();
> > +             goto fail;
> > +     }
> > +
> >       return ret;
> >
> >   fail:
> >