All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] fix TCG2 error handling
@ 2021-12-07  5:15 Masahisa Kojima
  2021-12-07  5:15 ` [PATCH v2 1/3] efi_loader: efi_tcg2_register returns appropriate error Masahisa Kojima
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Masahisa Kojima @ 2021-12-07  5:15 UTC (permalink / raw)
  To: u-boot
  Cc: Heinrich Schuchardt, Ilias Apalodimas, Simon Glass, Masahisa Kojima

This series fix the efi_tcg2.c error handling.

Masahisa Kojima (3):
  efi_loader: efi_tcg2_register returns appropriate error
  efi_loader: check tcg2 protocol installation outside the TCG protocol
  efi_loader: correctly handle no tpm device error

 include/efi_loader.h              |  2 +
 lib/efi_loader/Kconfig            |  2 +
 lib/efi_loader/efi_boottime.c     |  9 ++--
 lib/efi_loader/efi_image_loader.c | 13 +++--
 lib/efi_loader/efi_setup.c        |  4 ++
 lib/efi_loader/efi_tcg2.c         | 87 ++++++++++++++++++++++++-------
 6 files changed, 91 insertions(+), 26 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 1/3] efi_loader: efi_tcg2_register returns appropriate error
  2021-12-07  5:15 [PATCH v2 0/3] fix TCG2 error handling Masahisa Kojima
@ 2021-12-07  5:15 ` Masahisa Kojima
  2021-12-09 16:01   ` Ilias Apalodimas
  2021-12-07  5:15 ` [PATCH v2 2/3] efi_loader: check tcg2 protocol installation outside the TCG protocol Masahisa Kojima
  2021-12-07  5:15 ` [PATCH v2 3/3] efi_loader: correctly handle no tpm device error Masahisa Kojima
  2 siblings, 1 reply; 7+ messages in thread
From: Masahisa Kojima @ 2021-12-07  5:15 UTC (permalink / raw)
  To: u-boot
  Cc: Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
	Masahisa Kojima, Alexander Graf

This commit modify efi_tcg2_register() to return the
appropriate error.
With this fix, sandbox will not boot because efi_tcg2_register()
fails due to some missing feature in GetCapabilities.
So disable sandbox if EFI_TCG2_PROTOCOL is enabled.

UEFI secure boot variable measurement is not directly related
to TCG2 protocol installation, tcg2_measure_secure_boot_variable()
is moved to the separate function.

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

Changes in v2:
- return EFI_SECURITY_VIOLATION if there is no tpm device found
  in efi_tcg2_do_initial_measurement()

 include/efi_loader.h       |  2 ++
 lib/efi_loader/Kconfig     |  2 ++
 lib/efi_loader/efi_setup.c |  4 +++
 lib/efi_loader/efi_tcg2.c  | 65 +++++++++++++++++++++++++++-----------
 4 files changed, 55 insertions(+), 18 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index 67c40ca57a..f4860e87fc 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -525,6 +525,8 @@ efi_status_t efi_disk_register(void);
 efi_status_t efi_rng_register(void);
 /* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */
 efi_status_t efi_tcg2_register(void);
+/* Called by efi_init_obj_list() to do initial measurement */
+efi_status_t efi_tcg2_do_initial_measurement(void);
 /* measure the pe-coff image, extend PCR and add Event Log */
 efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
 				   struct efi_loaded_image_obj *handle,
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 700dc838dd..24f9a2bb75 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -308,6 +308,8 @@ config EFI_TCG2_PROTOCOL
 	bool "EFI_TCG2_PROTOCOL support"
 	default y
 	depends on TPM_V2
+	# Sandbox TPM currently fails on GetCapabilities needed for TCG2
+	depends on !SANDBOX
 	select SHA1
 	select SHA256
 	select SHA384
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index 1aba71cd96..49172e3579 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -241,6 +241,10 @@ efi_status_t efi_init_obj_list(void)
 		ret = efi_tcg2_register();
 		if (ret != EFI_SUCCESS)
 			goto out;
+
+		ret = efi_tcg2_do_initial_measurement();
+		if (ret == EFI_SECURITY_VIOLATION)
+			goto out;
 	}
 
 	/* Secure boot */
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 5f71b188a0..bdfd9a37b5 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -153,6 +153,15 @@ static u16 alg_to_len(u16 hash_alg)
 	return 0;
 }
 
+static bool is_tcg2_protocol_installed(void)
+{
+	struct efi_handler *handler;
+	efi_status_t ret;
+
+	ret = efi_search_protocol(efi_root, &efi_guid_tcg2_protocol, &handler);
+	return ret == EFI_SUCCESS;
+}
+
 static u32 tcg_event_final_size(struct tpml_digest_values *digest_list)
 {
 	u32 len;
@@ -1664,6 +1673,14 @@ void tcg2_uninit(void)
 	event_log.buffer = NULL;
 	efi_free_pool(event_log.final_buffer);
 	event_log.final_buffer = NULL;
+
+	if (!is_tcg2_protocol_installed())
+		return;
+
+	ret = efi_remove_protocol(efi_root, &efi_guid_tcg2_protocol,
+				  (void *)&efi_tcg2_protocol);
+	if (ret != EFI_SUCCESS)
+		log_err("Failed to remove EFI TCG2 protocol\n");
 }
 
 /**
@@ -2345,12 +2362,37 @@ error:
 	return ret;
 }
 
+/**
+ * efi_tcg2_do_initial_measurement() - do initial measurement
+ *
+ * Return:	status code
+ */
+efi_status_t efi_tcg2_do_initial_measurement(void)
+{
+	efi_status_t ret;
+	struct udevice *dev;
+
+	if (!is_tcg2_protocol_installed())
+		return EFI_SUCCESS;
+
+	ret = platform_get_tpm2_device(&dev);
+	if (ret != EFI_SUCCESS)
+		return EFI_SECURITY_VIOLATION;
+
+	ret = tcg2_measure_secure_boot_variable(dev);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+out:
+	return ret;
+}
+
 /**
  * efi_tcg2_register() - register EFI_TCG2_PROTOCOL
  *
  * If a TPM2 device is available, the TPM TCG2 Protocol is registered
  *
- * Return:	An error status is only returned if adding the protocol fails.
+ * Return:	status code
  */
 efi_status_t efi_tcg2_register(void)
 {
@@ -2373,8 +2415,10 @@ efi_status_t efi_tcg2_register(void)
 	}
 
 	ret = efi_init_event_log();
-	if (ret != EFI_SUCCESS)
+	if (ret != EFI_SUCCESS) {
+		tcg2_uninit();
 		goto fail;
+	}
 
 	ret = efi_add_protocol(efi_root, &efi_guid_tcg2_protocol,
 			       (void *)&efi_tcg2_protocol);
@@ -2391,24 +2435,9 @@ efi_status_t efi_tcg2_register(void)
 		goto fail;
 	}
 
-	ret = tcg2_measure_secure_boot_variable(dev);
-	if (ret != EFI_SUCCESS) {
-		tcg2_uninit();
-		goto fail;
-	}
-
 	return ret;
 
 fail:
 	log_err("Cannot install EFI_TCG2_PROTOCOL\n");
-	/*
-	 * Return EFI_SUCCESS and don't stop the EFI subsystem.
-	 * That's done for 2 reasons
-	 * - If the protocol is not installed the PCRs won't be extended.  So
-	 *   someone later in the boot flow will notice that and take the
-	 *   necessary actions.
-	 * - The TPM sandbox is limited and we won't be able to run any efi
-	 *   related tests with TCG2 enabled
-	 */
-	return EFI_SUCCESS;
+	return ret;
 }
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 2/3] efi_loader: check tcg2 protocol installation outside the TCG protocol
  2021-12-07  5:15 [PATCH v2 0/3] fix TCG2 error handling Masahisa Kojima
  2021-12-07  5:15 ` [PATCH v2 1/3] efi_loader: efi_tcg2_register returns appropriate error Masahisa Kojima
