Attention is currently required from: Christian Walter, Filip Lewiński, Julius Werner.
Michał Żygowski has posted comments on this change by Filip Lewiński. ( https://review.coreboot.org/c/coreboot/+/82695?usp=email )
Change subject: security/intel/txt: Handle TPM properly when vboot enabled ......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
Please justify what you're doing here and why?
I simply wanted to have vboot working with Intel TXT. The commit message is just beyond the state of poor.
I don't think making vboot play nice with INIT_BOOTBLOCK is as simple as turning on IGNORE_POSTINIT. IGNORE_POSTINIT is a hack and not really a secure thing to use by default — there are good reasons why you want to make sure the TPM was properly reset on boot. Also, you're still running tpm_setup() twice, that's still not good, even if you ignore the error that makes it fail.
Right, but wasn't it the same also for the case when tpm_setup is run in verstage (for antirollback) and then in ramstage again?
You're calling tspi_measure_cache_to_pcr() again and end up adding all those entries twice.
I agree, this is something that needs some handling. How about a check like this:
``` if (CONFIG(TPM_MEASURED_BOOT_INIT_BOOTBLOCK) && err == IGNORE_POSTINIT) <do not report error and return from tpm_setup with success skipping cache to PCR measurement> ```