[PATCH v3 0/5] add measurement support

[PATCH v3 0/5] 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 (5):
  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            |  77 +++++++-
 include/tpm-v2.h              |  18 +-
 lib/efi_loader/efi_boottime.c |  25 +++
 lib/efi_loader/efi_tcg2.c     | 356 +++++++++++++++++++++++++++++++++-
 5 files changed, 471 insertions(+), 10 deletions(-)

-- 
2.17.1

[PATCH v3 1/5] 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", "dbx", "dbt", and "dbr".

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 v3:
- add "dbt" and "dbr" measurement
- accept empty variable measurement for "SecureBoot", "PK",
  "KEK", "db" and "dbx" as TCG2 spec requires
- fix comment format

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 | 165 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 185 insertions(+)

diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
index bcfb98168a..497ba3ce94 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 - event log structure of UEFI variable
+ * @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..a2e9587cd0 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,118 @@ 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)));
+	if (data) {
+		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++) {
+		/*
+		 * According to the TCG2 PC Client PFP spec, "SecureBoot",
+		 * "PK", "KEK", "db" and "dbx" variables must be measured
+		 * even if they are empty.
+		 */
+		data = efi_get_var(secure_variables[i].name,
+				   secure_variables[i].guid,
+				   &data_size);
+
+		ret = tcg2_measure_variable(dev, 7,
+					    EV_EFI_VARIABLE_DRIVER_CONFIG,
+					    secure_variables[i].name,
+					    secure_variables[i].guid,
+					    data_size, data);
+		free(data);
+		if (ret != EFI_SUCCESS)
+			goto error;
+	}
+
+	/*
+	 * TCG2 PC Client PFP spec says "dbt" and "dbr" are
+	 * measured if present and not empty.
+	 */
+	data = efi_get_var(L"dbt",
+			   &efi_global_variable_guid,
+			   &data_size);
+	if (data) {
+		ret = tcg2_measure_variable(dev, 7,
+					    EV_EFI_VARIABLE_DRIVER_CONFIG,
+					    L"dbt",
+					    &efi_global_variable_guid,
+					    data_size, data);
+		free(data);
+	}
+
+	data = efi_get_var(L"dbr",
+			   &efi_global_variable_guid,
+			   &data_size);
+	if (data) {
+		ret = tcg2_measure_variable(dev, 7,
+					    EV_EFI_VARIABLE_DRIVER_CONFIG,
+					    L"dbr",
+					    &efi_global_variable_guid,
+					    data_size, data);
+		free(data);
+	}
+
+error:
+	return ret;
+}
+
 /**
  * efi_tcg2_register() - register EFI_TCG2_PROTOCOL
  *
@@ -1328,6 +1486,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

Re: [PATCH v3 1/5] efi_loader: add secure boot variable measurement

[AKASHI Takahiro]

Kojima-san,

On Fri, Aug 06, 2021 at 04:02:11PM +0900, 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", "dbx", "dbt", and "dbr".
> 
> 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 v3:
> - add "dbt" and "dbr" measurement
> - accept empty variable measurement for "SecureBoot", "PK",
>   "KEK", "db" and "dbx" as TCG2 spec requires
> - fix comment format
> 
> 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 | 165 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 185 insertions(+)
> 
> diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
> index bcfb98168a..497ba3ce94 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 - event log structure of UEFI variable
> + * @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..a2e9587cd0 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,118 @@ 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)));
> +	if (data) {
> +		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++) {
> +		/*
> +		 * According to the TCG2 PC Client PFP spec, "SecureBoot",
> +		 * "PK", "KEK", "db" and "dbx" variables must be measured
> +		 * even if they are empty.
> +		 */
> +		data = efi_get_var(secure_variables[i].name,
> +				   secure_variables[i].guid,
> +				   &data_size);
> +
> +		ret = tcg2_measure_variable(dev, 7,
> +					    EV_EFI_VARIABLE_DRIVER_CONFIG,
> +					    secure_variables[i].name,
> +					    secure_variables[i].guid,
> +					    data_size, data);
> +		free(data);
> +		if (ret != EFI_SUCCESS)
> +			goto error;
> +	}
> +
> +	/*
> +	 * TCG2 PC Client PFP spec says "dbt" and "dbr" are
> +	 * measured if present and not empty.
> +	 */