@ 2021-12-07  5:15 ` Masahisa Kojima
  2021-12-09 15:48   ` Ilias Apalodimas
  2021-12-07  5:15 ` [PATCH v2 3/3] efi_loader: correctly handle no tpm device error Masahisa Kojima
  2 siblings, 1 reply; 7+ messages in thread
From: Masahisa Kojima @ 2021-12-07  5:15 UTC (permalink / raw)
  To: u-boot
  Cc: Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
	Masahisa Kojima, Alexander Graf

There are functions that calls tcg2_agile_log_append() outside
of the TCG protocol invocation (e.g tcg2_measure_pe_image).
These functions must to check that TCG2 protocol is installed.
If not, measurement shall be skipped.

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

No update since v1

 lib/efi_loader/efi_tcg2.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index bdfd9a37b5..59bce85028 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -972,6 +972,9 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
 	IMAGE_NT_HEADERS32 *nt;
 	struct efi_handler *handler;
 
+	if (!is_tcg2_protocol_installed())
+		return EFI_SUCCESS;
+
 	ret = platform_get_tpm2_device(&dev);
 	if (ret != EFI_SUCCESS)
 		return ret;
@@ -2189,6 +2192,9 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(struct efi_loaded_image_obj *ha
 	u32 event = 0;
 	struct smbios_entry *entry;
 
+	if (!is_tcg2_protocol_installed())
+		return EFI_SUCCESS;
+
 	if (tcg2_efi_app_invoked)
 		return EFI_SUCCESS;
 
@@ -2239,6 +2245,9 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void)
 	efi_status_t ret;
 	struct udevice *dev;
 
+	if (!is_tcg2_protocol_installed())
+		return EFI_SUCCESS;
+
 	ret = platform_get_tpm2_device(&dev);
 	if (ret != EFI_SUCCESS)
 		return ret;
@@ -2264,6 +2273,12 @@ efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context)
 	EFI_ENTRY("%p, %p", event, context);
 
 	event_log.ebs_called = true;
+
+	if (!is_tcg2_protocol_installed()) {
+		ret = EFI_SUCCESS;
+		goto out;
+	}
+
 	ret = platform_get_tpm2_device(&dev);
 	if (ret != EFI_SUCCESS)
 		goto out;
@@ -2293,6 +2308,9 @@ efi_status_t efi_tcg2_notify_exit_boot_services_failed(void)
 	struct udevice *dev;
 	efi_status_t ret;
 
+	if (!is_tcg2_protocol_installed())
+		return EFI_SUCCESS;
+
 	ret = platform_get_tpm2_device(&dev);
 	if (ret != EFI_SUCCESS)
 		goto out;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 3/3] efi_loader: correctly handle no tpm device error
  2021-12-07  5:15 [PATCH v2 0/3] fix TCG2 error handling Masahisa Kojima
  2021-12-07  5:15 ` [PATCH v2 1/3] efi_loader: efi_tcg2_register returns appropriate error Masahisa Kojima
  2021-12-07  5:15 ` [PATCH v2 2/3] efi_loader: check tcg2 protocol installation outside the TCG protocol Masahisa Kojima
@ 2021-12-07  5:15 ` Masahisa Kojima
  2021-12-09 15:40   ` Ilias Apalodimas
  2 siblings, 1 reply; 7+ messages in thread
From: Masahisa Kojima @ 2021-12-07  5:15 UTC (permalink / raw)
  To: u-boot
  Cc: Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
	Masahisa Kojima, Alexander Graf

When the TCG2 protocol is installed in efi_tcg2_register(),
TPM2 device must be present.
tcg2_measure_pe_image() expects that TCP2 protocol is installed
and TPM device is available. If TCG2 Protocol is installed but
TPM device is not found, tcg2_measure_pe_image() returns
EFI_SECURITY_VIOLATION and efi_load_image() ends with failure.

The same error handling is applied to
efi_tcg2_measure_efi_app_invocation().

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

Changes in v2:
- EFI_SECURITY_VIOLATION is returned in
  efi_tcg2_measure_efi_app_invocation()

 lib/efi_loader/efi_boottime.c     |  9 ++++++---
 lib/efi_loader/efi_image_loader.c | 13 ++++++++++---
 lib/efi_loader/efi_tcg2.c         |  4 ++--
 3 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 8492b732f3..20b69699fe 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -3016,9 +3016,12 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,
 	if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
 		if (image_obj->image_type == IMAGE_SUBSYSTEM_EFI_APPLICATION) {
 			ret = efi_tcg2_measure_efi_app_invocation(image_obj);
-			if (ret != EFI_SUCCESS) {
-				log_warning("tcg2 measurement fails(0x%lx)\n",
-					    ret);
+			if (ret == EFI_SECURITY_VIOLATION) {
+				/*
+				 * TCG2 Protocol is installed but no TPM device found,
+				 * this is not expected.
+				 */
+				return EFI_EXIT(EFI_SECURITY_VIOLATION);
 			}
 		}
 	}
diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
index eb95580538..773bd0677c 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -934,9 +934,16 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
 
 #if CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL)
 	/* Measure an PE/COFF image */
-	if (tcg2_measure_pe_image(efi, efi_size, handle,
-				  loaded_image_info))
-		log_err("PE image measurement failed\n");
+	ret = tcg2_measure_pe_image(efi, efi_size, handle, loaded_image_info);
+	if (ret == EFI_SECURITY_VIOLATION) {
+		/*
+		 * TCG2 Protocol is installed but no TPM device found,
+		 * this is not expected.
+		 */
+		log_err("PE image measurement failed, no tpm device found\n");
+		goto err;
+	}
+
 #endif
 
 	/* Copy PE headers */
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 59bce85028..0ae07ef083 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -977,7 +977,7 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
 
 	ret = platform_get_tpm2_device(&dev);
 	if (ret != EFI_SUCCESS)
-		return ret;
+		return EFI_SECURITY_VIOLATION;
 
 	switch (handle->image_type) {
 	case IMAGE_SUBSYSTEM_EFI_APPLICATION:
@@ -2200,7 +2200,7 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(struct efi_loaded_image_obj *ha
 
 	ret = platform_get_tpm2_device(&dev);
 	if (ret != EFI_SUCCESS)
-		return ret;
+		return EFI_SECURITY_VIOLATION;
 
 	ret = tcg2_measure_boot_variable(dev);
 	if (ret != EFI_SUCCESS)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 3/3] efi_loader: correctly handle no tpm device error
  2021-12-07  5:15 ` [PATCH v2 3/3] efi_loader: correctly handle no tpm device error Masahisa Kojima
