| Message ID | 20211125113628.29609-1-masahisa.kojima@linaro.org |
|---|---|
| State | Superseded, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | [v2] efi_loader: check tcg2 protocol installation outside the TCG protocol | expand |
Hi Kojima-san, On Thu, Nov 25, 2021 at 08:36:28PM +0900, Masahisa Kojima wrote: > +/** [...] > + * is_tcg2_protocol_installed - chech whether tcg2 protocol is installed > + * > + * @Return: true if tcg2 protocol is installed, false if not > + */ > +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) ? true : false); > +} return ret == EFI_SUCCESS; is enough here. > + > static u32 tcg_event_final_size(struct tpml_digest_values *digest_list) > { > u32 len; > @@ -962,6 +976,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_NOT_READY; > + > ret = platform_get_tpm2_device(&dev); > if (ret != EFI_SUCCESS) > return ret; > @@ -2140,6 +2157,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_NOT_READY; > + > if (tcg2_efi_app_invoked) > return EFI_SUCCESS; > > @@ -2190,6 +2210,9 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void) > efi_status_t ret; > struct udevice *dev; > > + if (!is_tcg2_protocol_installed()) [...] Heinrich, this whole patch is needed because installing the tcg2 protocol always returns EFI_SUCCESS. The reason is that some sandbox tests with sandbox_tpm used to fail. Do you want to keep this or perhaps just failing the boot now is the protocol fails to install is an option ? Thanks /Ilias
On 11/25/21 14:22, Ilias Apalodimas wrote: > Hi Kojima-san, > > On Thu, Nov 25, 2021 at 08:36:28PM +0900, Masahisa Kojima wrote: >> +/** > > [...] > >> + * is_tcg2_protocol_installed - chech whether tcg2 protocol is installed >> + * >> + * @Return: true if tcg2 protocol is installed, false if not >> + */ >> +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) ? true : false); >> +} > > return ret == EFI_SUCCESS; is enough here. > >> + >> static u32 tcg_event_final_size(struct tpml_digest_values *digest_list) >> { >> u32 len; >> @@ -962,6 +976,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_NOT_READY; >> + >> ret = platform_get_tpm2_device(&dev); >> if (ret != EFI_SUCCESS) >> return ret; >> @@ -2140,6 +2157,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_NOT_READY; >> + >> if (tcg2_efi_app_invoked) >> return EFI_SUCCESS; >> >> @@ -2190,6 +2210,9 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void) >> efi_status_t ret; >> struct udevice *dev; >> >> + if (!is_tcg2_protocol_installed()) > > [...] > > Heinrich, this whole patch is needed because installing the tcg2 protocol > always returns EFI_SUCCESS. The reason is that some sandbox tests with > sandbox_tpm used to fail. Do you want to keep this or perhaps just failing > the boot now is the protocol fails to install is an option ? Which test failed? We should consistently test the TCG2 protocol using swtpm both on QEMU and on the sandbox. I am still waiting for Tom to apply [U-BOOT-TEST-HOOKS,1/1] Enable TPMv2 emulation https://patchwork.ozlabs.org/project/uboot/patch/20211115101106.36479-1-heinrich.schuchardt@canonical.com/ to move to that target. Until then we can disable the tcg2 test or the TCG2 protocol on the sandbox. Best regards Heinrich
Hi Heinrich, [...] > > > u32 len; > > > @@ -962,6 +976,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_NOT_READY; > > > + > > > ret = platform_get_tpm2_device(&dev); > > > if (ret != EFI_SUCCESS) > > > return ret; > > > @@ -2140,6 +2157,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_NOT_READY; > > > + > > > if (tcg2_efi_app_invoked) > > > return EFI_SUCCESS; > > > > > > @@ -2190,6 +2210,9 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void) > > > efi_status_t ret; > > > struct udevice *dev; > > > > > > + if (!is_tcg2_protocol_installed()) > > > > [...] > > > > Heinrich, this whole patch is needed because installing the tcg2 protocol > > always returns EFI_SUCCESS. The reason is that some sandbox tests with > > sandbox_tpm used to fail. Do you want to keep this or perhaps just failing > > the boot now is the protocol fails to install is an option ? > > Which test failed? It's been a while, but if my memory serves me correctly, during the protocol installation we need to call: efi_init_event_log() -> create_specid_event() -> tpm2_get_pcr_info() -> tpm2_get_capability(). That get_capability call wasn't supported in sandbox. So the result was EFI TCG2 stopping the boot process. Simon did fix a few things on sandbox since then, but I can't remember if capabilities was one of them. > > We should consistently test the TCG2 protocol using swtpm both on QEMU > and on the sandbox. I am still waiting for Tom to apply > > [U-BOOT-TEST-HOOKS,1/1] Enable TPMv2 emulation > https://patchwork.ozlabs.org/project/uboot/patch/20211115101106.36479-1-heinrich.schuchardt@canonical.com/ > > to move to that target. > > Until then we can disable the tcg2 test or the TCG2 protocol on the sandbox. That would be fine by me. Not stopping the boot on failures introduces the need for patches like this. So you suggest we drop this and just fail the boot ? Thanks /Ilias > > Best regards > > Heinrich
On 11/25/21 21:40, Ilias Apalodimas wrote: > Hi Heinrich, > > [...] > >>>> u32 len; >>>> @@ -962,6 +976,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_NOT_READY; >>>> + >>>> ret = platform_get_tpm2_device(&dev); >>>> if (ret != EFI_SUCCESS) >>>> return ret; >>>> @@ -2140,6 +2157,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_NOT_READY; >>>> + >>>> if (tcg2_efi_app_invoked) >>>> return EFI_SUCCESS; >>>> >>>> @@ -2190,6 +2210,9 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void) >>>> efi_status_t ret; >>>> struct udevice *dev; >>>> >>>> + if (!is_tcg2_protocol_installed()) >>> >>> [...] >>> >>> Heinrich, this whole patch is needed because installing the tcg2 protocol >>> always returns EFI_SUCCESS. The reason is that some sandbox tests with >>> sandbox_tpm used to fail. Do you want to keep this or perhaps just failing >>> the boot now is the protocol fails to install is an option ? >> >> Which test failed? > > It's been a while, but if my memory serves me correctly, during the > protocol installation we need to call: > efi_init_event_log() -> create_specid_event() -> tpm2_get_pcr_info() -> > tpm2_get_capability(). > > That get_capability call wasn't supported in sandbox. So the result was > EFI TCG2 stopping the boot process. Simon did fix a few things on sandbox > since then, but I can't remember if capabilities was one of them. > >> >> We should consistently test the TCG2 protocol using swtpm both on QEMU >> and on the sandbox. I am still waiting for Tom to apply >> >> [U-BOOT-TEST-HOOKS,1/1] Enable TPMv2 emulation >> https://patchwork.ozlabs.org/project/uboot/patch/20211115101106.36479-1-heinrich.schuchardt@canonical.com/ >> >> to move to that target. >> >> Until then we can disable the tcg2 test or the TCG2 protocol on the sandbox. > > That would be fine by me. Not stopping the boot on failures introduces the > need for patches like this. So you suggest we drop this and just fail the > boot ? If the sandbox makes problems due to its incomplete TPM emulation I would suggest: diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index 700dc838dd..201a0d62e2 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -307,7 +307,7 @@ config EFI_RNG_PROTOCOL config EFI_TCG2_PROTOCOL bool "EFI_TCG2_PROTOCOL support" default y - depends on TPM_V2 + depends on TPM_V2 && !SANDBOX We can revert such a change once swtpm can be used to provide a tpm emulation for the sandbox. Best regards Heinrich
diff --git a/include/efi_loader.h b/include/efi_loader.h index d52e399841..fe29c10906 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -525,6 +525,10 @@ 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 the required setup for the measurement */ +efi_status_t efi_tcg2_setup_measurement(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/efi_setup.c b/lib/efi_loader/efi_setup.c index a2338d74af..781d10590d 100644 --- a/lib/efi_loader/efi_setup.c +++ b/lib/efi_loader/efi_setup.c @@ -271,6 +271,9 @@ efi_status_t efi_init_obj_list(void) ret = efi_tcg2_register(); if (ret != EFI_SUCCESS) goto out; + + efi_tcg2_setup_measurement(); + efi_tcg2_do_initial_measurement(); } /* Secure boot */ diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index b44eed0ec9..0a33f5ab5f 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -153,6 +153,20 @@ static u16 alg_to_len(u16 hash_alg) return 0; } +/** + * is_tcg2_protocol_installed - chech whether tcg2 protocol is installed + * + * @Return: true if tcg2 protocol is installed, false if not + */ +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) ? true : false); +} + static u32 tcg_event_final_size(struct tpml_digest_values *digest_list) { u32 len; @@ -962,6 +976,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_NOT_READY; + ret = platform_get_tpm2_device(&dev); if (ret != EFI_SUCCESS) return ret; @@ -2140,6 +2157,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_NOT_READY; + if (tcg2_efi_app_invoked) return EFI_SUCCESS; @@ -2190,6 +2210,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_NOT_READY; + ret = platform_get_tpm2_device(&dev); if (ret != EFI_SUCCESS) return ret; @@ -2214,6 +2237,11 @@ efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context) EFI_ENTRY("%p, %p", event, context); + if (!is_tcg2_protocol_installed()) { + ret = EFI_NOT_READY; + goto out; + } + event_log.ebs_called = true; ret = platform_get_tpm2_device(&dev); if (ret != EFI_SUCCESS) @@ -2244,6 +2272,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_NOT_READY; + ret = platform_get_tpm2_device(&dev); if (ret != EFI_SUCCESS) goto out; @@ -2313,6 +2344,45 @@ error: return ret; } +/** + * efi_tcg2_setup_measurement() - setup for the measurement + * + * Return: status code + */ +efi_status_t efi_tcg2_setup_measurement(void) +{ + efi_status_t ret; + struct efi_event *event; + + ret = efi_create_event(EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK, + efi_tcg2_notify_exit_boot_services, NULL, + NULL, &event); + + 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; + + ret = platform_get_tpm2_device(&dev); + if (ret != EFI_SUCCESS) + goto out; + + ret = tcg2_measure_secure_boot_variable(dev); + if (ret != EFI_SUCCESS) + goto out; + +out: + return ret; +} + /** * efi_tcg2_register() - register EFI_TCG2_PROTOCOL * @@ -2324,7 +2394,6 @@ efi_status_t efi_tcg2_register(void) { efi_status_t ret = EFI_SUCCESS; struct udevice *dev; - struct efi_event *event; u32 err; ret = platform_get_tpm2_device(&dev); @@ -2344,6 +2413,10 @@ efi_status_t efi_tcg2_register(void) if (ret != EFI_SUCCESS) goto fail; + /* + * efi_add_protocol() is called at last on purpose. + * tcg2_uninit() does not uninstall the tcg2 protocol, but it is intended. + */ ret = efi_add_protocol(efi_root, &efi_guid_tcg2_protocol, (void *)&efi_tcg2_protocol); if (ret != EFI_SUCCESS) { @@ -2351,20 +2424,6 @@ 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(); - goto fail; - } - return ret; fail:
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. Together with above change, this commit also removes the unnecessary tcg2_uninit() call in efi_tcg2_register(), refactors efi_tcg2_register() not to include the process that is not related to the tcg2 protocol registration. Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> --- Changes in v2: - rebased on top of the TF-A eventlog handover support include/efi_loader.h | 4 ++ lib/efi_loader/efi_setup.c | 3 ++ lib/efi_loader/efi_tcg2.c | 89 +++++++++++++++++++++++++++++++------- 3 files changed, 81 insertions(+), 15 deletions(-)