The current implementation of secure boot doesn't support neither
dbt or dbr. Do we have to measure them now?
I think that EFI_OS_INDICATIONS_TIMESTAMP_REVOCATION bit in
osIndicationSupported should always be checked first.

> +	data = efi_get_var(L"dbt",
> +			   &efi_global_variable_guid,
> +			   &data_size);
> +	if (data) {
> +		ret = tcg2_measure_variable(dev, 7,
> +					    EV_EFI_VARIABLE_DRIVER_CONFIG,
> +					    L"dbt",
> +					    &efi_global_variable_guid,

=> efi_guid_image_security_database

> +					    data_size, data);
> +		free(data);
> +	}
> +
> +	data = efi_get_var(L"dbr",
> +			   &efi_global_variable_guid,
> +			   &data_size);
> +	if (data) {
> +		ret = tcg2_measure_variable(dev, 7,
> +					    EV_EFI_VARIABLE_DRIVER_CONFIG,
> +					    L"dbr",
> +					    &efi_global_variable_guid,

ditto for dbr

-Takahiro Akashi

> +					    data_size, data);
> +		free(data);
> +	}
> +
> +error:
> +	return ret;
> +}
> +
>  /**
>   * efi_tcg2_register() - register EFI_TCG2_PROTOCOL
>   *
> @@ -1328,6 +1486,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
> 

Re: [PATCH v3 1/5] efi_loader: add secure boot variable measurement

[Masahisa Kojima]

Hi Akashi-san,

Thank you for your comment.

On Tue, 10 Aug 2021 at 10:44, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> Kojima-san,
>
> On Fri, Aug 06, 2021 at 04:02:11PM +0900, 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", "dbx", "dbt", and "dbr".
> >
> > 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 v3:
> > - add "dbt" and "dbr" measurement
> > - accept empty variable measurement for "SecureBoot", "PK",
> >   "KEK", "db" and "dbx" as TCG2 spec requires
> > - fix comment format
> >
> > 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 | 165 ++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 185 insertions(+)
> >
> > diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
> > index bcfb98168a..497ba3ce94 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 - event log structure of UEFI variable
> > + * @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..a2e9587cd0 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,118 @@ 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)));
> > +     if (data) {
> > +             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++) {
> > +             /*
> > +              * According to the TCG2 PC Client PFP spec, "SecureBoot",
> > +              * "PK", "KEK", "db" and "dbx" variables must be measured
> > +              * even if they are empty.
> > +              */
> > +             data = efi_get_var(secure_variables[i].name,
> > +                                secure_variables[i].guid,
> > +                                &data_size);
> > +
> > +             ret = tcg2_measure_variable(dev, 7,
> > +                                         EV_EFI_VARIABLE_DRIVER_CONFIG,
> > +                                         secure_variables[i].name,
> > +                                         secure_variables[i].guid,
> > +                                         data_size, data);
> > +             free(data);
> > +             if (ret != EFI_SUCCESS)
> > +                     goto error;
> > +     }
> > +
> > +     /*
> > +      * TCG2 PC Client PFP spec says "dbt" and "dbr" are
> > +      * measured if present and not empty.
> > +      */
>
> The current implementation of secure boot doesn't support neither
> dbt or dbr. Do we have to measure them now?

As Heinrich commented to my v2 patch, dbt and dbr measurement are added,
because now would not harm anything.

> I think that EFI_OS_INDICATIONS_TIMESTAMP_REVOCATION bit in
> osIndicationSupported should always be checked first.

According to the TCG PC Client PFP spec,
dbt and dbr are measured if present and not empty, regardless of
the osIndicationSupported bit.
 # EDK2 also measures dbt without checking osIndicationSupported bit.

Thanks,
Masahisa Kojima