@ 2021-12-09 15:40   ` Ilias Apalodimas
  0 siblings, 0 replies; 7+ messages in thread
From: Ilias Apalodimas @ 2021-12-09 15:40 UTC (permalink / raw)
  To: Masahisa Kojima; +Cc: u-boot, Heinrich Schuchardt, Simon Glass, Alexander Graf

Heinrich does this approach work for you till we fix the DM-EFI
integration?

At least it tries to cover some cases were the efi protocol is installed
(which means the tpm was there in the beginning), but later on is removed

On Tue, 7 Dec 2021 at 07:11, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:
>
> When the TCG2 protocol is installed in efi_tcg2_register(),
> TPM2 device must be present.
> tcg2_measure_pe_image() expects that TCP2 protocol is installed
> and TPM device is available. If TCG2 Protocol is installed but
> TPM device is not found, tcg2_measure_pe_image() returns
> EFI_SECURITY_VIOLATION and efi_load_image() ends with failure.
>
> The same error handling is applied to
> efi_tcg2_measure_efi_app_invocation().
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
>
> Changes in v2:
> - EFI_SECURITY_VIOLATION is returned in
>   efi_tcg2_measure_efi_app_invocation()
>
>  lib/efi_loader/efi_boottime.c     |  9 ++++++---
>  lib/efi_loader/efi_image_loader.c | 13 ++++++++++---
>  lib/efi_loader/efi_tcg2.c         |  4 ++--
>  3 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 8492b732f3..20b69699fe 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -3016,9 +3016,12 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,
>         if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
>                 if (image_obj->image_type == IMAGE_SUBSYSTEM_EFI_APPLICATION) {
>                         ret = efi_tcg2_measure_efi_app_invocation(image_obj);
> -                       if (ret != EFI_SUCCESS) {
> -                               log_warning("tcg2 measurement fails(0x%lx)\n",
> -                                           ret);
> +                       if (ret == EFI_SECURITY_VIOLATION) {
> +                               /*
> +                                * TCG2 Protocol is installed but no TPM device found,
> +                                * this is not expected.
> +                                */
> +                               return EFI_EXIT(EFI_SECURITY_VIOLATION);
>                         }
>                 }
>         }
> diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
> index eb95580538..773bd0677c 100644
> --- a/lib/efi_loader/efi_image_loader.c
> +++ b/lib/efi_loader/efi_image_loader.c
> @@ -934,9 +934,16 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
>
>  #if CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL)
>         /* Measure an PE/COFF image */
> -       if (tcg2_measure_pe_image(efi, efi_size, handle,
> -                                 loaded_image_info))
> -               log_err("PE image measurement failed\n");
> +       ret = tcg2_measure_pe_image(efi, efi_size, handle, loaded_image_info);
> +       if (ret == EFI_SECURITY_VIOLATION) {
> +               /*
> +                * TCG2 Protocol is installed but no TPM device found,
> +                * this is not expected.
> +                */
> +               log_err("PE image measurement failed, no tpm device found\n");
> +               goto err;
> +       }
> +
>  #endif
>
>         /* Copy PE headers */
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 59bce85028..0ae07ef083 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -977,7 +977,7 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
>
>         ret = platform_get_tpm2_device(&dev);
>         if (ret != EFI_SUCCESS)
> -               return ret;
> +               return EFI_SECURITY_VIOLATION;
>
>         switch (handle->image_type) {
>         case IMAGE_SUBSYSTEM_EFI_APPLICATION:
> @@ -2200,7 +2200,7 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(struct efi_loaded_image_obj *ha
>
>         ret = platform_get_tpm2_device(&dev);
>         if (ret != EFI_SUCCESS)
> -               return ret;
> +               return EFI_SECURITY_VIOLATION;
>
>         ret = tcg2_measure_boot_variable(dev);
>         if (ret != EFI_SUCCESS)
> --
> 2.17.1
>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/3] efi_loader: check tcg2 protocol installation outside the TCG protocol
  2021-12-07  5:15 ` [PATCH v2 2/3] efi_loader: check tcg2 protocol installation outside the TCG protocol Masahisa Kojima
