Re: [PATCH 1/3] efi_loader: efi_tcg2_register returns appropriate error

On Fri, Dec 03, 2021 at 12:58:13PM +0900, Masahisa Kojima 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

include/efi_loader.h | 2 ++ lib/efi_loader/Kconfig | 2 ++ lib/efi_loader/efi_setup.c | 2 ++ lib/efi_loader/efi_tcg2.c | 65 +++++++++++++++++++++++++++----------- 4 files changed, 53 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..f58a4afa7f 100644 --- a/lib/efi_loader/efi_setup.c +++ b/lib/efi_loader/efi_setup.c @@ -241,6 +241,8 @@ efi_status_t efi_init_obj_list(void) ret = efi_tcg2_register(); if (ret != EFI_SUCCESS) goto out;

• efi_tcg2_do_initial_measurement();
  

  }

  /* Secure boot */

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index 5f71b188a0..6dbdd35f29 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)

• goto out;
  

Would it make more sense to return a security violation here and treat this error similarly to patch [3/3]?

• 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

Cheers /Ilias