>
> > +     data = efi_get_var(L"dbt",
> > +                        &efi_global_variable_guid,
> > +                        &data_size);
> > +     if (data) {
> > +             ret = tcg2_measure_variable(dev, 7,
> > +                                         EV_EFI_VARIABLE_DRIVER_CONFIG,
> > +                                         L"dbt",
> > +                                         &efi_global_variable_guid,
>
> => efi_guid_image_security_database
>
> > +                                         data_size, data);
> > +             free(data);
> > +     }
> > +
> > +     data = efi_get_var(L"dbr",
> > +                        &efi_global_variable_guid,
> > +                        &data_size);
> > +     if (data) {
> > +             ret = tcg2_measure_variable(dev, 7,
> > +                                         EV_EFI_VARIABLE_DRIVER_CONFIG,
> > +                                         L"dbr",
> > +                                         &efi_global_variable_guid,
>
> ditto for dbr
>
> -Takahiro Akashi
>
> > +                                         data_size, data);
> > +             free(data);
> > +     }
> > +
> > +error:
> > +     return ret;
> > +}
> > +
> >  /**
> >   * efi_tcg2_register() - register EFI_TCG2_PROTOCOL
> >   *
> > @@ -1328,6 +1486,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 v3 2/5] 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 v3:
- modify log output

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 a120d94431..345cbb72c4 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -499,6 +499,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 0b98e91813..13ab139222 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -2994,6 +2994,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) {
+				log_warning("tcg2 measurement fails(0x%lx)\n",
+					    ret);
+			}
+		}
+	}
+
 	/* call the image! */
 	if (setjmp(&exit_jmp)) {
 		/*
@@ -3252,6 +3262,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) {
+				log_warning("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 a2e9587cd0..7dd30c2bc9 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
@@ -1385,6 +1386,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) {
+		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

Re: [PATCH v3 2/5] efi_loader: add boot variable measurement

[AKASHI Takahiro]

On Fri, Aug 06, 2021 at 04:02:12PM +0900, 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 v3:
> - modify log output
> 
> 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 a120d94431..345cbb72c4 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -499,6 +499,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);

I don't think that we need to have EFIAPI specifiers for those functions
as they are not api's.

>  /* 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 0b98e91813..13ab139222 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -2994,6 +2994,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) {
> +				log_warning("tcg2 measurement fails(0x%lx)\n",
> +					    ret);
> +			}
> +		}
> +	}
> +
>  	/* call the image! */
>  	if (setjmp(&exit_jmp)) {
>  		/*
> @@ -3252,6 +3262,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) {
> +				log_warning("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 a2e9587cd0..7dd30c2bc9 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
> @@ -1385,6 +1386,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) {
> +		ret = EFI_NOT_FOUND;
> +		goto error;
> +	}

What if BootOrder is not defined, but BootNext is defined?

-Takahiro Akashi

> +
> +	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
> 

Re: [PATCH v3 2/5] efi_loader: add boot variable measurement

[Masahisa Kojima]

On Tue, 10 Aug 2021 at 11:19, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> On Fri, Aug 06, 2021 at 04:02:12PM +0900, 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 v3:
> > - modify log output
> >
> > 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 a120d94431..345cbb72c4 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -499,6 +499,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);
>
> I don't think that we need to have EFIAPI specifiers for those functions
> as they are not api's.

Yes, I will remove EFIAPI specifiers.

>
> >  /* 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 0b98e91813..13ab139222 100644
> > --- a/lib/efi_loader/efi_boottime.c
> > +++ b/lib/efi_loader/efi_boottime.c
> > @@ -2994,6 +2994,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) {
> > +                             log_warning("tcg2 measurement fails(0x%lx)\n",
> > +                                         ret);
> > +                     }
> > +             }
> > +     }
> > +
> >       /* call the image! */
> >       if (setjmp(&exit_jmp)) {
> >               /*
> > @@ -3252,6 +3262,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) {
> > +                             log_warning("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 a2e9587cd0..7dd30c2bc9 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
> > @@ -1385,6 +1386,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) {
> > +             ret = EFI_NOT_FOUND;
> > +             goto error;
> > +     }
>
> What if BootOrder is not defined, but BootNext is defined?

TCG PC Client PFP spec does not require to measure the case
when the system boots with BootNext.
So in this case, BootNext is defined but BootOrder is not defined,
no measurement occurs for boot variables.

Thanks,
Masahisa Kojima

>
> -Takahiro Akashi
>
> > +
> > +     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
> >

Re: [PATCH v3 2/5] efi_loader: add boot variable measurement

[AKASHI Takahiro]

On Wed, Aug 11, 2021 at 12:05:39PM +0900, Masahisa Kojima wrote:
> On Tue, 10 Aug 2021 at 11:19, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> >
> > On Fri, Aug 06, 2021 at 04:02:12PM +0900, 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 v3:
> > > - modify log output
> > >
> > > 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 a120d94431..345cbb72c4 100644
> > > --- a/include/efi_loader.h
> > > +++ b/include/efi_loader.h
> > > @@ -499,6 +499,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);
> >
> > I don't think that we need to have EFIAPI specifiers for those functions
> > as they are not api's.
> 
> Yes, I will remove EFIAPI specifiers.
> 
> >
> > >  /* 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 0b98e91813..13ab139222 100644
> > > --- a/lib/efi_loader/efi_boottime.c
> > > +++ b/lib/efi_loader/efi_boottime.c
> > > @@ -2994,6 +2994,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) {
> > > +                             log_warning("tcg2 measurement fails(0x%lx)\n",
> > > +                                         ret);
> > > +                     }
> > > +             }
> > > +     }
> > > +
> > >       /* call the image! */
> > >       if (setjmp(&exit_jmp)) {
> > >               /*
> > > @@ -3252,6 +3262,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) {
> > > +                             log_warning("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 a2e9587cd0..7dd30c2bc9 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
> > > @@ -1385,6 +1386,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) {
> > > +             ret = EFI_NOT_FOUND;
> > > +             goto error;
> > > +     }
> >
> > What if BootOrder is not defined, but BootNext is defined?
> 
> TCG PC Client PFP spec does not require to measure the case
> when the system boots with BootNext.
> So in this case, BootNext is defined but BootOrder is not defined,
> no measurement occurs for boot variables.

I simply wonder why the spec does care about BootOrder, but not BootNext.
Let take another case; BootNext, BootOrder and BootXXXX are all defined,
and the system boots up by BootNext. In this case, we will have measurement
for boot variables. Why do we need this although we don't take measurement
in case of BootNext & !BootOrder?

-Takahiro Akashi


> Thanks,
> Masahisa Kojima
> 
> >
> > -Takahiro Akashi
> >
> > > +
> > > +     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
> > >

Re: [PATCH v3 2/5] efi_loader: add boot variable measurement

[Masahisa Kojima]

On Wed, 11 Aug 2021 at 15:50, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> On Wed, Aug 11, 2021 at 12:05:39PM +0900, Masahisa Kojima wrote:
> > On Tue, 10 Aug 2021 at 11:19, AKASHI Takahiro
> > <takahiro.akashi@linaro.org> wrote:
> > >
> > > On Fri, Aug 06, 2021 at 04:02:12PM +0900, 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 v3:
> > > > - modify log output
> > > >
> > > > 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 a120d94431..345cbb72c4 100644
> > > > --- a/include/efi_loader.h
> > > > +++ b/include/efi_loader.h
> > > > @@ -499,6 +499,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);
> > >
> > > I don't think that we need to have EFIAPI specifiers for those functions
> > > as they are not api's.
> >
> > Yes, I will remove EFIAPI specifiers.
> >
> > >
> > > >  /* 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 0b98e91813..13ab139222 100644
> > > > --- a/lib/efi_loader/efi_boottime.c
> > > > +++ b/lib/efi_loader/efi_boottime.c
> > > > @@ -2994,6 +2994,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) {
> > > > +                             log_warning("tcg2 measurement fails(0x%lx)\n",
> > > > +                                         ret);
> > > > +                     }
> > > > +             }
> > > > +     }
> > > > +
> > > >       /* call the image! */
> > > >       if (setjmp(&exit_jmp)) {
> > > >               /*
> > > > @@ -3252,6 +3262,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) {
> > > > +                             log_warning("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 a2e9587cd0..7dd30c2bc9 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
> > > > @@ -1385,6 +1386,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) {
> > > > +             ret = EFI_NOT_FOUND;
> > > > +             goto error;
> > > > +     }
> > >
> > > What if BootOrder is not defined, but BootNext is defined?
> >
> > TCG PC Client PFP spec does not require to measure the case
> > when the system boots with BootNext.
> > So in this case, BootNext is defined but BootOrder is not defined,
> > no measurement occurs for boot variables.
>
> I simply wonder why the spec does care about BootOrder, but not BootNext.
> Let take another case; BootNext, BootOrder and BootXXXX are all defined,
> and the system boots up by BootNext. In this case, we will have measurement
> for boot variables. Why do we need this although we don't take measurement
> in case of BootNext & !BootOrder?

Good point, Akashi-san.
In my understanding, TCG spec(Measured Boot) tries to cover
the typical boot flow, system boot with BootNext is a bit special use case.

Host Platform Configuration(BootOrder and BootXXXX, etc.) are
measured into PCR[1], and next boot image(OS Loader, etc.) is
measured in PCR[4].
So at least boot image is measured in BootNext & !BootOrder case.

This is also related to the usage of PCRs in the later process such as
attestation.

Thanks,
Masahisa Kojima

>
> -Takahiro Akashi
>
>
> > Thanks,
> > Masahisa Kojima
> >
> > >
> > > -Takahiro Akashi
> > >
> > > > +
> > > > +     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 v3 3/5] 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>
---
(no changes since v2)

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 345cbb72c4..d2b6768899 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -499,6 +499,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 13ab139222..b818cbb540 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -2182,6 +2182,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 7dd30c2bc9..bcd8626ff7 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -1506,6 +1506,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
  *
@@ -1584,6 +1645,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) {
@@ -1608,6 +1670,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 v3 4/5] 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>
---

(no changes since v1)


 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 bcd8626ff7..f38d578a32 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 v3 5/5] 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 v3:
- update comment format

Changes in v2:
- newly create commit from v2

 include/efi_tcg2.h | 57 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 54 insertions(+), 3 deletions(-)

diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
index 497ba3ce94..b6b958da51 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 - structure of EFI TCG2 version
+ * @major:	major version
+ * @minor:	minor version
+ */
 struct efi_tcg2_version {
 	u8 major;
 	u8 minor;
 };
 
+/**
+ * struct tdEFI_TCG2_EVENT_HEADER - structure of EFI 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 - structure of EFI 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 - structure of PE/COFF image measurement
+ * @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 - protocol capability information
+ * @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;
@@ -86,7 +137,7 @@ struct efi_tcg2_boot_service_capability {
 #define TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_ERRATA_TPM2 2
 
 /**
- *  struct TCG_EfiSpecIdEventAlgorithmSize
+ *  struct TCG_EfiSpecIdEventAlgorithmSize - hashing algorithm information
  *
  *  @algorithm_id:	algorithm defined in enum tpm2_algorithms
  *  @digest_size:	size of the algorithm
@@ -97,7 +148,7 @@ struct tcg_efi_spec_id_event_algorithm_size {
 } __packed;
 
 /**
- * struct TCG_EfiSpecIDEventStruct
+ * struct TCG_EfiSpecIDEventStruct - content of the event log header
  *
  * @signature:			signature, set to Spec ID Event03
  * @platform_class:		class defined in TCG ACPI Specification
@@ -130,7 +181,7 @@ struct tcg_efi_spec_id_event {
 } __packed;
 
 /**
- * struct tdEFI_TCG2_FINAL_EVENTS_TABLE
+ * struct tdEFI_TCG2_FINAL_EVENTS_TABLE - log entries after Get Event Log
  * @version:		version number for this structure
  * @number_of_events:	number of events recorded after invocation of
  *			GetEventLog()
-- 
2.17.1

Re: [PATCH v3 0/5] add measurement support

[Heinrich Schuchardt]

On 8/6/21 9:02 AM, Masahisa Kojima wrote:
> 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.

Do you plan to measure the devicetree using event type 
EV_EFI_HANDOFF_TABLES? Or does Linux measure it?

Are there cases where devicetrees contain non-static information which 
would have to be excluded from such measurement (e.g. randomized MAC 
address)?

Best regards

Heinrich

> 
> Masahisa Kojima (5):
>    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            |  77 +++++++-
>   include/tpm-v2.h              |  18 +-
>   lib/efi_loader/efi_boottime.c |  25 +++
>   lib/efi_loader/efi_tcg2.c     | 356 +++++++++++++++++++++++++++++++++-
>   5 files changed, 471 insertions(+), 10 deletions(-)
>