@ 2021-12-09 15:48   ` Ilias Apalodimas
  0 siblings, 0 replies; 7+ messages in thread
From: Ilias Apalodimas @ 2021-12-09 15:48 UTC (permalink / raw)
  To: Masahisa Kojima; +Cc: u-boot, Heinrich Schuchardt, Simon Glass, Alexander Graf

On Tue, 7 Dec 2021 at 07:11, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:
>
> There are functions that calls tcg2_agile_log_append() outside
> of the TCG protocol invocation (e.g tcg2_measure_pe_image).
> These functions must to check that TCG2 protocol is installed.
> If not, measurement shall be skipped.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
>
> No update since v1
>
>  lib/efi_loader/efi_tcg2.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index bdfd9a37b5..59bce85028 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -972,6 +972,9 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
>         IMAGE_NT_HEADERS32 *nt;
>         struct efi_handler *handler;
>
> +       if (!is_tcg2_protocol_installed())
> +               return EFI_SUCCESS;
> +
>         ret = platform_get_tpm2_device(&dev);
>         if (ret != EFI_SUCCESS)
>                 return ret;
> @@ -2189,6 +2192,9 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(struct efi_loaded_image_obj *ha
>         u32 event = 0;
>         struct smbios_entry *entry;
>
> +       if (!is_tcg2_protocol_installed())
> +               return EFI_SUCCESS;
> +
>         if (tcg2_efi_app_invoked)
>                 return EFI_SUCCESS;
>
> @@ -2239,6 +2245,9 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void)
>         efi_status_t ret;
>         struct udevice *dev;
>
> +       if (!is_tcg2_protocol_installed())
> +               return EFI_SUCCESS;
> +
>         ret = platform_get_tpm2_device(&dev);
>         if (ret != EFI_SUCCESS)
>                 return ret;
> @@ -2264,6 +2273,12 @@ efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context)
>         EFI_ENTRY("%p, %p", event, context);
>
>         event_log.ebs_called = true;
> +
> +       if (!is_tcg2_protocol_installed()) {
> +               ret = EFI_SUCCESS;
> +               goto out;
> +       }
> +
>         ret = platform_get_tpm2_device(&dev);
>         if (ret != EFI_SUCCESS)
>                 goto out;
> @@ -2293,6 +2308,9 @@ efi_status_t efi_tcg2_notify_exit_boot_services_failed(void)
>         struct udevice *dev;
>         efi_status_t ret;
>
> +       if (!is_tcg2_protocol_installed())
> +               return EFI_SUCCESS;
> +
>         ret = platform_get_tpm2_device(&dev);
>         if (ret != EFI_SUCCESS)
>                 goto out;
> --
> 2.17.1
>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/3] efi_loader: efi_tcg2_register returns appropriate error
  2021-12-07  5:15 ` [PATCH v2 1/3] efi_loader: efi_tcg2_register returns appropriate error Masahisa Kojima
