diff mbox series

[v2,1/3] efi_loader: efi_tcg2_register returns appropriate error

Message ID 20211207051533.5597-2-masahisa.kojima@linaro.org
State Accepted
Commit 54bec17f6b0326bbc22f993d28170d4c4df4ceed
Headers show
Series fix TCG2 error handling | expand

Commit Message

Masahisa Kojima Dec. 7, 2021, 5:15 a.m. UTC
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(-)

Comments

Ilias Apalodimas Dec. 9, 2021, 4:01 p.m. UTC | #1
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>
diff mbox series

Patch

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;
 }