@ 2021-12-09 16:01   ` Ilias Apalodimas
  0 siblings, 0 replies; 7+ messages in thread
From: Ilias Apalodimas @ 2021-12-09 16:01 UTC (permalink / raw)
  To: Masahisa Kojima; +Cc: u-boot, Heinrich Schuchardt, Simon Glass, Alexander Graf

On Tue, 7 Dec 2021 at 07:11, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:
>
> This commit modify efi_tcg2_register() to return the
> appropriate error.
> With this fix, sandbox will not boot because efi_tcg2_register()
> fails due to some missing feature in GetCapabilities.
> So disable sandbox if EFI_TCG2_PROTOCOL is enabled.
>
> UEFI secure boot variable measurement is not directly related
> to TCG2 protocol installation, tcg2_measure_secure_boot_variable()
> is moved to the separate function.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
>
> Changes in v2:
> - return EFI_SECURITY_VIOLATION if there is no tpm device found
>   in efi_tcg2_do_initial_measurement()
>
>  include/efi_loader.h       |  2 ++
>  lib/efi_loader/Kconfig     |  2 ++
>  lib/efi_loader/efi_setup.c |  4 +++
>  lib/efi_loader/efi_tcg2.c  | 65 +++++++++++++++++++++++++++-----------
>  4 files changed, 55 insertions(+), 18 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 67c40ca57a..f4860e87fc 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -525,6 +525,8 @@ efi_status_t efi_disk_register(void);
>  efi_status_t efi_rng_register(void);
>  /* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */
>  efi_status_t efi_tcg2_register(void);
> +/* Called by efi_init_obj_list() to do initial measurement */
> +efi_status_t efi_tcg2_do_initial_measurement(void);
>  /* measure the pe-coff image, extend PCR and add Event Log */
>  efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
>                                    struct efi_loaded_image_obj *handle,
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 700dc838dd..24f9a2bb75 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -308,6 +308,8 @@ config EFI_TCG2_PROTOCOL
>         bool "EFI_TCG2_PROTOCOL support"
>         default y
>         depends on TPM_V2
> +       # Sandbox TPM currently fails on GetCapabilities needed for TCG2
> +       depends on !SANDBOX
>         select SHA1
>         select SHA256
>         select SHA384
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index 1aba71cd96..49172e3579 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -241,6 +241,10 @@ efi_status_t efi_init_obj_list(void)
>                 ret = efi_tcg2_register();
>                 if (ret != EFI_SUCCESS)
>                         goto out;
> +
> +               ret = efi_tcg2_do_initial_measurement();
> +               if (ret == EFI_SECURITY_VIOLATION)
> +                       goto out;
>         }
>
>         /* Secure boot */
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 5f71b188a0..bdfd9a37b5 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -153,6 +153,15 @@ static u16 alg_to_len(u16 hash_alg)
>         return 0;
>  }
>
> +static bool is_tcg2_protocol_installed(void)
> +{
> +       struct efi_handler *handler;
> +       efi_status_t ret;
> +
> +       ret = efi_search_protocol(efi_root, &efi_guid_tcg2_protocol, &handler);
> +       return ret == EFI_SUCCESS;
> +}
> +
>  static u32 tcg_event_final_size(struct tpml_digest_values *digest_list)
>  {
>         u32 len;
> @@ -1664,6 +1673,14 @@ void tcg2_uninit(void)
>         event_log.buffer = NULL;
>         efi_free_pool(event_log.final_buffer);
>         event_log.final_buffer = NULL;
> +
> +       if (!is_tcg2_protocol_installed())
> +               return;
> +
> +       ret = efi_remove_protocol(efi_root, &efi_guid_tcg2_protocol,
> +                                 (void *)&efi_tcg2_protocol);
> +       if (ret != EFI_SUCCESS)
> +               log_err("Failed to remove EFI TCG2 protocol\n");
>  }
>
>  /**
> @@ -2345,12 +2362,37 @@ error:
>         return ret;
>  }
>
> +/**
> + * efi_tcg2_do_initial_measurement() - do initial measurement
> + *
> + * Return:     status code
> + */
> +efi_status_t efi_tcg2_do_initial_measurement(void)
> +{
> +       efi_status_t ret;
> +       struct udevice *dev;
> +
> +       if (!is_tcg2_protocol_installed())
> +               return EFI_SUCCESS;
> +
> +       ret = platform_get_tpm2_device(&dev);
> +       if (ret != EFI_SUCCESS)
> +               return EFI_SECURITY_VIOLATION;
> +
> +       ret = tcg2_measure_secure_boot_variable(dev);
> +       if (ret != EFI_SUCCESS)
> +               goto out;
> +
> +out:
> +       return ret;
> +}
> +
>  /**
>   * efi_tcg2_register() - register EFI_TCG2_PROTOCOL
>   *
>   * If a TPM2 device is available, the TPM TCG2 Protocol is registered
>   *
> - * Return:     An error status is only returned if adding the protocol fails.
> + * Return:     status code
>   */
>  efi_status_t efi_tcg2_register(void)
>  {
> @@ -2373,8 +2415,10 @@ efi_status_t efi_tcg2_register(void)
>         }
>
>         ret = efi_init_event_log();
> -       if (ret != EFI_SUCCESS)
> +       if (ret != EFI_SUCCESS) {
> +               tcg2_uninit();
>                 goto fail;
> +       }
>
>         ret = efi_add_protocol(efi_root, &efi_guid_tcg2_protocol,
>                                (void *)&efi_tcg2_protocol);
> @@ -2391,24 +2435,9 @@ efi_status_t efi_tcg2_register(void)
>                 goto fail;
>         }
>
> -       ret = tcg2_measure_secure_boot_variable(dev);
> -       if (ret != EFI_SUCCESS) {
> -               tcg2_uninit();
> -               goto fail;
> -       }
> -
>         return ret;
>
>  fail:
>         log_err("Cannot install EFI_TCG2_PROTOCOL\n");
> -       /*
> -        * Return EFI_SUCCESS and don't stop the EFI subsystem.
> -        * That's done for 2 reasons
> -        * - If the protocol is not installed the PCRs won't be extended.  So
> -        *   someone later in the boot flow will notice that and take the
> -        *   necessary actions.
> -        * - The TPM sandbox is limited and we won't be able to run any efi
> -        *   related tests with TCG2 enabled
> -        */
> -       return EFI_SUCCESS;
> +       return ret;
>  }
> --
> 2.17.1
>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-12-09 16:01 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-07  5:15 [PATCH v2 0/3] fix TCG2 error handling Masahisa Kojima
2021-12-07  5:15 ` [PATCH v2 1/3] efi_loader: efi_tcg2_register returns appropriate error Masahisa Kojima
2021-12-09 16:01   ` Ilias Apalodimas
2021-12-07  5:15 ` [PATCH v2 2/3] efi_loader: check tcg2 protocol installation outside the TCG protocol Masahisa Kojima
2021-12-09 15:48   ` Ilias Apalodimas
2021-12-07  5:15 ` [PATCH v2 3/3] efi_loader: correctly handle no tpm device error Masahisa Kojima
2021-12-09 15:40   ` Ilias Apalodimas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.